From ce34aa16f9b9fcbd903a6f0ea70434be2b68458f Mon Sep 17 00:00:00 2001 From: Michael Cerisano Date: Sat, 17 Oct 2026 13:21:00 -0400 Subject: [PATCH] Reject destination name changes after verification --- .../File/DestinationWriter.swift | 60 ++++- .../BitMatchEngine/TransferPipeline.swift | 33 +++ .../DestinationReadbackNameRaceTests.swift | 206 ++++++++++++++++++ 3 files changed, 298 insertions(+), 1 deletion(-) create mode 100644 Packages/BitMatchEngine/Tests/BitMatchEngineTests/DestinationReadbackNameRaceTests.swift diff --git a/Packages/BitMatchEngine/Sources/BitMatchEngine/File/DestinationWriter.swift b/Packages/BitMatchEngine/Sources/BitMatchEngine/File/DestinationWriter.swift index add9f353..25835d05 100644 --- a/Packages/BitMatchEngine/Sources/BitMatchEngine/File/DestinationWriter.swift +++ b/Packages/BitMatchEngine/Sources/BitMatchEngine/File/DestinationWriter.swift @@ -87,6 +87,47 @@ public final class PinnedDestinationDirectory: @unchecked Sendable { } } + /// Inspect the final public name without creating any missing parent + /// directory or following a substituted symlink. Coverage uses this to + /// compare the current file with the inode that supplied readback bytes. + func currentRegularFileIdentity(at relativePath: String) throws -> VerifiedFileIdentity? { + let components = relativePath.split(separator: "/", omittingEmptySubsequences: false).map(String.init) + guard !components.isEmpty else { return nil } + for component in components { try Self.validate(component: component) } + + var currentFD = Darwin.dup(directoryFD) + guard currentFD >= 0 else { throw Self.posixError("Unable to inspect pinned destination directory") } + defer { _ = Darwin.close(currentFD) } + for component in components.dropLast() { + let childFD = component.withCString { + openat(currentFD, $0, O_SEARCH | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC) + } + if childFD < 0 { + if errno == ENOENT || errno == ENOTDIR || errno == ELOOP { return nil } + throw Self.posixError("Unable to inspect destination folder") + } + _ = Darwin.close(currentFD) + currentFD = childFD + } + + var info = stat() + let status = components[components.count - 1].withCString { + fstatat(currentFD, $0, &info, AT_SYMLINK_NOFOLLOW) + } + if status < 0 { + if errno == ENOENT || errno == ENOTDIR || errno == ELOOP { return nil } + throw Self.posixError("Unable to inspect destination file") + } + guard (info.st_mode & S_IFMT) == S_IFREG else { return nil } + return VerifiedFileIdentity( + device: UInt64(info.st_dev), inode: UInt64(info.st_ino), size: Int64(info.st_size), + modificationSeconds: Int64(info.st_mtimespec.tv_sec), + modificationNanoseconds: Int64(info.st_mtimespec.tv_nsec), + changeSeconds: Int64(info.st_ctimespec.tv_sec), + changeNanoseconds: Int64(info.st_ctimespec.tv_nsec) + ) + } + /// Opens a destination file below the pinned directory. The returned /// descriptor, not `logicalRootURL`, is the authority for subsequent /// reads. This is deliberately separate from the display URL above. @@ -352,6 +393,22 @@ public final class PinnedDestinationFile: @unchecked Sendable { return info } + /// A pinned descriptor may remain readable after its final name has been + /// replaced. Verify that the name still identifies this exact file. + func nameStillMatchesPinnedFile(_ pinned: stat) -> Bool { + var current = stat() + let status = name.withCString { fstatat(parentFD, $0, ¤t, AT_SYMLINK_NOFOLLOW) } + return status == 0 + && (current.st_mode & S_IFMT) == S_IFREG + && current.st_dev == pinned.st_dev + && current.st_ino == pinned.st_ino + && current.st_size == pinned.st_size + && current.st_mtimespec.tv_sec == pinned.st_mtimespec.tv_sec + && current.st_mtimespec.tv_nsec == pinned.st_mtimespec.tv_nsec + && current.st_ctimespec.tv_sec == pinned.st_ctimespec.tv_sec + && current.st_ctimespec.tv_nsec == pinned.st_ctimespec.tv_nsec + } + public func inspectClipIntegrity() throws -> ClipIntegrityFinding { let info = try snapshot() return try ClipIntegrityCheck.inspect(fileDescriptor: fileFD, fileSize: info.st_size) @@ -1563,7 +1620,8 @@ public final class DestinationWriter { } } let final = try destination.snapshot() - guard bytesRead == Int64(initial.st_size), pinnedFileRemainedStable(initial, final) else { + guard bytesRead == Int64(initial.st_size), pinnedFileRemainedStable(initial, final), + destination.nameStillMatchesPinnedFile(final) else { throw NSError( domain: "DestinationWriter", code: -11, diff --git a/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferPipeline.swift b/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferPipeline.swift index 14172392..eecc82fc 100644 --- a/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferPipeline.swift +++ b/Packages/BitMatchEngine/Sources/BitMatchEngine/TransferPipeline.swift @@ -117,6 +117,14 @@ public actor RunLedger { public func results() -> [FileOperationResult] { rows } + func result(relativePath: String, destination: Int) -> FileOperationResult? { + guard let index = rowIndex[FileResultKey( + sourceRelativePath: relativePath, + destinationIndex: destination + )] else { return nil } + return rows[index] + } + /// Completion coverage gates add failures for rows that were successful. /// They must not replace a more specific copy or verification failure. func alreadyFailed(relativePath: String, destination: Int) -> Bool { @@ -1069,6 +1077,31 @@ public final class TransferPipeline: FileOperationsService, Sendable { await onFileResult?(failure) continue } + if operation.verificationMode != .quick { + let verifiedIdentity = await ledger.result( + relativePath: entry.relativePath, + destination: destIndex + )?.verificationResult?.destinationReadIdentity + let currentIdentity = try? pinned.currentRegularFileIdentity(at: entry.relativePath) + if verifiedIdentity == nil || currentIdentity != verifiedIdentity { + let reason = "\(destinationURL.lastPathComponent)'s \(entry.relativePath) changed or could not be rechecked after verification. Keep the source media." + let failure = FileOperationResult( + sourceURL: entry.url, + destinationURL: pinned.logicalRootURL.appendingPathComponent(entry.relativePath), + success: false, + error: completionGateError(reason), + fileSize: entry.size, + verificationResult: nil, + processingTime: 0 + ) + await ledger.record( + failure, + relativePath: entry.relativePath, + destination: destIndex + ) + await onFileResult?(failure) + } + } } } diff --git a/Packages/BitMatchEngine/Tests/BitMatchEngineTests/DestinationReadbackNameRaceTests.swift b/Packages/BitMatchEngine/Tests/BitMatchEngineTests/DestinationReadbackNameRaceTests.swift new file mode 100644 index 00000000..c025c4ef --- /dev/null +++ b/Packages/BitMatchEngine/Tests/BitMatchEngineTests/DestinationReadbackNameRaceTests.swift @@ -0,0 +1,206 @@ +#if os(macOS) +import Darwin +import Foundation +import Synchronization +import XCTest +@testable import BitMatchEngine + +/// The readback descriptor can stay valid after its public name is replaced. +/// This test changes only disposable test files and keeps the original inode +/// under another name, so a failure cannot be mistaken for deleted source data. +final class DestinationReadbackNameRaceTests: XCTestCase { + private var fixtureParent: URL { + ProcessInfo.processInfo.environment["BITMATCH_RACE_TEST_ROOT"].map { + URL(fileURLWithPath: $0, isDirectory: true) + } ?? FileManager.default.temporaryDirectory + } + + func testSameSizeNameReplacementDuringReadbackIsRejected() async throws { + let scratch = fixtureParent + .appendingPathComponent("bitmatch-readback-name-race-\(UUID().uuidString)", isDirectory: true) + let sourceDirectory = scratch.appendingPathComponent("source", isDirectory: true) + let destinationDirectory = scratch.appendingPathComponent("destination", isDirectory: true) + try FileManager.default.createDirectory(at: sourceDirectory, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: destinationDirectory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: scratch) } + + let originalBytes = Data(repeating: 0x31, count: 8_193) + let replacementBytes = Data(repeating: 0x72, count: originalBytes.count) + let source = sourceDirectory.appendingPathComponent("clip.bin") + let destination = destinationDirectory.appendingPathComponent("clip.bin") + let waitingReplacement = destinationDirectory.appendingPathComponent("waiting-replacement.bin") + let savedOriginal = destinationDirectory.appendingPathComponent("saved-original.bin") + try originalBytes.write(to: source) + try originalBytes.write(to: destination) + try replacementBytes.write(to: waitingReplacement) + + let parentFD = Darwin.open(destinationDirectory.path, O_RDONLY | O_DIRECTORY | O_CLOEXEC) + guard parentFD >= 0 else { throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) } + defer { _ = Darwin.close(parentFD) } + let swap = Mutex((attempted: false, firstStatus: Int32.min, firstError: Int32(0), + secondStatus: Int32.min, secondError: Int32(0))) + let hooks = DestinationWriter.FanOutHooks(destinationDidRead: { _, relativePath, _ in + guard relativePath == "clip.bin" else { return } + swap.withLock { state in + guard !state.attempted else { return } + state.attempted = true + state.firstStatus = "clip.bin".withCString { oldName in + "saved-original.bin".withCString { savedName in + renameat(parentFD, oldName, parentFD, savedName) + } + } + if state.firstStatus != 0 { + state.firstError = errno + return + } + state.secondStatus = "waiting-replacement.bin".withCString { waitingName in + "clip.bin".withCString { finalName in + renameat(parentFD, waitingName, parentFD, finalName) + } + } + if state.secondStatus != 0 { state.secondError = errno } + } + }) + + let pinned = try PinnedDestinationDirectory.open(destination: destinationDirectory, rootComponents: []) + let outcome: Result + do { + let checked = try await DestinationWriter.verifyPinnedDestinationFileAndInspectClip( + source: source, + pinnedRoot: pinned, + relativePath: "clip.bin", + verificationMode: .standard, + checksumService: ChecksumEngine.shared, + clipURL: destination, + hooks: hooks + ) + outcome = .success(checked.verification.matches) + } catch { + outcome = .failure(error) + } + + let swapped = swap.withLock { $0 } + XCTAssertTrue(swapped.attempted, "The name change must happen after the reader opened the old inode") + XCTAssertEqual(swapped.firstStatus, 0, "Unable to retain original: errno \(swapped.firstError)") + XCTAssertEqual(swapped.secondStatus, 0, "Unable to install replacement: errno \(swapped.secondError)") + XCTAssertEqual(try Data(contentsOf: source), originalBytes) + XCTAssertEqual(try Data(contentsOf: savedOriginal), originalBytes) + XCTAssertEqual(try Data(contentsOf: destination), replacementBytes) + + switch outcome { + case .success(let matches): + XCTAssertFalse(matches, "Readback of an orphaned inode must not verify the current destination name") + case .failure: + // APFS changes the moved inode's ctime, which the existing + // pinned-descriptor stability check already rejects. + break + } + } + + /// A successful readback row is not enough if its public pathname changes + /// before the final destination-coverage pass. The callback gives this + /// test a deterministic point between those two phases. + func testSameSizeReplacementAfterVerificationCannotPassFinalCoverage() async throws { + let parent = fixtureParent + try await FileOperationsTestLock.shared.run { + try await Self.assertReplacementAfterVerificationIsRejected(parent: parent, relativePath: "clip.bin") + } + } + + func testNestedSameSizeReplacementAfterVerificationCannotPassFinalCoverage() async throws { + let parent = fixtureParent + try await FileOperationsTestLock.shared.run { + try await Self.assertReplacementAfterVerificationIsRejected( + parent: parent, relativePath: "DCIM/100MEDIA/clip.bin" + ) + } + } + + private static func assertReplacementAfterVerificationIsRejected(parent: URL, relativePath: String) async throws { + let scratch = parent + .appendingPathComponent("bitmatch-post-verify-name-race-\(UUID().uuidString)", isDirectory: true) + let sourceScratch = FileManager.default.temporaryDirectory + .appendingPathComponent("bitmatch-post-verify-source-\(UUID().uuidString)", isDirectory: true) + let sourceDirectory = sourceScratch.appendingPathComponent("Card", isDirectory: true) + let destinationDirectory = scratch.appendingPathComponent("backup", isDirectory: true) + try FileManager.default.createDirectory(at: sourceDirectory, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: destinationDirectory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: scratch) } + defer { try? FileManager.default.removeItem(at: sourceScratch) } + + let originalBytes = Data(repeating: 0x31, count: 8_193) + let replacementBytes = Data(repeating: 0x72, count: originalBytes.count) + let source = sourceDirectory.appendingPathComponent(relativePath) + let waitingReplacement = scratch.appendingPathComponent("waiting-replacement.bin") + let savedOriginal = scratch.appendingPathComponent("saved-original.bin") + try FileManager.default.createDirectory(at: source.deletingLastPathComponent(), withIntermediateDirectories: true) + try originalBytes.write(to: source) + try replacementBytes.write(to: waitingReplacement) + + let scratchFD = Darwin.open(scratch.path, O_RDONLY | O_DIRECTORY | O_CLOEXEC) + guard scratchFD >= 0 else { throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) } + defer { _ = Darwin.close(scratchFD) } + let swap = Mutex((attempted: false, firstStatus: Int32.min, firstError: Int32(0), + secondStatus: Int32.min, secondError: Int32(0))) + let reportedComplete = Mutex(false) + let pipeline = TransferPipeline(fileSystem: LocalFileAccess(), checksum: ChecksumEngine.shared) + let operation = try await pipeline.performFileOperation( + sourceURL: sourceDirectory, + destinationURLs: [destinationDirectory], + verificationMode: .standard, + settings: CameraLabelSettings(), + progressCallback: { progress in + if progress.currentStage == .completed { + reportedComplete.withLock { $0 = true } + } + }, + onFileResult: { row in + guard row.sourceURL.lastPathComponent == "clip.bin", + row.verificationResult?.matches == true else { return } + swap.withLock { state in + guard !state.attempted else { return } + state.attempted = true + let parentFD = Darwin.open(row.destinationURL.deletingLastPathComponent().path, + O_RDONLY | O_DIRECTORY | O_CLOEXEC) + guard parentFD >= 0 else { + state.firstStatus = -1 + state.firstError = errno + return + } + defer { _ = Darwin.close(parentFD) } + state.firstStatus = "clip.bin".withCString { finalName in + "saved-original.bin".withCString { savedName in + renameat(parentFD, finalName, scratchFD, savedName) + } + } + if state.firstStatus != 0 { + state.firstError = errno + return + } + state.secondStatus = "waiting-replacement.bin".withCString { waitingName in + "clip.bin".withCString { finalName in + renameat(scratchFD, waitingName, parentFD, finalName) + } + } + if state.secondStatus != 0 { state.secondError = errno } + } + } + ) + + let swapped = swap.withLock { $0 } + XCTAssertTrue(swapped.attempted, "A verified row must reach the interphase callback") + XCTAssertEqual(swapped.firstStatus, 0, "Unable to retain original: errno \(swapped.firstError)") + XCTAssertEqual(swapped.secondStatus, 0, "Unable to install replacement: errno \(swapped.secondError)") + XCTAssertEqual(try Data(contentsOf: source), originalBytes) + XCTAssertEqual(try Data(contentsOf: savedOriginal), originalBytes) + let final = destinationDirectory.appendingPathComponent("Card").appendingPathComponent(relativePath) + XCTAssertEqual(try Data(contentsOf: final), replacementBytes) + + let row = try XCTUnwrap(operation.results.first { $0.sourceURL.lastPathComponent == "clip.bin" }) + XCTAssertFalse(row.success, "A same-size replacement must fail the final coverage gate") + XCTAssertNil(row.verificationResult, "The failure must not retain stale verification evidence") + XCTAssertEqual(row.outcome, .failed) + XCTAssertFalse(reportedComplete.withLock { $0 }, "The pipeline must not report completion") + } +} +#endif