From 9f5f9c7e7e9b144ee431d7536aa8600ff19d6393 Mon Sep 17 00:00:00 2001 From: Varun Varma Date: Fri, 9 Oct 2026 01:14:15 -0400 Subject: [PATCH 1/2] chown: set group to user's login group on "USER:" "chown USER:" left the group unchanged instead of setting it to USER's login group like in GNU. --- src/uu/chown/src/chown.rs | 37 ++++++++++++++++-------- tests/by-util/test_chown.rs | 57 +++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index ac71abd3804..574645296a0 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -194,11 +194,13 @@ fn parse_gid(group: &str, spec: &str) -> UResult> { /// * `"owner:group"`, /// * `"owner"`, /// * `":group"`, +/// * `"owner:"`, /// /// and the owner or group can be specified either as an ID or a /// name. The `sep` argument specifies which character to use as a /// separator between the owner and group; calling code should set -/// this to `':'`. +/// this to `':'`. If a separator is given but the group is omitted +/// (as in `"owner:"`), the group is set to the owner's login group. fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { assert!(['.', ':'].contains(&sep)); let mut args = spec.splitn(2, sep); @@ -218,19 +220,20 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option, Option)> { return parse_spec(spec, '.'); } - let uid = parse_uid(user, spec)?; - let gid = parse_gid(group, spec)?; - - if user.chars().next().is_some_and(char::is_numeric) && group.is_empty() && spec != user { - // if the arg starts with an id numeric value, the group isn't set but the separator is provided, - // we should fail with an error - return Err(USimpleError::new( - 1, - translate!("chown-error-invalid-spec", "spec" => spec.quote()), - )); + if !user.is_empty() && group.is_empty() && spec != user { + // A separator was given but the group was not ("owner:"), so the + // group is set to the owner's login group. + return match Passwd::locate(user) { + // Lookup must occur through name, not ID, to be valid here. + Ok(passwd) if passwd.name == user => Ok((Some(passwd.uid), Some(passwd.gid))), + _ => Err(USimpleError::new( + 1, + translate!("chown-error-invalid-spec", "spec" => spec.quote()), + )), + }; } - Ok((uid, gid)) + Ok((parse_uid(user, spec)?, parse_gid(group, spec)?)) } #[cfg(test)] @@ -252,6 +255,16 @@ mod test { assert!(format!("{}", parse_spec("..", ':').err().unwrap()).starts_with("invalid group: ")); } + /// Test that "USER:" sets the group to the user's login group. + #[test] + fn test_parse_spec_login_group() { + if let Ok(passwd) = Passwd::locate("root") { + let (uid, gid) = parse_spec("root:", ':').unwrap(); + assert_eq!(uid, Some(passwd.uid)); + assert_eq!(gid, Some(passwd.gid)); + } + } + /// Test for parsing IDs that don't correspond to a named user or group. #[test] fn test_parse_spec_nameless_ids() { diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 534c690dd79..8c3862d6439 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -165,6 +165,63 @@ fn test_chown_only_owner_colon() { .stderr_contains("failed to change"); } +/// Test that `chown USER:` sets the group to USER's login group. +#[test] +#[cfg(all(unix, not(target_os = "openbsd")))] +fn test_chown_login_group() { + use chown::entries::usr2gid; + + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + let file1 = "test_chown_login_group"; + at.touch(file1); + + let Ok(root_gid) = usr2gid("root") else { + print!("Test skipped; no root user"); + return; + }; + let other_gid = root_gid.wrapping_add(1).to_string(); + + // Move the file's group out of root's login group. Changing the + // ownership to root requires root privileges. + let Ok(result) = run_ucmd_as_root(&ts, &[&format!("0:{other_gid}"), file1]) else { + print!("Test skipped; requires root user"); + return; + }; + result.success().no_output(); + + // "root:" must set the group back to root's login group. + let Ok(result) = run_ucmd_as_root(&ts, &["root:", file1]) else { + print!("Test skipped; requires root user"); + return; + }; + result.success().no_output(); + + assert_eq!(at.metadata(file1).gid(), root_gid); +} + +/// Test that an owner whose login group cannot be resolved is an invalid spec. +#[test] +fn test_chown_invalid_spec() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.touch("f"); + + scene + .ucmd() + .arg("auserthatdoesntexist:") + .arg("f") + .fails() + .stderr_contains("chown: invalid spec: 'auserthatdoesntexist:'"); + + scene + .ucmd() + .arg("12345:") + .arg("f") + .fails() + .stderr_contains("chown: invalid spec: '12345:'"); +} + #[test] fn test_chown_dot_separator_warning() { // test that using '.' as separator emits a warning From 3642771761fca7e37f9afd69e6415d047cbc4cca Mon Sep 17 00:00:00 2001 From: Varun Varma Date: Fri, 9 Oct 2026 12:56:18 -0400 Subject: [PATCH 2/2] tests: chown: correct only_owner_colon test for login group semantics --- tests/by-util/test_chown.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 8c3862d6439..c2daf8f4663 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -135,16 +135,25 @@ fn test_chown_only_owner_colon() { let user_name = String::from(result.stdout_str().trim()); assert!(!user_name.is_empty()); + let result = scene.cmd("id").arg("-gn").run(); + if skipping_test_is_okay(&result, "id: cannot find name for group ID") { + return; + } + let login_group_name = String::from(result.stdout_str().trim()); + let expected = format!("{user_name}:{login_group_name}"); + let file1 = "test_chown_file1"; at.touch(file1); + // "username:" sets the owner and the owner's login group; the + // ownership is "retained" only if the group already matches. scene .ucmd() .arg(format!("{user_name}:")) .arg("--verbose") .arg(file1) .succeeds() - .stdout_contains("retained as") + .stdout_contains(&expected) .no_stderr(); scene @@ -153,7 +162,7 @@ fn test_chown_only_owner_colon() { .arg("--verbose") .arg(file1) .succeeds() - .stdout_contains("retained as") + .stdout_contains(&expected) .stderr_contains("warning: '.' should be ':'"); scene