{activation-scripts,activate-system}: purify environment #1303
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It seems like a bad idea to keep restarting the activation daemon when it fails.
I believe this has been obsolete since `set -e` was added in 8708ebb.
This ensures that system activation does not depend on various
details of its process environment, ensuring uniformity across various
invocation contexts and with the `activate-system` daemon. This becomes
more important in a post‐user‐activation world to avoid problematic
dependencies like `$SUDO_USER`, but is a good idea in general.
The `sudoers(5)` defaults on my Sequoia system are:
Defaults env_reset
Defaults env_keep += "BLOCKSIZE"
Defaults env_keep += "COLORFGBG COLORTERM"
Defaults env_keep += "__CF_USER_TEXT_ENCODING"
Defaults env_keep += "CHARSET LANG LANGUAGE LC_ALL LC_COLLATE LC_CTYPE"
Defaults env_keep += "LC_MESSAGES LC_MONETARY LC_NUMERIC LC_TIME"
Defaults env_keep += "LINES COLUMNS"
Defaults env_keep += "LSCOLORS"
Defaults env_keep += "SSH_AUTH_SOCK"
Defaults env_keep += "TZ"
Defaults env_keep += "DISPLAY XAUTHORIZATION XAUTHORITY"
Defaults env_keep += "EDITOR VISUAL"
Defaults env_keep += "HOME MAIL"
Of these preserved environment variables, the ones that are set in
practice when I run `sudo env` that aren’t set in the activation
script here are:
* `$COLORTERM`
* `$DISPLAY`
* `$EDITOR`
* `$MAIL`
* `$SSH_AUTH_SOCK`
* `$TERM`
* `$__CF_USER_TEXT_ENCODING`
Most of these seem either pointless or actively harmful to set for
the purpose of the system activation script.
This will mean that tools run during activation won’t print output
in the user’s preferred language, but that’s probably the right
trade‐off overall, as that is likely to break activation scripts
that parse command output anyway.
Enzime
approved these changes
Jan 27, 2025
| export PATH="${pkgs.gnugrep}/bin:${pkgs.coreutils}/bin:@out@/sw/bin:/usr/bin:/bin:/usr/sbin:/sbin" | ||
| export USER=root | ||
| export LOGNAME=root |
There was a problem hiding this comment.
Was this the primary user previously?
There was a problem hiding this comment.
Nope, mainly $HOME is weird like that (on macOS):
yuyuko:~
❭ sudo env
COLORTERM=truecolor
EDITOR=kak
DISPLAY=/private/tmp/com.apple.launchd.BNiGA7fTUw/org.xquartz:0
__CF_USER_TEXT_ENCODING=0x0:0:2
PATH=/etc/profiles/per-user/emily/bin:/run/current-system/sw/bin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/Library/Apple/usr/bin
LANG=en_GB.UTF-8
HOME=/Users/emily
SSH_AUTH_SOCK=/Users/emily/.ssh/agent
TERM=xterm-ghostty
MAIL=/var/mail/root
LOGNAME=root
USER=root
SHELL=/bin/zsh
SUDO_COMMAND=/usr/bin/env
SUDO_USER=emily
SUDO_UID=501
SUDO_GID=20
There was a problem hiding this comment.
Interesting, on Linux $SUDO_HOME exists
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Split off from the Plan stack; see commit messages for details.