Repository navigation
usermod: move home directory files to the new primary group with -g - #349
jayhemnani9910 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
| use std::os::unix::fs::MetadataExt; | ||
|
|
||
| // Like GNU, only a home directory the user owns is walked, and a | ||
| // symlinked home is not followed. |
There was a problem hiding this comment.
GNU exits 12 for a symlinked home, could you please open an issue so we don't forget it?
There was a problem hiding this comment.
Opened #351. GNU still updates /etc/passwd there, prints "Failed to change ownership of the home directory" and exits 12, for -u, -g and both.
|
|
||
| // Like GNU, only a home directory the user owns is walked, and a | ||
| // symlinked home is not followed. | ||
| let home_path = root.resolve(&home_for_chown); |
There was a problem hiding this comment.
this also changes -u on a root-owned or symlinked home
please add a test for -u too
There was a problem hiding this comment.
Yes, -u follows the same check now. test_home_not_owned_by_user_is_left_alone runs -g and -u on a root-owned home and on a symlinked one (c26923d). On main, -u re-owns the files under both.
| ("mine", 1000, 1000), | ||
| ("root-owned", 0, 1000), | ||
| ("other-group", 1000, 0), | ||
| ] { |
There was a problem hiding this comment.
could you please add a symlink in the old group pointing outside the home?
to check that the link is regrouped and its target is not
There was a problem hiding this comment.
Added to the shared setup (c26923d): link in the old group points to outside, a file next to the home. After -g and after -u -g, the link is regrouped and outside keeps 1000:1000.
| std::os::unix::fs::chown(home.join("sub"), Some(1000), Some(1000)).expect("chown failed"); | ||
|
|
||
| let code = run_with_prefix(&dir, &["-g", "dev", "testuser"]); | ||
| assert_eq!(code, 0, "changing the primary group should succeed"); |
There was a problem hiding this comment.
please add a -u -g test too, both changes go through the same chownat
There was a problem hiding this comment.
Added test_change_uid_and_gid_rechowns_home_files (c26923d): -u 1500 -g dev on the same home. Each entry gets the new UID if it had the old one, and the new group if it had the old one; the root-owned file only changes group.
|
|
||
| let gid = |p: &str| { | ||
| std::fs::symlink_metadata(home.join(p)) | ||
| .expect("stat failed") |
There was a problem hiding this comment.
same closure in both tests, could be a helper, no?
There was a problem hiding this comment.
Yes: owner_of returns (uid, gid) without following symlinks, and setup_home_with_files builds the home for both regroup tests (c26923d). Root suite in the Ubuntu 26.04 container: 946 passed.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Fixes #347
-gnow re-groups the home directory the way-ualready re-owns it: in the same walk, an entry whose group is the old primary GID gets the new one, and an entry owned by the old UID gets the new UID. Symlinks are changed themselves, not followed. As before, nothing is touched when-dis given or the home directory does not exist.Like GNU, the walk now only runs when the home directory is a real directory owned by the user. A home owned by root (common for service accounts) or a symlinked home is left alone; before this,
-ure-owned files under both. GNU also exits 12 for a symlinked home; shadow-rs still exits 0 there (#351).Tests: root tests for
-gand for-u -gon a home with files in the old group (one owned by root), one in another group, and a symlink in the old group to a file outside the home (the link changes, its target does not), and one for-gand-uon a root-owned and a symlinked home, which must be left alone. The first two fail on main; the third fails without the owner check.cargo test --workspacerun as root in an Ubuntu 26.04 container: 946 passed (943 on main, plus the three new).cargo fmt --checkandcargo clippy --workspace --all-targets -- -D warningsare clean; thepamconfigurations were not run locally. Against GNU shadow 4.17.4 in the same container,-g,-u -g,-uand-g -d, with a user-owned, root-owned and symlinked home, give the same ownership for every file in and outside the home directory.AI tools assisted with this contribution.