From 0050b1369dc3cd9200681f0c932a3116db3e7fdd Mon Sep 17 00:00:00 2001 From: Raphael Barreiros Date: Sat, 3 Oct 2026 08:57:39 +0200 Subject: [PATCH 1/4] Add: Store API keys in the Keychain The 8 API keys and tokens in AppSettings are saved to the Keychain (after first unlock, this device only) and written blank to settings.json. A settings.json from an older build is migrated on launch. A key the Keychain can't read yet (before first unlock) is never overwritten or deleted, and one whose write fails stays in the file, so no key is lost. AppSettings keeps the values in memory, so views and services are unchanged. Closes #65 Co-Authored-By: Claude Opus 5.5 (1M context) --- OpenVision.xcodeproj/project.pbxproj | 8 ++ OpenVision/Managers/SettingsManager.swift | 61 +++++++++- OpenVision/Managers/SettingsSecrets.swift | 107 ++++++++++++++++++ OpenVision/Models/AppSettings.swift | 2 +- .../Services/OAuth/OAuthTokenStore.swift | 4 +- OpenVisionTests/SettingsSecretsTests.swift | 66 +++++++++++ README.md | 2 +- docs/architecture.md | 2 +- 8 files changed, 243 insertions(+), 9 deletions(-) create mode 100644 OpenVision/Managers/SettingsSecrets.swift create mode 100644 OpenVisionTests/SettingsSecretsTests.swift diff --git a/OpenVision.xcodeproj/project.pbxproj b/OpenVision.xcodeproj/project.pbxproj index f1093430..e15850b1 100644 --- a/OpenVision.xcodeproj/project.pbxproj +++ b/OpenVision.xcodeproj/project.pbxproj @@ -16,6 +16,7 @@ 1D0AEF3C7A810FF410C8F2E5 /* SessionRecorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 06B8E5499FB1BA51EF6F1741 /* SessionRecorder.swift */; }; 1EEA4B0E379A76881244FAB9 /* SettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3DE51DA812A9394A5064F60E /* SettingsView.swift */; }; 223474C328BCC3C81ADCB93D /* OpenAIService.swift in Sources */ = {isa = PBXBuildFile; fileRef = FFDF4A693D2CEB248757FBD3 /* OpenAIService.swift */; }; + F999D96ED7C879D8C3826786 /* SettingsSecretsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A2D0406DCB5F627611BBD2B4 /* SettingsSecretsTests.swift */; }; 2241E9878F41D5D5856C9502 /* HermesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 641FA355FB2C0B37F5575730 /* HermesTests.swift */; }; 23D894B8F06FA0DBE93C658B /* NeuralSpeech.swift in Sources */ = {isa = PBXBuildFile; fileRef = A82F52D647139EC398E043BF /* NeuralSpeech.swift */; }; 2947D6E5D6D5B7080C53D478 /* OnboardingView.swift in Sources */ = {isa = PBXBuildFile; fileRef = C27052ECD2FFC79652B5CDB3 /* OnboardingView.swift */; }; @@ -117,6 +118,7 @@ F1D86E9A3AB2A994A205AB83 /* GeminiSettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 773753CA73EBA9F8C78BD8E4 /* GeminiSettingsView.swift */; }; F5155F1DD262090EC1804C5F /* SwirlOrb.swift in Sources */ = {isa = PBXBuildFile; fileRef = A1E4A28F9C0979733D4ABDB2 /* SwirlOrb.swift */; }; F6489F185DBE88FE282819D1 /* DocumentStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = DAF763EF8FCBF8EC57D8B943 /* DocumentStore.swift */; }; + 92C71533063AC23805A32861 /* SettingsSecrets.swift in Sources */ = {isa = PBXBuildFile; fileRef = B50F1F8FB3F5F4D700EA591B /* SettingsSecrets.swift */; }; F7718545E4E6BB4A7773B564 /* SettingsManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = ADC230E2AE44E5132DC5AB01 /* SettingsManager.swift */; }; F7AB848D2FF4A3A019358670 /* CloudChat.swift in Sources */ = {isa = PBXBuildFile; fileRef = E6327DA000918E311DF6E9B8 /* CloudChat.swift */; }; F7B70D8A815C84149A038294 /* HermesGateway.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8B7B8054D97114E3269E13EC /* HermesGateway.swift */; }; @@ -178,6 +180,7 @@ 60E2210A5A20177647BDB5AD /* Config.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Config.swift; sourceTree = ""; }; 6102B753E2737E698B780C8E /* HermesSettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HermesSettingsView.swift; sourceTree = ""; }; 6188DD26571BACB30096654D /* Conversation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Conversation.swift; sourceTree = ""; }; + A2D0406DCB5F627611BBD2B4 /* SettingsSecretsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsSecretsTests.swift; sourceTree = ""; }; 641FA355FB2C0B37F5575730 /* HermesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HermesTests.swift; sourceTree = ""; }; 669EB2820345D230AB0F9FAC /* MetricsCollector+Configuration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MetricsCollector+Configuration.swift"; sourceTree = ""; }; 6BD412B34D09394FE85F0181 /* NeuralVoiceListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NeuralVoiceListView.swift; sourceTree = ""; }; @@ -204,6 +207,7 @@ A82F52D647139EC398E043BF /* NeuralSpeech.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NeuralSpeech.swift; sourceTree = ""; }; A900F00D2451513A29BC4C6F /* AIBackendProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AIBackendProtocol.swift; sourceTree = ""; }; ACA3BB4BDC73148785327C9A /* OpenVisionTests.xctest */ = {isa = PBXFileReference; includeInIndex = 0; lastKnownFileType = wrapper.cfbundle; path = OpenVisionTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; + B50F1F8FB3F5F4D700EA591B /* SettingsSecrets.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsSecrets.swift; sourceTree = ""; }; ADC230E2AE44E5132DC5AB01 /* SettingsManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsManager.swift; sourceTree = ""; }; ADCFF581A7AE50FBF35EA257 /* GeminiLiveService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeminiLiveService.swift; sourceTree = ""; }; B2B78D6C28BEFF1F8426997C /* AudioCaptureService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AudioCaptureService.swift; sourceTree = ""; }; @@ -570,6 +574,7 @@ children = ( 9BCCC84E6880175BD83F37FD /* ConversationManager.swift */, 8DD25C635E9A37E7D6CA7677 /* GlassesManager.swift */, + B50F1F8FB3F5F4D700EA591B /* SettingsSecrets.swift */, ADC230E2AE44E5132DC5AB01 /* SettingsManager.swift */, ); path = Managers; @@ -662,6 +667,7 @@ 39552315579536D518404FED /* DocumentChunkingTests.swift */, B88E1A3CDAF6C7B3D7DF36FE /* FrameChangeTests.swift */, 3A5609ACD2D7FE78F322E775 /* GrokTests.swift */, + A2D0406DCB5F627611BBD2B4 /* SettingsSecretsTests.swift */, 641FA355FB2C0B37F5575730 /* HermesTests.swift */, DD3DFE96152AAE9145F104D6 /* LocalAgentRouteTests.swift */, BE4D7E31C137C29246EB9281 /* NativeToolSupportTests.swift */, @@ -836,6 +842,7 @@ E9628B20FB4654FBBFC61EF9 /* DocumentChunkingTests.swift in Sources */, 9A7811CF026E8DF74F74C2A2 /* FrameChangeTests.swift in Sources */, 41DD47E61F53A95E861466C1 /* GrokTests.swift in Sources */, + F999D96ED7C879D8C3826786 /* SettingsSecretsTests.swift in Sources */, 2241E9878F41D5D5856C9502 /* HermesTests.swift in Sources */, DC6776A7CBFBB8E151FB6B85 /* LocalAgentRouteTests.swift in Sources */, 5F19226406CF5B705B57BACF /* NativeToolSupportTests.swift in Sources */, @@ -924,6 +931,7 @@ 5F9D55BB6AA2E4CF8AF85A49 /* PomodoroTool.swift in Sources */, CB6636C853387785316153FD /* ReminderTool.swift in Sources */, 1D0AEF3C7A810FF410C8F2E5 /* SessionRecorder.swift in Sources */, + 92C71533063AC23805A32861 /* SettingsSecrets.swift in Sources */, F7718545E4E6BB4A7773B564 /* SettingsManager.swift in Sources */, 1EEA4B0E379A76881244FAB9 /* SettingsView.swift in Sources */, 6C065C29CC13488E67A70363 /* SoundService.swift in Sources */, diff --git a/OpenVision/Managers/SettingsManager.swift b/OpenVision/Managers/SettingsManager.swift index 1b71297e..4764bca9 100644 --- a/OpenVision/Managers/SettingsManager.swift +++ b/OpenVision/Managers/SettingsManager.swift @@ -1,5 +1,6 @@ // OpenVision - SettingsManager.swift -// Singleton manager for settings persistence with debounced auto-save +// Singleton manager for settings persistence with debounced auto-save. +// API keys and tokens are saved to the Keychain (SettingsSecrets), not settings.json. import Foundation import Combine @@ -28,6 +29,14 @@ final class SettingsManager: ObservableObject { private var saveTask: Task? private let debounceInterval: TimeInterval = 0.5 + /// What the Keychain holds for each secret, so a save only writes the ones that changed. + private var storedSecrets: [String: String] = [:] + /// Secrets the Keychain couldn't be read for (before first unlock). Never written or deleted + /// until a read succeeds, so a locked launch can't wipe them. + private var unreadableSecrets: Set = [] + /// Secrets whose Keychain write failed: kept in settings.json rather than lost. + private var secretsKeptInFile: Set = [] + // MARK: - Callbacks /// Called when settings change (for live session updates) @@ -40,10 +49,22 @@ final class SettingsManager: ObservableObject { let documentsURL = FileManager.default.urls(for: .documentDirectory, in: .userDomainMask).first! settingsURL = documentsURL.appendingPathComponent("settings.json") - // Load existing settings or create defaults - settings = Self.loadSettings(from: settingsURL) + // Load existing settings or create defaults, then the secrets from the Keychain + let fileSettings = Self.loadSettings(from: settingsURL) + var keychain: [String: SettingsSecrets.Stored] = [:] + for (account, _) in SettingsSecrets.fields { keychain[account] = SettingsSecrets.read(account) } + let merged = SettingsSecrets.merge(file: fileSettings, keychain: keychain) + settings = merged.settings + unreadableSecrets = merged.unreadable + for (account, stored) in keychain { if case .value(let value) = stored { storedSecrets[account] = value } } print("[SettingsManager] Initialized with settings from: \(settingsURL.path)") + + // A settings.json from an older build still has keys in plain text: move them now. + if SettingsSecrets.fields.contains(where: { !fileSettings[keyPath: $0.keyPath].isEmpty }) { + print("[SettingsManager] Moving API keys from settings.json to the Keychain") + saveNow() + } } // MARK: - Public Methods @@ -96,10 +117,15 @@ final class SettingsManager: ObservableObject { } private func performSave() { + saveSecrets() do { let encoder = JSONEncoder() encoder.outputFormatting = [.prettyPrinted, .sortedKeys] - let data = try encoder.encode(settings) + // A secret typed while the Keychain was unreadable stays in the file until it isn't. + let pending = unreadableSecrets.filter { account in + SettingsSecrets.fields.contains { $0.account == account && !settings[keyPath: $0.keyPath].isEmpty } + } + let data = try encoder.encode(SettingsSecrets.forFile(settings, keepInFile: secretsKeptInFile.union(pending))) try data.write(to: settingsURL, options: .atomic) print("[SettingsManager] Settings saved") } catch { @@ -107,6 +133,33 @@ final class SettingsManager: ObservableObject { } } + /// Write the secrets that changed to the Keychain. + private func saveSecrets() { + for (account, keyPath) in SettingsSecrets.fields { + if unreadableSecrets.contains(account) { + // Try again: the phone may have been unlocked since launch. + switch SettingsSecrets.read(account) { + case .unreadable: + continue + case .value(let stored): + storedSecrets[account] = stored + if settings[keyPath: keyPath].isEmpty { settings[keyPath: keyPath] = stored } + case .none: + break + } + unreadableSecrets.remove(account) + } + let value = settings[keyPath: keyPath] + guard storedSecrets[account, default: ""] != value else { continue } + if SettingsSecrets.write(value, account: account) { + storedSecrets[account] = value + secretsKeptInFile.remove(account) + } else { + secretsKeptInFile.insert(account) + } + } + } + private static func loadSettings(from url: URL) -> AppSettings { guard FileManager.default.fileExists(atPath: url.path), let data = try? Data(contentsOf: url) else { diff --git a/OpenVision/Managers/SettingsSecrets.swift b/OpenVision/Managers/SettingsSecrets.swift new file mode 100644 index 00000000..c30b8626 --- /dev/null +++ b/OpenVision/Managers/SettingsSecrets.swift @@ -0,0 +1,107 @@ +// OpenVision - SettingsSecrets.swift +// API keys and tokens from AppSettings, kept in the Keychain instead of settings.json. +// +// AppSettings still holds them in memory, so the views and services read and write them as before. +// Only persistence changes: SettingsManager writes them here and saves settings.json with those +// fields blank. A settings.json from an older build still has them in plain text; loading it +// moves them here and the next save blanks them in the file. + +import Foundation +import Security + +enum SettingsSecrets { + + /// Every secret field, by its Keychain account. + static let fields: [(account: String, keyPath: WritableKeyPath)] = [ + ("openClawAuthToken", \.openClawAuthToken), + ("geminiAPIKey", \.geminiAPIKey), + ("openAIAPIKey", \.openAIAPIKey), + ("grokAPIKey", \.grokAPIKey), + ("hermesAPIKey", \.hermesAPIKey), + ("tavilyAPIKey", \.tavilyAPIKey), + ("telemetryToken", \.telemetryToken), + ("telemetryPassword", \.telemetryPassword), + ] + + /// What the Keychain said about one account: a value, nothing stored, or no answer (it can't + /// be read before the phone's first unlock). + enum Stored: Equatable { + case value(String) + case none + case unreadable + } + + /// The settings to use after loading: a value still in the file (an older build's) wins, since + /// it's what the user last set; otherwise the Keychain's. Also returns the accounts that + /// couldn't be read, which mustn't be overwritten or deleted until they can. + static func merge(file: AppSettings, keychain: [String: Stored]) -> (settings: AppSettings, unreadable: Set) { + var settings = file + var unreadable: Set = [] + for (account, keyPath) in fields where file[keyPath: keyPath].isEmpty { + switch keychain[account] ?? .none { + case .value(let value): settings[keyPath: keyPath] = value + case .none: break + case .unreadable: unreadable.insert(account) + } + } + return (settings, unreadable) + } + + /// The settings as written to settings.json: secrets blank, except any whose Keychain write + /// failed, which stay in the file rather than be lost. + static func forFile(_ settings: AppSettings, keepInFile: Set) -> AppSettings { + var stripped = settings + for (account, keyPath) in fields where !keepInFile.contains(account) { + stripped[keyPath: keyPath] = "" + } + return stripped + } + + // MARK: - Keychain + + private static let service = (Bundle.main.bundleIdentifier ?? "openvision") + ".settings" + + private static func baseQuery(_ account: String) -> [String: Any] { + [kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account] + } + + static func read(_ account: String) -> Stored { + var query = baseQuery(account) + query[kSecReturnData as String] = true + query[kSecMatchLimit as String] = kSecMatchLimitOne + var result: AnyObject? + let status = SecItemCopyMatching(query as CFDictionary, &result) + switch status { + case errSecSuccess: + guard let data = result as? Data, let value = String(data: data, encoding: .utf8) else { return .none } + return value.isEmpty ? .none : .value(value) + case errSecItemNotFound: + return .none + default: + NSLog("[Secrets] keychain read for %@ failed: %d", account, status) + return .unreadable + } + } + + /// Store `value`, or delete the item when it's empty. Returns whether the Keychain took it. + @discardableResult + static func write(_ value: String, account: String) -> Bool { + if value.isEmpty { + let status = SecItemDelete(baseQuery(account) as CFDictionary) + return status == errSecSuccess || status == errSecItemNotFound + } + // Accessible after first unlock, like the sign-in tokens: replies still work in a pocket. + let attributes: [String: Any] = [ + kSecValueData as String: Data(value.utf8), + kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, + ] + var status = SecItemUpdate(baseQuery(account) as CFDictionary, attributes as CFDictionary) + if status == errSecItemNotFound { + status = SecItemAdd(baseQuery(account).merging(attributes) { $1 } as CFDictionary, nil) + } + if status != errSecSuccess { NSLog("[Secrets] keychain save failed for %@: %d", account, status) } + return status == errSecSuccess + } +} diff --git a/OpenVision/Models/AppSettings.swift b/OpenVision/Models/AppSettings.swift index 472ccf13..91fdca21 100644 --- a/OpenVision/Models/AppSettings.swift +++ b/OpenVision/Models/AppSettings.swift @@ -158,7 +158,7 @@ enum TTSEngineType: String, Codable, CaseIterable, Identifiable { } } -/// App settings persisted to Documents/settings.json +/// App settings persisted to Documents/settings.json (API keys and tokens go to the Keychain; see SettingsSecrets) struct AppSettings: Codable, Equatable { // MARK: - AI Backend Selection diff --git a/OpenVision/Services/OAuth/OAuthTokenStore.swift b/OpenVision/Services/OAuth/OAuthTokenStore.swift index 9a6d19bc..c2c6e42d 100644 --- a/OpenVision/Services/OAuth/OAuthTokenStore.swift +++ b/OpenVision/Services/OAuth/OAuthTokenStore.swift @@ -1,8 +1,8 @@ // OpenVision - OAuthTokenStore.swift // Keychain persistence for subscription sign-ins, plus refresh-on-demand. // -// Unlike API keys (settings.json), these are long-lived refresh tokens for the user's whole -// subscription account, so they live in the Keychain. Accessible after first unlock (not "when +// These are long-lived refresh tokens for the user's whole subscription account, so they live in +// the Keychain (API keys too, via SettingsSecrets). Accessible after first unlock (not "when // unlocked") so a reply can still go out while the phone is locked in a pocket. // // Refreshes are single-flight per provider: providers rotate the refresh token on use, so two diff --git a/OpenVisionTests/SettingsSecretsTests.swift b/OpenVisionTests/SettingsSecretsTests.swift new file mode 100644 index 00000000..84ab3aab --- /dev/null +++ b/OpenVisionTests/SettingsSecretsTests.swift @@ -0,0 +1,66 @@ +// OpenVision - SettingsSecretsTests.swift +// API keys move from settings.json to the Keychain without being lost on the way. + +import XCTest +@testable import OpenVision + +final class SettingsSecretsTests: XCTestCase { + + func testEverySecretFieldIsCovered() { + let accounts = SettingsSecrets.fields.map(\.account) + XCTAssertEqual(Set(accounts).count, accounts.count, "one Keychain account per field") + XCTAssertTrue(accounts.contains("hermesAPIKey")) + XCTAssertTrue(accounts.contains("telemetryPassword")) + } + + func testFileIsWrittenWithoutSecrets() throws { + var settings = AppSettings() + settings.hermesAPIKey = "hermes-key" + settings.openAIAPIKey = "sk-test" + settings.openClawGatewayURL = "wss://gateway.example" + let json = String(decoding: try JSONEncoder().encode(SettingsSecrets.forFile(settings, keepInFile: [])), as: UTF8.self) + XCTAssertFalse(json.contains("hermes-key") || json.contains("sk-test")) + XCTAssertTrue(json.contains("gateway.example"), "other settings stay in the file") + } + + func testAFailedKeychainWriteKeepsTheKeyInTheFile() { + var settings = AppSettings() + settings.grokAPIKey = "xai-key" + settings.geminiAPIKey = "gemini-key" + let file = SettingsSecrets.forFile(settings, keepInFile: ["grokAPIKey"]) + XCTAssertEqual(file.grokAPIKey, "xai-key") + XCTAssertEqual(file.geminiAPIKey, "") + } + + func testLoadingTakesKeysFromTheKeychain() { + let merged = SettingsSecrets.merge(file: AppSettings(), keychain: ["hermesAPIKey": .value("from-keychain")]) + XCTAssertEqual(merged.settings.hermesAPIKey, "from-keychain") + XCTAssertTrue(merged.unreadable.isEmpty) + } + + func testAnOlderFilesKeyWinsSoItCanBeMoved() { + var file = AppSettings() + file.tavilyAPIKey = "from-old-file" + let merged = SettingsSecrets.merge(file: file, keychain: ["tavilyAPIKey": .value("stale")]) + XCTAssertEqual(merged.settings.tavilyAPIKey, "from-old-file") + } + + func testAnUnreadableKeychainIsRememberedNotTreatedAsEmpty() { + let merged = SettingsSecrets.merge(file: AppSettings(), keychain: ["openAIAPIKey": .unreadable]) + XCTAssertEqual(merged.settings.openAIAPIKey, "") + XCTAssertEqual(merged.unreadable, ["openAIAPIKey"], "so a save can't delete the stored key") + } + + func testKeychainRoundTrip() { + // A test-only account, so the app's real keys on the device are never touched. + let account = "test-\(UUID().uuidString)" + defer { SettingsSecrets.write("", account: account) } + XCTAssertEqual(SettingsSecrets.read(account), .none) + XCTAssertTrue(SettingsSecrets.write("first", account: account)) + XCTAssertEqual(SettingsSecrets.read(account), .value("first")) + XCTAssertTrue(SettingsSecrets.write("second", account: account)) + XCTAssertEqual(SettingsSecrets.read(account), .value("second")) + XCTAssertTrue(SettingsSecrets.write("", account: account), "clearing a key deletes it") + XCTAssertEqual(SettingsSecrets.read(account), .none) + } +} diff --git a/README.md b/README.md index 9a05dc73..79a75d29 100644 --- a/README.md +++ b/README.md @@ -402,7 +402,7 @@ Face recognition, web search, and conversation memory all run on the **on-device ├─────────────────────────────────────────────────────────────────┤ │ Managers │ │ ├── GlassesManager Meta DAT SDK wrapper │ -│ ├── SettingsManager JSON persistence with debounce │ +│ ├── SettingsManager JSON settings, API keys in Keychain │ │ └── ConversationManager Chat history storage │ ├─────────────────────────────────────────────────────────────────┤ │ External │ diff --git a/docs/architecture.md b/docs/architecture.md index af9e9154..b5382256 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -6,7 +6,7 @@ How OpenVision is organized, and the seams to use when extending it. OpenVision/ ├── App/ App entry (OpenVisionApp) ├── Config/ Constants + xcconfig-backed configuration (zero hardcoding) -├── Managers/ App-wide state: SettingsManager, GlassesManager +├── Managers/ App-wide state: SettingsManager (API keys in the Keychain), GlassesManager ├── Models/ Value types & settings enums (AIBackendType, TTSEngineType, …) ├── Services/ One folder per domain — the backbone of the app │ ├── AIBackend/ AIBackend protocol + registry + conformances, OpenAIService, From 899fafafd137c4ee186eddf2409e8363b0b6c3a5 Mon Sep 17 00:00:00 2001 From: Raphael Barreiros Date: Sat, 3 Oct 2026 10:16:38 +0200 Subject: [PATCH 2/4] Fix: Keychain secrets after a reset or a failed write - Resetting to defaults while a key couldn't be read yet deletes it once the Keychain is readable, instead of restoring it. - A key kept in settings.json after a failed write leaves the file as soon as the Keychain holds the current value again. Co-Authored-By: Claude Opus 5.5 (1M context) --- OpenVision/Managers/SettingsManager.swift | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/OpenVision/Managers/SettingsManager.swift b/OpenVision/Managers/SettingsManager.swift index 4764bca9..5d2ee46d 100644 --- a/OpenVision/Managers/SettingsManager.swift +++ b/OpenVision/Managers/SettingsManager.swift @@ -36,6 +36,8 @@ final class SettingsManager: ObservableObject { private var unreadableSecrets: Set = [] /// Secrets whose Keychain write failed: kept in settings.json rather than lost. private var secretsKeptInFile: Set = [] + /// Unreadable secrets the user reset: deleted, not restored, once the Keychain can be read. + private var secretsToClear: Set = [] // MARK: - Callbacks @@ -78,6 +80,7 @@ final class SettingsManager: ObservableObject { /// Reset settings to defaults func resetToDefaults() { + secretsToClear = unreadableSecrets settings = AppSettings() saveNow() } @@ -143,14 +146,22 @@ final class SettingsManager: ObservableObject { continue case .value(let stored): storedSecrets[account] = stored - if settings[keyPath: keyPath].isEmpty { settings[keyPath: keyPath] = stored } + // Reset since launch: the write below deletes it instead. + if settings[keyPath: keyPath].isEmpty && !secretsToClear.contains(account) { + settings[keyPath: keyPath] = stored + } case .none: break } unreadableSecrets.remove(account) + secretsToClear.remove(account) } let value = settings[keyPath: keyPath] - guard storedSecrets[account, default: ""] != value else { continue } + guard storedSecrets[account, default: ""] != value else { + // The Keychain already has it (e.g. changed back after a failed write). + secretsKeptInFile.remove(account) + continue + } if SettingsSecrets.write(value, account: account) { storedSecrets[account] = value secretsKeptInFile.remove(account) From 478f2fc506c23f0672cb69f77cb26e01efe9ddb9 Mon Sep 17 00:00:00 2001 From: Raphael Barreiros Date: Sun, 4 Oct 2026 06:49:43 +0200 Subject: [PATCH 3/4] Fix: Reload unreadable Keychain secrets on unlock; let keys migrate - Secrets the Keychain couldn't read at launch (a relaunch before first unlock) are read again when protected data becomes available and when the app becomes active, then restored into settings so the backends pick them up. Before, that only happened on the next save. - API keys use kSecAttrAccessibleAfterFirstUnlock (not ThisDeviceOnly), so they move with an encrypted backup or Quick Start to a new phone, as they did in settings.json. Sign-in tokens stay device-bound. Co-Authored-By: Claude Opus 5.5 (1M context) --- OpenVision/Managers/SettingsManager.swift | 57 ++++++++++++++++------- OpenVision/Managers/SettingsSecrets.swift | 6 ++- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/OpenVision/Managers/SettingsManager.swift b/OpenVision/Managers/SettingsManager.swift index 5d2ee46d..a7ebaede 100644 --- a/OpenVision/Managers/SettingsManager.swift +++ b/OpenVision/Managers/SettingsManager.swift @@ -4,6 +4,7 @@ import Foundation import Combine +import UIKit /// Manages app settings with JSON persistence and debounced saving @MainActor @@ -67,6 +68,41 @@ final class SettingsManager: ObservableObject { print("[SettingsManager] Moving API keys from settings.json to the Keychain") saveNow() } + + // A launch before first unlock (Bluetooth relaunch after a reboot) can't read the + // Keychain: load the keys as soon as it can, without waiting for a settings change. + let center = NotificationCenter.default + for name in [UIApplication.protectedDataDidBecomeAvailableNotification, UIApplication.didBecomeActiveNotification] { + center.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in + MainActor.assumeIsolated { self?.reloadUnreadableSecrets() } + } + } + } + + /// Read again the secrets the Keychain couldn't give us at launch, and put them back into + /// `settings` so the backends see them (the change saves and fires `onSettingsChanged`). + func reloadUnreadableSecrets() { + guard !unreadableSecrets.isEmpty else { return } + var resetWhileLocked = false + for (account, keyPath) in SettingsSecrets.fields where unreadableSecrets.contains(account) { + switch SettingsSecrets.read(account) { + case .unreadable: + continue + case .value(let stored): + storedSecrets[account] = stored + if secretsToClear.contains(account) { + resetWhileLocked = true // the save below deletes it + } else if settings[keyPath: keyPath].isEmpty { + settings[keyPath: keyPath] = stored + } + case .none: + break + } + unreadableSecrets.remove(account) + secretsToClear.remove(account) + } + print("[SettingsManager] Reloaded Keychain secrets; \(unreadableSecrets.count) still unreadable") + if resetWhileLocked { scheduleSave() } } // MARK: - Public Methods @@ -138,24 +174,9 @@ final class SettingsManager: ObservableObject { /// Write the secrets that changed to the Keychain. private func saveSecrets() { - for (account, keyPath) in SettingsSecrets.fields { - if unreadableSecrets.contains(account) { - // Try again: the phone may have been unlocked since launch. - switch SettingsSecrets.read(account) { - case .unreadable: - continue - case .value(let stored): - storedSecrets[account] = stored - // Reset since launch: the write below deletes it instead. - if settings[keyPath: keyPath].isEmpty && !secretsToClear.contains(account) { - settings[keyPath: keyPath] = stored - } - case .none: - break - } - unreadableSecrets.remove(account) - secretsToClear.remove(account) - } + // Try again first: the phone may have been unlocked since launch. + reloadUnreadableSecrets() + for (account, keyPath) in SettingsSecrets.fields where !unreadableSecrets.contains(account) { let value = settings[keyPath: keyPath] guard storedSecrets[account, default: ""] != value else { // The Keychain already has it (e.g. changed back after a failed write). diff --git a/OpenVision/Managers/SettingsSecrets.swift b/OpenVision/Managers/SettingsSecrets.swift index c30b8626..7aaa0fcf 100644 --- a/OpenVision/Managers/SettingsSecrets.swift +++ b/OpenVision/Managers/SettingsSecrets.swift @@ -92,10 +92,12 @@ enum SettingsSecrets { let status = SecItemDelete(baseQuery(account) as CFDictionary) return status == errSecSuccess || status == errSecItemNotFound } - // Accessible after first unlock, like the sign-in tokens: replies still work in a pocket. + // Accessible after first unlock, so replies still work in a pocket. Not "this device only" + // (unlike the sign-in tokens): pasted API keys travel with an encrypted backup or Quick + // Start to a new phone, as they did when they lived in settings.json. let attributes: [String: Any] = [ kSecValueData as String: Data(value.utf8), - kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly, + kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock, ] var status = SecItemUpdate(baseQuery(account) as CFDictionary, attributes as CFDictionary) if status == errSecItemNotFound { From 2eb5e83c67831bc5e6fae30177ed39636d0f3f63 Mon Sep 17 00:00:00 2001 From: Raphael Barreiros Date: Sun, 4 Oct 2026 07:18:37 +0200 Subject: [PATCH 4/4] Fix: Keep a cleared key cleared when its Keychain delete fails A failed delete is remembered across launches (UserDefaults, account names only). Until it succeeds, the key isn't loaded back from the Keychain, and every save retries the delete. Co-Authored-By: Claude Opus 5.5 (1M context) --- OpenVision/Managers/SettingsManager.swift | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/OpenVision/Managers/SettingsManager.swift b/OpenVision/Managers/SettingsManager.swift index a7ebaede..f26470f2 100644 --- a/OpenVision/Managers/SettingsManager.swift +++ b/OpenVision/Managers/SettingsManager.swift @@ -39,6 +39,12 @@ final class SettingsManager: ObservableObject { private var secretsKeptInFile: Set = [] /// Unreadable secrets the user reset: deleted, not restored, once the Keychain can be read. private var secretsToClear: Set = [] + /// Cleared secrets whose Keychain delete failed. Kept across launches, so a value still in + /// the Keychain isn't loaded back; the delete is retried on every save until it succeeds. + private var pendingDeletions = Set(UserDefaults.standard.stringArray(forKey: SettingsManager.pendingDeletionsKey) ?? []) { + didSet { UserDefaults.standard.set(Array(pendingDeletions), forKey: Self.pendingDeletionsKey) } + } + private static let pendingDeletionsKey = "SettingsSecrets.pendingDeletions" // MARK: - Callbacks @@ -54,8 +60,14 @@ final class SettingsManager: ObservableObject { // Load existing settings or create defaults, then the secrets from the Keychain let fileSettings = Self.loadSettings(from: settingsURL) + for account in pendingDeletions where SettingsSecrets.write("", account: account) { + pendingDeletions.remove(account) + } var keychain: [String: SettingsSecrets.Stored] = [:] - for (account, _) in SettingsSecrets.fields { keychain[account] = SettingsSecrets.read(account) } + for (account, _) in SettingsSecrets.fields { + // A key the user cleared stays cleared, even if the Keychain still has it. + keychain[account] = pendingDeletions.contains(account) ? .none : SettingsSecrets.read(account) + } let merged = SettingsSecrets.merge(file: fileSettings, keychain: keychain) settings = merged.settings unreadableSecrets = merged.unreadable @@ -178,7 +190,8 @@ final class SettingsManager: ObservableObject { reloadUnreadableSecrets() for (account, keyPath) in SettingsSecrets.fields where !unreadableSecrets.contains(account) { let value = settings[keyPath: keyPath] - guard storedSecrets[account, default: ""] != value else { + let retryDelete = value.isEmpty && pendingDeletions.contains(account) + guard storedSecrets[account, default: ""] != value || retryDelete else { // The Keychain already has it (e.g. changed back after a failed write). secretsKeptInFile.remove(account) continue @@ -186,6 +199,9 @@ final class SettingsManager: ObservableObject { if SettingsSecrets.write(value, account: account) { storedSecrets[account] = value secretsKeptInFile.remove(account) + if pendingDeletions.contains(account) { pendingDeletions.remove(account) } + } else if value.isEmpty { + pendingDeletions.insert(account) // retried, and never loaded back meanwhile } else { secretsKeptInFile.insert(account) }