From a1fe6e748857a510c24ba663d19025cd96423213 Mon Sep 17 00:00:00 2001 From: D1a0y1bb <1962389612@qq.com> Date: Fri, 15 May 2026 11:22:50 +0800 Subject: [PATCH] fix: remove legacy assistant keychain entries Move migrated audit assistant credentials out of the legacy default account instead of copying them. Make the Settings delete action remove both the current credential ID and the legacy default account so stale keys cannot reappear on the next launch. Add tests for migration cleanup and delete behavior. --- .../Core/AuditAssistantService.swift | 8 +++++++ .../Features/Settings/SettingsRootView.swift | 2 +- .../AuditAssistantServiceTests.swift | 22 ++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/PitcherPlantApp/Sources/PitcherPlantApp/Core/AuditAssistantService.swift b/PitcherPlantApp/Sources/PitcherPlantApp/Core/AuditAssistantService.swift index 16adb03..f0b47b8 100644 --- a/PitcherPlantApp/Sources/PitcherPlantApp/Core/AuditAssistantService.swift +++ b/PitcherPlantApp/Sources/PitcherPlantApp/Core/AuditAssistantService.swift @@ -515,6 +515,7 @@ struct AuditAssistantCredentialStore { } let value = try read(id: Self.legacyDefaultCredentialID, allowAuthenticationUI: false) try save(value, id: targetID) + try deleteIfPresent(id: Self.legacyDefaultCredentialID) return true } @@ -526,6 +527,13 @@ struct AuditAssistantCredentialStore { try delete(id: id, ignoreMissing: true) } + func deleteCredentialAndLegacyIfPresent(id: String) throws { + try deleteIfPresent(id: id) + if normalizedCredentialID(id) != Self.legacyDefaultCredentialID { + try deleteIfPresent(id: Self.legacyDefaultCredentialID) + } + } + private func authenticationContext(allowAuthenticationUI: Bool) -> LAContext { let context = LAContext() context.interactionNotAllowed = allowAuthenticationUI == false diff --git a/PitcherPlantApp/Sources/PitcherPlantApp/Features/Settings/SettingsRootView.swift b/PitcherPlantApp/Sources/PitcherPlantApp/Features/Settings/SettingsRootView.swift index 6220a64..550c1b2 100644 --- a/PitcherPlantApp/Sources/PitcherPlantApp/Features/Settings/SettingsRootView.swift +++ b/PitcherPlantApp/Sources/PitcherPlantApp/Features/Settings/SettingsRootView.swift @@ -778,7 +778,7 @@ struct SettingsRootView: View { private func deleteAuditAssistantCredential() { do { let configuration = appState.appSettings.auditAssistant ?? AuditAssistantConfiguration() - try AuditAssistantCredentialStore().deleteIfPresent(id: configuration.credentialID) + try AuditAssistantCredentialStore().deleteCredentialAndLegacyIfPresent(id: configuration.credentialID) auditAssistantCredentialDraft = "" hasSavedAuditAssistantCredential = false auditAssistantCredentialRevealed = false diff --git a/PitcherPlantApp/Tests/PitcherPlantAppTests/AuditAssistantServiceTests.swift b/PitcherPlantApp/Tests/PitcherPlantAppTests/AuditAssistantServiceTests.swift index 60dc0c2..6f118e5 100644 --- a/PitcherPlantApp/Tests/PitcherPlantAppTests/AuditAssistantServiceTests.swift +++ b/PitcherPlantApp/Tests/PitcherPlantAppTests/AuditAssistantServiceTests.swift @@ -491,7 +491,27 @@ func keychainCredentialStoreMigratesLegacyDefaultKey() throws { try store.save("legacy-unit-test-secret", id: legacyID) #expect(try store.migrateLegacyDefaultCredentialIfNeeded(to: targetID)) #expect(try store.read(id: targetID) == "legacy-unit-test-secret") - #expect(try store.read(id: legacyID) == "legacy-unit-test-secret") + #expect(try store.exists(id: legacyID) == false) + #expect(try store.migrateLegacyDefaultCredentialIfNeeded(to: targetID) == false) +} + +@Test +func keychainCredentialDeleteRemovesLegacyDefaultKey() throws { + let store = AuditAssistantCredentialStore(service: "com.pitcherplant.desktop.audit-assistant.tests.\(UUID().uuidString)") + let legacyID = AuditAssistantCredentialStore.legacyDefaultCredentialID + let targetID = "pitcherplant.tests.deleted.\(UUID().uuidString)" + defer { + try? store.delete(id: targetID) + try? store.delete(id: legacyID) + } + + try store.save("target-unit-test-secret", id: targetID) + try store.save("legacy-unit-test-secret", id: legacyID) + + try store.deleteCredentialAndLegacyIfPresent(id: targetID) + + #expect(try store.exists(id: targetID) == false) + #expect(try store.exists(id: legacyID) == false) #expect(try store.migrateLegacyDefaultCredentialIfNeeded(to: targetID) == false) }