Well, it's been about 14 years since I last contributed to busybox.
I changed jobs back then, and no longer had a day-to-day need for
(or time for!) improvements.  But I've still used it often.  Nice
to see familiar names still. (Hi Denys!)

Today's fix is for something that's bitten me several times over the
years.  busybox crond doesn't allow setting PATH within a crontab. 
This wouldn't be so bad, except that many distros/devices that use
busybox don't provide for easy access to logging (and certainly not
email!) for errors.  It took me an embarrassing amount of time
yesterday to figure out why my command (from $HOME/bin) wasn't running
as expected, using piCorePlayer (based on Tiny Core Linux) on a
Raspberry Pi.

I've tested this with and without ENABLE_FEATURE_CROND_CALL_SENDMAIL,
as well as resetting PATH part way through a crontab, to see that the
changes only affect subsequent lines.

I'm afraid I can't remember how to generate the cool automatic size
diffs.  The code change seems to add fewer than 100 bytes, but I also
ifdefed a very old level 5 debug loop in Parsefield(), which saved
over 100.  So I think there's been a net shrinkage of about 30 bytes. 

paul
=----------------------
 paul fox, p...@foxharp.boston.ma.us (arlington, ma)

From 6b20114fb967022255749b55ca1611b797003bb8 Mon Sep 17 00:00:00 2001
From: Paul Fox <p...@foxharp.boston.ma.us>
Date: Mon, 7 Mar 2022 11:35:28 -0500
Subject: [PATCH] crond: implement support for setting PATH in crontab files

It's very inconvenient for a cron user not to be able to set a
"personal" PATH for their cron jobs, as is possible with other crons

Also, disabled some level 5 logging code which doesn't seem useful
in production.  (Resulted in a net shrinkage of code.)
---
 miscutils/crond.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/miscutils/crond.c b/miscutils/crond.c
index 1965af656..62356ba7f 100644
--- a/miscutils/crond.c
+++ b/miscutils/crond.c
@@ -125,6 +125,7 @@ typedef struct CronLine {
 	char *cl_mailto;                /* whom to mail results, may be NULL */
 #endif
 	char *cl_shell;
+	char *cl_path;
 	/* ordered by size, not in natural order. makes code smaller: */
 	char cl_Dow[7];                 /* 0-6, beginning sunday */
 	char cl_Mons[12];               /* 0-11 */
@@ -323,6 +324,9 @@ static void ParseField(char *user, char *ary, int modvalue, int off,
 		return;
 	}
 
+/* disabled:  this produced nothing but long streams of '1' digits
+ * when I set logging to 5 -pgf */
+#if 0
 	/* can't use log5 (it inserts newlines), open-coding it */
 	if (G.log_level <= 5 && logmode != LOGMODE_SYSLOG) {
 		int i;
@@ -330,6 +334,7 @@ static void ParseField(char *user, char *ary, int modvalue, int off,
 			fprintf(stderr, "%d", (unsigned char)ary[i]);
 		bb_putchar_stderr('\n');
 	}
+#endif
 }
 
 static void FixDayDow(CronLine *line)
@@ -421,6 +426,7 @@ static void load_crontab(const char *fileName)
 	char *mailTo = NULL;
 #endif
 	char *shell = NULL;
+	char *path = NULL;
 
 	delete_cronfile(fileName);
 
@@ -470,6 +476,11 @@ static void load_crontab(const char *fileName)
 				shell = xstrdup(&tokens[0][6]);
 				continue;
 			}
+			if (is_prefixed_with(tokens[0], "PATH=")) {
+				free(path);
+				path = xstrdup(&tokens[0][5]);
+				continue;
+			}
 //TODO: handle HOME= too? "man crontab" says:
 //name = value
 //
@@ -480,8 +491,8 @@ static void load_crontab(const char *fileName)
 //
 //Several environment variables are set up automatically by the cron(8) daemon.
 //SHELL is set to /bin/sh, and LOGNAME and HOME are set from the /etc/passwd
-//line of the crontab's owner. HOME and SHELL may be overridden by settings
-//in the crontab; LOGNAME may not.
+//line of the crontab's owner.  HOME, SHELL, and PATH may be overridden by
+//settings in the crontab; LOGNAME may not.
 
 #if ENABLE_FEATURE_CROND_SPECIAL_TIMES
 			if (tokens[0][0] == '@') {
@@ -567,6 +578,7 @@ static void load_crontab(const char *fileName)
 			line->cl_mailto = xstrdup(mailTo);
 #endif
 			line->cl_shell = xstrdup(shell);
+			line->cl_path = xstrdup(path);
 			/* copy command */
 			line->cl_cmd = xstrdup(tokens[5]);
 			pline = &line->cl_next;
@@ -653,21 +665,22 @@ static void safe_setenv(char **pvar_val, const char *var, const char *val)
 }
 #endif
 
-static void set_env_vars(struct passwd *pas, const char *shell)
+static void set_env_vars(struct passwd *pas, const char *shell, const char *path)
 {
 	/* POSIX requires crond to set up at least HOME, LOGNAME, PATH, SHELL.
-	 * We assume crond inherited suitable PATH.
 	 */
 #if SETENV_LEAKS
 	safe_setenv(&G.env_var_logname, "LOGNAME", pas->pw_name);
 	safe_setenv(&G.env_var_user, "USER", pas->pw_name);
 	safe_setenv(&G.env_var_home, "HOME", pas->pw_dir);
 	safe_setenv(&G.env_var_shell, "SHELL", shell);
+	if (path) safe_setenv(&G.env_var_shell, "PATH", path);
 #else
 	xsetenv("LOGNAME", pas->pw_name);
 	xsetenv("USER", pas->pw_name);
 	xsetenv("HOME", pas->pw_dir);
 	xsetenv("SHELL", shell);
+	if (path) xsetenv("PATH", path);
 #endif
 }
 
@@ -701,7 +714,7 @@ fork_job(const char *user, int mailFd, CronLine *line, bool run_sendmail)
 	shell = line->cl_shell ? line->cl_shell : G.default_shell;
 	prog = run_sendmail ? SENDMAIL : shell;
 
-	set_env_vars(pas, shell);
+	set_env_vars(pas, shell, NULL); /* don't use crontab's PATH for sendmail */
 
 	sv_logmode = logmode;
 	pid = vfork();
@@ -845,7 +858,7 @@ static pid_t start_one_job(const char *user, CronLine *line)
 
 	/* Prepare things before vfork */
 	shell = line->cl_shell ? line->cl_shell : G.default_shell;
-	set_env_vars(pas, shell);
+	set_env_vars(pas, shell, line->cl_path);
 
 	/* Fork as the user in question and run program */
 	pid = vfork();
-- 
2.25.1

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to