NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.07k stars 14.05k forks source link

`sudo zsh` does not inherit `$PATH` #305385

Open xzfc opened 6 months ago

xzfc commented 6 months ago

cc @Mic92

Issue

zsh running sudo does not inherit $PATH nor other important environment variables. This is particularly annoying when running #!/usr/bin/env zsh scripts as root inside nix-shell. This is not an issue with the default shell, Bash.

Steps to reproduce

$ nix-shell -p hello --run 'sudo zsh -c hello'
zsh:1: command not found: hello

$ nix-shell -p hello --run 'sudo bash -c hello'
Hello, world!

$ nix-shell -p hello --run 'sudo sh -c hello'
Hello, world!

Cause

This is caused by sourcing /etc/zshenv unconditionally when running zsh, even if -c is passed.

https://github.com/NixOS/nixpkgs/blob/7eceafd98fcc18e57305ae82f2e6cba51b06ceac/nixos/modules/programs/zsh/zsh.nix#L180-L189

This file calls setEnvironment which overwrites the environment variables, including $PATH. The call is conditional, guarded by $__ETC_ZSHENV_SOURCED and $__NIXOS_SET_ENVIRONMENT_DONE from being sourced again. So, as intended, it's not called for regular (non-sudo) nested shells. However, sudo clears these variables, but not $PATH.

This is not an issue for Bash because bash -c does not source any files by default.[^1]

[^1]: Unless $BASH_ENV is set, but let's keep these nuances aside.

Possible Solutions

  1. Drop /etc/zshenv. We don't have it for Bash (because it's not possible), so maybe we don't need it for the Z shell either? It would be sufficient to keep only /etc/zprofile, akin to /etc/profile for Bash. Does this option have any unwanted implications?

  2. Make sudo pass $__ETC_ZSHENV_SOURCED/$__NIXOS_SET_ENVIRONMENT_DONE variables by default by adding it to env_keep in the /etc/sudoers file. Tho, to be honest, I don't like this solution.

Workaround

Set a guardian variable manually to prevent setEnvironment from being sourced.

$ nix-shell -p hello --run 'sudo __ETC_ZSHENV_SOURCED=1 zsh -c hello'
Hello, world!
Mic92 commented 6 months ago

Option 1. also sounds better to me. But I also haven't thought about all the implications.