diff --git a/CLI Pulse Bar/CLI Pulse Bar/ProviderConfigEditor.swift b/CLI Pulse Bar/CLI Pulse Bar/ProviderConfigEditor.swift index 1c00b412..7dadfb41 100644 --- a/CLI Pulse Bar/CLI Pulse Bar/ProviderConfigEditor.swift +++ b/CLI Pulse Bar/CLI Pulse Bar/ProviderConfigEditor.swift @@ -835,11 +835,28 @@ struct ProviderConfigEditor: View { @discardableResult private func save() -> Bool { + state.withProviderAccountPersistenceLock(or: false) { + saveAssumingProviderAccountPersistenceLock() + } + } + + @discardableResult + private func saveAssumingProviderAccountPersistenceLock() + -> Bool + { guard let idx = state.providerConfigs.firstIndex( where: { $0.accountID == accountID } ) else { return false } + guard state.recoverPendingProviderAccountSave(accountID) else { + #if os(macOS) + testState = .failure( + "Could not restore the previous provider configuration. Please retry before saving new changes." + ) + #endif + return false + } guard state.persistProviderAccountCredentialRecoveryAnchor( accountID @@ -852,22 +869,39 @@ struct ProviderConfigEditor: View { #endif return false } - #if os(macOS) - if kind == .gemini && allowsLiveProviderActions { + let secretCheckpoint: + ProviderConfig.SecretPersistenceCheckpoint? + if allowsLiveProviderActions { guard - geminiCredentialDraft.commit( - accountID: accountID - ) + let captured = + state.providerConfigs[idx] + .makeSecretPersistenceCheckpoint() else { + #if os(macOS) testState = .failure( - GeminiOAuthError - .credentialPersistenceFailed - .localizedDescription + "Could not safely read existing provider credentials. Please retry." ) + #endif return false } + secretCheckpoint = captured + } else { + secretCheckpoint = nil + } + let secretRecoveryConfig = state.providerConfigs[idx] + guard + let persistenceCheckpoint = + state.makeProviderAccountPersistenceCheckpoint( + accountID + ) + else { + #if os(macOS) + testState = .failure( + "Could not safely prepare provider configuration. Please retry." + ) + #endif + return false } - #endif state.providerConfigs[idx].sourceMode = sourceMode state.providerConfigs[idx].accountLabel = accountLabel.isEmpty ? nil : accountLabel state.providerConfigs[idx].setPlanOverride( @@ -899,24 +933,99 @@ struct ProviderConfigEditor: View { ? true : nil #endif - if allowsLiveProviderActions { - guard state.providerConfigs[idx].saveSecrets() else { + let transactionResult = ProviderAccountSaveTransaction.commit( + persistSecrets: { + guard allowsLiveProviderActions else { + return true + } + guard state.providerConfigs[idx].saveSecrets() else { + #if os(macOS) + testState = .failure( + "Could not safely save provider credentials. Please retry." + ) + #endif + return false + } + return true + }, + rollbackSecrets: { + guard allowsLiveProviderActions else { + return true + } + guard let secretCheckpoint else { + return false + } + return secretRecoveryConfig + .restoreSecrets( + from: secretCheckpoint + ) + }, + persistMetadata: { + guard state.persistProviderAccountDraftMetadata(accountID) else { + #if os(macOS) + testState = .failure( + "Could not safely save provider configuration. Please retry." + ) + #endif + return false + } + return true + }, + rollbackMetadata: { + persistenceCheckpoint.restore() + }, + commitProviderCredential: { #if os(macOS) - testState = .failure( - "Could not safely save provider credentials. Please retry." - ) + guard kind == .gemini && allowsLiveProviderActions else { + return true + } + guard geminiCredentialDraft.commit(accountID: accountID) else { + testState = .failure( + GeminiOAuthError + .credentialPersistenceFailed + .localizedDescription + ) + return false + } #endif - return false + return true + }, + finalize: { + state.finalizeProviderAccountDraft(accountID) } - } - guard state.commitProviderAccountDraft(accountID) else { + ) + switch transactionResult { + case .committed: + return true + case .failedRolledBack: + return false + case .failedRollbackIncomplete: + let recovery = ProviderAccountSaveRecovery( + restoreMetadata: { + persistenceCheckpoint.restore() + }, + restoreSecrets: { + guard allowsLiveProviderActions else { + return true + } + guard let secretCheckpoint else { + return false + } + return secretRecoveryConfig.restoreSecrets( + from: secretCheckpoint + ) + } + ) + state.retainProviderAccountSaveRecovery( + recovery, + for: accountID + ) #if os(macOS) testState = .failure( - "Could not safely save provider configuration. Please retry." + "Save failed and recovery is incomplete. Retry to restore the previous configuration before saving again." ) #endif return false } - return true } } diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/AppState.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/AppState.swift index 3cbf5078..62212db8 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/AppState.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/AppState.swift @@ -74,6 +74,8 @@ public final class AppState: ObservableObject { // computed forwarders so setProviderEnabled/toggleProvider/DemoDataProvider/ // DataRefreshManager-payload-assign compile unchanged. public let providerState = ProviderState() + private var pendingProviderAccountSaveRecoveries: + [UUID: ProviderAccountSaveRecovery] = [:] public var providers: [ProviderUsage] { get { providerState.providers } @@ -1431,9 +1433,91 @@ public final class AppState: ObservableObject { return resolvedStore.save(providerConfigs) } + public func makeProviderAccountPersistenceCheckpoint( + _ accountID: UUID, + using metadataStore: ProviderConfigMetadataStore? = nil + ) -> ProviderAccountPersistenceCheckpoint? { + guard providerConfigs.contains(where: { + $0.accountID == accountID + }) else { + return nil + } + let allowsHelperMirror = + runtimeEnvironment.capabilities.allowsHelperRegistration + let resolvedStore = metadataStore ?? ProviderConfigMetadataStore( + defaults: providerConfigDefaults, + helperDefaults: allowsHelperMirror + ? providerConfigHelperDefaults + : nil + ) + let ownerCheckpoint: + ProviderSharedCredentialOwner.PersistenceCheckpoint? + if allowsHelperMirror { + guard + let captured = + ProviderSharedCredentialOwner + .makePersistenceCheckpoint() + else { + return nil + } + ownerCheckpoint = captured + } else { + ownerCheckpoint = nil + } + return ProviderAccountPersistenceCheckpoint( + metadataStore: resolvedStore, + metadataCheckpoint: + resolvedStore.makePersistenceCheckpoint(), + ownerCheckpoint: ownerCheckpoint, + restoresOwner: allowsHelperMirror + ) + } + + public func withProviderAccountPersistenceLock( + or failure: T, + _ body: () -> T + ) -> T { + ProviderSharedCredentialOwner.withPersistenceLock( + or: failure, + body + ) + } + + public func retainProviderAccountSaveRecovery( + _ recovery: ProviderAccountSaveRecovery, + for accountID: UUID + ) { + // Never replace the original baseline with one captured from a + // partially compensated state. + guard pendingProviderAccountSaveRecoveries[accountID] == nil else { + return + } + pendingProviderAccountSaveRecoveries[accountID] = recovery + } + + /// Retry an incomplete compensation before the editor is allowed to + /// capture a fresh baseline. A failed recovery remains pending. @discardableResult - public func commitProviderAccountDraft( + public func recoverPendingProviderAccountSave( _ accountID: UUID + ) -> Bool { + guard + let recovery = + pendingProviderAccountSaveRecoveries[accountID] + else { + return true + } + guard recovery.recover() else { + return false + } + pendingProviderAccountSaveRecoveries[accountID] = nil + return true + } + + @discardableResult + public func persistProviderAccountDraftMetadata( + _ accountID: UUID, + using metadataStore: ProviderConfigMetadataStore? = nil ) -> Bool { guard providerConfigs.contains(where: { $0.accountID == accountID @@ -1442,11 +1526,42 @@ public final class AppState: ObservableObject { } // Persist final metadata while the in-memory draft marker still // exists. A failed write leaves the editor transaction retryable. - guard saveProviderConfigMetadata() else { - return false + let allowsHelperMirror = + runtimeEnvironment.capabilities.allowsHelperRegistration + if allowsHelperMirror { + guard ProviderSharedCredentialOwner.reconcile(configs: providerConfigs) else { + return false + } + } + let resolvedStore = metadataStore ?? ProviderConfigMetadataStore( + defaults: providerConfigDefaults, + helperDefaults: allowsHelperMirror + ? providerConfigHelperDefaults + : nil + ) + return resolvedStore.save(providerConfigs) + } + + /// Complete the in-memory portion only after every fallible persistence + /// step, including provider-specific credential mutation, has succeeded. + public func finalizeProviderAccountDraft(_ accountID: UUID) { + guard providerConfigs.contains(where: { + $0.accountID == accountID + }) else { + return } _ = providerState.commitProviderAccountDraft(accountID) buildProviderDetails() + } + + @discardableResult + public func commitProviderAccountDraft( + _ accountID: UUID + ) -> Bool { + guard persistProviderAccountDraftMetadata(accountID) else { + return false + } + finalizeProviderAccountDraft(accountID) return true } @@ -1505,10 +1620,21 @@ public final class AppState: ObservableObject { guard deleteLocalProviderAccountSecrets(config) else { return false } + let remainingConfigs = providerConfigs.filter { + $0.accountID != accountID + } + guard saveProviderConfigMetadata( + remainingConfigs, + requireSharedCredentialOwnerReconciliation: true + ) else { + // Keep the in-memory account as the visible retry anchor. The + // durable outbox (when present) remains pending, and cloud + // deletion must not start until local metadata commits. + return false + } guard providerState.removeProviderAccount(accountID) != nil else { return false } - saveProviderConfigMetadata() buildProviderDetails() if let deletionOwnerID { Task { [weak self] in @@ -1526,7 +1652,9 @@ public final class AppState: ObservableObject { private func deleteLocalProviderAccountSecrets( _ config: ProviderConfig ) -> Bool { - guard config.deleteSecrets(using: providerSecretStore) else { + guard config.deleteSecretsForAccountRemoval( + using: providerSecretStore + ) else { return false } #if os(macOS) @@ -1541,10 +1669,14 @@ public final class AppState: ObservableObject { } #endif if runtimeEnvironment.capabilities.allowsHelperRegistration { - ProviderSharedCredentialOwner.release( - kind: config.kind, - accountID: config.accountID - ) + guard + ProviderSharedCredentialOwner.release( + kind: config.kind, + accountID: config.accountID + ) + else { + return false + } } return true } @@ -1630,9 +1762,15 @@ public final class AppState: ObservableObject { else { return } + let locallyPresentAccountIDs = Set( + providerConfigs.map(\.accountID) + ) let pending = providerAccountDeletionOutbox .pendingIntents(for: expectedUserID) - .filter { $0.provider != nil } + .filter { + $0.provider != nil + && !locallyPresentAccountIDs.contains($0.accountID) + } .prefix(10) for intent in pending { guard @@ -1646,6 +1784,14 @@ public final class AppState: ObservableObject { // local metadata recovery can safely enrich them. continue } + guard !providerConfigs.contains(where: { + $0.accountID == intent.accountID + }) else { + // The preceding delete awaits, so MainActor can restore a + // later account while this batch is in progress. Recheck each + // intent at the last local gate before contacting the server. + continue + } let deleted = await api.deleteProviderAccount( intent.accountID, provider: provider, @@ -1657,10 +1803,7 @@ public final class AppState: ObservableObject { // refresh instead of hammering the endpoint. return } - providerAccountDeletionOutbox.markCompleted( - userID: expectedUserID, - accountID: intent.accountID - ) + providerAccountDeletionOutbox.markCompleted(intent) } } @@ -1972,17 +2115,35 @@ public final class AppState: ObservableObject { /// for enable/order/label changes that must never mutate Keychain state. @discardableResult public func saveProviderConfigMetadata() -> Bool { + saveProviderConfigMetadata( + providerConfigs, + requireSharedCredentialOwnerReconciliation: false + ) + } + + private func saveProviderConfigMetadata( + _ configs: [ProviderConfig], + requireSharedCredentialOwnerReconciliation: Bool + ) -> Bool { let allowsHelperMirror = runtimeEnvironment.capabilities.allowsHelperRegistration if allowsHelperMirror { - ProviderSharedCredentialOwner.reconcile(configs: providerConfigs) + let reconciled = ProviderSharedCredentialOwner.reconcile( + configs: configs + ) + guard + reconciled + || !requireSharedCredentialOwnerReconciliation + else { + return false + } } return ProviderConfigMetadataStore( defaults: providerConfigDefaults, helperDefaults: allowsHelperMirror ? providerConfigHelperDefaults : nil - ).save(providerConfigs) + ).save(configs) } public func buildProviderDetails() { @@ -2162,7 +2323,10 @@ public final class AppState: ObservableObject { /// Finish a local deletion whose durable intent was committed before a /// previous process stopped. Owner matching is mandatory so a queued /// user-A intent can never remove a user-B account after an account switch. - private func recoverPendingLocalProviderAccountDeletions() { + /// Internal so deterministic composition tests can exercise the relaunch + /// recovery transaction without running AppState's production launch + /// migrations or touching the real Keychain. + func recoverPendingLocalProviderAccountDeletions() { let accountIDs = ProviderAccountDeletionRecovery.accountIDsToRemove( from: providerConfigs, @@ -2189,6 +2353,15 @@ public final class AppState: ObservableObject { else { continue } + let remainingConfigs = providerConfigs.filter { + $0.accountID != accountID + } + guard saveProviderConfigMetadata( + remainingConfigs, + requireSharedCredentialOwnerReconciliation: true + ) else { + continue + } _ = providerState.removeProviderAccount(accountID) } } diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeHelperContract.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeHelperContract.swift index 67cf8746..34e0dd43 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeHelperContract.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeHelperContract.swift @@ -79,7 +79,8 @@ public enum ClaudeHelperContract { /// App-group container directory when available to the running process. public static var appGroupHelperDir: String? { - FileManager.default + if ClaudeCredentials.isolatedTestHomeDirectory != nil { return nil } + return FileManager.default .containerURL(forSecurityApplicationGroupIdentifier: appGroupID)? .path } diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeSourceStrategy.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeSourceStrategy.swift index f494952c..c75f6606 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeSourceStrategy.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/Collectors/Claude/ClaudeSourceStrategy.swift @@ -226,9 +226,27 @@ public enum ClaudeResultBuilder { /// Shared helpers for Claude credential resolution, used by multiple strategies. public enum ClaudeCredentials { + private static let isRunningUnderXCTest = NSClassFromString("XCTestCase") != nil + + static var isolatedTestHomeDirectory: String? { + guard isRunningUnderXCTest else { return nil } + if let fixedUserHome = ProcessInfo.processInfo.environment["CFFIXED_USER_HOME"]? + .trimmingCharacters(in: .whitespacesAndNewlines), + !fixedUserHome.isEmpty + { + return fixedUserHome + } + return (NSTemporaryDirectory() as NSString).appendingPathComponent( + "clipulse-xctest-\(ProcessInfo.processInfo.processIdentifier)" + ) + } + /// Real home directory (not sandbox-remapped). Uses the thread-safe - /// `passwdHomeDirectory()` (`getpwuid_r`). + /// `passwdHomeDirectory()` (`getpwuid_r`). XCTest may opt into an isolated + /// home so offline tests never read or overwrite the developer's real + /// Claude credentials and helper snapshots. public static var realHomeDir: String { + if let isolatedTestHomeDirectory { return isolatedTestHomeDirectory } if let dir = passwdHomeDirectory() { return dir } let nsHome = NSHomeDirectory() if let range = nsHome.range(of: "/Library/Containers/") { @@ -280,6 +298,10 @@ public enum ClaudeCredentials { bypassCooldown: Bool = false, cacheResult: Bool = true ) -> Creds? { + // XCTest uses an isolated Claude home and must not fall through to the + // developer's app cache or Claude Code's cross-app Keychain item. + if isolatedTestHomeDirectory != nil { return nil } + // 1. Try the app's own keychain cache (never triggers a prompt) if let cached = KeychainHelper.load(key: keychainCacheKey), let data = cached.data(using: .utf8), @@ -395,7 +417,10 @@ public enum ClaudeCredentials { return creds } - private static let keychainCacheKey = "claude-code-creds-cache" + // Internal so XCTest can seed the exact cache entry without duplicating a + // security-sensitive key name and accidentally falling through to the + // cross-app Keychain path when that name changes. + static let keychainCacheKey = "claude-code-creds-cache" enum TokenSource: Equatable { case accountConfig diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CookieResolver.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CookieResolver.swift index 9ef2900d..ed35fb45 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CookieResolver.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/CookieResolver.swift @@ -30,10 +30,30 @@ public enum CookieResolutionResult: Sendable { } public enum CookieResolver { + /// Unit tests must never inherit the production browser importer. Reading + /// Chrome/Edge cookie stores can cross into the user's login Keychain and + /// present an interactive consent dialog from an XCTest process. + private static let isRunningUnderXCTest: Bool = { + let environment = ProcessInfo.processInfo.environment + let bundlePath = Bundle.main.bundleURL.path + let processName = ProcessInfo.processInfo.processName.lowercased() + + return NSClassFromString("XCTestCase") != nil + || environment["XCTestConfigurationFilePath"] != nil + || environment["XCTestBundlePath"] != nil + || bundlePath.hasSuffix(".xctest") + || bundlePath.contains(".xctest/") + || processName == "xctest" + || processName.hasSuffix("tests.xctest") + }() + /// Platform-appropriate default importer: SweetCookieKit on macOS, /// a no-op everywhere else. public static var platformDefaultImporter: CookieImporting { #if os(macOS) + if isRunningUnderXCTest { + return NullCookieImporter() + } return BrowserCookieAutoImporter.shared #else return NullCookieImporter() diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/DataRefreshManager.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/DataRefreshManager.swift index 72d23ccb..ad2dde6a 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/DataRefreshManager.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/DataRefreshManager.swift @@ -1470,7 +1470,10 @@ internal final class DataRefreshManager { nonisolated static func parseHelperCollectorResults( _ data: Data, providerConfigs: [ProviderConfig], - now: Date = Date() + now: Date = Date(), + sharedCredentialOwner: (ProviderKind) -> ProviderSharedCredentialOwner.Lookup = { + ProviderSharedCredentialOwner.lookup(kind: $0) + } ) -> HelperCollectorSnapshot { guard let decoded = try? HelperIPC.decodeCollectorResults(data, now: now) else { return .empty @@ -1499,12 +1502,35 @@ internal final class DataRefreshManager { ) } let accountResults = providerResults.compactMap { result -> AccountScopedCollectorResult? in - guard let kind = ProviderKind(rawValue: result.usage.provider), - let config = providerConfigs - .filter({ $0.kind == kind && $0.isEnabled }) - .sorted(by: providerConfigComesBefore) - .first - else { return nil } + guard let kind = ProviderKind(rawValue: result.usage.provider) else { + return nil + } + let usesSharedCredentials = kind == .claude || kind == .gemini + let eligible = providerConfigs + .filter { + guard $0.kind == kind && $0.isEnabled else { + return false + } + return !usesSharedCredentials + || $0.sharedCredentialFallbackDisabled != true + } + .sorted(by: providerConfigComesBefore) + let config: ProviderConfig? + + if usesSharedCredentials { + switch sharedCredentialOwner(kind) { + case let .owned(accountID): + config = eligible.first { $0.accountID == accountID } + case .unowned: + config = eligible.count == 1 ? eligible[0] : nil + case .unavailable, .corrupt: + config = nil + } + } else { + config = eligible.count == 1 ? eligible[0] : nil + } + + guard let config else { return nil } return AccountScopedCollectorResult( accountID: config.accountID, config: config, diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/KeychainHelper.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/KeychainHelper.swift index a9a2dda5..d615d184 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/KeychainHelper.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/KeychainHelper.swift @@ -1,6 +1,19 @@ import Foundation import Security +enum ProviderSecretReadResult: Equatable { + case value(String) + case missing + case failure + + var value: String? { + guard case let .value(value) = self else { + return nil + } + return value + } +} + protocol ProviderSecretStoring { @discardableResult func save( @@ -9,10 +22,26 @@ protocol ProviderSecretStoring { accessGroup: String? ) -> Bool func load(key: String, accessGroup: String?) -> String? + func read( + key: String, + accessGroup: String? + ) -> ProviderSecretReadResult @discardableResult func delete(key: String, accessGroup: String?) -> Bool } +extension ProviderSecretStoring { + func read( + key: String, + accessGroup: String? + ) -> ProviderSecretReadResult { + guard let value = load(key: key, accessGroup: accessGroup) else { + return .missing + } + return .value(value) + } +} + struct KeychainProviderSecretStore: ProviderSecretStoring { @discardableResult func save( @@ -31,6 +60,16 @@ struct KeychainProviderSecretStore: ProviderSecretStoring { KeychainHelper.load(key: key, accessGroup: accessGroup) } + func read( + key: String, + accessGroup: String? + ) -> ProviderSecretReadResult { + KeychainHelper.readResult( + key: key, + accessGroup: accessGroup + ) + } + @discardableResult func delete(key: String, accessGroup: String?) -> Bool { KeychainHelper.delete(key: key, accessGroup: accessGroup) @@ -174,7 +213,10 @@ public enum KeychainHelper { return false } - public static func load(key: String, accessGroup: String? = nil) -> String? { + static func readResult( + key: String, + accessGroup: String? = nil + ) -> ProviderSecretReadResult { var query: [String: Any] = [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, @@ -186,12 +228,35 @@ public enum KeychainHelper { query[kSecAttrAccessGroup as String] = group } if isRunningUnderXCTest { - return inMemoryStoreForTesting[testStoreKey(key, accessGroup)] + guard + let value = inMemoryStoreForTesting[ + testStoreKey(key, accessGroup) + ] + else { + return .missing + } + return .value(value) } var item: CFTypeRef? let status = SecItemCopyMatching(query as CFDictionary, &item) - guard status == errSecSuccess, let data = item as? Data else { return nil } - return String(data: data, encoding: .utf8) + if status == errSecItemNotFound { + return .missing + } + guard + status == errSecSuccess, + let data = item as? Data, + let value = String(data: data, encoding: .utf8) + else { + return .failure + } + return .value(value) + } + + public static func load( + key: String, + accessGroup: String? = nil + ) -> String? { + readResult(key: key, accessGroup: accessGroup).value } @discardableResult diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountDeletionOutbox.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountDeletionOutbox.swift index f05f34c8..5b47382a 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountDeletionOutbox.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountDeletionOutbox.swift @@ -15,15 +15,21 @@ final class ProviderAccountDeletionOutbox { /// Every newly enqueued delete records the non-secret provider and /// provider-less legacy intents are never sent to the server. let provider: ProviderKind? + /// Identifies one enqueue operation so a stale in-flight response + /// cannot complete a newer retry for the same owner and account. + /// Optional only so existing v1 queue payloads remain decodable. + let generation: UUID? init( userID: String, accountID: UUID, - provider: ProviderKind? + provider: ProviderKind?, + generation: UUID? = UUID() ) { self.userID = userID self.accountID = accountID self.provider = provider + self.generation = generation } } @@ -114,11 +120,10 @@ final class ProviderAccountDeletionOutbox { } @discardableResult - func markCompleted( - userID: String, - accountID: UUID - ) -> Bool { - guard let owner = normalizedUserID(userID) else { + func markCompleted(_ completedIntent: Intent) -> Bool { + guard + let owner = normalizedUserID(completedIntent.userID) + else { return false } guard case var .valid(records) = @@ -127,9 +132,14 @@ final class ProviderAccountDeletionOutbox { preserveCorruptStorage() return false } - records = Set(records.filter { - $0.userID != owner || $0.accountID != accountID - }) + records.remove( + Intent( + userID: owner, + accountID: completedIntent.accountID, + provider: completedIntent.provider, + generation: completedIntent.generation + ) + ) return saveRecords(records) } @@ -242,8 +252,12 @@ final class ProviderAccountDeletionOutbox { return lhs.accountID.uuidString < rhs.accountID.uuidString } - return (lhs.provider?.rawValue ?? "") - < (rhs.provider?.rawValue ?? "") + if lhs.provider != rhs.provider { + return (lhs.provider?.rawValue ?? "") + < (rhs.provider?.rawValue ?? "") + } + return (lhs.generation?.uuidString ?? "") + < (rhs.generation?.uuidString ?? "") } } diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountSaveTransaction.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountSaveTransaction.swift new file mode 100644 index 00000000..54ac9287 --- /dev/null +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderAccountSaveTransaction.swift @@ -0,0 +1,65 @@ +/// Orders provider-account persistence and compensates every completed phase +/// when a later phase fails. Each persistence primitive must also restore its +/// own partial writes before returning `false`. +public enum ProviderAccountSaveTransactionResult: Equatable { + case committed + case failedRolledBack + case failedRollbackIncomplete +} + +/// Keeps the original rollback closures alive after an incomplete +/// compensation. Callers must clear this gate successfully before capturing a +/// fresh persistence baseline for the same account. +public final class ProviderAccountSaveRecovery { + private let restoreMetadata: () -> Bool + private let restoreSecrets: () -> Bool + + public init( + restoreMetadata: @escaping () -> Bool, + restoreSecrets: @escaping () -> Bool + ) { + self.restoreMetadata = restoreMetadata + self.restoreSecrets = restoreSecrets + } + + @discardableResult + public func recover() -> Bool { + let metadataRestored = restoreMetadata() + let secretsRestored = restoreSecrets() + return metadataRestored && secretsRestored + } +} + +public enum ProviderAccountSaveTransaction { + @discardableResult + public static func commit( + persistSecrets: () -> Bool, + rollbackSecrets: () -> Bool, + persistMetadata: () -> Bool, + rollbackMetadata: () -> Bool, + commitProviderCredential: () -> Bool, + finalize: () -> Void + ) -> ProviderAccountSaveTransactionResult { + guard persistSecrets() else { + return rollbackSecrets() + ? .failedRolledBack + : .failedRollbackIncomplete + } + guard persistMetadata() else { + let metadataRestored = rollbackMetadata() + let secretsRestored = rollbackSecrets() + return metadataRestored && secretsRestored + ? .failedRolledBack + : .failedRollbackIncomplete + } + guard commitProviderCredential() else { + let metadataRestored = rollbackMetadata() + let secretsRestored = rollbackSecrets() + return metadataRestored && secretsRestored + ? .failedRolledBack + : .failedRollbackIncomplete + } + finalize() + return .committed + } +} diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfig.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfig.swift index d885e2d4..4d79112a 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfig.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfig.swift @@ -153,23 +153,71 @@ public struct ProviderConfig: Codable, Identifiable, Sendable { saveSecrets(using: KeychainProviderSecretStore()) } + /// Opaque rollback point for the account-scoped secret entries and their + /// migration markers. The values stay in memory only for the duration of + /// the editor save transaction. + /// + /// Each entry keeps the FULL read result, not just a value, so an entry we + /// could not read is recorded as exactly that rather than collapsing the + /// whole checkpoint. A checkpoint is a rollback *aid*; it must never become + /// a precondition for writing, or one unreadable Keychain item would make + /// an account permanently un-editable and un-deletable — see the header on + /// `makeSecretPersistenceCheckpoint(using:)`. + public struct SecretPersistenceCheckpoint { + fileprivate let apiKey: ProviderSecretReadResult + fileprivate let apiKeyMarker: ProviderSecretReadResult + fileprivate let cookie: ProviderSecretReadResult + fileprivate let cookieMarker: ProviderSecretReadResult + } + + public func makeSecretPersistenceCheckpoint() + -> SecretPersistenceCheckpoint? + { + makeSecretPersistenceCheckpoint( + using: KeychainProviderSecretStore() + ) + } + + @discardableResult + public func restoreSecrets( + from checkpoint: SecretPersistenceCheckpoint + ) -> Bool { + restoreSecrets( + from: checkpoint, + using: KeychainProviderSecretStore() + ) + } + @discardableResult func saveSecrets( using store: any ProviderSecretStoring ) -> Bool { - let apiKeySaved = + guard + let checkpoint = makeSecretPersistenceCheckpoint( + using: store + ) + else { + return false + } + guard persistSecret( apiKey, suffix: "apiKey", using: store - ) - let cookieSaved = + ), persistSecret( manualCookieHeader, suffix: "cookie", using: store ) - return apiKeySaved && cookieSaved + else { + _ = restoreSecrets( + from: checkpoint, + using: store + ) + return false + } + return true } /// Load secrets from Keychain into the in-memory fields. @@ -192,11 +240,171 @@ public struct ProviderConfig: Codable, Identifiable, Sendable { func deleteSecrets( using store: any ProviderSecretStoring ) -> Bool { - let apiKeyDeleted = - persistSecret(nil, suffix: "apiKey", using: store) - let cookieDeleted = + guard + let checkpoint = makeSecretPersistenceCheckpoint( + using: store + ) + else { + return false + } + guard + persistSecret(nil, suffix: "apiKey", using: store), persistSecret(nil, suffix: "cookie", using: store) - return apiKeyDeleted && cookieDeleted + else { + _ = restoreSecrets( + from: checkpoint, + using: store + ) + return false + } + return true + } + + /// Retire every credential entry owned by an explicitly deleted account. + /// + /// `deleteSecrets(using:)` intentionally retains migration markers so a + /// disconnected account cannot resurrect a legacy provider-scoped value. + /// Once the account itself is being removed, those markers would become + /// orphaned. The one account designated as the legacy migration owner must + /// also retire the provider-scoped entries before its retry anchor (the + /// account metadata) can be removed. + @discardableResult + func deleteSecretsForAccountRemoval( + using store: any ProviderSecretStoring + ) -> Bool { + guard deleteSecrets(using: store) else { + return false + } + + let group = Self.secretsAccessGroup + if legacySecretMigrationEligible == true { + for suffix in ["apiKey", "cookie"] { + guard deleteAndConfirmMissing( + key: Self.legacyKeychainKey(kind, suffix), + accessGroup: group, + using: store + ) else { + return false + } + } + } + + for suffix in ["apiKey", "cookie"] { + guard deleteAndConfirmMissing( + key: Self.migrationMarkerKey(accountID, suffix), + accessGroup: group, + using: store + ) else { + return false + } + } + return true + } + + func makeSecretPersistenceCheckpoint( + using store: any ProviderSecretStoring + ) -> SecretPersistenceCheckpoint? { + let group = Self.secretsAccessGroup + let apiKey = store.read( + key: Self.accountKeychainKey(accountID, "apiKey"), + accessGroup: group + ) + let apiKeyMarker = store.read( + key: Self.migrationMarkerKey(accountID, "apiKey"), + accessGroup: group + ) + let cookie = store.read( + key: Self.accountKeychainKey(accountID, "cookie"), + accessGroup: group + ) + let cookieMarker = store.read( + key: Self.migrationMarkerKey(accountID, "cookie"), + accessGroup: group + ) + // Deliberately NOT `guard ... != .failure else { return nil }`. + // + // That is what this used to do, and it was a trap: `readResult` maps + // EVERY OSStatus other than success/itemNotFound to `.failure`, so a + // single unreadable entry — a denied authorization prompt, an ACL + // mismatch after a re-sign or a MAS/Developer-ID channel switch, a + // missing entitlement, a non-UTF8 payload — made this return nil, and + // both `saveSecrets` and `deleteSecrets` guarded on it. The account + // could then be neither overwritten nor removed: fail-closed had become + // fail-forever, with no route back for the user. Reproduced against the + // real Keychain by planting a non-UTF8 item. + // + // An entry we cannot read has no known previous value, so there is + // nothing to preserve and nothing a caller could usefully refuse over. + // Record it as `.failure` and let `restoreSecrets` skip it. + return SecretPersistenceCheckpoint( + apiKey: apiKey, + apiKeyMarker: apiKeyMarker, + cookie: cookie, + cookieMarker: cookieMarker + ) + } + + @discardableResult + func restoreSecrets( + from checkpoint: SecretPersistenceCheckpoint, + using store: any ProviderSecretStoring + ) -> Bool { + let group = Self.secretsAccessGroup + let entries: [(String, ProviderSecretReadResult)] = [ + ( + Self.accountKeychainKey(accountID, "apiKey"), + checkpoint.apiKey + ), + ( + Self.migrationMarkerKey(accountID, "apiKey"), + checkpoint.apiKeyMarker + ), + ( + Self.accountKeychainKey(accountID, "cookie"), + checkpoint.cookie + ), + ( + Self.migrationMarkerKey(accountID, "cookie"), + checkpoint.cookieMarker + ), + ] + var restored = true + for (key, captured) in entries { + let entryRestored: Bool + switch captured { + case let .value(value): + entryRestored = + store.save( + key: key, + value: value, + accessGroup: group + ) + && store.read( + key: key, + accessGroup: group + ) == .value(value) + case .missing: + entryRestored = + store.delete( + key: key, + accessGroup: group + ) + && store.read( + key: key, + accessGroup: group + ) == .missing + case .failure: + // The entry was already unreadable when the checkpoint was + // taken, so there is no prior state to put back. Skipping is + // the only honest option: writing would invent a value and + // deleting would destroy one we never managed to see. It does + // NOT count against the rollback — we are no worse off here + // than before the attempt. + entryRestored = true + } + restored = entryRestored && restored + } + return restored } @discardableResult @@ -217,7 +425,12 @@ public struct ProviderConfig: Codable, Identifiable, Sendable { ) else { return false } - guard store.load(key: accountKey, accessGroup: group) == value else { + guard + store.read( + key: accountKey, + accessGroup: group + ) == .value(value) + else { return false } } else { @@ -226,10 +439,10 @@ public struct ProviderConfig: Codable, Identifiable, Sendable { key: accountKey, accessGroup: group ), - store.load( + store.read( key: accountKey, accessGroup: group - ) == nil + ) == .missing else { return false } @@ -242,10 +455,27 @@ public struct ProviderConfig: Codable, Identifiable, Sendable { ) else { return false } - return store.load( + return store.read( key: markerKey, accessGroup: group - ) == "1" + ) == .value("1") + } + + private func deleteAndConfirmMissing( + key: String, + accessGroup: String?, + using store: any ProviderSecretStoring + ) -> Bool { + guard store.delete( + key: key, + accessGroup: accessGroup + ) else { + return false + } + return store.read( + key: key, + accessGroup: accessGroup + ) == .missing } private func loadSecret( diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfigMetadataStore.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfigMetadataStore.swift index 4388e9de..3bee3194 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfigMetadataStore.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderConfigMetadataStore.swift @@ -5,6 +5,13 @@ import Foundation /// `ProviderConfig.saveSecrets()` explicitly; passive onboarding completion /// uses this store so a transient Keychain read failure cannot erase a secret. public struct ProviderConfigMetadataStore { + struct PersistenceCheckpoint { + let appConfigs: Data? + let appSchemaVersion: Int? + let helperConfigs: Data? + let helperWriteV2: Bool? + } + private let defaults: UserDefaults private let helperDefaults: UserDefaults? private let encoder: JSONEncoder @@ -26,6 +33,7 @@ public struct ProviderConfigMetadataStore { guard let data = try? encoder.encode(configs) else { return false } + let checkpoint = makePersistenceCheckpoint() defaults.set( data, @@ -43,6 +51,7 @@ public struct ProviderConfigMetadataStore { forKey: ProviderAccountMigration.schemaVersionKey ) == ProviderAccountMigration.currentSchemaVersion else { + _ = restore(checkpoint) return false } helperDefaults?.set( @@ -68,9 +77,166 @@ public struct ProviderConfigMetadataStore { .writeDefaultsKey ) else { + _ = restore(checkpoint) return false } } return true } + + func makePersistenceCheckpoint() -> PersistenceCheckpoint { + PersistenceCheckpoint( + appConfigs: defaults.data( + forKey: ProviderAccountMigration.configsKey + ), + appSchemaVersion: + defaults.object( + forKey: + ProviderAccountMigration.schemaVersionKey + ) == nil + ? nil + : defaults.integer( + forKey: + ProviderAccountMigration.schemaVersionKey + ), + helperConfigs: helperDefaults?.data( + forKey: HelperIPC.providerConfigsKey + ), + helperWriteV2: + helperDefaults?.object( + forKey: + HelperIPC.providerAccountsWriteV2Key + ) == nil + ? nil + : helperDefaults?.bool( + forKey: + HelperIPC.providerAccountsWriteV2Key + ) + ) + } + + @discardableResult + func restore( + _ checkpoint: PersistenceCheckpoint + ) -> Bool { + restore( + checkpoint.appConfigs, + in: defaults, + forKey: ProviderAccountMigration.configsKey + ) + restore( + checkpoint.appSchemaVersion, + in: defaults, + forKey: ProviderAccountMigration.schemaVersionKey + ) + if let helperDefaults { + restore( + checkpoint.helperConfigs, + in: helperDefaults, + forKey: HelperIPC.providerConfigsKey + ) + restore( + checkpoint.helperWriteV2, + in: helperDefaults, + forKey: HelperIPC.providerAccountsWriteV2Key + ) + } + + let appRestored = + defaults.data( + forKey: ProviderAccountMigration.configsKey + ) == checkpoint.appConfigs + && optionalInteger( + defaults, + key: ProviderAccountMigration.schemaVersionKey + ) == checkpoint.appSchemaVersion + let helperRestored: Bool + if let helperDefaults { + helperRestored = + helperDefaults.data( + forKey: HelperIPC.providerConfigsKey + ) == checkpoint.helperConfigs + && optionalBool( + helperDefaults, + key: HelperIPC.providerAccountsWriteV2Key + ) == checkpoint.helperWriteV2 + } else { + helperRestored = true + } + return appRestored && helperRestored + } + + private func restore( + _ value: Any?, + in defaults: UserDefaults, + forKey key: String + ) { + if let value { + defaults.set(value, forKey: key) + } else { + defaults.removeObject(forKey: key) + } + } + + private func optionalInteger( + _ defaults: UserDefaults, + key: String + ) -> Int? { + defaults.object(forKey: key) == nil + ? nil + : defaults.integer(forKey: key) + } + + private func optionalBool( + _ defaults: UserDefaults, + key: String + ) -> Bool? { + defaults.object(forKey: key) == nil + ? nil + : defaults.bool(forKey: key) + } +} + +/// Opaque rollback point spanning both provider metadata copies and the +/// provider-global compatibility owner records. +public final class ProviderAccountPersistenceCheckpoint { + private let metadataStore: ProviderConfigMetadataStore + private let metadataCheckpoint: + ProviderConfigMetadataStore.PersistenceCheckpoint + private let ownerCheckpoint: + ProviderSharedCredentialOwner.PersistenceCheckpoint? + private let restoresOwner: Bool + + init( + metadataStore: ProviderConfigMetadataStore, + metadataCheckpoint: + ProviderConfigMetadataStore.PersistenceCheckpoint, + ownerCheckpoint: + ProviderSharedCredentialOwner.PersistenceCheckpoint?, + restoresOwner: Bool + ) { + self.metadataStore = metadataStore + self.metadataCheckpoint = metadataCheckpoint + self.ownerCheckpoint = ownerCheckpoint + self.restoresOwner = restoresOwner + } + + @discardableResult + public func restore() -> Bool { + let metadataRestored = metadataStore.restore( + metadataCheckpoint + ) + let ownerRestored: Bool + if restoresOwner { + guard let ownerCheckpoint else { + return false + } + ownerRestored = ProviderSharedCredentialOwner.restore( + ownerCheckpoint + ) + } else { + ownerRestored = true + } + return metadataRestored && ownerRestored + } } diff --git a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderSharedCredentialOwner.swift b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderSharedCredentialOwner.swift index 4830143d..8157b2ae 100644 --- a/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderSharedCredentialOwner.swift +++ b/CLI Pulse Bar/CLIPulseCore/Sources/CLIPulseCore/ProviderSharedCredentialOwner.swift @@ -21,6 +21,15 @@ enum ProviderSharedCredentialOwner { ] private static let lock = NSLock() + struct PersistenceCheckpoint { + fileprivate struct Entry { + let kind: ProviderKind + let rawValue: String? + } + + fileprivate let entries: [Entry] + } + /// Injectable for focused tests. The production value is shared with the /// helper so both processes make the same legacy-account assignment. static var defaults: UserDefaults? = @@ -48,6 +57,13 @@ enum ProviderSharedCredentialOwner { return false } _ = synchronizeDefaults(defaults) + guard + let checkpoint = persistenceCheckpoint( + defaults: defaults + ) + else { + return false + } let kinds = supportedKinds.sorted { $0.rawValue < $1.rawValue } @@ -107,6 +123,10 @@ enum ProviderSharedCredentialOwner { forKey: key ) else { + _ = restore( + checkpoint, + defaults: defaults + ) return false } } else { @@ -117,6 +137,10 @@ enum ProviderSharedCredentialOwner { forKey: key ) else { + _ = restore( + checkpoint, + defaults: defaults + ) return false } } @@ -126,6 +150,46 @@ enum ProviderSharedCredentialOwner { } } + static func makePersistenceCheckpoint() + -> PersistenceCheckpoint? + { + withMutationLock(or: nil) { + lock.withLock { + guard let defaults else { + return nil + } + _ = synchronizeDefaults(defaults) + return persistenceCheckpoint( + defaults: defaults + ) + } + } + } + + @discardableResult + static func restore( + _ checkpoint: PersistenceCheckpoint + ) -> Bool { + withMutationLock(or: false) { + lock.withLock { + guard let defaults else { + return false + } + return restore( + checkpoint, + defaults: defaults + ) + } + } + } + + static func withPersistenceLock( + or failure: T, + _ body: () -> T + ) -> T { + withMutationLock(or: failure, body) + } + /// Returns true for the existing owner, or atomically claims an unowned /// global source. Production refreshes call `reconcile` before fan-out, /// while this fallback keeps direct strategy calls deterministic. @@ -263,8 +327,10 @@ enum ProviderSharedCredentialOwner { kind: ProviderKind, accountID: UUID ) -> Bool { + // Idempotent cleanup: providers without a shared compatibility source + // have no owner record to release. guard supportedKinds.contains(kind) else { - return false + return true } return withMutationLock(or: false) { lock.withLock { @@ -337,6 +403,54 @@ enum ProviderSharedCredentialOwner { return true } + private static func persistenceCheckpoint( + defaults: UserDefaults + ) -> PersistenceCheckpoint? { + let kinds = supportedKinds.sorted { + $0.rawValue < $1.rawValue + } + var entries: [PersistenceCheckpoint.Entry] = [] + for kind in kinds { + let raw = defaults.object( + forKey: ownerKey(for: kind) + ) + guard raw == nil || raw is String else { + return nil + } + entries.append( + PersistenceCheckpoint.Entry( + kind: kind, + rawValue: raw as? String + ) + ) + } + return PersistenceCheckpoint(entries: entries) + } + + private static func restore( + _ checkpoint: PersistenceCheckpoint, + defaults: UserDefaults + ) -> Bool { + for entry in checkpoint.entries { + let key = ownerKey(for: entry.kind) + if let rawValue = entry.rawValue { + defaults.set(rawValue, forKey: key) + } else { + defaults.removeObject(forKey: key) + } + } + guard synchronizeDefaults(defaults) else { + return false + } + return checkpoint.entries.allSatisfy { entry in + let key = ownerKey(for: entry.kind) + if let rawValue = entry.rawValue { + return defaults.string(forKey: key) == rawValue + } + return defaults.object(forKey: key) == nil + } + } + private static func ownerKey(for kind: ProviderKind) -> String { "cli_pulse_provider_shared_credential_owner_\(kind.rawValue)" } diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/AppStateProviderAccountSyncTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/AppStateProviderAccountSyncTests.swift index ecca9d90..bfba8784 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/AppStateProviderAccountSyncTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/AppStateProviderAccountSyncTests.swift @@ -96,6 +96,173 @@ final class AppStateProviderAccountSyncTests: XCTestCase { ) } + func testFailedLocalMetadataRemovalIsNotFlushedToCloud() + async throws + { + let removalSuiteName = + "AppStateProviderAccountSyncTests.metadata-failure.\(UUID())" + let removalDefaults = try XCTUnwrap( + DroppingUserDefaults(suiteName: removalSuiteName) + ) + defer { + removalDefaults.removePersistentDomain( + forName: removalSuiteName + ) + } + let accountID = try XCTUnwrap( + UUID( + uuidString: + "12121212-1212-4212-8212-121212121212" + ) + ) + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + syncOwnerUserID: "user-a" + ) + XCTAssertTrue( + ProviderConfigMetadataStore( + defaults: removalDefaults, + helperDefaults: nil + ).save([config]) + ) + let outbox = ProviderAccountDeletionOutbox( + defaults: removalDefaults, + storageKey: + "AppStateProviderAccountSyncTests.metadata-failure.outbox" + ) + AppStateProviderAccountStubProtocol.configure( + responses: [ + .json( + #"{"accounts_deleted":1,"tombstones_persisted":1}"# + ), + ] + ) + let runtime = + CLIPulseRuntimeEnvironment.resolveForTesting( + infoDictionary: [ + "CFBundleIdentifier": + "tests.clipulse.failed-local-removal", + ], + environment: [:] + ) + let state = AppState( + runtimeEnvironment: runtime, + defaults: removalDefaults, + providerSecretStore: MemoryProviderSecretStore(), + api: await makeAuthenticatedAPI(), + providerAccountDeletionOutbox: outbox, + performLaunchSetup: false + ) + state.isAuthenticated = true + state.userId = "user-a" + state.providerConfigs = [config] + removalDefaults.droppedSetKeys.insert( + ProviderAccountMigration.configsKey + ) + + XCTAssertFalse(state.removeProviderAccount(accountID)) + await state.flushPendingProviderAccountDeletions( + for: "user-a" + ) + + XCTAssertTrue( + AppStateProviderAccountStubProtocol.recordedRequests() + .isEmpty, + "a prepared intent must not reach the server before local metadata commits" + ) + XCTAssertEqual( + outbox.pendingAccountIDs(for: "user-a"), + [accountID] + ) + } + + func testRestoredAccountIsRecheckedBeforeEachBatchedCloudDelete() + async throws + { + let firstAccountID = try XCTUnwrap( + UUID( + uuidString: + "10101010-1010-4010-8010-101010101010" + ) + ) + let restoredAccountID = try XCTUnwrap( + UUID( + uuidString: + "20202020-2020-4020-8020-202020202020" + ) + ) + let firstRequestStarted = expectation( + description: "first batched delete started" + ) + let firstResponseGate = DispatchSemaphore(value: 0) + AppStateProviderAccountStubProtocol.configure( + responses: [ + .json( + #"{"accounts_deleted":1,"tombstones_persisted":1}"#, + responseGate: firstResponseGate + ), + .json( + #"{"accounts_deleted":1,"tombstones_persisted":1}"# + ), + ], + onRequest: { request in + if AppStateProviderAccountStubProtocol + .recordedRequests().count == 1 + { + firstRequestStarted.fulfill() + } + } + ) + let outbox = makeOutbox() + XCTAssertTrue( + outbox.enqueue( + userID: "user-a", + accountID: firstAccountID, + provider: .claude + ) + ) + XCTAssertTrue( + outbox.enqueue( + userID: "user-a", + accountID: restoredAccountID, + provider: .codex + ) + ) + let state = makeState( + api: await makeAuthenticatedAPI(), + outbox: outbox, + userID: "user-a" + ) + + let flush = Task { @MainActor in + await state.flushPendingProviderAccountDeletions( + for: "user-a" + ) + } + await fulfillment(of: [firstRequestStarted], timeout: 3) + state.providerConfigs = [ + ProviderConfig( + kind: .codex, + accountID: restoredAccountID, + syncOwnerUserID: "user-a" + ), + ] + firstResponseGate.signal() + await flush.value + + XCTAssertEqual( + AppStateProviderAccountStubProtocol.recordedRequests() + .count, + 1, + "each awaited delete must recheck whether its account became local again" + ) + XCTAssertEqual( + outbox.pendingAccountIDs(for: "user-a"), + [restoredAccountID] + ) + } + func testProviderlessLegacyIntentsDoNotStarveTypedDeletion() async throws { @@ -284,6 +451,69 @@ final class AppStateProviderAccountSyncTests: XCTestCase { ) } + func testStaleSuccessfulDeleteDoesNotClearNewerIntentForSameAccount() + async throws + { + let accountID = try XCTUnwrap( + UUID( + uuidString: + "34343434-3434-4434-8434-343434343434" + ) + ) + let requestStarted = expectation( + description: "original delete request started" + ) + let responseGate = DispatchSemaphore(value: 0) + AppStateProviderAccountStubProtocol.configure( + responses: [ + .json( + #"{"accounts_deleted":1,"tombstones_persisted":1}"#, + responseGate: responseGate + ), + ], + onRequest: { _ in requestStarted.fulfill() } + ) + let outbox = makeOutbox() + XCTAssertTrue( + outbox.enqueue( + userID: "user-a", + accountID: accountID, + provider: .claude + ) + ) + let state = makeState( + api: await makeAuthenticatedAPI(), + outbox: outbox, + userID: "user-a" + ) + + let flush = Task { @MainActor in + await state.flushPendingProviderAccountDeletions( + for: "user-a" + ) + } + await fulfillment(of: [requestStarted], timeout: 3) + XCTAssertTrue( + outbox.enqueue( + userID: "user-a", + accountID: accountID, + provider: .claude + ), + "a retry while the original request is in flight must create a new durable intent" + ) + responseGate.signal() + await flush.value + + let pending = outbox.pendingIntents(for: "user-a") + XCTAssertEqual( + pending.count, + 1, + "an old response must not clear a newer retry authority" + ) + XCTAssertEqual(pending.first?.accountID, accountID) + XCTAssertEqual(pending.first?.provider, .claude) + } + func testStatusSyncFiltersCurrentOwnerAndDoesNotRetryStaleLeaseWithNewSession() async throws { diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ClaudeStrategyTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ClaudeStrategyTests.swift index f3fd6367..27883868 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ClaudeStrategyTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ClaudeStrategyTests.swift @@ -1,9 +1,101 @@ #if os(macOS) import XCTest @testable import CLIPulseCore +import Darwin final class ClaudeStrategyTests: XCTestCase { + func testRealHomeDirDefaultsToProcessIsolatedTemporaryHomeUnderXCTest() { + let key = "CFFIXED_USER_HOME" + let previousValue = getenv(key).map { String(cString: $0) } + _ = unsetenv(key) + defer { + if let previousValue { + _ = setenv(key, previousValue, 1) + } + } + + let expectedSuffix = "clipulse-xctest-\(ProcessInfo.processInfo.processIdentifier)" + XCTAssertTrue(ClaudeCredentials.realHomeDir.hasPrefix(NSTemporaryDirectory())) + XCTAssertTrue(ClaudeCredentials.realHomeDir.hasSuffix(expectedSuffix)) + } + + func testRealHomeDirUsesExplicitFixedHomeUnderXCTest() { + let key = "CFFIXED_USER_HOME" + let previousValue = getenv(key).map { String(cString: $0) } + let isolatedHome = NSTemporaryDirectory() + + "clipulse-claude-home-\(UUID().uuidString)" + + XCTAssertEqual(setenv(key, isolatedHome, 1), 0) + defer { + if let previousValue { + _ = setenv(key, previousValue, 1) + } else { + _ = unsetenv(key) + } + } + + XCTAssertEqual(ClaudeCredentials.realHomeDir, isolatedHome) + } + + func testFixedTestHomeExcludesTheRealAppGroupContainer() { + let key = "CFFIXED_USER_HOME" + let previousValue = getenv(key).map { String(cString: $0) } + let isolatedHome = NSTemporaryDirectory() + + "clipulse-claude-home-\(UUID().uuidString)" + + XCTAssertEqual(setenv(key, isolatedHome, 1), 0) + defer { + if let previousValue { + _ = setenv(key, previousValue, 1) + } else { + _ = unsetenv(key) + } + } + + XCTAssertEqual( + ClaudeHelperContract.helperDirCandidates, + [(isolatedHome as NSString).appendingPathComponent(".clipulse")] + ) + } + + func testKeychainCredentialReadIsDisabledUnderXCTest() { + let cacheKey = ClaudeCredentials.keychainCacheKey + let previousCachedCredentials = KeychainHelper.load(key: cacheKey) + let cachedCredentials = """ + { + "claude_ai_oauth": { + "access_token": "sk-ant-oat-test", + "rate_limit_tier": "pro" + } + } + """ + + XCTAssertTrue( + KeychainHelper.save(key: cacheKey, value: cachedCredentials) + ) + defer { + if let previousCachedCredentials { + XCTAssertTrue( + KeychainHelper.save( + key: cacheKey, + value: previousCachedCredentials + ) + ) + } else { + XCTAssertTrue(KeychainHelper.delete(key: cacheKey)) + } + } + + XCTAssertNil( + ClaudeCredentials.readKeychainCredentials( + bypassCooldown: true, + cacheResult: false + ), + "XCTest must not consult either the app cache or Claude Code's cross-app Keychain item" + ) + } + // MARK: - ClaudeSnapshot → CollectorResult func testResultBuilderFullSnapshot() { diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/CookieResolverTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/CookieResolverTests.swift index e52a4220..083f8b91 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/CookieResolverTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/CookieResolverTests.swift @@ -90,4 +90,11 @@ final class CookieResolverTests: XCTestCase { importer: NullCookieImporter()) if case .unavailable = result {} else { XCTFail("NullCookieImporter must yield .unavailable") } } + + func test_platform_default_importer_is_null_in_xctest_runtime() { + XCTAssertTrue( + CookieResolver.platformDefaultImporter is NullCookieImporter, + "xctest must never receive the importer that reads real browser cookies or Keychain entries" + ) + } } diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/HelperProviderAccountsIPCTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/HelperProviderAccountsIPCTests.swift index 16a7d959..d647a5a5 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/HelperProviderAccountsIPCTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/HelperProviderAccountsIPCTests.swift @@ -33,7 +33,8 @@ final class HelperProviderAccountsIPCTests: XCTestCase { let snapshot = DataRefreshManager.parseHelperCollectorResults( data, providerConfigs: [config], - now: now + now: now, + sharedCredentialOwner: { _ in .unowned } ) XCTAssertEqual(snapshot.providerResults.count, 1) @@ -42,6 +43,110 @@ final class HelperProviderAccountsIPCTests: XCTestCase { XCTAssertEqual(snapshot.accountResults.first?.result.usage.remaining, 64) } + func testV1SharedProviderMapsToPersistedOwnerInsteadOfFirstSortOrder() throws { + let firstID = try XCTUnwrap(UUID(uuidString: "12121212-1212-4212-8212-121212121212")) + let ownerID = try XCTUnwrap(UUID(uuidString: "34343434-3434-4434-8434-343434343434")) + let configs = [ + ProviderConfig(kind: .claude, accountID: firstID, isEnabled: true, sortOrder: 0), + ProviderConfig(kind: .claude, accountID: ownerID, isEnabled: true, sortOrder: 1), + ] + + let snapshot = DataRefreshManager.parseHelperCollectorResults( + makeV1Data(provider: .claude), + providerConfigs: configs, + now: now, + sharedCredentialOwner: { kind in + kind == .claude ? .owned(ownerID) : .unowned + } + ) + + XCTAssertEqual(snapshot.accountResults.count, 1) + XCTAssertEqual(snapshot.accountResults.first?.accountID, ownerID) + } + + func testV1SharedProviderDoesNotGuessBetweenUnownedAccounts() throws { + let firstID = try XCTUnwrap(UUID(uuidString: "56565656-5656-4656-8656-565656565656")) + let secondID = try XCTUnwrap(UUID(uuidString: "78787878-7878-4878-8878-787878787878")) + let configs = [ + ProviderConfig(kind: .claude, accountID: firstID, isEnabled: true, sortOrder: 0), + ProviderConfig(kind: .claude, accountID: secondID, isEnabled: true, sortOrder: 1), + ] + + let snapshot = DataRefreshManager.parseHelperCollectorResults( + makeV1Data(provider: .claude), + providerConfigs: configs, + now: now, + sharedCredentialOwner: { _ in .unowned } + ) + + XCTAssertTrue(snapshot.accountResults.isEmpty) + XCTAssertEqual(snapshot.providerResults.count, 1) + } + + func testV1SharedProviderFailsClosedForInvalidOwnerState() throws { + let accountID = try XCTUnwrap(UUID(uuidString: "89898989-8989-4989-8989-898989898989")) + let staleOwnerID = try XCTUnwrap(UUID(uuidString: "90909090-9090-4090-8090-909090909090")) + let config = ProviderConfig(kind: .claude, accountID: accountID, isEnabled: true) + let invalidStates: [ProviderSharedCredentialOwner.Lookup] = [ + .unavailable, + .corrupt, + .owned(staleOwnerID), + ] + + for ownerState in invalidStates { + let snapshot = DataRefreshManager.parseHelperCollectorResults( + makeV1Data(provider: .claude), + providerConfigs: [config], + now: now, + sharedCredentialOwner: { _ in ownerState } + ) + + XCTAssertTrue( + snapshot.accountResults.isEmpty, + "owner state \(ownerState) must not be guessed" + ) + XCTAssertEqual(snapshot.providerResults.count, 1) + } + } + + func testV1SharedProviderRejectsFallbackDisabledAccount() throws { + let accountID = try XCTUnwrap(UUID(uuidString: "93939393-9393-4393-8393-939393939393")) + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + isEnabled: true, + sharedCredentialFallbackDisabled: true + ) + + let snapshot = DataRefreshManager.parseHelperCollectorResults( + makeV1Data(provider: .claude), + providerConfigs: [config], + now: now, + sharedCredentialOwner: { _ in .owned(accountID) } + ) + + XCTAssertTrue(snapshot.accountResults.isEmpty) + XCTAssertEqual(snapshot.providerResults.count, 1) + } + + func testV1NonSharedProviderDoesNotGuessBetweenAccounts() throws { + let firstID = try XCTUnwrap(UUID(uuidString: "91919191-9191-4191-8191-919191919191")) + let secondID = try XCTUnwrap(UUID(uuidString: "92929292-9292-4292-8292-929292929292")) + let configs = [ + ProviderConfig(kind: .codex, accountID: firstID, isEnabled: true, sortOrder: 0), + ProviderConfig(kind: .codex, accountID: secondID, isEnabled: true, sortOrder: 1), + ] + + let snapshot = DataRefreshManager.parseHelperCollectorResults( + makeV1Data(provider: .codex), + providerConfigs: configs, + now: now + ) + + XCTAssertTrue(snapshot.accountResults.isEmpty) + XCTAssertEqual(snapshot.providerResults.count, 1) + } + func testV1DisabledProviderProjectionIsRejected() throws { let accountID = try XCTUnwrap(UUID(uuidString: "77777777-7777-4777-8777-777777777777")) let data = Data(""" @@ -506,5 +611,20 @@ final class HelperProviderAccountsIPCTests: XCTestCase { metadata: nil ) } + + private func makeV1Data(provider: ProviderKind) -> Data { + Data(""" + { + "timestamp": "2026-03-21T04:00:00Z", + "providers": { + "\(provider.rawValue)": { + "quota": 100, + "remaining": 64, + "tiers": [] + } + } + } + """.utf8) + } } #endif diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountDeletionOutboxTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountDeletionOutboxTests.swift index c0001e99..a323f530 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountDeletionOutboxTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountDeletionOutboxTests.swift @@ -90,12 +90,12 @@ final class ProviderAccountDeletionOutboxTests: XCTestCase { accountID: account, provider: .codex ) - - outbox.markCompleted( - userID: "user-a", - accountID: account + let userAIntent = try XCTUnwrap( + outbox.pendingIntents(for: "user-a").first ) + outbox.markCompleted(userAIntent) + XCTAssertTrue( outbox.pendingAccountIDs(for: "user-a").isEmpty ) @@ -134,6 +134,10 @@ final class ProviderAccountDeletionOutboxTests: XCTestCase { restored.pendingIntents().first?.provider, .claude ) + XCTAssertNotNil( + restored.pendingIntents().first?.generation, + "new intent identity must survive process recreation" + ) } func testProviderlessV1IntentDecodesAndCanBeSafelyEnriched() @@ -160,6 +164,10 @@ final class ProviderAccountDeletionOutboxTests: XCTestCase { ) XCTAssertNil(outbox.pendingIntents().first?.provider) + XCTAssertNil( + outbox.pendingIntents().first?.generation, + "an older payload without generation must remain decodable" + ) XCTAssertTrue( outbox.enqueue( userID: "user-a", @@ -172,6 +180,7 @@ final class ProviderAccountDeletionOutboxTests: XCTestCase { outbox.pendingIntents().first?.provider, .claude ) + XCTAssertNotNil(outbox.pendingIntents().first?.generation) } func testPersistedIntentRecoversLocalConfigAfterCrash() throws { diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountKeychainMigrationTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountKeychainMigrationTests.swift index bbd74e84..3fe187e5 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountKeychainMigrationTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountKeychainMigrationTests.swift @@ -247,6 +247,592 @@ final class ProviderAccountKeychainMigrationTests: XCTestCase { ) } + func testSaveSecretsRestoresBothEntriesWhenSecondWriteFails() + throws + { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "ABABABAB-ABAB-4BAB-8BAB-ABABABABABAB" + ) + ) + let original = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "old-api-key", + manualCookieHeader: "old-cookie" + ) + XCTAssertTrue(original.saveSecrets(using: store)) + store.failingSaveAttemptsByKey[ + accountKey(accountID, "cookie") + ] = 1 + let edited = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "new-api-key", + manualCookieHeader: "new-cookie" + ) + + XCTAssertFalse(edited.saveSecrets(using: store)) + XCTAssertEqual( + store.load( + key: accountKey(accountID, "apiKey"), + accessGroup: ProviderConfig.secretsAccessGroup + ), + "old-api-key" + ) + XCTAssertEqual( + store.load( + key: accountKey(accountID, "cookie"), + accessGroup: ProviderConfig.secretsAccessGroup + ), + "old-cookie" + ) + } + + /// An unreadable entry must NOT collapse the checkpoint. + /// + /// Replaces `testSaveSecretsFailsWithoutMutationWhenCheckpointReadFails`, + /// which pinned the opposite promise — "if any entry cannot be read, refuse + /// to write at all". That promise was the bug. `readResult` maps every + /// OSStatus other than success/itemNotFound to `.failure`, so one denied + /// authorization prompt, ACL mismatch after a re-sign, missing entitlement + /// or non-UTF8 payload made `makeSecretPersistenceCheckpoint` return nil — + /// and both `saveSecrets` and `deleteSecrets` guarded on it, leaving the + /// account neither editable nor removable with no route back for the user. + /// Reproduced against the real Keychain by planting a non-UTF8 item. + /// + /// The trade this encodes: an entry we could never read cannot be rolled + /// back either, because its previous value was never observed. Readable + /// entries are still rolled back exactly as before. + func testCheckpointSurvivesAnUnreadableEntry() throws { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "ACACACAC-ACAC-4CAC-8CAC-ACACACACACAC" + ) + ) + let original = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "old-api-key", + manualCookieHeader: "old-cookie" + ) + XCTAssertTrue(original.saveSecrets(using: store)) + let apiKey = accountKey(accountID, "apiKey") + store.failingLoadKeys.insert(apiKey) + + XCTAssertNotNil( + original.makeSecretPersistenceCheckpoint(using: store), + "an unreadable entry must be recorded, not turned into a refusal" + ) + + let edited = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "new-api-key", + manualCookieHeader: "new-cookie" + ) + // This double fails reads for that key permanently, so the write-back + // verification inside `persistSecret` can never succeed and the save + // still reports failure. What matters here is what happens to the OTHER + // entry while that is going on. + XCTAssertFalse(edited.saveSecrets(using: store)) + + XCTAssertEqual( + store.load( + key: accountKey(accountID, "cookie"), + accessGroup: ProviderConfig.secretsAccessGroup + ), + "old-cookie", + "the readable entry must still be rolled back" + ) + } + + /// The rollback path itself: a `.failure` entry is skipped rather than + /// invented or destroyed, and skipping it does not count as a failed + /// rollback — we end up no worse off than before the attempt. + func testRestoreSkipsUnreadableEntryAndStillReportsSuccess() throws { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "ADADADAD-ADAD-4DAD-8DAD-ADADADADADAD" + ) + ) + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "seed-api-key", + manualCookieHeader: "seed-cookie" + ) + XCTAssertTrue(config.saveSecrets(using: store)) + + store.failingLoadKeys.insert(accountKey(accountID, "apiKey")) + let checkpoint = try XCTUnwrap( + config.makeSecretPersistenceCheckpoint(using: store) + ) + store.failingLoadKeys.removeAll() + + XCTAssertTrue( + store.save( + key: accountKey(accountID, "cookie"), + value: "drifted-cookie", + accessGroup: ProviderConfig.secretsAccessGroup + ) + ) + + XCTAssertTrue( + config.restoreSecrets(from: checkpoint, using: store), + "skipping an unreadable entry must not be reported as a failed rollback" + ) + XCTAssertEqual( + store.load( + key: accountKey(accountID, "cookie"), + accessGroup: ProviderConfig.secretsAccessGroup + ), + "seed-cookie", + "the readable entry is restored" + ) + } + + func testRestoreMissingSecretReportsReadFailureAfterDelete() + throws + { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "ADADADAD-ADAD-4DAD-8DAD-ADADADADADAD" + ) + ) + let config = ProviderConfig( + kind: .claude, + accountID: accountID + ) + let checkpoint = try XCTUnwrap( + config.makeSecretPersistenceCheckpoint(using: store) + ) + let apiKey = accountKey(accountID, "apiKey") + XCTAssertTrue( + store.save( + key: apiKey, + value: "must-be-removed", + accessGroup: ProviderConfig.secretsAccessGroup + ) + ) + store.failingLoadKeys.insert(apiKey) + + XCTAssertFalse( + config.restoreSecrets( + from: checkpoint, + using: store + ), + "a read failure must not verify deletion as successful" + ) + + store.failingLoadKeys.remove(apiKey) + XCTAssertNil( + store.load( + key: apiKey, + accessGroup: ProviderConfig.secretsAccessGroup + ) + ) + } + + @MainActor + func testRemovingMigrationOwnerDeletesLegacySlotsAndOnlyItsAccountSecrets() + throws + { + let store = InMemoryProviderSecretStore() + let ownerID = try XCTUnwrap( + UUID( + uuidString: + "AEAEAEAE-AEAE-4EAE-8EAE-AEAEAEAEAEAE" + ) + ) + let siblingID = try XCTUnwrap( + UUID( + uuidString: + "AFAFAFAF-AFAF-4FAF-8FAF-AFAFAFAFAFAF" + ) + ) + let group = ProviderConfig.secretsAccessGroup + let owner = ProviderConfig( + kind: .claude, + accountID: ownerID, + apiKey: "owner-api-key", + manualCookieHeader: "owner-cookie", + legacySecretMigrationEligible: true + ) + let sibling = ProviderConfig( + kind: .claude, + accountID: siblingID, + apiKey: "sibling-api-key", + manualCookieHeader: "sibling-cookie" + ) + XCTAssertTrue(owner.saveSecrets(using: store)) + XCTAssertTrue(sibling.saveSecrets(using: store)) + XCTAssertTrue( + store.save( + key: legacyKey(.claude, "apiKey"), + value: "legacy-api-key", + accessGroup: group + ) + ) + XCTAssertTrue( + store.save( + key: legacyKey(.claude, "cookie"), + value: "legacy-cookie", + accessGroup: group + ) + ) + let state = try makeIsolatedState(store: store) + state.providerConfigs = [owner, sibling] + + XCTAssertTrue(state.removeProviderAccount(ownerID)) + + XCTAssertEqual(state.providerConfigs.map(\.accountID), [siblingID]) + XCTAssertNil( + store.load( + key: legacyKey(.claude, "apiKey"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: legacyKey(.claude, "cookie"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: markerKey(ownerID, "apiKey"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: markerKey(ownerID, "cookie"), + accessGroup: group + ) + ) + XCTAssertEqual( + store.load( + key: accountKey(siblingID, "apiKey"), + accessGroup: group + ), + "sibling-api-key" + ) + XCTAssertEqual( + store.load( + key: accountKey(siblingID, "cookie"), + accessGroup: group + ), + "sibling-cookie" + ) + } + + @MainActor + func testLegacyDeleteFailureKeepsAccountRetryableUntilCleanupCompletes() + throws + { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "B0B0B0B0-B0B0-40B0-80B0-B0B0B0B0B0B0" + ) + ) + let group = ProviderConfig.secretsAccessGroup + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "account-api-key", + manualCookieHeader: "account-cookie", + legacySecretMigrationEligible: true + ) + XCTAssertTrue(config.saveSecrets(using: store)) + XCTAssertTrue( + store.save( + key: legacyKey(.claude, "apiKey"), + value: "legacy-api-key", + accessGroup: group + ) + ) + let legacyCookie = legacyKey(.claude, "cookie") + XCTAssertTrue( + store.save( + key: legacyCookie, + value: "legacy-cookie", + accessGroup: group + ) + ) + store.failingDeleteKeys.insert(legacyCookie) + let state = try makeIsolatedState(store: store) + state.providerConfigs = [config] + + XCTAssertFalse(state.removeProviderAccount(accountID)) + XCTAssertEqual(state.providerConfigs.map(\.accountID), [accountID]) + XCTAssertNil( + store.load( + key: legacyKey(.claude, "apiKey"), + accessGroup: group + ), + "completed deletion steps may remain monotonic while metadata stays retryable" + ) + XCTAssertEqual( + store.load(key: legacyCookie, accessGroup: group), + "legacy-cookie" + ) + XCTAssertEqual( + store.load( + key: markerKey(accountID, "apiKey"), + accessGroup: group + ), + "1", + "the marker must prevent retained legacy data from reappearing before retry" + ) + + store.failingDeleteKeys.remove(legacyCookie) + + XCTAssertTrue(state.removeProviderAccount(accountID)) + XCTAssertTrue(state.providerConfigs.isEmpty) + XCTAssertNil(store.load(key: legacyCookie, accessGroup: group)) + XCTAssertNil( + store.load( + key: markerKey(accountID, "apiKey"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: markerKey(accountID, "cookie"), + accessGroup: group + ) + ) + } + + @MainActor + func testLegacyReadFailureCannotVerifyAccountRemoval() throws { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "B1B1B1B1-B1B1-41B1-81B1-B1B1B1B1B1B1" + ) + ) + let group = ProviderConfig.secretsAccessGroup + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "account-api-key", + legacySecretMigrationEligible: true + ) + XCTAssertTrue(config.saveSecrets(using: store)) + let legacyAPIKey = legacyKey(.claude, "apiKey") + XCTAssertTrue( + store.save( + key: legacyAPIKey, + value: "legacy-api-key", + accessGroup: group + ) + ) + store.failingLoadKeys.insert(legacyAPIKey) + let state = try makeIsolatedState(store: store) + state.providerConfigs = [config] + + XCTAssertFalse(state.removeProviderAccount(accountID)) + XCTAssertEqual(state.providerConfigs.map(\.accountID), [accountID]) + + store.failingLoadKeys.remove(legacyAPIKey) + + XCTAssertTrue(state.removeProviderAccount(accountID)) + XCTAssertTrue(state.providerConfigs.isEmpty) + } + + @MainActor + func testMarkerDeleteFailureKeepsAccountRetryableUntilCleanupCompletes() + throws + { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "B2B2B2B2-B2B2-42B2-82B2-B2B2B2B2B2B2" + ) + ) + let group = ProviderConfig.secretsAccessGroup + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "account-api-key", + manualCookieHeader: "account-cookie" + ) + XCTAssertTrue(config.saveSecrets(using: store)) + let cookieMarker = markerKey(accountID, "cookie") + store.failingDeleteKeys.insert(cookieMarker) + let state = try makeIsolatedState(store: store) + state.providerConfigs = [config] + + XCTAssertFalse(state.removeProviderAccount(accountID)) + XCTAssertEqual(state.providerConfigs.map(\.accountID), [accountID]) + XCTAssertNil( + store.load( + key: accountKey(accountID, "apiKey"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: accountKey(accountID, "cookie"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: markerKey(accountID, "apiKey"), + accessGroup: group + ) + ) + XCTAssertEqual( + store.load(key: cookieMarker, accessGroup: group), + "1" + ) + + store.failingDeleteKeys.remove(cookieMarker) + + XCTAssertTrue(state.removeProviderAccount(accountID)) + XCTAssertTrue(state.providerConfigs.isEmpty) + XCTAssertNil(store.load(key: cookieMarker, accessGroup: group)) + } + + @MainActor + func testRemovingNonMigrationSiblingPreservesLegacySlotsAndRemovesMarkers() + throws + { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "B3B3B3B3-B3B3-43B3-83B3-B3B3B3B3B3B3" + ) + ) + let group = ProviderConfig.secretsAccessGroup + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "sibling-api-key" + ) + XCTAssertTrue(config.saveSecrets(using: store)) + XCTAssertTrue( + store.save( + key: legacyKey(.claude, "apiKey"), + value: "migration-owner-legacy-key", + accessGroup: group + ) + ) + let state = try makeIsolatedState(store: store) + state.providerConfigs = [config] + + XCTAssertTrue(state.removeProviderAccount(accountID)) + + XCTAssertEqual( + store.load( + key: legacyKey(.claude, "apiKey"), + accessGroup: group + ), + "migration-owner-legacy-key" + ) + XCTAssertNil( + store.load( + key: markerKey(accountID, "apiKey"), + accessGroup: group + ) + ) + XCTAssertNil( + store.load( + key: markerKey(accountID, "cookie"), + accessGroup: group + ) + ) + } + + #if os(macOS) + @MainActor + func testSharedOwnerReleaseFailureKeepsAccountRetryable() throws { + let store = InMemoryProviderSecretStore() + let accountID = try XCTUnwrap( + UUID( + uuidString: + "B4B4B4B4-B4B4-44B4-84B4-B4B4B4B4B4B4" + ) + ) + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + apiKey: "account-api-key" + ) + XCTAssertTrue(config.saveSecrets(using: store)) + let state = try makeIsolatedState( + store: store, + bundleIdentifier: "yyh.CLI-Pulse" + ) + state.providerConfigs = [config] + + let ownerSuiteName = + "ProviderAccountKeychainMigrationTests.Owner.\(UUID().uuidString)" + let ownerDefaults = try XCTUnwrap( + UserDefaults(suiteName: ownerSuiteName) + ) + ownerDefaults.removePersistentDomain(forName: ownerSuiteName) + let ownerKey = + "cli_pulse_provider_shared_credential_owner_\(ProviderKind.claude.rawValue)" + ownerDefaults.set(accountID.uuidString, forKey: ownerKey) + + let originalOwnerDefaults = ProviderSharedCredentialOwner.defaults + let originalSynchronizeDefaults = + ProviderSharedCredentialOwner.synchronizeDefaults + let originalMutationLock = + ProviderSharedCredentialOwner.mutationLock + let mutationLockPath = FileManager.default.temporaryDirectory + .appendingPathComponent("\(ownerSuiteName).lock") + .path + defer { + ProviderSharedCredentialOwner.defaults = originalOwnerDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = + originalSynchronizeDefaults + ProviderSharedCredentialOwner.mutationLock = + originalMutationLock + ownerDefaults.removePersistentDomain(forName: ownerSuiteName) + try? FileManager.default.removeItem( + atPath: mutationLockPath + ) + } + ProviderSharedCredentialOwner.defaults = ownerDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in false } + ProviderSharedCredentialOwner.mutationLock = + GeminiCredentialMutationLock( + lockFilePath: mutationLockPath + ) + + XCTAssertFalse(state.removeProviderAccount(accountID)) + XCTAssertEqual(state.providerConfigs.map(\.accountID), [accountID]) + XCTAssertEqual( + ownerDefaults.string(forKey: ownerKey), + accountID.uuidString + ) + + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in true } + + XCTAssertTrue(state.removeProviderAccount(accountID)) + XCTAssertTrue(state.providerConfigs.isEmpty) + XCTAssertNil(ownerDefaults.string(forKey: ownerKey)) + } + #endif + private func legacyKey(_ kind: ProviderKind, _ suffix: String) -> String { "cli_pulse_provider_\(kind.rawValue)_\(suffix)" } @@ -254,6 +840,41 @@ final class ProviderAccountKeychainMigrationTests: XCTestCase { private func accountKey(_ accountID: UUID, _ suffix: String) -> String { "cli_pulse_provider_account_\(accountID.uuidString)_\(suffix)" } + + private func markerKey(_ accountID: UUID, _ suffix: String) -> String { + "\(accountKey(accountID, suffix))_legacy_migrated" + } + + @MainActor + private func makeIsolatedState( + store: InMemoryProviderSecretStore, + bundleIdentifier: String = + "tests.clipulse.provider-account-removal" + ) throws -> AppState { + let suiteName = + "ProviderAccountKeychainMigrationTests.\(UUID().uuidString)" + let defaults = try XCTUnwrap( + UserDefaults(suiteName: suiteName) + ) + defaults.removePersistentDomain(forName: suiteName) + addTeardownBlock { + defaults.removePersistentDomain(forName: suiteName) + } + let runtime = + CLIPulseRuntimeEnvironment.resolveForTesting( + infoDictionary: [ + "CFBundleIdentifier": + bundleIdentifier, + ], + environment: [:] + ) + return AppState( + runtimeEnvironment: runtime, + defaults: defaults, + providerSecretStore: store, + performLaunchSetup: false + ) + } } private final class InMemoryProviderSecretStore: ProviderSecretStoring { @@ -264,7 +885,9 @@ private final class InMemoryProviderSecretStore: ProviderSecretStoring { private var values: [Slot: String] = [:] var failingSaveKeys: Set = [] + var failingSaveAttemptsByKey: [String: Int] = [:] var failingDeleteKeys: Set = [] + var failingLoadKeys: Set = [] @discardableResult func save( @@ -275,12 +898,38 @@ private final class InMemoryProviderSecretStore: ProviderSecretStoring { guard !failingSaveKeys.contains(key) else { return false } + if let remaining = failingSaveAttemptsByKey[key], + remaining > 0 + { + failingSaveAttemptsByKey[key] = remaining - 1 + return false + } values[Slot(key: key, accessGroup: accessGroup)] = value return true } func load(key: String, accessGroup: String?) -> String? { - values[Slot(key: key, accessGroup: accessGroup)] + guard !failingLoadKeys.contains(key) else { + return nil + } + return values[Slot(key: key, accessGroup: accessGroup)] + } + + func read( + key: String, + accessGroup: String? + ) -> ProviderSecretReadResult { + guard !failingLoadKeys.contains(key) else { + return .failure + } + guard + let value = values[ + Slot(key: key, accessGroup: accessGroup) + ] + else { + return .missing + } + return .value(value) } @discardableResult diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountSaveTransactionTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountSaveTransactionTests.swift new file mode 100644 index 00000000..3e1d1413 --- /dev/null +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderAccountSaveTransactionTests.swift @@ -0,0 +1,171 @@ +import XCTest +@testable import CLIPulseCore + +final class ProviderAccountSaveTransactionTests: XCTestCase { + func testCredentialMutationRunsAfterAllFallibleConfigWrites() { + var events: [String] = [] + + let result = ProviderAccountSaveTransaction.commit( + persistSecrets: { + events.append("secrets") + return true + }, + rollbackSecrets: { + events.append("rollback-secrets") + return true + }, + persistMetadata: { + events.append("metadata") + return true + }, + rollbackMetadata: { + events.append("rollback-metadata") + return true + }, + commitProviderCredential: { + events.append("credential") + return true + }, + finalize: { + events.append("finalize") + } + ) + + XCTAssertEqual(result, .committed) + XCTAssertEqual(events, ["secrets", "metadata", "credential", "finalize"]) + } + + func testMetadataFailurePreventsCredentialMutationAndFinalization() { + var events: [String] = [] + + let result = ProviderAccountSaveTransaction.commit( + persistSecrets: { + events.append("secrets") + return true + }, + rollbackSecrets: { + events.append("rollback-secrets") + return true + }, + persistMetadata: { + events.append("metadata") + return false + }, + rollbackMetadata: { + events.append("rollback-metadata") + return true + }, + commitProviderCredential: { + events.append("credential") + return true + }, + finalize: { + events.append("finalize") + } + ) + + XCTAssertEqual(result, .failedRolledBack) + XCTAssertEqual( + events, + [ + "secrets", + "metadata", + "rollback-metadata", + "rollback-secrets", + ] + ) + } + + func testCredentialFailureLeavesDraftUnfinalized() { + var events: [String] = [] + + let result = ProviderAccountSaveTransaction.commit( + persistSecrets: { + events.append("secrets") + return true + }, + rollbackSecrets: { + events.append("rollback-secrets") + return true + }, + persistMetadata: { + events.append("metadata") + return true + }, + rollbackMetadata: { + events.append("rollback-metadata") + return true + }, + commitProviderCredential: { + events.append("credential") + return false + }, + finalize: { + events.append("finalize") + } + ) + + XCTAssertEqual(result, .failedRolledBack) + XCTAssertEqual( + events, + [ + "secrets", + "metadata", + "credential", + "rollback-metadata", + "rollback-secrets", + ] + ) + } + + func testSecretFailureRetriesSecretRollbackBeforeReturning() { + var events: [String] = [] + + let result = ProviderAccountSaveTransaction.commit( + persistSecrets: { + events.append("secrets") + return false + }, + rollbackSecrets: { + events.append("rollback-secrets") + return true + }, + persistMetadata: { + events.append("metadata") + return true + }, + rollbackMetadata: { + events.append("rollback-metadata") + return true + }, + commitProviderCredential: { + events.append("credential") + return true + }, + finalize: { + events.append("finalize") + } + ) + + XCTAssertEqual(result, .failedRolledBack) + XCTAssertEqual(events, ["secrets", "rollback-secrets"]) + } + + func testRollbackFailureIsDistinguishableFromCleanCompensation() { + var finalized = false + + let result = ProviderAccountSaveTransaction.commit( + persistSecrets: { true }, + rollbackSecrets: { true }, + persistMetadata: { true }, + rollbackMetadata: { false }, + commitProviderCredential: { false }, + finalize: { + finalized = true + } + ) + + XCTAssertEqual(result, .failedRollbackIncomplete) + XCTAssertFalse(finalized) + } +} diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderConfigMetadataStoreTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderConfigMetadataStoreTests.swift index b4fbadd8..0715c0e3 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderConfigMetadataStoreTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderConfigMetadataStoreTests.swift @@ -83,6 +83,240 @@ final class ProviderConfigMetadataStoreTests: XCTestCase { } } + func testHelperWriteFailureRestoresAppAndHelperMetadata() + throws + { + let appSuite = + "ProviderConfigMetadataStoreTests.rollback.app.\(UUID())" + let helperSuite = + "ProviderConfigMetadataStoreTests.rollback.helper.\(UUID())" + let appDefaults = try XCTUnwrap( + UserDefaults(suiteName: appSuite) + ) + let helperDefaults = try XCTUnwrap( + DroppingUserDefaults(suiteName: helperSuite) + ) + defer { + appDefaults.removePersistentDomain(forName: appSuite) + helperDefaults.removePersistentDomain(forName: helperSuite) + } + let accountID = try XCTUnwrap( + UUID( + uuidString: + "ACACACAC-ACAC-4CAC-8CAC-ACACACACACAC" + ) + ) + let original = ProviderConfig( + kind: .claude, + accountID: accountID, + accountLabel: "Original" + ) + let edited = ProviderConfig( + kind: .claude, + accountID: accountID, + accountLabel: "Edited" + ) + let store = ProviderConfigMetadataStore( + defaults: appDefaults, + helperDefaults: helperDefaults + ) + XCTAssertTrue(store.save([original])) + helperDefaults.droppedSetKeys.insert( + HelperIPC.providerConfigsKey + ) + + XCTAssertFalse(store.save([edited])) + for (defaults, key) in [ + (appDefaults, ProviderAccountMigration.configsKey), + (helperDefaults, HelperIPC.providerConfigsKey), + ] { + let data = try XCTUnwrap(defaults.data(forKey: key)) + let configs = try JSONDecoder().decode( + [ProviderConfig].self, + from: data + ) + XCTAssertEqual(configs.first?.accountLabel, "Original") + } + } + + @MainActor + func testAccountRemovalMetadataFailureKeepsAccountRetryable() + throws + { + let suiteName = + "ProviderConfigMetadataStoreTests.remove-failure.\(UUID())" + let defaults = try XCTUnwrap( + DroppingUserDefaults(suiteName: suiteName) + ) + defer { + defaults.removePersistentDomain(forName: suiteName) + } + let accountID = try XCTUnwrap( + UUID( + uuidString: + "ABABABAB-ABAB-4BAB-8BAB-ABABABABABAB" + ) + ) + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + syncOwnerUserID: "user-a" + ) + let metadataStore = ProviderConfigMetadataStore( + defaults: defaults, + helperDefaults: nil + ) + XCTAssertTrue(metadataStore.save([config])) + + let outbox = ProviderAccountDeletionOutbox( + defaults: defaults, + storageKey: + "ProviderConfigMetadataStoreTests.remove-failure.outbox" + ) + let runtime = + CLIPulseRuntimeEnvironment.resolveForTesting( + infoDictionary: [ + "CFBundleIdentifier": + "tests.clipulse.provider-metadata-removal", + ], + environment: [:] + ) + XCTAssertFalse( + runtime.capabilities.allowsHelperRegistration + ) + let secretStore = MemoryProviderSecretStore() + XCTAssertTrue( + config.deleteSecretsForAccountRemoval( + using: secretStore + ) + ) + let state = AppState( + runtimeEnvironment: runtime, + defaults: defaults, + providerSecretStore: secretStore, + providerAccountDeletionOutbox: outbox, + performLaunchSetup: false + ) + state.providerConfigs = [config] + defaults.droppedSetKeys.insert( + ProviderAccountMigration.configsKey + ) + XCTAssertFalse( + metadataStore.save([]), + "the fixture must reach the metadata write failure" + ) + + XCTAssertFalse(state.removeProviderAccount(accountID)) + XCTAssertEqual( + state.providerConfigs.map(\.accountID), + [accountID], + "a failed metadata write must leave a visible retry anchor" + ) + XCTAssertEqual( + outbox.pendingAccountIDs(for: "user-a"), + [accountID], + "the durable deletion intent must remain available for retry" + ) + let persistedData = try XCTUnwrap( + defaults.data( + forKey: ProviderAccountMigration.configsKey + ) + ) + XCTAssertEqual( + try JSONDecoder().decode( + [ProviderConfig].self, + from: persistedData + ).map(\.accountID), + [accountID] + ) + } + + @MainActor + func testPendingDeletionRecoveryMetadataFailureKeepsAccountRetryable() + throws + { + let suiteName = + "ProviderConfigMetadataStoreTests.recovery-failure.\(UUID())" + let defaults = try XCTUnwrap( + DroppingUserDefaults(suiteName: suiteName) + ) + defer { + defaults.removePersistentDomain(forName: suiteName) + } + let accountID = try XCTUnwrap( + UUID( + uuidString: + "CDCDCDCD-CDCD-4DCD-8DCD-CDCDCDCDCDCD" + ) + ) + let config = ProviderConfig( + kind: .claude, + accountID: accountID, + syncOwnerUserID: "user-a" + ) + XCTAssertTrue( + ProviderConfigMetadataStore( + defaults: defaults, + helperDefaults: nil + ).save([config]) + ) + let outbox = ProviderAccountDeletionOutbox( + defaults: defaults, + storageKey: + "ProviderConfigMetadataStoreTests.recovery-failure.outbox" + ) + XCTAssertTrue( + outbox.enqueue( + userID: "user-a", + accountID: accountID, + provider: .claude + ) + ) + let runtime = + CLIPulseRuntimeEnvironment.resolveForTesting( + infoDictionary: [ + "CFBundleIdentifier": + "tests.clipulse.provider-deletion-recovery", + ], + environment: [:] + ) + let state = AppState( + runtimeEnvironment: runtime, + defaults: defaults, + providerSecretStore: MemoryProviderSecretStore(), + providerAccountDeletionOutbox: outbox, + performLaunchSetup: false + ) + state.providerConfigs = [config] + defaults.droppedSetKeys.insert( + ProviderAccountMigration.configsKey + ) + + state.recoverPendingLocalProviderAccountDeletions() + + XCTAssertEqual( + state.providerConfigs.map(\.accountID), + [accountID], + "relaunch recovery must keep the account visible when metadata cannot commit" + ) + XCTAssertEqual( + outbox.pendingAccountIDs(for: "user-a"), + [accountID] + ) + let persistedData = try XCTUnwrap( + defaults.data( + forKey: ProviderAccountMigration.configsKey + ) + ) + XCTAssertEqual( + try JSONDecoder().decode( + [ProviderConfig].self, + from: persistedData + ).map(\.accountID), + [accountID] + ) + } + @MainActor func testDraftRecoveryAnchorSurvivesRestartBeforeCredentialCommit() throws @@ -159,9 +393,484 @@ final class ProviderConfigMetadataStoreTests: XCTestCase { ) } + @MainActor + func testIncompleteSaveRecoveryIsRetainedUntilCompensationSucceeds() + throws + { + let defaults = try XCTUnwrap( + UserDefaults( + suiteName: + "ProviderConfigMetadataStoreTests.pending-recovery.\(UUID())" + ) + ) + defer { + clear(defaults) + } + let state = AppState( + api: APIClient(), + providerAccountDeletionOutbox: + ProviderAccountDeletionOutbox( + defaults: defaults, + storageKey: + "ProviderConfigMetadataStoreTests.pending-recovery.outbox" + ), + performLaunchSetup: false + ) + let accountID = UUID() + var metadataAttempts = 0 + var secretAttempts = 0 + let recovery = ProviderAccountSaveRecovery( + restoreMetadata: { + metadataAttempts += 1 + return metadataAttempts >= 2 + }, + restoreSecrets: { + secretAttempts += 1 + return true + } + ) + state.retainProviderAccountSaveRecovery( + recovery, + for: accountID + ) + + XCTAssertFalse( + state.recoverPendingProviderAccountSave(accountID) + ) + XCTAssertTrue( + state.recoverPendingProviderAccountSave(accountID) + ) + XCTAssertTrue( + state.recoverPendingProviderAccountSave(accountID), + "a successful recovery must clear the gate" + ) + XCTAssertEqual(metadataAttempts, 2) + XCTAssertEqual(secretAttempts, 2) + } + + #if os(macOS) + @MainActor + func testOwnerReconcileFailureBlocksDeletionButNotOrdinaryMetadataSave() + throws + { + let appSuiteName = + "ProviderConfigMetadataStoreTests.remove-owner.app.\(UUID())" + let helperSuiteName = + "ProviderConfigMetadataStoreTests.remove-owner.helper.\(UUID())" + let appDefaults = try XCTUnwrap( + UserDefaults(suiteName: appSuiteName) + ) + let helperDefaults = try XCTUnwrap( + UserDefaults(suiteName: helperSuiteName) + ) + let originalOwnerDefaults = + ProviderSharedCredentialOwner.defaults + let originalSynchronizeDefaults = + ProviderSharedCredentialOwner.synchronizeDefaults + let originalMutationLock = + ProviderSharedCredentialOwner.mutationLock + let lockPath = FileManager.default.temporaryDirectory + .appendingPathComponent( + "ProviderConfigMetadataStoreTests.remove-owner.\(UUID()).lock" + ) + .path + defer { + ProviderSharedCredentialOwner.defaults = + originalOwnerDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = + originalSynchronizeDefaults + ProviderSharedCredentialOwner.mutationLock = + originalMutationLock + appDefaults.removePersistentDomain( + forName: appSuiteName + ) + helperDefaults.removePersistentDomain( + forName: helperSuiteName + ) + try? FileManager.default.removeItem(atPath: lockPath) + } + ProviderSharedCredentialOwner.defaults = helperDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in + false + } + ProviderSharedCredentialOwner.mutationLock = + GeminiCredentialMutationLock( + lockFilePath: lockPath + ) + + let accountID = try XCTUnwrap( + UUID( + uuidString: + "DEDEDEDE-DEDE-4EDE-8EDE-DEDEDEDEDEDE" + ) + ) + let config = ProviderConfig( + kind: .codex, + accountID: accountID + ) + let metadataStore = ProviderConfigMetadataStore( + defaults: appDefaults, + helperDefaults: helperDefaults + ) + XCTAssertTrue(metadataStore.save([config])) + let state = AppState( + runtimeEnvironment: TestRuntimeFixtures.productionApp, + defaults: appDefaults, + helperDefaults: helperDefaults, + providerSecretStore: MemoryProviderSecretStore(), + providerAccountDeletionOutbox: + ProviderAccountDeletionOutbox( + defaults: appDefaults, + storageKey: + "ProviderConfigMetadataStoreTests.remove-owner.outbox" + ), + performLaunchSetup: false + ) + state.providerConfigs = [config] + + XCTAssertFalse(state.removeProviderAccount(accountID)) + XCTAssertEqual( + state.providerConfigs.map(\.accountID), + [accountID] + ) + let persistedData = try XCTUnwrap( + appDefaults.data( + forKey: ProviderAccountMigration.configsKey + ) + ) + XCTAssertEqual( + try JSONDecoder().decode( + [ProviderConfig].self, + from: persistedData + ).map(\.accountID), + [accountID] + ) + XCTAssertTrue( + state.saveProviderConfigMetadata(), + "the deletion-only owner gate must not change legacy save callers that do not handle reconciliation failure" + ) + } + + @MainActor + func testFinalDraftMetadataPersistenceReconcilesSharedOwner() throws { + let appDefaults = try XCTUnwrap( + UserDefaults( + suiteName: "ProviderConfigMetadataStoreTests.final.app.\(UUID())" + ) + ) + let helperDefaults = try XCTUnwrap( + UserDefaults( + suiteName: "ProviderConfigMetadataStoreTests.final.helper.\(UUID())" + ) + ) + let originalOwnerDefaults = ProviderSharedCredentialOwner.defaults + let originalSynchronizeDefaults = ProviderSharedCredentialOwner.synchronizeDefaults + let originalMutationLock = ProviderSharedCredentialOwner.mutationLock + let lockPath = FileManager.default.temporaryDirectory + .appendingPathComponent("ProviderConfigMetadataStoreTests.final.\(UUID()).lock") + .path + defer { + ProviderSharedCredentialOwner.defaults = originalOwnerDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = originalSynchronizeDefaults + ProviderSharedCredentialOwner.mutationLock = originalMutationLock + clear(appDefaults) + clear(helperDefaults) + try? FileManager.default.removeItem(atPath: lockPath) + } + ProviderSharedCredentialOwner.defaults = helperDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in true } + ProviderSharedCredentialOwner.mutationLock = GeminiCredentialMutationLock( + lockFilePath: lockPath + ) + + let accountID = try XCTUnwrap( + UUID(uuidString: "CCCCCCCC-CCCC-4CCC-8CCC-CCCCCCCCCCCC") + ) + let state = AppState( + runtimeEnvironment: TestRuntimeFixtures.productionApp, + defaults: appDefaults, + helperDefaults: helperDefaults, + providerAccountDeletionOutbox: ProviderAccountDeletionOutbox( + defaults: appDefaults, + storageKey: "ProviderConfigMetadataStoreTests.final.outbox" + ), + performLaunchSetup: false + ) + state.providerConfigs = [ + ProviderConfig( + kind: .claude, + accountID: accountID, + isEnabled: true + ), + ] + let metadataStore = ProviderConfigMetadataStore( + defaults: appDefaults, + helperDefaults: helperDefaults + ) + + XCTAssertEqual(ProviderSharedCredentialOwner.lookup(kind: .claude), .unowned) + XCTAssertTrue( + state.persistProviderAccountDraftMetadata( + accountID, + using: metadataStore + ) + ) + XCTAssertEqual( + ProviderSharedCredentialOwner.lookup(kind: .claude), + .owned(accountID) + ) + } + + @MainActor + func testOwnerReconcileFailureStopsCredentialCommitAndDraftFinalization() throws { + let appDefaults = try XCTUnwrap( + UserDefaults( + suiteName: "ProviderConfigMetadataStoreTests.failed-owner.app.\(UUID())" + ) + ) + let helperDefaults = try XCTUnwrap( + UserDefaults( + suiteName: "ProviderConfigMetadataStoreTests.failed-owner.helper.\(UUID())" + ) + ) + let originalOwnerDefaults = ProviderSharedCredentialOwner.defaults + let originalSynchronizeDefaults = ProviderSharedCredentialOwner.synchronizeDefaults + let originalMutationLock = ProviderSharedCredentialOwner.mutationLock + let lockPath = FileManager.default.temporaryDirectory + .appendingPathComponent("ProviderConfigMetadataStoreTests.failed-owner.\(UUID()).lock") + .path + defer { + ProviderSharedCredentialOwner.defaults = originalOwnerDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = originalSynchronizeDefaults + ProviderSharedCredentialOwner.mutationLock = originalMutationLock + clear(appDefaults) + clear(helperDefaults) + try? FileManager.default.removeItem(atPath: lockPath) + } + ProviderSharedCredentialOwner.defaults = helperDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in true } + ProviderSharedCredentialOwner.mutationLock = GeminiCredentialMutationLock( + lockFilePath: lockPath + ) + helperDefaults.set( + "not-a-uuid", + forKey: "cli_pulse_provider_shared_credential_owner_Claude" + ) + + let accountID = try XCTUnwrap( + UUID(uuidString: "DDDDDDDD-DDDD-4DDD-8DDD-DDDDDDDDDDDD") + ) + let state = AppState( + runtimeEnvironment: TestRuntimeFixtures.productionApp, + defaults: appDefaults, + helperDefaults: helperDefaults, + providerAccountDeletionOutbox: ProviderAccountDeletionOutbox( + defaults: appDefaults, + storageKey: "ProviderConfigMetadataStoreTests.failed-owner.outbox" + ), + performLaunchSetup: false + ) + _ = state.addProviderAccount(kind: .gemini, accountID: accountID) + state.providerConfigs[0].isEnabled = true + let metadataStore = ProviderConfigMetadataStore( + defaults: appDefaults, + helperDefaults: helperDefaults + ) + var credentialCommitted = false + + let result = ProviderAccountSaveTransaction.commit( + persistSecrets: { true }, + rollbackSecrets: { true }, + persistMetadata: { + state.persistProviderAccountDraftMetadata( + accountID, + using: metadataStore + ) + }, + rollbackMetadata: { true }, + commitProviderCredential: { + credentialCommitted = true + return true + }, + finalize: { + state.finalizeProviderAccountDraft(accountID) + } + ) + + XCTAssertEqual(result, .failedRolledBack) + XCTAssertFalse(credentialCommitted) + XCTAssertTrue(state.providerState.isProviderAccountDraft(accountID)) + } + + @MainActor + func testCredentialFailureRestoresMetadataAndSharedOwner() + throws + { + let appDefaults = try XCTUnwrap( + UserDefaults( + suiteName: + "ProviderConfigMetadataStoreTests.credential-rollback.app.\(UUID())" + ) + ) + let helperDefaults = try XCTUnwrap( + UserDefaults( + suiteName: + "ProviderConfigMetadataStoreTests.credential-rollback.helper.\(UUID())" + ) + ) + let originalOwnerDefaults = ProviderSharedCredentialOwner.defaults + let originalSynchronizeDefaults = + ProviderSharedCredentialOwner.synchronizeDefaults + let originalMutationLock = + ProviderSharedCredentialOwner.mutationLock + let lockPath = FileManager.default.temporaryDirectory + .appendingPathComponent( + "ProviderConfigMetadataStoreTests.credential-rollback.\(UUID()).lock" + ) + .path + defer { + ProviderSharedCredentialOwner.defaults = originalOwnerDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = + originalSynchronizeDefaults + ProviderSharedCredentialOwner.mutationLock = + originalMutationLock + clear(appDefaults) + clear(helperDefaults) + try? FileManager.default.removeItem(atPath: lockPath) + } + ProviderSharedCredentialOwner.defaults = helperDefaults + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in true } + ProviderSharedCredentialOwner.mutationLock = + GeminiCredentialMutationLock(lockFilePath: lockPath) + + let accountID = try XCTUnwrap( + UUID( + uuidString: + "EFEFEFEF-EFEF-4FEF-8FEF-EFEFEFEFEFEF" + ) + ) + let state = AppState( + runtimeEnvironment: TestRuntimeFixtures.productionApp, + defaults: appDefaults, + helperDefaults: helperDefaults, + providerAccountDeletionOutbox: + ProviderAccountDeletionOutbox( + defaults: appDefaults, + storageKey: + "ProviderConfigMetadataStoreTests.credential-rollback.outbox" + ), + performLaunchSetup: false + ) + state.providerConfigs = [ + ProviderConfig( + kind: .claude, + accountID: accountID, + isEnabled: true, + accountLabel: "Original" + ), + ] + let metadataStore = ProviderConfigMetadataStore( + defaults: appDefaults, + helperDefaults: helperDefaults + ) + XCTAssertTrue( + state.persistProviderAccountDraftMetadata( + accountID, + using: metadataStore + ) + ) + let checkpoint = try XCTUnwrap( + state.makeProviderAccountPersistenceCheckpoint( + accountID, + using: metadataStore + ) + ) + state.providerConfigs[0].accountLabel = "Edited" + state.providerConfigs[0] + .sharedCredentialFallbackDisabled = true + + XCTAssertEqual( + ProviderAccountSaveTransaction.commit( + persistSecrets: { true }, + rollbackSecrets: { true }, + persistMetadata: { + state.persistProviderAccountDraftMetadata( + accountID, + using: metadataStore + ) + }, + rollbackMetadata: { + checkpoint.restore() + }, + commitProviderCredential: { false }, + finalize: {} + ), + .failedRolledBack + ) + XCTAssertEqual( + ProviderSharedCredentialOwner.lookup(kind: .claude), + .owned(accountID) + ) + for (defaults, key) in [ + (appDefaults, ProviderAccountMigration.configsKey), + (helperDefaults, HelperIPC.providerConfigsKey), + ] { + let data = try XCTUnwrap(defaults.data(forKey: key)) + let configs = try JSONDecoder().decode( + [ProviderConfig].self, + from: data + ) + XCTAssertEqual(configs.first?.accountLabel, "Original") + XCTAssertNil( + configs.first?.sharedCredentialFallbackDisabled + ) + } + } + #endif + private func clear(_ defaults: UserDefaults) { for key in defaults.dictionaryRepresentation().keys { defaults.removeObject(forKey: key) } } } + +final class DroppingUserDefaults: UserDefaults { + var droppedSetKeys: Set = [] + + override func set(_ value: Any?, forKey defaultName: String) { + guard !droppedSetKeys.contains(defaultName) else { + return + } + super.set(value, forKey: defaultName) + } +} + +final class MemoryProviderSecretStore: ProviderSecretStoring { + private struct Slot: Hashable { + let key: String + let accessGroup: String? + } + + private var values: [Slot: String] = [:] + + func save( + key: String, + value: String, + accessGroup: String? + ) -> Bool { + values[Slot(key: key, accessGroup: accessGroup)] = value + return true + } + + func load(key: String, accessGroup: String?) -> String? { + values[Slot(key: key, accessGroup: accessGroup)] + } + + func delete(key: String, accessGroup: String?) -> Bool { + values.removeValue( + forKey: Slot(key: key, accessGroup: accessGroup) + ) + return true + } +} diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderSharedCredentialOwnerTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderSharedCredentialOwnerTests.swift index 20d5152b..bd06ce5d 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderSharedCredentialOwnerTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/ProviderSharedCredentialOwnerTests.swift @@ -211,6 +211,15 @@ final class ProviderSharedCredentialOwnerTests: XCTestCase { ) } + func testReleaseForProviderWithoutSharedSourceIsSuccessfulNoOp() { + XCTAssertTrue( + ProviderSharedCredentialOwner.release( + kind: .codex, + accountID: UUID() + ) + ) + } + func testOwnerCallIsReentrantInsideCredentialMutationLock() throws { @@ -240,6 +249,37 @@ final class ProviderSharedCredentialOwnerTests: XCTestCase { ) } + @MainActor + func testAppSaveTransactionLockAllowsReentrantOwnerMutation() + throws + { + let accountID = try XCTUnwrap( + UUID( + uuidString: + "05050505-0505-4505-8505-050505050505" + ) + ) + let state = AppState( + runtimeEnvironment: TestRuntimeFixtures.productionApp, + defaults: testDefaults, + helperDefaults: testDefaults, + performLaunchSetup: false + ) + + XCTAssertTrue( + state.withProviderAccountPersistenceLock(or: false) { + ProviderSharedCredentialOwner.claim( + kind: .gemini, + accountID: accountID + ) + } + ) + XCTAssertEqual( + ProviderSharedCredentialOwner.lookup(kind: .gemini), + .owned(accountID) + ) + } + func testMutationLockRejectsHardLinkBeforeChangingPermissions() throws { @@ -339,6 +379,57 @@ final class ProviderSharedCredentialOwnerTests: XCTestCase { ) } + func testReconcileRestoresEveryProviderWhenLaterOwnerWriteFails() + throws + { + let claudeID = try XCTUnwrap( + UUID( + uuidString: + "DADADADA-DADA-4ADA-8ADA-DADADADADADA" + ) + ) + let geminiID = try XCTUnwrap( + UUID( + uuidString: + "EAEAEAEA-EAEA-4AEA-8AEA-EAEAEAEAEAEA" + ) + ) + var synchronizeCalls = 0 + ProviderSharedCredentialOwner.synchronizeDefaults = { _ in + synchronizeCalls += 1 + return synchronizeCalls != 3 + } + + XCTAssertFalse( + ProviderSharedCredentialOwner.reconcile( + configs: [ + ProviderConfig( + kind: .claude, + accountID: claudeID, + isEnabled: true + ), + ProviderConfig( + kind: .gemini, + accountID: geminiID, + isEnabled: true + ), + ] + ) + ) + XCTAssertNil( + testDefaults.object( + forKey: + "cli_pulse_provider_shared_credential_owner_Claude" + ) + ) + XCTAssertNil( + testDefaults.object( + forKey: + "cli_pulse_provider_shared_credential_owner_Gemini" + ) + ) + } + func testReconcileSkipsAccountsThatDisableSharedFallback() throws { let isolatedID = try XCTUnwrap( UUID(uuidString: "12121212-1212-4121-8121-121212121212") diff --git a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/QARuntimeSideEffectPolicyTests.swift b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/QARuntimeSideEffectPolicyTests.swift index 5b61a1cc..7a2518ab 100644 --- a/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/QARuntimeSideEffectPolicyTests.swift +++ b/CLI Pulse Bar/CLIPulseCore/Tests/CLIPulseCoreTests/QARuntimeSideEffectPolicyTests.swift @@ -840,7 +840,10 @@ final class QARuntimeSideEffectPolicyTests: XCTestCase { ownerFixture.defaults.string(forKey: ownerKey), draftID.uuidString ) - XCTAssertEqual(secretStore.deletedKeys.count, 2) + XCTAssertEqual( + secretStore.deletedKeys, + expectedAccountDeletionKeys(draftID) + ) } @MainActor @@ -886,7 +889,23 @@ final class QARuntimeSideEffectPolicyTests: XCTestCase { ownerFixture.defaults.string(forKey: ownerKey), accountID.uuidString ) - XCTAssertEqual(secretStore.deletedKeys.count, 2) + XCTAssertEqual( + secretStore.deletedKeys, + expectedAccountDeletionKeys(accountID) + ) + } + + private func expectedAccountDeletionKeys( + _ accountID: UUID + ) -> [String] { + let prefix = + "cli_pulse_provider_account_\(accountID.uuidString)" + return [ + "\(prefix)_apiKey", + "\(prefix)_cookie", + "\(prefix)_apiKey_legacy_migrated", + "\(prefix)_cookie_legacy_migrated", + ] } private func makeRuntime(