From 4af1b8b0ec0cb9ad79b1e7d3bbf8d304e738a4df Mon Sep 17 00:00:00 2001 From: Uarz <149165079+Uarz@users.noreply.github.com> Date: Thu, 27 Aug 2026 02:54:21 +0800 Subject: [PATCH] rm: remove directory symlinks via the directory-removal API on Windows In remove_dir_recursive, the symlink base case routed every symbolic link through remove_file (std::fs::remove_file / DeleteFileW). On Windows a symbolic link to a directory (a directory reparse point) cannot be deleted this way: DeleteFileW returns ERROR_ACCESS_DENIED, so 'rm -r' over a tree containing a directory symlink fails with 'cannot remove ...: Permission denied'. Route directory symlinks through remove_dir (std::fs::remove_dir / RemoveDirectoryW) via the existing is_symlink_dir helper, matching how the top-level remove() already handles them. Non-Windows behavior is unchanged; the new branch is #[cfg(windows)] and otherwise falls back to remove_file. Adds a regression test in tests/by-util/test_rm.rs. --- src/uu/rm/src/rm.rs | 19 ++++++++++++++++++- tests/by-util/test_rm.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index f1436b4bd1..555238b24e 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -666,7 +666,24 @@ fn remove_dir_recursive( // a directory and we don't want to recurse. In particular, this // avoids an infinite recursion in the case of a link to the current // directory, like `ln -s . link`. - if !path.is_dir() || path.is_symlink() { + // + // On Windows, a symbolic link to a directory (a directory reparse + // point) cannot be removed with the file-deletion API used by + // `remove_file` -> it fails with `ERROR_ACCESS_DENIED`. Route such + // links through `remove_dir` (the directory-removal API), which + // removes the reparse point itself instead of following the link. + // This mirrors how the top-level `remove` already handles directory + // symlinks. See microsoft/coreutils#84. + if path.is_symlink() { + #[cfg(windows)] + if let Ok(metadata) = fs::symlink_metadata(path) { + if is_symlink_dir(&metadata) { + return remove_dir(path, options, progress_bar); + } + } + return remove_file(path, options, progress_bar); + } + if !path.is_dir() { return remove_file(path, options, progress_bar); } diff --git a/tests/by-util/test_rm.rs b/tests/by-util/test_rm.rs index c315d4112d..cf092c23b9 100644 --- a/tests/by-util/test_rm.rs +++ b/tests/by-util/test_rm.rs @@ -461,6 +461,33 @@ fn test_symlink_dir() { scene.ucmd().arg("-r").arg(link).succeeds(); } +#[test] +fn test_recursive_removes_directory_symlink_in_tree() { + // `rm -r` over a tree that contains a symbolic link to a directory must + // remove the link itself, not follow it, and leave the target intact. + // On Windows this previously failed with "Permission denied" because + // directory symlinks were routed through the file-deletion API + // (DeleteFileW) instead of the directory-removal API (RemoveDirectoryW). + let (at, mut ucmd) = at_and_ucmd!(); + + let target = "test_rm_dir_symlink_in_tree_target"; + let tree = "test_rm_dir_symlink_in_tree"; + let link = "test_rm_dir_symlink_in_tree/link"; + + at.mkdir(target); + at.touch(format!("{target}/keepme")); + at.mkdir(tree); + at.symlink_dir(target, link); + + ucmd.arg("-r").arg(tree).succeeds().no_stderr(); + + assert!(!at.dir_exists(tree)); + // The link is gone (it lived inside `tree`), but the directory it pointed + // at must be untouched. + assert!(at.dir_exists(target)); + assert!(at.file_exists(&format!("{target}/keepme"))); +} + #[test] fn test_invalid_symlink() { let (at, mut ucmd) = at_and_ucmd!();