run-command: treat inaccessible directories as ENOENT

When execvp reports EACCES, it can be one of two things:

  1. We found a file to execute, but did not have
     permissions to do so.

  2. We did not have permissions to look in some directory
     in the $PATH.

In the former case, we want to consider this a
permissions problem and report it to the user as such (since
getting this for something like "git foo" is likely a
configuration error).

In the latter case, there is a good chance that the
inaccessible directory does not contain anything of
interest. Reporting "permission denied" is confusing to the
user (and prevents our usual "did you mean...?" lookup). It
also prevents git from trying alias lookup, since we do so
only when an external command does not exist (not when it
exists but has an error).

This patch detects EACCES from execvp, checks whether we are
in case (2), and if so converts errno to ENOENT. This
behavior matches that of "bash" (but not of simpler shells
that use execvp more directly, like "dash").

Test stolen from Junio.

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

diff --git a/cache.h b/cache.h
index 10afd71d435920a3cc2152208a06058791ea7a04..5067226c2f70743a7ff61ec136501b829920f250 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1261,4 +1261,6 @@ extern struct startup_info *startup_info;
 /* builtin/merge.c */
 int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
 
+int sane_execvp(const char *file, char *const argv[]);
+
 #endif /* CACHE_H */

diff --git a/exec_cmd.c b/exec_cmd.c
index 171e841531de7fd5b51aa26f639104382395b854..125fa6fabf503d29cb82b2ccbc92359abf95b0e8 100644 (file)
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -134,7 +134,7 @@ int execv_git_cmd(const char **argv) {
        trace_argv_printf(nargv, "trace: exec:");
 
        /* execvp() can only ever return if it fails */
-       execvp("git", (char **)nargv);
+       sane_execvp("git", (char **)nargv);
 
        trace_printf("trace: exec failed: %s\n", strerror(errno));
 

diff --git a/run-command.c b/run-command.c
index 1c5104388429e50722769358d5d89948c55bb3aa..805d41f93d66b9147b837fb51f365e85f8f44fda 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -18,6 +18,68 @@ static inline void dup_devnull(int to)
 }
 #endif
 
+static char *locate_in_PATH(const char *file)
+{
+       const char *p = getenv("PATH");
+       struct strbuf buf = STRBUF_INIT;
+
+       if (!p || !*p)
+               return NULL;
+
+       while (1) {
+               const char *end = strchrnul(p, ':');
+
+               strbuf_reset(&buf);
+
+               /* POSIX specifies an empty entry as the current directory. */
+               if (end != p) {
+                       strbuf_add(&buf, p, end - p);
+                       strbuf_addch(&buf, '/');
+               }
+               strbuf_addstr(&buf, file);
+
+               if (!access(buf.buf, F_OK))
+                       return strbuf_detach(&buf, NULL);
+
+               if (!*end)
+                       break;
+               p = end + 1;
+       }
+
+       strbuf_release(&buf);
+       return NULL;
+}
+
+static int exists_in_PATH(const char *file)
+{
+       char *r = locate_in_PATH(file);
+       free(r);
+       return r != NULL;
+}
+
+int sane_execvp(const char *file, char * const argv[])
+{
+       if (!execvp(file, argv))
+               return 0; /* cannot happen ;-) */
+
+       /*
+        * When a command can't be found because one of the directories
+        * listed in $PATH is unsearchable, execvp reports EACCES, but
+        * careful usability testing (read: analysis of occasional bug
+        * reports) reveals that "No such file or directory" is more
+        * intuitive.
+        *
+        * We avoid commands with "/", because execvp will not do $PATH
+        * lookups in that case.
+        *
+        * The reassignment of EACCES to errno looks like a no-op below,
+        * but we need to protect against exists_in_PATH overwriting errno.
+        */
+       if (errno == EACCES && !strchr(file, '/'))
+               errno = exists_in_PATH(file) ? EACCES : ENOENT;
+       return -1;
+}
+
 static const char **prepare_shell_cmd(const char **argv)
 {
        int argc, nargc = 0;
@@ -56,7 +118,7 @@ static int execv_shell_cmd(const char **argv)
 {
        const char **nargv = prepare_shell_cmd(argv);
        trace_argv_printf(nargv, "trace: exec:");
-       execvp(nargv[0], (char **)nargv);
+       sane_execvp(nargv[0], (char **)nargv);
        free(nargv);
        return -1;
 }
@@ -278,7 +340,7 @@ fail_pipe:
                } else if (cmd->use_shell) {
                        execv_shell_cmd(cmd->argv);
                } else {
-                       execvp(cmd->argv[0], (char *const*) cmd->argv);
+                       sane_execvp(cmd->argv[0], (char *const*) cmd->argv);
                }
                if (errno == ENOENT) {
                        if (!cmd->silent_exec_failure)

diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 8d4938f019ca406c0f248d19d046356dff1ac09a..17e969df609f71b0b4562cff8fda112632d27442 100755 (executable)
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -34,4 +34,17 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
        grep "fatal: cannot exec.*hello.sh" err
 '
 
+test_expect_success POSIXPERM 'unreadable directory in PATH' '
+       mkdir local-command &&
+       test_when_finished "chmod u+rwx local-command && rm -fr local-command" &&
+       git config alias.nitfol "!echo frotz" &&
+       chmod a-rx local-command &&
+       (
+               PATH=./local-command:$PATH &&
+               git nitfol >actual
+       ) &&
+       echo frotz >expect &&
+       test_cmp expect actual
+'
+
 test_done