Allow invoking path-bound built-in commands by direct path or preceding PATH assignment (#275)

Path-bound builtins on ksh (such as /opt/ast/bin/cat) break some
basic assumptions about paths in the shell that should hold true,
e.g., that a path output by whence -p or command -v should actually
point to an executable command. This commit should fix the
following:

1. Path-bound built-ins (such as /opt/ast/bin/cat) can now be
   executed by invoking the canonical path (independently of the
   value of $PATH), so the following will now work as expected:

        $ /opt/ast/bin/cat --version
          version         cat (AT&T Research) 2012-05-31
        $ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
          version         cat (AT&T Research) 2012-05-31

   In the event an external command by that path exists, the
   path-bound builtin will now override it when invoked using the
   canonical path. To invoke a possible external command at that
   path, you can still use a non-canonical path, e.g.:
   /opt//ast/bin/cat or /opt/ast/./bin/cat

2. Path-bound built-ins will now also be found on a PATH set
   locally using an assignment preceding the command, so something
   like the following will now work as expected:

        $ PATH=/opt/ast/bin cat --version
          version         cat (AT&T Research) 2012-05-31

   The builtin is not found by sh_exec() because the search for
   builtins happens long before invocation-local preceding
   assignments are processsed. This only happens in sh_ntfork(),
   before forking, or in sh_fork(), after forking. Both sh_ntfork()
   and sh_fork() call path_spawn() to do the actual path search, so
   a check there will cover both cases.

   This does mean the builtin will be run in the forked child if
   sh_fork() is used (which is the case on interactive shells with
   job.jobcontrol set, or always after compiling with SHOPT_SPAWN
   disabled). Searching for it before forking would mean
   fundamentally redesigning that function to be basically like
   sh_ntfork(), so this is hard to avoid.

src/cmd/ksh93/sh/path.c: path_spawn():
- Before doing anything else, check if the passed path appears in
  the builtins tree as a pathbound builtin. If so, run it. Since a
  builtin will only be found if a preceding PATH assignment
  temporarily changed the PATH, and that assignment is currently in
  effect, we can just sh_run() the builtin so a nested sh_exec()
  invocation will find and run it.
- If 'spawn' is not set (i.e. we must return), set errno to 0 and
  return -2. See the change to sh_ntfork() below.

src/cmd/ksh93/sh/xec.c:
- sh_exec(): When searching for built-ins and the restricted option
  isn't active, also search bltin_tree for names beginning with a
  slash.
- sh_ntfork(): Only throw an error if the PID value returned is
  exactly -1. This allows path_spawn() to return -2 after running a
  built-in to tell sh_ntfork() to do the right things to restore
  state.

src/cmd/ksh93/sh/parse.c: simple():
- When searching for built-ins at parse time, only exclude names
  containing a slash if the restricted option is active. This
  allows finding pointers to built-ins invoked by literal path like
  /opt/ast/bin/cat, as long as that does not result from an
  expansion. This is not actually necessary as sh_exec() will also
  cover this case, but it is an optimisation.

src/lib/libcmd/getconf.c:
- Replace convoluted deferral to external command by a simple
  invocation of the path to the native getconf command determined
  at compile time (by src/lib/libast/comp/conf.sh). Based on:
  https://github.com/ksh93/ksh/issues/138#issuecomment-816384871
  If there is ever a system that has /opt/ast/bin/getconf as its
  default native external 'getconf', then there would still be an
  infinite recursion crash, but this seems extremely unlikely.

Resolves: https://github.com/ksh93/ksh/issues/138

NEWS

@@ -3,6 +3,21 @@ For full details, see the git log at: https://github.com/ksh93/ksh
 
 Any uppercase BUG_* names are modernish shell bug IDs.
 
+2021-04-14:
+
+- Path-bound built-ins (such as /opt/ast/bin/cat) can now be executed by
+  invoking the canonical path, so the following will now work as expected:
+	$ /opt/ast/bin/cat --version
+	  version         cat (AT&T Research) 2012-05-31
+	$ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
+	  version         cat (AT&T Research) 2012-05-31
+  Non-canonical paths such as /opt/ast/./bin/cat will not find the built-ins.
+
+- Path-bound built-ins will now also be found on a PATH set locally using an
+  assignment preceding the command, so the following will now work as expected:
+	$ PATH=/opt/ast/bin cat --version
+	  version         cat (AT&T Research) 2012-05-31
+
 2021-04-13:
 
 - Fixed a few bugs that could cause ksh to show the wrong error message and/or

src/cmd/ksh93/COMPATIBILITY

@@ -117,6 +117,18 @@ For more details, see the NEWS file and for complete details, see the git log.
 		ksh -c 'OLDPWD=/bin; OLDPWD=/tmp cd - > /dev/null; echo $OLDPWD'
 		ksh -c 'cd /var; PWD=/tmp cd /usr; echo $PWD'
 	now prints '/bin' followed by '/var'.
+
+23.	Path-bound builtins (such as /opt/ast/bin/cat) can now be executed
+	by invoking the canonical path, so the following will now work:
+		$ /opt/ast/bin/cat --version
+		  version         cat (AT&T Research) 2012-05-31
+		$ (PATH=/opt/ast/bin:$PATH; "$(whence -p cat)" --version)
+		  version         cat (AT&T Research) 2012-05-31
+	In the event an external command by that path exists, the path-bound
+	builtin will now override it when invoked using the canonical path.
+	To invoke a possible external command at that path, you can still use
+	a non-canonical path, e.g.: /opt//ast/bin/cat or /opt/ast/./bin/cat
+
 ____________________________________________________________________________
 
 		KSH-93 VS. KSH-88

src/cmd/ksh93/include/version.h

@@ -20,7 +20,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.0-alpha"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2021-04-13"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2021-04-14"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2021 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/sh/parse.c

@@ -1439,9 +1439,10 @@ static Shnode_t *simple(Lex_t *lexp,int flag, struct ionod *io)
 		{
 			if(!(argp->argflag&ARG_RAW))
 				argno = -1;
-			if(argno>=0 && argno++==cmdarg && !(flag&SH_ARRAY) && *argp->argval!='/')
+			if(argno>=0 && argno++==cmdarg && !(flag&SH_ARRAY)
+			&& !(sh_isoption(SH_RESTRICTED) && strchr(argp->argval,'/')))
 			{
-				/* check for builtin command */
+				/* check for builtin command (including path-bound builtins executed by full pathname) */
 				Namval_t *np=nv_bfsearch(argp->argval,lexp->sh->fun_tree, (Namval_t**)&t->comnamq,(char**)0);
 				if(np && is_abuiltin(np))
 				{

src/cmd/ksh93/sh/path.c

@@ -1085,6 +1085,20 @@ pid_t path_spawn(Shell_t *shp,const char *opath,register char **argv, char **env
 	char		*s, *v;
 	int		r, n, pidsize;
 	pid_t		pid= -1;
+	if(nv_search(opath,shp->bltin_tree,0))
+	{
+		/* Found a path-bound built-in. Since this was not caught earlier in sh_exec(), it must
+		   have been found on a temporarily assigned PATH, as with 'PATH=/opt/ast/bin:$PATH cat'.
+		   Now that that local PATH assignment is in effect, we can just sh_run() the built-in. */
+		int argc = 0;
+		while(argv[argc])
+			argc++;
+		sh_run(argc,argv);
+		if(!spawn)
+			sh_done(shp,0);
+		errno = 0;
+		return(-2);  /* treat like failure to spawn in sh_ntfork() except for the error message */
+	}
 	/* leave room for inserting _= pathname in environment */
 	envp--;
 #if _lib_readlink

src/cmd/ksh93/sh/xec.c

@@ -1218,9 +1218,19 @@ int sh_exec(register const Shnode_t *t, int flags)
 				if(io)
 					sfsync(shp->outpool);
 				shp->lastpath = 0;
-				if(!np  && !strchr(com0,'/'))
+				if(!np)
 				{
-					if(path_search(shp,com0,NIL(Pathcomp_t**),1))
+					if(*com0 == '/' && !sh_isoption(SH_RESTRICTED))
+					{
+						/* Check for path-bound builtin referenced by absolute canonical path, in
+						   case the parser didn't provide a pointer (e.g. '$(whence -p cat) foo') */
+						np = nv_search(com0, shp->bltin_tree, 0);
+					}
+					else if(strchr(com0,'/'))
+					{
+						/* Do nothing */
+					}
+					else if(path_search(shp,com0,NIL(Pathcomp_t**),1))
 					{
 						error_info.line = t->com.comline-shp->st.firstline;
 #if SHOPT_NAMESPACE
@@ -3617,7 +3627,7 @@ static pid_t sh_ntfork(Shell_t *shp,const Shnode_t *t,char *argv[],int *jobid,in
 	fail:
 		if(jobfork && spawnpid<0) 
 			job_fork(-2);
-		if(spawnpid < 0) switch(errno=shp->path_err)
+		if(spawnpid == -1) switch(errno=shp->path_err)
 		{
 		    case ENOENT:
 			errormsg(SH_DICT,ERROR_exit(ERROR_NOENT),e_found+4);

src/cmd/ksh93/tests/builtins.sh

@@ -1184,15 +1184,23 @@ got=$(ulimit -t unlimited; uname -d > /dev/null; uname -o)
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
 # ======
-# Default path-bound builtins should be available to restricted shells if they are in $PATH on invocation
+# https://github.com/ksh93/ksh/issues/138
-# https://github.com/ksh93/ksh/issues/138#issuecomment-813886069
 builtin -d cat
 if	[[ $'\n'${ builtin; }$'\n' == *$'\n/opt/ast/bin/cat\n'* ]]
 then	exp='  version         cat (*) ????-??-??'
+	got=$(/opt/ast/bin/cat --version 2>&1)
+	[[ $got == $exp ]] || err_exit "path-bound builtin not executable by literal canonical path" \
+		"(expected match of $(printf %q "$exp"), got $(printf %q "$got"))"
+	got=$(PATH=/opt/ast/bin:$PATH; "${ whence -p cat; }" --version 2>&1)
+	[[ $got == $exp ]] || err_exit "path-bound builtin not executable by canonical path resulting from expansion" \
+		"(expected match of $(printf %q "$exp"), got $(printf %q "$got"))"
 	got=$(PATH=/opt/ast/bin:$PATH "$SHELL" -o restricted -c 'cat --version' 2>&1)
 	[[ $got == $exp ]] || err_exit "restricted shells do not recognize path-bound builtins" \
 		"(expected match of $(printf %q "$exp"), got $(printf %q "$got"))"
-else	warning 'skipping path-bound builtin test for restricted shells: builtin /opt/ast/bin/cat not found'
+	got=$(PATH=/opt/ast/bin cat --version 2>&1)
+	[[ $got == $exp ]] || err_exit "path-bound builtin not found on PATH in preceding assignment" \
+		"(expected match of $(printf %q "$exp"), got $(printf %q "$got"))"
+else	warning 'skipping path-bound builtin tests: builtin /opt/ast/bin/cat not found'
 fi
 
 # ======

src/lib/libcmd/date.c

@@ -31,8 +31,8 @@ static const char usage[] =
 "[--catalog?" ERROR_CATALOG "]"
 "[+NAME?date - set/list/convert dates]"
 "[+DESCRIPTION?\bdate\b sets the current date and time (with appropriate"
-"	privilege), lists the current date or file dates, or converts"
+"	privileges, provided the shell isn't in restricted mode), lists"
-"	dates.]"
+"	the current date or file dates, or converts dates.]"
 "[+?Most common \adate\a forms are recognized, including those for"
 "	\bcrontab\b(1), \bls\b(1), \btouch\b(1), and the default"
 "	output from \bdate\b itself.]"

src/lib/libcmd/getconf.c

@@ -27,7 +27,7 @@
  */
 
 static const char usage[] =
-"[-?\n@(#)$Id: getconf (AT&T Research) 2012-06-25 $\n]"
+"[-?\n@(#)$Id: getconf (ksh 93u+m) 2021-04-09 $\n]"
 "[--catalog?" ERROR_CATALOG "]"
 "[+NAME?getconf - get configuration values]"
 "[+DESCRIPTION?\bgetconf\b displays the system configuration value for"
@@ -53,11 +53,11 @@ static const char usage[] =
 "	Only one of \b--call\b, \b--name\b or \b--standard\b may be specified.]"
 "[+?This implementation uses the \bastgetconf\b(3) string interface to the native"
 "	\bsysconf\b(2), \bconfstr\b(2), \bpathconf\b(2), and \bsysinfo\b(2)"
-"	system calls. If \bgetconf\b on \b$PATH\b is not the default native"
+"	system calls."
-"	\bgetconf\b, named by \b$(getconf GETCONF)\b, then \bastgetconf\b(3)"
+"	Invalid options and/or names not supported by \bastgetconf\b(3) cause"
-"	checks only \bast\b specific extensions and the native system calls;"
+"	the default native \bgetconf\b, named by \b$(getconf GETCONF)\b, to"
-"	invalid options and/or names not supported by \bastgetconf\b(3) cause"
+"	be executed (unless the shell is in restricted mode, in which case"
-"	the \bgetconf\b on \b$PATH\b to be executed.]"
+"	an error will occur).]"
 
 "[a:all?Call the native \bgetconf\b(1) with option \b-a\b.]"
 "[b:base?List base variable name sans call and standard prefixes.]"
@@ -137,25 +137,12 @@ b_getconf(int argc, char** argv, Shbltin_t* context)
 	register char*		path;
 	register char*		value;
 	register const char*	s;
-	register const char*	t;
 	char*			pattern;
 	char*			native;
-	char*			cmd;
-	Path_t*			e;
-	Path_t*			p;
 	int			flags;
 	int			n;
-	int			i;
-	int			m;
-	int			q;
 	char**			oargv;
-	char			buf[PATH_MAX];
-	Path_t			std[64];
-	struct stat		st0;
-	struct stat		st1;
-
 	static const char	empty[] = "-";
-	static const Path_t	equiv[] = { { "/bin", 4 }, { "/usr/bin", 8 } };
 
 	cmdinit(argc, argv, context, ERROR_CATALOG, 0);
 	oargv = argv;
@@ -288,121 +275,11 @@ b_getconf(int argc, char** argv, Shbltin_t* context)
 	return error_info.errors != 0;
 
  defer:
-
 	/*
-	 * defer to argv[0] if absolute and it exists
+	 * Run the external getconf command
 	 */
-
+	oargv[0] = native;
-	if ((cmd = oargv[0]) && *cmd == '/' && !access(cmd, X_OK))
-		goto found;
-
-	/*
-	 * defer to the first getconf on $PATH that is also on the standard PATH
-	 */
-
-	e = std;
-	s = astconf("PATH", NiL, NiL); 
-	q = !stat(equiv[0].path, &st0) && !stat(equiv[1].path, &st1) && st0.st_ino == st1.st_ino && st0.st_dev == st1.st_dev;
-	m = 0;
-	do
-	{
-		for (t = s; *s && *s != ':'; s++);
-		if ((n = s - t) && *t == '/')
-		{
-			if (q)
-				for (i = 0; i < 2; i++)
-					if (n == equiv[i].len && !strncmp(t, equiv[i].path, n))
-					{
-						if (m & (i+1))
-							t = 0;
-						else
-						{
-							m |= (i+1);
-							if (!(m & (!i+1)))
-							{
-								m |= (!i+1);
-								e->path = t;
-								e->len = n;
-								e++;
-								if (e >= &std[elementsof(std)])
-									break;
-								t = equiv[!i].path;
-								n = equiv[!i].len;
-							}
-						}
-					}
-			if (t)
-			{
-				e->path = t;
-				e->len = n;
-				e++;
-			}
-		}
-		while (*s == ':')
-			s++;
-	} while (*s && e < &std[elementsof(std)]);
-	if (e < &std[elementsof(std)])
-	{
-		e->len = strlen(e->path = "/usr/sbin");
-		if (++e < &std[elementsof(std)])
-		{
-			e->len = strlen(e->path = "/sbin");
-			e++;
-		}
-	}
-	if (s = getenv("PATH"))
-		do
-		{
-			for (t = s; *s && *s != ':'; s++);
-			if ((n = s - t) && *t == '/')
-			{
-				for (p = std; p < e; p++)
-					if (p->len == n && !strncmp(t, p->path, n))
-					{
-						sfsprintf(buf, sizeof(buf), "%-*.*s/%s", n, n, t, error_info.id);
-						if (!access(buf, X_OK))
-						{
-							cmd = buf;
-							goto found;
-						}
-					}
-			}
-			while (*s == ':')
-				s++;
-		} while (*s);
-
-	/*
-	 * defer to the first getconf on the standard PATH
-	 */
-
-	for (p = std; p < e; p++)
-	{
-		sfsprintf(buf, sizeof(buf), "%-*.*s/%s", p->len, p->len, p->path, error_info.id);
-		if (!access(buf, X_OK))
-		{
-			cmd = buf;
-			goto found;
-		}
-	}
-
-	/*
-	 * out of deferrals
-	 */
-
-	if (name)
-		error(4, "%s: unknown name -- no native getconf(1) to defer to", name);
-	else
-		error(4, "no native getconf(1) to defer to");
-	return 2;
-
- found:
-
-	/*
-	 * don't blame us for crappy diagnostics
-	 */
-
-	oargv[0] = cmd;
 	if ((n = sh_run(context, argc, oargv)) >= EXIT_NOEXEC)
-		error(ERROR_SYSTEM|2, "%s: exec error [%d]", cmd, n);
+		error(ERROR_SYSTEM|2, "%s: exec error [%d]", native, n);
 	return n;
 }

src/lib/libcmd/uname.c

@@ -37,7 +37,8 @@ static const char usage[] =
 "	separated, on a single line. When more than one option is specified"
 "	the output is in the order specified by the \b-A\b option below."
 "	Unsupported option values are listed as \a[option]]\a. If any unknown"
-"	options are specified, the OS default \buname\b(1) is called.]"
+"	options are specified, the OS default \buname\b(1) is called (unless"
+"	the shell is in restricted mode, in which case an error will occur).]"
 "[+?If any \aname\a operands are specified then the \bsysinfo\b(2) values"
 "	for each \aname\a are listed, separated by space, on one line."
 "	\bgetconf\b(1), a pre-existing \astandard\a interface, provides"