Skip to content

Commit 42bc5ea

Browse files
committed
AI API keys: move to OS secret store
- Per-provider API keys now persisted via `crate::secrets::store()` (macOS Keychain, Linux Secret Service, or encrypted-file fallback) instead of plaintext in `settings.json`. New `ai/api_keys.rs` module + four Tauri commands (`save_ai_api_key`, `get_ai_api_key`, `delete_ai_api_key`, `has_ai_api_key`). - `ai.cloudProviderConfigs` keeps only `model` and `baseUrl`; `apiKey` field is gone. Old MCP redaction of the JSON blob removed (no longer carries secrets). - Inline error block + persistent toast on save/read failure with platform-aware guidance (macOS Keychain Access vs. Linux keyring). Translator lives in `ai-secret-error.ts` with unit tests. - One-time migration on startup moves any pre-launch `apiKey` strings from `settings.json` into the secret store, then strips the field. TODO to delete the migration after 2026-09-01. - INFO-level logs on key save/delete (no value, just provider + length) so a quiet keychain bug shows up in postmortems. - `pnpm dev` smoke verified: key round-trips through UI -> secret store -> backend config; migration ran end-to-end; bogus test keys cleaned up.
1 parent d7a85a3 commit 42bc5ea

18 files changed

Lines changed: 774 additions & 89 deletions

apps/desktop/src-tauri/src/ai/CLAUDE.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Three provider modes:
1212
| File | Purpose |
1313
|---|---|
1414
| `mod.rs` | Types (`AiStatus`, `AiState`, `DownloadProgress`, `ModelInfo`), model registry (`AVAILABLE_MODELS`, `DEFAULT_MODEL_ID`), `is_local_ai_supported()` gate |
15+
| `api_keys.rs` | Per-provider cloud API key storage. Delegates to `crate::secrets::store()` (macOS Keychain, Linux Secret Service, or encrypted-file fallback). One entry per provider under key `ai.apiKey.<providerId>`. Exposes `save_ai_api_key` / `get_ai_api_key` / `delete_ai_api_key` / `has_ai_api_key` Tauri commands. Keys are NOT stored in `settings.json`. |
1516
| `manager.rs` | Central coordinator. Global `Mutex<Option<ManagerState>>` singleton. Most Tauri commands live here. Stores provider + cloud-AI config (`cloud_api_key`/`cloud_base_url`/`cloud_model`). Exposes `resolve_backend() -> BackendResolution` so callers don't reinvent provider routing. Also owns the `STREAM_CANCEL_TOKENS` registry (`register_stream`/`unregister_stream`/`cancel_stream`) for in-flight `stream_folder_suggestions` cancellation. |
1617
| `download.rs` | HTTP streaming download with Range-based resume. Emits `ai-download-progress` events (200ms throttle). Cooperative cancellation via function parameter (`Fn() -> bool`). |
1718
| `extract.rs` | Copies bundled `llama-server` binary + dylibs from `resources/ai/` to the AI data dir. Sets Unix permissions, handles symlinks. |
@@ -27,6 +28,7 @@ Three provider modes:
2728
### Tauri commands
2829

2930
Core: `get_ai_status`, `get_ai_model_info`, `get_ai_runtime_status`, `configure_ai`, `start_ai_server`, `stop_ai_server`, `check_ai_connection`, `start_ai_download`, `cancel_ai_download`, `get_folder_suggestions`, `stream_folder_suggestions`, `cancel_folder_suggestions`. Note: `get_system_memory_info` moved to top-level `system_memory.rs`.
31+
API keys: `save_ai_api_key`, `get_ai_api_key`, `delete_ai_api_key`, `has_ai_api_key` (in `api_keys.rs`).
3032
Legacy (still wired, used by toast): `uninstall_ai`, `dismiss_ai_offer`, `opt_out_ai`, `opt_in_ai`, `is_ai_opted_out`.
3133

