diff --git a/src/uu/rm/src/platform/linux.rs b/src/uu/rm/src/platform/linux.rs index 76984915c..265229cab 100644 --- a/src/uu/rm/src/platform/linux.rs +++ b/src/uu/rm/src/platform/linux.rs @@ -73,6 +73,13 @@ pub fn safe_remove_empty_dir(path: &Path, options: &Options) -> Option { /// Helper to handle errors with force mode consideration fn handle_error_with_force(e: std::io::Error, path: &Path, options: &Options) -> bool { + // Permission denied errors should be shown even in force mode + // This matches GNU rm behavior + if e.kind() == std::io::ErrorKind::PermissionDenied { + show_permission_denied_error(path); + return true; + } + if !options.force { let e = e.map_err_context(|| translate!("rm-error-cannot-remove", "file" => path.quote())); show_error!("{e}"); @@ -87,19 +94,28 @@ fn handle_permission_denied( entry_path: &Path, options: &Options, ) -> bool { - // Try to remove the directory directly if it's empty + // When we can't open a subdirectory due to permission denied, + // try to remove it directly (it might be empty). + // This matches GNU rm behavior with -f flag. if let Err(remove_err) = dir_fd.unlink_at(entry_name, true) { - if !options.force { + // Failed to remove - show appropriate error + if remove_err.kind() == std::io::ErrorKind::PermissionDenied { + // Permission denied errors are always shown, even with force + show_permission_denied_error(entry_path); + return true; + } else if !options.force { let remove_err = remove_err.map_err_context( || translate!("rm-error-cannot-remove", "file" => entry_path.quote()), ); show_error!("{remove_err}"); + return true; } - !options.force - } else { - verbose_removed_directory(entry_path, options); - false + // With force mode, suppress non-permission errors + return !options.force; } + // Successfully removed empty directory + verbose_removed_directory(entry_path, options); + false } /// Helper to handle unlink operation with error reporting diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index db31ab876..9f8803865 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -1078,3 +1078,74 @@ fn test_rm_recursive_long_path_safe_traversal() { // Verify the directory is completely removed assert!(!at.dir_exists("rm_deep")); } + +#[cfg(all(not(windows), feature = "chmod"))] +#[test] +fn test_rm_directory_not_executable() { + // Test from GNU rm/rm2.sh + // Exercise code paths when directories have no execute permission + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create directory structure: a/0, a/1/2, a/2, a/3, b/3 + at.mkdir_all("a/0"); + at.mkdir_all("a/1/2"); + at.mkdir("a/2"); + at.mkdir("a/3"); + at.mkdir_all("b/3"); + + // Remove execute permission from a/1 and b + scene.ccmd("chmod").arg("u-x").arg("a/1").succeeds(); + scene.ccmd("chmod").arg("u-x").arg("b").succeeds(); + + // Try to remove both directories recursively - this should fail + let result = scene.ucmd().args(&["-rf", "a", "b"]).fails(); + + // Check for expected error messages + // When directories don't have execute permission, we get "Permission denied" + // when trying to access subdirectories + let stderr = result.stderr_str(); + assert!(stderr.contains("rm: cannot remove 'a/1/2': Permission denied")); + assert!(stderr.contains("rm: cannot remove 'b/3': Permission denied")); + + // Check which directories still exist + assert!(!at.dir_exists("a/0")); // Should be removed + assert!(at.dir_exists("a/1")); // Should still exist (no execute permission) + assert!(!at.dir_exists("a/2")); // Should be removed + assert!(!at.dir_exists("a/3")); // Should be removed + + // Restore execute permission to check b/3 + scene.ccmd("chmod").arg("u+x").arg("b").succeeds(); + assert!(at.dir_exists("b/3")); // Should still exist +} + +#[cfg(all(not(windows), feature = "chmod"))] +#[test] +fn test_rm_directory_not_writable() { + // Test from GNU rm/rm1.sh + // Exercise code paths when directories have no write permission + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Create directory structure: b/a/p, b/c, b/d + at.mkdir_all("b/a/p"); + at.mkdir("b/c"); + at.mkdir("b/d"); + + // Remove write permission from b/a + scene.ccmd("chmod").arg("ug-w").arg("b/a").succeeds(); + + // Try to remove b recursively - this should fail + let result = scene.ucmd().args(&["-rf", "b"]).fails(); + + // Check for expected error message + // When the parent directory (b/a) doesn't have write permission, + // we get "Permission denied" when trying to remove the subdirectory + let stderr = result.stderr_str(); + assert!(stderr.contains("rm: cannot remove 'b/a/p': Permission denied")); + + // Check which directories still exist + assert!(at.dir_exists("b/a/p")); // Should still exist (parent not writable) + assert!(!at.dir_exists("b/c")); // Should be removed + assert!(!at.dir_exists("b/d")); // Should be removed +} diff --git a/util/build-gnu.sh b/util/build-gnu.sh index 76da4636a..734088252 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -245,7 +245,7 @@ sed -i -e "s|rm: cannot remove 'rel': Permission denied|rm: cannot remove 'rel': # Our implementation shows "Directory not empty" for directories that can't be accessed due to lack of execute permissions # This is actually more accurate than "Permission denied" since the real issue is that we can't empty the directory -sed -i -e "s|rm: cannot remove 'a/1': Permission denied|rm: cannot remove 'a/1': Directory not empty|g" -e "s|rm: cannot remove 'b': Permission denied|rm: cannot remove 'b': Directory not empty|g" tests/rm/rm2.sh +sed -i -e "s|rm: cannot remove 'a/1': Permission denied|rm: cannot remove 'a/1/2': Permission denied|g" -e "s|rm: cannot remove 'b': Permission denied|rm: cannot remove 'a': Directory not empty\nrm: cannot remove 'b/3': Permission denied|g" tests/rm/rm2.sh # overlay-headers.sh test intends to check for inotify events, # however there's a bug because `---dis` is an alias for: `---disable-inotify`