Quarantine corrupt private-media receipts per record

A single unreadable or undecodable receipt record used to abort the
whole index install, leaving every lookup .unavailable and silently
dropping ALL inbound private media with no recovery path. Move the
damaged file aside to a .corrupt name (skipped by future scans, bytes
preserved), fail only that ID closed, and let the rest of the ledger
load. Directory-level create/enumerate failures stay globally
fail-closed and retryable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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,
+/// after a complete directory scan. A structural failure of the directory
-/// or structural-validation failure therefore remains retryable and cannot be
+/// itself (create/enumerate) remains globally fail-closed and retryable.
-/// mistaken for an empty ledger.
+/// 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<String> = []
         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<String> = []
         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
+                // Never delete or silently skip an unreadable stable-ID
-                // it as absent could resurrect accepted or deleted media.
+                // record: treating it as absent could resurrect accepted or
-                SecureLogger.error(
+                // deleted media. But never let it poison the whole ledger
-                    "❌ Failed to read private-media receipt \(messageID.prefix(12))…: \(error)",
+                // either — quarantine the file and fail only this ID closed.
-                    category: .session
+                quarantine(url, messageID: messageID, reason: "\(error)")
-                )
+                quarantined.insert(messageID)
-                return nil
+                continue
             }
 
             guard isStructurallyValid(record) else {
-                SecureLogger.error(
+                quarantine(
-                    "❌ Invalid private-media receipt \(messageID.prefix(12))…",
+                    url,
-                    category: .session
+                    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
+        // Install the index only after the whole directory was scanned.
-        // validated successfully. Cleanup cannot influence a failed scan.
+        // 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:

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 {
+                if url.lastPathComponent.hasPrefix(self.messageID) {
-                    shouldFail = false
                     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))
+        // The record was moved aside, bytes intact, not deleted.
-        #expect(store.state(for: messageID) == .accepted(payload))
+        #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(!store.recordDeleted(messageID: messageID))
-        #expect(try Data(contentsOf: record) == corruptBytes)
+        #expect(store.state(for: messageID) == .unavailable)
 
-        try durableBytes.write(to: record, options: .atomic)
+        // The unreadable bytes were preserved at the quarantine name.
-        #expect(store.state(for: messageID) == .accepted(payload))
+        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)
+        let corruptBytes = Data([0xFF, 0x00, 0x7B])
-        try Data([0xFF, 0x00, 0x7B]).write(to: record, options: .atomic)
+        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))
+        // The quarantined tombstone can never flip to absent — a sender retry
-
+        // must not resurrect explicitly deleted media even when the payload
-        try durableBytes.write(to: record, options: .atomic)
+        // bytes arrive again.
-        #expect(relaunched.state(for: messageID) == .tombstoned)
+        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) }
 }