3234
## Startup flow
@@ -37,9 +39,10 @@ Tauri setup()
3739
3840
Frontend loads
3941
-> initializeSettings() <- loads settings from tauri-plugin-store
42+
-> getAiApiKey(providerId) <- fetches API key from OS secret store
4043
-> configureAi({ <- pushes AI config to backend
4144
provider, contextSize,
42-
openaiApiKey, openaiBaseUrl, openaiModel
45+
cloudApiKey, cloudBaseUrl, cloudModel
4346
})
4447
-> backend: if provider === 'local' && model installed && local AI supported
4548
-> spawn_and_track_server() (sync, inside lock — PID tracked immediately)
@@ -78,7 +81,7 @@ The frontend (`AiSection.svelte`) tracks `installStep` state and displays "Step
7881
- Download guard: `download_in_progress` flag prevents concurrent downloads.
7982
- Server logs written to `llama-server.log` in the AI dir for debugging.
8083
- `opted_out` field in `AiState` is legacy. `ai.provider` in frontend settings store is the source of truth.
81-
- OpenAI config (api_key, base_url, model) stored in `ManagerState` so suggestions.rs can read without settings files.
84+
- OpenAI config (api_key, base_url, model) stored in `ManagerState` so suggestions.rs can read without settings files. The api_key originates from the OS secret store (`api_keys.rs`), pushed in via `configure_ai`.
8285
- `configure_ai` is idempotent -- frontend calls it on startup and whenever any AI setting changes.
8386
- `ModelInfo` includes `kv_bytes_per_token` and `base_overhead_bytes` for frontend memory estimation.
8487

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
//! Cloud AI provider API key storage.
2+
//!
3+
//! Delegates to `crate::secrets::store()` for platform-agnostic secret storage so each provider's
4+
//! key sits in the OS-native secret backend (macOS Keychain, Linux Secret Service, etc.) instead
5+
//! of `settings.json`. One entry per provider keyed as `ai.apiKey.<providerId>`.
6+
7+
use crate::secrets::SecretStoreError;
8+
use log::{debug, info};
9+
use serde::{Deserialize, Serialize};
10+
11+
/// Builds the secret-store key for a given provider id.
12+
fn store_key(provider_id: &str) -> String {
13+
format!("ai.apiKey.{provider_id}")
14+
}
15+
16+
/// Error types surfaced over IPC for AI API key operations.
17+
#[derive(Debug, Clone, Serialize, Deserialize, specta::Type)]
18+
#[serde(rename_all = "snake_case", tag = "type", content = "message")]
19+
pub enum AiApiKeyError {
20+
NotFound(String),
21+
AccessDenied(String),
22+
Other(String),
23+
}
24+
25+
impl std::fmt::Display for AiApiKeyError {
26+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
27+
match self {
28+
Self::NotFound(msg) => write!(f, "AI API key not found: {msg}"),
29+
Self::AccessDenied(msg) => write!(f, "AI API key access denied: {msg}"),
30+
Self::Other(msg) => write!(f, "AI API key error: {msg}"),
31+
}
32+
}
33+
}
34+
35+
impl std::error::Error for AiApiKeyError {}
36+
37+
impl From<SecretStoreError> for AiApiKeyError {
38+
fn from(e: SecretStoreError) -> Self {
39+
match e {
40+
SecretStoreError::NotFound(msg) => Self::NotFound(msg),
41+
SecretStoreError::AccessDenied(msg) => Self::AccessDenied(msg),
42+
SecretStoreError::Other(msg) => Self::Other(msg),
43+
}
44+
}
45+
}
46+
47+
/// Saves the API key for a provider. Overwrites any existing entry. Logs at INFO without ever
48+
/// touching the key value — the *change event* is the actionable signal for postmortem debugging
49+
/// (when did the key get set? did the save reach the keychain?), the key itself is not.
50+
pub fn save(provider_id: &str, api_key: &str) -> Result<(), AiApiKeyError> {
51+
let key = store_key(provider_id);
52+
let key_len = api_key.len();
53+
crate::secrets::store().set(&key, api_key.as_bytes())?;
54+
info!("AI API key saved for provider {provider_id} ({key_len} bytes)");
55+
Ok(())
56+
}
57+
58+
/// Returns the stored API key for a provider, or an error if none is stored.
59+
pub fn get(provider_id: &str) -> Result<String, AiApiKeyError> {
60+
let key = store_key(provider_id);
61+
let data = crate::secrets::store().get(&key)?;
62+
String::from_utf8(data).map_err(|e| AiApiKeyError::Other(format!("Stored key is not valid UTF-8: {e}")))
63+
}
64+
65+
/// Deletes the API key for a provider. Returns `Ok(())` even if no entry existed (idempotent).
66+
pub fn delete(provider_id: &str) -> Result<(), AiApiKeyError> {
67+
let key = store_key(provider_id);
68+
match crate::secrets::store().delete(&key) {
69+
Ok(()) => {
70+
info!("AI API key deleted for provider {provider_id}");
71+
Ok(())
72+
}
73+
Err(SecretStoreError::NotFound(_)) => {
74+
debug!("AI API key delete for {provider_id} was a no-op (none stored)");
75+
Ok(())
76+
}
77+
Err(e) => Err(e.into()),
78+
}
79+
}
80+
81+
/// Returns true if an API key is stored for the provider.
82+
pub fn has(provider_id: &str) -> bool {
83+
get(provider_id).is_ok()
84+
}
85+
86+
// --- Tauri commands ---
87+
88+
#[tauri::command]
89+
#[specta::specta]
90+
pub fn save_ai_api_key(provider_id: String, api_key: String) -> Result<(), AiApiKeyError> {
91+
save(&provider_id, &api_key)
92+
}
93+
94+
/// Returns the stored API key for the provider, or an empty string if none is stored.
95+
/// Returning empty (rather than an error) on missing keys keeps the call sites simple — they all
96+
/// pass the value through to `configure_ai`, which already treats empty-string as "not configured."
97+
#[tauri::command]
98+
#[specta::specta]
99+
pub fn get_ai_api_key(provider_id: String) -> Result<String, AiApiKeyError> {
100+
match get(&provider_id) {
101+
Ok(key) => Ok(key),
102+
Err(AiApiKeyError::NotFound(_)) => Ok(String::new()),
103+
Err(e) => Err(e),
104+
}
105+
}
106+
107+
#[tauri::command]
108+
#[specta::specta]
109+
pub fn delete_ai_api_key(provider_id: String) -> Result<(), AiApiKeyError> {
110+
delete(&provider_id)
111+
}
112+
113+
#[tauri::command]
114+
#[specta::specta]
115+
pub fn has_ai_api_key(provider_id: String) -> bool {
116+
has(&provider_id)
117+
}
118+
119+
#[cfg(test)]
120+
mod tests {
121+
use super::*;
122+
use std::sync::atomic::{AtomicU64, Ordering};
123+
124+
/// Per-test isolation: each test runs in its own data dir so the PlainFileStore's JSON file
125+
/// doesn't race across nextest's per-test processes (which would share the prod app-support
126+
/// dir otherwise — secrets `save` succeeds but the subsequent `get` sees another test's write).
127+
///
128+
/// Must be called BEFORE the first secret store access in the test — the secret store backend
129+
/// is a `LazyLock` and reads these env vars exactly once.
130+
///
131+
/// SAFETY: `std::env::set_var` is racy across threads, but each nextest test runs in its own
132+
/// process, so the only env-var writes happen here on the test's main thread before any code
133+
/// that reads them.
134+
fn isolate_secrets() {
135+
static COUNTER: AtomicU64 = AtomicU64::new(0);
136+
let id = COUNTER.fetch_add(1, Ordering::Relaxed);
137+
let dir = std::env::temp_dir().join(format!("cmdr-api-keys-test-{}-{}", std::process::id(), id));
138+
std::fs::create_dir_all(&dir).expect("create test data dir");
139+
unsafe {
140+
std::env::set_var("CMDR_DATA_DIR", &dir);
141+
std::env::set_var("CMDR_SECRET_STORE", "file");
142+
}
143+
}
144+
145+
#[test]
146+
fn save_and_get_roundtrip() {
147+
isolate_secrets();
148+
save("openai", "sk-test-abc123").unwrap();
149+
assert_eq!(get("openai").unwrap(), "sk-test-abc123");
150+
}
151+
152+
#[test]
153+
fn get_missing_returns_not_found() {
154+
isolate_secrets();
155+
match get("openai") {
156+
Err(AiApiKeyError::NotFound(_)) => {}
157+
other => panic!("expected NotFound, got {other:?}"),
158+
}
159+
}
160+
161+
#[test]
162+
fn has_reflects_save_and_delete() {
163+
isolate_secrets();
164+
assert!(!has("openai"));
165+
save("openai", "sk-test").unwrap();
166+
assert!(has("openai"));
167+
delete("openai").unwrap();
168+
assert!(!has("openai"));
169+
}
170+
171+
#[test]
172+
fn delete_missing_is_idempotent() {
173+
isolate_secrets();
174+
delete("openai").unwrap();
175+
delete("openai").unwrap();
176+
}
177+
178+
#[test]
179+
fn save_overwrites_existing() {
180+
isolate_secrets();
181+
save("openai", "sk-first").unwrap();
182+
save("openai", "sk-second").unwrap();
183+
assert_eq!(get("openai").unwrap(), "sk-second");
184+
}
185+
}

