From 7b39d72beca06350f2f9534fb135e3aa7e327162 Mon Sep 17 00:00:00 2001 From: heyaim <223061694+heyaim@users.noreply.github.com> Date: Fri, 31 Jul 2026 05:40:02 -0500 Subject: [PATCH] Give media the explicit file-protection class other stores use (#1552) Media payload writes used .atomic alone and inherited the container default; the courier store, outbox, gossip archive, and receipt index all state their protection class at the write site. Media now follows the same convention: until-first-user-authentication on payload writes and on every site that creates a media directory (the store's helpers, live captures, the outgoing writers, and the files/ root creators), so recordings that save as they go inherit it. A best-effort launch migration stamps files written by older builds, applying only to items at the container default or weaker so it can never downgrade, running detached after the retention sweep from #1484. On stock devices the container default already yields this class, so behavior does not change; the protection is now stated in the code instead of inherited. Full iOS suite green; macOS builds; swiftlint adds no violations. Co-authored-by: jack <212554440+jackjackbits@users.noreply.github.com> --- bitchat/App/AppRuntime.swift | 19 +-- bitchat/Features/media/ImageUtils.swift | 2 +- .../Features/voice/VoiceCaptureSession.swift | 2 +- bitchat/Features/voice/VoiceRecorder.swift | 2 +- bitchat/Models/BitchatMessage+Media.swift | 2 +- .../Services/BLE/BLEIncomingFileStore.swift | 109 +++++++++++++++++- .../ViewModels/ChatLiveVoiceCoordinator.swift | 6 +- .../ChatMediaTransferCoordinator.swift | 2 +- .../Services/MediaRetentionTests.swift | 79 +++++++++++++ 9 files changed, 205 insertions(+), 18 deletions(-) diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index b7c20511..0158ea0a 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -152,18 +152,23 @@ final class AppRuntime: ObservableObject { NetworkActivationService.shared.start() GeohashPresenceService.shared.start() checkForSharedContent() - expireAgedMedia() + performMediaMaintenance() record(.launched) record(.startupCompleted) } - /// Drops media that has outlived the retention window. Off the main thread - /// and best-effort: the sweep walks the media tree, and nothing at launch - /// depends on its result. - private func expireAgedMedia() { - Task(priority: .utility) { - BLEIncomingFileStore().expireAgedMedia() + /// Drops media that has outlived the retention window, then applies the + /// explicit protection class to files that older builds wrote without + /// one. Expiry runs first so the migration never touches files the + /// sweep is about to delete. Detached because `AppRuntime` is + /// main-actor and both passes go file by file through the media tree; + /// best-effort, nothing at launch depends on their results. + private func performMediaMaintenance() { + Task.detached(priority: .utility) { + let store = BLEIncomingFileStore() + store.expireAgedMedia() + store.migrateFileProtectionIfNeeded() } } diff --git a/bitchat/Features/media/ImageUtils.swift b/bitchat/Features/media/ImageUtils.swift index b49f92ca..a6eb25d1 100644 --- a/bitchat/Features/media/ImageUtils.swift +++ b/bitchat/Features/media/ImageUtils.swift @@ -206,7 +206,7 @@ enum ImageUtils { } else { directory = try applicationFilesDirectory().appendingPathComponent("images/outgoing", isDirectory: true) } - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true, attributes: nil) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true, attributes: BLEIncomingFileStore.mediaProtectionAttributes) return directory.appendingPathComponent(fileName) } diff --git a/bitchat/Features/voice/VoiceCaptureSession.swift b/bitchat/Features/voice/VoiceCaptureSession.swift index b49c3beb..75451552 100644 --- a/bitchat/Features/voice/VoiceCaptureSession.swift +++ b/bitchat/Features/voice/VoiceCaptureSession.swift @@ -244,7 +244,7 @@ final class PTTLiveVoiceSession: VoiceCaptureSession { let directory = base .appendingPathComponent("files", isDirectory: true) .appendingPathComponent("voicenotes/outgoing", isDirectory: true) - try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true, attributes: nil) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true, attributes: BLEIncomingFileStore.mediaProtectionAttributes) return directory.appendingPathComponent("voice_\(burstID.hexEncodedString()).m4a") } } diff --git a/bitchat/Features/voice/VoiceRecorder.swift b/bitchat/Features/voice/VoiceRecorder.swift index 80412856..909879cd 100644 --- a/bitchat/Features/voice/VoiceRecorder.swift +++ b/bitchat/Features/voice/VoiceRecorder.swift @@ -300,7 +300,7 @@ actor VoiceRecorder { let baseDirectory = try outputDirectory ?? applicationFilesDirectory().appendingPathComponent("voicenotes/outgoing", isDirectory: true) - try FileManager.default.createDirectory(at: baseDirectory, withIntermediateDirectories: true, attributes: nil) + try FileManager.default.createDirectory(at: baseDirectory, withIntermediateDirectories: true, attributes: BLEIncomingFileStore.mediaProtectionAttributes) return baseDirectory.appendingPathComponent(fileName) } diff --git a/bitchat/Models/BitchatMessage+Media.swift b/bitchat/Models/BitchatMessage+Media.swift index a718e484..16e35a79 100644 --- a/bitchat/Models/BitchatMessage+Media.swift +++ b/bitchat/Models/BitchatMessage+Media.swift @@ -24,7 +24,7 @@ extension BitchatMessage { do { let base = try FileManager.default.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: true) let filesDir = base.appendingPathComponent("files", isDirectory: true) - try FileManager.default.createDirectory(at: filesDir, withIntermediateDirectories: true, attributes: nil) + try FileManager.default.createDirectory(at: filesDir, withIntermediateDirectories: true, attributes: BLEIncomingFileStore.mediaProtectionAttributes) self.filesDir = filesDir } catch { filesDir = nil diff --git a/bitchat/Services/BLE/BLEIncomingFileStore.swift b/bitchat/Services/BLE/BLEIncomingFileStore.swift index 826a391c..08ee3649 100644 --- a/bitchat/Services/BLE/BLEIncomingFileStore.swift +++ b/bitchat/Services/BLE/BLEIncomingFileStore.swift @@ -140,6 +140,20 @@ struct BLEIncomingFileStore: @unchecked Sendable { /// orphans a previous session left behind. static let liveCapturePrefix = "voice_live_" + /// Media payloads follow the same at-rest posture as the app's other + /// persistence layers (courier, outbox, receipt index): protected until + /// first unlock, so the launch-time retention sweep can still run after + /// a reboot. Applied to the media directories so recordings that save + /// as they go (live captures, `AVAudioRecorder`) inherit it, and stated + /// explicitly at the payload write site like every other store. + static var mediaProtectionAttributes: [FileAttributeKey: Any]? { + #if os(iOS) + return [.protectionKey: FileProtectionType.completeUntilFirstUserAuthentication] + #else + return nil + #endif + } + /// Exposed so callers that write progressively into the store's /// directories (live voice captures) share the same file manager. let fileManager: FileManager @@ -223,7 +237,7 @@ struct BLEIncomingFileStore: @unchecked Sendable { isDirectory: true ), withIntermediateDirectories: true, - attributes: nil + attributes: Self.mediaProtectionAttributes ) } } catch { @@ -268,7 +282,7 @@ struct BLEIncomingFileStore: @unchecked Sendable { /// write progressively instead of via `save` (live voice captures). func incomingDirectory(subdirectory: String) throws -> URL { let directory = try filesDirectory().appendingPathComponent(subdirectory, isDirectory: true) - try fileManager.createDirectory(at: directory, withIntermediateDirectories: true, attributes: nil) + try fileManager.createDirectory(at: directory, withIntermediateDirectories: true, attributes: Self.mediaProtectionAttributes) return directory } @@ -284,7 +298,7 @@ struct BLEIncomingFileStore: @unchecked Sendable { do { let base = try filesDirectory().appendingPathComponent(subdirectory, isDirectory: true) - try fileManager.createDirectory(at: base, withIntermediateDirectories: true, attributes: nil) + try fileManager.createDirectory(at: base, withIntermediateDirectories: true, attributes: Self.mediaProtectionAttributes) let sanitized = sanitizedFileName( preferredName, defaultName: "\(defaultPrefix)_\(Self.timestampString(from: dateProvider()))", @@ -306,7 +320,11 @@ struct BLEIncomingFileStore: @unchecked Sendable { ), forceRandomizedName: reservedPaths == nil ) - try data.write(to: destination, options: .atomic) + var options: Data.WritingOptions = [.atomic] + #if os(iOS) + options.insert(.completeFileProtectionUntilFirstUserAuthentication) + #endif + try data.write(to: destination, options: options) payloadCoordination.pendingDeliveryPaths.insert( destination.standardizedFileURL.path ) @@ -650,9 +668,90 @@ struct BLEIncomingFileStore: @unchecked Sendable { return removed } + /// Stamps the media directories and any resident payloads with the + /// explicit protection class, covering files written by builds that + /// relied on the container default. Runs every launch: re-stamping an + /// equal class is a metadata no-op, and anything carrying a stronger + /// class is left alone, so repetition is cheap and can never downgrade. + /// In-flight live captures are skipped for symmetry with the retention + /// sweep; they receive the class at creation and need no repair. + /// Best-effort like the sweep it runs alongside; a file that cannot be + /// stamped is logged, not fatal, and the migration moves on to the next + /// item. Returns the number of items stamped so the launch path and + /// tests can observe coverage. + @discardableResult + func migrateFileProtectionIfNeeded() -> Int { + #if os(iOS) + guard let attributes = Self.mediaProtectionAttributes else { return 0 } + var stamped = 0 + guard let base = try? filesDirectory() else { return 0 } + for subdirectory in Self.mediaSubdirectories { + let dir = base.appendingPathComponent(subdirectory, isDirectory: true) + guard fileManager.fileExists(atPath: dir.path) else { continue } + let files = (try? fileManager.contentsOfDirectory( + at: dir, + includingPropertiesForKeys: [.isRegularFileKey, .isDirectoryKey, .fileProtectionKey], + options: [.skipsHiddenFiles] + )) ?? [] + stamped += stampProtectionIfWeaker(dir, requireRegularFile: false, attributes: attributes) + for fileURL in files { + guard !fileURL.lastPathComponent.hasPrefix(Self.liveCapturePrefix) else { continue } + stamped += stampProtectionIfWeaker(fileURL, requireRegularFile: true, attributes: attributes) + } + } + return stamped + #else + return 0 + #endif + } + + #if os(iOS) + /// Applies the class to one item, but only when the item currently sits + /// at the container default or weaker. The list names the classes that + /// are safe to replace; anything else, including classes added in later + /// iOS versions, is left alone. Only regular files are stamped when + /// `requireRegularFile` is set (and only real directories otherwise), + /// matching the caution the legacy-file removal path applies; symlinks + /// and other non-regular files are left untouched. + private func stampProtectionIfWeaker( + _ itemURL: URL, + requireRegularFile: Bool, + attributes: [FileAttributeKey: Any] + ) -> Int { + let values = try? itemURL.resourceValues( + forKeys: [.isRegularFileKey, .isDirectoryKey, .fileProtectionKey] + ) + if requireRegularFile { + guard values?.isRegularFile == true else { return 0 } + } else { + guard values?.isDirectory == true else { return 0 } + } + if let current = values?.fileProtection, + current != .none, + current != .completeUntilFirstUserAuthentication { + return 0 + } + do { + try fileManager.setAttributes(attributes, ofItemAtPath: itemURL.path) + return 1 + } catch let error as CocoaError where error.code == .fileNoSuchFile { + // Quota eviction or a deletion commit on another store instance + // can delete an item out from under this migration; that is not + // a failure. + return 0 + } catch { + SecureLogger.warning( + "⚠️ Failed to migrate media file protection: \(error)", + category: .security + ) + return 0 + } + } + #endif + private func filesDirectory() throws -> URL { let filesDir = try rootDirectory().appendingPathComponent("files", isDirectory: true) - try fileManager.createDirectory(at: filesDir, withIntermediateDirectories: true, attributes: nil) + try fileManager.createDirectory(at: filesDir, withIntermediateDirectories: true, attributes: Self.mediaProtectionAttributes) return filesDir } diff --git a/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift b/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift index c3ec5ce9..78fa83e1 100644 --- a/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift +++ b/bitchat/ViewModels/ChatLiveVoiceCoordinator.swift @@ -353,7 +353,11 @@ final class ChatLiveVoiceCoordinator { // Eviction skips voice_live_* names, so partials still streaming in // are safe no matter which caller triggers enforcement. fileStore.enforceQuota(reservingBytes: TransportConfig.pttMaxBurstBytes) - fileManager.createFile(atPath: fileURL.path, contents: nil) + fileManager.createFile( + atPath: fileURL.path, + contents: nil, + attributes: BLEIncomingFileStore.mediaProtectionAttributes + ) guard let handle = try? FileHandle(forWritingTo: fileURL) else { SecureLogger.error("PTT: cannot open capture file for burst \(burstID.hexEncodedString())", category: .session) try? fileManager.removeItem(at: fileURL) diff --git a/bitchat/ViewModels/ChatMediaTransferCoordinator.swift b/bitchat/ViewModels/ChatMediaTransferCoordinator.swift index 1d663044..84295135 100644 --- a/bitchat/ViewModels/ChatMediaTransferCoordinator.swift +++ b/bitchat/ViewModels/ChatMediaTransferCoordinator.swift @@ -1899,7 +1899,7 @@ private extension ChatMediaTransferCoordinator { try FileManager.default.createDirectory( at: filesDirectory, withIntermediateDirectories: true, - attributes: nil + attributes: BLEIncomingFileStore.mediaProtectionAttributes ) return filesDirectory } diff --git a/bitchatTests/Services/MediaRetentionTests.swift b/bitchatTests/Services/MediaRetentionTests.swift index 18b33ca7..6c92af10 100644 --- a/bitchatTests/Services/MediaRetentionTests.swift +++ b/bitchatTests/Services/MediaRetentionTests.swift @@ -114,4 +114,83 @@ struct MediaRetentionTests { func defaultRetentionIsSevenDays() { #expect(BLEIncomingFileStore.defaultMediaRetention == 7 * 24 * 60 * 60) } + + #if os(iOS) + /// Media was the one persistence layer that never stated a protection + /// class at its write site, so payloads inherited the container + /// default. Saves must survive the added write option, + /// and on device the class must read back. The simulator's filesystem + /// does not model data protection (the attribute reads back nil there), + /// so the readback assertion is device-only. + @Test + func savedMediaSurvivesExplicitProtectionClass() throws { + let root = makeRoot() + defer { try? FileManager.default.removeItem(at: root) } + let store = BLEIncomingFileStore(baseDirectory: root) + + let payload = Data([0xFF, 0xD8, 0xFF, 0xD9]) + let saved = try #require(store.save( + data: payload, + preferredName: "note.m4a", + subdirectory: "voicenotes/incoming", + fallbackExtension: "m4a", + defaultPrefix: "voice" + )) + + #expect(try Data(contentsOf: saved) == payload) + #if !targetEnvironment(simulator) + let protection = try FileManager.default.attributesOfItem( + atPath: saved.path + )[.protectionKey] as? FileProtectionType + #expect(protection == .completeUntilFirstUserAuthentication) + #endif + } + + /// Files written before payloads carried an explicit class are stamped + /// by the launch-time migration that follows the retention sweep: the + /// directory plus each resident file, without error. In-flight live + /// captures are left alone, exactly as the sweep leaves them: the + /// coordinator may still be writing to one through an open FileHandle, + /// and new captures receive the class at creation. Readback is device-only for the same + /// reason as above. + @Test + func migrationStampsPreexistingMediaAndSkipsLiveCaptures() throws { + let root = makeRoot() + defer { try? FileManager.default.removeItem(at: root) } + let store = BLEIncomingFileStore(baseDirectory: root) + let incoming = try store.incomingDirectory(subdirectory: "voicenotes/incoming") + + let legacy = try write( + "received.m4a", + in: incoming, + modified: Date(timeIntervalSinceNow: -60) + ) + _ = try write( + "\(BLEIncomingFileStore.liveCapturePrefix)00112233445566ff_dm.aac", + in: incoming, + modified: Date(timeIntervalSinceNow: -60) + ) + + // Exactly the directory itself plus the legacy file; strict equality + // is what proves the live capture was not stamped. + #expect(store.migrateFileProtectionIfNeeded() == 2) + #expect(FileManager.default.fileExists(atPath: legacy.path)) + #if !targetEnvironment(simulator) + let protection = try FileManager.default.attributesOfItem( + atPath: legacy.path + )[.protectionKey] as? FileProtectionType + #expect(protection == .completeUntilFirstUserAuthentication) + #endif + } + + /// A store with no media on disk has nothing to stamp. + @Test + func migrationWithNoMediaIsANoOp() { + let root = makeRoot() + defer { try? FileManager.default.removeItem(at: root) } + let store = BLEIncomingFileStore(baseDirectory: root) + + #expect(store.migrateFileProtectionIfNeeded() == 0) + } + #endif }