Merge branch 'jk/run-command-eacces'

When PATH contains an unreadable directory, alias expansion code did not
kick in, and failed with an error that said "git-subcmd" was not found.

By Jeff King (1) and Ramsay Jones (1)
* jk/run-command-eacces:
  run-command: treat inaccessible directories as ENOENT
  compat/mingw.[ch]: Change return type of exec functions to int

6 files changed, 86 insertions, 7 deletions

diff --git a/cache.h b/cache.h
index 806bf2b76f..5e6eef9b4d 100644
--- a/cache.h
+++ b/cache.h
@@ -1305,4 +1305,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/compat/mingw.c b/compat/mingw.c
index a0ac487c0c..afc892d6b1 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1003,7 +1003,7 @@ static void mingw_execve(const char *cmd, char *const *argv, char *const *env)
 	}
 }
 
-void mingw_execvp(const char *cmd, char *const *argv)
+int mingw_execvp(const char *cmd, char *const *argv)
 {
 	char **path = get_path_split();
 	char *prog = path_lookup(cmd, path, 0);
@@ -1015,11 +1015,13 @@ void mingw_execvp(const char *cmd, char *const *argv)
 		errno = ENOENT;
 
 	free_path_split(path);
+	return -1;
 }
 
-void mingw_execv(const char *cmd, char *const *argv)
+int mingw_execv(const char *cmd, char *const *argv)
 {
 	mingw_execve(cmd, argv, environ);
+	return -1;
 }
 
 int mingw_kill(pid_t pid, int sig)
diff --git a/compat/mingw.h b/compat/mingw.h
index 0ff1e04812..ef5b15014e 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -274,9 +274,9 @@ int mingw_utime(const char *file_name, const struct utimbuf *times);
 pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
 		     const char *dir,
 		     int fhin, int fhout, int fherr);
-void mingw_execvp(const char *cmd, char *const *argv);
+int mingw_execvp(const char *cmd, char *const *argv);
 #define execvp mingw_execvp
-void mingw_execv(const char *cmd, char *const *argv);
+int mingw_execv(const char *cmd, char *const *argv);
 #define execv mingw_execv
 
 static inline unsigned int git_ntohl(unsigned int x)
diff --git a/exec_cmd.c b/exec_cmd.c
index 171e841531..125fa6fabf 100644
--- 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 2af3e0fa52..5be1b4b5ba 100644
--- a/run-command.c
+++ b/run-command.c
@@ -80,6 +80,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;
@@ -118,7 +180,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;
 }
@@ -343,7 +405,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 8d4938f019..17e969df60 100755
--- 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