Do not follow reparse points when removing a directory tree on Windows

Sources/System/FilePath/FilePathTempWindows.swift

@@ -35,7 +35,7 @@ internal func _getTemporaryDirectory() throws -> FilePath {
 fileprivate func forEachFile(
   at path: FilePath,
   _ body: (WIN32_FIND_DATAW) throws -> ()
-) rethrows {
+) throws {
   let searchPath = path.appending("\\*")
 
   try searchPath.withPlatformString { szPath in
@@ -59,6 +59,15 @@ fileprivate func forEachFile(
 
       try body(findData)
     } while FindNextFileW(hFind, &findData)
+
+    // FindNextFileW returns false both at the end of the enumeration and on
+    // error; only ERROR_NO_MORE_FILES is the normal terminator. Treating a
+    // transient error as end-of-directory would silently skip the remaining
+    // entries and leave the tree partially deleted.
+    let error = GetLastError()
+    if error != ERROR_NO_MORE_FILES {
+      throw Errno(windowsError: error)
+    }
   }
 }
 
@@ -73,14 +82,28 @@ internal func _recursiveRemove(
 ) throws {
   // First, deal with subdirectories
   try forEachFile(at: path) { findData in
-    if (findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 {
-      let name = withUnsafeBytes(of: findData.cFileName) {
-        return SystemString(platformString: $0.assumingMemoryBound(
-                              to: CInterop.PlatformChar.self).baseAddress!)
-      }
-      let component = FilePath.Component(name)!
-      let subpath = path.appending(component)
+    guard (findData.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) != 0 else {
+      return
+    }
+
+    let name = withUnsafeBytes(of: findData.cFileName) {
+      return SystemString(platformString: $0.assumingMemoryBound(
+                            to: CInterop.PlatformChar.self).baseAddress!)
+    }
+    let component = FilePath.Component(name)!
+    let subpath = path.appending(component)
 
+    // A directory that is also a reparse point (a junction or directory
+    // symlink) must not be recursed into: enumerating it would traverse into
+    // the *target* and delete its contents. Remove the link itself instead;
+    // RemoveDirectoryW deletes the reparse point without touching the target.
+    if (findData.dwFileAttributes & DWORD(FILE_ATTRIBUTE_REPARSE_POINT)) != 0 {
+      try subpath.withPlatformString { subpath in
+        if try !subpath.withCanonicalPathRepresentation({ RemoveDirectoryW($0) }) {
+          throw Errno(windowsError: GetLastError())
+        }
+      }
+    } else {
       try _recursiveRemove(at: subpath)
     }
   }

Tests/SystemTests/FilePathTests/FilePathTempTest.swift

@@ -8,13 +8,18 @@
 */
 
 import XCTest
+import Foundation
 
 #if SYSTEM_PACKAGE
 @testable import SystemPackage
 #else
 @testable import System
 #endif
 
+#if os(Windows)
+import WinSDK
+#endif
+
 final class TemporaryPathTest: XCTestCase {
   #if SYSTEM_PACKAGE_DARWIN
   func testNotInSlashTmp() throws {
@@ -55,4 +60,62 @@ final class TemporaryPathTest: XCTestCase {
       XCTAssertEqual(error as! Errno, Errno.noSuchFileOrDirectory)
     }
   }
+
+  #if os(Windows)
+  /// Recursive removal must not follow directory reparse points (junctions or
+  /// directory symlinks). Enumerating one traverses into its *target*, so a
+  /// naive recursion would delete files that live outside the tree being
+  /// removed.
+  func testCleanupDoesNotFollowReparsePoints() throws {
+    func createDirectory(_ path: FilePath) throws {
+      try path.withPlatformString {
+        if CreateDirectoryW($0, nil) == false {
+          throw Errno(windowsError: GetLastError())
+        }
+      }
+    }
+
+    let tmp = try _getTemporaryDirectory()
+    let victim = tmp.appending("ss-reparse-victim")
+    let container = tmp.appending("ss-reparse-container")
+    // Start from a clean slate and always clean up afterwards.
+    try? _recursiveRemove(at: victim)
+    try? _recursiveRemove(at: container)
+    defer {
+      try? _recursiveRemove(at: victim)
+      try? _recursiveRemove(at: container)
+    }
+
+    // A file that lives outside the tree we are about to remove, and so must
+    // survive it.
+    try createDirectory(victim)
+    let victimFile = victim.appending("keep.txt")
+    let vfd = try FileDescriptor.open(victimFile, .readWrite,
+                                      options: [.create, .truncate],
+                                      permissions: .ownerReadWrite)
+    try vfd.closeAfter { try vfd.writeAll("keep".utf8) }
+
+    // A directory junction inside the container, pointing at the victim.
+    // A junction (mklink /J) is a reparse point that, unlike a symlink, needs
+    // no special privilege, so this exercises the reparse-point path in CI too.
+    try createDirectory(container)
+    let link = container.appending("link")
+    let mklink = Process()
+    mklink.executableURL = URL(fileURLWithPath: "C:\\Windows\\System32\\cmd.exe")
+    mklink.arguments = ["/c", "mklink", "/J",
+                        String(decoding: link), String(decoding: victim)]
+    mklink.standardOutput = nil
+    mklink.standardError = nil
+    try mklink.run()
+    mklink.waitUntilExit()
+    try XCTSkipUnless(mklink.terminationStatus == 0,
+                      "could not create a directory junction")
+
+    // Removing the container must delete the link, not what it points at.
+    try _recursiveRemove(at: container)
+
+    let reopened = try FileDescriptor.open(victimFile, .readOnly)
+    try reopened.close()
+  }
+  #endif
 }