diff --git a/bitchat/Services/BLE/BLEPrivateMediaReceiptStore.swift b/bitchat/Services/BLE/BLEPrivateMediaReceiptStore.swift index bbbc492c..60e70fea 100644 --- a/bitchat/Services/BLE/BLEPrivateMediaReceiptStore.swift +++ b/bitchat/Services/BLE/BLEPrivateMediaReceiptStore.swift @@ -17,13 +17,18 @@ enum BLEPrivateMediaReceiptState: Equatable { /// /// Each ID has its own atomic record so one hot lookup never rewrites or /// decodes the entire ledger. The process-lifetime index is installed only -/// after a complete, successful directory scan. An enumeration, read, decode, -/// or structural-validation failure therefore remains retryable and cannot be -/// mistaken for an empty ledger. +/// after a complete directory scan. A structural failure of the directory +/// itself (create/enumerate) remains globally fail-closed and retryable. +/// An individual record that cannot be read, decoded, or validated is +/// quarantined instead: the file is moved aside with a `.corrupt` suffix, +/// excluded from future scans, and only that ID stays fail-closed — the rest +/// of the ledger keeps working, so one damaged record can never make every +/// inbound private media payload vanish. final class BLEPrivateMediaReceiptStore: @unchecked Sendable { typealias DirectoryReader = (_ directory: URL) throws -> [URL] typealias DataReader = (_ url: URL) throws -> Data private static let receiptDirectoryName = ".private-media-receipts" + private static let quarantinePathExtension = "corrupt" private struct ReceiptRecord: Codable, Equatable { enum Kind: String, Codable { @@ -41,6 +46,10 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { private final class Runtime: @unchecked Sendable { let lock = NSLock() var records: [String: ReceiptRecord]? + /// IDs whose durable record was quarantined as unreadable. Installed + /// together with `records`; these IDs stay fail-closed while every + /// other record keeps serving. + var quarantined: Set = [] var volatileTombstones: [String: Date] = [:] } @@ -78,6 +87,7 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { func resetForPanic() { runtime.lock.lock() runtime.records = nil + runtime.quarantined.removeAll(keepingCapacity: false) runtime.volatileTombstones.removeAll(keepingCapacity: false) runtime.lock.unlock() } @@ -102,6 +112,12 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { var records = loadIndexIfNeeded(from: directory, at: date) else { return .unavailable } + // A quarantined record could have been an acceptance or a tombstone; + // only this ID fails closed, so a retry can neither resurrect deleted + // media nor double-deliver, while every other payload keeps working. + if runtime.quarantined.contains(messageID) { + return .unavailable + } guard let record = records[messageID] else { return .absent } if isExpired(record.recordedAt, at: date) { @@ -150,7 +166,8 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { } guard let directory = resolvedReceiptDirectory(), - var records = loadIndexIfNeeded(from: directory, at: date) else { + var records = loadIndexIfNeeded(from: directory, at: date), + !runtime.quarantined.contains(messageID) else { return false } if let existing = records[messageID], @@ -202,7 +219,8 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { addVolatileTombstone(messageID, at: date) guard let directory = resolvedReceiptDirectory(), - var records = loadIndexIfNeeded(from: directory, at: date) else { + var records = loadIndexIfNeeded(from: directory, at: date), + !runtime.quarantined.contains(messageID) else { runtime.volatileTombstones.removeValue(forKey: messageID) return false } @@ -291,9 +309,22 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { } var records: [String: ReceiptRecord] = [:] + var quarantined: Set = [] var expired: [String] = [] var tombstones: [ReceiptRecord] = [] for url in urls { + // Records quarantined by an earlier scan stay fail-closed on + // every launch without being re-read: only their ID matters. + if url.pathExtension == Self.quarantinePathExtension { + let messageID = url + .deletingPathExtension() + .deletingPathExtension() + .lastPathComponent + if PrivateMediaMessageIdentity.isStableID(messageID) { + quarantined.insert(messageID) + } + continue + } guard url.pathExtension == "json" else { continue } let messageID = url.deletingPathExtension().lastPathComponent guard PrivateMediaMessageIdentity.isStableID(messageID) else { @@ -305,21 +336,23 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { let data = try dataReader?(url) ?? Data(contentsOf: url) record = try JSONDecoder().decode(ReceiptRecord.self, from: data) } catch { - // Never delete or skip an unreadable stable-ID record. Treating - // it as absent could resurrect accepted or deleted media. - SecureLogger.error( - "❌ Failed to read private-media receipt \(messageID.prefix(12))…: \(error)", - category: .session - ) - return nil + // Never delete or silently skip an unreadable stable-ID + // record: treating it as absent could resurrect accepted or + // deleted media. But never let it poison the whole ledger + // either — quarantine the file and fail only this ID closed. + quarantine(url, messageID: messageID, reason: "\(error)") + quarantined.insert(messageID) + continue } guard isStructurallyValid(record) else { - SecureLogger.error( - "❌ Invalid private-media receipt \(messageID.prefix(12))…", - category: .session + quarantine( + url, + messageID: messageID, + reason: "structurally invalid" ) - return nil + quarantined.insert(messageID) + continue } if isExpired(record.recordedAt, at: date) { expired.append(messageID) @@ -331,14 +364,21 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { } } + // A readable duplicate of a quarantined ID must not override the + // fail-closed decision. + for messageID in quarantined { + records.removeValue(forKey: messageID) + } + let overflow = overflowVictims(in: records) for messageID in overflow { records.removeValue(forKey: messageID) } - // Install the index only after every stable-ID record was read and - // validated successfully. Cleanup cannot influence a failed scan. + // Install the index only after the whole directory was scanned. + // Cleanup cannot influence a failed scan. runtime.records = records + runtime.quarantined = quarantined for messageID in expired + overflow { removeRecord(messageID: messageID, from: directory) @@ -349,6 +389,34 @@ final class BLEPrivateMediaReceiptStore: @unchecked Sendable { return records } + /// Moves an unreadable record aside so future scans skip it while its ID + /// stays fail-closed. The bytes are preserved for offline inspection — + /// quarantine never deletes receiver decisions. + private func quarantine(_ url: URL, messageID: String, reason: String) { + SecureLogger.error( + "❌ Quarantining unreadable private-media receipt \(messageID.prefix(12))…: \(reason)", + category: .session + ) + let destination = url.appendingPathExtension( + Self.quarantinePathExtension + ) + do { + if fileManager.fileExists(atPath: destination.path) { + // Same ID, already fail-closed; keep the earlier evidence. + try fileManager.removeItem(at: url) + } else { + try fileManager.moveItem(at: url, to: destination) + } + } catch { + // The record stays where it is and will be re-quarantined (in + // memory at minimum) by the next scan. Still fail-closed. + SecureLogger.warning( + "⚠️ Failed to move corrupt private-media receipt aside: \(error)", + category: .session + ) + } + } + private func isStructurallyValid(_ record: ReceiptRecord) -> Bool { switch record.kind { case .tombstone: diff --git a/bitchatTests/Services/BLEPrivateMediaReceiptStoreTests.swift b/bitchatTests/Services/BLEPrivateMediaReceiptStoreTests.swift index 9efa71ce..1fd564c5 100644 --- a/bitchatTests/Services/BLEPrivateMediaReceiptStoreTests.swift +++ b/bitchatTests/Services/BLEPrivateMediaReceiptStoreTests.swift @@ -54,7 +54,7 @@ struct BLEPrivateMediaReceiptStoreTests { } @Test - func recordReadFailureIsUnavailableAndPreservesReceiptForRetry() throws { + func recordReadFailureQuarantinesOnlyThatRecord() throws { let root = makeRoot("read-failure") defer { try? FileManager.default.removeItem(at: root) } let payload = try makePayload(in: root) @@ -63,13 +63,12 @@ struct BLEPrivateMediaReceiptStoreTests { storedURL: payload )) let record = receiptRecord(in: root) + let durableBytes = try Data(contentsOf: record) - var shouldFail = true let store = BLEPrivateMediaReceiptStore( baseDirectory: root, dataReader: { url in - if shouldFail { - shouldFail = false + if url.lastPathComponent.hasPrefix(self.messageID) { throw TestError() } return try Data(contentsOf: url) @@ -77,12 +76,17 @@ struct BLEPrivateMediaReceiptStoreTests { ) #expect(store.state(for: messageID) == .unavailable) - #expect(FileManager.default.fileExists(atPath: record.path)) - #expect(store.state(for: messageID) == .accepted(payload)) + // The record was moved aside, bytes intact, not deleted. + #expect(!FileManager.default.fileExists(atPath: record.path)) + let quarantined = quarantinedRecord(in: root) + #expect(FileManager.default.fileExists(atPath: quarantined.path)) + #expect(try Data(contentsOf: quarantined) == durableBytes) + // The quarantine is sticky for this ID; no absent/accepted flapping. + #expect(store.state(for: messageID) == .unavailable) } @Test - func decodeFailureIsUnavailableAndDoesNotDeleteOrCachePastRepair() throws { + func decodeFailureQuarantinesRecordWithoutRetryOrDeletion() throws { let root = makeRoot("decode-failure") defer { try? FileManager.default.removeItem(at: root) } let payload = try makePayload(in: root) @@ -91,18 +95,74 @@ struct BLEPrivateMediaReceiptStoreTests { storedURL: payload )) let record = receiptRecord(in: root) - let durableBytes = try Data(contentsOf: record) let corruptBytes = Data("{not-json".utf8) try corruptBytes.write(to: record, options: .atomic) let store = BLEPrivateMediaReceiptStore(baseDirectory: root) + // Only this ID fails closed; it cannot be re-recorded past the + // quarantine either. #expect(store.state(for: messageID) == .unavailable) #expect(!store.commitAccepted(messageID: messageID, storedURL: payload)) - #expect(FileManager.default.fileExists(atPath: record.path)) - #expect(try Data(contentsOf: record) == corruptBytes) + #expect(!store.recordDeleted(messageID: messageID)) + #expect(store.state(for: messageID) == .unavailable) - try durableBytes.write(to: record, options: .atomic) - #expect(store.state(for: messageID) == .accepted(payload)) + // The unreadable bytes were preserved at the quarantine name. + let quarantined = quarantinedRecord(in: root) + #expect(!FileManager.default.fileExists(atPath: record.path)) + #expect(FileManager.default.fileExists(atPath: quarantined.path)) + #expect(try Data(contentsOf: quarantined) == corruptBytes) + + // A relaunch stays fail-closed for this ID without ever re-reading + // the quarantined file. + let readURLs = ReadTracker() + let relaunched = BLEPrivateMediaReceiptStore( + baseDirectory: root, + dataReader: { url in + readURLs.append(url) + return try Data(contentsOf: url) + } + ) + #expect(relaunched.state(for: messageID) == .unavailable) + #expect(!readURLs.urls.contains { + $0.lastPathComponent == quarantined.lastPathComponent + }) + } + + @Test + func corruptRecordDoesNotBlockAnotherSendersMedia() throws { + let root = makeRoot("quarantine-isolation") + defer { try? FileManager.default.removeItem(at: root) } + let otherMessageID = "media-ffeeddccbbaa99887766554433221100" + let corruptPayload = try makePayload(in: root, name: "corrupt.jpg") + let healthyPayload = try makePayload(in: root, name: "healthy.jpg") + + let seed = BLEPrivateMediaReceiptStore(baseDirectory: root) + #expect(seed.commitAccepted( + messageID: messageID, + storedURL: corruptPayload + )) + #expect(seed.commitAccepted( + messageID: otherMessageID, + storedURL: healthyPayload + )) + try Data("{not-json".utf8).write( + to: receiptRecord(in: root), + options: .atomic + ) + + let store = BLEPrivateMediaReceiptStore(baseDirectory: root) + // Only the damaged record fails closed; the other sender's ledger + // entry keeps serving and new decisions still commit durably. + #expect(store.state(for: messageID) == .unavailable) + #expect(store.state(for: otherMessageID) == .accepted(healthyPayload)) + + let freshMessageID = "media-0102030405060708090a0b0c0d0e0f10" + let freshPayload = try makePayload(in: root, name: "fresh.jpg") + #expect(store.commitAccepted( + messageID: freshMessageID, + storedURL: freshPayload + )) + #expect(store.state(for: freshMessageID) == .accepted(freshPayload)) } @Test @@ -116,15 +176,26 @@ struct BLEPrivateMediaReceiptStoreTests { #expect(!FileManager.default.fileExists(atPath: payload.path)) let record = receiptRecord(in: root) - let durableBytes = try Data(contentsOf: record) - try Data([0xFF, 0x00, 0x7B]).write(to: record, options: .atomic) + let corruptBytes = Data([0xFF, 0x00, 0x7B]) + try corruptBytes.write(to: record, options: .atomic) let relaunched = BLEPrivateMediaReceiptStore(baseDirectory: root) #expect(relaunched.state(for: messageID) == .unavailable) - #expect(FileManager.default.fileExists(atPath: record.path)) - - try durableBytes.write(to: record, options: .atomic) - #expect(relaunched.state(for: messageID) == .tombstoned) + // The quarantined tombstone can never flip to absent — a sender retry + // must not resurrect explicitly deleted media even when the payload + // bytes arrive again. + try Data([0xFF, 0xD8, 0xFF, 0xD9]).write(to: payload) + #expect(!relaunched.commitAccepted( + messageID: messageID, + storedURL: payload + )) + let quarantined = quarantinedRecord(in: root) + #expect(FileManager.default.fileExists(atPath: quarantined.path)) + #expect(try Data(contentsOf: quarantined) == corruptBytes) + #expect( + BLEPrivateMediaReceiptStore(baseDirectory: root) + .state(for: messageID) == .unavailable + ) } @Test @@ -180,7 +251,10 @@ struct BLEPrivateMediaReceiptStoreTests { ) } - private func makePayload(in root: URL) throws -> URL { + private func makePayload( + in root: URL, + name: String = "image.jpg" + ) throws -> URL { let directory = root.appendingPathComponent( "files/images/incoming", isDirectory: true @@ -189,7 +263,7 @@ struct BLEPrivateMediaReceiptStoreTests { at: directory, withIntermediateDirectories: true ) - let payload = directory.appendingPathComponent("image.jpg") + let payload = directory.appendingPathComponent(name) try Data([0xFF, 0xD8, 0xFF, 0xD9]).write(to: payload) return payload } @@ -203,4 +277,15 @@ struct BLEPrivateMediaReceiptStoreTests { .appendingPathComponent(messageID) .appendingPathExtension("json") } + + private func quarantinedRecord(in root: URL) -> URL { + receiptRecord(in: root).appendingPathExtension("corrupt") + } +} + +/// Collects the URLs a store's data reader touched. A reference type so the +/// `@Sendable`-shaped reader closure can record without mutating captures. +private final class ReadTracker: @unchecked Sendable { + private(set) var urls: [URL] = [] + func append(_ url: URL) { urls.append(url) } }