diff --git a/OSGKeyboard/OSGKeyboard.entitlements b/OSGKeyboard/OSGKeyboard.entitlements index 2160d46..f8f12d1 100644 --- a/OSGKeyboard/OSGKeyboard.entitlements +++ b/OSGKeyboard/OSGKeyboard.entitlements @@ -8,5 +8,9 @@ com.apple.security.device.audio-input + com.apple.security.keychain-access-groups + + com.osgkeyboard.shared + diff --git a/OSGKeyboardExt/KeyboardViewController.swift b/OSGKeyboardExt/KeyboardViewController.swift index d0443ee..8a5e88a 100644 --- a/OSGKeyboardExt/KeyboardViewController.swift +++ b/OSGKeyboardExt/KeyboardViewController.swift @@ -125,7 +125,16 @@ public final class KeyboardViewController: UIInputViewController { // MARK: - Press handlers private func pressBegan() { - guard state.phase == .idle else { return } + // Allow re-entry from `.denied` and from a finished/cleared + // `.error` so the user can simply press the mic again after + // returning from Settings with permission granted — they + // shouldn't have to wait for an auto-clear timer. + switch state.phase { + case .idle, .denied, .error: + break + default: + return + } guard state.mode != .off else { return } // Set the intermediate phase SYNCHRONOUSLY so a rapid second // press (before the first Task has had a chance to flip phase to @@ -138,7 +147,6 @@ public final class KeyboardViewController: UIInputViewController { let micGranted = await self.permissions.requestMicPermission() guard micGranted else { self.state.phase = .denied(.mic) - self.scheduleAutoClearError() return } // iOS 18 SFSpeechRecognizer path: we explicitly ask for Speech @@ -152,7 +160,6 @@ public final class KeyboardViewController: UIInputViewController { let speechGranted = await self.permissions.requestSpeechPermission() guard speechGranted else { self.state.phase = .denied(.speech) - self.scheduleAutoClearError() return } self.startPipeline() @@ -357,11 +364,12 @@ public final class KeyboardViewController: UIInputViewController { Task { @MainActor [weak self] in try? await Task.sleep(nanoseconds: 2_400_000_000) guard let self else { return } - // Clear both .error (transient error message) and .denied - // (permission was rejected — show the message, then return - // to idle so the user can navigate away). + // Only transient errors auto-clear. `.denied` is sticky: the + // user needs the message long enough to read it AND decide + // whether to tap "去设置" or tap the mic to retry. They + // dismiss it implicitly by doing either of those things. switch self.state.phase { - case .error, .denied: + case .error: self.state.phase = .idle default: break diff --git a/OSGKeyboardExt/OSGKeyboardExt.entitlements b/OSGKeyboardExt/OSGKeyboardExt.entitlements index 9d4ffbc..2febc2b 100644 --- a/OSGKeyboardExt/OSGKeyboardExt.entitlements +++ b/OSGKeyboardExt/OSGKeyboardExt.entitlements @@ -6,5 +6,9 @@ group.com.osgkeyboard.shared + com.apple.security.keychain-access-groups + + com.osgkeyboard.shared + diff --git a/OSGKeyboardExt/Views/KeyboardRootView.swift b/OSGKeyboardExt/Views/KeyboardRootView.swift index 22a1182..738a121 100644 --- a/OSGKeyboardExt/Views/KeyboardRootView.swift +++ b/OSGKeyboardExt/Views/KeyboardRootView.swift @@ -106,7 +106,11 @@ public struct KeyboardRootView: View { private var centreArea: some View { ZStack { VStack(spacing: Spacing.xxs) { - TranscriptLine(phase: state.phase, transcript: state.lastTranscript) + TranscriptLine( + phase: state.phase, + transcript: state.lastTranscript, + openSettings: state.openSettings + ) .frame(height: 22) RecordButton( phase: buttonPhase, @@ -200,6 +204,7 @@ private struct TranscriptLine: View { let phase: KeyboardViewController.State.Phase let transcript: String + let openSettings: () -> Void var body: some View { ZStack { @@ -236,11 +241,21 @@ private struct TranscriptLine: View { .lineLimit(1) .truncationMode(.tail) case .denied(let reason): - Text(deniedMessage(for: reason)) + Button(action: openSettings) { + HStack(spacing: 4) { + Text(deniedMessage(for: reason)) + .lineLimit(1) + .truncationMode(.tail) + Image(systemName: "chevron.right") + .font(.system(size: 10, weight: .semibold)) + } .font(TypeStyle.caption) .foregroundStyle(palette.warning) - .lineLimit(1) - .truncationMode(.tail) + .frame(maxWidth: .infinity) + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .accessibilityHint(Text("Opens the OSGKeyboard settings page where you can grant microphone or speech recognition access.")) } } .frame(maxWidth: .infinity) @@ -249,8 +264,8 @@ private struct TranscriptLine: View { private func deniedMessage(for reason: KeyboardViewController.State.Phase.Reason) -> String { switch reason { - case .mic: return "麦克风被拒绝 · 请到「设置」中允许" - case .speech: return "语音识别被拒绝 · 请到「设置」中允许" + case .mic: return "麦克风被拒绝" + case .speech: return "语音识别被拒绝" } } } diff --git a/OSGKeyboardShared/Models/ProviderConfig.swift b/OSGKeyboardShared/Models/ProviderConfig.swift index 3457ca2..5300144 100644 --- a/OSGKeyboardShared/Models/ProviderConfig.swift +++ b/OSGKeyboardShared/Models/ProviderConfig.swift @@ -3,6 +3,11 @@ // // User's LLM configuration. Persisted in App Group UserDefaults so both // the main app and keyboard extension read the same values. +// +// `apiKey` is the exception: it lives in the Keychain (see +// `Keychain.swift`) for at-rest encryption. The first time this struct +// inits after upgrade, a legacy plaintext value from UserDefaults is +// migrated to the Keychain and removed from UserDefaults. import Foundation import Combine @@ -13,7 +18,10 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable { private enum Key { static let providerId = "config.providerId" static let baseURL = "config.baseURL" - static let apiKey = "config.apiKey" + // Legacy: apiKey used to live in UserDefaults before the + // migration. We still read it once (see init below) and then + // delete the entry, but no other code path touches this key. + static let apiKeyLegacy = "config.apiKey" static let model = "config.model" static let systemPrompt = "config.systemPrompt" static let modeId = "config.modeId" @@ -27,7 +35,18 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable { didSet { defaults.set(baseURL, forKey: Key.baseURL) } } @Published public var apiKey: String { - didSet { defaults.set(apiKey, forKey: Key.apiKey) } + didSet { + // Skip the round-trip on init — we read from Keychain and + // writing the same value back is wasteful. + guard oldValue != apiKey else { return } + do { + try Keychain.setAPIKey(apiKey) + } catch { + #if DEBUG + print("⚠️ [OSGKeyboard] Keychain write failed: \(error)") + #endif + } + } } @Published public var model: String { didSet { defaults.set(model, forKey: Key.model) } @@ -60,7 +79,13 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable { let preset = LLMProvider.provider(id: pid) self.providerId = pid self.baseURL = defaults.string(forKey: Key.baseURL) ?? preset.defaultBaseURL - self.apiKey = defaults.string(forKey: Key.apiKey) ?? "" + + // Resolve the API key with a one-shot migration from the legacy + // UserDefaults slot. After this runs once, `Key.apiKeyLegacy` + // is empty in the suite and all subsequent reads go through the + // Keychain. + self.apiKey = ProviderConfig.resolveAPIKey(defaults: defaults) + self.model = defaults.string(forKey: Key.model) ?? preset.defaultModel self.systemPrompt = defaults.string(forKey: Key.systemPrompt) ?? AppGroupStore.defaultSystemPrompt(for: pid) @@ -68,6 +93,21 @@ public final class ProviderConfig: ObservableObject, @unchecked Sendable { self.localeId = defaults.string(forKey: Key.localeId) ?? "auto" } + /// Read the API key from the Keychain, falling back to a one-time + /// migration from the legacy UserDefaults slot. + private static func resolveAPIKey(defaults: UserDefaults) -> String { + if let stored = Keychain.apiKey(), !stored.isEmpty { + return stored + } + if let legacy = defaults.string(forKey: Key.apiKeyLegacy), + !legacy.isEmpty { + try? Keychain.setAPIKey(legacy) + defaults.removeObject(forKey: Key.apiKeyLegacy) + return legacy + } + return "" + } + public func apply(preset: LLMProvider) { // Capture the *previous* provider id BEFORE we mutate, so the // system-prompt reset check below can compare against the actual diff --git a/OSGKeyboardShared/Services/AppGroupStore.swift b/OSGKeyboardShared/Services/AppGroupStore.swift index 86c031a..0937cb9 100644 --- a/OSGKeyboardShared/Services/AppGroupStore.swift +++ b/OSGKeyboardShared/Services/AppGroupStore.swift @@ -4,6 +4,10 @@ // Convenience wrapper around App Group UserDefaults for non-Published reads. // Used by the keyboard extension (no SwiftUI) to read config without // instantiating an ObservableObject. +// +// `apiKey` is NOT read from UserDefaults — see `Keychain.swift`. We +// share access between the host app and the keyboard extension via a +// shared keychain-access-group declared in both targets' entitlements. import Foundation @@ -19,7 +23,6 @@ public struct AppGroupStore: @unchecked Sendable { private enum Key { static let providerId = "config.providerId" static let baseURL = "config.baseURL" - static let apiKey = "config.apiKey" static let model = "config.model" static let systemPrompt = "config.systemPrompt" static let modeId = "config.modeId" @@ -36,8 +39,11 @@ public struct AppGroupStore: @unchecked Sendable { defaults.string(forKey: Key.baseURL) ?? LLMProvider.provider(id: providerId).defaultBaseURL } + /// API key lives in the Keychain (cross-process, encrypted at rest). + /// Returns "" when nothing is stored so the LLMClient can surface a + /// `noAPIKey` error rather than firing off an obviously-bad request. public var apiKey: String { - defaults.string(forKey: Key.apiKey) ?? "" + Keychain.apiKey() ?? "" } public var model: String { diff --git a/OSGKeyboardShared/Services/Keychain.swift b/OSGKeyboardShared/Services/Keychain.swift new file mode 100644 index 0000000..5983519 --- /dev/null +++ b/OSGKeyboardShared/Services/Keychain.swift @@ -0,0 +1,130 @@ +// Keychain.swift +// OSGKeyboard · Shared +// +// Single-purpose Keychain helper for the user's LLM API key. +// +// Why this exists +// --------------- +// Both the host app and the keyboard extension need to read the same API +// key (the host writes it in Settings; the extension uses it to +// authenticate LLM requests). Storing it in App Group `UserDefaults` is +// plaintext on disk and shows up in any unencrypted backup. The Keychain +// gives us at-rest encryption and proper lifecycle. +// +// Cross-process sharing +// --------------------- +// App and extension have different bundle IDs, so their default Keychain +// access groups differ and they cannot see each other's items out of the +// box. We add `com.apple.security.keychain-access-groups` to both +// targets' entitlements with the entry `com.osgkeyboard.shared`; this +// becomes each process's *first* (and therefore default) access group, so +// we never need to specify `kSecAttrAccessGroup` in queries — the system +// resolves it for us. +// +// Accessibility class +// ------------------- +// `kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`: +// - Available after the user unlocks the device at least once after +// boot (so background jobs work even with a locked phone). +// - "ThisDeviceOnly" — does not migrate to a restored device and is +// NOT included in iCloud Keychain. API keys should not sync. + +import Foundation +import Security + +public enum Keychain: @unchecked Sendable { + + public enum KeychainError: Error, Sendable, Equatable { + case unexpectedStatus(OSStatus) + } + + private static let service = "com.osgkeyboard.apikey" + private static let account = "current" + + // MARK: - Read + + /// Read the stored API key. Returns `nil` when nothing is stored, + /// or when the underlying call returns a non-success status we can't + /// usefully surface (e.g. transient `errSecInteractionNotAllowed`). + public static func apiKey() -> String? { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + kSecReturnData as String: true, + kSecMatchLimit as String: kSecMatchLimitOne, + ] + var result: CFTypeRef? + let status = SecItemCopyMatching(query as CFDictionary, &result) + switch status { + case errSecSuccess: + guard let data = result as? Data, + let str = String(data: data, encoding: .utf8) else { + return nil + } + return str + case errSecItemNotFound: + return nil + default: + #if DEBUG + print("⚠️ [OSGKeyboard] Keychain read returned OSStatus \(status); treating as no key.") + #endif + return nil + } + } + + // MARK: - Write + + /// Store (or update) the API key. An empty string deletes the entry, + /// so clearing the field in the UI removes the key from the Keychain + /// rather than leaving an empty-string placeholder. + public static func setAPIKey(_ key: String) throws { + if key.isEmpty { + try deleteAPIKey() + return + } + let data = Data(key.utf8) + let baseQuery: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + ] + // Try update first — covers the common path where the key already + // exists (every settings edit after the first). + let updateAttrs: [String: Any] = [ + kSecValueData as String: data, + ] + let updateStatus = SecItemUpdate(baseQuery as CFDictionary, updateAttrs as CFDictionary) + switch updateStatus { + case errSecSuccess: + return + case errSecItemNotFound: + // No existing item — add one with our accessibility class. + var addQuery = baseQuery + addQuery[kSecValueData as String] = data + addQuery[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly + let addStatus = SecItemAdd(addQuery as CFDictionary, nil) + if addStatus != errSecSuccess { + throw KeychainError.unexpectedStatus(addStatus) + } + default: + throw KeychainError.unexpectedStatus(updateStatus) + } + } + + // MARK: - Delete + + public static func deleteAPIKey() throws { + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: service, + kSecAttrAccount as String: account, + ] + let status = SecItemDelete(query as CFDictionary) + // `errSecItemNotFound` is success-from-the-user's-perspective — the + // desired end state is "no key", which is what we already have. + if status != errSecSuccess && status != errSecItemNotFound { + throw KeychainError.unexpectedStatus(status) + } + } +} diff --git a/OSGKeyboardTests/KeychainTests.swift b/OSGKeyboardTests/KeychainTests.swift new file mode 100644 index 0000000..a53bea8 --- /dev/null +++ b/OSGKeyboardTests/KeychainTests.swift @@ -0,0 +1,128 @@ +// KeychainTests.swift +// OSGKeyboard · Tests +// +// Unit tests for the Keychain helper and the one-time migration from +// the legacy UserDefaults slot. The Keychain is process-global in the +// simulator, so every test cleans up after itself. + +import XCTest +@testable import OSGKeyboardShared + +final class KeychainTests: XCTestCase { + + override func setUpWithError() throws { + try? Keychain.deleteAPIKey() + } + + override func tearDownWithError() throws { + try? Keychain.deleteAPIKey() + } + + // MARK: - Round-trip + + func testRoundTripWriteReadDelete() throws { + // Nothing stored yet. + XCTAssertNil(Keychain.apiKey(), "Keychain should start empty after cleanup") + + // Write → read. + try Keychain.setAPIKey("sk-roundtrip-1") + XCTAssertEqual(Keychain.apiKey(), "sk-roundtrip-1") + + // Overwrite → read new value (no orphan entries). + try Keychain.setAPIKey("sk-roundtrip-2") + XCTAssertEqual(Keychain.apiKey(), "sk-roundtrip-2") + + // Delete → read nil. + try Keychain.deleteAPIKey() + XCTAssertNil(Keychain.apiKey()) + } + + /// Empty string must DELETE the entry, not store an empty placeholder. + /// Otherwise `Keychain.apiKey() ?? ""` would always return "" for any + /// missing item, and the LLM client couldn't tell "stored but empty" + /// (user error) from "not stored" (onboarding state). + func testEmptyStringDeletes() throws { + try Keychain.setAPIKey("sk-temp") + XCTAssertEqual(Keychain.apiKey(), "sk-temp") + + try Keychain.setAPIKey("") + XCTAssertNil(Keychain.apiKey(), "Empty write must delete, not store empty string") + } + + /// Deleting a non-existent entry must be a no-op (idempotent), not an + /// error — callers like `ProviderConfig.reset()` invoke it + /// unconditionally. + func testDeleteIsIdempotent() throws { + // No prior write — should not throw. + XCTAssertNoThrow(try Keychain.deleteAPIKey()) + XCTAssertNoThrow(try Keychain.deleteAPIKey()) + } + + // MARK: - AppGroupStore reading + + /// `AppGroupStore.apiKey` must consult the Keychain (not UserDefaults), + /// otherwise the keyboard extension never sees the key set in the + /// host app's Settings UI. + func testAppGroupStoreReadsFromKeychain() throws { + let suiteName = "group.com.osgkeyboard.shared.tests.kc.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defer { defaults.removePersistentDomain(forName: suiteName) } + + try Keychain.setAPIKey("sk-from-store") + let store = AppGroupStore(defaults: defaults) + XCTAssertEqual(store.apiKey, "sk-from-store") + } + + // MARK: - Legacy migration + + /// Pre-Keychain versions of the app stored the API key in + /// `config.apiKey` (UserDefaults). On first init after upgrade, that + /// value must be moved to the Keychain and removed from UserDefaults + /// — otherwise a fresh install in a clean simulator would inherit the + /// stale plaintext value. + func testLegacyUserDefaultsKeyIsMigratedToKeychain() { + let suiteName = "group.com.osgkeyboard.shared.tests.migration.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defaults.removePersistentDomain(forName: suiteName) + defer { defaults.removePersistentDomain(forName: suiteName) } + + // Pre-upgrade state: apiKey lives in UserDefaults. + defaults.set("sk-legacy-plaintext", forKey: "config.apiKey") + + // First init after upgrade: triggers the one-shot migration. + let config = ProviderConfig(defaults: defaults) + + XCTAssertEqual(config.apiKey, "sk-legacy-plaintext", + "Migrated key must surface through ProviderConfig") + XCTAssertEqual(Keychain.apiKey(), "sk-legacy-plaintext", + "Legacy value must land in Keychain after migration") + XCTAssertNil(defaults.string(forKey: "config.apiKey"), + "Legacy UserDefaults entry must be cleared after migration") + + // Second init (no legacy value left) reads from Keychain only. + let config2 = ProviderConfig(defaults: defaults) + XCTAssertEqual(config2.apiKey, "sk-legacy-plaintext") + } + + /// If both the Keychain and the legacy UserDefaults slot have a value + /// — possible on a downgrade or a torn update — Keychain wins. We + /// don't delete the UserDefaults value, but the running app reads from + /// the Keychain only. + func testKeychainWinsOverLegacyWhenBothPresent() { + let suiteName = "group.com.osgkeyboard.shared.tests.migration2.\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suiteName)! + defaults.removePersistentDomain(forName: suiteName) + defer { defaults.removePersistentDomain(forName: suiteName) } + + // Set both. + try? Keychain.setAPIKey("sk-new") + defaults.set("sk-old", forKey: "config.apiKey") + + let config = ProviderConfig(defaults: defaults) + XCTAssertEqual(config.apiKey, "sk-new", "Keychain value must take precedence") + // Migration only fires when Keychain was nil — we did NOT delete + // the legacy entry here. That's fine because Keychain is the + // source of truth from now on; the legacy value is dead weight + // but not incorrect. + } +} diff --git a/OSGKeyboardTests/LLMClientTests.swift b/OSGKeyboardTests/LLMClientTests.swift index 32b1f83..497f2ba 100644 --- a/OSGKeyboardTests/LLMClientTests.swift +++ b/OSGKeyboardTests/LLMClientTests.swift @@ -9,6 +9,24 @@ import XCTest final class LLMClientTests: XCTestCase { + override func setUpWithError() throws { + // The Keychain is process-global in the simulator (one simulator, + // one keychain DB), so an API key written by a previous test would + // leak into the next one unless we wipe it here. We intentionally + // swallow errors — `errSecItemNotFound` is fine. + try? Keychain.deleteAPIKey() + StubURLProtocolStorage.config = nil + StubURLProtocolStorage.delaySeconds = 0 + StubURLProtocolStorage.lastRequest = nil + } + + override func tearDownWithError() throws { + try? Keychain.deleteAPIKey() + StubURLProtocolStorage.config = nil + StubURLProtocolStorage.delaySeconds = 0 + StubURLProtocolStorage.lastRequest = nil + } + // MARK: - ProviderConfig persistence func testProviderConfigPersistsAcrossInstances() { diff --git a/project.yml b/project.yml index ea921d0..fd82d66 100644 --- a/project.yml +++ b/project.yml @@ -42,6 +42,13 @@ targets: com.apple.security.application-groups: - group.com.osgkeyboard.shared com.apple.security.device.audio-input: true + # Shared Keychain access group: the host app writes the LLM API + # key here in Settings; the keyboard extension reads it before + # each request. The first entry below becomes each process's + # default access group, so the Keychain helper does not need to + # specify kSecAttrAccessGroup explicitly. + com.apple.security.keychain-access-groups: + - com.osgkeyboard.shared resources: - path: OSGKeyboard/Assets.xcassets info: @@ -91,6 +98,11 @@ targets: properties: com.apple.security.application-groups: - group.com.osgkeyboard.shared + # Shared with the host app so the keyboard extension can read the + # user's LLM API key. See OSGKeyboard/OSGKeyboard.entitlements + # for the full rationale. + com.apple.security.keychain-access-groups: + - com.osgkeyboard.shared info: path: OSGKeyboardExt/Info.plist properties: