Brandon Williams wrote:
> In some situations run-command will incorrectly try (and fail) to
> execute a directory instead of an executable file. This was observed by
> having a directory called "ssh" in $PATH before the real ssh and trying
> to use ssh protoccol, reslting in the following:
>
> $ git ls-remote ssh://url
> fatal: cannot exec 'ssh': Permission denied
>
> It ends up being worse and run-command will even try to execute a
> non-executable file if it preceeds the executable version of a file on
> the PATH. For example, if PATH=~/bin1:~/bin2:~/bin3 and there exists a
> directory 'git-hello' in 'bin1', a non-executable file 'git-hello' in
> bin2 and an executable file 'git-hello' (which prints "Hello World!") in
> bin3 the following will occur:
>
> $ git hello
> fatal: cannot exec 'git-hello': Permission denied
>
> This is due to only checking 'access()' when locating an executable in
> PATH, which doesn't distinguish between files and directories. Instead
> use 'is_executable()' which check that the path is to a regular,
> executable file. Now run-command won't try to execute the directory or
> non-executable file 'git-hello':
>
> $ git hello
> Hello World!
>
> Reported-by: Brian Hatfield <bhatfield@xxxxxxxxxx>
> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx>
> ---
> run-command.c | 19 ++++++++++++++++++-
> t/t0061-run-command.sh | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+), 1 deletion(-)
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks. Patch left unsnipped for reference.
> diff --git a/run-command.c b/run-command.c
> index 2ffbd7e67..9e36151bf 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -159,6 +159,23 @@ int is_executable(const char *name)
> return st.st_mode & S_IXUSR;
> }
>
> +/*
> + * Search $PATH for a command. This emulates the path search that
> + * execvp would perform, without actually executing the command so it
> + * can be used before fork() to prepare to run a command using
> + * execve() or after execvp() to diagnose why it failed.
> + *
> + * The caller should ensure that file contains no directory
> + * separators.
> + *
> + * Returns the path to the command, as found in $PATH or NULL if the
> + * command could not be found. The caller inherits ownership of the memory
> + * used to store the resultant path.
> + *
> + * This should not be used on Windows, where the $PATH search rules
> + * are more complicated (e.g., a search for "foo" should find
> + * "foo.exe").
> + */
> static char *locate_in_PATH(const char *file)
> {
> const char *p = getenv("PATH");
> @@ -179,7 +196,7 @@ static char *locate_in_PATH(const char *file)
> }
> strbuf_addstr(&buf, file);
>
> - if (!access(buf.buf, F_OK))
> + if (is_executable(buf.buf))
> return strbuf_detach(&buf, NULL);
>
> if (!*end)
> diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
> index 98c09dd98..e4739170a 100755
> --- a/t/t0061-run-command.sh
> +++ b/t/t0061-run-command.sh
> @@ -37,6 +37,36 @@ test_expect_success !MINGW 'run_command can run a script without a #! line' '
> test_cmp empty err
> '
>
> +test_expect_success 'run_command does not try to execute a directory' '
> + test_when_finished "rm -rf bin1 bin2" &&
> + mkdir -p bin1/greet bin2 &&
> + write_script bin2/greet <<-\EOF &&
> + cat bin2/greet
> + EOF
> +
> + PATH=$PWD/bin1:$PWD/bin2:$PATH \
> + test-run-command run-command greet >actual 2>err &&
> + test_cmp bin2/greet actual &&
> + test_cmp empty err
> +'
> +
> +test_expect_success POSIXPERM 'run_command passes over non-executable file' '
> + test_when_finished "rm -rf bin1 bin2" &&
> + mkdir -p bin1 bin2 &&
> + write_script bin1/greet <<-\EOF &&
> + cat bin1/greet
> + EOF
> + chmod -x bin1/greet &&
> + write_script bin2/greet <<-\EOF &&
> + cat bin2/greet
> + EOF
> +
> + PATH=$PWD/bin1:$PWD/bin2:$PATH \
> + test-run-command run-command greet >actual 2>err &&
> + test_cmp bin2/greet actual &&
> + test_cmp empty err
> +'
> +
> test_expect_success POSIXPERM 'run_command reports EACCES' '
> cat hello-script >hello.sh &&
> chmod -x hello.sh &&