Skip to content

Commit ce0b403

Browse files
committed
fix(settings): preserve provider keys during migration
1 parent ac9b3ad commit ce0b403

4 files changed

Lines changed: 510 additions & 33 deletions

File tree

Fluid.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
7C5AF14C2F15041600DE21B0 /* MediaRemoteAdapter in Embed Frameworks */ = {isa = PBXBuildFile; productRef = 7C5AF14A2F15041600DE21B0 /* MediaRemoteAdapter */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
1414
7C9A71022F58B00000FB7CAF /* TranscribeCpp in Frameworks */ = {isa = PBXBuildFile; productRef = 7C9A71012F58B00000FB7CAF /* TranscribeCpp */; };
1515
7C91B0012F42AA0100C0DEF0 /* HotkeyShortcutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C91B0022F42AA0100C0DEF0 /* HotkeyShortcutTests.swift */; };
16+
7C91B0032F42AA0100C0DEF0 /* ProviderAPIKeyMigrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C91B0042F42AA0100C0DEF0 /* ProviderAPIKeyMigrationTests.swift */; };
1617
7CDB0A2D2F3C4D5600FB7CAD /* DictationE2ETests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CDB0A292F3C4D5600FB7CAD /* DictationE2ETests.swift */; };
1718
CD1C7A0000000000000000B2 /* CustomDictionaryManualEntryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CD1C7A0000000000000000B1 /* CustomDictionaryManualEntryTests.swift */; };
1819
7CDB0A2E2F3C4D5600FB7CAD /* AudioFixtureLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7CDB0A2A2F3C4D5600FB7CAD /* AudioFixtureLoader.swift */; };
@@ -70,6 +71,7 @@
7071
7C91B0022F42AA0100C0DEF0 /* HotkeyShortcutTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HotkeyShortcutTests.swift; sourceTree = "<group>"; };
7172
7CDB0A202F3C4D5600FB7CAD /* FluidDictationIntegrationTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = FluidDictationIntegrationTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
7273
7CDB0A292F3C4D5600FB7CAD /* DictationE2ETests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DictationE2ETests.swift; sourceTree = "<group>"; };
74+
7C91B0042F42AA0100C0DEF0 /* ProviderAPIKeyMigrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProviderAPIKeyMigrationTests.swift; sourceTree = "<group>"; };
7375
CD1C7A0000000000000000B1 /* CustomDictionaryManualEntryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomDictionaryManualEntryTests.swift; sourceTree = "<group>"; };
7476
7CFA1D0B2F500000C0DEF002 /* TypingServiceTransientPasteboardTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TypingServiceTransientPasteboardTests.swift; sourceTree = "<group>"; };
7577
803000000000000000000001 /* MediaPlaybackServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaPlaybackServiceTests.swift; sourceTree = "<group>"; };
@@ -155,6 +157,7 @@
155157
DA7100010000000000000001 /* DirectAudioReliabilityTests.swift */,
156158
7CFA1D0B2F500000C0DEF002 /* TypingServiceTransientPasteboardTests.swift */,
157159
803000000000000000000001 /* MediaPlaybackServiceTests.swift */,
160+
7C91B0042F42AA0100C0DEF0 /* ProviderAPIKeyMigrationTests.swift */,
158161
);
159162
path = FluidDictationIntegrationTests;
160163
sourceTree = "<group>";
@@ -324,6 +327,7 @@
324327
DA7100020000000000000002 /* DirectAudioReliabilityTests.swift in Sources */,
325328
7CFA1D0B2F500000C0DEF001 /* TypingServiceTransientPasteboardTests.swift in Sources */,
326329
803000000000000000000002 /* MediaPlaybackServiceTests.swift in Sources */,
330+
7C91B0032F42AA0100C0DEF0 /* ProviderAPIKeyMigrationTests.swift in Sources */,
327331
);
328332
runOnlyForDeploymentPostprocessing = 0;
329333
};

