run-command: mark path lookup errors with ENOENT

Since commit e3a434468f (run-command: use the
async-signal-safe execv instead of execvp, 2017-04-19),
prepare_cmd() does its own PATH lookup for any commands we
run (on non-Windows platforms).

However, its logic does not match the old execvp call when
we fail to find a matching entry in the PATH. Instead of
feeding the name directly to execv, execvp would consider
that an ENOENT error. By continuing and passing the name
directly to execv, we effectively behave as if "." was
included at the end of the PATH. This can have confusing and
even dangerous results.

The fix itself is pretty straight-forward. There's a new
test in t0061 to cover this explicitly, and I've also added
a duplicate of the ENOENT test to ensure that we return the
correct errno for this case.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/run-command.c b/run-command.c
index 014b2165b5a2f92ff18869effafd4c6df01b33eb..8d42a4f534f7ad0a1e06d5ce898b114253f06b18 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -378,7 +378,7 @@ static void child_err_spew(struct child_process *cmd, struct child_err *cerr)
        set_error_routine(old_errfn);
 }
 
-static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
+static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)
 {
        if (!cmd->argv[0])
                die("BUG: command is empty");
@@ -401,16 +401,22 @@ static void prepare_cmd(struct argv_array *out, const struct child_process *cmd)
        /*
         * If there are no '/' characters in the command then perform a path
         * lookup and use the resolved path as the command to exec.  If there
-        * are no '/' characters or if the command wasn't found in the path,
-        * have exec attempt to invoke the command directly.
+        * are '/' characters, we have exec attempt to invoke the command
+        * directly.
         */
        if (!strchr(out->argv[1], '/')) {
                char *program = locate_in_PATH(out->argv[1]);
                if (program) {
                        free((char *)out->argv[1]);
                        out->argv[1] = program;
+               } else {
+                       argv_array_clear(out);
+                       errno = ENOENT;
+                       return -1;
                }
        }
+
+       return 0;
 }
 
 static char **prep_childenv(const char *const *deltaenv)
@@ -635,6 +641,12 @@ int start_command(struct child_process *cmd)
        struct child_err cerr;
        struct atfork_state as;
 
+       if (prepare_cmd(&argv, cmd) < 0) {
+               failed_errno = errno;
+               cmd->pid = -1;
+               goto end_of_spawn;
+       }
+
        if (pipe(notify_pipe))
                notify_pipe[0] = notify_pipe[1] = -1;
 
@@ -645,7 +657,6 @@ int start_command(struct child_process *cmd)
                set_cloexec(null_fd);
        }
 
-       prepare_cmd(&argv, cmd);
        childenv = prep_childenv(cmd->env);
        atfork_prepare(&as);
 
@@ -773,6 +784,8 @@ int start_command(struct child_process *cmd)
        argv_array_clear(&argv);
        free(childenv);
 }
+end_of_spawn:
+
 #else
 {
        int fhin = 0, fhout = 1, fherr = 2;

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index e4739170aa2b7c833cd51a06a7ac6764c8bf0494..0303ddbb6440f0f8ef8051f390447f9406694239 100755 (executable)
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -13,10 +13,14 @@ cat >hello-script <<-EOF
 EOF
 >empty
 
-test_expect_success 'start_command reports ENOENT' '
+test_expect_success 'start_command reports ENOENT (slash)' '
        test-run-command start-command-ENOENT ./does-not-exist
 '
 
+test_expect_success 'start_command reports ENOENT (no slash)' '
+       test-run-command start-command-ENOENT does-not-exist
+'
+
 test_expect_success 'run_command can run a command' '
        cat hello-script >hello.sh &&
        chmod +x hello.sh &&
@@ -26,6 +30,13 @@ test_expect_success 'run_command can run a command' '
        test_cmp empty err
 '
 
+test_expect_success 'run_command is restricted to PATH' '
+       write_script should-not-run <<-\EOF &&
+       echo yikes
+       EOF
+       test_must_fail test-run-command run-command should-not-run
+'
+
 test_expect_success !MINGW 'run_command can run a script without a #! line' '
        cat >hello <<-\EOF &&
        cat hello-script