From ad0081ef6431e606709cf89779aa4eee40156d9f Mon Sep 17 00:00:00 2001 From: Gerard Slee Date: Sat, 25 Jul 2026 11:03:46 -0400 Subject: [PATCH 1/5] Store LLM provider API keys in the OS keychain Keys for the cloud/local AI providers (chat + coding agent, one per saved provider) now live in the OS keychain (Electron safeStorage) via the existing secure-store, instead of plaintext in the Zustand store, localStorage, and ai-settings-cache.json. - secure-store: add "llm" ToolKind + setLlmApiKey/deleteLlmApiKey/ resolveLlmApiKey/llmSecretRef helpers (+ unit tests) - secrets IPC/preload accept the "llm" toolType (identity guard updated) - renderer stores the raw key via secrets.set and keeps only a secret://llm:/apiKey reference; the API-key field never shows the key - main-process IPC resolvers (chat, ai-handlers, flow, pi-agent) resolve the ref to the real key at request time only; downstream header sites unchanged - config-cache scrubs any non-ref apiKey so no plaintext key is written to disk - new ai:fetchModels main IPC so model listing no longer fetches with the key in the renderer (also keeps it off the CSP path) - providers-only connection UI: drop the standalone Base URL/API Key/Model rows from AI + Agent settings (the provider form owns them) - one-time hydrate migration relocates existing plaintext keys into the keychain and scrubs the old locations --- changelogs/v2.5.9.md | 6 +- electron/ipc/ai-handlers.ts | 39 +++++++-- electron/ipc/chat.ts | 18 +++- electron/ipc/flow-handlers.ts | 13 ++- electron/ipc/pi-agent.ts | 58 +++++++------ electron/ipc/tools.ts | 2 +- electron/lib/config-cache.ts | 20 +++-- electron/lib/secure-store.test.ts | 50 +++++++++++ electron/lib/secure-store.ts | 58 ++++++++++++- electron/preload.ts | 8 +- src/components/settings/AISettings.tsx | 53 ++---------- src/components/settings/AgentSettings.tsx | 59 +------------ src/components/settings/ProviderManager.tsx | 62 ++++++++++---- .../settings/endpoint-components.tsx | 46 +++++----- src/store/index.ts | 23 ++++- src/store/slices/ui.ts | 83 +++++++++++++++++++ 16 files changed, 398 insertions(+), 200 deletions(-) diff --git a/changelogs/v2.5.9.md b/changelogs/v2.5.9.md index 4153ee38a..20fafd8ad 100644 --- a/changelogs/v2.5.9.md +++ b/changelogs/v2.5.9.md @@ -7,10 +7,14 @@ ## Changed -- **Cleaner endpoint settings.** Now that saved providers cover switching endpoints, the old OpenAI / Ollama / LM Studio quick-preset pills under the Base URL field have been removed from both the AI settings and the Coding Agent settings — pick or add a saved provider instead. The provider picker now uses Cairn's standard dropdown menu (matching the model picker and other settings) instead of a plain OS select box, and shows each provider's model beneath its name. +- **Cleaner endpoint settings.** Now that saved providers cover switching endpoints, the old OpenAI / Ollama / LM Studio quick-preset pills under the Base URL field have been removed from both the AI settings and the Coding Agent settings — pick or add a saved provider instead. The provider picker now uses Cairn's standard dropdown menu (matching the model picker and other settings) instead of a plain OS select box, and shows each provider's model beneath its name. The standalone Base URL / API Key / Model rows are gone too — a provider's connection is edited entirely inside its Add/Edit form. - **Consistent model picker everywhere.** The model selector from the in-chat quick settings — a searchable dropdown of the endpoint's models with a Refresh action and a "Custom model…" option for anything the endpoint doesn't list — is now used across the app: in Settings → AI, in the Coding Agent settings, and in the saved-provider Add/Edit form. This replaces the older text box + Fetch button + wall of model pills, so choosing a model looks and behaves the same in every place. - **The model dropdown only shows real models.** It no longer pre-fills with a hardcoded list of guessed model names. Instead each endpoint's model list is fetched and cached per endpoint (base URL + key), so it appears instantly the next time you open the picker; if nothing is cached yet, it fetches once automatically. You can still Refresh to re-query or enter a custom model id. Cached lists refresh after a week. +## Security + +- **AI provider API keys now live in the OS keychain.** Keys for the cloud/local AI providers (chat + coding agent, one per saved provider) are stored encrypted in your operating system's keychain (macOS Keychain, Windows DPAPI, Linux libsecret) — the same secure store already used for MCP/service credentials — instead of in plain text in the app's settings files or local storage. The app only ever holds an opaque reference token; the real key is decrypted in the background process at request time, and never written to disk unencrypted, kept in the window, or exposed to web content. Existing keys are relocated into the keychain automatically on first launch and scrubbed from the old plaintext locations. Fetching an endpoint's model list now also happens in the background process, so your key isn't handled by the in-app browser layer. The API-key field shows only whether a key is stored ("•••• stored — type to replace"), never the saved key. + ## Fixed - **AI settings no longer crash with an infinite render loop.** Opening Settings → AI could throw "The result of getSnapshot should be cached to avoid an infinite loop" because the saved-providers list was rebuilt on every render. The provider switcher now reads a stable list reference, so the settings panel renders cleanly. diff --git a/electron/ipc/ai-handlers.ts b/electron/ipc/ai-handlers.ts index 4f0124f1c..96af5e06d 100644 --- a/electron/ipc/ai-handlers.ts +++ b/electron/ipc/ai-handlers.ts @@ -12,6 +12,7 @@ import { err, handle, type DbContext } from "./result-helpers"; import { generatePrd } from "../lib/prd"; import { callLLM, isLocalEndpoint, normaliseBaseUrl, type LLMConfig } from "../lib/llm"; import { saveCachedConfig, getCachedConfig } from "../lib/config-cache"; +import { resolveLlmApiKey, isSecretRef } from "../lib/secure-store"; function resolveConfig( config: { baseUrl?: string; model?: string; apiKey?: string } | undefined, @@ -19,7 +20,8 @@ function resolveConfig( ): { error: string } | LLMConfig { const cached = cacheKey === "ai" ? getCachedConfig().aiConfig : getCachedConfig().agentConfig; - // Merge request config with cached config, always merging cached values if not provided. + // Merge request config with cached config, always merging cached values if not + // provided. `apiKey` here is a keychain reference token (or empty), not a raw key. const mergedConfig = { baseUrl: config?.baseUrl || cached?.baseUrl || "", model: config?.model || cached?.model || "", @@ -28,13 +30,14 @@ function resolveConfig( const baseUrl = normaliseBaseUrl(mergedConfig.baseUrl || "https://api.openai.com"); const model = mergedConfig.model || "gpt-4o-mini"; - const apiKey = mergedConfig.apiKey || ""; + const keyRef = mergedConfig.apiKey || ""; const isLocal = isLocalEndpoint(baseUrl); - if (!apiKey && !isLocal) { + if (!keyRef && !isLocal) { const sectionName = cacheKey === "ai" ? "Settings → AI & Chat" : "Settings"; return { error: `AI is not configured. Add an API key in ${sectionName}, or use a local endpoint.` }; } - return { baseUrl, model, apiKey }; + // Resolve the ref to the real key only now, for this request. + return { baseUrl, model, apiKey: resolveLlmApiKey(keyRef) }; } function cleanOutput(text: string): string { @@ -98,6 +101,27 @@ export function registerAiHandlers(ctx: DbContext): void { }); }); + // ── Model discovery (GET {baseUrl}/v1/models) ───── + // Runs in the main process so the API key (a keychain ref) is resolved here + // and the raw key never lives in the renderer or crosses the CSP boundary. + registerIpcHandle( + "ai:fetchModels", + async (_e, args: { baseUrl?: string; apiKey?: string }) => + handle(async () => { + const url = normaliseBaseUrl(args.baseUrl || "https://api.openai.com"); + const realKey = resolveLlmApiKey(args.apiKey); + const headers: Record = {}; + if (realKey) headers["Authorization"] = `Bearer ${realKey}`; + const res = await fetch(`${url}/v1/models`, { headers }); + if (!res.ok) throw new Error(`${res.status}`); + const data = (await res.json()) as { data?: Array<{ id: string }> }; + return (data?.data ?? []) + .map((m) => m.id) + .filter((id) => !id.includes("embed") && !id.includes("whisper") && !id.includes("tts") && !id.includes("dall-e")) + .sort(); + }) + ); + // ── AI PRD generation (direct, no chat loop) ────── registerIpcHandle("ai:generatePrd", async (_e, args: { projectId: string; @@ -105,13 +129,16 @@ export function registerAiHandlers(ctx: DbContext): void { requirements: string; config: { baseUrl: string; model: string; apiKey: string }; }) => { - // PRD returns its own { error } shape for user-facing validation errors - if (args.config?.apiKey) { + // PRD returns its own { error } shape for user-facing validation errors. + // Only persist a keychain ref to the cache — never a raw key. + if (args.config?.apiKey && isSecretRef(args.config.apiKey)) { saveCachedConfig("ai", { baseUrl: args.config.baseUrl, model: args.config.model, apiKey: args.config.apiKey, }); + } else if (args.config?.baseUrl || args.config?.model) { + saveCachedConfig("ai", { baseUrl: args.config.baseUrl, model: args.config.model }); } const resolved = resolveConfig(args.config, "ai"); diff --git a/electron/ipc/chat.ts b/electron/ipc/chat.ts index 2ac69fb91..ee135821e 100644 --- a/electron/ipc/chat.ts +++ b/electron/ipc/chat.ts @@ -16,6 +16,7 @@ import { TOOLS, buildSystemPrompt, type ChatRequest } from "../lib/tools"; import { getExternalToolDefs } from "../lib/external-tools"; import { saveCachedConfig, getCachedConfig } from "../lib/config-cache"; import { runToolLoop } from "../lib/chat-loop"; +import { resolveLlmApiKey, isSecretRef } from "../lib/secure-store"; // Track one AbortController per renderer webContents ID const abortControllers = new Map(); @@ -31,13 +32,24 @@ function resolveAIConfig(config?: { model: string; apiKey: string; } { - if (config?.apiKey) { + // `config.apiKey` from the renderer is a keychain reference token + // (`secret://llm:…/apiKey`), not a raw key. Persist only refs to the on-disk + // cache — never a plaintext key. (A literal key can still arrive from older + // callers; we tolerate it for the request but keep it out of the cache.) + if (config?.apiKey && isSecretRef(config.apiKey)) { saveCachedConfig("ai", { provider: config.provider, baseUrl: config.baseUrl, model: config.model, apiKey: config.apiKey, }); + } else if (config?.provider || config?.baseUrl || config?.model) { + // Persist connection fields without touching the cached (ref) key. + saveCachedConfig("ai", { + provider: config.provider, + baseUrl: config.baseUrl, + model: config.model, + }); } let reqConfig = config; @@ -59,7 +71,9 @@ function resolveAIConfig(config?: { provider: reqConfig?.provider ?? "openai", baseUrl: normaliseBaseUrl(reqConfig?.baseUrl ?? "https://api.openai.com"), model: reqConfig?.model ?? "gpt-4o-mini", - apiKey: reqConfig?.apiKey ?? "", + // Resolve the keychain ref (or pass a literal through) to the real key used + // for this request only. Never stored anywhere from here on. + apiKey: resolveLlmApiKey(reqConfig?.apiKey), }; } diff --git a/electron/ipc/flow-handlers.ts b/electron/ipc/flow-handlers.ts index b284ff55e..00bbcd2bb 100644 --- a/electron/ipc/flow-handlers.ts +++ b/electron/ipc/flow-handlers.ts @@ -19,6 +19,7 @@ import { handle, type DbContext } from "./result-helpers"; import * as q from "../db/queries"; import { isLocalEndpoint, callLLM, normaliseBaseUrl } from "../lib/llm"; import { saveCachedConfig, getCachedConfig } from "../lib/config-cache"; +import { resolveLlmApiKey, isSecretRef } from "../lib/secure-store"; /** * Resolve the effective AI config (cache fallback + normalisation). @@ -46,12 +47,13 @@ function resolveAiConfig(input: { baseUrl?: string; model?: string; apiKey?: str const baseUrl = normaliseBaseUrl(reqConfig?.baseUrl || "https://api.openai.com"); const model = reqConfig?.model || "gpt-4o-mini"; - const apiKey = reqConfig?.apiKey || ""; + const keyRef = reqConfig?.apiKey || ""; const isLocal = isLocalEndpoint(baseUrl); - if (!apiKey && !isLocal) { + if (!keyRef && !isLocal) { return { error: "AI is not configured. Add an API key in Settings → AI & Chat, or use a local endpoint." }; } - return { baseUrl, model, apiKey }; + // `keyRef` is a keychain reference token; resolve to the real key for this request only. + return { baseUrl, model, apiKey: resolveLlmApiKey(keyRef) }; } export function registerFlowHandlers(ctx: DbContext): void { @@ -102,12 +104,15 @@ export function registerFlowHandlers(ctx: DbContext): void { "db:flow:node:summarize", (_e, args: { nodeId: string; config: { baseUrl: string; model: string; apiKey: string } }) => handle(async () => { - if (args.config?.apiKey) { + // Persist only a keychain ref to the cache — never a raw key. + if (args.config?.apiKey && isSecretRef(args.config.apiKey)) { saveCachedConfig("ai", { baseUrl: args.config.baseUrl, model: args.config.model, apiKey: args.config.apiKey, }); + } else if (args.config?.baseUrl || args.config?.model) { + saveCachedConfig("ai", { baseUrl: args.config.baseUrl, model: args.config.model }); } const resolved = resolveAiConfig(args.config); diff --git a/electron/ipc/pi-agent.ts b/electron/ipc/pi-agent.ts index ec692c7cc..595b9773f 100644 --- a/electron/ipc/pi-agent.ts +++ b/electron/ipc/pi-agent.ts @@ -28,6 +28,7 @@ import type { DbContext } from "./handlers"; import * as q from "../db/queries"; import { ts } from "../db/utils"; import { saveCachedConfig, getCachedConfig } from "../lib/config-cache"; +import { resolveLlmApiKey, isSecretRef } from "../lib/secure-store"; // ── Session registry ────────────────────────────────────────────────────────── @@ -192,16 +193,15 @@ export function registerPiAgentHandler( return; } - if (req.config?.apiKey) { - saveCachedConfig("agent", { - baseUrl: req.config.baseUrl, - model: req.config.model, - apiKey: req.config.apiKey, - maxSteps: req.config.maxSteps, - temperature: req.config.temperature, - autoApprove: req.config.autoApprove, - }); - } + // Persist connection fields; only cache a keychain ref for the key, never raw. + saveCachedConfig("agent", { + baseUrl: req.config?.baseUrl, + model: req.config?.model, + ...(req.config?.apiKey && isSecretRef(req.config.apiKey) ? { apiKey: req.config.apiKey } : {}), + maxSteps: req.config?.maxSteps, + temperature: req.config?.temperature, + autoApprove: req.config?.autoApprove, + }); let reqConfig = req.config; const cached = getCachedConfig().agentConfig; @@ -225,7 +225,7 @@ export function registerPiAgentHandler( const llmConfig: AgentLLMConfig = { baseUrl: normaliseBaseUrl(reqConfig?.baseUrl || "https://api.openai.com"), model: reqConfig?.model || "gpt-4o", - apiKey: reqConfig?.apiKey || "", + apiKey: resolveLlmApiKey(reqConfig?.apiKey), maxSteps: reqConfig?.maxSteps ?? 20, temperature: reqConfig?.temperature ?? 0.3, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : true, @@ -284,16 +284,15 @@ export function registerPiAgentHandler( return; } - if (req.config?.apiKey) { - saveCachedConfig("agent", { - baseUrl: req.config.baseUrl, - model: req.config.model, - apiKey: req.config.apiKey, - maxSteps: req.config.maxSteps, - temperature: req.config.temperature, - autoApprove: req.config.autoApprove, - }); - } + // Persist connection fields; only cache a keychain ref for the key, never raw. + saveCachedConfig("agent", { + baseUrl: req.config?.baseUrl, + model: req.config?.model, + ...(req.config?.apiKey && isSecretRef(req.config.apiKey) ? { apiKey: req.config.apiKey } : {}), + maxSteps: req.config?.maxSteps, + temperature: req.config?.temperature, + autoApprove: req.config?.autoApprove, + }); let reqConfig = req.config; const cached = getCachedConfig().agentConfig; @@ -317,7 +316,7 @@ export function registerPiAgentHandler( const llmConfig: AgentLLMConfig = { baseUrl: normaliseBaseUrl(reqConfig?.baseUrl || "https://api.openai.com"), model: reqConfig?.model || "gpt-4o", - apiKey: reqConfig?.apiKey || "", + apiKey: resolveLlmApiKey(reqConfig?.apiKey), maxSteps: reqConfig?.maxSteps ?? 20, temperature: reqConfig?.temperature ?? 0.3, autoApprove: reqConfig?.autoApprove !== undefined ? reqConfig.autoApprove : true, @@ -367,13 +366,12 @@ export function registerPiAgentHandler( broadcastEvent(channel, payload); }; - if (req.config?.apiKey) { - saveCachedConfig("agent", { - baseUrl: req.config.baseUrl, - model: req.config.model, - apiKey: req.config.apiKey, - }); - } + // Persist connection fields; only cache a keychain ref for the key, never raw. + saveCachedConfig("agent", { + baseUrl: req.config?.baseUrl, + model: req.config?.model, + ...(req.config?.apiKey && isSecretRef(req.config.apiKey) ? { apiKey: req.config.apiKey } : {}), + }); let reqConfig = req.config; if (!reqConfig?.apiKey) { @@ -391,7 +389,7 @@ export function registerPiAgentHandler( const llmConfig: AgentLLMConfig = { baseUrl: normaliseBaseUrl(reqConfig?.baseUrl || "https://api.openai.com"), model: reqConfig?.model || "gpt-4o", - apiKey: reqConfig?.apiKey || "", + apiKey: resolveLlmApiKey(reqConfig?.apiKey), maxSteps: 20, temperature: 0.1, }; diff --git a/electron/ipc/tools.ts b/electron/ipc/tools.ts index 033e7f590..2fbfaf078 100644 --- a/electron/ipc/tools.ts +++ b/electron/ipc/tools.ts @@ -356,7 +356,7 @@ export function registerToolsHandlers(db: Database): void { * never create a junk ref or address the wrong tool kind's namespace. */ function assertSecretIdentity(toolType: string, toolId: string, key: string): void { - if (toolType !== "mcp" && toolType !== "service") { + if (toolType !== "mcp" && toolType !== "service" && toolType !== "llm") { throw new Error(`Invalid secret toolType: ${String(toolType)}`); } if (typeof toolId !== "string" || !toolId.trim()) { diff --git a/electron/lib/config-cache.ts b/electron/lib/config-cache.ts index f3d1028cf..502d48548 100644 --- a/electron/lib/config-cache.ts +++ b/electron/lib/config-cache.ts @@ -88,14 +88,20 @@ export function saveCachedConfig(type: "ai" | "agent" | "embeddings" | "theme" | // Type-guard/assert type safe access const configRecord = config as Record | null | undefined; - + + // Only keychain reference tokens (`secret://…`) may be cached for an apiKey — + // never a raw key. Anything else is dropped so plaintext keys can't land on + // disk (defence-in-depth; callers already send refs). + const refOnly = (v: unknown): string | undefined => + typeof v === "string" && v.startsWith("secret://") ? v : undefined; + if (type === "ai" && configRecord) { current.aiConfig = { ...current.aiConfig, provider: typeof configRecord.provider === "string" ? configRecord.provider : current.aiConfig?.provider, baseUrl: typeof configRecord.baseUrl === "string" ? configRecord.baseUrl : current.aiConfig?.baseUrl, model: typeof configRecord.model === "string" ? configRecord.model : current.aiConfig?.model, - apiKey: typeof configRecord.apiKey === "string" ? configRecord.apiKey : current.aiConfig?.apiKey, + apiKey: refOnly(configRecord.apiKey) ?? current.aiConfig?.apiKey, // Preserve behavioural fields too — omitting maxSteps here made the chat // tool-call limit reset to the default on the next hydrate. maxSteps: typeof configRecord.maxSteps === "number" ? configRecord.maxSteps : current.aiConfig?.maxSteps, @@ -103,10 +109,12 @@ export function saveCachedConfig(type: "ai" | "agent" | "embeddings" | "theme" | contextLimit: typeof configRecord.contextLimit === "number" ? configRecord.contextLimit : current.aiConfig?.contextLimit, aiEnabled: typeof configRecord.aiEnabled === "boolean" ? configRecord.aiEnabled : current.aiConfig?.aiEnabled, subagentsEnabled: typeof configRecord.subagentsEnabled === "boolean" ? configRecord.subagentsEnabled : current.aiConfig?.subagentsEnabled, - // Saved-provider switcher state (array + active id). `configRecord` is - // typed for scalars, so read the raw config object for these. + // Saved-provider switcher state (array + active id). Each provider's + // apiKey is scrubbed to a ref (or dropped) so no raw key is cached. savedProviders: Array.isArray((config as { savedProviders?: unknown }).savedProviders) - ? (config as { savedProviders: NonNullable["savedProviders"] }).savedProviders + ? (config as { savedProviders: NonNullable["savedProviders"] }).savedProviders!.map( + (p) => ({ ...p, apiKey: refOnly(p.apiKey) ?? "" }), + ) : current.aiConfig?.savedProviders, activeProviderId: typeof configRecord.activeProviderId === "string" ? configRecord.activeProviderId : current.aiConfig?.activeProviderId, }; @@ -115,7 +123,7 @@ export function saveCachedConfig(type: "ai" | "agent" | "embeddings" | "theme" | ...current.agentConfig, baseUrl: typeof configRecord.baseUrl === "string" ? configRecord.baseUrl : current.agentConfig?.baseUrl, model: typeof configRecord.model === "string" ? configRecord.model : current.agentConfig?.model, - apiKey: typeof configRecord.apiKey === "string" ? configRecord.apiKey : current.agentConfig?.apiKey, + apiKey: refOnly(configRecord.apiKey) ?? current.agentConfig?.apiKey, maxSteps: typeof configRecord.maxSteps === "number" ? configRecord.maxSteps : current.agentConfig?.maxSteps, temperature: typeof configRecord.temperature === "number" ? configRecord.temperature : current.agentConfig?.temperature, autoApprove: typeof configRecord.autoApprove === "boolean" ? configRecord.autoApprove : current.agentConfig?.autoApprove, diff --git a/electron/lib/secure-store.test.ts b/electron/lib/secure-store.test.ts index 2f87bf219..df1841ea5 100644 --- a/electron/lib/secure-store.test.ts +++ b/electron/lib/secure-store.test.ts @@ -158,3 +158,53 @@ describe("secure-store persistence round-trip", () => { expect(store.hasSecret("service", "shared", "Authorization")).toBe(true); }); }); + +describe("LLM API keys", () => { + it("stores a provider key and returns its reference token", () => { + const ref = store.setLlmApiKey("prov1", "sk-abc123"); + expect(ref).toBe("secret://llm:prov1/apiKey"); + expect(store.llmSecretRef("prov1")).toBe(ref); + expect(store.hasSecret("llm", "prov1", "apiKey")).toBe(true); + }); + + it("resolves a reference token back to the raw key", () => { + const ref = store.setLlmApiKey("prov1", "sk-abc123"); + expect(store.resolveLlmApiKey(ref)).toBe("sk-abc123"); + }); + + it("passes a literal (non-ref) key through unchanged", () => { + expect(store.resolveLlmApiKey("sk-literal")).toBe("sk-literal"); + }); + + it("returns empty for empty/undefined input", () => { + expect(store.resolveLlmApiKey("")).toBe(""); + expect(store.resolveLlmApiKey(undefined)).toBe(""); + expect(store.resolveLlmApiKey(null)).toBe(""); + }); + + it("returns empty when a ref can't be resolved (never leaks the token)", () => { + expect(store.resolveLlmApiKey("secret://llm:missing/apiKey")).toBe(""); + }); + + it("clears the stored key when set to empty and returns empty ref", () => { + store.setLlmApiKey("prov1", "sk-abc123"); + expect(store.hasSecret("llm", "prov1", "apiKey")).toBe(true); + const ref = store.setLlmApiKey("prov1", ""); + expect(ref).toBe(""); + expect(store.hasSecret("llm", "prov1", "apiKey")).toBe(false); + }); + + it("deleteLlmApiKey removes the stored key", () => { + store.setLlmApiKey("prov1", "sk-abc123"); + store.deleteLlmApiKey("prov1"); + expect(store.hasSecret("llm", "prov1", "apiKey")).toBe(false); + }); + + it("never writes the raw key to disk (only ciphertext)", () => { + store.setLlmApiKey("prov1", "super-secret-llm-key"); + const dump = JSON.stringify(virtualFiles); + expect(dump).not.toContain("super-secret-llm-key"); + // Stored as base64 of the mock cipher ("enc:"), never the plaintext. + expect(dump).toContain(Buffer.from("enc:super-secret-llm-key", "utf-8").toString("base64")); + }); +}); diff --git a/electron/lib/secure-store.ts b/electron/lib/secure-store.ts index 8fec49097..3f5f99088 100644 --- a/electron/lib/secure-store.ts +++ b/electron/lib/secure-store.ts @@ -34,8 +34,11 @@ type SecretStore = Record; // ── Pure helpers (unit-testable, no Electron) ──────────────────────────────── -/** Tool kinds that own secrets. Mirrors ToolType in src/types. */ -export type ToolKind = "mcp" | "service"; +/** + * Tool kinds that own secrets. Mirrors ToolType in src/types, plus "llm" for + * the built-in AI provider API keys (the OpenAI-compatible chat/agent endpoints). + */ +export type ToolKind = "mcp" | "service" | "llm"; /** * Build the canonical reference token for a tool's named secret. The toolType is @@ -280,3 +283,54 @@ export function resolveSecrets(headers: Record): Record/apiKey`); the raw key is resolved +// exclusively in the main process, at the IPC boundary, right before a request. + +/** Canonical secret key under which every LLM provider stores its API key. */ +export const LLM_API_KEY = "apiKey"; + +/** The reference token for a saved provider's API key. */ +export function llmSecretRef(providerId: string): string { + return secretRef("llm", providerId, LLM_API_KEY); +} + +/** + * Store a provider's raw API key in the keychain and return its reference token. + * Empty/placeholder input clears any stored key and returns "" (meaning "no + * key" — e.g. a keyless local server). Main-process only. + */ +export function setLlmApiKey(providerId: string, rawKey: string): string { + const trimmed = (rawKey ?? "").trim(); + if (!trimmed || isPlaceholder(trimmed) || containsPlaceholder(trimmed)) { + deleteSecret("llm", providerId, LLM_API_KEY); + return ""; + } + return setSecret("llm", providerId, LLM_API_KEY, trimmed); +} + +/** Delete a provider's stored API key. No-op if absent. */ +export function deleteLlmApiKey(providerId: string): void { + deleteSecret("llm", providerId, LLM_API_KEY); +} + +/** + * Resolve an incoming `apiKey` config value to the real key for an outbound + * request. Accepts either: + * - a `secret://llm:…/apiKey` reference token → decrypt from the keychain + * - any other non-empty string → treat as a literal key (already raw) + * - "" / undefined → "" (no key; keyless local endpoint) + * Returns "" when a ref can't be resolved, so we never send the token on the wire. + * Main-process only. + */ +export function resolveLlmApiKey(apiKey: string | undefined | null): string { + const v = apiKey ?? ""; + if (!v) return ""; + if (isSecretRef(v)) return getSecretByRef(v) ?? ""; + return v; // literal key +} diff --git a/electron/preload.ts b/electron/preload.ts index f739f3475..61b2bfcb9 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -362,6 +362,8 @@ const api = { explainArchitecture: (args: { summary: string; config: { baseUrl: string; model: string; apiKey: string } }) => invoke<{ overview: string; modules: string }>("ai:explainArchitecture", args), localLLMStatus: () => invoke<{ available: boolean; reason?: string }>("ai:localLLMStatus"), + fetchModels: (args: { baseUrl?: string; apiKey?: string }) => + invoke("ai:fetchModels", args), }, // ── App paths ───────────────────────────────── @@ -668,11 +670,11 @@ const api = { // ── Secrets (OS keychain). No get() by design — renderer only learns set/not-set. secrets: { available: () => invoke("secrets:available"), - set: (toolType: "mcp" | "service", toolId: string, key: string, value: string) => + set: (toolType: "mcp" | "service" | "llm", toolId: string, key: string, value: string) => invoke("secrets:set", { toolType, toolId, key, value }), - has: (toolType: "mcp" | "service", toolId: string, key: string) => + has: (toolType: "mcp" | "service" | "llm", toolId: string, key: string) => invoke("secrets:has", { toolType, toolId, key }), - delete: (toolType: "mcp" | "service", toolId: string, key: string) => + delete: (toolType: "mcp" | "service" | "llm", toolId: string, key: string) => invoke("secrets:delete", { toolType, toolId, key }), }, diff --git a/src/components/settings/AISettings.tsx b/src/components/settings/AISettings.tsx index f6564ebd2..10c19e8a3 100644 --- a/src/components/settings/AISettings.tsx +++ b/src/components/settings/AISettings.tsx @@ -7,10 +7,6 @@ import { Cpu, Globe } from "lucide-react"; import { cn } from "@/lib/utils"; import { contextLimitForModel } from "@/lib/models-dev"; import { SettingsGroup, SettingsRow, Toggle, StepperSettingsRow } from "./shared"; -import { - BaseUrlRow, ApiKeyRow, ModelSelectionRow, CloudConnectionStatus, - useEndpointConfig, isLocalBaseUrl, -} from "./endpoint-components"; import { MCPServerSettings } from "./MCPSettings"; import { LlamaServerConsole } from "./LlamaServerConsole"; import { ProviderManager } from "./ProviderManager"; @@ -21,31 +17,16 @@ export function AISettings() { setAIConfig: s.setAIConfig, }))); - const { - showKey, testState, testError, availableModels, modelsLoading, - setShowKey, fetchModels, ensureModels, resetModels, - } = useEndpointConfig(); - - // General config destructuring - const { provider = "openai", baseUrl, model, apiKey, aiEnabled, activeProviderId } = aiConfig; - const isLocal = isLocalBaseUrl(baseUrl); + // General config destructuring. Connection fields (baseUrl/apiKey/model) are + // now managed entirely by the ProviderManager switcher below; here we only + // need `provider` (which backend), `model` (for the context lookup), and the + // behavioural fields. + const { provider = "openai", model, aiEnabled } = aiConfig; function updateAIConfig(patch: Partial) { setAIConfig(patch); - if (patch.baseUrl !== undefined || patch.apiKey !== undefined) { - resetModels(); - } } - // Populate the model list from cache (or fetch once) when the cloud provider - // is selected or the active saved provider changes. Not on every keystroke in - // the URL/key fields (Refresh in the picker re-queries after edits). No - // hardcoded fallbacks — the picker shows only real models. - useEffect(() => { - if (provider !== "localllm") ensureModels(baseUrl, apiKey); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [provider, activeProviderId]); - // Look up the current model's context window from models.dev (cached). When // Auto is enabled, the detected value is applied to contextLimit automatically // as the model changes; the user can still override with a manual value or @@ -161,25 +142,6 @@ export function AISettings() { ) : ( <> - updateAIConfig({ baseUrl: url })} showPresets={false} /> - setShowKey((s) => !s)} - onChange={(key) => updateAIConfig({ apiKey: key })} - /> - updateAIConfig({ model: m })} - onFetch={() => fetchModels(baseUrl, apiKey)} - /> {/* Max Steps — applies to all providers */} {maxStepsRow} @@ -242,11 +204,6 @@ export function AISettings() { label="Enable subagents" /> - - {/* Cloud connection status */} -
- -
)} diff --git a/src/components/settings/AgentSettings.tsx b/src/components/settings/AgentSettings.tsx index 964ee4226..9c9104750 100644 --- a/src/components/settings/AgentSettings.tsx +++ b/src/components/settings/AgentSettings.tsx @@ -12,10 +12,6 @@ import { contextLimitForModel } from "@/lib/models-dev"; import { useCopyToClipboard } from "@/hooks/useCopyToClipboard"; import type { CodingAgent } from "@/store/slices/coding-agents"; import { SettingsGroup, SettingsRow, StepperSettingsRow } from "./shared"; -import { - BaseUrlRow, ApiKeyRow, ModelSelectionRow, CloudConnectionStatus, - useEndpointConfig, isLocalBaseUrl, -} from "./endpoint-components"; import { ProviderManager } from "./ProviderManager"; // ── Agent form ──────────────────────────────────────────────────────────────── @@ -409,33 +405,16 @@ export function AgentSettings() { const [adding, setAdding] = useState(false); const [editingId, setEditingId] = useState(null); - const { - showKey: showKeyAgent, testState: testStateAgent, testError: testErrorAgent, - availableModels: availableModelsAgent, modelsLoading: modelsLoadingAgent, - setShowKey: setShowKeyAgent, fetchModels: fetchModelsAgent, ensureModels: ensureModelsAgent, resetModels: resetModelsAgent, - } = useEndpointConfig(); - useEffect(() => { fetchAgents(); }, [fetchAgents]); - const { baseUrl: baseUrlAgent, model: modelAgent, apiKey: apiKeyAgent, maxSteps: maxStepsAgent, temperature: temperatureAgent, contextLimit: contextLimitAgent, autoApprove = true } = agentConfig; - const isLocalAgent = isLocalBaseUrl(baseUrlAgent); + // Connection fields (baseUrl/apiKey/model) are managed by the ProviderManager + // switcher below; here we only need `model` (context lookup) + behavioural fields. + const { model: modelAgent, maxSteps: maxStepsAgent, temperature: temperatureAgent, contextLimit: contextLimitAgent, autoApprove = true } = agentConfig; function updateAgent(patch: Partial) { setAgentConfig(patch); - if (patch.baseUrl !== undefined || patch.apiKey !== undefined) { - resetModelsAgent(); - } } - // Populate the agent's model list from cache (or fetch once) on mount and when - // the active saved provider changes. Not on every keystroke in the URL/key - // fields — Refresh in the picker re-queries after edits. No hardcoded - // fallbacks — the picker shows only real models. - useEffect(() => { - ensureModelsAgent(baseUrlAgent, apiKeyAgent); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [agentConfig.activeProviderId]); - // Look up the agent model's context window from models.dev (cached). When Auto // is enabled, the detected value is applied to contextLimit automatically as // the model changes; a manual value or preset turns Auto off. Cloud only. @@ -485,34 +464,9 @@ export function AgentSettings() { title="Cairn Coding Agent (Pi)" description="Configure endpoint parameters for the native autonomous coding agent. Coding agents require high-capacity cloud/local models supporting function calling." > - {/* Saved providers switcher */} + {/* Saved providers switcher (base URL, key, model all live in the form) */} - {/* Base URL */} - updateAgent({ baseUrl: url })} showPresets={false} /> - - {/* API Key */} - setShowKeyAgent((s) => !s)} - onChange={(key) => updateAgent({ apiKey: key })} - /> - - {/* Model Selection */} - updateAgent({ model: m })} - onFetch={() => fetchModelsAgent(baseUrlAgent, apiKeyAgent)} - /> - {/* Max steps */} - - {/* Agent Connection Status */} -
- -
{/* Agent list */} diff --git a/src/components/settings/ProviderManager.tsx b/src/components/settings/ProviderManager.tsx index 8e708ed63..1c4648098 100644 --- a/src/components/settings/ProviderManager.tsx +++ b/src/components/settings/ProviderManager.tsx @@ -116,7 +116,10 @@ export function ProviderManager({ kind = "ai" }: { kind?: "ai" | "agent" }) { Edit diff --git a/src/components/settings/endpoint-components.tsx b/src/components/settings/endpoint-components.tsx index 8e6a64d96..03e1f7fca 100644 --- a/src/components/settings/endpoint-components.tsx +++ b/src/components/settings/endpoint-components.tsx @@ -94,9 +94,14 @@ export function useEndpointConfig(): UseEndpointConfigResult { const [availableModels, setAvailableModels] = useState([]); const [modelsLoading, setModelsLoading] = useState(false); const resetTimerRef = React.useRef | null>(null); + // Advances on every fetch so a slow older request can't overwrite the state of + // a newer one (endpoint/key changed, or a rapid re-fetch). + const fetchGenRef = React.useRef(0); const fetchModels = useCallback(async (baseUrl: string, apiKey: string) => { if (resetTimerRef.current) clearTimeout(resetTimerRef.current); + const gen = ++fetchGenRef.current; + const isCurrent = () => gen === fetchGenRef.current; setModelsLoading(true); setTestState("testing"); @@ -111,25 +116,36 @@ export function useEndpointConfig(): UseEndpointConfigResult { const url = normBaseUrl(baseUrl); const headers: Record = {}; if (apiKey) headers["Authorization"] = `Bearer ${apiKey}`; - const res = await fetch(`${url}/v1/models`, { headers }); - if (!res.ok) throw new Error(`${res.status}`); - const data = await res.json(); - ids = (data?.data ?? []) - .map((m: { id: string }) => m.id) - .filter((id: string) => !id.includes("embed") && !id.includes("whisper") && !id.includes("tts") && !id.includes("dall-e")) - .sort(); + // Abort a hung endpoint after 12s (matches the main-process handler). + const ac = new AbortController(); + const timer = setTimeout(() => ac.abort(), 12_000); + try { + const res = await fetch(`${url}/v1/models`, { headers, signal: ac.signal }); + if (!res.ok) throw new Error(`${res.status}`); + const data = await res.json(); + ids = (data?.data ?? []) + .map((m: { id: string }) => m.id) + .filter((id: string) => !id.includes("embed") && !id.includes("whisper") && !id.includes("tts") && !id.includes("dall-e")) + .sort(); + } finally { + clearTimeout(timer); + } } + if (!isCurrent()) return; // a newer fetch superseded this one setAvailableModels(ids); writeModelCache(baseUrl, apiKey, ids); // persist for next open (keyed by endpoint, not the key value) setTestState("ok"); } catch (err) { + if (!isCurrent()) return; setTestState("error"); setTestError(err instanceof Error ? err.message : "Failed to fetch models"); setAvailableModels([]); } finally { - setModelsLoading(false); - resetTimerRef.current = setTimeout(() => setTestState("idle"), 5000); + if (isCurrent()) { + setModelsLoading(false); + resetTimerRef.current = setTimeout(() => setTestState("idle"), 5000); + } } }, []); diff --git a/src/store/slices/ui.ts b/src/store/slices/ui.ts index 1ac066bb7..d86fe6f11 100644 --- a/src/store/slices/ui.ts +++ b/src/store/slices/ui.ts @@ -153,7 +153,10 @@ export async function migrateLlmKeysToKeychain( // 2. Mirror the active provider's (now-ref) key onto the top-level field. const activeAi = migratedProviders.find((p) => p.id === nextAi.activeProviderId); if (activeAi) { - nextAi = { ...nextAi, apiKey: activeAi.apiKey }; + if (nextAi.apiKey !== activeAi.apiKey) { + nextAi = { ...nextAi, apiKey: activeAi.apiKey }; + changed = true; // persist the refreshed top-level key + } } else if (nextAi.apiKey && !isKeyRef(nextAi.apiKey)) { // No provider selected but a legacy raw top-level key exists: store it under // a stable synthetic id so it survives as a ref (keyless if it fails). From 02ccd78e712cda02110d04310e0bac75651b155d Mon Sep 17 00:00:00 2001 From: Gerard Slee Date: Sat, 25 Jul 2026 11:54:21 -0400 Subject: [PATCH 5/5] Fix CI: generate What's New registry before tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/generated/new-features.json is git-ignored and imported by app code that the unit tests load (complete-onboarding → new-features-registry), so the Unit and E2E jobs failed with ERR_MODULE_NOT_FOUND. - npm test now runs generate-licenses + generate-features before compile/vitest, so it's self-sufficient locally and in CI - E2E job generates both JSON files before test:e2e (Playwright builds the app) --- .github/workflows/ci.yml | 7 +++++++ package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2fec7855..aa2da5a01 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,6 +104,8 @@ jobs: cp node_modules/better-sqlite3/build/Release/better_sqlite3.node vitest-native/ - name: Run tests + # `npm test` generates licenses + the What's New registry (imported by app + # code under test) and compiles before running vitest. run: npm test # ── Build ─────────────────────────────────────────────────────────────────── @@ -192,6 +194,11 @@ jobs: - name: Install Playwright system dependencies run: npx playwright install-deps chromium + - name: Generate licenses + What's New registry (baked into the built app) + run: | + node scripts/generate-licenses.js + node scripts/generate-features.js + - name: Run E2E tests run: npm run test:e2e diff --git a/package.json b/package.json index 45eb3f22a..ccca603b2 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "author": "Cairn", "private": true, "scripts": { - "test": "npm run compile && vitest run", + "test": "node scripts/generate-licenses.js && node scripts/generate-features.js && npm run compile && vitest run", "test:fast": "vitest run", "test:live": "CAIRN_LIVE_TESTS=1 vitest run", "test:unit": "vitest run --project node",