Bug 204813 - suggeted improvement for crontab(1)'s newly created files
Summary: suggeted improvement for crontab(1)'s newly created files
Status: Closed Works As Intended
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.2-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-25 17:53 UTC by Mason Loring Bliss
Modified: 2017-08-10 19:25 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mason Loring Bliss freebsd_triage 2015-11-25 17:53:35 UTC
crontab(1) creates new files in /var/cron/tabs. While crontab(5) notes a default PATH of "/usr/bin:/bin", generating a file for root ended up with the default PATH not including /sbin or /usr/sbin.

While this is fine and even documented, it occurred to me that it might be useful for crontab(1) to populate the new file with some explanatory text, and perhaps a PATH assignment - even if it were commented out - as a reminder to the user that the PATH has to be explicated in the crontab file.

There's precedent for this elsewhere... Debian populates new files with some comments. I've not got easy access to other BSDs at the moment to check.

Note: This *is* a user error and it *is* documented, but it would also be an easy way to prevent us from shooting ourselves in the foot sporadically. It seems like a "can't hurt, can help, effectively no cost" improvement. There's precedent for this variety of change - sysrc(8) is an example.

My suggested starter file:

#PATH=/bin:/usr/bin
Comment 1 Mason Loring Bliss freebsd_triage 2015-12-02 09:46:25 UTC
This is a week old and has not been triaged.
Comment 2 Jilles Tjoelker freebsd_committer freebsd_triage 2015-12-05 23:26:49 UTC
Perhaps _PATH_DEFPATH in include/paths.h should be changed to /usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/local/sbin, avoiding much of the problem in the first place (except for ~/bin, /usr/local/bin before /usr/bin, etc.).

Due to difficulties getting the value of _PATH_DEFPATH, the starter file will have to be defined in crontab.c.
Comment 3 Mason Loring Bliss freebsd_triage 2015-12-06 03:22:37 UTC
Expanding the path would obviate the problem, but it would also change the behaviour for regular users who arguably should have to explicitly add /sbin, /usr/sbin, and similar. I'd be happy with expanding the path.

My idea of a default file with a commented out PATH seemed like the least-invasive option - users with established crontabs wouldn't be impacted at all, and new crontabs would have a small, subtle reminder that their PATH isn't necessarily the same as it was in the shell they used to invoke crontab(1).

Regardless of whate solution you folks decide is appropriate, thank you for your time and work.
Comment 4 commit-hook freebsd_committer freebsd_triage 2016-01-05 16:21:46 UTC
A commit references this bug:

Author: jilles
Date: Tue Jan  5 16:21:21 UTC 2016
New revision: 293204
URL: https://svnweb.freebsd.org/changeset/base/293204

Log:
  Add sbin and /usr/local directories to _PATH_DEFPATH.

  Set _PATH_DEFPATH to
  /sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin. This is the
  path in the default class in the default /etc/login.conf,
  excluding ~/bin which would not be expanded properly in a string
  constant.

  For normal logins, _PATH_DEFPATH is overridden by /etc/login.conf,
  ~/.login_conf or shell startup files. _PATH_DEFPATH is still used as a
  default by execlp(), execvp(), posix_spawnp() and sh if PATH is not set, and
  by cron. Especially the latter is a common trap (most recently in PR
  204813).

  PR:		204813
  Reviewed by:	secteam (delphij), alfred

Changes:
  head/include/paths.h
  head/lib/libc/gen/exec.3
  head/lib/libc/gen/posix_spawn.3
  head/usr.sbin/cron/crontab/crontab.5
Comment 5 Mason Loring Bliss freebsd_triage 2017-08-10 19:25:32 UTC
FreeBSD is better off as it is. Sorry for the bother.