Re: [PATCH v2] git-gui - re-enable use of hook scripts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sat, 16 Sep 2023, Mark Levedahl wrote:

> Earlier, commit aae9560a introduced search in $PATH to find executables
> before running them, avoiding an issue where on Windows a same named
> file in the current directory can be executed in preference to anything
> in a directory in $PATH. This search is intended to find an absolute
> path for a bare executable ( e.g, a function "foo") by finding the first
> instance of "foo" in a directory given in $PATH, and this search works
> correctly.  The search is explicitly avoided for an executable named
> with an absolute path (e.g., /bin/sh), and that works as well.
>
> Unfortunately, the search is also applied to commands named with a
> relative path. A hook script (or executable) $HOOK is usually located
> relative to the project directory as .git/hooks/$HOOK. The search for
> this will generally fail as that relative path will (probably) not exist
> on any directory in $PATH. This means that git hooks in general now fail
> to run. Considerable mayhem could occur should a directory on $PATH be
> git controlled. If such a directory includes .git/hooks/$HOOK, that
> repository's $HOOK will be substituted for the one in the current
> project, with unknown consequences.
>
> This lookup failure also occurs in worktrees linked to a remote .git
> directory using git-new-workdir. However, a worktree using a .git file
> pointing to a separate git directory apparently avoids this: in that
> case the hook command is resolved to an absolute path before being
> passed down to the code introduced in aae9560a.
>
> Fix this by replacing the test for an "absolute" pathname to a check for
> a command name having more than one pathname component. This limits the
> search and absolute pathname resolution to bare commands. The new test
> uses tcl's "file split" command. Experiments on Linux and Windows, using
> tclsh, show that command names with relative and absolute paths always
> give at least two components, while a bare command gives only one.
>
> 	  Linux:   puts [file split {foo}]       ==>  foo
> 	  Linux:   puts [file split {/foo}]      ==>  / foo
> 	  Linux:   puts [file split {.git/foo}]  ==> .git foo
> 	  Windows: puts [file split {foo}]       ==>  foo
> 	  Windows: puts [file split {c:\foo}]    ==>  c:/ foo
> 	  Windows: puts [file split {.git\foo}]  ==> .git foo
>
> The above results show the new test limits search and replacement
> to bare commands on both Linux and Windows.

Sounds good. FWIW I ran a couple experiments here, too:

	% file pathtype "C:/foo"
	absolute
	% file pathtype ".git/hooks"
	relative
	% file pathtype ".git\\hooks"
	relative
	% file pathtype "/foo"
	volumerelative
	% file pathtype "foo"
	relative

The problem, therefore, is that `file pathtype` does not discern between a
bare file name and a relative path. The proposed patch looks correct to
me.

Thank you,
Johannes

>
> Signed-off-by: Mark Levedahl <mlevedahl@xxxxxxxxx>
> ---
>  git-gui.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 8bc8892..8603437 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -118,7 +118,7 @@ proc sanitize_command_line {command_line from_index} {
>  	set i $from_index
>  	while {$i < [llength $command_line]} {
>  		set cmd [lindex $command_line $i]
> -		if {[file pathtype $cmd] ne "absolute"} {
> +		if {[llength [file split $cmd]] < 2} {
>  			set fullpath [_which $cmd]
>  			if {$fullpath eq ""} {
>  				throw {NOT-FOUND} "$cmd not found in PATH"
> --
> 2.41.0.99.19
>
>




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux