mirror of
https://github.com/permissionlesstech/bitchat.git
synced 2026-07-26 21:25:22 +00:00
Actually delete legacy incoming media on explicit delete and /clear
Explicit deletion of LEGACY (non-stable-ID) incoming media left the decrypted payload on disk indefinitely, waiting for 100MB oldest-first quota cleanup. Restore real deletion safely with the machinery this PR built: BLEIncomingFileStore.removeLegacyIncomingFile unlinks under the payload-coordination lock only when the path is not pending delivery, not held by a deletion reservation, and not owned by a stable receipt or journal entry; otherwise the file keeps the leave-for-quota fallback. The view model routes per-bubble deletes and /clear through the gated unlink once no bubble in any conversation references the basename. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -392,6 +392,83 @@ struct BLEIncomingFileStore: @unchecked Sendable {
|
||||
)
|
||||
}
|
||||
|
||||
/// Explicit deletion of a LEGACY (non-stable-ID) incoming payload.
|
||||
///
|
||||
/// Legacy media has no durable receipt, so the only safe unlink is one
|
||||
/// that can prove no other owner may hold the basename: the path must
|
||||
/// not be pending delivery, must not belong to an in-flight deletion
|
||||
/// reservation, and must not be owned by a stable receipt or journal
|
||||
/// entry. When any of those hold — or receipt state cannot be read —
|
||||
/// the file stays for bounded quota cleanup (the fail-safe fallback).
|
||||
/// Returns true only when the payload was verifiably unlinked.
|
||||
@discardableResult
|
||||
func removeLegacyIncomingFile(relativePath: String) -> Bool {
|
||||
payloadCoordination.lock.lock()
|
||||
defer { payloadCoordination.lock.unlock() }
|
||||
|
||||
guard let payload = incomingPayloadURL(
|
||||
relativePath: relativePath
|
||||
) else {
|
||||
return false
|
||||
}
|
||||
let standardizedPath = payload.standardizedFileURL.path
|
||||
let reservedByDeletion = payloadCoordination.deletionReservations
|
||||
.values.contains { $0.contains(standardizedPath) }
|
||||
guard !reservedByDeletion,
|
||||
!payloadCoordination.pendingDeliveryPaths.contains(
|
||||
standardizedPath
|
||||
),
|
||||
let receiptOwnedPaths =
|
||||
privateMediaReceipts.reservedPayloadPaths(),
|
||||
!receiptOwnedPaths.contains(standardizedPath) else {
|
||||
return false
|
||||
}
|
||||
guard fileManager.fileExists(atPath: payload.path),
|
||||
(try? payload.resourceValues(
|
||||
forKeys: [.isRegularFileKey]
|
||||
).isRegularFile) == true else {
|
||||
return false
|
||||
}
|
||||
do {
|
||||
try fileManager.removeItem(at: payload)
|
||||
return !fileManager.fileExists(atPath: payload.path)
|
||||
} catch {
|
||||
SecureLogger.warning(
|
||||
"⚠️ Failed to remove explicitly deleted legacy media: \(error)",
|
||||
category: .session
|
||||
)
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolves a `files/`-relative path iff it lands directly inside one of
|
||||
/// the incoming media directories. Anything else is not a deletable
|
||||
/// incoming payload.
|
||||
private func incomingPayloadURL(relativePath: String) -> URL? {
|
||||
guard !relativePath.isEmpty,
|
||||
let base = try? filesDirectory().standardizedFileURL else {
|
||||
return nil
|
||||
}
|
||||
let candidate = base
|
||||
.appendingPathComponent(relativePath, isDirectory: false)
|
||||
.standardizedFileURL
|
||||
let parentPath = candidate.deletingLastPathComponent().path
|
||||
let incomingDirectories = [
|
||||
"voicenotes/incoming",
|
||||
"images/incoming",
|
||||
"files/incoming"
|
||||
]
|
||||
guard incomingDirectories.contains(where: { relativeDirectory in
|
||||
base.appendingPathComponent(
|
||||
relativeDirectory,
|
||||
isDirectory: true
|
||||
).standardizedFileURL.path == parentPath
|
||||
}) else {
|
||||
return nil
|
||||
}
|
||||
return candidate
|
||||
}
|
||||
|
||||
/// Releases the short window between disk save and synchronous
|
||||
/// conversation insertion. Before this callback, a deletion transaction
|
||||
/// may not infer ownership from a stale bubble that names the same path.
|
||||
|
||||
@@ -4511,6 +4511,14 @@ extension BLEService: PrivateMediaDeletionPersisting {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
func removeLegacyPrivateMediaPayload(relativePath: String) {
|
||||
let fileStore = incomingFileStore
|
||||
messageQueue.async(flags: .barrier) {
|
||||
fileStore.removeLegacyIncomingFile(relativePath: relativePath)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Private Helpers
|
||||
|
||||
@@ -108,6 +108,14 @@ protocol PrivateMediaDeletionPersisting: AnyObject {
|
||||
protectedPayloadRelativePaths: Set<String>,
|
||||
completion: @escaping @MainActor (Bool) -> Void
|
||||
)
|
||||
|
||||
/// Gated unlink for a LEGACY (non-stable-ID) incoming payload whose
|
||||
/// bubble was explicitly removed. Implementations delete the file only
|
||||
/// when its path is not pending delivery and not reserved by any receipt
|
||||
/// or in-flight deletion transaction; otherwise the file stays for
|
||||
/// bounded quota cleanup.
|
||||
@MainActor
|
||||
func removeLegacyPrivateMediaPayload(relativePath: String)
|
||||
}
|
||||
|
||||
protocol TransportEventDelegate: AnyObject {
|
||||
|
||||
@@ -226,18 +226,51 @@ extension ChatViewModel: ChatMediaTransferContext {
|
||||
let message = conversations.conversationsByID.values.lazy
|
||||
.flatMap(\.messages)
|
||||
.first { $0.id == messageID }
|
||||
if let message {
|
||||
if !isIncomingPrivateMessage(message) {
|
||||
mediaTransferCoordinator.cleanupOutgoingLocalFile(
|
||||
forMessage: message
|
||||
)
|
||||
}
|
||||
// Legacy/raw incoming media has no durable ID-to-file ownership.
|
||||
// Its basename may already belong to a pending newer arrival, so
|
||||
// leave the payload for bounded quota cleanup rather than unlink
|
||||
// an ambiguous path.
|
||||
if let message, !isIncomingPrivateMessage(message) {
|
||||
mediaTransferCoordinator.cleanupOutgoingLocalFile(
|
||||
forMessage: message
|
||||
)
|
||||
}
|
||||
removeMessage(withID: messageID, cleanupFile: false)
|
||||
if let message {
|
||||
cleanupLegacyIncomingMediaPayloads(for: [message])
|
||||
}
|
||||
}
|
||||
|
||||
/// Explicitly deleted LEGACY (non-stable-ID) incoming media has no
|
||||
/// durable ID-to-file ownership, so the actual unlink is delegated to
|
||||
/// the transport's gated cleanup: a basename that is pending delivery or
|
||||
/// reserved by a receipt/deletion transaction stays on disk for bounded
|
||||
/// quota cleanup instead. Must run after the bubbles were removed; a
|
||||
/// surviving reference in any conversation keeps the payload.
|
||||
func cleanupLegacyIncomingMediaPayloads(for messages: [BitchatMessage]) {
|
||||
guard let cleanup =
|
||||
meshService as? any PrivateMediaDeletionPersisting else {
|
||||
return
|
||||
}
|
||||
let legacyPaths = Set(messages.compactMap { message -> String? in
|
||||
guard !PrivateMediaMessageIdentity.isStableID(message.id),
|
||||
isIncomingPrivateMessage(message) else {
|
||||
return nil
|
||||
}
|
||||
return incomingMediaRelativePath(for: message)
|
||||
})
|
||||
guard !legacyPaths.isEmpty else { return }
|
||||
let survivingPaths = Set(
|
||||
conversations.conversationsByID.values.lazy
|
||||
.flatMap(\.messages)
|
||||
.compactMap { message -> String? in
|
||||
guard self.isIncomingPrivateMessage(message) else {
|
||||
return nil
|
||||
}
|
||||
return self.incomingMediaRelativePath(for: message)
|
||||
}
|
||||
)
|
||||
for relativePath in legacyPaths.subtracting(survivingPaths).sorted() {
|
||||
cleanup.removeLegacyPrivateMediaPayload(
|
||||
relativePath: relativePath
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
func removeOutgoingMediaMessage(withID messageID: String) {
|
||||
|
||||
@@ -908,8 +908,9 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage
|
||||
|
||||
// Stable payload cleanup belongs entirely to the durable receiver
|
||||
// journal. Legacy/raw incoming payloads have no durable identity,
|
||||
// so their basenames may already belong to a pending new arrival;
|
||||
// leave those files for bounded quota cleanup.
|
||||
// so once their bubbles are gone the transport's gated cleanup
|
||||
// decides per basename: unlink when unreferenced, or leave any
|
||||
// pending/reserved path for bounded quota cleanup.
|
||||
let finalPlan = currentRemovalPlan()
|
||||
|
||||
for (conversationID, messageIDs) in finalPlan {
|
||||
@@ -917,6 +918,7 @@ final class ChatViewModel: ObservableObject, BitchatDelegate, SynchronousMessage
|
||||
messageIDs.contains($0.id)
|
||||
}
|
||||
}
|
||||
cleanupLegacyIncomingMediaPayloads(for: capturedIncomingMedia)
|
||||
completion()
|
||||
}
|
||||
|
||||
|
||||
@@ -1278,47 +1278,6 @@ struct ChatViewModelPrivateMediaDeletionTests {
|
||||
)
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
func legacyIncomingDeleteLeavesAmbiguousPayloadForQuota() throws {
|
||||
let (viewModel, transport) = makeTestableViewModel()
|
||||
let peerID = PeerID(str: String(repeating: "3", count: 64))
|
||||
let incoming = try FileManager.default.url(
|
||||
for: .applicationSupportDirectory,
|
||||
in: .userDomainMask,
|
||||
appropriateFor: nil,
|
||||
create: true
|
||||
)
|
||||
.appendingPathComponent(
|
||||
"files/images/incoming",
|
||||
isDirectory: true
|
||||
)
|
||||
try FileManager.default.createDirectory(
|
||||
at: incoming,
|
||||
withIntermediateDirectories: true
|
||||
)
|
||||
let payload = incoming.appendingPathComponent(
|
||||
"legacy-delete-\(UUID().uuidString).jpg"
|
||||
)
|
||||
try Data([0x01]).write(to: payload)
|
||||
defer { try? FileManager.default.removeItem(at: payload) }
|
||||
let legacyID = UUID().uuidString
|
||||
viewModel.seedPrivateChat([
|
||||
privateMediaMessage(
|
||||
id: legacyID,
|
||||
sender: "Old client",
|
||||
senderPeerID: peerID,
|
||||
recipient: viewModel.nickname,
|
||||
filename: payload.lastPathComponent
|
||||
)
|
||||
], for: peerID)
|
||||
|
||||
viewModel.deleteMediaMessage(messageID: legacyID)
|
||||
|
||||
#expect(transport.deletedPrivateMediaMessageIDBatches.isEmpty)
|
||||
#expect((viewModel.privateChats[peerID] ?? []).isEmpty)
|
||||
#expect(FileManager.default.fileExists(atPath: payload.path))
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
func clearPrivateChatTombstonesIncomingAndCancelsOutgoingBeforeClear() {
|
||||
let (viewModel, transport) = makeTestableViewModel()
|
||||
@@ -1874,7 +1833,7 @@ struct ChatViewModelPrivateMediaDeletionTests {
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
func clearPrivateChatLeavesAmbiguousLegacyFileForQuotaCleanup()
|
||||
func clearPrivateChatUnlinksLegacyFileOnlyAfterLastReference()
|
||||
throws {
|
||||
let (viewModel, transport) = makeTestableViewModel()
|
||||
let firstPeerID = PeerID(str: String(repeating: "9", count: 64))
|
||||
@@ -1907,14 +1866,113 @@ struct ChatViewModelPrivateMediaDeletionTests {
|
||||
|
||||
viewModel.clearPrivateChat(firstPeerID)
|
||||
|
||||
// A surviving mirror in another conversation keeps the payload.
|
||||
#expect(transport.deletedPrivateMediaMessageIDBatches.isEmpty)
|
||||
#expect(transport.removedLegacyPrivateMediaPaths.isEmpty)
|
||||
#expect(FileManager.default.fileExists(atPath: fileURL.path))
|
||||
#expect(viewModel.privateChats[aliasPeerID]?.map(\.id) == [message.id])
|
||||
|
||||
viewModel.clearPrivateChat(aliasPeerID)
|
||||
|
||||
#expect(FileManager.default.fileExists(atPath: fileURL.path))
|
||||
// Clearing the last reference routes the legacy payload through the
|
||||
// transport's gated unlink, which deletes it (nothing pending or
|
||||
// reserved names this basename).
|
||||
#expect((viewModel.privateChats[aliasPeerID] ?? []).isEmpty)
|
||||
#expect(
|
||||
transport.removedLegacyPrivateMediaPaths
|
||||
== ["images/incoming/\(fileURL.lastPathComponent)"]
|
||||
)
|
||||
#expect(!FileManager.default.fileExists(atPath: fileURL.path))
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
func deleteLegacyIncomingMediaUnlinksUnreferencedPayload() throws {
|
||||
let (viewModel, transport) = makeTestableViewModel()
|
||||
let peerID = PeerID(str: String(repeating: "b", count: 64))
|
||||
let incomingDirectory = try FileManager.default.url(
|
||||
for: .applicationSupportDirectory,
|
||||
in: .userDomainMask,
|
||||
appropriateFor: nil,
|
||||
create: true
|
||||
)
|
||||
.appendingPathComponent("files/images/incoming", isDirectory: true)
|
||||
try FileManager.default.createDirectory(
|
||||
at: incomingDirectory,
|
||||
withIntermediateDirectories: true
|
||||
)
|
||||
let fileURL = incomingDirectory.appendingPathComponent(
|
||||
"legacy-delete-\(UUID().uuidString).jpg"
|
||||
)
|
||||
try Data("legacy-image".utf8).write(to: fileURL, options: .atomic)
|
||||
defer { try? FileManager.default.removeItem(at: fileURL) }
|
||||
let message = privateMediaMessage(
|
||||
id: UUID().uuidString,
|
||||
sender: "Old client",
|
||||
senderPeerID: peerID,
|
||||
recipient: viewModel.nickname,
|
||||
filename: fileURL.lastPathComponent
|
||||
)
|
||||
viewModel.seedPrivateChat([message], for: peerID)
|
||||
|
||||
viewModel.deleteMediaMessage(messageID: message.id)
|
||||
|
||||
// Legacy incoming media has no stable receipt, so no journal batch —
|
||||
// but the explicit delete must still remove the decrypted payload
|
||||
// through the gated unlink.
|
||||
#expect(transport.deletedPrivateMediaMessageIDBatches.isEmpty)
|
||||
#expect((viewModel.privateChats[peerID] ?? []).isEmpty)
|
||||
#expect(
|
||||
transport.removedLegacyPrivateMediaPaths
|
||||
== ["images/incoming/\(fileURL.lastPathComponent)"]
|
||||
)
|
||||
#expect(!FileManager.default.fileExists(atPath: fileURL.path))
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
func deleteLegacyIncomingMediaKeepsPayloadReferencedByAnotherBubble()
|
||||
throws {
|
||||
let (viewModel, transport) = makeTestableViewModel()
|
||||
let peerID = PeerID(str: String(repeating: "c", count: 64))
|
||||
let incomingDirectory = try FileManager.default.url(
|
||||
for: .applicationSupportDirectory,
|
||||
in: .userDomainMask,
|
||||
appropriateFor: nil,
|
||||
create: true
|
||||
)
|
||||
.appendingPathComponent("files/images/incoming", isDirectory: true)
|
||||
try FileManager.default.createDirectory(
|
||||
at: incomingDirectory,
|
||||
withIntermediateDirectories: true
|
||||
)
|
||||
let fileURL = incomingDirectory.appendingPathComponent(
|
||||
"legacy-shared-\(UUID().uuidString).jpg"
|
||||
)
|
||||
try Data("legacy-image".utf8).write(to: fileURL, options: .atomic)
|
||||
defer { try? FileManager.default.removeItem(at: fileURL) }
|
||||
let deleted = privateMediaMessage(
|
||||
id: UUID().uuidString,
|
||||
sender: "Old client",
|
||||
senderPeerID: peerID,
|
||||
recipient: viewModel.nickname,
|
||||
filename: fileURL.lastPathComponent
|
||||
)
|
||||
// A different bubble (different ID) references the same basename.
|
||||
let survivor = privateMediaMessage(
|
||||
id: UUID().uuidString,
|
||||
sender: "Old client",
|
||||
senderPeerID: peerID,
|
||||
recipient: viewModel.nickname,
|
||||
filename: fileURL.lastPathComponent
|
||||
)
|
||||
viewModel.seedPrivateChat([deleted, survivor], for: peerID)
|
||||
|
||||
viewModel.deleteMediaMessage(messageID: deleted.id)
|
||||
|
||||
#expect(
|
||||
viewModel.privateChats[peerID]?.map(\.id) == [survivor.id]
|
||||
)
|
||||
#expect(transport.removedLegacyPrivateMediaPaths.isEmpty)
|
||||
#expect(FileManager.default.fileExists(atPath: fileURL.path))
|
||||
}
|
||||
|
||||
@Test @MainActor
|
||||
|
||||
@@ -255,6 +255,18 @@ final class MockTransport: Transport, PrivateMediaDeletionPersisting {
|
||||
completion(result ?? persistDeletedPrivateMediaResult)
|
||||
}
|
||||
|
||||
/// Real store instance so view-model tests exercise the gated legacy
|
||||
/// unlink end to end (same Application Support tree the tests write to).
|
||||
let legacyIncomingFileStore = BLEIncomingFileStore()
|
||||
private(set) var removedLegacyPrivateMediaPaths: [String] = []
|
||||
|
||||
func removeLegacyPrivateMediaPayload(relativePath: String) {
|
||||
removedLegacyPrivateMediaPaths.append(relativePath)
|
||||
legacyIncomingFileStore.removeLegacyIncomingFile(
|
||||
relativePath: relativePath
|
||||
)
|
||||
}
|
||||
|
||||
func sendVerifyChallenge(to peerID: PeerID, noiseKeyHex: String, nonceA: Data) {
|
||||
sentVerifyChallenges.append((peerID, noiseKeyHex, nonceA))
|
||||
}
|
||||
|
||||
@@ -1040,6 +1040,103 @@ struct BLEFileTransferHandlerTests {
|
||||
#expect(!FileManager.default.fileExists(atPath: evictable.path))
|
||||
}
|
||||
|
||||
@Test
|
||||
func legacyIncomingDeleteUnlinksUnreferencedPayload() throws {
|
||||
let base = FileManager.default.temporaryDirectory
|
||||
.appendingPathComponent(
|
||||
"legacy-unlink-\(UUID().uuidString)",
|
||||
isDirectory: true
|
||||
)
|
||||
defer { try? FileManager.default.removeItem(at: base) }
|
||||
let store = BLEIncomingFileStore(baseDirectory: base)
|
||||
let incoming = try store.incomingDirectory(
|
||||
subdirectory: "images/incoming"
|
||||
)
|
||||
let payload = incoming.appendingPathComponent("legacy.jpg")
|
||||
try Data([0xFF, 0xD8, 0xFF, 0xD9]).write(to: payload)
|
||||
|
||||
#expect(store.removeLegacyIncomingFile(
|
||||
relativePath: "images/incoming/legacy.jpg"
|
||||
))
|
||||
#expect(!FileManager.default.fileExists(atPath: payload.path))
|
||||
|
||||
// Only paths directly inside an incoming media directory qualify.
|
||||
let outgoing = base.appendingPathComponent(
|
||||
"files/images/outgoing",
|
||||
isDirectory: true
|
||||
)
|
||||
try FileManager.default.createDirectory(
|
||||
at: outgoing,
|
||||
withIntermediateDirectories: true
|
||||
)
|
||||
let victim = outgoing.appendingPathComponent("victim.jpg")
|
||||
try Data([0x01]).write(to: victim)
|
||||
#expect(!store.removeLegacyIncomingFile(
|
||||
relativePath: "images/outgoing/victim.jpg"
|
||||
))
|
||||
#expect(!store.removeLegacyIncomingFile(
|
||||
relativePath: "images/incoming/../outgoing/victim.jpg"
|
||||
))
|
||||
#expect(FileManager.default.fileExists(atPath: victim.path))
|
||||
}
|
||||
|
||||
@Test
|
||||
func legacyIncomingDeleteLeavesPendingDeliveryPayload() throws {
|
||||
let base = FileManager.default.temporaryDirectory
|
||||
.appendingPathComponent(
|
||||
"legacy-pending-\(UUID().uuidString)",
|
||||
isDirectory: true
|
||||
)
|
||||
defer { try? FileManager.default.removeItem(at: base) }
|
||||
let store = BLEIncomingFileStore(baseDirectory: base)
|
||||
|
||||
// save() marks the stored path pending until delivery finishes; a
|
||||
// legacy delete naming the same basename must not unlink it.
|
||||
let stored = try #require(store.save(
|
||||
data: Data([0xFF, 0xD8, 0xFF, 0xD9]),
|
||||
preferredName: "pending.jpg",
|
||||
subdirectory: "images/incoming",
|
||||
fallbackExtension: "jpg",
|
||||
defaultPrefix: "img"
|
||||
))
|
||||
let relativePath = "images/incoming/\(stored.lastPathComponent)"
|
||||
|
||||
#expect(!store.removeLegacyIncomingFile(relativePath: relativePath))
|
||||
#expect(FileManager.default.fileExists(atPath: stored.path))
|
||||
|
||||
// Once the delivery window closes the same unlink is allowed.
|
||||
store.finishIncomingFileDelivery(at: stored)
|
||||
#expect(store.removeLegacyIncomingFile(relativePath: relativePath))
|
||||
#expect(!FileManager.default.fileExists(atPath: stored.path))
|
||||
}
|
||||
|
||||
@Test
|
||||
func legacyIncomingDeleteLeavesReceiptProtectedPayload() throws {
|
||||
let base = FileManager.default.temporaryDirectory
|
||||
.appendingPathComponent(
|
||||
"legacy-protected-\(UUID().uuidString)",
|
||||
isDirectory: true
|
||||
)
|
||||
defer { try? FileManager.default.removeItem(at: base) }
|
||||
let store = BLEIncomingFileStore(baseDirectory: base)
|
||||
let incoming = try store.incomingDirectory(
|
||||
subdirectory: "images/incoming"
|
||||
)
|
||||
let payload = incoming.appendingPathComponent("owned.jpg")
|
||||
try Data([0xFF, 0xD8, 0xFF, 0xD9]).write(to: payload)
|
||||
|
||||
// The basename belongs to a stable receipt (another record). The
|
||||
// legacy delete must leave it for that owner.
|
||||
#expect(store.commitPrivateMediaFile(
|
||||
messageID: "media-00112233445566778899aabbccddeeff",
|
||||
storedURL: payload
|
||||
))
|
||||
#expect(!store.removeLegacyIncomingFile(
|
||||
relativePath: "images/incoming/owned.jpg"
|
||||
))
|
||||
#expect(FileManager.default.fileExists(atPath: payload.path))
|
||||
}
|
||||
|
||||
@Test
|
||||
func panicWipeDeletesEveryManagedMediaFileAndRecreatesEmptyDirectories() throws {
|
||||
let base = FileManager.default.temporaryDirectory
|
||||
|
||||
Reference in New Issue
Block a user