apps/desktop/src-tauri/src/ai/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//! 3. Add a new entry to `AVAILABLE_MODELS`
1212
//! 4. Update `DEFAULT_MODEL_ID` if the new model should be the default
1313
14+
pub mod api_keys;
1415
pub mod client;
1516
#[cfg(test)]
1617
mod client_integration_test;

apps/desktop/src-tauri/src/ipc.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,10 @@ fn collect_cross_platform_types(types: &mut Types) -> Vec<Function> {
185185
crate::ai::manager::opt_out_ai,
186186
crate::ai::manager::opt_in_ai,
187187
crate::ai::manager::is_ai_opted_out,
188+
crate::ai::api_keys::save_ai_api_key,
189+
crate::ai::api_keys::get_ai_api_key,
190+
crate::ai::api_keys::delete_ai_api_key,
191+
crate::ai::api_keys::has_ai_api_key,
188192
crate::ai::suggestions::get_folder_suggestions,
189193
// set_mcp_enabled, set_mcp_port are generic (<R: Runtime>) — excluded from specta
190194
crate::commands::mcp::get_mcp_running,
@@ -884,6 +888,10 @@ pub fn builder() -> Builder<tauri::Wry> {
884888
crate::ai::manager::opt_out_ai,
885889
crate::ai::manager::opt_in_ai,
886890
crate::ai::manager::is_ai_opted_out,
891+
crate::ai::api_keys::save_ai_api_key,
892+
crate::ai::api_keys::get_ai_api_key,
893+
crate::ai::api_keys::delete_ai_api_key,
894+
crate::ai::api_keys::has_ai_api_key,
887895
crate::ai::suggestions::get_folder_suggestions,
888896
// stream_folder_suggestions / cancel_folder_suggestions: streaming via tauri Channel<T>;
889897
// not specta-friendly yet, kept on raw invoke (eslint opt-out at FE call sites).

apps/desktop/src-tauri/src/secrets/CLAUDE.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,12 @@ Generic key-value secret storage with pluggable backends.
2424

2525
### Generic byte storage, not typed credentials
2626

27-
The trait stores opaque `&[u8]` / `Vec<u8>`. Callers (like `network/keychain.rs` for SMB) handle their own
28-
serialization format. This keeps the store reusable for any secret type.
27+
The trait stores opaque `&[u8]` / `Vec<u8>`. Callers handle their own serialization format. Current consumers:
28+
29+
- `network/keychain.rs` — SMB credentials, stored as `username\0password` under keys like `smb://server/share`.
30+
- `ai/api_keys.rs` — cloud AI provider API keys, stored as raw UTF-8 under keys like `ai.apiKey.openai`.
31+
32+
This keeps the store reusable for any future secret type.
2933

3034
### File-based stores use `CMDR_DATA_DIR`
3135

apps/desktop/src/lib/ipc/bindings.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,18 @@ export const commands = {
874874
optInAi: () => __TAURI_INVOKE<void>('opt_in_ai'),
875875
// Returns whether the user has opted out of AI features.
876876
isAiOptedOut: () => __TAURI_INVOKE<boolean>('is_ai_opted_out'),
877+
saveAiApiKey: (providerId: string, apiKey: string) =>
878+
typedError<null, AiApiKeyError>(__TAURI_INVOKE('save_ai_api_key', { providerId, apiKey })),
879+
/**
880+
* Returns the stored API key for the provider, or an empty string if none is stored.
881+
* Returning empty (rather than an error) on missing keys keeps the call sites simple — they all
882+
* pass the value through to `configure_ai`, which already treats empty-string as "not configured."
883+
*/
884+
getAiApiKey: (providerId: string) =>
885+
typedError<string, AiApiKeyError>(__TAURI_INVOKE('get_ai_api_key', { providerId })),
886+
deleteAiApiKey: (providerId: string) =>
887+
typedError<null, AiApiKeyError>(__TAURI_INVOKE('delete_ai_api_key', { providerId })),
888+
hasAiApiKey: (providerId: string) => __TAURI_INVOKE<boolean>('has_ai_api_key', { providerId }),
877889
/**
878890
* Generates folder name suggestions for the given directory.
879891
*
@@ -1748,6 +1760,12 @@ export type ActivityPhase =
17481760
// Idle — indexing initialized but no active work.
17491761
| 'idle'
17501762

1763+
// Error types surfaced over IPC for AI API key operations.
1764+
export type AiApiKeyError =
1765+
| { type: 'not_found'; message: string }
1766+
| { type: 'access_denied'; message: string }
1767+
| { type: 'other'; message: string }
1768+
17511769
// Result of checking connectivity to an AI API endpoint.
17521770
export type AiConnectionCheckResult = {
17531771
connected: boolean

apps/desktop/src/lib/settings/CLAUDE.md

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,11 @@ backend (via `getAiRuntimeStatus()` and Tauri events) with registry settings (`a
131131
handles provider switching (auto-stops local server when switching away), and conditionally renders one of the two
132132
sub-sections.
133133
- **`AiCloudSection.svelte`** — Cloud/API provider config. Provider preset dropdown (`cloud-providers.ts`), per-provider
134-
API key storage in a JSON blob (`ai.cloudProviderConfigs`), endpoint URL, model combobox. Old flat settings
135-
(`ai.openaiApiKey`, `ai.openaiBaseUrl`, `ai.openaiModel`) are migrated on first load. Includes a two-step connection
136-
check (`check_ai_connection` Tauri command) that auto-triggers on API key or base URL changes (1s debounce), fetches
137-
available models from the `/models` endpoint, and shows connection status (connected, auth error, unreachable). When
138-
models are available, the Model field becomes a combobox with filtered dropdown; otherwise it's a plain text input.
134+
endpoint URL and model stored in `ai.cloudProviderConfigs`, per-provider API key stored in the OS secret store via
135+
`saveAiApiKey` / `getAiApiKey`. Includes a two-step connection check (`check_ai_connection` Tauri command) that
136+
auto-triggers on API key or base URL changes (1s debounce), fetches available models from the `/models` endpoint, and
137+
shows connection status (connected, auth error, unreachable). When models are available, the Model field becomes a
138+
combobox with filtered dropdown; otherwise it's a plain text input.
139139
- **`AiLocalSection.svelte`** — Local LLM management. Server lifecycle (start/stop), model download with multi-step
140140
install tracking, context window setting with explicit "Apply" button (triggers server restart), RAM gauge (stacked
141141
bar) showing memory usage relative to system total with warning icons at >70% and >90% projected usage, system memory
@@ -176,7 +176,8 @@ row intentionally spans the full width.
176176

177177
- **cloud-providers.ts** — Cloud provider preset definitions (OpenAI, Anthropic, Groq, etc.) and per-provider config
178178
helpers (`getProviderConfigs`, `setProviderConfig`, `resolveCloudConfig`). Used by `AiSection` and the startup flow in
179-
`+layout.svelte` to resolve the effective API key, base URL, and model before calling `configureAi`.
179+
`+layout.svelte` to resolve the effective base URL and model. The API key is fetched separately from the OS secret
180+
store via `getAiApiKey(providerId)` before calling `configureAi`.
180181
- **settings-search.ts** — Fuzzy search over setting definitions; returns ranked matches with highlight ranges
181182
- **settings-applier.ts** — Listens for setting changes and applies side effects (CSS vars, backend config sync)
182183
- **network-settings.ts** — Network-specific setting helpers (proxy config, SMB auth defaults)
@@ -226,10 +227,13 @@ Defining all settings in a registry enables:
226227
3. UI generation for Advanced section (technical settings don't need custom UI)
227228
4. Schema migration (registry knows what's valid, can transform old data)
228229

229-
### Why store OpenAI API key in `settings.json`, not keychain?
230+
### Why store cloud AI API keys in the OS secret store, not `settings.json`?
230231

231-
Simpler first version. The file is already in the user's private app support directory. Keychain integration can come
232-
later if needed. The key is never sent anywhere except the user's configured endpoint.
232+
API keys live in the OS-native secret store (macOS Keychain, Linux Secret Service, or an encrypted file fallback on
233+
Linux without Secret Service) via `crate::secrets`. Access goes through the `saveAiApiKey` / `getAiApiKey` /
234+
`deleteAiApiKey` / `hasAiApiKey` Tauri commands. `ai.cloudProviderConfigs` in `settings.json` only holds non-secret
235+
fields (`model`, `baseUrl`). This keeps API keys out of Time Machine, cloud-sync backups, and any tool that mirrors
236+
`~/Library/Application Support`. Same secret store backs SMB credentials — see `src-tauri/src/secrets/CLAUDE.md`.
233237

234238
### Why debounced saves?
235239

0 commit comments

Comments
 (0)