Verify this if `$HOME` exists, it is owned by current user in `getHome()` #6676

virusdave · 2022-06-15T20:43:18Z

Useful because a default sudo on darwin doesn't clear $HOME, so things like sudo nix-channel --list
will surprisingly return the USER'S channels, rather than root's.

Other counterintuitive outcomes can be seen in this PR description:
#6622

thufschmitt · 2022-06-16T14:01:25Z

I don't think this is a good idea. Having sudo not reset $HOME is intentional (although annoying in your case), but being able to override $HOME is definitely a feature (nix's own testsuite uses that extensively for instance).

Probably the second option from #6622 (comment) would make more sense (although it could be very confusing in some cases)

virusdave · 2022-06-16T15:34:55Z

Probably the second option from #6622 (comment) would make more sense (although it could be very confusing in some cases)

OK, this seems fair enough. I'll give that a shot since 6622 seems to have stalled a bit.

virusdave · 2022-06-17T04:46:15Z

Before:

dave @ davembp [/tmp]
$ nix-shell -p nix

dave @ davembp [nix-shell:/tmp]
$ nix-channel --list
darwin https://github.com/LnL7/nix-darwin/archive/master.tar.gz
nixpkgs https://nixos.org/channels/nixpkgs-unstable

dave @ davembp [nix-shell:/tmp]
$ sudo nix-channel --list
darwin https://github.com/LnL7/nix-darwin/archive/master.tar.gz
nixpkgs https://nixos.org/channels/nixpkgs-unstable

dave @ davembp [nix-shell:/tmp]
$ sudo -H nix-channel --list
nixos-22.05 https://nixos.org/channels/nixos-22.05
nixpkgs https://nixos.org/channels/nixos-22.05
nixpkgs-unstable https://nixos.org/channels/nixpkgs-unstable

After:

dave @ davembp [/tmp]
$ nix-shell -p 'with pkgs; nix.overrideAttrs (self: super: {patches = fetchpatch {url="https://github.com/virusdave/nix/commit/33ceafc0fc376d6bf07c1739bab24d95a6a9f27a.patch"; sha256="sha256-bD3f2jxoSlUjm5+YvUpmy8rw4JZuJ9UldyZ5D6gj6+w=";};})'

dave @ davembp [nix-shell:/tmp]
$ nix-channel --list
darwin https://github.com/LnL7/nix-darwin/archive/master.tar.gz
nixpkgs https://nixos.org/channels/nixpkgs-unstable

dave @ davembp [nix-shell:/tmp]
$ sudo nix-channel --list
nixos-22.05 https://nixos.org/channels/nixos-22.05
nixpkgs https://nixos.org/channels/nixos-22.05
nixpkgs-unstable https://nixos.org/channels/nixpkgs-unstable

dave @ davembp [nix-shell:/tmp]
$ sudo -H nix-channel --list
nixos-22.05 https://nixos.org/channels/nixos-22.05
nixpkgs https://nixos.org/channels/nixos-22.05
nixpkgs-unstable https://nixos.org/channels/nixpkgs-unstable

thufschmitt

Thanks. A couple of comments, but I like the overall approach :)

src/libutil/util.cc

Useful because a default `sudo` on darwin doesn't clear `$HOME`, so things like `sudo nix-channel --list` will surprisingly return the USER'S channels, rather than `root`'s. Other counterintuitive outcomes can be seen in this PR description: NixOS#6622

virusdave · 2022-06-17T21:44:08Z

Updated PR based on @thufschmitt 's review.

Now:

dave @ davembp [nix-shell:/tmp]
$ nix-channel --list
darwin https://github.com/LnL7/nix-darwin/archive/master.tar.gz
nixpkgs https://nixos.org/channels/nixpkgs-unstable

dave @ davembp [nix-shell:/tmp]
$ sudo nix-channel --list
warning: $HOME ('/Users/dave') is not owned by you, falling back to the one defined in the 'passwd' file
nixos-22.05 https://nixos.org/channels/nixos-22.05
nixpkgs https://nixos.org/channels/nixos-22.05
nixpkgs-unstable https://nixos.org/channels/nixpkgs-unstable

dave @ davembp [nix-shell:/tmp]
$ sudo -H nix-channel --list
nixos-22.05 https://nixos.org/channels/nixos-22.05
nixpkgs https://nixos.org/channels/nixos-22.05
nixpkgs-unstable https://nixos.org/channels/nixpkgs-unstable

mkenigs · 2022-06-20T15:44:19Z

Thanks for fixing this @virusdave

