fix: make descriptor path recovery sanitizer-safe (#1451)

Author: batonogov

Pine/TerminationSaveCoordinator.swift

@@ -1113,43 +1113,75 @@ nonisolated enum TerminationSaveCoordinator {
         leaf: String,
         fallback: URL
     ) -> URL {
-        currentArtifactURL(
+        resolvedArtifactURL(
             parentDescriptor: parentDescriptor,
-            leaf: leaf
+            leaf: leaf,
+            fallback: fallback
         ) ?? fallback
     }
 
+    private static func resolvedArtifactURL(
+        parentDescriptor: Int32,
+        leaf: String,
+        fallback: URL?
+    ) -> URL? {
+        if let fallback,
+           fallback.lastPathComponent == leaf,
+           directoryPathMatches(
+               parentDescriptor: parentDescriptor,
+               url: fallback.deletingLastPathComponent()
+           ) {
+            return fallback
+        }
+        return currentArtifactURL(
+            parentDescriptor: parentDescriptor,
+            leaf: leaf
+        )
+    }
+
     private static func currentArtifactURL(
         parentDescriptor: Int32,
         leaf: String
     ) -> URL? {
-        var path = [CChar](
-            repeating: 0,
-            count: 4 * Int(MAXPATHLEN)
-        )
-        guard let handle = Darwin.dlopen(nil, RTLD_LAZY | RTLD_LOCAL) else {
-            return nil
+        var descriptorInfo = vnode_fdinfowithpath()
+        let expectedSize = MemoryLayout<vnode_fdinfowithpath>.size
+        guard expectedSize <= Int(Int32.max) else { return nil }
+        let result = withUnsafeMutablePointer(to: &descriptorInfo) { pointer in
+            Darwin.proc_pidfdinfo(
+                Darwin.getpid(),
+                parentDescriptor,
+                PROC_PIDFDVNODEPATHINFO,
+                pointer,
+                Int32(expectedSize)
+            )
         }
-        defer { Darwin.dlclose(handle) }
-        guard let symbol = Darwin.dlsym(handle, "fcntl") else {
+        guard result == Int32(expectedSize) else { return nil }
+        let currentPath = withUnsafeBytes(
+            of: &descriptorInfo.pvip.vip_path
+        ) { buffer -> String? in
+            let path = buffer.bindMemory(to: CChar.self)
+            let terminator = path.firstIndex(of: 0) ?? path.endIndex
+            guard terminator != path.startIndex else { return nil }
+            let pathBytes = path[..<terminator].map(UInt8.init(bitPattern:))
+            return String(bytes: pathBytes, encoding: .utf8)
+        }
+        guard let currentPath else {
             return nil
         }
-        typealias FcntlPath = @convention(c) (
-            Int32,
-            Int32,
-            UnsafeMutableRawPointer?
-        ) -> Int32
-        let fcntlPath = unsafeBitCast(symbol, to: FcntlPath.self)
-        let result = path.withUnsafeMutableBytes { buffer in
-            fcntlPath(parentDescriptor, F_GETPATH, buffer.baseAddress)
-        }
-        guard result == 0 else { return nil }
-        let terminator = path.firstIndex(of: 0) ?? path.endIndex
-        let pathBytes = path[..<terminator].map(UInt8.init(bitPattern:))
-        guard let currentPath = String(bytes: pathBytes, encoding: .utf8) else {
+        let currentParentURL = URL(
+            fileURLWithPath: currentPath,
+            isDirectory: true
+        )
+        var descriptorStatus = stat()
+        var pathStatus = stat()
+        guard Darwin.fstat(parentDescriptor, &descriptorStatus) == 0,
+              (descriptorStatus.st_mode & S_IFMT) == S_IFDIR,
+              Darwin.lstat(currentParentURL.path, &pathStatus) == 0,
+              (pathStatus.st_mode & S_IFMT) == S_IFDIR,
+              sameObject(descriptorStatus, pathStatus) else {
             return nil
         }
-        return URL(fileURLWithPath: currentPath)
+        return currentParentURL
             .appendingPathComponent(leaf)
     }
 
@@ -2034,21 +2066,23 @@ nonisolated enum TerminationSaveCoordinator {
             if errno == ENOENT { return .removed }
             return .failed(
                 message: "Could not quarantine an artifact before cleanup",
-                retainedURL: currentArtifactURL(
+                retainedURL: resolvedArtifactURL(
                     parentDescriptor: parentDescriptor,
-                    leaf: leaf
-                ) ?? fallbackOriginalURL
+                    leaf: leaf,
+                    fallback: fallbackOriginalURL
+                )
             )
         }
         do {
             try synchronizeDirectory(parentDescriptor)
         } catch {
             return .failed(
                 message: "Could not make artifact quarantine durable",
-                retainedURL: currentArtifactURL(
+                retainedURL: resolvedArtifactURL(
                     parentDescriptor: parentDescriptor,
-                    leaf: cleanupLeaf
-                ) ?? fallbackCleanupURL
+                    leaf: cleanupLeaf,
+                    fallback: fallbackCleanupURL
+                )
             )
         }
         guard regularFileMatches(
@@ -2067,19 +2101,23 @@ nonisolated enum TerminationSaveCoordinator {
             }
             return .failed(
                 message: "An artifact changed identity during cleanup",
-                retainedURL: currentArtifactURL(
+                retainedURL: resolvedArtifactURL(
                     parentDescriptor: parentDescriptor,
-                    leaf: restored ? leaf : cleanupLeaf
-                ) ?? (restored ? fallbackOriginalURL : fallbackCleanupURL)
+                    leaf: restored ? leaf : cleanupLeaf,
+                    fallback: restored
+                        ? fallbackOriginalURL
+                        : fallbackCleanupURL
+                )
             )
         }
         guard Darwin.unlinkat(parentDescriptor, cleanupLeaf, 0) == 0 else {
             return .failed(
                 message: "Could not unlink a quarantined artifact",
-                retainedURL: currentArtifactURL(
+                retainedURL: resolvedArtifactURL(
                     parentDescriptor: parentDescriptor,
-                    leaf: cleanupLeaf
-                ) ?? fallbackCleanupURL
+                    leaf: cleanupLeaf,
+                    fallback: fallbackCleanupURL
+                )
             )
         }
         do {

PineTests/TerminationSaveCoordinatorSecurityTests.swift

@@ -268,6 +268,43 @@ struct TerminationSaveCoordinatorSecurityTests {
         )
     }
 
+    @Test func failedCleanupReportsArtifactInRenamedDirectory() async throws {
+        let container = try makeDirectory()
+        defer { remove(container) }
+        let directory = container.appendingPathComponent("project")
+        let movedDirectory = container.appendingPathComponent("project-moved")
+        try FileManager.default.createDirectory(
+            at: directory,
+            withIntermediateDirectories: false
+        )
+        let staged = try await stage(
+            content: "private bytes retained after rename",
+            destination: directory.appendingPathComponent("cleanup-moved.txt")
+        )
+        try FileManager.default.moveItem(at: directory, to: movedDirectory)
+        try FileManager.default.createDirectory(
+            at: directory,
+            withIntermediateDirectories: false
+        )
+        #expect(Darwin.chmod(movedDirectory.path, mode_t(0o500)) == 0)
+        let result = TerminationSaveCoordinator.cleanup([staged])
+        #expect(Darwin.chmod(movedDirectory.path, mode_t(0o700)) == 0)
+
+        guard case .failed(_, let retainedArtifacts) = result else {
+            Issue.record("Expected cleanup failure in the renamed directory")
+            return
+        }
+        let expectedArtifact = movedDirectory.appendingPathComponent(
+            staged.stagingURL.lastPathComponent
+        )
+        #expect(
+            retainedArtifacts.map { $0.resolvingSymlinksInPath() }
+                == [expectedArtifact.resolvingSymlinksInPath()]
+        )
+        #expect(FileManager.default.fileExists(atPath: expectedArtifact.path))
+        #expect(TerminationSaveCoordinator.cleanup([staged]) == .cleaned)
+    }
+
     @Test func installedDestinationReportsRetainedRecoveryFailure() async throws {
         let directory = try makeDirectory()
         defer { remove(directory) }
@@ -658,6 +695,54 @@ struct TerminationSaveCoordinatorSecurityTests {
         )
     }
 
+    @Test func finalVerificationReportsDestinationAfterParentRename() async throws {
+        let container = try makeDirectory()
+        defer { remove(container) }
+        let directory = container.appendingPathComponent("project")
+        let movedDirectory = container.appendingPathComponent("project-moved")
+        try FileManager.default.createDirectory(
+            at: directory,
+            withIntermediateDirectories: false
+        )
+        let destination = directory.appendingPathComponent("new-moved.txt")
+        let staged = try await stage(
+            content: "installed before parent rename",
+            destination: destination
+        )
+
+        let result = await TerminationSaveCoordinator.install(
+            staged,
+            until: .now() + 5,
+            beforeFinalInstalledFence: {
+                try? FileManager.default.moveItem(
+                    at: directory,
+                    to: movedDirectory
+                )
+                try? FileManager.default.createDirectory(
+                    at: directory,
+                    withIntermediateDirectories: false
+                )
+            }
+        )
+
+        guard case .failed(_, let retainedArtifacts) = result else {
+            Issue.record("A renamed parent must fail final verification")
+            return
+        }
+        let movedDestination = movedDirectory.appendingPathComponent(
+            destination.lastPathComponent
+        )
+        #expect(
+            retainedArtifacts.map { $0.resolvingSymlinksInPath() }
+                == [movedDestination.resolvingSymlinksInPath()]
+        )
+        #expect(
+            try String(contentsOf: movedDestination, encoding: .utf8)
+                == "installed before parent rename"
+        )
+        #expect(!FileManager.default.fileExists(atPath: destination.path))
+    }
+
     private func stage(
         content: String,
         destination: URL