From 7f3b3dbde7bab3172ff90128c6e116baceedee04 Mon Sep 17 00:00:00 2001 From: Toni Jovanoski Date: Sat, 3 Jan 2026 21:28:01 +0100 Subject: [PATCH 1/5] Fix #10024: chown/chgrp exit code with multiple files Use |= instead of = to accumulate errors when processing multiple files. Previously, if the last file succeeded, the exit code would be 0 even if earlier files failed. Added tests to verify correct exit code behavior. --- src/uucore/src/lib/features/perms.rs | 2 +- tests/by-util/test_chgrp.rs | 35 ++++++++++++++++++++++++++++ tests/by-util/test_chown.rs | 35 ++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 2823b35b1a2..cfe662d7bf2 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -620,7 +620,7 @@ impl ChownExecutor { continue; } - ret = match wrap_chown( + ret |= match wrap_chown( path, &meta, self.dest_uid, diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index cc0727dd3eb..f71e4ecad52 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -640,3 +640,38 @@ fn test_chgrp_recursive_on_file() { current_gid ); } + +#[test] +#[cfg(unix)] +fn test_chgrp_multiple_files_error_on_first_success_on_last() { + use std::os::unix::fs::PermissionsExt; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let groups = nix::unistd::getgroups().unwrap(); + // Skip test if we don't have at least one group to work with + if groups.is_empty() { + return; + } + let current_group = groups[0]; + + at.mkdir("a_readonly_dir"); + at.mkdir("a_readonly_dir/subdir"); + at.touch("a_readonly_dir/subdir/file"); + at.touch("b_writable_file"); + + std::fs::set_permissions( + at.plus("a_readonly_dir/subdir"), + std::fs::Permissions::from_mode(0o000) + ).unwrap(); + + scene + .ucmd() + .arg("-R") + .arg(current_group.to_string()) + .arg("a_readonly_dir") + .arg("b_writable_file") + .fails() + .stderr_contains("Permission denied"); +} \ No newline at end of file diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 2602aabde06..7cfe4ca085b 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -838,3 +838,38 @@ fn test_chown_reference_file() { .stderr_contains("ownership of 'b' retained as") .no_stdout(); } + +#[test] +#[cfg(unix)] +fn test_chown_multiple_files_error_on_first_success_on_last() { + use std::os::unix::fs::PermissionsExt; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let result = scene.cmd("whoami").run(); + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { + return; + } + let user_name = String::from(result.stdout_str().trim()); + assert!(!user_name.is_empty()); + + at.mkdir("a_readonly_dir"); + at.mkdir("a_readonly_dir/subdir"); + at.touch("a_readonly_dir/subdir/file"); + at.touch("b_writable_file"); + + std::fs::set_permissions( + at.plus("a_readonly_dir/subdir"), + std::fs::Permissions::from_mode(0o000) + ).unwrap(); + + scene + .ucmd() + .arg("-R") + .arg(&user_name) + .arg("a_readonly_dir") + .arg("b_writable_file") + .fails() + .stderr_contains("Permission denied"); +} \ No newline at end of file From 83a75ef042f52de483e39a584cefc101d9601a55 Mon Sep 17 00:00:00 2001 From: Toni Jovanoski Date: Sat, 3 Jan 2026 23:11:02 +0100 Subject: [PATCH 2/5] Fix formatting --- tests/by-util/test_chgrp.rs | 7 ++++--- tests/by-util/test_chown.rs | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index f71e4ecad52..041164846b5 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -663,8 +663,9 @@ fn test_chgrp_multiple_files_error_on_first_success_on_last() { std::fs::set_permissions( at.plus("a_readonly_dir/subdir"), - std::fs::Permissions::from_mode(0o000) - ).unwrap(); + std::fs::Permissions::from_mode(0o000), + ) + .unwrap(); scene .ucmd() @@ -674,4 +675,4 @@ fn test_chgrp_multiple_files_error_on_first_success_on_last() { .arg("b_writable_file") .fails() .stderr_contains("Permission denied"); -} \ No newline at end of file +} diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 7cfe4ca085b..50dbcd3fb05 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -861,8 +861,9 @@ fn test_chown_multiple_files_error_on_first_success_on_last() { std::fs::set_permissions( at.plus("a_readonly_dir/subdir"), - std::fs::Permissions::from_mode(0o000) - ).unwrap(); + std::fs::Permissions::from_mode(0o000), + ) + .unwrap(); scene .ucmd() @@ -872,4 +873,4 @@ fn test_chown_multiple_files_error_on_first_success_on_last() { .arg("b_writable_file") .fails() .stderr_contains("Permission denied"); -} \ No newline at end of file +} From fdb7cf75dce079a25440cfe99dcbdbbe56466782 Mon Sep 17 00:00:00 2001 From: Toni Jovanoski Date: Sun, 4 Jan 2026 08:43:17 +0100 Subject: [PATCH 3/5] Don't run chgrp multiple files test on apple The test test_chgrp_multiple_files_error_on_first_success_on_last is disabled on apple scenario. Clean up unnecessary comment. --- tests/by-util/test_chgrp.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 041164846b5..7941d6b5a56 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -642,7 +642,7 @@ fn test_chgrp_recursive_on_file() { } #[test] -#[cfg(unix)] +#[cfg(not(target_vendor = "apple"))] fn test_chgrp_multiple_files_error_on_first_success_on_last() { use std::os::unix::fs::PermissionsExt; @@ -650,7 +650,6 @@ fn test_chgrp_multiple_files_error_on_first_success_on_last() { let at = &scene.fixtures; let groups = nix::unistd::getgroups().unwrap(); - // Skip test if we don't have at least one group to work with if groups.is_empty() { return; } From b72df8b6cce37fd1155ee4b7229d1d3d88216236 Mon Sep 17 00:00:00 2001 From: Toni Jovanoski Date: Sun, 4 Jan 2026 09:06:31 +0100 Subject: [PATCH 4/5] Reenable test on chgrp multiple files error for apple Test reenabled for apple scenario by using getegid(). --- tests/by-util/test_chgrp.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 7941d6b5a56..7d51ba4f47e 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -642,18 +642,13 @@ fn test_chgrp_recursive_on_file() { } #[test] -#[cfg(not(target_vendor = "apple"))] fn test_chgrp_multiple_files_error_on_first_success_on_last() { use std::os::unix::fs::PermissionsExt; let scene = TestScenario::new(util_name!()); let at = &scene.fixtures; - let groups = nix::unistd::getgroups().unwrap(); - if groups.is_empty() { - return; - } - let current_group = groups[0]; + let current_gid = getegid(); at.mkdir("a_readonly_dir"); at.mkdir("a_readonly_dir/subdir"); @@ -669,7 +664,7 @@ fn test_chgrp_multiple_files_error_on_first_success_on_last() { scene .ucmd() .arg("-R") - .arg(current_group.to_string()) + .arg(current_gid.to_string()) .arg("a_readonly_dir") .arg("b_writable_file") .fails() From f06ac2e06e5b25ee738e28a59dac4d6a9415229c Mon Sep 17 00:00:00 2001 From: Toni Jovanoski Date: Sun, 4 Jan 2026 10:30:03 +0100 Subject: [PATCH 5/5] Fix test chggrp multiple files on apple Use at_and_ucmd! macro instead TestScenario. --- tests/by-util/test_chgrp.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 7d51ba4f47e..ede65956fce 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -645,8 +645,7 @@ fn test_chgrp_recursive_on_file() { fn test_chgrp_multiple_files_error_on_first_success_on_last() { use std::os::unix::fs::PermissionsExt; - let scene = TestScenario::new(util_name!()); - let at = &scene.fixtures; + let (at, mut ucmd) = at_and_ucmd!(); let current_gid = getegid(); @@ -661,9 +660,7 @@ fn test_chgrp_multiple_files_error_on_first_success_on_last() { ) .unwrap(); - scene - .ucmd() - .arg("-R") + ucmd.arg("-R") .arg(current_gid.to_string()) .arg("a_readonly_dir") .arg("b_writable_file")