diff --git a/Sources/ShotdeckCore/Support/TransportSettings.swift b/Sources/ShotdeckCore/Support/TransportSettings.swift index cf907f1..c278d61 100644 --- a/Sources/ShotdeckCore/Support/TransportSettings.swift +++ b/Sources/ShotdeckCore/Support/TransportSettings.swift @@ -189,16 +189,33 @@ public enum OneDriveLocator { fileManager: FileManager = .default ) -> Bool { let probeURL = folder.appendingPathComponent(".redline-probe-\(UUID().uuidString)") + // Belt-and-suspenders cleanup, unconditional: AtomicFile.write renames the temp + // file onto probeURL and THEN fsyncs the containing directory — if that last + // fsync throws, the probe file already exists on disk but the catch below + // returns false before ever reaching the explicit removeItem call. And if the + // explicit removeItem itself throws, this is the only retry it gets. Either + // way, never leave the probe file behind just because we're about to return. + defer { + if fileManager.fileExists(atPath: probeURL.path) { + try? fileManager.removeItem(at: probeURL) + } + } + do { try AtomicFile.write(Data(), to: probeURL) } catch { return false } + do { try fileManager.removeItem(at: probeURL) } catch { return false } - return true + + // Only true when the explicit removal above actually succeeded AND the file is + // confirmed gone — never trust a removeItem call that returned without throwing + // as proof of anything on a File Provider domain. + return !fileManager.fileExists(atPath: probeURL.path) } } diff --git a/Tests/ShotdeckCoreTests/TransportSettingsTests.swift b/Tests/ShotdeckCoreTests/TransportSettingsTests.swift index 835cfef..3eb787f 100644 --- a/Tests/ShotdeckCoreTests/TransportSettingsTests.swift +++ b/Tests/ShotdeckCoreTests/TransportSettingsTests.swift @@ -160,6 +160,49 @@ func probeWritableFalseForAPlainFilePath() throws { #expect(!OneDriveLocator.probeWritable(at: filePath)) } +/// The write itself succeeds (a real probe file lands on disk via AtomicFile.write, +/// which never touches this injected FileManager — it uses raw POSIX calls), but the +/// FIRST call to `removeItem(at:)` throws, simulating a transient File Provider +/// removal failure. probeWritable's own defer-based cleanup must retry and succeed +/// (the second call through this same override falls through to `super`), so no +/// probe file is left behind even though the function correctly still reports false +/// (the removal it explicitly attempted did fail). +private final class ThrowOnceOnRemoveFileManager: FileManager, @unchecked Sendable { + private let lock = NSLock() + private var hasThrown = false + + override func removeItem(at URL: URL) throws { + lock.lock() + let shouldThrow = !hasThrown + hasThrown = true + lock.unlock() + if shouldThrow { + throw NSError(domain: "ShotdeckCoreTests.ThrowOnceOnRemove", code: 1) + } + try super.removeItem(at: URL) + } +} + +@Test +func probeWritableFalseAndLeavesNoProbeFileWhenRemoveItemThrowsOnce() throws { + // Directory fsync failure (the OTHER way probeWritable's cleanup can be needed) has + // no injectable seam: AtomicFile.write's directory fsync is a raw Darwin fsync(2) + // call on an already-open file descriptor, not parameterized by any FileManager or + // other dependency this test can substitute, and there is no portable way to make + // fsync(2) itself fail via chmod or other standard test techniques (fsync failures + // are OS/filesystem/hardware-level events). Covering the removeItem-throws path + // (below) is what this test does; the fsync-throws path is covered by code + // inspection only — the same `defer` block guards both. + let dir = try makeTransportTemporaryDirectory(prefix: "shotdeck-probe-remove-throws") + defer { try? FileManager.default.removeItem(at: dir) } + let injectedFileManager = ThrowOnceOnRemoveFileManager() + + #expect(!OneDriveLocator.probeWritable(at: dir, fileManager: injectedFileManager)) + + let leftovers = try FileManager.default.contentsOfDirectory(atPath: dir.path) + #expect(leftovers.isEmpty) +} + struct TransportDefaultsSuite { let name: String let defaults: UserDefaults