Page MenuHomeFreeBSD
Differential D23597

Respect PATH from login.conf in cron.
ClosedPublic
Actions

Authored by andrew_tao173.riddles.org.uk on Feb 10 2020, 1:20 AM.
Tags
None
Referenced Files
F81680223: D23597.id68031.diff
Fri, Apr 19, 8:27 PM
Unknown Object (File)
Tue, Mar 26, 1:49 AM
Unknown Object (File)
Feb 23 2024, 11:44 PM
Unknown Object (File)
Feb 23 2024, 11:43 PM
Unknown Object (File)
Feb 23 2024, 11:43 PM
Unknown Object (File)
Feb 23 2024, 11:43 PM
Unknown Object (File)
Feb 23 2024, 4:39 PM
Unknown Object (File)
Feb 6 2024, 5:17 PM
View All 59 Files
Subscribers
bcr
imp

Details

Reviewers
kevans
sigsys_gmail.com
• ian
Group Reviewers
manpages
Commits
rS359434: cron: respect PATH from login.conf
Summary

As a followup to the use of login.conf environment vars (other than PATH) in cron, this patch adds PATH (and HOME) to the list of login.conf settings respected.

The new logic is as follows:

  1. SHELL is always _PATH_BSHELL unless explicitly overridden in the crontab file itself; no other settings are respected. This is unchanged.
  1. PATH is taken from the first of: crontab file, login.conf, _PATH_DEFPATH
  1. HOME is taken from the first of: crontab file, login.conf, passwd entry, unset
  1. The current directory for invoking the command is taken from the crontab file's value of HOME (existing behavior), or the passwd entry, but not anywhere else (so it might not equal HOME if that was set in login.conf).
Test Plan

I've done basic functionality tests, open to suggestions for specific things that need checking.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

andrew_tao173.riddles.org.uk created this revision.Feb 10 2020, 1:20 AM
Herald added a subscriber: imp. · View Herald TranscriptFeb 10 2020, 1:20 AM
kevans accepted this revision.Feb 11 2020, 4:46 AM

I think this LGTM; I do wish we had some way to formally test cron(8) in a non-painful way (e.g. intercept and fake CLOCK_REALTIME so we can step through jobs at an expedited rate).

usr.sbin/cron/cron/do_command.c
302 ↗(On Diff #68031)

The style around here is so odd, but I did confirm this lined up with the prevailing wrapping...

This revision is now accepted and ready to land.Feb 11 2020, 4:46 AM
sigsys_gmail.com added inline comments.Feb 11 2020, 5:06 AM
usr.sbin/cron/cron/do_command.c
303 ↗(On Diff #68031)

...here. Should it be done also in the !LOGIN_CAP case?

usr.sbin/cron/lib/entry.c
393 ↗(On Diff #68031)

Since getting the home directory isn't being done there anymore but done...

sigsys_gmail.com accepted this revision.Feb 11 2020, 5:15 AM

I don't notice any problems apart from that minor thing.

andrew_tao173.riddles.org.uk added inline comments.Feb 12 2020, 6:42 AM
usr.sbin/cron/cron/do_command.c
303 ↗(On Diff #68031)

Good catch. Looks like the simplest fix is to put it back in entry.c in the !LOGIN_CAP case.

andrew_tao173.riddles.org.uk updated this revision to Diff 68161.Feb 12 2020, 6:48 AM

Fix bug in non-LOGIN_CAP case. Update manpage.

This revision now requires review to proceed.Feb 12 2020, 6:48 AM
Herald added a reviewer: manpages. · View Herald TranscriptFeb 12 2020, 6:48 AM
andrew_tao173.riddles.org.uk marked an inline comment as done.Feb 12 2020, 6:49 AM
bcr accepted this revision as: manpages.Feb 12 2020, 9:12 AM
bcr added a subscriber: bcr.

OK from manpages. Don't forget to bump the .Dd in the manpage for this content change.

sigsys_gmail.com accepted this revision.Feb 13 2020, 6:15 PM

I don't see any problems with the new version.

This revision is now accepted and ready to land.Feb 13 2020, 6:15 PM
Closed by commit rS359434: cron: respect PATH from login.conf (authored by kevans). · Explain WhyMar 30 2020, 3:27 AM
This revision was automatically updated to reflect the committed changes.
kevans added a commit: rS359434: cron: respect PATH from login.conf.

Revision Contents
Changeset List

PathSize
head/
usr.sbin/
cron/
cron/
59 lines
crontab/
35 lines
lib/
19 lines

Diff 69992

View Options

head/usr.sbin/cron/cron/do_command.c

Loading...
View Options

head/usr.sbin/cron/crontab/crontab.5

Loading...
View Options

head/usr.sbin/cron/lib/entry.c

Loading...