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:
jack
2026-07-26 14:15:38 +02:00
co-authored by Claude Opus 4.8
parent c030e38420
commit d2e5afd6e7
8 changed files with 350 additions and 55 deletions
+101 -43
View File
@@ -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
+12
View File
@@ -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