Skip to content

rm: remove directory symlinks via the directory-removal API on Windows - #14159

Open
Uarz wants to merge 1 commit into
uutils:mainfrom
Uarz:fix/rm-dir-symlink-uutils
Open

rm: remove directory symlinks via the directory-removal API on Windows#14159
Uarz wants to merge 1 commit into
uutils:mainfrom
Uarz:fix/rm-dir-symlink-uutils

Conversation

@Uarz

@Uarz Uarz commented Aug 26, 2026

Copy link
Copy Markdown

rm -r over a directory tree that contains a symbolic link to a directory fails on Windows with cannot remove ...: Permission denied.

In remove_dir_recursive, the symlink base case routes every symbolic link through remove_file (std::fs::remove_file / DeleteFileW). On Windows a directory symlink (a directory reparse point) cannot be deleted that way — DeleteFileW returns ERROR_ACCESS_DENIED. It needs RemoveDirectoryW (std::fs::remove_dir), which removes the reparse point itself instead of following the link.

Route directory symlinks through remove_dir via the existing is_symlink_dir helper, matching how the top-level remove already handles them. Non-Windows is unchanged (the new branch is #[cfg(windows)] and otherwise falls back to remove_file).

Added a regression test in tests/by-util/test_rm.rs. It needs SeCreateSymbolicLinkPrivilege, like the existing test_symlink_dir, so it runs on Windows CI rather than privilege-less dev boxes.

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.
Comment thread src/uu/rm/src/rm.rs
Comment on lines +669 to +675
//
// 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make the comment shorter

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout-group (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/dd/no-allocate is now passing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants