From af5c4eafe17db5288496483be52d0107fa6950af Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 22:02:40 +0000 Subject: [PATCH 1/4] Initial plan From 125387c3a5f2b60b751c9beafca2d17098bfd536 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 22:33:49 +0000 Subject: [PATCH 2/4] Fix Directory.Delete recursive failing on directories containing junctions - Use a local `mountPointException` variable in RemoveDirectoryRecursive when handling IO_REPARSE_TAG_MOUNT_POINT reparse points. Previously, if DeleteVolumeMountPoint() failed (as it always does for directory junctions, which share the same reparse tag as volume mount points), the error was stored in `exception`, causing the parent directory deletion to fail even though RemoveDirectory() would succeed. - Now mountPointException is only promoted to exception if RemoveDirectory also fails. If RemoveDirectory succeeds (the normal case for junctions), the DeleteVolumeMountPoint error is discarded. - Add test RecursiveDelete_DirectoryContainingJunction to verify the fix. Co-authored-by: ViveliDuCh <50237907+ViveliDuCh@users.noreply.github.com> --- .../src/System/IO/FileSystem.Windows.cs | 27 ++++++++++++++----- .../Directory/Delete.Windows.cs | 27 +++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index cc29702789a71f..a49078fd4bbbee 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -357,32 +357,45 @@ private static void RemoveDirectoryRecursive(string fullPath, ref Interop.Kernel else { // Name surrogate reparse point, don't recurse, simply remove the directory. - // If a mount point, we have to delete the mount point first. + // If a volume mount point, we have to delete the mount point first. + // Note that IO_REPARSE_TAG_MOUNT_POINT is used for both volume mount points + // and directory junctions. DeleteVolumeMountPoint only works for volume mount + // points; for directory junctions, RemoveDirectory handles removal directly. + Exception? mountPointException = null; if (findData.dwReserved0 == Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) { // Mount point. Unmount using full path plus a trailing '\'. // (Note: This doesn't remove the underlying directory) string mountPoint = Path.Join(fullPath, fileName, PathInternal.DirectorySeparatorCharAsString); - if (!Interop.Kernel32.DeleteVolumeMountPoint(mountPoint) && exception == null) + if (!Interop.Kernel32.DeleteVolumeMountPoint(mountPoint)) { errorCode = Marshal.GetLastPInvokeError(); if (errorCode != Interop.Errors.ERROR_SUCCESS && errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) { - exception = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); + mountPointException = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); } } } // Note that RemoveDirectory on a symbolic link will remove the link itself. - if (!Interop.Kernel32.RemoveDirectory(Path.Combine(fullPath, fileName)) && exception == null) + if (!Interop.Kernel32.RemoveDirectory(Path.Combine(fullPath, fileName))) { - errorCode = Marshal.GetLastPInvokeError(); - if (errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) + if (exception == null) { - exception = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); + errorCode = Marshal.GetLastPInvokeError(); + if (errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) + { + // For a true volume mount point, use its error (it indicates why the + // unmount step failed). If this is a directory junction, RemoveDirectory + // succeeds and this code path is not reached. + exception = mountPointException ?? Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); + } } } + // If RemoveDirectory succeeded, mountPointException is discarded. This correctly + // handles directory junctions: DeleteVolumeMountPoint fails for them (since they + // are not volume mount points), but RemoveDirectory removes them successfully. } } } while (Interop.Kernel32.FindNextFile(handle, ref findData)); diff --git a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Delete.Windows.cs b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Delete.Windows.cs index afe712f9fbab90..84360ffd5a91bb 100644 --- a/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Delete.Windows.cs +++ b/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/Delete.Windows.cs @@ -11,6 +11,33 @@ namespace System.IO.Tests { public partial class Directory_Delete_str_bool : Directory_Delete_str { + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public void RecursiveDelete_DirectoryContainingJunction() + { + // Junctions (NTFS directory junctions) share the IO_REPARSE_TAG_MOUNT_POINT reparse + // tag with volume mount points, but DeleteVolumeMountPoint only works for volume mount + // points. Ensure that recursive delete succeeds when the directory contains a junction. + string target = GetTestFilePath(); + Directory.CreateDirectory(target); + + string linkParent = GetTestFilePath(); + Directory.CreateDirectory(linkParent); + + string junctionPath = Path.Combine(linkParent, GetTestFileName()); + Assert.True(MountHelper.CreateJunction(junctionPath, target)); + + // Both the junction and the target exist before deletion + Assert.True(Directory.Exists(junctionPath), "junction should exist before delete"); + Assert.True(Directory.Exists(target), "target should exist before delete"); + + // Recursive delete of the parent should succeed and remove the junction without following it + Delete(linkParent, recursive: true); + + Assert.False(Directory.Exists(linkParent), "parent should be deleted"); + Assert.True(Directory.Exists(target), "target should still exist after deleting junction"); + } + [Fact] [PlatformSpecific(TestPlatforms.Windows)] public void RecursiveDelete_NoListDirectoryPermission() // https://github.com/dotnet/runtime/issues/56922 From ec07f5b096fbc3b2cc27b008703f19e55d14af20 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 22:34:30 +0000 Subject: [PATCH 3/4] Add technical explanation document for Directory.Delete junction fix Co-authored-by: ViveliDuCh <50237907+ViveliDuCh@users.noreply.github.com> --- docs/fix-directory-delete-junctions.md | 126 +++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 docs/fix-directory-delete-junctions.md diff --git a/docs/fix-directory-delete-junctions.md b/docs/fix-directory-delete-junctions.md new file mode 100644 index 00000000000000..1d1fa6f023daca --- /dev/null +++ b/docs/fix-directory-delete-junctions.md @@ -0,0 +1,126 @@ +# Fix: `Directory.Delete(path, recursive: true)` Fails on Directories Containing Junctions + +## Background + +On Windows, NTFS supports several types of reparse points for directory traversal: + +| Type | Reparse Tag | Created by | +|---|---|---| +| Symbolic link | `IO_REPARSE_TAG_SYMLINK` (0xA000000C) | `mklink /D`, `Directory.CreateSymbolicLink` | +| Directory junction | `IO_REPARSE_TAG_MOUNT_POINT` (0xA0000003) | `mklink /J`, popular tools like `pnpm` | +| Volume mount point | `IO_REPARSE_TAG_MOUNT_POINT` (0xA0000003) | `mountvol`, disk management | + +**Key insight**: directory junctions and volume mount points share the same reparse tag (`IO_REPARSE_TAG_MOUNT_POINT`). The difference is in their reparse data: +- Volume mount points store a volume GUID path (`\??\Volume{GUID}\`) +- Directory junctions store a target directory path (`\??\C:\path\to\target`) + +## Root Cause + +`Directory.Delete(path, recursive: true)` on Windows is implemented in +`FileSystem.Windows.cs` in the `RemoveDirectoryRecursive` method. When the code +encounters a name-surrogate reparse point with the `IO_REPARSE_TAG_MOUNT_POINT` tag, +it calls `DeleteVolumeMountPoint` before `RemoveDirectory`: + +```csharp +// BEFORE (buggy) +if (findData.dwReserved0 == Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) +{ + string mountPoint = Path.Join(fullPath, fileName, PathInternal.DirectorySeparatorCharAsString); + if (!Interop.Kernel32.DeleteVolumeMountPoint(mountPoint) && exception == null) + { + errorCode = Marshal.GetLastPInvokeError(); + if (errorCode != Interop.Errors.ERROR_SUCCESS && + errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) + { + exception = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); // <-- sets exception + } + } +} + +// Note that RemoveDirectory on a symbolic link will remove the link itself. +if (!Interop.Kernel32.RemoveDirectory(Path.Combine(fullPath, fileName)) && exception == null) +{ + // exception is already set from above, so this block is skipped + // even though RemoveDirectory succeeds! +} +``` + +`DeleteVolumeMountPoint` is a Windows API that only works for **volume mount points** — it cannot unmount a directory junction. When called on a junction, it fails and sets an error code (e.g., `ERROR_INVALID_PARAMETER` or `ERROR_NOT_A_REPARSE_POINT`). + +This error was stored directly in the outer `exception` variable. Then `RemoveDirectory` was called and **succeeded** (removing the junction), but the error from `DeleteVolumeMountPoint` was already stored. After the loop, `exception != null` caused the exception to be thrown and `RemoveDirectoryInternal` (which removes the parent directory) was never called, leaving the parent directory behind. + +## The Fix + +The fix uses a local `mountPointException` variable to capture the `DeleteVolumeMountPoint` failure, without storing it in the loop's `exception` variable. This exception is only promoted to `exception` if `RemoveDirectory` **also** fails — meaning the directory could not be removed by either mechanism. + +```csharp +// AFTER (fixed) +Exception? mountPointException = null; +if (findData.dwReserved0 == Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) +{ + string mountPoint = Path.Join(fullPath, fileName, PathInternal.DirectorySeparatorCharAsString); + if (!Interop.Kernel32.DeleteVolumeMountPoint(mountPoint)) + { + errorCode = Marshal.GetLastPInvokeError(); + if (errorCode != Interop.Errors.ERROR_SUCCESS && + errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) + { + mountPointException = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); + } + } +} + +if (!Interop.Kernel32.RemoveDirectory(Path.Combine(fullPath, fileName))) +{ + if (exception == null) + { + errorCode = Marshal.GetLastPInvokeError(); + if (errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) + { + // For a true volume mount point, use its error (it indicates why the + // unmount step failed). If this is a directory junction, RemoveDirectory + // succeeds and this code path is not reached. + exception = mountPointException ?? Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); + } + } +} +// If RemoveDirectory succeeded, mountPointException is discarded. This correctly +// handles directory junctions: DeleteVolumeMountPoint fails for them (since they +// are not volume mount points), but RemoveDirectory removes them successfully. +``` + +## Behavior Matrix + +| Scenario | `DeleteVolumeMountPoint` | `RemoveDirectory` | Result | +|---|---|---|---| +| Volume mount point (normal) | ✅ succeeds | ✅ succeeds | Directory removed, no exception | +| Volume mount point (access denied) | ❌ fails | ❌ fails | Exception from `DeleteVolumeMountPoint` (descriptive) | +| Directory junction | ❌ fails (expected) | ✅ succeeds | `mountPointException` discarded, directory removed ✅ **BUG FIX** | +| Symbolic link | N/A (different tag) | ✅ succeeds | Directory removed, no exception | +| Symlink (access denied) | N/A | ❌ fails | Exception from `RemoveDirectory` | + +## Test Coverage + +A new Windows-specific test `RecursiveDelete_DirectoryContainingJunction` was added to +`Directory/Delete.Windows.cs`. It: + +1. Creates a parent directory and a separate target directory +2. Creates a junction inside the parent pointing to the target (using `MountHelper.CreateJunction`) +3. Calls `Directory.Delete(linkParent, recursive: true)` and asserts it succeeds +4. Verifies the parent directory is deleted and the target directory still exists + (the junction should not be followed) + +This test would have failed with the old code (throwing `UnauthorizedAccessException` or +`IOException` depending on privilege level) and passes with the fix. + +## Platforms Affected + +This bug is Windows-only. Directory junctions are an NTFS-specific concept and the affected +code path (`FileSystem.Windows.cs`) is only compiled for Windows. The fix has no impact on +Linux or macOS behavior. + +## Impact + +Directory junctions are commonly created by package managers (notably `pnpm`) in +`node_modules` directories. Any application doing recursive directory cleanup on directories +created by these tools was affected by this bug. From 4d14f78d3f3b4e1e77da06577d75e3fd5fe3b77e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Feb 2026 18:57:20 +0000 Subject: [PATCH 4/4] Remove docs/fix-directory-delete-junctions.md Co-authored-by: ViveliDuCh <50237907+ViveliDuCh@users.noreply.github.com> --- docs/fix-directory-delete-junctions.md | 126 ------------------------- 1 file changed, 126 deletions(-) delete mode 100644 docs/fix-directory-delete-junctions.md diff --git a/docs/fix-directory-delete-junctions.md b/docs/fix-directory-delete-junctions.md deleted file mode 100644 index 1d1fa6f023daca..00000000000000 --- a/docs/fix-directory-delete-junctions.md +++ /dev/null @@ -1,126 +0,0 @@ -# Fix: `Directory.Delete(path, recursive: true)` Fails on Directories Containing Junctions - -## Background - -On Windows, NTFS supports several types of reparse points for directory traversal: - -| Type | Reparse Tag | Created by | -|---|---|---| -| Symbolic link | `IO_REPARSE_TAG_SYMLINK` (0xA000000C) | `mklink /D`, `Directory.CreateSymbolicLink` | -| Directory junction | `IO_REPARSE_TAG_MOUNT_POINT` (0xA0000003) | `mklink /J`, popular tools like `pnpm` | -| Volume mount point | `IO_REPARSE_TAG_MOUNT_POINT` (0xA0000003) | `mountvol`, disk management | - -**Key insight**: directory junctions and volume mount points share the same reparse tag (`IO_REPARSE_TAG_MOUNT_POINT`). The difference is in their reparse data: -- Volume mount points store a volume GUID path (`\??\Volume{GUID}\`) -- Directory junctions store a target directory path (`\??\C:\path\to\target`) - -## Root Cause - -`Directory.Delete(path, recursive: true)` on Windows is implemented in -`FileSystem.Windows.cs` in the `RemoveDirectoryRecursive` method. When the code -encounters a name-surrogate reparse point with the `IO_REPARSE_TAG_MOUNT_POINT` tag, -it calls `DeleteVolumeMountPoint` before `RemoveDirectory`: - -```csharp -// BEFORE (buggy) -if (findData.dwReserved0 == Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) -{ - string mountPoint = Path.Join(fullPath, fileName, PathInternal.DirectorySeparatorCharAsString); - if (!Interop.Kernel32.DeleteVolumeMountPoint(mountPoint) && exception == null) - { - errorCode = Marshal.GetLastPInvokeError(); - if (errorCode != Interop.Errors.ERROR_SUCCESS && - errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) - { - exception = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); // <-- sets exception - } - } -} - -// Note that RemoveDirectory on a symbolic link will remove the link itself. -if (!Interop.Kernel32.RemoveDirectory(Path.Combine(fullPath, fileName)) && exception == null) -{ - // exception is already set from above, so this block is skipped - // even though RemoveDirectory succeeds! -} -``` - -`DeleteVolumeMountPoint` is a Windows API that only works for **volume mount points** — it cannot unmount a directory junction. When called on a junction, it fails and sets an error code (e.g., `ERROR_INVALID_PARAMETER` or `ERROR_NOT_A_REPARSE_POINT`). - -This error was stored directly in the outer `exception` variable. Then `RemoveDirectory` was called and **succeeded** (removing the junction), but the error from `DeleteVolumeMountPoint` was already stored. After the loop, `exception != null` caused the exception to be thrown and `RemoveDirectoryInternal` (which removes the parent directory) was never called, leaving the parent directory behind. - -## The Fix - -The fix uses a local `mountPointException` variable to capture the `DeleteVolumeMountPoint` failure, without storing it in the loop's `exception` variable. This exception is only promoted to `exception` if `RemoveDirectory` **also** fails — meaning the directory could not be removed by either mechanism. - -```csharp -// AFTER (fixed) -Exception? mountPointException = null; -if (findData.dwReserved0 == Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) -{ - string mountPoint = Path.Join(fullPath, fileName, PathInternal.DirectorySeparatorCharAsString); - if (!Interop.Kernel32.DeleteVolumeMountPoint(mountPoint)) - { - errorCode = Marshal.GetLastPInvokeError(); - if (errorCode != Interop.Errors.ERROR_SUCCESS && - errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) - { - mountPointException = Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); - } - } -} - -if (!Interop.Kernel32.RemoveDirectory(Path.Combine(fullPath, fileName))) -{ - if (exception == null) - { - errorCode = Marshal.GetLastPInvokeError(); - if (errorCode != Interop.Errors.ERROR_PATH_NOT_FOUND) - { - // For a true volume mount point, use its error (it indicates why the - // unmount step failed). If this is a directory junction, RemoveDirectory - // succeeds and this code path is not reached. - exception = mountPointException ?? Win32Marshal.GetExceptionForWin32Error(errorCode, fileName); - } - } -} -// If RemoveDirectory succeeded, mountPointException is discarded. This correctly -// handles directory junctions: DeleteVolumeMountPoint fails for them (since they -// are not volume mount points), but RemoveDirectory removes them successfully. -``` - -## Behavior Matrix - -| Scenario | `DeleteVolumeMountPoint` | `RemoveDirectory` | Result | -|---|---|---|---| -| Volume mount point (normal) | ✅ succeeds | ✅ succeeds | Directory removed, no exception | -| Volume mount point (access denied) | ❌ fails | ❌ fails | Exception from `DeleteVolumeMountPoint` (descriptive) | -| Directory junction | ❌ fails (expected) | ✅ succeeds | `mountPointException` discarded, directory removed ✅ **BUG FIX** | -| Symbolic link | N/A (different tag) | ✅ succeeds | Directory removed, no exception | -| Symlink (access denied) | N/A | ❌ fails | Exception from `RemoveDirectory` | - -## Test Coverage - -A new Windows-specific test `RecursiveDelete_DirectoryContainingJunction` was added to -`Directory/Delete.Windows.cs`. It: - -1. Creates a parent directory and a separate target directory -2. Creates a junction inside the parent pointing to the target (using `MountHelper.CreateJunction`) -3. Calls `Directory.Delete(linkParent, recursive: true)` and asserts it succeeds -4. Verifies the parent directory is deleted and the target directory still exists - (the junction should not be followed) - -This test would have failed with the old code (throwing `UnauthorizedAccessException` or -`IOException` depending on privilege level) and passes with the fix. - -## Platforms Affected - -This bug is Windows-only. Directory junctions are an NTFS-specific concept and the affected -code path (`FileSystem.Windows.cs`) is only compiled for Windows. The fix has no impact on -Linux or macOS behavior. - -## Impact - -Directory junctions are commonly created by package managers (notably `pnpm`) in -`node_modules` directories. Any application doing recursive directory cleanup on directories -created by these tools was affected by this bug.