From d8ece04faeb4256c2a2f6c224fca24afa9df28a2 Mon Sep 17 00:00:00 2001 From: Michael Cerisano Date: Sat, 17 Oct 2026 11:08:04 -0400 Subject: [PATCH] Make first remote upload exclusive and recover safely --- .../Services/MacRemoteBackupController.swift | 41 +- .../Services/SFTPRemoteBackupProvider.swift | 361 +++++++++++++++--- BitMatch/Resources/SFTPRemoteHelper.py | 237 ++++++++++++ .../PhotographerJobViewModelTests.swift | 34 ++ BitMatchTests/RemoteBackupQueueTests.swift | 306 ++++++++++++++- BitMatchTests/SFTPRemoteBackupLiveTests.swift | 131 +++++++ .../SFTPRemoteBackupProcessTests.swift | 190 +++++++++ ...FTPRemoteBackupProviderContractTests.swift | 198 ++++++++-- Scripts/test-local-sftp.sh | 212 ++++++++++ Shared/Core/Models/RemoteBackupModels.swift | 18 +- .../Core/Services/RemoteBackupProvider.swift | 21 +- Shared/Core/Services/RemoteBackupQueue.swift | 175 +++++---- .../ViewModels/PhotographerJobViewModel.swift | 11 +- docs/GUIDE.md | 4 +- 14 files changed, 1741 insertions(+), 198 deletions(-) create mode 100644 BitMatch/Resources/SFTPRemoteHelper.py create mode 100644 BitMatchTests/SFTPRemoteBackupLiveTests.swift create mode 100755 Scripts/test-local-sftp.sh diff --git a/BitMatch/Core/Services/MacRemoteBackupController.swift b/BitMatch/Core/Services/MacRemoteBackupController.swift index 7155076c..2596986c 100644 --- a/BitMatch/Core/Services/MacRemoteBackupController.swift +++ b/BitMatch/Core/Services/MacRemoteBackupController.swift @@ -25,6 +25,9 @@ final class MacRemoteBackupController: ObservableObject { private var remoteBackupTimer: Timer? private var remoteSchedulerIsRunning = false private var remoteSchedulerNeedsRun = false + /// Only controller-owned workers have a task handle that can stop their + /// SSH subprocess after a durable user cancellation. + private var remoteWorkerTasks: [UUID: Task] = [:] private var remoteTimerGeneration: UInt64 = 0 @Published private(set) var hostTrustPrompt: HostTrustPrompt? private var hostTrustContinuation: CheckedContinuation? @@ -145,7 +148,10 @@ final class MacRemoteBackupController: ObservableObject { try await queue.restore() await refreshAllRemoteBackupSummaries() for id in await queue.runnableIDs() { - await queue.run(id) + let worker = Task { await queue.run(id) } + remoteWorkerTasks[id] = worker + await worker.value + remoteWorkerTasks[id] = nil if let item = await queue.item(id: id) { await refreshRemoteBackupSummary(for: item.cardIngestID) } @@ -237,19 +243,28 @@ final class MacRemoteBackupController: ObservableObject { /// cancellation is persisted before returning. func cancelRemoteBackup(for cardIngestID: UUID) { Task { [weak self] in - guard let self, let queue = self.remoteBackupQueue else { return } - do { - try await queue.restore() - for item in await queue.itemsForCardIngest(cardIngestID) { - try await queue.cancel(item.id) - } - } catch { - self.photographerJobViewModel.setRemoteFeedback("Could not cancel off-site backup: \(error.localizedDescription)") - await self.armRemoteBackupTimer() - return + await self?.cancelRemoteBackupNow(for: cardIngestID) + } + } + + /// Exposed internally so the durable cancellation and worker signal can + /// be tested as one ordered operation. + func cancelRemoteBackupNow(for cardIngestID: UUID) async { + guard let queue = remoteBackupQueue else { return } + do { + try await queue.restore() + for item in await queue.itemsForCardIngest(cardIngestID) { + try await queue.cancel(item.id) + // Persist the cancellation first. Only then terminate the + // owned worker and its SSH process group. + remoteWorkerTasks[item.id]?.cancel() } - await self.armRemoteBackupTimer() - await self.refreshRemoteBackupSummary(for: cardIngestID) + } catch { + photographerJobViewModel.setRemoteFeedback("Could not cancel off-site backup: \(error.localizedDescription)") + await armRemoteBackupTimer() + return } + await armRemoteBackupTimer() + await refreshRemoteBackupSummary(for: cardIngestID) } } diff --git a/BitMatch/Core/Services/SFTPRemoteBackupProvider.swift b/BitMatch/Core/Services/SFTPRemoteBackupProvider.swift index 32cb033f..fe96b503 100644 --- a/BitMatch/Core/Services/SFTPRemoteBackupProvider.swift +++ b/BitMatch/Core/Services/SFTPRemoteBackupProvider.swift @@ -1,5 +1,6 @@ import Foundation import BitMatchEngine +import CryptoKit struct OpenSSHHostTrustRequest: Equatable, Sendable { let host: String @@ -26,6 +27,24 @@ struct OpenSSHCommand: Equatable, Sendable { let executable: String let arguments: [String] let standardInput: Data? + /// Stream a verified local backup directly into SSH without loading it + /// into memory or reopening a remote pathname after the exclusive claim. + let standardInputFile: URL? + /// Immutable manifest digest. The exact descriptor sent to SSH is hashed + /// before launch, closing the validation-then-reopen pathname gap. + let expectedInputSHA256: String? + /// Stream remote readback into an exclusive local file. Capturing the + /// command's stdout in Data would scale memory with footage size. + let standardOutputFile: URL? + + init(executable: String, arguments: [String], standardInput: Data?, standardInputFile: URL? = nil, expectedInputSHA256: String? = nil, standardOutputFile: URL? = nil) { + self.executable = executable + self.arguments = arguments + self.standardInput = standardInput + self.standardInputFile = standardInputFile + self.expectedInputSHA256 = expectedInputSHA256 + self.standardOutputFile = standardOutputFile + } } struct OpenSSHCommandResult: Sendable { @@ -43,14 +62,46 @@ private final class OpenSSHProcessOutput: @unchecked Sendable { private let lock = NSLock() private var stdout = Data() private var stderr = Data() + private var drainError: Error? private var inputWriteFailed = false func setStdout(_ data: Data) { lock.withLock { stdout = data } } func setStderr(_ data: Data) { lock.withLock { stderr = data } } + func setDrainError(_ error: Error) { lock.withLock { drainError = error } } func markInputWriteFailure() { lock.withLock { inputWriteFailed = true } } - func didInputWriteFail() -> Bool { lock.withLock { inputWriteFailed } } - func result(status: Int32) -> OpenSSHCommandResult { - lock.withLock { OpenSSHCommandResult(status: status, stdout: stdout, stderr: stderr) } + func result(status: Int32) throws -> OpenSSHCommandResult { + try lock.withLock { + if let drainError { throw drainError } + if status == 0 && inputWriteFailed { throw RemoteBackupError.providerUnavailable } + return OpenSSHCommandResult(status: status, stdout: stdout, stderr: stderr) + } + } +} + +private struct OpenSSHInputIdentity: Equatable, Sendable { + let device: Int64 + let inode: UInt64 + let byteCount: Int64 + let modifiedSeconds: Int64 + let modifiedNanoseconds: Int64 + let changedSeconds: Int64 + let changedNanoseconds: Int64 + + init(fileDescriptor: Int32) throws { + var info = stat() + guard Darwin.fstat(fileDescriptor, &info) == 0 else { + throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) + } + guard (info.st_mode & S_IFMT) == S_IFREG, info.st_size >= 0 else { + throw RemoteBackupError.localArtifactNotVerified + } + device = Int64(info.st_dev) + inode = UInt64(info.st_ino) + byteCount = Int64(info.st_size) + modifiedSeconds = Int64(info.st_mtimespec.tv_sec) + modifiedNanoseconds = Int64(info.st_mtimespec.tv_nsec) + changedSeconds = Int64(info.st_ctimespec.tv_sec) + changedNanoseconds = Int64(info.st_ctimespec.tv_nsec) } } @@ -64,7 +115,7 @@ private final class OpenSSHProcessLifetime: @unchecked Sendable { private var finished = false private var forceKill: DispatchWorkItem? - func launch(_ command: OpenSSHCommand, input: Pipe, output: Pipe, error: Pipe) throws -> pid_t { + func launch(_ command: OpenSSHCommand, input: Pipe, inputFileFD: Int32?, output: Pipe, error: Pipe) throws -> pid_t { try lock.withLock { guard !cancelled else { throw CancellationError() } var actions: posix_spawn_file_actions_t? @@ -74,13 +125,15 @@ private final class OpenSSHProcessLifetime: @unchecked Sendable { let inputRead = input.fileHandleForReading.fileDescriptor let outputWrite = output.fileHandleForWriting.fileDescriptor let errorWrite = error.fileHandleForWriting.fileDescriptor - try Self.check(posix_spawn_file_actions_adddup2(&actions, inputRead, STDIN_FILENO)) + try Self.check(posix_spawn_file_actions_adddup2(&actions, inputFileFD ?? inputRead, STDIN_FILENO)) try Self.check(posix_spawn_file_actions_adddup2(&actions, outputWrite, STDOUT_FILENO)) try Self.check(posix_spawn_file_actions_adddup2(&actions, errorWrite, STDERR_FILENO)) - for descriptor in [inputRead, outputWrite, errorWrite, + let descriptors = [inputRead, outputWrite, errorWrite, input.fileHandleForWriting.fileDescriptor, output.fileHandleForReading.fileDescriptor, - error.fileHandleForReading.fileDescriptor] where descriptor > STDERR_FILENO { + error.fileHandleForReading.fileDescriptor, + inputFileFD].compactMap { $0 } + for descriptor in Set(descriptors) where descriptor > STDERR_FILENO { try Self.check(posix_spawn_file_actions_addclose(&actions, descriptor)) } @@ -174,21 +227,17 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { throw RemoteBackupError.authenticationFailed } try await establishExplicitTrustIfNeeded() - let result = try await run(try ssh(command: "if command -v ln >/dev/null 2>&1 && command -v rm >/dev/null 2>&1; then exit 0; else printf __BITMATCH_NO_LN_RM__ >&2; exit 42; fi")) - guard result.status == 0 else { - let stderr = String(decoding: result.stderr, as: UTF8.self).lowercased() - if result.status == 42, stderr.contains("__bitmatch_no_ln_rm__") { - throw RemoteBackupError.capabilityUnavailable(.noReplacePromotion) - } - throw classify(result) + let result = try await run(try helperCommand(action: "check")) + guard result.status == 0, + String(decoding: result.stdout, as: UTF8.self).contains("__BITMATCH_HELPER_READY__") else { + throw result.status == 255 ? classify(result) : RemoteBackupError.streamUploadUnavailable } return RemoteProviderCapabilities(supportsNoReplacePromotion: true, supportsVerificationEvidence: true) } func inspect(path: RemoteRelativePath) async throws -> RemoteObject? { - let remotePath = try remote(path) - let result = try await run(try ssh(command: "if test -e \(shellQuote(remotePath)); then wc -c < \(shellQuote(remotePath)); fi")) - guard result.status == 0 else { throw classify(result) } + let result = try await run(try helperCommand(action: "inspect", paths: [path])) + guard result.status == 0 else { throw classifyHelper(result) } let output = String(decoding: result.stdout, as: UTF8.self).trimmingCharacters(in: .whitespacesAndNewlines) guard !output.isEmpty else { return nil } guard let count = Int64(output), count >= 0 else { throw RemoteBackupError.providerUnavailable } @@ -196,8 +245,8 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { } func ensureDirectory(_ path: RemoteRelativePath) async throws { - let result = try await run(try ssh(command: "mkdir -p \(shellQuote(try remote(path)))")) - guard result.status == 0 else { throw classify(result) } + let result = try await run(try helperCommand(action: "mkdir", paths: [path])) + guard result.status == 0 else { throw classifyHelper(result) } } func discardTemporary(_ path: RemoteRelativePath) async throws { @@ -207,36 +256,77 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { else { throw RemoteBackupError.unsafePath } - let result = try await run(try ssh(command: "rm -f \(shellQuote(try remote(path)))")) - guard result.status == 0 else { throw classify(result) } + // Even an item-owned name can be replaced between a check and `rm`. + // The queue rotates attempt names and leaves old partials for explicit + // operator cleanup instead of unlinking a possible replacement. + throw RemoteBackupError.temporaryCleanupFailed } - func upload(local: URL, toTemporary path: RemoteRelativePath, fromOffset: Int64, progress: @Sendable (Int64) async -> Void) async throws { - guard fromOffset >= 0 else { throw RemoteBackupError.resumeOffsetMismatch(local: fromOffset, remote: 0) } + func upload(local: URL, toTemporary path: RemoteRelativePath, fromOffset: Int64, expectedSHA256: String, progress: @Sendable (Int64) async -> Void) async throws { + try Task.checkCancellation() + guard expectedSHA256.count == 64, + expectedSHA256.utf8.allSatisfy({ (48...57).contains($0) || (65...70).contains($0) || (97...102).contains($0) }) else { + throw RemoteBackupError.localArtifactNotVerified + } let remoteObject = try await inspect(path: path) - guard (remoteObject?.byteCount ?? 0) == fromOffset else { + try Task.checkCancellation() + guard fromOffset == 0 else { throw RemoteBackupError.resumeOffsetMismatch(local: fromOffset, remote: remoteObject?.byteCount ?? 0) } - let batch = "put -a \(try sftpQuote(local.path)) \(try sftpQuote(try remote(path)))\n" - let result = try await run(try sftp(batch: batch)) - guard result.status == 0 else { throw classify(result) } + guard remoteObject == nil else { + throw RemoteBackupError.temporaryClaimFailed + } + // The helper pins each parent directory and opens the leaf with + // O_EXCL|O_NOFOLLOW. A pathname swap cannot redirect its stream. + let sshCommand = try helperCommand(action: "upload", paths: [path]) + let result = try await run(OpenSSHCommand( + executable: sshCommand.executable, + arguments: sshCommand.arguments, + standardInput: nil, + standardInputFile: local, + expectedInputSHA256: expectedSHA256 + )) + try Task.checkCancellation() + guard result.status == 0 else { throw classifyHelper(result, claim: true) } let complete = try await inspect(path: path)?.byteCount ?? 0 + try Task.checkCancellation() await progress(complete) } func promoteNoReplace(temporary: RemoteRelativePath, final: RemoteRelativePath) async throws { - let temporaryPath = try remote(temporary) - let finalPath = try remote(final) - let command = "ln \(shellQuote(temporaryPath)) \(shellQuote(finalPath)) && rm \(shellQuote(temporaryPath))" - let result = try await run(try ssh(command: command)) - guard result.status == 0 else { throw classifyPromotion(result) } + // linkat pins both parent directories, refuses an existing final name + // (including a directory symlink), and leaves the temp hard link for + // explicit operator cleanup. No pathname-only removal follows it. + let result = try await run(try helperCommand(action: "promote", paths: [temporary, final])) + guard result.status == 0 else { throw classifyHelper(result, promotion: true) } + } + + func confirmsOwnedFinal(temporary: RemoteRelativePath, final: RemoteRelativePath) async throws -> Bool { + let result = try await run(try helperCommand(action: "same", paths: [temporary, final])) + // A replaced symlink or non-file is never ownership evidence. + if result.status == 43 { return false } + guard result.status == 0 else { throw classifyHelper(result) } + let answer = String(decoding: result.stdout, as: UTF8.self) + .trimmingCharacters(in: .whitespacesAndNewlines) + switch answer { + case "__BITMATCH_SAME__": return true + case "__BITMATCH_DIFFERENT__": return false + default: throw RemoteBackupError.providerUnavailable + } } func verificationEvidence(for path: RemoteRelativePath, expectedSHA256: String) async throws -> RemoteVerificationEvidence { let temporary = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) defer { try? FileManager.default.removeItem(at: temporary) } - let result = try await run(try sftp(batch: "get \(try sftpQuote(try remote(path))) \(try sftpQuote(temporary.path))\n")) - guard result.status == 0 else { throw classify(result) } + let sshCommand = try helperCommand(action: "download", paths: [path]) + let result = try await run(OpenSSHCommand( + executable: sshCommand.executable, + arguments: sshCommand.arguments, + standardInput: nil, + standardOutputFile: temporary + )) + guard result.status == 0 else { throw classifyHelper(result) } + try Task.checkCancellation() let actual = try await ChecksumEngine.shared.generateChecksum(for: temporary, type: .sha256) guard actual.caseInsensitiveCompare(expectedSHA256) == .orderedSame else { throw RemoteBackupError.verificationFailed } return .readBackSHA256(actual) @@ -340,6 +430,55 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { private func shellQuote(_ value: String) -> String { Self.shellQuote(value) } private func sftpQuote(_ value: String) throws -> String { try Self.sftpQuote(value) } + private static let helperSource: String? = { + let url = Bundle.main.url(forResource: "SFTPRemoteHelper", withExtension: "py") + ?? Bundle.main.url(forResource: "SFTPRemoteHelper", withExtension: "py", subdirectory: "Resources") + guard let url else { return nil } + return try? String(contentsOf: url, encoding: .utf8) + }() + + private func helperCommand(action: String, paths: [RemoteRelativePath] = []) throws -> OpenSSHCommand { + guard let source = Self.helperSource else { throw RemoteBackupError.providerUnavailable } + let encodedPaths = try paths.map { path -> String in + let components = try profile.root.appending(path: path).components + let jsonData = try JSONSerialization.data(withJSONObject: components) + guard let json = String(data: jsonData, encoding: .utf8) else { + throw RemoteBackupError.unsafePath + } + return Self.shellQuote(json) + } + let command = (["python3", "-c", Self.shellQuote(source), Self.shellQuote(action)] + encodedPaths) + .joined(separator: " ") + return try ssh(command: command) + } + + private func classifyHelper( + _ result: OpenSSHCommandResult, + claim: Bool = false, + promotion: Bool = false + ) -> RemoteBackupError { + switch result.status { + case 42: + return promotion ? .conflict : claim ? .temporaryClaimFailed : .providerUnavailable + case 43: + return .unsafePath + case 44, 127: + return .streamUploadUnavailable + case 46: + return .remoteStorageFull + case 47: + return .permissionDenied + case 48: + return .capabilityUnavailable(.noReplacePromotion) + case 45: + return .providerUnavailable + case 255: + return classify(result) + default: + return .providerUnavailable + } + } + private func classify(_ result: OpenSSHCommandResult) -> RemoteBackupError { Self.classify(stderr: result.stderr) } @@ -347,15 +486,12 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { nonisolated static func classify(stderr: Data) -> RemoteBackupError { let text = String(decoding: stderr, as: UTF8.self).lowercased() if text.contains("host key verification") || text.contains("no matching host key") { return .hostKeyMismatch } + if text.contains("no space left on device") || text.contains("disk quota exceeded") { return .remoteStorageFull } if text.contains("permission denied") { return .permissionDenied } if text.contains("authentication") { return .authenticationFailed } return .networkUnavailable } - private func classifyPromotion(_ result: OpenSSHCommandResult) -> RemoteBackupError { - Self.promotionError(stderr: result.stderr) - } - static func fixedArguments(knownHostsURL: URL, port: Int, username: String?, portFlag: String) -> [String] { var arguments = ["-o", "BatchMode=yes", "-o", "StrictHostKeyChecking=yes", "-o", "UserKnownHostsFile=\(knownHostsURL.path)", "-o", "GlobalKnownHostsFile=/dev/null", "-o", "PasswordAuthentication=no", "-o", "KbdInteractiveAuthentication=no", "-o", "PreferredAuthentications=publickey", "-o", "PubkeyAuthentication=yes", portFlag, "\(port)"] if let username { arguments += ["-l", username] } @@ -363,7 +499,7 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { } nonisolated static func shellQuote(_ value: String) -> String { - "'\(value.replacingOccurrences(of: "'", with: "'\\\"'\\\"'"))'" + "'\(value.replacingOccurrences(of: "'", with: "'\"'\"'"))'" } nonisolated static func sftpQuote(_ value: String) throws -> String { @@ -377,14 +513,99 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { try root.appending(path: path).description } - nonisolated static func promotionError(stderr: Data) -> RemoteBackupError { - String(decoding: stderr, as: UTF8.self).lowercased().contains("file exists") - ? .conflict - : .capabilityUnavailable(.noReplacePromotion) + private nonisolated static func validatePinnedInput( + fileDescriptor: Int32, + identity: OpenSSHInputIdentity, + expectedSHA256: String + ) throws { + let bufferSize = 1024 * 1024 + let buffer = UnsafeMutableRawPointer.allocate(byteCount: bufferSize, alignment: 4096) + defer { buffer.deallocate() } + var hasher = SHA256() + var offset: Int64 = 0 + while offset < identity.byteCount { + try Task.checkCancellation() + let requested = Int(min(Int64(bufferSize), identity.byteCount - offset)) + let count = Darwin.pread(fileDescriptor, buffer, requested, off_t(offset)) + if count < 0 { + if errno == EINTR { continue } + throw RemoteBackupError.localArtifactNotVerified + } + guard count > 0 else { throw RemoteBackupError.localArtifactNotVerified } + autoreleasepool { + hasher.update(data: Data(bytes: buffer, count: count)) + } + offset += Int64(count) + } + let actual = hasher.finalize().map { String(format: "%02x", $0) }.joined() + guard actual.caseInsensitiveCompare(expectedSHA256) == .orderedSame, + try OpenSSHInputIdentity(fileDescriptor: fileDescriptor) == identity else { + throw RemoteBackupError.localArtifactNotVerified + } + } + + /// Keep command responses and error markers, but never let an unexpected + /// remote shell flood grow the app's memory. Continue draining to EOF so + /// the child can exit and be reaped even after the capture limit is hit. + private nonisolated static func captureBoundedOutput(_ handle: FileHandle) -> Data { + let limit = 1024 * 1024 + var captured = Data() + while true { + let reachedEnd = autoreleasepool { () -> Bool in + let chunk = handle.readData(ofLength: 64 * 1024) + if chunk.isEmpty { return true } + if captured.count < limit { + captured.append(chunk.prefix(limit - captured.count)) + } + return false + } + if reachedEnd { return captured } + } } // Kept internal so the real subprocess lifetime can be exercised by tests. static func runProcess(_ command: OpenSSHCommand) async throws -> OpenSSHCommandResult { + guard command.standardInput == nil || command.standardInputFile == nil else { + throw RemoteBackupError.providerUnavailable + } + guard command.expectedInputSHA256 == nil || command.standardInputFile != nil else { + throw RemoteBackupError.providerUnavailable + } + try Task.checkCancellation() + let openedInput: (handle: FileHandle, identity: OpenSSHInputIdentity)? = try command.standardInputFile.map { url in + // The queue already hashed this local artifact. Reject a replaced + // leaf rather than following it between that check and SSH launch. + let fd = Darwin.open(url.path, O_RDONLY | O_NONBLOCK | O_NOFOLLOW | O_CLOEXEC) + guard fd >= 0 else { + let code = errno + if [ELOOP, ENOENT, ENOTDIR, EACCES, EPERM].contains(code) { + throw RemoteBackupError.localArtifactNotVerified + } + throw NSError(domain: NSPOSIXErrorDomain, code: Int(code)) + } + do { + let identity = try OpenSSHInputIdentity(fileDescriptor: fd) + return (FileHandle(fileDescriptor: fd, closeOnDealloc: true), identity) + } catch { + _ = Darwin.close(fd) + throw error + } + } + defer { try? openedInput?.handle.close() } + if let openedInput, let expectedSHA256 = command.expectedInputSHA256 { + try Self.validatePinnedInput( + fileDescriptor: openedInput.handle.fileDescriptor, + identity: openedInput.identity, + expectedSHA256: expectedSHA256 + ) + } + let fileOutput: FileHandle? = try command.standardOutputFile.map { url in + let fd = Darwin.open(url.path, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, S_IRUSR | S_IWUSR) + guard fd >= 0 else { throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) } + return FileHandle(fileDescriptor: fd, closeOnDealloc: true) + } + defer { try? fileOutput?.close() } + let lifetime = OpenSSHProcessLifetime() let result: OpenSSHCommandResult = try await withTaskCancellationHandler(operation: { try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in @@ -395,29 +616,46 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { drains.enter() drains.enter() do { - // A fast-exiting ssh/sftp child can close stdin while a - // batch is being written. Return EPIPE, never SIGPIPE. + // A fast-exiting child can close stdin while a batch is + // being written. Return EPIPE instead of raising SIGPIPE. guard Darwin.fcntl(input.fileHandleForWriting.fileDescriptor, F_SETNOSIGPIPE, 1) != -1 else { throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) } - let pid = try lifetime.launch(command, input: input, output: output, error: stderrPipe) - // Only the child keeps the duplicated standard streams. + let pid = try lifetime.launch(command, input: input, inputFileFD: openedInput?.handle.fileDescriptor, output: output, error: stderrPipe) + // The child keeps only its duplicated standard streams. input.fileHandleForReading.closeFile() output.fileHandleForWriting.closeFile() stderrPipe.fileHandleForWriting.closeFile() + DispatchQueue.global().async { - collected.setStdout(output.fileHandleForReading.readDataToEndOfFile()) + if let fileOutput { + var writeFailure: Error? + while true { + let reachedEnd = autoreleasepool { () -> Bool in + let chunk = output.fileHandleForReading.readData(ofLength: 1024 * 1024) + if chunk.isEmpty { return true } + if writeFailure == nil { + do { try fileOutput.write(contentsOf: chunk) } + catch { writeFailure = error } + } + return false + } + if reachedEnd { break } + } + if let writeFailure { collected.setDrainError(writeFailure) } + } else { + collected.setStdout(Self.captureBoundedOutput(output.fileHandleForReading)) + } output.fileHandleForReading.closeFile() drains.leave() } DispatchQueue.global().async { - collected.setStderr(stderrPipe.fileHandleForReading.readDataToEndOfFile()) + collected.setStderr(Self.captureBoundedOutput(stderrPipe.fileHandleForReading)) stderrPipe.fileHandleForReading.closeFile() drains.leave() } - // Do not reap the group leader until its output closes. - // Observe its exit without reaping, then disarm the group - // timer while its PID is still reserved. Only then reap. + // Reap only after all streams close. While the leader is + // unreaped, its PID cannot be reused by an unrelated job. drains.notify(queue: .global()) { var exitInfo = siginfo_t() var observed: Int32 @@ -434,16 +672,13 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { if wasCancelled { continuation.resume(throwing: CancellationError()) } else if let status { - if status == 0 && collected.didInputWriteFail() { - continuation.resume(throwing: RemoteBackupError.providerUnavailable) - } else { - continuation.resume(returning: collected.result(status: status)) - } + do { continuation.resume(returning: try collected.result(status: status)) } + catch { continuation.resume(throwing: error) } } else { continuation.resume(throwing: RemoteBackupError.providerUnavailable) } } - if let data = command.standardInput { + if openedInput == nil, let data = command.standardInput { do { try input.fileHandleForWriting.write(contentsOf: data) } catch { collected.markInputWriteFailure() } } @@ -460,17 +695,17 @@ actor SFTPRemoteBackupProvider: RemoteBackupProvider { drains.leave() drains.leave() drains.leave() - if wasCancelled { - continuation.resume(throwing: CancellationError()) - } else { - continuation.resume(throwing: error) - } + continuation.resume(throwing: wasCancelled ? CancellationError() : error) } } }, onCancel: { lifetime.cancel() }) try Task.checkCancellation() + if let openedInput, + try OpenSSHInputIdentity(fileDescriptor: openedInput.handle.fileDescriptor) != openedInput.identity { + throw RemoteBackupError.localArtifactNotVerified + } return result } diff --git a/BitMatch/Resources/SFTPRemoteHelper.py b/BitMatch/Resources/SFTPRemoteHelper.py new file mode 100644 index 00000000..5761cdc5 --- /dev/null +++ b/BitMatch/Resources/SFTPRemoteHelper.py @@ -0,0 +1,237 @@ +"""One-shot SSH helper for BitMatch's opt-in remote backup. + +All names are relative to the authenticated account's working directory. +Directory descriptors pin each ancestor; leaf opens refuse symlinks, and +promotion uses linkat without replacing an existing final name. No pathname +is removed here. The app sends this bundled source to Python 3 over SSH. +This does not defend against another process with the same account privileges +mutating an already-open inode or moving a pinned directory itself. +""" + +import ctypes +import errno +import json +import os +import shutil +import stat +import sys + +# The SSH account can inherit a restrictive or permissive umask. Keep newly +# created directories accessible only to this account and leaf files private. +os.umask(0o077) +libc = ctypes.CDLL(None, use_errno=True) +for symbol in ("openat", "mkdirat", "linkat"): + if not hasattr(libc, symbol): + sys.exit(44) +libc.openat.argtypes = (ctypes.c_int, ctypes.c_char_p, ctypes.c_int, ctypes.c_int) +libc.openat.restype = ctypes.c_int +libc.mkdirat.argtypes = (ctypes.c_int, ctypes.c_char_p, ctypes.c_int) +libc.mkdirat.restype = ctypes.c_int +libc.linkat.argtypes = (ctypes.c_int, ctypes.c_char_p, ctypes.c_int, ctypes.c_char_p, ctypes.c_int) +libc.linkat.restype = ctypes.c_int +for name in ("O_NOFOLLOW", "O_DIRECTORY", "O_EXCL", "O_CLOEXEC"): + if not hasattr(os, name): + sys.exit(44) +DIR_FLAGS = os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC +FILE_FLAGS = os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK | os.O_CLOEXEC + + +def check(result): + if result < 0: + code = ctypes.get_errno() + raise OSError(code, os.strerror(code)) + return result + + +def openat(parent, name, flags, mode=0): + return check(libc.openat(parent, os.fsencode(name), flags, mode)) + + +def mkdirat(parent, name): + check(libc.mkdirat(parent, os.fsencode(name), 0o700)) + + +def linkat(src_parent, src_name, dst_parent, dst_name): + check(libc.linkat(src_parent, os.fsencode(src_name), dst_parent, os.fsencode(dst_name), 0)) + + +def parts(index): + value = json.loads(sys.argv[index]) + if not isinstance(value, list) or not value: + raise ValueError("bad path") + if any(not isinstance(p, str) or not p or p in (".", "..") or "/" in p or "\\" in p or "\x00" in p for p in value): + raise ValueError("bad path") + return value + + +def walk(parts, create=False): + current = os.open(".", DIR_FLAGS) + try: + for name in parts: + try: + next_fd = openat(current, name, DIR_FLAGS) + except OSError as exc: + if not create or exc.errno != errno.ENOENT: + raise + try: + mkdirat(current, name) + except OSError as mkdir_error: + if mkdir_error.errno != errno.EEXIST: + raise + next_fd = openat(current, name, DIR_FLAGS) + os.close(current) + current = next_fd + return current + except BaseException: + os.close(current) + raise + + +def regular(fd): + info = os.fstat(fd) + if not stat.S_ISREG(info.st_mode): + raise ValueError("not regular") + return info + + +def main(): + action = sys.argv[1] + if action == "check": + fd = walk([]) + os.close(fd) + print("__BITMATCH_HELPER_READY__") + elif action == "mkdir": + fd = walk(parts(2), create=True) + os.close(fd) + elif action == "inspect": + path = parts(2) + try: + parent = walk(path[:-1]) + except OSError as exc: + if exc.errno == errno.ENOENT: + return + raise + try: + try: + fd = openat(parent, path[-1], FILE_FLAGS) + except OSError as exc: + if exc.errno == errno.ENOENT: + return + raise + try: + print(regular(fd).st_size) + finally: + os.close(fd) + finally: + os.close(parent) + elif action == "upload": + path = parts(2) + parent = walk(path[:-1]) + try: + fd = openat(parent, path[-1], os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW | os.O_CLOEXEC, 0o600) + os.fchmod(fd, 0o600) + with os.fdopen(fd, "wb", buffering=1024 * 1024) as output: + shutil.copyfileobj(sys.stdin.buffer, output, 1024 * 1024) + output.flush() + os.fsync(output.fileno()) + finally: + os.close(parent) + elif action == "download": + path = parts(2) + parent = walk(path[:-1]) + try: + fd = openat(parent, path[-1], FILE_FLAGS) + regular(fd) + with os.fdopen(fd, "rb", buffering=1024 * 1024) as source: + shutil.copyfileobj(source, sys.stdout.buffer, 1024 * 1024) + sys.stdout.buffer.flush() + finally: + os.close(parent) + elif action == "promote": + source = parts(2) + final = parts(3) + src_parent = walk(source[:-1]) + dst_parent = walk(final[:-1]) + try: + src_fd = openat(src_parent, source[-1], FILE_FLAGS) + try: + original = regular(src_fd) + linkat(src_parent, source[-1], dst_parent, final[-1]) + final_fd = openat(dst_parent, final[-1], FILE_FLAGS) + try: + linked = regular(final_fd) + if (original.st_dev, original.st_ino) != (linked.st_dev, linked.st_ino): + raise ValueError("source changed") + finally: + os.close(final_fd) + finally: + os.close(src_fd) + finally: + os.close(src_parent) + os.close(dst_parent) + elif action == "same": + source = parts(2) + final = parts(3) + try: + src_parent = walk(source[:-1]) + except OSError as exc: + if exc.errno == errno.ENOENT: + print("__BITMATCH_DIFFERENT__") + return + raise + try: + try: + dst_parent = walk(final[:-1]) + except OSError as exc: + if exc.errno == errno.ENOENT: + print("__BITMATCH_DIFFERENT__") + return + raise + try: + src_fd = dst_fd = None + try: + src_fd = openat(src_parent, source[-1], FILE_FLAGS) + dst_fd = openat(dst_parent, final[-1], FILE_FLAGS) + original = regular(src_fd) + linked = regular(dst_fd) + same = (original.st_dev, original.st_ino) == (linked.st_dev, linked.st_ino) + print("__BITMATCH_SAME__" if same else "__BITMATCH_DIFFERENT__") + except OSError as exc: + if exc.errno == errno.ENOENT: + print("__BITMATCH_DIFFERENT__") + return + raise + finally: + if src_fd is not None: + os.close(src_fd) + if dst_fd is not None: + os.close(dst_fd) + finally: + os.close(dst_parent) + finally: + os.close(src_parent) + else: + raise ValueError("bad action") + + +try: + main() +except OSError as exc: + print("__BITMATCH_ERRNO__" + str(exc.errno), file=sys.stderr) + if exc.errno == errno.EEXIST and sys.argv[1] in ("upload", "promote"): + sys.exit(42) + if exc.errno in (errno.ELOOP, errno.ENOTDIR): + sys.exit(43) + if exc.errno in (errno.ENOSPC, getattr(errno, "EDQUOT", -1)): + print("__BITMATCH_STORAGE_FULL__", file=sys.stderr) + sys.exit(46) + if exc.errno in (errno.EACCES, errno.EPERM): + print("__BITMATCH_PERMISSION__", file=sys.stderr) + sys.exit(47) + if sys.argv[1] == "promote" and exc.errno in (errno.ENOSYS, errno.ENOTSUP, errno.EXDEV, getattr(errno, "EOPNOTSUPP", -1)): + print("__BITMATCH_NO_HARDLINK__", file=sys.stderr) + sys.exit(48) + sys.exit(45) +except (ValueError, json.JSONDecodeError): + print("__BITMATCH_UNSAFE_PATH__", file=sys.stderr) + sys.exit(43) diff --git a/BitMatchTests/PhotographerJobViewModelTests.swift b/BitMatchTests/PhotographerJobViewModelTests.swift index 006f0fe1..bd97e1be 100644 --- a/BitMatchTests/PhotographerJobViewModelTests.swift +++ b/BitMatchTests/PhotographerJobViewModelTests.swift @@ -643,6 +643,40 @@ struct PhotographerJobViewModelTests { #expect(RemoteBackupStatusPresentation.make(summary: consistent).isFullyBackedUp == (terminalState == .verified)) } + @Test func completedRemoteBackupStillShowsEarlierPartialCleanupPath() throws { + let store = InMemoryPhotographerJobStore() + let jobID = UUID() + let cardID = UUID() + let targetID = UUID() + try store.save(summaryJob(id: jobID, cardID: cardID, updatedAt: eventDate)) + let entry = RemoteManifestEntry( + id: UUID(), relativePath: try RemoteRelativePath(components: ["A.mov"]), + byteCount: 10, sha256: String(repeating: "a", count: 64) + ) + let manifest = try RemoteManifest( + id: UUID(), jobID: jobID, cardIngestID: cardID, + destinationProfileID: targetID, + packageRelativePath: RemoteRelativePath(components: ["Jobs", "Background"]), + entries: [entry], createdAt: eventDate + ) + try store.save(manifest) + var item = summaryItem( + jobID: jobID, cardID: cardID, targetID: targetID, + manifest: manifest, entry: entry, state: .verified, uploadedBytes: 10 + ) + item.verificationEvidence = .readBackSHA256(entry.sha256) + let oldTemporary = try RemoteRelativePath(components: ["Jobs", ".bitmatch-upload-11111111-2222-3333-4444-555555555555"]) + item.abandonedTemporaryRemoteRelativePaths = [oldTemporary] + + makeViewModel(store: store).refreshRemoteBackupSummary(for: cardID, items: [item]) + + let summary = try #require(store.storedJobs.first?.cardIngests.first?.remoteBackupSummaries[targetID]) + #expect(summary.state == .verified) + #expect(RemoteBackupStatusPresentation.make(summary: summary).isFullyBackedUp) + #expect(summary.errorSummary?.contains(oldTemporary.description) == true) + #expect(summary.errorSummary?.contains("restart from zero") == true) + } + @Test func zeroByteRemoteReadbackRemainsValid() throws { let store = InMemoryPhotographerJobStore() let jobID = UUID() diff --git a/BitMatchTests/RemoteBackupQueueTests.swift b/BitMatchTests/RemoteBackupQueueTests.swift index b405154b..1a4e742c 100644 --- a/BitMatchTests/RemoteBackupQueueTests.swift +++ b/BitMatchTests/RemoteBackupQueueTests.swift @@ -15,6 +15,48 @@ struct RemoteBackupQueueTests { #expect(await provider.uploadCallCount == 0) } + @Test func finalAppearingAtPromotionNeverCompletesUploadOnlyBackup() async throws { + let fixture = try QueueFixture(verificationMode: .uploadOnly) + let provider = FakeRemoteBackupProvider(injectFinalBeforePromotion: true) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + let stopped = try #require(await queue.item(id: fixture.item.id)) + #expect(stopped.state == .conflict) + #expect(stopped.verificationEvidence == .none) + #expect(await provider.promoteCallCount == 1) + } + + @Test func uploadOnlyRejectsUnsafeFinalAfterPromotion() async throws { + let fixture = try QueueFixture(verificationMode: .uploadOnly) + let provider = FakeRemoteBackupProvider(inspectErrorAfterPromotion: .unsafePath) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + let stopped = try #require(await queue.item(id: fixture.item.id)) + #expect(stopped.state == .paused) + #expect(stopped.verificationEvidence == .none) + #expect(await provider.promoteCallCount == 1) + } + + @Test func uploadOnlyRejectsWrongLengthFinalAfterPromotion() async throws { + let fixture = try QueueFixture(verificationMode: .uploadOnly) + let provider = FakeRemoteBackupProvider(promotionFinalByteCount: 9) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + let stopped = try #require(await queue.item(id: fixture.item.id)) + #expect(stopped.state == .failed) + #expect(stopped.verificationEvidence == .none) + #expect(await provider.promoteCallCount == 1) + } + @Test func ownedFinalAfterPromotionResumesVerificationInsteadOfConflicting() async throws { let fixture = try QueueFixture(verificationMode: .sha256) let provider = FakeRemoteBackupProvider(verificationErrors: [.networkUnavailable]) @@ -90,7 +132,84 @@ struct RemoteBackupQueueTests { #expect(await fixture.persistence.queueItem(id: fixture.item.id)?.verificationEvidence == RemoteVerificationEvidence.none) } - @Test func interruptedUploadResumesOnlyWhenStoredAndServerOffsetsMatch() async throws { + @Test func recoveredPromotionRecordsPossibleRetainedTemporaryName() async throws { + let fixture = try QueueFixture(uploadedByteCount: 10, verificationMode: .sha256, state: .verifying) + let provider = FakeRemoteBackupProvider(objects: [ + fixture.item.remoteRelativePath: .init(byteCount: 10), + fixture.item.temporaryRemoteRelativePath: .init(byteCount: 10) + ]) + await provider.markOwnedFinal( + temporary: fixture.item.temporaryRemoteRelativePath, + final: fixture.item.remoteRelativePath + ) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + let completed = try #require(await queue.item(id: fixture.item.id)) + #expect(completed.state == .verified) + #expect(completed.abandonedTemporaryRemoteRelativePaths == [fixture.item.temporaryRemoteRelativePath]) + #expect(await provider.promoteCallCount == 0) + } + + @Test func unrelatedSameSizeFinalAfterSavedPromotionIntentIsConflict() async throws { + let fixture = try QueueFixture(uploadedByteCount: 10, verificationMode: .uploadOnly, state: .verifying) + let provider = FakeRemoteBackupProvider(objects: [ + fixture.item.remoteRelativePath: .init(byteCount: 10), + fixture.item.temporaryRemoteRelativePath: .init(byteCount: 10) + ]) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + let stopped = try #require(await queue.item(id: fixture.item.id)) + #expect(stopped.state == .conflict) + #expect(stopped.verificationEvidence == .none) + #expect(await provider.uploadCallCount == 0) + #expect(await provider.promoteCallCount == 0) + } + + @Test func missingRetainedTemporaryAfterPromotionIsConflict() async throws { + let fixture = try QueueFixture(uploadedByteCount: 10, verificationMode: .uploadOnly, state: .verifying) + let provider = FakeRemoteBackupProvider(objects: [fixture.item.remoteRelativePath: .init(byteCount: 10)]) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + #expect(await queue.item(id: fixture.item.id)?.state == .conflict) + #expect(await provider.uploadCallCount == 0) + } + + @Test func sameSizeOldTemporaryParksUntilExplicitFreshRetry() async throws { + let fixture = try QueueFixture(uploadedByteCount: 10, verificationMode: .uploadOnly, state: .verifying) + let oldTemporary = fixture.item.temporaryRemoteRelativePath + let provider = FakeRemoteBackupProvider(objects: [oldTemporary: .init(byteCount: 10)]) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + await queue.run(fixture.item.id) + + let parked = try #require(await queue.item(id: fixture.item.id)) + #expect(parked.state == .paused) + #expect(parked.verificationEvidence == .none) + #expect(parked.abandonedTemporaryRemoteRelativePaths.contains(oldTemporary)) + #expect(await provider.uploadCallCount == 0) + #expect(await provider.promoteCallCount == 0) + + try await queue.retry(fixture.item.id) + await queue.run(fixture.item.id) + + let retried = try #require(await queue.item(id: fixture.item.id)) + #expect(retried.state == .uploadedUnverified) + #expect(retried.temporaryRemoteRelativePath != oldTemporary) + #expect(await provider.uploadOffsets == [0]) + #expect(try await provider.inspect(path: oldTemporary)?.byteCount == 10) + } + + @Test func interruptedUploadRestartsAtZeroWithNewPersistedAttempt() async throws { let fixture = try QueueFixture(uploadedByteCount: 4) let provider = FakeRemoteBackupProvider(objects: [fixture.item.temporaryRemoteRelativePath: .init(byteCount: 4)]) let queue = fixture.makeQueue(provider: provider) @@ -98,11 +217,63 @@ struct RemoteBackupQueueTests { try await queue.enqueue(fixture.item) await queue.run(fixture.item.id) - #expect(await provider.uploadOffsets == [4]) - #expect(await queue.item(id: fixture.item.id)?.state == .uploadedUnverified) + let completed = try #require(await queue.item(id: fixture.item.id)) + #expect(await provider.uploadOffsets == [0]) + #expect(completed.state == .uploadedUnverified) + #expect(completed.temporaryRemoteRelativePath != fixture.item.temporaryRemoteRelativePath) + #expect(completed.abandonedTemporaryRemoteRelativePaths == [fixture.item.temporaryRemoteRelativePath, completed.temporaryRemoteRelativePath]) + #expect(await provider.discardTemporaryCallCount == 0) + #expect(try await provider.inspect(path: fixture.item.temporaryRemoteRelativePath)?.byteCount == 4) } - @Test func mismatchedTemporaryOffsetDiscardsTemporaryAndRestartsSafely() async throws { + @Test func replacedOldTemporaryIsRecordedAndNeverReopened() async throws { + let fixture = try QueueFixture(uploadedByteCount: 4) + let oldTemporary = fixture.item.temporaryRemoteRelativePath + let provider = FakeRemoteBackupProvider(unsafeInspectPath: oldTemporary) + let queue = fixture.makeQueue(provider: provider) + + try await queue.enqueue(fixture.item) + await queue.run(fixture.item.id) + + let completed = try #require(await queue.item(id: fixture.item.id)) + #expect(completed.state == .uploadedUnverified) + #expect(completed.temporaryRemoteRelativePath != oldTemporary) + #expect(completed.abandonedTemporaryRemoteRelativePaths == [oldTemporary, completed.temporaryRemoteRelativePath]) + #expect(await provider.uploadOffsets == [0]) + #expect(await provider.discardTemporaryCallCount == 0) + } + + @Test func newAttemptPathIsSavedBeforeUploadStarts() async throws { + let fixture = try QueueFixture() + let provider = FakeRemoteBackupProvider(blockUpload: true) + let queue = fixture.makeQueue(provider: provider) + try await queue.enqueue(fixture.item) + + let running = Task { await queue.run(fixture.item.id) } + await provider.waitForUploadStart() + let persisted = try #require(await fixture.persistence.queueItem(id: fixture.item.id)) + #expect(persisted.state == .uploading) + #expect(persisted.uploadAttemptID != fixture.item.id) + #expect(persisted.temporaryRemoteRelativePath == (await provider.uploadPaths.first)) + #expect(await provider.uploadOffsets == [0]) + await provider.resumeUpload() + await running.value + } + + @Test func legacyQueueItemDecodesWithItsOriginalTemporaryIdentity() throws { + let item = try QueueFixture().item + let encoded = try JSONEncoder().encode(item) + var object = try #require(JSONSerialization.jsonObject(with: encoded) as? [String: Any]) + object.removeValue(forKey: "uploadAttemptID") + object.removeValue(forKey: "abandonedTemporaryRemoteRelativePaths") + + let decoded = try JSONDecoder().decode(RemoteQueueItem.self, from: JSONSerialization.data(withJSONObject: object)) + #expect(decoded.uploadAttemptID == item.id) + #expect(decoded.temporaryRemoteRelativePath == item.temporaryRemoteRelativePath) + #expect(decoded.abandonedTemporaryRemoteRelativePaths.isEmpty) + } + + @Test func mismatchedTemporaryOffsetLeavesOldObjectAndRestartsSafely() async throws { let fixture = try QueueFixture(uploadedByteCount: 4) let provider = FakeRemoteBackupProvider(objects: [fixture.item.temporaryRemoteRelativePath: .init(byteCount: 3)]) let queue = fixture.makeQueue(provider: provider) @@ -111,11 +282,13 @@ struct RemoteBackupQueueTests { await queue.run(fixture.item.id) #expect(await queue.item(id: fixture.item.id)?.state == .uploadedUnverified) - #expect(await provider.discardTemporaryCallCount == 1) + #expect(await provider.discardTemporaryCallCount == 0) #expect(await provider.uploadOffsets == [0]) + let completed = try #require(await queue.item(id: fixture.item.id)) + #expect(completed.abandonedTemporaryRemoteRelativePaths == [fixture.item.temporaryRemoteRelativePath, completed.temporaryRemoteRelativePath]) } - @Test func failedTemporaryCleanupPausesWithoutAppendingUnknownBytes() async throws { + @Test func unavailableCleanupIsNeverUsedForFreshAttempt() async throws { let fixture = try QueueFixture(uploadedByteCount: 4) let provider = FakeRemoteBackupProvider( objects: [fixture.item.temporaryRemoteRelativePath: .init(byteCount: 3)], @@ -126,9 +299,9 @@ struct RemoteBackupQueueTests { try await queue.enqueue(fixture.item) await queue.run(fixture.item.id) - #expect(await queue.item(id: fixture.item.id)?.state == .paused) - #expect(await queue.item(id: fixture.item.id)?.errorSummary?.contains("safely discarded") == true) - #expect(await provider.uploadCallCount == 0) + #expect(await queue.item(id: fixture.item.id)?.state == .uploadedUnverified) + #expect(await provider.uploadOffsets == [0]) + #expect(await provider.discardTemporaryCallCount == 0) } @Test func transientNetworkFailurePersistsRetryAndBackoff() async throws { @@ -145,6 +318,22 @@ struct RemoteBackupQueueTests { #expect(stored?.nextAttemptAt != nil) } + @Test func remoteStorageFullPausesWithoutPromotionOrAutomaticRetry() async throws { + let fixture = try QueueFixture() + let provider = FakeRemoteBackupProvider(uploadError: .remoteStorageFull) + let queue = fixture.makeQueue(provider: provider) + + try await queue.enqueue(fixture.item) + await queue.run(fixture.item.id) + + let stopped = try #require(await queue.item(id: fixture.item.id)) + #expect(stopped.state == .paused) + #expect(stopped.nextAttemptAt == nil) + #expect(stopped.uploadedByteCount == 0) + #expect(stopped.errorSummary?.contains(stopped.temporaryRemoteRelativePath.description) == true) + #expect(await provider.promoteCallCount == 0) + } + @Test func queueStoreWriteFailureFailsClosedThenRecoversOnRetry() async throws { let fixture = try QueueFixture() let provider = FakeRemoteBackupProvider() @@ -236,7 +425,14 @@ struct RemoteBackupQueueTests { try await queue.enqueue(fixture.item) await queue.run(fixture.item.id) - #expect(await provider.discardTemporaryCallCount == 1) + #expect(await provider.discardTemporaryCallCount == 0) + #expect(await provider.uploadCallCount == 0) + #expect(await provider.promoteCallCount == 0) + #expect(await queue.item(id: fixture.item.id)?.state == .paused) + + try await queue.retry(fixture.item.id) + await queue.run(fixture.item.id) + #expect(await provider.uploadOffsets == [0]) #expect(await provider.promoteCallCount == 1) #expect(await queue.item(id: fixture.item.id)?.state == .uploadedUnverified) @@ -295,6 +491,34 @@ struct RemoteBackupQueueTests { #expect(controller.photographerJobViewModel.activeJob?.id == activeJob.id) } + @Test @MainActor func controllerCancellationReachesRunningUploadTask() async throws { + let fixture = try QueueFixture() + let store = InMemoryPhotographerJobStore() + try store.save(fixture.job) + try store.save(fixture.manifest) + let viewModel = PhotographerJobViewModel( + store: store, + now: { Date(timeIntervalSince1970: 200) }, + workflowDefaults: .isolatedWorkflowDefaults(remembering: .photography) + ) + let provider = FakeRemoteBackupProvider(blockUpload: true) + let queue = fixture.makeQueue(provider: provider) + let controller = MacRemoteBackupController( + photographerJobViewModel: viewModel, results: { [] }, queue: queue + ) + try await queue.enqueue(fixture.item) + + let scheduler = Task { await controller.runDueRemoteBackups() } + await provider.waitForUploadStart() + await controller.cancelRemoteBackupNow(for: fixture.item.cardIngestID) + await provider.resumeUpload() + await scheduler.value + + #expect(await provider.uploadObservedCancellation) + #expect(await queue.item(id: fixture.item.id)?.state == .cancelled) + #expect(await provider.promoteCallCount == 0) + } + @Test func deferredBackoffWaitsForNextAttemptThenRunsAgain() async throws { let fixture = try QueueFixture() let provider = FakeRemoteBackupProvider(uploadError: .networkUnavailable) @@ -502,6 +726,8 @@ struct RemoteBackupQueueTests { #expect(await queue.item(id: fixture.item.id)?.state == .cancelled) #expect(await fixture.persistence.queueItem(id: fixture.item.id)?.state == .cancelled) + let cancelled = try #require(await queue.item(id: fixture.item.id)) + #expect(cancelled.errorSummary?.contains(cancelled.temporaryRemoteRelativePath.description) == true) #expect(await provider.promoteCallCount == 0) } @@ -522,7 +748,8 @@ struct RemoteBackupQueueTests { let saved = try #require(await fixture.persistence.queueItem(id: fixture.item.id)) #expect(saved.state == .paused) #expect(saved.verificationEvidence == .none) - #expect(saved.errorSummary == RemoteBackupError.interrupted.errorDescription) + #expect(saved.errorSummary?.hasPrefix(RemoteBackupError.interrupted.errorDescription ?? "") == true) + #expect(saved.errorSummary?.contains(saved.temporaryRemoteRelativePath.description) == true) #expect(RemoteBackupStatusPresentation.make(state: saved.state, evidence: saved.verificationEvidence).isFullyBackedUp == false) } @@ -540,7 +767,8 @@ struct RemoteBackupQueueTests { let saved = try #require(await fixture.persistence.queueItem(id: fixture.item.id)) #expect(saved.state == .failed) - #expect(saved.errorSummary == RemoteBackupError.verificationFailed.errorDescription) + #expect(saved.errorSummary?.hasPrefix(RemoteBackupError.verificationFailed.errorDescription ?? "") == true) + #expect(saved.errorSummary?.contains(saved.temporaryRemoteRelativePath.description) == true) #expect(saved.verificationEvidence == .none) #expect(await provider.promoteCallCount == 0) } @@ -586,7 +814,7 @@ struct RemoteBackupQueueTests { #expect(await fixture.persistence.queueItem(id: fixture.item.id)?.state == .paused) } - @Test func pausedUploadWithRemoteDriftDiscardsTemporaryBeforeRetry() async throws { + @Test func pausedUploadWithRemoteDriftLeavesOldTemporaryBeforeRetry() async throws { let fixture = try QueueFixture() let provider = FakeRemoteBackupProvider(blockUpload: true) let queue = fixture.makeQueue(provider: provider) @@ -600,15 +828,20 @@ struct RemoteBackupQueueTests { #expect(await queue.item(id: fixture.item.id)?.state == .paused) #expect(await queue.item(id: fixture.item.id)?.uploadedByteCount == 0) + #expect(await queue.item(id: fixture.item.id)?.errorSummary?.contains("starts this file from zero") == true) + let firstAttemptPath = try #require(await queue.item(id: fixture.item.id)?.temporaryRemoteRelativePath) try await queue.retry(fixture.item.id) await queue.run(fixture.item.id) - #expect(await provider.discardTemporaryCallCount == 1) + #expect(await provider.discardTemporaryCallCount == 0) #expect(await provider.uploadOffsets == [0, 0]) #expect(await queue.item(id: fixture.item.id)?.state == .uploadedUnverified) + #expect(await queue.item(id: fixture.item.id)?.temporaryRemoteRelativePath != firstAttemptPath) + let secondAttempt = try #require(await queue.item(id: fixture.item.id)?.temporaryRemoteRelativePath) + #expect(await queue.item(id: fixture.item.id)?.abandonedTemporaryRemoteRelativePaths == [firstAttemptPath, secondAttempt]) } - @Test(arguments: ["manifest", "entry", "profile", "sameTemporary"]) + @Test(arguments: ["manifest", "entry", "profile", "sameTemporary", "attemptID"]) func invalidPersistedLinkagePausesBeforeProviderUse(_ mismatch: String) async throws { let fixture = try QueueFixture(mismatch: mismatch) let provider = FakeRemoteBackupProvider() @@ -778,6 +1011,7 @@ private struct QueueFixture { localArtifactRelativePath: file, remoteRelativePath: remotePath, temporaryRemoteRelativePath: temporaryPath, + uploadAttemptID: mismatch == "attemptID" ? UUID() : nil, state: state, uploadedByteCount: uploadedByteCount, retryCount: 0, @@ -909,12 +1143,18 @@ private enum QueuePersistenceError: Error, Equatable { private actor FakeRemoteBackupProvider: RemoteBackupProvider { private var remoteObjects: [RemoteRelativePath: RemoteObject] + private var ownedFinals: [RemoteRelativePath: RemoteRelativePath] = [:] private let capabilities: RemoteProviderCapabilities private let preflightError: RemoteBackupError? private let uploadError: RemoteBackupError? private var verificationErrors: [RemoteBackupError] private var blockUpload: Bool private let blockPromotion: Bool + private let injectFinalBeforePromotion: Bool + private let inspectErrorAfterPromotion: RemoteBackupError? + private let unsafeInspectPath: RemoteRelativePath? + private let promotionFinalByteCount: Int64? + private var promotedFinalPath: RemoteRelativePath? private let discardTemporaryError: RemoteBackupError? private let uploadByteCount: Int64 private var uploadStartedContinuation: CheckedContinuation? @@ -922,8 +1162,10 @@ private actor FakeRemoteBackupProvider: RemoteBackupProvider { private var promotionStartedContinuation: CheckedContinuation? private var promotionContinuation: CheckedContinuation? private(set) var uploadCallCount = 0 + private(set) var uploadObservedCancellation = false private(set) var preflightCallCount = 0 private(set) var uploadOffsets: [Int64] = [] + private(set) var uploadPaths: [RemoteRelativePath] = [] private(set) var promoteCallCount = 0 private(set) var discardTemporaryCallCount = 0 @@ -935,6 +1177,10 @@ private actor FakeRemoteBackupProvider: RemoteBackupProvider { verificationErrors: [RemoteBackupError] = [], blockUpload: Bool = false, blockPromotion: Bool = false, + injectFinalBeforePromotion: Bool = false, + inspectErrorAfterPromotion: RemoteBackupError? = nil, + unsafeInspectPath: RemoteRelativePath? = nil, + promotionFinalByteCount: Int64? = nil, discardTemporaryError: RemoteBackupError? = nil, uploadByteCount: Int64 = 10 ) { @@ -945,6 +1191,10 @@ private actor FakeRemoteBackupProvider: RemoteBackupProvider { self.verificationErrors = verificationErrors self.blockUpload = blockUpload self.blockPromotion = blockPromotion + self.injectFinalBeforePromotion = injectFinalBeforePromotion + self.inspectErrorAfterPromotion = inspectErrorAfterPromotion + self.unsafeInspectPath = unsafeInspectPath + self.promotionFinalByteCount = promotionFinalByteCount self.discardTemporaryError = discardTemporaryError self.uploadByteCount = uploadByteCount } @@ -955,7 +1205,13 @@ private actor FakeRemoteBackupProvider: RemoteBackupProvider { return capabilities } - func inspect(path: RemoteRelativePath) async throws -> RemoteObject? { remoteObjects[path] } + func inspect(path: RemoteRelativePath) async throws -> RemoteObject? { + if path == unsafeInspectPath { throw RemoteBackupError.unsafePath } + if path == promotedFinalPath, let inspectErrorAfterPromotion { + throw inspectErrorAfterPromotion + } + return remoteObjects[path] + } func ensureDirectory(_: RemoteRelativePath) async throws {} func discardTemporary(_ path: RemoteRelativePath) async throws { @@ -968,15 +1224,18 @@ private actor FakeRemoteBackupProvider: RemoteBackupProvider { local _: URL, toTemporary path: RemoteRelativePath, fromOffset: Int64, + expectedSHA256 _: String, progress: @Sendable (Int64) async -> Void ) async throws { uploadCallCount += 1 uploadOffsets.append(fromOffset) + uploadPaths.append(path) uploadStartedContinuation?.resume() uploadStartedContinuation = nil if blockUpload { await withCheckedContinuation { uploadContinuation = $0 } } + uploadObservedCancellation = Task.isCancelled if let uploadError { throw uploadError } remoteObjects[path] = RemoteObject(byteCount: uploadByteCount) await progress(uploadByteCount) @@ -989,8 +1248,21 @@ private actor FakeRemoteBackupProvider: RemoteBackupProvider { if blockPromotion { await withCheckedContinuation { promotionContinuation = $0 } } + if injectFinalBeforePromotion { + remoteObjects[final] = .init(byteCount: uploadByteCount) + } if remoteObjects[final] != nil { throw RemoteBackupError.conflict } - remoteObjects[final] = remoteObjects.removeValue(forKey: temporary) + remoteObjects[final] = promotionFinalByteCount.map { RemoteObject(byteCount: $0) } ?? remoteObjects[temporary] + promotedFinalPath = final + ownedFinals[final] = temporary + } + + func confirmsOwnedFinal(temporary: RemoteRelativePath, final: RemoteRelativePath) async throws -> Bool { + ownedFinals[final] == temporary && remoteObjects[temporary] != nil && remoteObjects[final] != nil + } + + func markOwnedFinal(temporary: RemoteRelativePath, final: RemoteRelativePath) { + ownedFinals[final] = temporary } func verificationEvidence(for _: RemoteRelativePath, expectedSHA256 _: String) async throws -> RemoteVerificationEvidence { diff --git a/BitMatchTests/SFTPRemoteBackupLiveTests.swift b/BitMatchTests/SFTPRemoteBackupLiveTests.swift new file mode 100644 index 00000000..b23abc1c --- /dev/null +++ b/BitMatchTests/SFTPRemoteBackupLiveTests.swift @@ -0,0 +1,131 @@ +import Foundation +import Testing +import BitMatchEngine +@testable import BitMatch + +#if os(macOS) +/// Run through Scripts/test-local-sftp.sh. It supplies a disposable localhost +/// sshd, temporary agent key, pinned known_hosts, and temporary remote root. +/// This test never uses saved BitMatch profiles or credentials. +struct SFTPRemoteBackupLiveTests { + @Test(.enabled(if: ProcessInfo.processInfo.environment["BITMATCH_LIVE_SFTP_TEST"] == "1" + || ProcessInfo.processInfo.environment["TEST_RUNNER_BITMATCH_LIVE_SFTP_TEST"] == "1")) + func firstUploadReadbackAndNoReplacePromotion() async throws { + let environment = ProcessInfo.processInfo.environment + let portText = try #require(environment["BITMATCH_LIVE_SFTP_PORT"] + ?? environment["TEST_RUNNER_BITMATCH_LIVE_SFTP_PORT"]) + let port = try #require(Int(portText)) + let knownHostsPath = try #require(environment["BITMATCH_LIVE_SFTP_KNOWN_HOSTS"] + ?? environment["TEST_RUNNER_BITMATCH_LIVE_SFTP_KNOWN_HOSTS"]) + let agentSocket = try #require(environment["BITMATCH_LIVE_SFTP_AGENT_SOCK"] + ?? environment["TEST_RUNNER_BITMATCH_LIVE_SFTP_AGENT_SOCK"]) + let remoteDirectoryPath = try #require(environment["BITMATCH_LIVE_SFTP_REMOTE_DIR"] + ?? environment["TEST_RUNNER_BITMATCH_LIVE_SFTP_REMOTE_DIR"]) + let remoteDirectory = URL(fileURLWithPath: remoteDirectoryPath).standardizedFileURL + try #require(remoteDirectory.path.hasPrefix("/tmp/bitmatch-sftp-smoke.") + || remoteDirectory.path.hasPrefix("/private/tmp/bitmatch-sftp-smoke.")) + try #require(FileManager.default.fileExists(atPath: remoteDirectory + .appendingPathComponent(".bitmatch-disposable-sftp-server").path)) + #expect(FileManager.default.fileExists(atPath: agentSocket)) + // This opt-in test runs alone. Only its process and OpenSSH children + // see the disposable agent; the user's own agent remains untouched. + setenv("SSH_AUTH_SOCK", agentSocket, 1) + + let profile = RemoteDestinationProfile( + id: UUID(), name: "Disposable localhost test", host: "127.0.0.1", port: port, + username: NSUserName(), root: try RemoteRelativePath(components: ["Backups"]), + verificationMode: .sha256 + ) + let provider = SFTPRemoteBackupProvider( + profile: profile, credential: .sshAgent, + knownHostsURL: URL(fileURLWithPath: knownHostsPath) + ) + let capabilities = try await provider.preflight(profile: profile, credential: .sshAgent) + #expect(capabilities.missingRequirements(for: .sha256).isEmpty) + + let folder = try RemoteRelativePath(components: ["Run-\(UUID().uuidString.lowercased())"]) + try await provider.ensureDirectory(folder) + let firstTemporary = try folder.appending(".bitmatch-upload-\(UUID().uuidString.lowercased())") + let secondTemporary = try folder.appending(".bitmatch-upload-\(UUID().uuidString.lowercased())") + let final = try folder.appending("clip.bin") + + let localFolder = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: localFolder, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: localFolder) } + let source = localFolder.appendingPathComponent("source.bin") + let contender = localFolder.appendingPathComponent("contender.bin") + try Data("BitMatch disposable first upload\n".utf8).write(to: source) + try Data("different bytes\n".utf8).write(to: contender) + let expected = try await ChecksumEngine.shared.generateChecksum(for: source, type: .sha256) + let contenderExpected = try await ChecksumEngine.shared.generateChecksum(for: contender, type: .sha256) + + try await provider.upload(local: source, toTemporary: firstTemporary, fromOffset: 0, expectedSHA256: expected) { _ in } + #expect(try await provider.inspect(path: firstTemporary)?.byteCount == Int64(Data("BitMatch disposable first upload\n".utf8).count)) + try await provider.promoteNoReplace(temporary: firstTemporary, final: final) + #expect(try await provider.confirmsOwnedFinal(temporary: firstTemporary, final: final)) + #expect(try await provider.inspect(path: firstTemporary)?.byteCount == Int64(Data("BitMatch disposable first upload\n".utf8).count)) + let firstTemporaryOnDisk = remoteDirectory.appendingPathComponent("Backups") + .appendingPathComponent(folder.description) + .appendingPathComponent(try #require(firstTemporary.components.last)) + let finalOnDisk = remoteDirectory.appendingPathComponent("Backups") + .appendingPathComponent(folder.description).appendingPathComponent("clip.bin") + let temporaryInode = try #require(FileManager.default.attributesOfItem(atPath: firstTemporaryOnDisk.path)[.systemFileNumber] as? NSNumber) + let finalInode = try #require(FileManager.default.attributesOfItem(atPath: finalOnDisk.path)[.systemFileNumber] as? NSNumber) + #expect(temporaryInode == finalInode) + #expect(try await provider.verificationEvidence(for: final, expectedSHA256: expected) == .readBackSHA256(expected)) + + let linked = try folder.appending("linked") + let dangling = try folder.appending("dangling") + try FileManager.default.createSymbolicLink( + at: finalOnDisk.deletingLastPathComponent().appendingPathComponent("linked"), + withDestinationURL: finalOnDisk + ) + try FileManager.default.createSymbolicLink( + at: finalOnDisk.deletingLastPathComponent().appendingPathComponent("dangling"), + withDestinationURL: finalOnDisk.deletingLastPathComponent().appendingPathComponent("missing.bin") + ) + await #expect(throws: RemoteBackupError.unsafePath) { _ = try await provider.inspect(path: linked) } + await #expect(throws: RemoteBackupError.unsafePath) { _ = try await provider.inspect(path: dangling) } + + // A symlink to an existing sink is more dangerous than a dangling + // link: shell noclobber can follow it and report a successful upload. + let deviceLink = try folder.appending(".bitmatch-upload-\(UUID().uuidString.lowercased())") + try FileManager.default.createSymbolicLink( + at: finalOnDisk.deletingLastPathComponent().appendingPathComponent(try #require(deviceLink.components.last)), + withDestinationURL: URL(fileURLWithPath: "/dev/null") + ) + await #expect(throws: RemoteBackupError.unsafePath) { + try await provider.upload(local: source, toTemporary: deviceLink, fromOffset: 0, expectedSHA256: expected) { _ in } + } + + try await provider.upload(local: contender, toTemporary: secondTemporary, fromOffset: 0, expectedSHA256: contenderExpected) { _ in } + #expect(try await provider.confirmsOwnedFinal(temporary: secondTemporary, final: final) == false) + await #expect(throws: RemoteBackupError.conflict) { + try await provider.promoteNoReplace(temporary: secondTemporary, final: final) + } + #expect(try await provider.verificationEvidence(for: final, expectedSHA256: expected) == .readBackSHA256(expected)) + + // Plain `ln temp link-to-directory` treats the link as a directory and + // succeeds by writing inside it. No-follow promotion must refuse it. + let outsideDirectory = remoteDirectory.appendingPathComponent("outside") + try FileManager.default.createDirectory(at: outsideDirectory, withIntermediateDirectories: true) + let directoryLink = finalOnDisk.deletingLastPathComponent().appendingPathComponent("directory-link") + try FileManager.default.createSymbolicLink(at: directoryLink, withDestinationURL: outsideDirectory) + await #expect(throws: RemoteBackupError.conflict) { + try await provider.promoteNoReplace( + temporary: secondTemporary, + final: try folder.appending("directory-link") + ) + } + #expect(try FileManager.default.contentsOfDirectory(atPath: outsideDirectory.path).isEmpty) + + // Names are JSON path components inside one shell-quoted helper + // invocation; punctuation must remain data, never shell syntax. + let quotedTemporary = try folder.appending(".bitmatch-upload-\(UUID().uuidString.lowercased())") + let quotedFinal = try folder.appending("O'Reilly; clip.bin") + try await provider.upload(local: source, toTemporary: quotedTemporary, fromOffset: 0, expectedSHA256: expected) { _ in } + try await provider.promoteNoReplace(temporary: quotedTemporary, final: quotedFinal) + #expect(try await provider.verificationEvidence(for: quotedFinal, expectedSHA256: expected) == .readBackSHA256(expected)) + } +} +#endif diff --git a/BitMatchTests/SFTPRemoteBackupProcessTests.swift b/BitMatchTests/SFTPRemoteBackupProcessTests.swift index 22aa291a..f2e34f2f 100644 --- a/BitMatchTests/SFTPRemoteBackupProcessTests.swift +++ b/BitMatchTests/SFTPRemoteBackupProcessTests.swift @@ -29,6 +29,196 @@ struct SFTPRemoteBackupProcessTests { #expect(result.stderr.isEmpty) } + @Test func verboseChildOutputIsDrainedButCaptureStaysBounded() async throws { + let result = try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/sh", + arguments: ["-c", "/usr/bin/head -c 4194304 /dev/zero; /usr/bin/head -c 4194304 /dev/zero >&2"], + standardInput: nil + )) + + #expect(result.status == 0) + #expect(result.stdout.count == 1024 * 1024) + #expect(result.stderr.count == 1024 * 1024) + } + + @Test func streamsFileInputToExclusiveOutputWithoutCapturingFootage() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let source = directory.appendingPathComponent("source") + let readback = directory.appendingPathComponent("readback") + let chunk = Data(repeating: 0x8f, count: 1024 * 1024) + try chunk.write(to: source) + let sourceHandle = try FileHandle(forWritingTo: source) + try sourceHandle.seekToEnd() + for _ in 0..<15 { try sourceHandle.write(contentsOf: chunk) } + try sourceHandle.close() + + let result = try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/cat", arguments: [], standardInput: nil, + standardInputFile: source, standardOutputFile: readback + )) + + #expect(result.status == 0) + #expect(result.stdout.isEmpty) + #expect(result.stderr.isEmpty) + #expect(try FileManager.default.attributesOfItem(atPath: readback.path)[.size] as? Int == 16 * 1024 * 1024) + #expect(try Data(contentsOf: readback) == Data(contentsOf: source)) + } + + @Test func exclusiveOutputRejectsExistingSymlinkWithoutChangingTarget() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let target = directory.appendingPathComponent("target") + let output = directory.appendingPathComponent("output") + try Data("untouched".utf8).write(to: target) + try FileManager.default.createSymbolicLink(at: output, withDestinationURL: target) + + await #expect(throws: Error.self) { + try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/sh", arguments: ["-c", "printf replaced"], + standardInput: nil, standardOutputFile: output + )) + } + #expect(try Data(contentsOf: target) == Data("untouched".utf8)) + } + + @Test func fileInputRejectsSymlinkAndFIFOBeforeStartingChild() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let target = directory.appendingPathComponent("target") + let link = directory.appendingPathComponent("source-link") + let fifo = directory.appendingPathComponent("source-fifo") + let marker = directory.appendingPathComponent("started") + try Data("private bytes".utf8).write(to: target) + try FileManager.default.createSymbolicLink(at: link, withDestinationURL: target) + try #require(Darwin.mkfifo(fifo.path, 0o600) == 0) + + await #expect(throws: RemoteBackupError.localArtifactNotVerified) { + try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/sh", arguments: ["-c", "printf started > \"$1\"", "sh", marker.path], + standardInput: nil, standardInputFile: link + )) + } + await #expect(throws: RemoteBackupError.localArtifactNotVerified) { + try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/sh", arguments: ["-c", "printf started > \"$1\"", "sh", marker.path], + standardInput: nil, standardInputFile: fifo + )) + } + #expect(!FileManager.default.fileExists(atPath: marker.path)) + #expect(try Data(contentsOf: target) == Data("private bytes".utf8)) + } + + @Test func sameSizeLeafReplacementAfterValidationCannotStartUpload() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let source = directory.appendingPathComponent("source") + let old = directory.appendingPathComponent("old") + let marker = directory.appendingPathComponent("started") + try Data("original".utf8).write(to: source) + let expected = try await ChecksumEngine.shared.generateChecksum(for: source, type: .sha256) + try FileManager.default.moveItem(at: source, to: old) + try Data("replaced".utf8).write(to: source) + + await #expect(throws: RemoteBackupError.localArtifactNotVerified) { + try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/sh", arguments: ["-c", "printf started > \"$1\"", "sh", marker.path], + standardInput: nil, standardInputFile: source, expectedInputSHA256: expected + )) + } + #expect(!FileManager.default.fileExists(atPath: marker.path)) + } + + @Test func sameSizeAncestorSwapAfterValidationCannotStartUpload() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + let current = directory.appendingPathComponent("current") + let other = directory.appendingPathComponent("other") + let old = directory.appendingPathComponent("old") + try FileManager.default.createDirectory(at: current, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: other, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let source = current.appendingPathComponent("source") + let marker = directory.appendingPathComponent("started") + try Data("original".utf8).write(to: source) + try Data("replaced".utf8).write(to: other.appendingPathComponent("source")) + let expected = try await ChecksumEngine.shared.generateChecksum(for: source, type: .sha256) + try FileManager.default.moveItem(at: current, to: old) + try FileManager.default.createSymbolicLink(at: current, withDestinationURL: other) + + await #expect(throws: RemoteBackupError.localArtifactNotVerified) { + try await SFTPRemoteBackupProvider.runProcess(OpenSSHCommand( + executable: "/bin/sh", arguments: ["-c", "printf started > \"$1\"", "sh", marker.path], + standardInput: nil, standardInputFile: source, expectedInputSHA256: expected + )) + } + #expect(!FileManager.default.fileExists(atPath: marker.path)) + } + + @Test func changedPinnedInputCannotReturnSuccessfulUpload() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let source = directory.appendingPathComponent("source") + let ready = directory.appendingPathComponent("ready") + let gate = directory.appendingPathComponent("gate") + try Data("original".utf8).write(to: source) + try #require(Darwin.mkfifo(gate.path, 0o600) == 0) + let command = OpenSSHCommand( + executable: "/bin/sh", + arguments: ["-c", "cat >/dev/null; printf ready > \"$1\"; cat \"$2\" >/dev/null", "sh", ready.path, gate.path], + standardInput: nil, standardInputFile: source + ) + let task = Task { try await SFTPRemoteBackupProvider.runProcess(command) } + guard await waitUntil(timeout: .seconds(3), { FileManager.default.fileExists(atPath: ready.path) }) else { + task.cancel() + _ = try? await task.value + Issue.record("The child did not consume the pinned input") + return + } + let editor = try FileHandle(forWritingTo: source) + try editor.truncate(atOffset: 0) + try editor.write(contentsOf: Data("changed".utf8)) + try editor.close() + let release = try FileHandle(forWritingTo: gate) + try release.write(contentsOf: Data("go".utf8)) + try release.close() + + await #expect(throws: RemoteBackupError.localArtifactNotVerified) { try await task.value } + } + + @Test func cancellingFileInputCommandReapsChild() async throws { + let directory = FileManager.default.temporaryDirectory.appendingPathComponent("bitmatch-sftp-stream-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: directory) } + let source = directory.appendingPathComponent("source") + let output = directory.appendingPathComponent("output") + let marker = directory.appendingPathComponent("pid") + try Data(repeating: 0x77, count: 1024 * 1024).write(to: source) + let command = OpenSSHCommand( + executable: "/bin/sh", + arguments: ["-c", "printf '%s\\n' \"$$\" > \"$1\"; exec /bin/sleep 8", "sh", marker.path], + standardInput: nil, standardInputFile: source, standardOutputFile: output + ) + let task = Task { try await SFTPRemoteBackupProvider.runProcess(command) } + guard await waitUntil(timeout: .seconds(3), { FileManager.default.fileExists(atPath: marker.path) }) else { + task.cancel() + _ = try? await task.value + Issue.record("The file-stream child never started") + return + } + let pid = try #require(Int32(String(contentsOf: marker, encoding: .utf8).trimmingCharacters(in: .whitespacesAndNewlines))) + let started = ContinuousClock.now + task.cancel() + + await #expect(throws: CancellationError.self) { try await task.value } + #expect(ContinuousClock.now - started < .seconds(3)) + #expect(Darwin.kill(pid, 0) == -1 && errno == ESRCH) + } + @Test func earlyExitWhileWritingInputCannotReportSuccess() async throws { let command = OpenSSHCommand( executable: "/bin/sh", diff --git a/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift b/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift index 69143291..748f5632 100644 --- a/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift +++ b/BitMatchTests/SFTPRemoteBackupProviderContractTests.swift @@ -25,7 +25,22 @@ struct SFTPRemoteBackupProviderContractTests { @Test func remotePathsUsePOSIXSingleQuoteEscaping() { #if os(macOS) - #expect(SFTPRemoteBackupProvider.shellQuote("/remote/O'Reilly.mov") == "'/remote/O'\\\"'\\\"'Reilly.mov'") + #expect(SFTPRemoteBackupProvider.shellQuote("/remote/O'Reilly.mov") == "'/remote/O'\"'\"'Reilly.mov'") + #endif + } + + @Test func quotedRemoteNamesRemainOneShellArgument() throws { + #if os(macOS) + let name = "O'Reilly; clip.bin" + let process = Process() + let output = Pipe() + process.executableURL = URL(fileURLWithPath: "/bin/sh") + process.arguments = ["-c", "printf %s " + SFTPRemoteBackupProvider.shellQuote(name)] + process.standardOutput = output + try process.run() + process.waitUntilExit() + #expect(process.terminationStatus == 0) + #expect(String(decoding: output.fileHandleForReading.readDataToEndOfFile(), as: UTF8.self) == name) #endif } @@ -46,16 +61,16 @@ struct SFTPRemoteBackupProviderContractTests { #endif } - @Test func existingFinalFromLnIsClassifiedAsConflict() { + @Test func hostKeyVerificationFailureIsClassifiedBeforeCapabilityFailure() { #if os(macOS) - #expect(SFTPRemoteBackupProvider.promotionError(stderr: Data("ln: File exists".utf8)) == .conflict) - #expect(SFTPRemoteBackupProvider.promotionError(stderr: Data("shell disabled".utf8)) == .capabilityUnavailable(.noReplacePromotion)) + #expect(SFTPRemoteBackupProvider.classify(stderr: Data("Host key verification failed.".utf8)) == .hostKeyMismatch) #endif } - @Test func hostKeyVerificationFailureIsClassifiedBeforeCapabilityFailure() { + @Test func remoteStorageFailureIsNotMisclassifiedAsNetworkRetry() { #if os(macOS) - #expect(SFTPRemoteBackupProvider.classify(stderr: Data("Host key verification failed.".utf8)) == .hostKeyMismatch) + #expect(SFTPRemoteBackupProvider.classify(stderr: Data("cat: write error: No space left on device".utf8)) == .remoteStorageFull) + #expect(SFTPRemoteBackupProvider.classify(stderr: Data("Disk quota exceeded".utf8)) == .remoteStorageFull) #endif } @@ -77,7 +92,45 @@ struct SFTPRemoteBackupProviderContractTests { #endif } - @Test func resumeOffsetMismatchStopsBeforeSFTPTransfer() async throws { + @Test func missingRemotePythonFailsClearlyAtPreflight() async throws { + #if os(macOS) + let recorder = OpenSSHCommandRecorder(missingPython: true) + let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") + try FileManager.default.createDirectory(at: knownHosts.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data("backup.example ssh-ed25519 AAAA\n".utf8).write(to: knownHosts) + defer { try? FileManager.default.removeItem(at: knownHosts.deletingLastPathComponent()) } + let profile = try fixtureProfile() + let provider = SFTPRemoteBackupProvider(profile: profile, credential: .sshAgent, + knownHostsURL: knownHosts, run: { command in await recorder.run(command) }) + + await #expect(throws: RemoteBackupError.streamUploadUnavailable) { + _ = try await provider.preflight(profile: profile, credential: .sshAgent) + } + #expect((await recorder.commands()).count == 1) + #endif + } + + @Test func promotionLinksWithoutPathnameCleanup() async throws { + #if os(macOS) + let recorder = OpenSSHCommandRecorder() + let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") + try FileManager.default.createDirectory(at: knownHosts.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data("backup.example ssh-ed25519 AAAA\n".utf8).write(to: knownHosts) + defer { try? FileManager.default.removeItem(at: knownHosts.deletingLastPathComponent()) } + let provider = SFTPRemoteBackupProvider(profile: try fixtureProfile(), credential: .sshAgent, + knownHostsURL: knownHosts, run: { command in await recorder.run(command) }) + let temporary = try RemoteRelativePath(components: ["Jobs", ".bitmatch-upload-11111111-2222-3333-4444-555555555555"]) + let final = try RemoteRelativePath(components: ["Jobs", "A.mov"]) + + try await provider.promoteNoReplace(temporary: temporary, final: final) + + let commands = await recorder.commands() + #expect(commands.count == 1) + #expect(commands[0].arguments.last?.contains("'promote' '[\"Backups\",\"Jobs\",\".bitmatch-upload-11111111-2222-3333-4444-555555555555\"]' '[\"Backups\",\"Jobs\",\"A.mov\"]'") == true) + #endif + } + + @Test func nonzeroOffsetStopsBeforeStreaming() async throws { #if os(macOS) let recorder = OpenSSHCommandRecorder(inspectLength: 3) let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") @@ -88,14 +141,81 @@ struct SFTPRemoteBackupProviderContractTests { let temporary = try RemoteRelativePath(components: ["Jobs", ".bitmatch-upload-item"]) await #expect(throws: RemoteBackupError.resumeOffsetMismatch(local: 2, remote: 3)) { - try await provider.upload(local: URL(fileURLWithPath: "/tmp/local.mov"), toTemporary: temporary, fromOffset: 2) { _ in } + try await provider.upload(local: URL(fileURLWithPath: "/tmp/local.mov"), toTemporary: temporary, fromOffset: 2, expectedSHA256: String(repeating: "a", count: 64)) { _ in } + } + + #expect(!(await recorder.commands()).contains(where: { $0.standardInputFile != nil })) + #endif + } + + @Test func firstUploadClaimsAndStreamsThroughOneSSHCommand() async throws { + #if os(macOS) + let recorder = OpenSSHCommandRecorder() + let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") + try FileManager.default.createDirectory(at: knownHosts.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data("backup.example ssh-ed25519 AAAA\n".utf8).write(to: knownHosts) + defer { try? FileManager.default.removeItem(at: knownHosts.deletingLastPathComponent()) } + let provider = SFTPRemoteBackupProvider( + profile: try fixtureProfile(), credential: .sshAgent, + knownHostsURL: knownHosts, run: { command in await recorder.run(command) } + ) + let temporary = try RemoteRelativePath(components: ["Jobs", ".bitmatch-upload-11111111-2222-3333-4444-555555555555"]) + + try await provider.upload(local: URL(fileURLWithPath: "/tmp/local.mov"), toTemporary: temporary, fromOffset: 0, expectedSHA256: String(repeating: "a", count: 64)) { _ in } + + let commands = await recorder.commands() + #expect(commands.count == 3) + #expect(commands[0].executable == "/usr/bin/ssh") + #expect(commands[1].executable == "/usr/bin/ssh") + #expect(commands[1].arguments.last?.contains("'upload' '[\"Backups\",\"Jobs\",\".bitmatch-upload-11111111-2222-3333-4444-555555555555\"]'") == true) + #expect(commands[1].standardInputFile == URL(fileURLWithPath: "/tmp/local.mov")) + #expect(commands[1].expectedInputSHA256 == String(repeating: "a", count: 64)) + #expect(commands[1].standardInput == nil) + #expect(commands[2].executable == "/usr/bin/ssh") + #endif + } + + @Test func failedExclusiveClaimNeverReportsUploadComplete() async throws { + #if os(macOS) + let recorder = OpenSSHCommandRecorder(claimFails: true) + let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") + try FileManager.default.createDirectory(at: knownHosts.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data("backup.example ssh-ed25519 AAAA\n".utf8).write(to: knownHosts) + defer { try? FileManager.default.removeItem(at: knownHosts.deletingLastPathComponent()) } + let provider = SFTPRemoteBackupProvider( + profile: try fixtureProfile(), credential: .sshAgent, + knownHostsURL: knownHosts, run: { command in await recorder.run(command) } + ) + let temporary = try RemoteRelativePath(components: ["Jobs", ".bitmatch-upload-11111111-2222-3333-4444-555555555555"]) + + await #expect(throws: RemoteBackupError.temporaryClaimFailed) { + try await provider.upload(local: URL(fileURLWithPath: "/tmp/local.mov"), toTemporary: temporary, fromOffset: 0, expectedSHA256: String(repeating: "a", count: 64)) { _ in } } + #expect((await recorder.commands()).filter { $0.standardInputFile != nil }.count == 1) + #endif + } - #expect(!(await recorder.commands()).contains(where: { $0.executable == "/usr/bin/sftp" })) + @Test func symlinkedRemoteObjectNeverStartsStream() async throws { + #if os(macOS) + let recorder = OpenSSHCommandRecorder(unsafeInspect: true) + let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") + try FileManager.default.createDirectory(at: knownHosts.deletingLastPathComponent(), withIntermediateDirectories: true) + try Data("backup.example ssh-ed25519 AAAA\n".utf8).write(to: knownHosts) + defer { try? FileManager.default.removeItem(at: knownHosts.deletingLastPathComponent()) } + let provider = SFTPRemoteBackupProvider( + profile: try fixtureProfile(), credential: .sshAgent, + knownHostsURL: knownHosts, run: { command in await recorder.run(command) } + ) + let temporary = try RemoteRelativePath(components: ["Jobs", ".bitmatch-upload-11111111-2222-3333-4444-555555555555"]) + + await #expect(throws: RemoteBackupError.unsafePath) { + try await provider.upload(local: URL(fileURLWithPath: "/tmp/local.mov"), toTemporary: temporary, fromOffset: 0, expectedSHA256: String(repeating: "a", count: 64)) { _ in } + } + #expect(!(await recorder.commands()).contains(where: { $0.standardInputFile != nil })) #endif } - @Test func discardTemporaryAcceptsOnlyItemOwnedUUIDPath() async throws { + @Test func discardTemporaryRefusesPathnameOnlyDeletion() async throws { #if os(macOS) let recorder = OpenSSHCommandRecorder() let knownHosts = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString).appendingPathComponent("known_hosts") @@ -113,12 +233,12 @@ struct SFTPRemoteBackupProviderContractTests { components: ["Jobs", ".bitmatch-upload-\(itemID.uuidString.lowercased())"] ) - try await provider.discardTemporary(temporary) + await #expect(throws: RemoteBackupError.temporaryCleanupFailed) { + try await provider.discardTemporary(temporary) + } let commands = await recorder.commands() - #expect(commands.count == 1) - #expect(commands[0].executable == "/usr/bin/ssh") - #expect(commands[0].arguments.last == "rm -f 'Backups/Jobs/.bitmatch-upload-11111111-2222-3333-4444-555555555555'") + #expect(commands.isEmpty) #endif } @@ -296,8 +416,16 @@ private func fixtureProfile() throws -> RemoteDestinationProfile { private actor OpenSSHCommandRecorder { private var recorded: [OpenSSHCommand] = [] private let inspectLength: Int64? - - init(inspectLength: Int64? = nil) { self.inspectLength = inspectLength } + private let claimFails: Bool + private let unsafeInspect: Bool + private let missingPython: Bool + + init(inspectLength: Int64? = nil, claimFails: Bool = false, unsafeInspect: Bool = false, missingPython: Bool = false) { + self.inspectLength = inspectLength + self.claimFails = claimFails + self.unsafeInspect = unsafeInspect + self.missingPython = missingPython + } func run(_ command: OpenSSHCommand) -> OpenSSHCommandResult { recorded.append(command) @@ -307,8 +435,25 @@ private actor OpenSSHCommandRecorder { case "/usr/bin/ssh-keygen": return .init(status: 0, stdout: Data("256 SHA256:test backup.example\n".utf8), stderr: Data()) case "/usr/bin/ssh": - let output = inspectLength.map { Data("\($0)\n".utf8) } ?? Data() + let remoteCommand = command.arguments.last ?? "" + if missingPython, remoteCommand.hasSuffix(" 'check'") { + return .init(status: 127, stdout: Data(), stderr: Data("python3: command not found".utf8)) + } + if remoteCommand.hasSuffix(" 'check'") { + return .init(status: 0, stdout: Data("__BITMATCH_HELPER_READY__\n".utf8), stderr: Data()) + } + if command.standardInputFile != nil, claimFails { + return .init(status: 42, stdout: Data(), stderr: Data("cannot overwrite existing file".utf8)) + } + if remoteCommand.contains("'inspect'"), unsafeInspect { + return .init(status: 43, stdout: Data(), stderr: Data("__BITMATCH_UNSAFE_PATH__".utf8)) + } + let output = remoteCommand.contains("'inspect'") + ? (inspectLength.map { Data("\($0)\n".utf8) } ?? Data()) + : Data() return .init(status: 0, stdout: output, stderr: Data()) + case "/usr/bin/sftp": + return .init(status: 0, stdout: Data(), stderr: Data()) default: return .init(status: 1, stdout: Data(), stderr: Data("unexpected command".utf8)) } @@ -320,26 +465,25 @@ private actor OpenSSHCommandRecorder { private actor SFTPDownloadRecorder { private let bytes: Data private var recorded: [OpenSSHCommand] = [] - private var path: String? + private var outputPath: String? init(bytes: Data) { self.bytes = bytes } func run(_ command: OpenSSHCommand) throws -> OpenSSHCommandResult { recorded.append(command) - guard command.executable == "/usr/bin/sftp", - let batch = command.standardInput.flatMap({ String(data: $0, encoding: .utf8) }) else { - throw RemoteBackupError.providerUnavailable - } - let fields = batch.components(separatedBy: "\"") - guard fields.count >= 4, fields[0] == "get ", fields[1] == "Backups/Jobs/clip.mov" else { + guard command.executable == "/usr/bin/ssh", + command.arguments.last?.contains("'download'") == true, + command.standardInput == nil, + command.standardInputFile == nil, + let outputURL = command.standardOutputFile else { throw RemoteBackupError.providerUnavailable } - path = fields[3] - try bytes.write(to: URL(fileURLWithPath: fields[3])) + outputPath = outputURL.path + try bytes.write(to: outputURL) return .init(status: 0, stdout: Data(), stderr: Data()) } func commands() -> [OpenSSHCommand] { recorded } - func downloadedPath() -> String? { path } + func downloadedPath() -> String? { outputPath } } #endif diff --git a/Scripts/test-local-sftp.sh b/Scripts/test-local-sftp.sh new file mode 100755 index 00000000..1669cd5e --- /dev/null +++ b/Scripts/test-local-sftp.sh @@ -0,0 +1,212 @@ +#!/usr/bin/env bash +set -euo pipefail + +# A real OpenSSH round trip with disposable keys, agent, server, and files. +# Nothing is written to ~/.ssh or a saved BitMatch destination. +ROOT=$(cd "$(dirname "$0")/.." && pwd) +SCRATCH=$(mktemp -d /private/tmp/bitmatch-sftp-smoke.XXXXXX) +SSHD_PID= +AGENT_PID= +RACE_PID= +cleanup() { + [[ -z "$RACE_PID" ]] || { kill "$RACE_PID" 2>/dev/null || true; wait "$RACE_PID" 2>/dev/null || true; } + [[ -z "$AGENT_PID" ]] || { kill "$AGENT_PID" 2>/dev/null || true; wait "$AGENT_PID" 2>/dev/null || true; } + [[ -z "$SSHD_PID" ]] || { kill "$SSHD_PID" 2>/dev/null || true; wait "$SSHD_PID" 2>/dev/null || true; } + if [[ "$SCRATCH" == /private/tmp/bitmatch-sftp-smoke.* ]]; then + rm -rf -- "$SCRATCH" + fi +} +trap cleanup EXIT + +chmod 700 "$SCRATCH" +mkdir "$SCRATCH/remote" +touch "$SCRATCH/remote/.bitmatch-disposable-sftp-server" +ssh-keygen -q -t ed25519 -N '' -f "$SCRATCH/host_key" +ssh-keygen -q -t ed25519 -N '' -f "$SCRATCH/client_key" +cp "$SCRATCH/client_key.pub" "$SCRATCH/authorized_keys" +chmod 600 "$SCRATCH/authorized_keys" + +PORT=$(python3 -c 'import socket; s=socket.socket(); s.bind(("127.0.0.1", 0)); print(s.getsockname()[1]); s.close()') +read -r KEY_TYPE KEY_DATA _ < "$SCRATCH/host_key.pub" +printf '[127.0.0.1]:%s %s %s\n' "$PORT" "$KEY_TYPE" "$KEY_DATA" > "$SCRATCH/known_hosts" + +cat > "$SCRATCH/force-command.sh" < "$SCRATCH/sshd_config" < "$SCRATCH/sshd.log" 2>&1 & +SSHD_PID=$! + +READY=0 +for _ in {1..30}; do + if /usr/bin/ssh -i "$SCRATCH/client_key" -o IdentitiesOnly=yes -o BatchMode=yes \ + -o StrictHostKeyChecking=yes -o UserKnownHostsFile="$SCRATCH/known_hosts" \ + -o GlobalKnownHostsFile=/dev/null -o PasswordAuthentication=no \ + -o KbdInteractiveAuthentication=no -p "$PORT" -l "$(id -un)" \ + -T -- 127.0.0.1 pwd > /dev/null 2>&1; then + READY=1 + break + fi + sleep 0.1 +done +if [[ "$READY" != 1 ]]; then + cat "$SCRATCH/sshd.log" >&2 + echo "Disposable localhost sshd did not start" >&2 + exit 1 +fi + +/usr/bin/ssh-agent -D -a "$SCRATCH/agent.sock" > "$SCRATCH/agent.log" 2>&1 & +AGENT_PID=$! +for _ in {1..30}; do + [[ ! -S "$SCRATCH/agent.sock" ]] || break + sleep 0.1 +done +SSH_AUTH_SOCK="$SCRATCH/agent.sock" /usr/bin/ssh-add "$SCRATCH/client_key" > /dev/null + +# Exercise the exact bundled helper through a disposable localhost SSH server. +# No test writes to the user's SSH configuration or a saved destination. +RACE_SOURCE="$SCRATCH/race-source.bin" +dd if=/dev/urandom of="$RACE_SOURCE" bs=1048576 count=16 status=none +printf 'VICTIM-UNCHANGED' > "$SCRATCH/remote/victim" +cp "$SCRATCH/remote/victim" "$SCRATCH/victim-expected" +remote_helper() { + local remote_command + remote_command=$(python3 -c 'import pathlib,shlex,sys; print("python3 -c " + shlex.quote(pathlib.Path(sys.argv[1]).read_text()) + " " + " ".join(shlex.quote(s) for s in sys.argv[2:]))' "$ROOT/BitMatch/Resources/SFTPRemoteHelper.py" "$@") + /usr/bin/ssh -i "$SCRATCH/client_key" -o IdentitiesOnly=yes -o BatchMode=yes \ + -o StrictHostKeyChecking=yes -o UserKnownHostsFile="$SCRATCH/known_hosts" \ + -o GlobalKnownHostsFile=/dev/null -o PasswordAuthentication=no \ + -o KbdInteractiveAuthentication=no -p "$PORT" -l "$(id -un)" \ + -T -- 127.0.0.1 "$remote_command" +} + +remote_helper check +remote_helper mkdir '["Backups","Jobs"]' +remote_helper upload '["Backups","Jobs","normal"]' < "$RACE_SOURCE" +ln -s /dev/null "$SCRATCH/remote/Backups/Jobs/device-link" +if remote_helper upload '["Backups","Jobs","device-link"]' < "$RACE_SOURCE"; then + echo 'Exclusive upload followed a preexisting symlink to /dev/null' >&2 + exit 1 +fi +ln -s missing-target "$SCRATCH/remote/Backups/Jobs/dangling-link" +if remote_helper upload '["Backups","Jobs","dangling-link"]' < "$RACE_SOURCE"; then + echo 'Exclusive upload followed a dangling symlink' >&2 + exit 1 +fi +[[ ! -e "$SCRATCH/remote/Backups/Jobs/missing-target" ]] +printf 'EXISTING-UNCHANGED' > "$SCRATCH/remote/Backups/Jobs/existing" +if remote_helper upload '["Backups","Jobs","existing"]' < "$RACE_SOURCE"; then + echo 'Exclusive upload replaced an existing regular file' >&2 + exit 1 +fi +[[ "$(cat "$SCRATCH/remote/Backups/Jobs/existing")" == 'EXISTING-UNCHANGED' ]] +remote_helper inspect '["Backups","Jobs","normal"]' >&2 +remote_helper download '["Backups","Jobs","normal"]' > "$SCRATCH/downloaded" +cmp "$RACE_SOURCE" "$SCRATCH/downloaded" +remote_helper inspect '["Backups","Jobs","normal"]' > "$SCRATCH/length" +[[ "$(cat "$SCRATCH/length")" == "$(stat -f %z "$RACE_SOURCE")" ]] +remote_helper promote '["Backups","Jobs","normal"]' '["Backups","Jobs","final"]' +[[ "$(remote_helper same '["Backups","Jobs","normal"]' '["Backups","Jobs","final"]')" == '__BITMATCH_SAME__' ]] +[[ "$(remote_helper same '["Backups","Jobs","existing"]' '["Backups","Jobs","final"]')" == '__BITMATCH_DIFFERENT__' ]] +[[ "$(remote_helper same '["Backups","Jobs","missing"]' '["Backups","Jobs","final"]')" == '__BITMATCH_DIFFERENT__' ]] +cmp "$RACE_SOURCE" "$SCRATCH/remote/Backups/Jobs/final" +cmp "$RACE_SOURCE" "$SCRATCH/remote/Backups/Jobs/normal" + +if remote_helper download '["Backups","Jobs","device-link"]' > "$SCRATCH/unsafe-download"; then + echo 'Readback followed a preexisting symlink to /dev/null' >&2; exit 1 +fi +mkdir "$SCRATCH/helper-outside" +ln -s "$SCRATCH/helper-outside" "$SCRATCH/remote/Backups/linked-parent" +if remote_helper upload '["Backups","linked-parent","escaped"]' < "$RACE_SOURCE"; then + echo 'Helper followed ancestor symlink' >&2; exit 1 +fi +[[ -z "$(ls -A "$SCRATCH/helper-outside")" ]] +ln -s "$SCRATCH/helper-outside" "$SCRATCH/remote/Backups/Jobs/directory-link" +if remote_helper promote '["Backups","Jobs","normal"]' '["Backups","Jobs","directory-link"]'; then + echo 'Helper followed final directory symlink' >&2; exit 1 +fi +[[ -z "$(ls -A "$SCRATCH/helper-outside")" ]] +if remote_helper promote '["Backups","Jobs","normal"]' '["Backups","Jobs","final"]'; then + echo 'Helper replaced an existing final' >&2; exit 1 +fi +cmp "$RACE_SOURCE" "$SCRATCH/remote/Backups/Jobs/final" + +mkfifo "$SCRATCH/helper-upload.fifo" +remote_helper upload '["Backups","Jobs","swapped-upload"]' \ + < "$SCRATCH/helper-upload.fifo" > "$SCRATCH/helper-upload.out" 2> "$SCRATCH/helper-upload.err" & +RACE_PID=$! +exec 9> "$SCRATCH/helper-upload.fifo" +OPENED=0 +for _ in {1..100}; do + if [[ -e "$SCRATCH/remote/Backups/Jobs/swapped-upload" ]]; then OPENED=1; break; fi + sleep 0.05 +done +[[ "$OPENED" == 1 ]] || { echo 'Helper did not open upload FD' >&2; exit 1; } +mv "$SCRATCH/remote/Backups/Jobs/swapped-upload" "$SCRATCH/remote/Backups/Jobs/upload-inode" +ln -s "$SCRATCH/remote/victim" "$SCRATCH/remote/Backups/Jobs/swapped-upload" +cat "$RACE_SOURCE" >&9 +exec 9>&- +wait "$RACE_PID" +RACE_PID= +cmp "$RACE_SOURCE" "$SCRATCH/remote/Backups/Jobs/upload-inode" +cmp "$SCRATCH/victim-expected" "$SCRATCH/remote/victim" + +mkfifo "$SCRATCH/helper-download.fifo" +remote_helper download '["Backups","Jobs","normal"]' \ + > "$SCRATCH/helper-download.fifo" 2> "$SCRATCH/helper-download.err" & +RACE_PID=$! +exec 8< "$SCRATCH/helper-download.fifo" +dd if=/dev/fd/8 of="$SCRATCH/helper-download.bin" bs=4096 count=1 status=none +mv "$SCRATCH/remote/Backups/Jobs/normal" "$SCRATCH/remote/Backups/Jobs/download-inode" +ln -s "$SCRATCH/remote/victim" "$SCRATCH/remote/Backups/Jobs/normal" +cat <&8 >> "$SCRATCH/helper-download.bin" +exec 8<&- +wait "$RACE_PID" +RACE_PID= +cmp "$RACE_SOURCE" "$SCRATCH/helper-download.bin" +cmp "$SCRATCH/victim-expected" "$SCRATCH/remote/victim" +echo 'Pinned-directory remote helper upload, promotion, and readback probes passed.' + +if [[ "${BITMATCH_SFTP_HELPER_ONLY:-0}" == 1 ]]; then + exit 0 +fi + +TEST_RUNNER_BITMATCH_LIVE_SFTP_TEST=1 \ +TEST_RUNNER_BITMATCH_LIVE_SFTP_PORT="$PORT" \ +TEST_RUNNER_BITMATCH_LIVE_SFTP_KNOWN_HOSTS="$SCRATCH/known_hosts" \ +TEST_RUNNER_BITMATCH_LIVE_SFTP_AGENT_SOCK="$SCRATCH/agent.sock" \ +TEST_RUNNER_BITMATCH_LIVE_SFTP_REMOTE_DIR="$SCRATCH/remote" \ +xcodebuild -quiet -project "$ROOT/BitMatch.xcodeproj" \ + -derivedDataPath "$SCRATCH/derived-data" -resultBundlePath "$SCRATCH/live.xcresult" \ + test -scheme BitMatch -destination 'platform=macOS' \ + -only-testing:BitMatchTests/SFTPRemoteBackupLiveTests \ + -parallel-testing-enabled NO CODE_SIGNING_ALLOWED=NO + +xcrun xcresulttool get test-results summary --path "$SCRATCH/live.xcresult" --format json | + python3 -c 'import json, sys; s=json.load(sys.stdin); assert (s["passedTests"], s["failedTests"], s["skippedTests"]) == (1, 0, 0), s; print("Live localhost SFTP test passed (1/1).")' diff --git a/Shared/Core/Models/RemoteBackupModels.swift b/Shared/Core/Models/RemoteBackupModels.swift index 7abdde84..08916757 100644 --- a/Shared/Core/Models/RemoteBackupModels.swift +++ b/Shared/Core/Models/RemoteBackupModels.swift @@ -285,7 +285,14 @@ struct RemoteQueueItem: Identifiable, Codable, Equatable, Sendable { let localArtifactBookmarkReference: String let localArtifactRelativePath: RemoteRelativePath let remoteRelativePath: RemoteRelativePath - let temporaryRemoteRelativePath: RemoteRelativePath + var temporaryRemoteRelativePath: RemoteRelativePath + /// A fresh UUID is persisted before each upload attempt. Older saved + /// items used the item ID as their first temporary-file identity. + var uploadAttemptID: UUID + /// Possible remaining temporary paths: earlier partial uploads and the + /// retained hard link used for no-replace promotion. Keep them for a + /// private operator cleanup note; never remove them by pathname. + var abandonedTemporaryRemoteRelativePaths: [RemoteRelativePath] var state: RemoteBackupState var uploadedByteCount: Int64 var retryCount: Int @@ -304,7 +311,8 @@ struct RemoteQueueItem: Identifiable, Codable, Equatable, Sendable { case id, jobID, cardIngestID, destinationProfileID, manifestID, manifestEntryID, localArtifactBookmarkReference, localArtifactRelativePath, remoteRelativePath, - temporaryRemoteRelativePath, state, uploadedByteCount, retryCount, + temporaryRemoteRelativePath, uploadAttemptID, + abandonedTemporaryRemoteRelativePaths, state, uploadedByteCount, retryCount, nextAttemptAt, verificationEvidence, errorSummary, promotionIntent, createdAt, updatedAt } @@ -320,6 +328,8 @@ struct RemoteQueueItem: Identifiable, Codable, Equatable, Sendable { localArtifactRelativePath: RemoteRelativePath, remoteRelativePath: RemoteRelativePath, temporaryRemoteRelativePath: RemoteRelativePath, + uploadAttemptID: UUID? = nil, + abandonedTemporaryRemoteRelativePaths: [RemoteRelativePath] = [], state: RemoteBackupState, uploadedByteCount: Int64, retryCount: Int, @@ -340,6 +350,8 @@ struct RemoteQueueItem: Identifiable, Codable, Equatable, Sendable { self.localArtifactRelativePath = localArtifactRelativePath self.remoteRelativePath = remoteRelativePath self.temporaryRemoteRelativePath = temporaryRemoteRelativePath + self.uploadAttemptID = uploadAttemptID ?? id + self.abandonedTemporaryRemoteRelativePaths = abandonedTemporaryRemoteRelativePaths self.state = state self.uploadedByteCount = uploadedByteCount self.retryCount = retryCount @@ -364,6 +376,8 @@ struct RemoteQueueItem: Identifiable, Codable, Equatable, Sendable { localArtifactRelativePath: try values.decode(RemoteRelativePath.self, forKey: .localArtifactRelativePath), remoteRelativePath: try values.decode(RemoteRelativePath.self, forKey: .remoteRelativePath), temporaryRemoteRelativePath: try values.decode(RemoteRelativePath.self, forKey: .temporaryRemoteRelativePath), + uploadAttemptID: try values.decodeIfPresent(UUID.self, forKey: .uploadAttemptID), + abandonedTemporaryRemoteRelativePaths: try values.decodeIfPresent([RemoteRelativePath].self, forKey: .abandonedTemporaryRemoteRelativePaths) ?? [], state: try values.decode(RemoteBackupState.self, forKey: .state), uploadedByteCount: try values.decode(Int64.self, forKey: .uploadedByteCount), retryCount: try values.decode(Int.self, forKey: .retryCount), diff --git a/Shared/Core/Services/RemoteBackupProvider.swift b/Shared/Core/Services/RemoteBackupProvider.swift index eae56d2a..54320785 100644 --- a/Shared/Core/Services/RemoteBackupProvider.swift +++ b/Shared/Core/Services/RemoteBackupProvider.swift @@ -42,12 +42,14 @@ struct RemoteObject: Equatable, Sendable { enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { case providerUnavailable + case streamUploadUnavailable case capabilityUnavailable(RemoteProviderRequirement) case missingProfile case missingCredential case invalidDestination case conflict case networkUnavailable + case remoteStorageFull case authenticationFailed case hostKeyMismatch case unsafePath @@ -56,6 +58,7 @@ enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { case permissionDenied case manifestUnavailable case resumeOffsetMismatch(local: Int64, remote: Int64) + case temporaryClaimFailed case temporaryCleanupFailed case verificationFailed case interrupted @@ -82,6 +85,8 @@ enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { switch self { case .providerUnavailable: return "The remote backup provider is unavailable." + case .streamUploadUnavailable: + return "The remote server needs Python 3 with safe filesystem calls for off-site backups." case .capabilityUnavailable(let requirement): return "The remote backup provider lacks required capability: \(requirement.rawValue)." case .missingProfile: @@ -94,6 +99,8 @@ enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { return "A final object already exists at the remote destination." case .networkUnavailable: return "The network connection to the remote backup destination was interrupted." + case .remoteStorageFull: + return "The remote backup destination is out of space or has reached its storage quota. The upload is paused." case .authenticationFailed: return "Authentication to the remote backup destination failed." case .hostKeyMismatch: @@ -110,6 +117,8 @@ enum RemoteBackupError: Error, Equatable, Sendable, LocalizedError { return "The persisted remote backup manifest is unavailable." case .resumeOffsetMismatch(let local, let remote): return "The saved upload offset (\(local)) does not match the remote temporary object (\(remote))." + case .temporaryClaimFailed: + return "The remote temporary upload could not be created without replacing an existing object. Nothing was overwritten." case .temporaryCleanupFailed: return "The remote temporary upload could not be safely discarded; retry when the destination is reachable." case .verificationFailed: @@ -125,16 +134,24 @@ protocol RemoteBackupProvider: Sendable { func preflight(profile: RemoteDestinationProfile, credential: RemoteCredential) async throws -> RemoteProviderCapabilities func inspect(path: RemoteRelativePath) async throws -> RemoteObject? func ensureDirectory(_ path: RemoteRelativePath) async throws - /// Removes only the item-owned temporary object. Implementations must - /// never remove a final destination through this operation. + /// Optional cleanup of an item-owned temporary object. Implementations + /// must refuse if they cannot prove the pathname still names that object; + /// no cleanup may remove a final destination. func discardTemporary(_ path: RemoteRelativePath) async throws func upload( local: URL, toTemporary path: RemoteRelativePath, fromOffset: Int64, + expectedSHA256: String, progress: @Sendable (Int64) async -> Void ) async throws + /// Publish without replacing a final object. Leave the temporary name in + /// place; pathname-only cleanup can remove a different writer's object. func promoteNoReplace(temporary: RemoteRelativePath, final: RemoteRelativePath) async throws + /// Prove the final name and retained temporary name still refer to the + /// same regular remote inode. A saved promotion intent alone is not proof + /// that this app created a preexisting final object after a restart. + func confirmsOwnedFinal(temporary: RemoteRelativePath, final: RemoteRelativePath) async throws -> Bool func verificationEvidence(for path: RemoteRelativePath, expectedSHA256: String) async throws -> RemoteVerificationEvidence func close() async } diff --git a/Shared/Core/Services/RemoteBackupQueue.swift b/Shared/Core/Services/RemoteBackupQueue.swift index 1b5c498d..b02ca15b 100644 --- a/Shared/Core/Services/RemoteBackupQueue.swift +++ b/Shared/Core/Services/RemoteBackupQueue.swift @@ -217,9 +217,12 @@ actor RemoteBackupQueue { cancellingItemIDs.insert(id) do { _ = try await transition(itemID: id, allowingCancellation: true) { item in + let interruptedUpload = item.state == .uploading item.state = .cancelled item.nextAttemptAt = nil - item.errorSummary = RemoteBackupError.cancelled.errorDescription + item.errorSummary = interruptedUpload + ? "The off-site upload was cancelled. A partial file may remain at \(item.temporaryRemoteRelativePath.description) under the selected remote root; BitMatch leaves it untouched." + : RemoteBackupError.cancelled.errorDescription item.promotionIntent = false } runGenerations[id] = nextRunGeneration @@ -279,9 +282,12 @@ actor RemoteBackupQueue { do { _ = try await transition(itemID: id, allowingCancellation: true) { queuedItem in guard !queuedItem.state.isTerminal else { return } + let interruptedUpload = queuedItem.state == .uploading queuedItem.state = .paused queuedItem.nextAttemptAt = nil - queuedItem.errorSummary = nil + queuedItem.errorSummary = interruptedUpload + ? "The off-site upload is paused. A partial file may remain at \(queuedItem.temporaryRemoteRelativePath.description) under the selected remote root. Retry starts this file from zero." + : nil // Keep the promotion marker so a provider-side completion that // races this pause can be finalized safely after retry. queuedItem.promotionIntent = hadPromotionIntent @@ -342,39 +348,36 @@ actor RemoteBackupQueue { return } - let remoteTemporaryObject = try await provider.inspect(path: item.temporaryRemoteRelativePath) - var remoteTemporaryByteCount = remoteTemporaryObject?.byteCount ?? 0 - guard isRunnable(itemID, runGeneration: runGeneration) else { return } - - // `.verifying` is the durable promotion-intent marker. It is written - // before promotion, so a crash after promotion can only resume finalization. if item.state == .verifying { - if let remoteTemporaryObject, - remoteTemporaryObject.byteCount == manifestEntry.byteCount { - try await promoteAndFinalize( - itemID: itemID, - runGeneration: runGeneration, - profile: profile, - manifestEntry: manifestEntry, - provider: provider - ) - return - } else { - try await resetMismatchedTemporary( - itemID: itemID, - runGeneration: runGeneration, - provider: provider - ) - remoteTemporaryByteCount = 0 + // Saving promotion intent does not prove that a temporary file + // still has the bytes we uploaded. Leave that name untouched and + // require an explicit retry with a new exclusive attempt. + _ = try await transition(itemID: itemID) { queuedItem in + queuedItem.state = .paused + queuedItem.promotionIntent = false + queuedItem.nextAttemptAt = nil + queuedItem.errorSummary = "Off-site publication was interrupted before the final file appeared. Review the old temporary file at \(queuedItem.temporaryRemoteRelativePath.description) under the selected remote root; BitMatch leaves it untouched. Retry starts this file from zero with a new temporary name." + if !queuedItem.abandonedTemporaryRemoteRelativePaths.contains(queuedItem.temporaryRemoteRelativePath) { + queuedItem.abandonedTemporaryRemoteRelativePaths.append(queuedItem.temporaryRemoteRelativePath) + } } - } else if item.uploadedByteCount != remoteTemporaryByteCount { - try await resetMismatchedTemporary( - itemID: itemID, - runGeneration: runGeneration, - provider: provider - ) - remoteTemporaryByteCount = 0 + return + } + + // A previous attempt's leaf may have been replaced by another + // object, including a symlink. Never reopen or delete it. Rotate to + // a fresh durable name; ensureDirectory still rejects an unsafe + // ancestor before any upload starts. + let remoteTemporaryObject: RemoteObject? + let previousTemporaryContested: Bool + do { + remoteTemporaryObject = try await provider.inspect(path: item.temporaryRemoteRelativePath) + previousTemporaryContested = false + } catch RemoteBackupError.unsafePath { + remoteTemporaryObject = nil + previousTemporaryContested = true } + guard isRunnable(itemID, runGeneration: runGeneration) else { return } if item.remoteRelativePath.components.count > 1 { let parent = try RemoteRelativePath(components: Array(item.remoteRelativePath.components.dropLast())) @@ -382,25 +385,43 @@ actor RemoteBackupQueue { guard isRunnable(itemID, runGeneration: runGeneration) else { return } } + // Never append to a pathname that might have been replaced since its + // last upload. A new UUID path is durable before the provider opens it. + // Old partial files remain untouched; pathname-only removal cannot + // prove which inode another writer left there. + let newAttemptID = UUID() + let newTemporaryPath = try expectedTemporaryPath( + final: item.remoteRelativePath, + itemID: item.id, + attemptID: newAttemptID + ) guard try await transition(itemID: itemID, { queuedItem in + if (remoteTemporaryObject != nil || previousTemporaryContested), + !queuedItem.abandonedTemporaryRemoteRelativePaths.contains(item.temporaryRemoteRelativePath) { + queuedItem.abandonedTemporaryRemoteRelativePaths.append(item.temporaryRemoteRelativePath) + } + queuedItem.uploadAttemptID = newAttemptID + queuedItem.temporaryRemoteRelativePath = newTemporaryPath queuedItem.state = .uploading - queuedItem.uploadedByteCount = remoteTemporaryByteCount + queuedItem.uploadedByteCount = 0 queuedItem.nextAttemptAt = nil queuedItem.errorSummary = nil + queuedItem.promotionIntent = false }) else { return } try Task.checkCancellation() try await provider.upload( local: localURL, - toTemporary: item.temporaryRemoteRelativePath, - fromOffset: remoteTemporaryByteCount, + toTemporary: newTemporaryPath, + fromOffset: 0, + expectedSHA256: manifestEntry.sha256, progress: { [weak self] byteCount in await self?.recordProgress(itemID: itemID, runGeneration: runGeneration, byteCount: byteCount) } ) guard isRunnable(itemID, runGeneration: runGeneration) else { return } - let completedTemporaryByteCount = try await provider.inspect(path: item.temporaryRemoteRelativePath)?.byteCount ?? 0 + let completedTemporaryByteCount = try await provider.inspect(path: newTemporaryPath)?.byteCount ?? 0 guard isRunnable(itemID, runGeneration: runGeneration) else { return } guard completedTemporaryByteCount == manifestEntry.byteCount else { throw RemoteBackupError.resumeOffsetMismatch( @@ -415,6 +436,9 @@ actor RemoteBackupQueue { queuedItem.nextAttemptAt = nil queuedItem.errorSummary = nil queuedItem.promotionIntent = true + if !queuedItem.abandonedTemporaryRemoteRelativePaths.contains(newTemporaryPath) { + queuedItem.abandonedTemporaryRemoteRelativePaths.append(newTemporaryPath) + } }) else { return } try await promoteAndFinalize( @@ -449,34 +473,6 @@ actor RemoteBackupQueue { ) } - /// A temporary object is disposable recovery state. When its observed - /// length disagrees with the durable queue offset, delete that object and - /// restart from zero rather than appending to bytes whose provenance is - /// unknown. This operation is scoped to the item-owned temporary path and - /// can never replace or delete the final object. - private func resetMismatchedTemporary( - itemID: UUID, - runGeneration: UInt64, - provider: any RemoteBackupProvider - ) async throws { - guard let item = items[itemID], isRunnable(itemID, runGeneration: runGeneration) else { return } - do { - try await provider.discardTemporary(item.temporaryRemoteRelativePath) - } catch { - // Do not retry or append to an object whose ownership/offset could - // not be reset safely. Leave the item paused for explicit retry. - throw RemoteBackupError.temporaryCleanupFailed - } - guard isRunnable(itemID, runGeneration: runGeneration) else { return } - _ = try await transition(itemID: itemID) { queuedItem in - queuedItem.state = .queued - queuedItem.uploadedByteCount = 0 - queuedItem.nextAttemptAt = nil - queuedItem.errorSummary = nil - queuedItem.promotionIntent = false - } - } - private func finalizeOwnedFinal( itemID: UUID, runGeneration: UInt64, @@ -485,7 +481,34 @@ actor RemoteBackupQueue { provider: any RemoteBackupProvider ) async throws { guard isRunnable(itemID, runGeneration: runGeneration) else { return } + guard let item = items[itemID] else { return } + guard try await provider.confirmsOwnedFinal( + temporary: item.temporaryRemoteRelativePath, + final: item.remoteRelativePath + ) else { + throw RemoteBackupError.conflict + } + guard isRunnable(itemID, runGeneration: runGeneration) else { return } + // Older saved queues may resume after the final link was created but + // before the retained temporary name was recorded. Save the possible + // cleanup path before accepting any terminal result. + if let item = items[itemID], + !item.abandonedTemporaryRemoteRelativePaths.contains(item.temporaryRemoteRelativePath) { + guard try await transition(itemID: itemID, { queuedItem in + queuedItem.abandonedTemporaryRemoteRelativePaths.append(item.temporaryRemoteRelativePath) + }) else { return } + } guard profile.verificationMode == .sha256 else { + // Upload-only has no digest, but it still must not turn a raced + // symlink/non-file or wrong-length final object into completion. + guard let finalPath = items[itemID]?.remoteRelativePath else { + throw RemoteBackupError.manifestUnavailable + } + let finalObject = try await provider.inspect(path: finalPath) + guard isRunnable(itemID, runGeneration: runGeneration) else { return } + guard finalObject?.byteCount == manifestEntry.byteCount else { + throw RemoteBackupError.verificationFailed + } _ = try await transition(itemID: itemID) { queuedItem in queuedItem.state = .uploadedUnverified queuedItem.uploadedByteCount = manifestEntry.byteCount @@ -538,7 +561,8 @@ actor RemoteBackupQueue { item.temporaryRemoteRelativePath != item.remoteRelativePath, item.temporaryRemoteRelativePath == (try expectedTemporaryPath( final: item.remoteRelativePath, - itemID: item.id + itemID: item.id, + attemptID: item.uploadAttemptID )), item.localArtifactBookmarkReference.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == false, entry.byteCount >= 0, @@ -560,10 +584,17 @@ actor RemoteBackupQueue { private func expectedTemporaryPath( final: RemoteRelativePath, - itemID: UUID + itemID: UUID, + attemptID: UUID ) throws -> RemoteRelativePath { let parent = try RemoteRelativePath(components: Array(final.components.dropLast())) - return try parent.appending(".bitmatch-upload-\(itemID.uuidString.lowercased())") + let itemName = itemID.uuidString.lowercased() + // The original item-ID-only name remains valid for saved queues, but + // all new names bind both the immutable queue item and its attempt. + let name = attemptID == itemID + ? ".bitmatch-upload-\(itemName)" + : ".bitmatch-upload-\(itemName)-\(attemptID.uuidString.lowercased())" + return try parent.appending(name) } private func recordProgress(itemID: UUID, runGeneration: UInt64, byteCount: Int64) async { @@ -597,6 +628,10 @@ actor RemoteBackupQueue { } else { backupError = .providerUnavailable } + let explanation = (backupError.errorDescription ?? "The remote upload stopped.") + + (item.state == .uploading + ? " A retry starts this file from zero with a new temporary name. Check \(item.temporaryRemoteRelativePath.description) under the selected remote root for a possible partial file; BitMatch leaves it untouched." + : "") do { if backupError.isTransientNetworkFault && !Task.isCancelled { @@ -606,7 +641,7 @@ actor RemoteBackupQueue { queuedItem.state = .failed queuedItem.retryCount = retryCount queuedItem.nextAttemptAt = nil - queuedItem.errorSummary = "\(backupError.errorDescription ?? "The network connection failed.") Gave up after \(Self.maxRetryCount) attempts; retry manually when the destination is reachable." + queuedItem.errorSummary = "\(explanation) Gave up after \(Self.maxRetryCount) attempts; retry manually when the destination is reachable." // Keep this marker across manual retry if promotion // may already have happened remotely. queuedItem.promotionIntent = queuedItem.promotionIntent || item.state == .verifying @@ -617,7 +652,7 @@ actor RemoteBackupQueue { queuedItem.state = item.state == .verifying ? .verifying : .retrying queuedItem.retryCount = retryCount queuedItem.nextAttemptAt = now().addingTimeInterval(delay) - queuedItem.errorSummary = backupError.errorDescription + queuedItem.errorSummary = explanation } } } else { @@ -635,7 +670,7 @@ actor RemoteBackupQueue { queuedItem.state = .paused } queuedItem.nextAttemptAt = nil - queuedItem.errorSummary = backupError.errorDescription + queuedItem.errorSummary = explanation } } } catch { diff --git a/Shared/Core/ViewModels/PhotographerJobViewModel.swift b/Shared/Core/ViewModels/PhotographerJobViewModel.swift index 54205314..3fb21304 100644 --- a/Shared/Core/ViewModels/PhotographerJobViewModel.swift +++ b/Shared/Core/ViewModels/PhotographerJobViewModel.swift @@ -851,6 +851,13 @@ final class PhotographerJobViewModel: ObservableObject { } else { verificationEvidence = .none } + let abandonedPaths = orderedItems.flatMap(\.abandonedTemporaryRemoteRelativePaths) + let abandonedNote: String? = abandonedPaths.isEmpty ? nil : + "Remote temporary names may remain under the selected root: \(abandonedPaths.prefix(5).map(\.description).joined(separator: ", "))\(abandonedPaths.count > 5 ? " (and \(abandonedPaths.count - 5) more)" : ""). Completed uploads use hard links, not extra copies; interrupted uploads restart from zero. Review these names before removing them manually." + let stateError = orderedItems.compactMap(\.errorSummary).first + ?? (hasMismatchedDigest ? "Remote verification evidence does not match the saved manifest." : nil) + ?? (hasCompleteManifestCoverage ? nil : "Off-site file evidence is incomplete or inconsistent. Queue a new backup.") + let visibleNote = [stateError, abandonedNote].compactMap { $0 }.joined(separator: " ") return RemoteBackupCardSummary( targetID: targetID, state: state, @@ -865,9 +872,7 @@ final class PhotographerJobViewModel: ObservableObject { uploadedByteCount: totalBytes, verificationEvidence: verificationEvidence, remotePath: orderedItems.first?.remoteRelativePath, - errorSummary: orderedItems.compactMap(\.errorSummary).first - ?? (hasMismatchedDigest ? "Remote verification evidence does not match the saved manifest." : nil) - ?? (hasCompleteManifestCoverage ? nil : "Off-site file evidence is incomplete or inconsistent. Queue a new backup."), + errorSummary: visibleNote.isEmpty ? nil : visibleNote, // Refreshing saved queue items (including on launch) is not a new // verification event. Keep the durable item's latest event time. updatedAt: orderedItems.map(\.updatedAt).max() diff --git a/docs/GUIDE.md b/docs/GUIDE.md index 97ba5e16..d3fbb7c3 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -83,9 +83,11 @@ Have a server to back up to? Save an SFTP destination for the job and queue an u A few things to know: - Authentication uses your SSH agent. You'll confirm the host key when connecting to a new host. -- The server needs SFTP **and SSH shell access**, with standard file utilities and hard-link support. An SFTP-only account won't do. +- The server needs SSH shell access and Python 3 with `openat`/`linkat` support; its destination filesystem must support hard links. An SFTP-only account won't do. The configured remote root is relative to the SSH login directory. BitMatch checks that the helper can run before uploading; if the destination cannot make a hard link, publication stops with an issue and leaves the temporary file for review. - **SHA-256 read-back** downloads the remote file to check it. That costs extra bandwidth and local temporary space. - **Upload only** means **Uploaded · Unverified**. Getting it onto the server isn't proof that the contents match. +- If an upload stops partway through, the next attempt starts that file from zero under a new hidden temporary name. BitMatch leaves the earlier partial file untouched and shows its remote path for you to review and remove manually. It never appends to an old partial file. +- After a completed SFTP upload, the hidden temporary name remains as a hard link to the final file. This uses no extra data space, and BitMatch shows the name for optional manual cleanup. It does not remove remote paths automatically because another writer could replace them between checking and deletion. - **Uploaded · Downloaded and checked** means BitMatch downloaded the remote file and confirmed it matches the verified local backup. Older server-checksum evidence says **Remote checksum verified**. Neither label establishes a new source-card reread or a whole-card safe-to-erase verdict; the local transfer retains its own verification mode, coverage and outcome. - iPad can keep the project's remote settings, but uploads run on Mac. S3 and WebDAV aren't available yet.