I think this approach feels pretty complex and non-obvious. This will make the behavior of nix depend on:

$HOME
Whether $HOME exists
Permissions of $HOME
/etc/passwd

That's a lot of conditionals based on the environment, which doesn't feel like it's really in the spirit of reproducibility. I don't know that there's a clean way to do it, but I wonder if even something like --home would be better? That said, I do appreciate that this makes the sudo behavior less silently confusing

thufschmitt · 2022-06-20T19:16:22Z

That's a lot of conditionals based on the environment, which doesn't feel like it's really in the spirit of reproducibility

That's UNIX systems for you 😛

Fwiw, as far as reproducibility goes, this only holds for the “impure” eval mode, as the only way the pure mode will use $HOME is as a cache. And the impure mode is… well, impure, so that's an acceptable tradeof 🙃

thufschmitt

Thanks @virusdave , let's merge

dhess · 2022-08-07T13:59:25Z

As a result of this change, on NixOS, any systemd process whose home is /var/empty and spawns a nix process spams the journal with:

warning: $HOME ('/var/empty') is not owned by you, falling back to the one defined in the 'passwd' file

which, for processes that run a lot of nix commands relative to the number of lines in their output, makes the logs quite a bit harder to read and significantly increases the log storage used.

virusdave · 2022-08-07T14:01:53Z

Oof, that's definitely not desirable. Out of curiosity, what is the /etc/passwd specified homedir for those systemd processes? Is it also /var/empty? If so, a simple anti-spamming measure would be to not print this message if the effect is a no-op. In fact, that might just make sense to do anyway.

dhess · 2022-08-07T14:02:58Z

Yes, it's /var/empty in the passwd file. I believe that's what happens by default on NixOS if you don't explicitly set a user's homedir.

virusdave · 2022-08-07T14:03:20Z

Yes, it's /var/empty in the passwd file. I believe that's what happens by default on NixOS if you don't explicitly set a user's homedir.

OK, i'll whip up a followup PR to make that change. Thanks for letting me know!

dhess · 2022-08-07T14:04:33Z

Thanks. In the meantime, I can downgrade the Nix we're using in the problematic systemd service. Do you happen to know which release of Nix this change went out with? (edit: Looks like 2.10.0?)

virusdave · 2022-08-07T14:36:15Z

Thanks. In the meantime, I can downgrade the Nix we're using in the problematic systemd service. Do you happen to know which release of Nix this change went out with? (edit: Looks like 2.10.0?)

Sure. You could also patch this PR into nix in an overlay if you don't want to go backwards (assuming the tests pass).

After NixOS/nix#6676, the following warning is displayed when running `darwin-rebuild switch`: warning: $HOME ('/Users/jamie') is not owned by you, falling back to the one defined in the 'passwd' file.

