search

LnL7 / nix-darwin

nix modules for darwin
MIT License
3.19k stars 457 forks source link

users: allow arbitrary group IDs #1059

Closed emilazy closed 2 months ago

emilazy commented 2 months ago

The upstream Nix UID/GID changes for Sequoia will require us to manage a group with GID 350. That will require more work on our end to ensure compatibility and a working migration path, but this is enough to allow hacking around it locally in system configurations for now.

emilazy commented 2 months ago

Test failure seems unrelated?

Enzime commented 2 months ago

Yeah that just needs to be fixed on master, you can merge :+1:

LnL7 commented 2 months ago

Are there still sanity checks in place system for users? I can say with experience that messing with os users can mess up a system to the point you can't login anymore.

emilazy commented 2 months ago

Users had no equivalent checks here, it was just groups, so I don’t think this should create the ability to screw things up where there wasn’t one before. Not sure how we could do something truly robust since I don’t know if there’s a reliable way to detect true system users (parsing /etc/passwd, I guess…?). Currently we can’t even represent the new UID/GID values that upstream has settled on for Sequoia compatibility so the status quo was pretty bad. Still needs work to actually move over to the new values and handle migration.

LnL7 commented 2 months ago

I haven't been following the situation closely. But from what I understand it indeed kind of sucks, I see users/groups with >400 on a base system but a large id is also not allowed anymore...

One reason I added this check is that it prevents potential deletion of id 501, which is the first admin user. If someone adds their main account to users.knownUsers, you don't want to delete the account running darwin-rebuild if something the users section is temporarily commented out. The activation also as an extra guard https://github.com/LnL7/nix-darwin/blob/master/modules/users/default.nix#L187

emilazy commented 2 months ago

Yes, it’s unfortunate. We settled on the 350 range after a lot of bikeshedding alternatives and in the end a suggestion from an Apple devrel.

There wasn’t any type‐based guard on UIDs, only GIDs. UIDs use the same type as GIDs use after this PR. Maybe that changed beforehand? But in any case this change doesn’t let you mess with any users that it didn’t before, just groups, so given that users seem like the bigger deal there I don’t think this is meaningfully reducing those safeguards.

LnL7 commented 2 months ago

Yeah, the delete check I linked would be something to be more careful with. But it might make sense to update the logic for other usecases where cleanup is expected.

For migration, the logic of the previous users change is still present, that included a rename which makes the logic simpler.

emilazy commented 2 months ago

I am hoping to find the time to review https://github.com/LnL7/nix-darwin/pull/1017 which may make reliable user management easier, which I hope would help with automating this migration. But it does require some care. We might end up just doing something hacky like running the upstream migration script on activation instead.