Skip to content

nix-darwin: sudo --set-home for multiple user activation#2857

Merged
berbiche merged 1 commit intonix-community:masterfrom
toonn:sudo-s-activate
Apr 1, 2022
Merged

nix-darwin: sudo --set-home for multiple user activation#2857
berbiche merged 1 commit intonix-community:masterfrom
toonn:sudo-s-activate

Conversation

@toonn
Copy link
Copy Markdown
Contributor

@toonn toonn commented Apr 1, 2022

Changing from sudo -i to sudo -s messes up activation when multiple
users are managed. --set-home should have similar behavior to -i in
that the activation script is run from the user's home directory.

Fixes #2856

Description

My previous fix broke use cases. Hopefully this fixes it again.

Checklist

  • Change is backwards compatible. (This and the previous are breaking but together they're backwards compatible? 😬 )

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • Test cases updated/added. See example.

  • Commit messages are formatted like

    {component}: {description}
    
    {long description}
    

    See CONTRIBUTING for more information and recent commit messages for examples.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

Changing from `sudo -i` to `sudo -s` messes up activation when multiple
users are managed. `--set-home` should have similar behavior to `-i` in
that the activation script is run from the user's home directory.

Fixes nix-community#2856
@toonn toonn marked this pull request as ready for review April 1, 2022 15:06
@toonn toonn requested a review from rycee as a code owner April 1, 2022 15:06
@berbiche berbiche merged commit e1fab01 into nix-community:master Apr 1, 2022
@kalbasit
Copy link
Copy Markdown
Member

kalbasit commented Apr 1, 2022

We should backport this to release-21.11 as we backported the -s in #2854. I'll prep a PR.

kalbasit pushed a commit that referenced this pull request Apr 1, 2022
Changing from `sudo -i` to `sudo -s` messes up activation when multiple
users are managed. `--set-home` should have similar behavior to `-i` in
that the activation script is run from the user's home directory.

Fixes #2856

(cherry picked from commit e1fab01)
@kalbasit
Copy link
Copy Markdown
Member

kalbasit commented Apr 1, 2022

OK I posted #2858 to backport this to 21.11.

berbiche pushed a commit that referenced this pull request Apr 1, 2022
Changing from `sudo -i` to `sudo -s` messes up activation when multiple
users are managed. `--set-home` should have similar behavior to `-i` in
that the activation script is run from the user's home directory.

Fixes #2856

(cherry picked from commit e1fab01)

Co-authored-by: toonn <toonn@toonn.io>
kalbasit added a commit to kalbasit/home-manager that referenced this pull request Apr 2, 2022
…t-link-name

* origin/master: (227 commits)
  doc: Add link to configuration options.html (nix-community#2851)
  nix-darwin: sudo --set-home for multiple user activation (nix-community#2857)
  Run sudo with -s in the darwin module (nix-community#807)
  zellij: add configuration for darwin
  types: fix `dagOf` behaviour with `mkIf`
  Translate using Weblate (Turkish)
  Translate using Weblate (Japanese)
  Update translation files
  home-manager: fix command option
  browserpass: add brave support
  overlay: rename parameters to flake specification
  flake: only support linux + darwin
  nix: add support for `nix profile`
  files: avoid cleanup if old home-files is missing
  dconf: note that system dconf must be enabled
  docs: bump nmd
  nix: fix attribute path of nix stable
  gtk: fix incorrect test assertion
  tests: bump nmt
  picom: use types.lines for extraOptions
  ...
jficz pushed a commit to jficz/home-manager that referenced this pull request Apr 7, 2022
…ty#2857)

Changing from `sudo -i` to `sudo -s` messes up activation when multiple
users are managed. `--set-home` should have similar behavior to `-i` in
that the activation script is run from the user's home directory.

Fixes nix-community#2856
toonn added a commit to toonn/home-manager that referenced this pull request Jun 20, 2022
In nix-community#587, kalbasit introduce the `-i` flag so the sudo invocation would
run in an environment with `HOME` set to the correct value for the
target user. This was necessary to be able to set up multiple users
without interfering with the invoking user's `HOME`.

In nix-community#807, I switched to `-s` instead because I managed to get an invalid
shell set for my user by switching `useUserPackages` from `true` to
`false` which changes the location where packages are installed and
`~/.nix-profile/bin/<my-shell>` was no longer valid. This was based on
the assumption that `SHELL` would be set to some sensible value by Home
Manager at this point. This turned out to be false as reported in nix-community#2900.

In 0ced6d6 (this commit's parent at this time), I explicitly set
`SHELL` to `${pkgs.bash}` so it is definitely set to a good shell when
invoking the activation script.

However, nix-community#807 broke activation for multiple users, the original
motivation for `-i`, as reported in nix-community#2856. I fixed this in nix-community#2857 by
additionally passing `--set-home`.

Further discussion with rycee in nix-community#3040 made me realize that the
activation script already has a good Nix store bash shebang. So all the
problems have been caused, not by the shell used for the activation
script but by sudo trying to use a different shell at all. `-i` uses the
shell set in the `passwd` file for the target user, but this can become
invalid as happened to me. `-s` uses either `SHELL` if it's defined or
the invoking user's shell as set in the `passwd` file. By explicitly
setting this to a shell provided by Nix we make sure we're not trying to
launch a non-existent shell. However, we're clearly already running in
an existing shell and because of `--set-home` we can activate other
users properly so there's not actually any need to try to have sudo
start a different shell first, it just adds an extra process that then
goes on to run the activation script with a good bash because of the
shebang.

Dropping `-s` altogether and keeping `--set-home` should avoid all of
these issues.
@teto teto mentioned this pull request Aug 22, 2022
7 tasks
teto pushed a commit to teto/home-manager that referenced this pull request Aug 22, 2022
…ty#2857)

Changing from `sudo -i` to `sudo -s` messes up activation when multiple
users are managed. `--set-home` should have similar behavior to `-i` in
that the activation script is run from the user's home directory.

Fixes nix-community#2856
rycee pushed a commit to toonn/home-manager that referenced this pull request Sep 19, 2022
In nix-community#587, kalbasit introduce the `-i` flag so the sudo invocation would
run in an environment with `HOME` set to the correct value for the
target user. This was necessary to be able to set up multiple users
without interfering with the invoking user's `HOME`.

In nix-community#807, I switched to `-s` instead because I managed to get an
invalid shell set for my user by switching `useUserPackages` from
`true` to `false` which changes the location where packages are
installed and `~/.nix-profile/bin/<my-shell>` was no longer valid.
This was based on the assumption that `SHELL` would be set to some
sensible value by Home Manager at this point. This turned out to be
false as reported in nix-community#2900.

In 0ced6d6 (this commit's parent at this time), I explicitly set
`SHELL` to `${pkgs.bash}` so it is definitely set to a good shell when
invoking the activation script.

However, nix-community#807 broke activation for multiple users, the original
motivation for `-i`, as reported in nix-community#2856. I fixed this in nix-community#2857 by
additionally passing `--set-home`.

Further discussion with rycee in nix-community#3040 made me realize that the
activation script already has a good Nix store bash shebang. So all
the problems have been caused, not by the shell used for the
activation script but by sudo trying to use a different shell at all.
`-i` uses the shell set in the `passwd` file for the target user, but
this can become invalid as happened to me. `-s` uses either `SHELL` if
it's defined or the invoking user's shell as set in the `passwd` file.
By explicitly setting this to a shell provided by Nix we make sure
we're not trying to launch a non-existent shell. However, we're
clearly already running in an existing shell and because of
`--set-home` we can activate other users properly so there's not
actually any need to try to have sudo start a different shell first,
it just adds an extra process that then goes on to run the activation
script with a good bash because of the shebang.

Dropping `-s` altogether and keeping `--set-home` should avoid all of
these issues.
austinharris pushed a commit to austinharris/home-manager that referenced this pull request Dec 23, 2022
In nix-community#587, kalbasit introduce the `-i` flag so the sudo invocation would
run in an environment with `HOME` set to the correct value for the
target user. This was necessary to be able to set up multiple users
without interfering with the invoking user's `HOME`.

In nix-community#807, I switched to `-s` instead because I managed to get an
invalid shell set for my user by switching `useUserPackages` from
`true` to `false` which changes the location where packages are
installed and `~/.nix-profile/bin/<my-shell>` was no longer valid.
This was based on the assumption that `SHELL` would be set to some
sensible value by Home Manager at this point. This turned out to be
false as reported in nix-community#2900.

In 0ced6d6 (this commit's parent at this time), I explicitly set
`SHELL` to `${pkgs.bash}` so it is definitely set to a good shell when
invoking the activation script.

However, nix-community#807 broke activation for multiple users, the original
motivation for `-i`, as reported in nix-community#2856. I fixed this in nix-community#2857 by
additionally passing `--set-home`.

Further discussion with rycee in nix-community#3040 made me realize that the
activation script already has a good Nix store bash shebang. So all
the problems have been caused, not by the shell used for the
activation script but by sudo trying to use a different shell at all.
`-i` uses the shell set in the `passwd` file for the target user, but
this can become invalid as happened to me. `-s` uses either `SHELL` if
it's defined or the invoking user's shell as set in the `passwd` file.
By explicitly setting this to a shell provided by Nix we make sure
we're not trying to launch a non-existent shell. However, we're
clearly already running in an existing shell and because of
`--set-home` we can activate other users properly so there's not
actually any need to try to have sudo start a different shell first,
it just adds an extra process that then goes on to run the activation
script with a good bash because of the shebang.

Dropping `-s` altogether and keeping `--set-home` should avoid all of
these issues.
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
…ty#2857)

Changing from `sudo -i` to `sudo -s` messes up activation when multiple
users are managed. `--set-home` should have similar behavior to `-i` in
that the activation script is run from the user's home directory.

Fixes nix-community#2856
spacekookie pushed a commit to spacekookie/home-manager that referenced this pull request Feb 10, 2023
In nix-community#587, kalbasit introduce the `-i` flag so the sudo invocation would
run in an environment with `HOME` set to the correct value for the
target user. This was necessary to be able to set up multiple users
without interfering with the invoking user's `HOME`.

In nix-community#807, I switched to `-s` instead because I managed to get an
invalid shell set for my user by switching `useUserPackages` from
`true` to `false` which changes the location where packages are
installed and `~/.nix-profile/bin/<my-shell>` was no longer valid.
This was based on the assumption that `SHELL` would be set to some
sensible value by Home Manager at this point. This turned out to be
false as reported in nix-community#2900.

In 0ced6d6 (this commit's parent at this time), I explicitly set
`SHELL` to `${pkgs.bash}` so it is definitely set to a good shell when
invoking the activation script.

However, nix-community#807 broke activation for multiple users, the original
motivation for `-i`, as reported in nix-community#2856. I fixed this in nix-community#2857 by
additionally passing `--set-home`.

Further discussion with rycee in nix-community#3040 made me realize that the
activation script already has a good Nix store bash shebang. So all
the problems have been caused, not by the shell used for the
activation script but by sudo trying to use a different shell at all.
`-i` uses the shell set in the `passwd` file for the target user, but
this can become invalid as happened to me. `-s` uses either `SHELL` if
it's defined or the invoking user's shell as set in the `passwd` file.
By explicitly setting this to a shell provided by Nix we make sure
we're not trying to launch a non-existent shell. However, we're
clearly already running in an existing shell and because of
`--set-home` we can activate other users properly so there's not
actually any need to try to have sudo start a different shell first,
it just adds an extra process that then goes on to run the activation
script with a good bash because of the shebang.

Dropping `-s` altogether and keeping `--set-home` should avoid all of
these issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: wrong home directory is used when activating multiple users

3 participants