A [recent-ish change](NixOS#6676) logs a warning when a potentially counterintuitive situation happens. This now causes the multi-user installer to [emit a warning](NixOS/nixpkgs#189043) when it's doing the "seed the Nix database" step via a low-level `nix-store --load-db` invocation. `nix-store` functionality implementations don't actually use profiles or channels or homedir as far as i can tell. So why are we hitting this code at all? Well, the current command approach for functionality here builds a [fat `nix` binary](https://github.com/NixOS/nix/blob/master/src/nix/local.mk#L23-L26) which has _all_ the functionality of previous individual binaries (nix-env, nix-store, etc) bundled in, then [uses the invocation name](https://github.com/NixOS/nix/blob/master/src/nix/main.cc#L274-L277) to select the set of commands to expose. `nix` itself has this behavior, even when just trying to parse the (sub)command and arguments: ``` dave @ davembp2 $ nix error: no subcommand specified Try 'nix --help' for more information. dave @ davembp2 $ sudo nix warning: $HOME ('/Users/dave') is not owned by you, falling back to the one defined in the 'passwd' file error: no subcommand specified Try 'nix --help' for more information. dave @ davembp2 $ HOME=~root sudo nix error: no subcommand specified Try 'nix --help' for more information. ``` This behavior can also be seen pretty easily with an arbitrary `nix-store` invocation: ``` dave @ davembp2 $ nix-store --realize dave @ davembp2 $ sudo nix-store --realize # what installer is doing now warning: $HOME ('/Users/dave') is not owned by you, falling back to the one defined in the 'passwd' file dave @ davembp2 $ sudo HOME=~root nix-store --realize # what this PR effectively does dave @ davembp2 $ ```

After NixOS/nix#6676, the following warning is displayed when running `darwin-rebuild switch`: warning: $HOME ('/Users/jamie') is not owned by you, falling back to the one defined in the 'passwd' file.

virusdave force-pushed the dnicponski/scratch/swap_homedir_check_master branch 2 times, most recently from d7b70ae to d0a110d Compare June 15, 2022 20:54

virusdave force-pushed the dnicponski/scratch/swap_homedir_check_master branch from d0a110d to 9996f5f Compare June 16, 2022 16:06

virusdave changed the title ~~Swap order in getHome() to check syscall first and $HOME second if necessary~~ Verify $HOME exists and is owned by current user in getHome() Jun 16, 2022

virusdave force-pushed the dnicponski/scratch/swap_homedir_check_master branch 3 times, most recently from b676606 to ab5c922 Compare June 17, 2022 03:39

thufschmitt requested changes Jun 17, 2022

View reviewed changes

src/libutil/util.cc Show resolved Hide resolved

src/libutil/util.cc Outdated Show resolved Hide resolved

virusdave force-pushed the dnicponski/scratch/swap_homedir_check_master branch from ab5c922 to 5488e45 Compare June 17, 2022 21:25

virusdave force-pushed the dnicponski/scratch/swap_homedir_check_master branch from 5488e45 to ca2be50 Compare June 17, 2022 21:42

virusdave requested a review from thufschmitt June 17, 2022 21:45

virusdave changed the title ~~Verify $HOME exists and is owned by current user in getHome()~~ Verify this if $HOME exists, it is owned by current user in getHome() Jun 19, 2022

thufschmitt approved these changes Jun 20, 2022

View reviewed changes

thufschmitt merged commit 16c6c6c into NixOS:master Jun 20, 2022

mkenigs mentioned this pull request Jun 20, 2022

Don't read HOME from environment #6622

Closed

Sciencentistguy mentioned this pull request Jul 17, 2022

Nix complains about $HOME being retained by sudo LnL7/nix-darwin#477

Closed

lilyball mentioned this pull request Jul 24, 2022

Running Nix with sudo on darwin now complains that /Users/lily isn't owned by me #6834

Open

virusdave mentioned this pull request Aug 7, 2022

Do not spam logs if the owned-homedir check results in a noop #6880

Merged

malob mentioned this pull request Aug 29, 2022

Add -H to sudo nix-env [...] calls in darwin-rebuild.sh to deal with warning LnL7/nix-darwin#509

Merged

virusdave mentioned this pull request Aug 31, 2022

Set HOME var to root's home when running nix-store as root #6980

Merged

thufschmitt mentioned this pull request Mar 20, 2023

Allow running nix without a /etc/passwd entry #8074

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify this if `$HOME` exists, it is owned by current user in `getHome()` #6676

Verify this if `$HOME` exists, it is owned by current user in `getHome()` #6676

virusdave commented Jun 15, 2022

thufschmitt commented Jun 16, 2022

virusdave commented Jun 16, 2022

virusdave commented Jun 17, 2022

thufschmitt left a comment

virusdave commented Jun 17, 2022

mkenigs commented Jun 20, 2022 •

edited

thufschmitt commented Jun 20, 2022

thufschmitt left a comment

dhess commented Aug 7, 2022 •

edited

virusdave commented Aug 7, 2022 •

edited

dhess commented Aug 7, 2022

virusdave commented Aug 7, 2022

dhess commented Aug 7, 2022 •

edited

virusdave commented Aug 7, 2022

Verify this if $HOME exists, it is owned by current user in getHome() #6676

Verify this if $HOME exists, it is owned by current user in getHome() #6676

Conversation

virusdave commented Jun 15, 2022

thufschmitt commented Jun 16, 2022

virusdave commented Jun 16, 2022

virusdave commented Jun 17, 2022

thufschmitt left a comment

Choose a reason for hiding this comment

virusdave commented Jun 17, 2022

mkenigs commented Jun 20, 2022 • edited

thufschmitt commented Jun 20, 2022

thufschmitt left a comment

Choose a reason for hiding this comment

dhess commented Aug 7, 2022 • edited

virusdave commented Aug 7, 2022 • edited

dhess commented Aug 7, 2022

virusdave commented Aug 7, 2022

dhess commented Aug 7, 2022 • edited

virusdave commented Aug 7, 2022

Verify this if `$HOME` exists, it is owned by current user in `getHome()` #6676

Verify this if `$HOME` exists, it is owned by current user in `getHome()` #6676

mkenigs commented Jun 20, 2022 •

edited

dhess commented Aug 7, 2022 •

edited

virusdave commented Aug 7, 2022 •

edited

dhess commented Aug 7, 2022 •

edited