Sources/Fluid/Persistence/KeychainService.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,15 @@ enum KeychainServiceError: Error, LocalizedError {
2020

2121
/// Lightweight helper for storing provider API keys in the system Keychain.
2222
/// Keys are stored as generic passwords scoped to the FluidVoice service.
23-
final class KeychainService {
23+
protocol ProviderKeychain {
24+
func storeKey(_ key: String, for providerID: String) throws
25+
func fetchAllKeys() throws -> [String: String]
26+
func storeAllKeys(_ values: [String: String]) throws
27+
func legacyProviderEntries() throws -> [String: String]
28+
func removeLegacyEntries(providerIDs: [String]) throws
29+
}
30+
31+
final class KeychainService: ProviderKeychain {
2432
static let shared = KeychainService()
2533

2634
private let service = "com.fluidvoice.provider-api-keys"
@@ -168,7 +176,6 @@ final class KeychainService {
168176

169177
switch status {
170178
case errSecSuccess:
171-
try self.removeLegacyEntries()
172179
return
173180
case errSecDuplicateItem:
174181
let updateAttributes: [String: Any] = [
@@ -181,7 +188,6 @@ final class KeychainService {
181188
guard updateStatus == errSecSuccess else {
182189
throw KeychainServiceError.unhandled(updateStatus)
183190
}
184-
try self.removeLegacyEntries()
185191
default:
186192
throw KeychainServiceError.unhandled(status)
187193
}

Sources/Fluid/Persistence/SettingsStore.swift

Lines changed: 116 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,20 @@ final class SettingsStore: ObservableObject {
2525
static let privateAIDictationRoundTripTokenCost = 2.75
2626
static let privateAIBackendPreferenceDefaultsKey = "FluidIntelligenceBackendPreference"
2727
private static let forcedOnboardingResetIntroducedAt = Date(timeIntervalSince1970: 1_782_091_732)
28-
private let defaults = UserDefaults.standard
29-
private let keychain = KeychainService.shared
28+
private let defaults: UserDefaults
29+
private var keychain: ProviderKeychain
3030
private(set) var launchAtStartupEnabled = false
3131
private(set) var launchAtStartupErrorMessage: String?
3232
private(set) var launchAtStartupStatusMessage =
3333
"FluidVoice reflects the actual macOS login item state. Unsigned or development builds may fail to enable this."
3434

35-
private init() {
35+
private init(
36+
defaults: UserDefaults = .standard,
37+
keychain: ProviderKeychain = KeychainService.shared
38+
) {
39+
self.defaults = defaults
40+
self.keychain = keychain
41+
3642
self.migrateTranscriptionStartSoundIfNeeded()
3743
self.ensureDebugLoggingDefaults()
3844
self.migrateProviderAPIKeysIfNeeded()
@@ -1491,9 +1497,36 @@ final class SettingsStore: ObservableObject {
14911497
func saveProviderAPIKeys(_ values: [String: String]) throws -> [String: String] {
14921498
let trimmed = self.sanitizeAPIKeys(values)
14931499
try self.keychain.storeAllKeys(trimmed)
1500+
self.purgeRemovedProvidersFromLegacySources(keeping: trimmed)
14941501
return try self.keychain.fetchAllKeys()
14951502
}
14961503

1504+
/// While migration is incomplete, legacy plaintext defaults and per-provider Keychain
1505+
/// entries still exist. A provider removed from the consolidated store must also leave
1506+
/// those sources, or a later migration pass would resurrect the deleted key.
1507+
private func purgeRemovedProvidersFromLegacySources(keeping stored: [String: String]) {
1508+
guard self.defaults.bool(forKey: Keys.providerAPIKeyMigrationCompleted) == false else { return }
1509+
1510+
var legacyDefaults = (self.defaults.dictionary(forKey: Keys.providerAPIKeys) as? [String: String]) ?? [:]
1511+
let legacyKeychainIDs = (try? self.keychain.legacyProviderEntries().keys).map(Array.init) ?? []
1512+
let removed = Set(legacyDefaults.keys).union(legacyKeychainIDs).filter { stored[$0] == nil }
1513+
guard removed.isEmpty == false else { return }
1514+
1515+
let removedFromDefaults = removed.filter { legacyDefaults[$0] != nil }
1516+
if removedFromDefaults.isEmpty == false {
1517+
for providerID in removedFromDefaults {
1518+
legacyDefaults.removeValue(forKey: providerID)
1519+
}
1520+
if legacyDefaults.isEmpty {
1521+
self.defaults.removeObject(forKey: Keys.providerAPIKeys)
1522+
} else {
1523+
self.defaults.set(legacyDefaults, forKey: Keys.providerAPIKeys)
1524+
}
1525+
}
1526+
1527+
try? self.keychain.removeLegacyEntries(providerIDs: Array(removed))
1528+
}
1529+
14971530
/// Securely retrieve API key for a provider, handling custom prefix logic
14981531
func getAPIKey(for providerID: String) -> String? {
14991532
let keys = self.providerAPIKeys
@@ -3450,31 +3483,63 @@ final class SettingsStore: ObservableObject {
34503483
private func migrateProviderAPIKeysIfNeeded() {
34513484
self.defaults.removeObject(forKey: Keys.providerAPIKeyIdentifiers)
34523485

3486+
if self.defaults.bool(forKey: Keys.providerAPIKeyMigrationCompleted) {
3487+
self.defaults.removeObject(forKey: Keys.providerAPIKeys)
3488+
do {
3489+
let legacyKeychain = try self.keychain.legacyProviderEntries()
3490+
if legacyKeychain.isEmpty == false {
3491+
try? self.keychain.removeLegacyEntries(providerIDs: Array(legacyKeychain.keys))
3492+
}
3493+
} catch {
3494+
self.logProviderAPIKeyPersistenceFailure(error)
3495+
}
3496+
return
3497+
}
3498+
3499+
let legacyKeychain: [String: String]
3500+
let didReadLegacyKeychain: Bool
3501+
do {
3502+
legacyKeychain = try self.keychain.legacyProviderEntries()
3503+
didReadLegacyKeychain = true
3504+
} catch {
3505+
self.logProviderAPIKeyPersistenceFailure(error)
3506+
legacyKeychain = [:]
3507+
didReadLegacyKeychain = false
3508+
}
3509+
34533510
var merged = (try? self.keychain.fetchAllKeys()) ?? [:]
3454-
var didMutate = false
3511+
let legacyDefaults = (self.defaults.dictionary(forKey: Keys.providerAPIKeys) as? [String: String]) ?? [:]
3512+
let sanitizedLegacyDefaults = self.sanitizeAPIKeys(legacyDefaults)
3513+
let hasLegacySources = legacyDefaults.isEmpty == false || legacyKeychain.isEmpty == false
3514+
3515+
if legacyKeychain.isEmpty == false {
3516+
for (provider, key) in self.sanitizeAPIKeys(legacyKeychain) {
3517+
guard merged[provider] == nil || merged[provider] == sanitizedLegacyDefaults[provider] else {
3518+
continue
3519+
}
3520+
merged[provider] = key
3521+
}
3522+
}
34553523

3456-
if let legacyDefaults = defaults.dictionary(forKey: Keys.providerAPIKeys) as? [String: String],
3457-
legacyDefaults.isEmpty == false
3458-
{
3459-
merged.merge(self.sanitizeAPIKeys(legacyDefaults)) { _, new in new }
3460-
didMutate = true
3524+
if legacyDefaults.isEmpty == false {
3525+
merged.merge(sanitizedLegacyDefaults) { current, _ in current }
34613526
}
3462-
self.defaults.removeObject(forKey: Keys.providerAPIKeys)
34633527

3464-
if let legacyKeychain = try? keychain.legacyProviderEntries(),
3465-
legacyKeychain.isEmpty == false
3466-
{
3467-
merged.merge(self.sanitizeAPIKeys(legacyKeychain)) { _, new in new }
3468-
didMutate = true
3469-
try? self.keychain.removeLegacyEntries(providerIDs: Array(legacyKeychain.keys))
3528+
guard hasLegacySources else { return }
3529+
3530+
do {
3531+
_ = try self.saveProviderAPIKeys(merged)
3532+
} catch {
3533+
self.logProviderAPIKeyPersistenceFailure(error)
3534+
return
34703535
}
34713536

3472-
if didMutate {
3473-
do {
3474-
_ = try self.saveProviderAPIKeys(merged)
3475-
} catch {
3476-
self.logProviderAPIKeyPersistenceFailure(error)
3477-
}
3537+
guard didReadLegacyKeychain else { return }
3538+
3539+
self.defaults.set(true, forKey: Keys.providerAPIKeyMigrationCompleted)
3540+
self.defaults.removeObject(forKey: Keys.providerAPIKeys)
3541+
if legacyKeychain.isEmpty == false {
3542+
try? self.keychain.removeLegacyEntries(providerIDs: Array(legacyKeychain.keys))
34783543
}
34793544
}
34803545

@@ -3730,21 +3795,20 @@ final class SettingsStore: ObservableObject {
37303795
do {
37313796
try self.keychain.storeKey(trimmed, for: keyID)
37323797
didModify = true
3798+
decoded[index] = SavedProvider(
3799+
id: provider.id,
3800+
name: provider.name,
3801+
baseURL: provider.baseURL,
3802+
apiKey: "",
3803+
models: provider.models
3804+
)
37333805
} catch {
37343806
DebugLogger.shared
37353807
.error(
37363808
"Failed to migrate API key for \(provider.name): \(error.localizedDescription)",
37373809
source: "SettingsStore"
37383810
)
37393811
}
3740-
3741-
decoded[index] = SavedProvider(
3742-
id: provider.id,
3743-
name: provider.name,
3744-
baseURL: provider.baseURL,
3745-
apiKey: "",
3746-
models: provider.models
3747-
)
37483812
}
37493813

37503814
if didModify,
@@ -5210,6 +5274,27 @@ final class SettingsStore: ObservableObject {
52105274
}
52115275
}
52125276

5277+
#if DEBUG
5278+
extension SettingsStore {
5279+
static func makeForTesting(defaults: UserDefaults, keychain: ProviderKeychain) -> SettingsStore {
5280+
SettingsStore(defaults: defaults, keychain: keychain)
5281+
}
5282+
5283+
func replaceKeychainForTesting(_ keychain: ProviderKeychain) {
5284+
self.keychain = keychain
5285+
}
5286+
5287+
func migrateProviderAPIKeysForTesting() {
5288+
self.migrateProviderAPIKeysIfNeeded()
5289+
}
5290+
5291+
func scrubSavedProviderAPIKeysForTesting() {
5292+
self.scrubSavedProviderAPIKeys()
5293+
}
5294+
5295+
}
5296+
#endif
5297+
52135298
// swiftlint:enable type_body_length
52145299

52155300
private extension SettingsStore {
@@ -5233,6 +5318,7 @@ private extension SettingsStore {
52335318
static let privateAIContextDefaultMigratedTo4K = "PrivateAIProviderContextDefaultMigratedTo4K"
52345319
static let providerAPIKeys = "ProviderAPIKeys"
52355320
static let providerAPIKeyIdentifiers = "ProviderAPIKeyIdentifiers"
5321+
static let providerAPIKeyMigrationCompleted = "ProviderAPIKeyMigrationCompleted"
52365322
static let savedProviders = "SavedProviders"
52375323
static let verifiedProviderFingerprints = "VerifiedProviderFingerprints"
52385324
static let shareAnonymousAnalytics = "ShareAnonymousAnalytics"

0 commit comments

Comments
 (0)