Re: [PATCH] run-command.c: Accept EACCES as command not found

@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "argv-array.h"
+#include "dir.h"
 
 static inline void close_pair(int fd[2])
 {

@@ -134,6 +135,119 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
 	return code;
 }
 
+#ifndef WIN32
+static int is_in_group(gid_t gid)
+{
+	gid_t *groups;
+	int ngroups, gc;
+	int yes;
+
+	if (gid == getgid())
+		return 1;
+
+	groups = NULL;
+	ngroups = getgroups(0, NULL);
+	if (ngroups > 0) {
+		groups = (gid_t *)xmalloc(ngroups * sizeof(gid_t));
+		if (getgroups(ngroups, groups) < 0) {
+			free(groups);
+			return 0;
+		}
+	}
+
+	yes = 0;
+	for (gc = 0; gc < ngroups; gc++)
+		if (groups[gc] == gid)
+			yes = 1;
+
+	free(groups);
+	return yes;
+}
+
+static int have_read_execute_permissions(const char *path)
+{
+	struct stat s;
+	trace_printf("checking '%s'\n", path);
+
+	if (stat(path, &s) < 0) {
+		trace_printf("could not stat '%s': %s\n",
+				path, strerror(errno));
+		return 0;
+	}
+	trace_printf("uid: %d, gid: %d\n", s.st_uid, s.st_gid);
+	trace_printf("mode: %o\n", s.st_mode);
+
+	/* check world permissions */
+	if ((s.st_mode&(S_IXOTH|S_IROTH)) == (S_IXOTH|S_IROTH))
+		return 1;
+
+	/* check group permissions & membership */
+	if ((s.st_mode&(S_IXGRP|S_IRGRP)) == (S_IXGRP|S_IRGRP) &&
+		is_in_group(s.st_gid))
+		return 1;
+
+	/* check owner permissions & ownership */
+	if ((s.st_mode&(S_IXUSR|S_IRUSR)) == (S_IXUSR|S_IRUSR) &&
+		s.st_uid == getuid())
+		return 1;
+
+	return 0;
+}
+
+static void diagnose_execvp_eacces(const char *cmd, const char **argv)
+{
+	/* man 2 execve states that EACCES is returned for:
+	 * - Search permission is denied on a component of the path prefix
+	 *   of cmd or the name of a script interpreter
+	 * - The file or script interpreter is not a regular file
+	 * - Execute permission is denied for the file, script or ELF
+	 *   interpreter
+	 * - The file system is mounted noexec
+	 */
+	struct strbuf sb = STRBUF_INIT;
+	char *path = getenv("PATH");
+	char *next;
+
+	if (strchr(cmd, '/')) {
+		if (!have_read_execute_permissions(cmd))
+			error("no read/execute permissions on '%s'\n", cmd);
+		return;
+	}
+
+	for (;;) {
+		next = strchrnul(path, ':');
+		if (path < next)
+			strbuf_add(&sb, path, next - path);
+		else
+			strbuf_addch(&sb, '.');
+
+		if (!have_read_execute_permissions(sb.buf))
+			error("no read/execute permissions on '%s'\n", sb.buf);
+
+		if (sb.len && sb.buf[sb.len - 1] != '/')
+			strbuf_addch(&sb, '/');
+		strbuf_addstr(&sb, cmd);
+
+		if (file_exists(sb.buf)) {
+			if (!have_read_execute_permissions(sb.buf))
+				error("no read/execute permissions on '%s'\n",
+						sb.buf);
+			else
+				warn("file '%s' exists and permissions "
+				"seem OK.\nIf this is a script, see if you "
+				"have sufficient privileges to run the "
+				"interpreter", sb.buf);
+		}
+
+		strbuf_release(&sb);
+
+		if (!*next)
+			break;
+		path = next + 1;
+	}
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;

@@ -285,6 +399,10 @@ fail_pipe:
 				error("cannot run %s: %s", cmd->argv[0],
 					strerror(ENOENT));
 			exit(127);
+		} else if (errno == EACCES) {
+			diagnose_execvp_eacces(cmd->argv[0], cmd->argv);
+			die("cannot exec '%s': %s", cmd->argv[0],
+				strerror(EACCES));
 		} else {
 			die_errno("cannot exec '%s'", cmd->argv[0]);
 		}

