Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't read HOME from environment #6622

Closed
wants to merge 1 commit into from
Closed

Don't read HOME from environment #6622

wants to merge 1 commit into from

Conversation

mkenigs
Copy link
Contributor

@mkenigs mkenigs commented Jun 6, 2022

On darwin, sudo does not clear HOME, so HOME will still be set to a
user's home directory rather than root's.

util::getHome reads that value of HOME and is used in a number of
different places, which causes unexpected behavior in quite a few ways

See #1548 for some issues with
nix-channel

nix profile will symlink ~/.nix-profile to the default system profile

nix will create ~/.cache owned by root:
$ rm -r ~/.cache
$ sudo nix --extra-experimental-features "nix-command flakes" run nixpkgs#hello
Hello, world!
$ ls -ld ~/.cache
drwxr-xr-x 3 root staff 96 Jun 6 17:14 /Users/matthew/.cache

I'm guessing similar problems occur elsewhere getHome is used

@mkenigs
Don't read HOME from environment
9c3ccb5 
On darwin, sudo does not clear HOME, so HOME will still be set to a
user's home directory rather than root's.

util::getHome reads that value of HOME and is used in a number of
different places, which causes unexpected behavior in quite a few ways

See NixOS#1548 for some issues with
nix-channel

nix profile will symlink ~/.nix-profile to the default system profile

nix will create ~/.cache owned by root:
$ rm -r ~/.cache
$ sudo nix --extra-experimental-features "nix-command flakes" run nixpkgs#hello
Hello, world!
$ ls -ld ~/.cache
drwxr-xr-x 3 root staff 96 Jun  6 17:14 /Users/matthew/.cache

I'm guessing similar problems occur elsewhere getHome is used
@mkenigs
Copy link
Contributor Author

mkenigs commented Jun 6, 2022

It is possible to just use sudo -H instead, so there is a workaround. Unless anyone is relying on overriding via HOME though it seems like it would be nicer for plain sudo to work

@edolstra
Copy link
Member

edolstra commented Jun 7, 2022

This will cause Nix to fail if the current user has no password entry, which could happen e.g. in a sandbox or user namespace. So I'm not sure this is a good idea.

@mkenigs
Copy link
Contributor Author

mkenigs commented Jun 7, 2022

Would these be some better options?

  1. use HOME if it is owned by current user, else fail
    • I think this is closest to preserving current behavior, but it would prevent users from doing something that's pretty clearly undesired like creating ~/.cache owned by root
    • Users would still have to use sudo -H on darwin
  2. use HOME if it is owned by current user, else check for passwd entry, else fail
    • I don't love that this means the same command will behave differently depending on who invoked it. For option 1 you'll at least get an error explaining why it's behaving differently
    • Users could use plain sudo
  3. check for passwd entry, if it doesn't exist use HOME if owned by current user, else fail
    • will still behave differently depending on the invoking user, but only if a user doesn't have a passwd entry

@virusdave
Copy link
Contributor

Ha, i was just about to start writing a similar patch. Personally i'd vote for option (3), and this was what i was about to implement.

virusdave added a commit to virusdave/nix that referenced this pull request Jun 15, 2022
@virusdave
Swap order of attempts in getHome() to check syscall first and `$HO…
44ce616 
…ME` second if necessary

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 added a commit to virusdave/nix that referenced this pull request Jun 15, 2022
@virusdave
Swap order of attempts in getHome() to check syscall first and `$HO…
904573d 
…ME` second if necessary

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 virusdave mentioned this pull request Jun 15, 2022
Merged
virusdave added a commit to virusdave/nix that referenced this pull request Jun 15, 2022
@virusdave
Swap order of attempts in getHome() to check syscall first and `$HO…
2875564 
…ME` second if necessary

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 added a commit to virusdave/nix that referenced this pull request Jun 15, 2022
@virusdave
Swap order of attempts in getHome() to check syscall first and `$HO…
d7b70ae 
…ME` second if necessary

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 added a commit to virusdave/nix that referenced this pull request Jun 15, 2022
@virusdave
Swap order of attempts in getHome() to check syscall first and `$HO…
5f2a291 
…ME` second if necessary

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 added a commit to virusdave/nix that referenced this pull request Jun 15, 2022
@virusdave
Swap order of attempts in getHome() to check syscall first and `$HO…
d0a110d 
…ME` second if necessary

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 added a commit to virusdave/nix that referenced this pull request Jun 16, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
9996f5f 
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 added a commit to virusdave/nix that referenced this pull request Jun 16, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
cedad70 
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 added a commit to virusdave/nix that referenced this pull request Jun 16, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
a4f49ee 
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 added a commit to virusdave/nix that referenced this pull request Jun 16, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
979b4a2 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
b676606 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
6cafb82 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
ab5c922 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
33ceafc 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
5488e45 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
1246c31 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
73d175d 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
aa45796 
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 added a commit to virusdave/nix that referenced this pull request Jun 17, 2022
@virusdave
Verify $HOME is owned by current user in getHome(), if it exists.
ca2be50 
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
@mkenigs
Copy link
Contributor Author

mkenigs commented Jun 20, 2022

Closing as #6676 was merged

@mkenigs mkenigs closed this Jun 20, 2022
virusdave added a commit to virusdave/nix that referenced this pull request Aug 7, 2022
@virusdave
Verify $HOME exists and is owned by current user in getHome()
b91617a 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mkenigs @edolstra @virusdave