From a86df3d1af36b29aaa7988e84e741aa4e4217c10 Mon Sep 17 00:00:00 2001 From: Bradley Mackey Date: Tue, 15 Sep 2026 09:26:41 +0400 Subject: [PATCH] Refresh the payload hash on killphrase, insert, and HOTP mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit triggerBackupIfNeeded compares currentPayloadHash against the last backup hash and skips when equal. Killphrase deletion, item insert, and HOTP counter increments all mutated the vault without refreshing the hash, so auto-backup silently skipped them — leaving killed items recoverable from the newest backup and new items unbacked-up until a later mutation. Match the update/delete/reorder ordering on all three. Co-Authored-By: Claude Fable 5 --- .../VaultFeed/Storage/VaultDataModel.swift | 8 ++ .../Storage/AutoBackupServiceImplTests.swift | 32 ++++++ .../Storage/VaultDataModelTests.swift | 105 +++++++++++++++++- 3 files changed, 143 insertions(+), 2 deletions(-) diff --git a/Vault/Sources/VaultFeed/Storage/VaultDataModel.swift b/Vault/Sources/VaultFeed/Storage/VaultDataModel.swift index 8f451cf46..2800e58dd 100644 --- a/Vault/Sources/VaultFeed/Storage/VaultDataModel.swift +++ b/Vault/Sources/VaultFeed/Storage/VaultDataModel.swift @@ -343,6 +343,11 @@ extension VaultDataModel { // If killphrase deletion occurred, sync OTP autofill store to remove deleted items if didDeleteKillphraseItems { try? await syncAllToOTPAutofillStore() + // Refresh the payload hash before notifying, otherwise + // auto-backup compares its last-backup hash against the + // stale value and skips — leaving the deleted items + // recoverable from the newest backup. + await updateCurrentPayloadHash() // Notify downstream observers (auto-backup, widget reload) that // the item set changed silently from a killphrase match. onDataChanged?() @@ -385,6 +390,8 @@ extension VaultDataModel { searchableLevel: item.searchableLevel, showInQuickType: item.showInQuickType, ) + await updateCurrentPayloadHash() + onDataChanged?() } public func update(itemID id: Identifier, data: VaultItem.Write) async throws { @@ -444,6 +451,7 @@ extension VaultDataModel: VaultStoreHOTPIncrementer { public func incrementCounter(id: Identifier) async throws { try await vaultStore.incrementCounter(id: id) await reloadItems() + await updateCurrentPayloadHash() onDataChanged?() } } diff --git a/Vault/Tests/VaultFeedTests/Storage/AutoBackupServiceImplTests.swift b/Vault/Tests/VaultFeedTests/Storage/AutoBackupServiceImplTests.swift index 5f4234e8c..0bd5df478 100644 --- a/Vault/Tests/VaultFeedTests/Storage/AutoBackupServiceImplTests.swift +++ b/Vault/Tests/VaultFeedTests/Storage/AutoBackupServiceImplTests.swift @@ -182,6 +182,38 @@ struct AutoBackupServiceImplTests { #expect(provider.writeCallCount == 0) } + @Test @LeakTracked + func triggerBackupIfNeeded_runsAfterKillphraseDeletionChangesHash() async throws { + let provider = BackupStorageProviderStub(id: "test") + let store = VaultStoreStub() + let killphraseDeleter = VaultStoreKillphraseDeleterMock() + let dataModel = anyVaultDataModel(vaultStore: store, vaultKillphraseDeleter: killphraseDeleter) + try await dataModel.store(backupPassword: anyBackupPassword()) + await dataModel.setup() + let sut = try makeSUT(providers: [provider], dataModel: dataModel) + await Task.yield() + await sut.setEnabled(true) + await sut.selectProvider(id: "test") + // Take a backup so lastBackupHash matches the current payload hash. + await sut.forceBackup() + #expect(provider.writeCallCount == 1) + + // Killphrase fires: the store's contents change out from under + // the model, and the payload hash must be refreshed on that path + // or this trigger compares the stale hash and skips — leaving the + // killed items recoverable from the newest backup. + killphraseDeleter.deleteItemsHandler = { _, _ in true } + dataModel.itemsSearchQuery = "kill phrase" + store.exportVaultHandler = { userDescription in + .init(userDescription: userDescription, items: [uniqueVaultItem()], tags: []) + } + await dataModel.reloadItems() + + await sut.triggerBackupIfNeeded() + + #expect(provider.writeCallCount == 2) + } + // MARK: - Force Backup @Test @LeakTracked diff --git a/Vault/Tests/VaultFeedTests/Storage/VaultDataModelTests.swift b/Vault/Tests/VaultFeedTests/Storage/VaultDataModelTests.swift index cf9647408..73e81f4af 100644 --- a/Vault/Tests/VaultFeedTests/Storage/VaultDataModelTests.swift +++ b/Vault/Tests/VaultFeedTests/Storage/VaultDataModelTests.swift @@ -492,6 +492,100 @@ final class VaultDataModelTests { #expect(vaultOtpAutofillStore.syncAllCallCount == 1) } + @Test + func reloadItems_refreshesPayloadHashWhenKillphraseDeletesItems() async throws { + let store = VaultStoreStub() + let killphraseDeleter = VaultStoreKillphraseDeleterMock() + let keyStore = KillphraseKeyStoreMock() + keyStore.loadOrCreateHandler = { + (try? KeyData<32>(data: Data(repeating: 0xAA, count: 32))) ?? .zero() + } + let sut = makeSUT( + vaultStore: store, + vaultKillphraseDeleter: killphraseDeleter, + killphraseKeyStore: keyStore, + ) + await sut.setup() + let hashBeforeDeletion = try #require(sut.currentPayloadHash) + sut.itemsSearchQuery = "hello world" + killphraseDeleter.deleteItemsHandler = { _, _ in true } + // The store's contents change out from under the model when the + // killphrase fires; auto-backup relies on the refreshed hash to + // notice, otherwise the deleted items persist in the newest backup. + store.exportVaultHandler = { userDescription in + VaultApplicationPayload( + userDescription: userDescription, + items: [uniqueVaultItem()], + tags: [], + ) + } + + await sut.reloadItems() + + #expect(sut.currentPayloadHash != nil) + #expect(sut.currentPayloadHash != hashBeforeDeletion) + } + + @Test + func reloadItems_doesNotRefreshPayloadHashWhenNoKillphraseDeletionOccurs() async throws { + let store = VaultStoreStub() + let sut = makeSUT(vaultStore: store) + await sut.setup() + let exportsAfterSetup = store.exportVaultCallCount + + await sut.reloadItems() + + // Plain reloads (every search keystroke) must not trigger a full + // vault export just to recompute the hash. + #expect(store.exportVaultCallCount == exportsAfterSetup) + } + + @Test + func insert_refreshesPayloadHashAndNotifiesDataChanged() async throws { + let store = VaultStoreStub() + let sut = makeSUT(vaultStore: store) + await sut.setup() + let hashBeforeInsert = try #require(sut.currentPayloadHash) + store.exportVaultHandler = { userDescription in + VaultApplicationPayload( + userDescription: userDescription, + items: [uniqueVaultItem()], + tags: [], + ) + } + + try await confirmation("Data change notified", expectedCount: 1) { confirmChange in + sut.onDataChanged = { + confirmChange() + } + + try await sut.insert(item: uniqueVaultItem().makeWritable()) + } + + #expect(sut.currentPayloadHash != nil) + #expect(sut.currentPayloadHash != hashBeforeInsert) + } + + @Test + func incrementCounter_refreshesPayloadHash() async throws { + let store = VaultStoreStub() + let sut = makeSUT(vaultStore: store) + await sut.setup() + let hashBeforeIncrement = try #require(sut.currentPayloadHash) + store.exportVaultHandler = { userDescription in + VaultApplicationPayload( + userDescription: userDescription, + items: [uniqueVaultItem()], + tags: [], + ) + } + + try await sut.incrementCounter(id: .new()) + + #expect(sut.currentPayloadHash != nil) + #expect(sut.currentPayloadHash != hashBeforeIncrement) + } + @Test func insert_createsItemInStoreAndReloads() async throws { let store = VaultStoreStub() @@ -500,7 +594,11 @@ final class VaultDataModelTests { try await sut.insert(item: item) - #expect(store.calledMethods == [.insert, .retrieve]) + #expect(store.calledMethods == [ + .insert, + .retrieve, // reload items + .export, // export for payload hash + ]) } @Test @@ -789,7 +887,10 @@ final class VaultDataModelTests { } #expect(store.incrementCounterCallCount == 1) - #expect(store.calledMethods == [.retrieve]) + #expect(store.calledMethods == [ + .retrieve, // reload items + .export, // export for payload hash + ]) } @Test