@@ -26,7 +26,7 @@ test_expect_success 'run_command can run a command' '
 	test_cmp empty err
 '
 
-test_expect_success POSIXPERM 'run_command reports EACCES' '
+test_expect_success POSIXPERM 'run_command reports EACCES, file permissions' '
 	cat hello-script >hello.sh &&
 	chmod -x hello.sh &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&

@@ -34,4 +34,18 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
 	grep "fatal: cannot exec.*hello.sh" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, search path permisions' '
+	mkdir -p inaccessible &&
+	PATH=$(pwd)/inaccessible:$PATH &&
+	export PATH &&
+
+	cat hello-script >inaccessible/hello.sh &&
+	chmod 400 inaccessible &&
+	test_must_fail test-run-command run-command hello.sh 2>err &&
+	chmod 755 inaccessible &&
+
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "no read/execute permissions on" err
+'
+
 test_done

@@ -194,6 +194,63 @@ static int have_read_execute_permissions(const char *path)
 	return 0;
 }
 
+static void check_interpreter(const char *cmd)
+{
+	FILE *f;
+	struct strbuf sb = STRBUF_INIT;
+	/* bash reads an 80 character line when determining the interpreter.
+	 * BSD apparently only allows 32 characters, as it is the size of
+	 * your average binary executable header.
+	 */
+	char firstline[80];
+	char *interpreter = NULL;
+	size_t s, i;
+
+	f = fopen(cmd, "r");
+	if (!f) {
+		error("cannot open file '%s': %s\n", cmd, strerror(errno));
+		return;
+	}
+
+	s = fread(firstline, 1, sizeof(firstline), f);
+	if (s < 2) {
+		trace_printf("cannot determine file type");
+		fclose(f);
+		return;
+	}
+
+	if (firstline[0] != '#' || firstline[1] != '!') {
+		trace_printf("file '%s' is not a script or"
+				" is a script without '#!'", cmd);
+		fclose(f);
+		return;
+	}
+
+	/* see if the given path has the executable bit set */
+	for (i = 2; i < s; i++) {
+		if (!interpreter && firstline[i] != ' ' && firstline[i] != '\t')
+			interpreter = firstline + i;
+
+		if (interpreter && (firstline[i] == ' ' ||
+				firstline[i] == '\n')) {
+			strbuf_add(&sb, interpreter,
+					(firstline + i) - interpreter);
+			break;
+		}
+	}
+	if (!sb.len) {
+		error("could not determine interpreter");
+		strbuf_release(&sb);
+		return;
+	}
+
+	if (!have_read_execute_permissions(sb.buf))
+		error("bad interpreter: no read/execute permissions on '%s'\n",
+				sb.buf);
+
+	strbuf_release(&sb);
+}
+
 static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 {
 	/* man 2 execve states that EACCES is returned for:

@@ -209,8 +266,8 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 	char *next;
 
 	if (strchr(cmd, '/')) {
-		if (!have_read_execute_permissions(cmd))
-			error("no read/execute permissions on '%s'\n", cmd);
+		if (have_read_execute_permissions(cmd))
+			check_interpreter(cmd);
 		return;
 	}
 

@@ -233,10 +290,7 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 				error("no read/execute permissions on '%s'\n",
 						sb.buf);
 			else
-				warn("file '%s' exists and permissions "
-				"seem OK.\nIf this is a script, see if you "
-				"have sufficient privileges to run the "
-				"interpreter", sb.buf);
+				check_interpreter(sb.buf);
 		}
 
 		strbuf_release(&sb);

@@ -13,6 +13,18 @@ cat >hello-script <<-EOF
 EOF
 >empty
 
+cat >someinterpreter <<-EOF
+	#!$SHELL_PATH
+	cat hello-script
+EOF
+>empty
+
+cat >incorrect-interpreter-script <<-EOF
+	#!someinterpreter
+	cat hello-script
+EOF
+>empty
+
 test_expect_success 'start_command reports ENOENT' '
 	test-run-command start-command-ENOENT ./does-not-exist
 '

@@ -48,4 +60,14 @@ test_expect_success POSIXPERM 'run_command reports EACCES, search path permision
 	grep "no read/execute permissions on" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
+	cat incorrect-interpreter-script >hello.sh &&
+	chmod +x hello.sh &&
+	chmod -x someinterpreter &&
+	test_must_fail test-run-command run-command ./hello.sh 2>err &&
+
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
+'
+
 test_done

@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "argv-array.h"
+#include "dir.h"
 
 static inline void close_pair(int fd[2])
 {

@@ -134,6 +135,80 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
 	return code;
 }
 
+#ifndef WIN32
+static int have_read_execute_permissions(const char *path)
+{
+	if (access(path, R_OK|X_OK) == 0)
+		return 1;
+
+	if (errno == EACCES)
+		return 0;
+
+	trace_printf("could not determine permissions for '%s': %s\n", path,
+				strerror(errno));
+	return 0;
+}
+
+static void diagnose_execvp_eacces(const char *cmd, const char **argv)
+{
+	/*
+	 * man 2 execve states that EACCES is returned for:
+	 * - Search permission is denied on a component of the path prefix
+	 *   of cmd or the name of a script interpreter
+	 * - The file or script interpreter is not a regular file
+	 * - Execute permission is denied for the file, script or ELF
+	 *   interpreter
+	 * - The file system is mounted noexec
+	 */
+	struct strbuf sb = STRBUF_INIT;
+	char *path;
+	char *next;
+
+	if (strchr(cmd, '/')) {
+		if (!have_read_execute_permissions(cmd))
+			error("no read/execute permissions on '%s'\n", cmd);
+		return;
+	}
+
+	path = getenv("PATH");
+	while (path) {
+		next = strchrnul(path, ':');
+		if (path < next)
+			strbuf_add(&sb, path, next - path);
+		else
+			strbuf_addch(&sb, '.');
+
+		if (!*next)
+			path = NULL;
+		else
+			path = next + 1;
+
+		if (!have_read_execute_permissions(sb.buf)) {
+			error("no read/execute permissions on '%s'\n", sb.buf);
+			strbuf_release(&sb);
+			continue;
+		}
+
+		if (sb.len && sb.buf[sb.len - 1] != '/')
+			strbuf_addch(&sb, '/');
+		strbuf_addstr(&sb, cmd);
+
+		if (file_exists(sb.buf)) {
+			if (!have_read_execute_permissions(sb.buf))
+				error("no read/execute permissions on '%s'\n",
+						sb.buf);
+			else
+				warning("file '%s' exists and permissions "
+				"seem OK.\nIf this is a script, see if you "
+				"have sufficient privileges to run the "
+				"interpreter", sb.buf);
+		}
+
+		strbuf_release(&sb);
+	}
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;

@@ -285,6 +360,10 @@ fail_pipe:
 				error("cannot run %s: %s", cmd->argv[0],
 					strerror(ENOENT));
 			exit(127);
+		} else if (errno == EACCES) {
+			diagnose_execvp_eacces(cmd->argv[0], cmd->argv);
+			die("cannot exec '%s': %s", cmd->argv[0],
+				strerror(EACCES));
 		} else {
 			die_errno("cannot exec '%s'", cmd->argv[0]);
 		}

