chown: set group to owner's login group on "OWNER:"

src/uu/chown/src/chown.rs

@@ -194,11 +194,13 @@ fn parse_gid(group: &str, spec: &str) -> UResult<Option<u32>> {
 /// * `"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<u32>, Option<u32>)> {
     assert!(['.', ':'].contains(&sep));
     let mut args = spec.splitn(2, sep);
@@ -218,19 +220,20 @@ fn parse_spec(spec: &str, sep: char) -> UResult<(Option<u32>, Option<u32>)> {
         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() {

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
@@ -165,6 +174,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