@@ -26,7 +26,7 @@ test_expect_success 'run_command can run a command' '
 	test_cmp empty err
 '
 
-test_expect_success POSIXPERM 'run_command reports EACCES' '
+test_expect_success POSIXPERM 'run_command reports EACCES, file permissions' '
 	cat hello-script >hello.sh &&
 	chmod -x hello.sh &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&

@@ -34,4 +34,18 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '
 	grep "fatal: cannot exec.*hello.sh" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, search path permisions' '
+	mkdir -p inaccessible &&
+	PATH=$(pwd)/inaccessible:$PATH &&
+	export PATH &&
+
+	cat hello-script >inaccessible/hello.sh &&
+	chmod 400 inaccessible &&
+	test_must_fail test-run-command run-command hello.sh 2>err &&
+	chmod 755 inaccessible &&
+
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "no read/execute permissions on" err
+'
+
 test_done

@@ -149,6 +149,55 @@ static int have_read_execute_permissions(const char *path)
 	return 0;
 }
 
+static void check_interpreter(const char *cmd)
+{
+	FILE *f;
+	struct strbuf sb = STRBUF_INIT;
+	/*
+	 * bash reads an 80 character line when determining the interpreter.
+	 * BSD apparently only allows 32 characters, as it is the size of
+	 * your average binary executable header.
+	 */
+	char firstline[80];
+	size_t s, start, end;
+
+	f = fopen(cmd, "r");
+	if (!f) {
+		error("cannot open file '%s': %s\n", cmd, strerror(errno));
+		return;
+	}
+
+	s = fread(firstline, 1, sizeof(firstline), f);
+	if (s < 2) {
+		trace_printf("cannot determine file type");
+		fclose(f);
+		return;
+	}
+
+	if (firstline[0] != '#' || firstline[1] != '!') {
+		trace_printf("file '%s' is not a script or"
+				" is a script without '#!'", cmd);
+		fclose(f);
+		return;
+	}
+
+	/* see if the given path has the executable bit set */
+	start = strspn(&firstline[2], " \t") + 2;
+	end = strcspn(&firstline[start], " \t\r\n") + start;
+	if (start >= end) {
+		error("could not determine interpreter\n");
+		return;
+	}
+
+	strbuf_add(&sb, &firstline[start], end - start);
+
+	if (!have_read_execute_permissions(sb.buf))
+		error("bad interpreter: no read/execute permissions on '%s'\n",
+				sb.buf);
+
+	strbuf_release(&sb);
+}
+
 static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 {
 	/*

@@ -167,6 +216,8 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 	if (strchr(cmd, '/')) {
 		if (!have_read_execute_permissions(cmd))
 			error("no read/execute permissions on '%s'\n", cmd);
+		else
+			check_interpreter(cmd);
 		return;
 	}
 

@@ -197,11 +248,11 @@ static void diagnose_execvp_eacces(const char *cmd, const char **argv)
 			if (!have_read_execute_permissions(sb.buf))
 				error("no read/execute permissions on '%s'\n",
 						sb.buf);
+			else if (access(sb.buf, R_OK) == 0)
+				check_interpreter(sb.buf);
 			else
-				warning("file '%s' exists and permissions "
-				"seem OK.\nIf this is a script, see if you "
-				"have sufficient privileges to run the "
-				"interpreter", sb.buf);
+				trace_printf("cannot determine interpreter "
+						"on '%s'\n", sb.buf);
 		}
 
 		strbuf_release(&sb);

@@ -13,6 +13,18 @@ cat >hello-script <<-EOF
 EOF
 >empty
 
+cat >someinterpreter <<-EOF
+	#!$SHELL_PATH
+	cat hello-script
+EOF
+>empty
+
+cat >incorrect-interpreter-script <<-EOF
+	#!someinterpreter
+	cat hello-script
+EOF
+>empty
+
 test_expect_success 'start_command reports ENOENT' '
 	test-run-command start-command-ENOENT ./does-not-exist
 '

@@ -48,4 +60,14 @@ test_expect_success POSIXPERM 'run_command reports EACCES, search path permision
 	grep "no read/execute permissions on" err
 '
 
+test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
+	cat incorrect-interpreter-script >hello.sh &&
+	chmod +x hello.sh &&
+	chmod -x someinterpreter &&
+	test_must_fail test-run-command run-command ./hello.sh 2>err &&
+
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
+'
+
 test_done