diff --git a/docs/superpowers/plans/2026-09-07-api-key-vault.md b/docs/superpowers/plans/2026-09-07-api-key-vault.md new file mode 100644 index 00000000..cc8e2d7e --- /dev/null +++ b/docs/superpowers/plans/2026-09-07-api-key-vault.md @@ -0,0 +1,1973 @@ +# API Key Vault + Session Text Delivery Implementation Plan + +## Implementation Review Amendments + +The original task/code sketches below are a historical plan, not a second implementation. The following review corrections supersede their affected examples; the colocated code and regression tests are authoritative. + +- [x] Reject stale authentication and secret-read completions after lock; share one OS prompt and broadcast lock to renderer caches. +- [x] Attempt Electron's user-presence API rather than using its biometric-only capability check as a password gate. +- [x] Serialize complete CRUD transactions; use exclusive unique temporary files, validate identifiers/index versions, and preserve damaged indexes instead of replacing them with empty data. +- [x] Reuse the app workspace context in the vault modal; support inline rename, retain failed edits, and prevent cached-key insertion from bypassing the main gate. +- [x] Resolve vault references AFTER ordinary template variables at final insertion. Preserve dynamic bodies, freeze the target session, and cancel stale preparation. +- [x] Deliver terminal text through the mounted xterm owner. Respect attach/replay, visibility and live bracketed-paste mode; reject multiline text without bracketed support and embedded terminal controls. Do not automatically retry a refused write. +- [x] Document vault-only at-rest protection: inserted text follows ordinary plaintext draft/scrollback/transcript retention. No end-to-end prompt secrecy is claimed. +- [x] Add service, filesystem, modal and paste regression tests; correct the permissions assertion to check file modes. +- [ ] Finish follow-up Claude/Codex review, PR CI and manual OS-authentication/terminal smoke checks before merging. + +Verification during review: targeted tests and typecheck pass; the desktop/remote build succeeds. A low-concurrency full suite timed out and reported unrelated lazy-Markdown/fixture failures; it is NOT a passing full-suite gate. On this Node 25 host, use `NODE_OPTIONS=--no-experimental-webstorage` to avoid Node's experimental global localStorage interfering with browser test storage; CI uses its pinned Node version. + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ship an encrypted API Key Vault (Touch ID / Mac-password unlock, once per app launch) whose keys insert into any focused pane — composer, agent terminal view, or raw terminal — via a shared session-text delivery helper that also makes prompt templates work over terminals, including `{{key:Provider/Key}}` template references. + +**Architecture:** Two features, one branch. (1) A renderer-owned `deliverTextToSession` helper dispatches by session kind and effective surface: composer draft append for rendered agent panes; bracketed-paste-without-Enter over `window.api.sendInput` for any focused PTY (plain terminals and agent panes in terminal view both use that channel). (2) A main-process `VaultService` persists providers in a plaintext index and one safeStorage-encrypted blob per key under `STATE_DIR/key-vault/`, gated by `systemPreferences.promptTouchID` once per app run and failing closed. Prompt templates resolve `{{key:Provider/Key}}` refs against the vault before insertion. + +**Tech Stack:** Electron 43 (`safeStorage`, `systemPreferences.promptTouchID`, `clipboard`), zustand uiShell, existing command palette + surface registry, vitest (unit + renderer projects). + +**Issues:** #830 (session-text delivery), #831 (key vault). Worktree: `.worktrees/feat-api-key-vault`, branch `feat/api-key-vault` (already created from `main`). + +**Verification commands** (run in the worktree root): + +```bash +npm run typecheck +npm run test:unit -- src/main/keyVault +npm run test:unit -- src/renderer/src/features/prompt-templates +npm run test:renderer -- src/renderer/src/features/session-text-delivery +npm run check:keybindings +npm run test:contract +``` + +--- + +### Task 0: Bootstrap the worktree + +**Files:** none (environment only). + +- [ ] **Step 1: Initialize submodules and install dependencies** + +The fresh worktree has no submodule checkouts or `node_modules`, and every script below needs both (dev aliases compile packages from `src/`). + +Run (workdir `.worktrees/feat-api-key-vault`): + +```bash +git submodule update --init --recursive && npm install +``` + +Expected: submodule checkouts populate `packages/*/src` and `npm install` succeeds (`postinstall` rebuilds node-pty). + +- [ ] **Step 2: Baseline the type check** + +Run: `npm run typecheck` +Expected: exits 0 before any edits. + +--- + +### Task 1: Vault store (shared types + encrypted per-key persistence) + +**Files:** +- Create: `src/shared/types/keyVault.ts` +- Create: `src/main/keyVault/vaultStore.ts` +- Test: `src/main/keyVault/vaultStore.test.ts` + +- [ ] **Step 1: Write the shared wire types** + +Create `src/shared/types/keyVault.ts`: + +```ts +// Wire contract for the API Key Vault (issue #831). +// +// WHY a shared module: vault data crosses the preload bridge in both +// directions (renderer edits metadata, main returns snapshots), so the +// renderer must type its UI against something importable from both +// processes without dragging main-process storage code into the bundle. +// Mirrors the other @shared/types contracts. + +/** A user-created provider bucket (e.g. "Brave", "OpenAI"). The vault + * starts empty by design (#831): no seed list of providers to go stale + * as services appear and disappear. */ +export type KeyVaultProvider = { + id: string + name: string + createdAt: number + updatedAt: number +} + +/** Non-secret metadata for one stored key. The secret value NEVER crosses + * this type: it lives in an encrypted per-key blob on disk and is only + * returned by the gated reveal/copy calls. `hint` is the last four + * characters of the value, captured at write time, so the vault UI can + * confirm identity without decrypting anything. */ +export type KeyVaultKey = { + id: string + providerId: string + name: string + note: string + hint: string + createdAt: number + updatedAt: number +} + +export type KeyVaultSnapshot = { + providers: KeyVaultProvider[] + keys: KeyVaultKey[] +} + +export type KeyVaultStatus = { + /** Electron safeStorage reports the OS keyring usable. */ + encryptionAvailable: boolean + /** macOS can present the Touch ID / login-password prompt. */ + authPromptAvailable: boolean + /** The once-per-app-run unlock gate has been passed. */ + unlocked: boolean +} + +export type KeyVaultKeyInput = { + providerId: string + /** Omit to create a new key; include to update an existing one. An + * empty `value` on update means "keep the existing secret" so users + * can rename/re-note without re-pasting the key. */ + id?: string + name: string + value: string + note: string +} +``` + +- [ ] **Step 2: Write the failing store tests** + +Create `src/main/keyVault/vaultStore.test.ts`: + +```ts +import { mkdtemp, readFile, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { beforeEach, describe, expect, it } from 'vitest' + +import { createFileVaultStore, type SecretCodec } from '@main/keyVault/vaultStore.js' + +// Deterministic fake codec: real safeStorage cannot run under vitest +// (the electron module is the packaged app), and the store's job here is +// file discipline, not cryptography — safeStorage itself is exercised by +// the packaged app and by Electron upstream. +const fakeCodec: SecretCodec = { + isEncryptionAvailable: () => true, + encrypt: plain => Buffer.from(`enc:${plain}`, 'utf8'), + decrypt: cipher => cipher.toString('utf8').slice(4), +} + +let root: string + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'agent-code-vault-')) +}) + +describe('vaultStore', () => { + it('round-trips index edits and secrets', async () => { + const store = createFileVaultStore(root, fakeCodec) + const snapshot = await store.loadIndex() + const provider = { id: 'p1', name: 'Brave', createdAt: 1, updatedAt: 1 } + snapshot.providers.push(provider) + snapshot.keys.push({ + id: 'k1', providerId: 'p1', name: 'main', note: '', hint: '_key', + createdAt: 1, updatedAt: 1, + }) + await store.saveIndex(snapshot) + await store.writeSecret('k1', 'BSA-secret-value') + + const reloaded = createFileVaultStore(root, fakeCodec) + expect((await reloaded.loadIndex()).providers).toEqual([provider]) + expect(await reloaded.readSecret('k1')).toBe('BSA-secret-value') + }) + + it('starts with an empty snapshot on a fresh directory', async () => { + const store = createFileVaultStore(root, fakeCodec) + const snapshot = await store.loadIndex() + expect(snapshot.providers).toEqual([]) + expect(snapshot.keys).toEqual([]) + }) + + it('isolates a corrupt blob to a single key', async () => { + const store = createFileVaultStore(root, fakeCodec) + const snapshot = await store.loadIndex() + snapshot.providers.push({ id: 'p1', name: 'Brave', createdAt: 1, updatedAt: 1 }) + snapshot.keys.push( + { id: 'good', providerId: 'p1', name: 'a', note: '', hint: 'aaaa', createdAt: 1, updatedAt: 1 }, + { id: 'bad', providerId: 'p1', name: 'b', note: '', hint: 'bbbb', createdAt: 1, updatedAt: 1 }, + ) + await store.saveIndex(snapshot) + await store.writeSecret('good', 'one') + await store.writeSecret('bad', 'two') + // Corrupt exactly one blob on disk — the Keychain-reset scenario from + // apiKeyStore.ts: a decrypt failure must cost one key, not the vault. + await writeFile(join(root, 'keys', 'bad.bin'), Buffer.from('garbage')) + + expect(await store.readSecret('good')).toBe('one') + expect(await store.readSecret('bad')).toBeNull() + }) + + it('reports encryption availability through the codec', async () => { + const unavailable = createFileVaultStore(root, { ...fakeCodec, isEncryptionAvailable: () => false }) + expect(unavailable.encryptionAvailable()).toBe(false) + }) + + it('deletes secret blobs without touching the index', async () => { + const store = createFileVaultStore(root, fakeCodec) + await store.writeSecret('k1', 'v') + await store.deleteSecret('k1') + expect(await store.readSecret('k1')).toBeNull() + // Index file still parses after secret deletion. + await expect(store.loadIndex()).resolves.toBeTruthy() + }) + + it('writes secret blobs with 0600 permissions', async () => { + const store = createFileVaultStore(root, fakeCodec) + await store.writeSecret('k1', 'v') + const stat = await readFile(join(root, 'keys', 'k1.bin')) + expect(stat.length).toBeGreaterThan(0) + }) +}) +``` + +- [ ] **Step 3: Run tests to verify they fail** + +Run: `npm run test:unit -- src/main/keyVault` +Expected: FAIL — cannot resolve `@main/keyVault/vaultStore.js`. + +- [ ] **Step 4: Implement the store** + +Create `src/main/keyVault/vaultStore.ts`: + +```ts +import { randomUUID } from 'node:crypto' +import { chmod, mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises' +import { dirname, join } from 'node:path' + +import { STATE_DIR } from '@main/storage/paths.js' +import type { KeyVaultSnapshot } from '@shared/types/keyVault' + +// Vault persistence: one plaintext index + one encrypted blob per key +// secret (issue #831). +// +// WHY per-key blobs instead of one encrypted vault document: the same +// reasoning as src/main/dictation/apiKeyStore.ts — a malformed cipher +// blob (safeStorage refusing to decrypt after a macOS Keychain reset) +// must cost exactly one key, never the whole vault. The index carries +// only non-secret metadata (names, notes, timestamps, last-4 hints), so +// the provider list renders without touching decryption at all. +// +// WHY the codec is injected: unit tests run outside the packaged app +// where the `electron` module (and therefore safeStorage) does not +// exist. The store's own responsibilities are file discipline; the real +// codec is wired in src/main/index.ts. +// +// Write discipline mirrors apiKeyStore / remote/auth/secret.ts: mkdir +// recursive, temp file + atomic rename, 0o600 on secret blobs, and a +// decrypt failure reads back as null instead of throwing so callers can +// degrade per key. + +export type SecretCodec = { + isEncryptionAvailable(): boolean + encrypt(plain: string): Buffer + decrypt(cipher: Buffer): string +} + +export function newVaultId(): string { + return randomUUID() +} + +type IndexFile = KeyVaultSnapshot & { version: 1 } + +export type VaultStore = { + encryptionAvailable(): boolean + loadIndex(): Promise + saveIndex(snapshot: KeyVaultSnapshot): Promise + readSecret(keyId: string): Promise + writeSecret(keyId: string, value: string): Promise + deleteSecret(keyId: string): Promise +} + +export function createFileVaultStore( + rootDir: string = join(STATE_DIR, 'key-vault'), + codec: SecretCodec, +): VaultStore { + const indexFile = join(rootDir, 'index.json') + const keysDir = join(rootDir, 'keys') + + async function atomicWrite(path: string, data: string | Buffer, mode: 0o600 | undefined): Promise { + await mkdir(dirname(path), { recursive: true }) + const tmp = `${path}.tmp` + await writeFile(tmp, data, mode !== undefined ? { mode } : undefined) + if (mode !== undefined) await chmod(tmp, mode).catch(() => {}) + await rename(tmp, path) + if (mode !== undefined) await chmod(path, mode).catch(() => {}) + } + + return { + encryptionAvailable: () => codec.isEncryptionAvailable(), + + async loadIndex() { + let raw: string + try { + raw = await readFile(indexFile, 'utf8') + } catch { + // Absent index = fresh vault. A CORRUPT index is handled by the + // service (it decides between fail-closed and reset); the store + // only reports what it can parse. + return { providers: [], keys: [] } + } + try { + const parsed = JSON.parse(raw) as IndexFile + return { providers: parsed.providers ?? [], keys: parsed.keys ?? [] } + } catch { + // Unparseable index is treated as absent: the vault degrades to + // empty rather than bricking startup. Secret blobs on disk become + // orphans, which is the safe direction (no silent secret loss — + // the loss already happened when the index corrupted). + return { providers: [], keys: [] } + } + }, + + async saveIndex(snapshot) { + const file: IndexFile = { version: 1, providers: snapshot.providers, keys: snapshot.keys } + // Index is non-secret metadata; 0o600 is harmless belt-and-braces. + await atomicWrite(indexFile, `${JSON.stringify(file, null, 2)}\n`, 0o600) + }, + + async readSecret(keyId) { + if (!codec.isEncryptionAvailable()) return null + let cipher: Buffer + try { + cipher = await readFile(join(keysDir, `${keyId}.bin`)) + } catch { + return null + } + try { + const plain = codec.decrypt(cipher) + return plain.length > 0 ? plain : null + } catch { + // Decrypt failure after a Keychain reset: report unreadable, + // leave the blob in place (matches apiKeyStore's reasoning that + // future safeStorage recovery should stay possible). + return null + } + }, + + async writeSecret(keyId, value) { + await atomicWrite(join(keysDir, `${keyId}.bin`), codec.encrypt(value), 0o600) + }, + + async deleteSecret(keyId) { + await rm(join(keysDir, `${keyId}.bin`), { force: true }) + }, + } +} +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `npm run test:unit -- src/main/keyVault` +Expected: PASS (6 tests). + +- [ ] **Step 6: Commit** + +```bash +git add src/shared/types/keyVault.ts src/main/keyVault/vaultStore.ts src/main/keyVault/vaultStore.test.ts +git commit -m "feat(vault): add encrypted per-key vault store + +Per-key safeStorage blobs under STATE_DIR/key-vault with a non-secret +metadata index, mirroring the dictation apiKeyStore failure discipline: +a corrupt cipher blob costs one key, never the vault. Codec injection +keeps the file discipline unit-testable outside the packaged app. + +Refs #831" +``` + +--- + +### Task 2: Vault service (Touch ID gate, CRUD, clipboard) + +**Files:** +- Create: `src/main/keyVault/VaultService.ts` +- Test: `src/main/keyVault/VaultService.test.ts` + +- [ ] **Step 1: Write the failing service tests** + +Create `src/main/keyVault/VaultService.test.ts`: + +```ts +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { VaultService, type VaultServiceDeps } from '@main/keyVault/VaultService.js' +import type { VaultStore } from '@main/keyVault/vaultStore.js' +import type { KeyVaultSnapshot } from '@shared/types/keyVault' + +// In-memory store: the file layer has its own tests (vaultStore.test.ts); +// these tests pin the SERVICE rules — gate semantics, ordering, and +// fail-closed behavior. +function makeStore(): VaultStore & { snapshot: KeyVaultSnapshot } { + const snapshot: KeyVaultSnapshot = { providers: [], keys: [] } + const secrets = new Map() + return { + snapshot, + encryptionAvailable: () => true, + loadIndex: async () => ({ providers: [...snapshot.providers], keys: [...snapshot.keys] }), + saveIndex: async next => { + snapshot.providers = [...next.providers] + snapshot.keys = [...next.keys] + }, + readSecret: async id => secrets.get(id) ?? null, + writeSecret: async (id, value) => void secrets.set(id, value), + deleteSecret: async id => void secrets.delete(id), + } +} + +function makeDeps(overrides: Partial = {}): VaultServiceDeps { + return { + store: makeStore(), + promptAuth: vi.fn(async () => {}), + canPromptAuth: () => true, + copyToClipboard: vi.fn(), + now: () => 1_000, + ...overrides, + } +} + +describe('VaultService unlock gate', () => { + it('prompts exactly once per run for repeated reveals', async () => { + const deps = makeDeps() + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'BSA-xyz', note: '' }) + + await service.reveal(provider.id, key.id) + await service.copyKey(provider.id, key.id) + await service.reveal(provider.id, key.id) + + expect(deps.promptAuth).toHaveBeenCalledTimes(1) + expect(service.getStatus().unlocked).toBe(true) + }) + + it('fails closed when the user cancels the OS prompt', async () => { + const deps = makeDeps({ promptAuth: vi.fn(async () => { throw new Error('user canceled') }) }) + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'BSA-xyz', note: '' }) + + await expect(service.reveal(provider.id, key.id)).rejects.toThrow('user canceled') + expect(service.getStatus().unlocked).toBe(false) + }) + + it('fails closed when no auth prompt mechanism exists', async () => { + const service = new VaultService(makeDeps({ canPromptAuth: () => false })) + await expect(service.unlock()).rejects.toThrow(/authentication is unavailable/i) + }) + + it('fails closed when the OS keyring is unavailable', async () => { + const store = makeStore() + const service = new VaultService(makeDeps({ + store: Object.assign(store, { encryptionAvailable: () => false }), + })) + await expect(service.unlock()).rejects.toThrow(/keyring unavailable/i) + }) + + it('lock() re-arms the gate within the same run', async () => { + const deps = makeDeps() + const service = new VaultService(deps) + await service.unlock() + service.lock() + await service.unlock() + expect(deps.promptAuth).toHaveBeenCalledTimes(2) + }) +}) + +describe('VaultService CRUD', () => { + let service: VaultService + let providerId: string + + beforeEach(async () => { + service = new VaultService(makeDeps()) + const provider = await service.createProvider('Brave') + providerId = provider.id + }) + + it('round-trips providers and keys with hints', async () => { + const key = await service.putKey({ providerId, name: 'main', value: 'BSA-abcdef1234', note: 'prod' }) + expect(key.hint).toBe('1234') + const list = await service.list() + expect(list.providers.map(p => p.name)).toEqual(['Brave']) + expect(list.keys.map(k => k.name)).toEqual(['main']) + expect(await service.reveal(providerId, key.id)).toBe('BSA-abcdef1234') + }) + + it('updating with an empty value keeps the existing secret', async () => { + const key = await service.putKey({ providerId, name: 'main', value: 'BSA-one', note: '' }) + await service.putKey({ providerId, id: key.id, name: 'renamed', value: '', note: 'n' }) + const list = await service.list() + expect(list.keys[0].name).toBe('renamed') + // Hint unchanged: the stored secret is unchanged. + expect(list.keys[0].hint).toBe('-one') + expect(await service.reveal(providerId, key.id)).toBe('BSA-one') + }) + + it('rejects duplicate provider names case-insensitively', async () => { + await expect(service.createProvider('brave')).rejects.toThrow(/already exists/i) + }) + + it('rejects duplicate key names within a provider', async () => { + await service.putKey({ providerId, name: 'main', value: 'a', note: '' }) + await expect(service.putKey({ providerId, name: 'main', value: 'b', note: '' })).rejects.toThrow(/already exists/i) + }) + + it('deleting a provider removes its key secrets too', async () => { + const key = await service.putKey({ providerId, name: 'main', value: 'x', note: '' }) + await service.deleteProvider(providerId) + const list = await service.list() + expect(list.providers).toEqual([]) + expect(list.keys).toEqual([]) + expect(await new VaultService(makeDeps()).list()).resolves.toBeTruthy() + }) + + it('resolveReference matches by provider/key name after gating', async () => { + await service.putKey({ providerId, name: 'main', value: 'BSA-ref', note: '' }) + await expect(service.resolveReference('Brave', 'main')).resolves.toBe('BSA-ref') + await expect(service.resolveReference('Brave', 'missing')).rejects.toThrow(/not found/i) + }) + + it('corrupt secret blob surfaces as a readable-key error', async () => { + // Simulate the Keychain-reset corruption: metadata present, secret + // unreadable. Contract: reveal names the key instead of returning + // null or throwing something opaque. + const nullStore = Object.assign(makeStore(), { readSecret: async () => null }) + const service2 = new VaultService(makeDeps({ store: nullStore })) + const provider = await service2.createProvider('Brave') + const key = await service2.putKey({ providerId: provider.id, name: 'main', value: 'y', note: '' }) + // Break the in-memory secret AFTER creation so putKey's availability + // probe still saw a value. + ;(nullStore as unknown as { readSecret: () => Promise }).readSecret = async () => null + await expect(service2.reveal(provider.id, key.id)).rejects.toThrow(/cannot be decrypted/i) + }) +}) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npm run test:unit -- src/main/keyVault` +Expected: FAIL — cannot resolve `VaultService.js`. + +- [ ] **Step 3: Implement the service** + +Create `src/main/keyVault/VaultService.ts`: + +```ts +import type { KeyVaultKey, KeyVaultKeyInput, KeyVaultProvider, KeyVaultSnapshot, KeyVaultStatus } from '@shared/types/keyVault' +import { newVaultId, type VaultStore } from '@main/keyVault/vaultStore.js' + +// Vault orchestration (issue #831): CRUD over the store, the once-per-run +// unlock gate, and clipboard copies. +// +// WHY the gate lives here and not in the IPC layer: every secret-leaving +// path (reveal, copy, resolveReference) must pass through ONE gate. The +// gate is a plain boolean per app run — `promptAuth` is injected so unit +// tests never trigger a real OS dialog, and production wires it to +// systemPreferences.promptTouchID, which presents Touch ID with the +// login-password fallback (the "mac password" requirement). If the +// platform cannot prompt at all, we fail closed: no secret leaves main. +// +// WHY secrets never appear in the snapshot: the renderer list view is +// built entirely from non-secret metadata; plaintext crosses the bridge +// only as the return value of the gated reveal/copy calls and lives in +// ephemeral component state at most. + +export type VaultServiceDeps = { + store: VaultStore + /** Presents the OS auth prompt. Rejects when the user cancels. */ + promptAuth: (reason: string) => Promise + /** Whether the OS can present the prompt at all. */ + canPromptAuth: () => boolean + copyToClipboard: (text: string) => void + /** Injectable clock for deterministic timestamps. */ + now?: () => number +} + +export class VaultService { + private unlocked = false + + constructor(private readonly deps: VaultServiceDeps) {} + + getStatus(): KeyVaultStatus { + return { + encryptionAvailable: this.deps.store.encryptionAvailable(), + authPromptAvailable: this.deps.canPromptAuth(), + unlocked: this.unlocked, + } + } + + lock(): void { + this.unlocked = false + } + + async unlock(): Promise { + await this.ensureUnlocked() + } + + private async ensureUnlocked(): Promise { + if (this.unlocked) return + if (!this.deps.store.encryptionAvailable()) { + throw new Error('System keyring unavailable — the vault cannot read or store keys on this machine.') + } + if (!this.deps.canPromptAuth()) { + throw new Error('macOS authentication is unavailable — the vault stays locked. (Touch ID / login password prompt required.)') + } + await this.deps.promptAuth('unlock the Agent Code API key vault') + this.unlocked = true + } + + async list(): Promise { + return this.deps.store.loadIndex() + } + + async reveal(providerId: string, keyId: string): Promise { + await this.ensureUnlocked() + const key = await this.findKey(providerId, keyId) + const secret = await this.deps.store.readSecret(keyId) + if (secret === null) { + throw new Error(`Key "${key.name}" cannot be decrypted (Keychain reset or corrupted blob). Re-enter the value to fix it.`) + } + return secret + } + + async copyKey(providerId: string, keyId: string): Promise { + const value = await this.reveal(providerId, keyId) + this.deps.copyToClipboard(value) + } + + /** Resolve a `{{key:Provider/Key}}` template reference by NAME (#831). + * Names, not ids, so user-authored templates stay readable; renaming + * breaks references loudly rather than silently. */ + async resolveReference(providerName: string, keyName: string): Promise { + await this.ensureUnlocked() + const snapshot = await this.deps.store.loadIndex() + const provider = snapshot.providers.find(p => p.name === providerName) + if (!provider) throw new Error(`Key vault provider "${providerName}" not found.`) + const key = snapshot.keys.find(k => k.providerId === provider.id && k.name === keyName) + if (!key) throw new Error(`Key "${keyName}" not found for provider "${providerName}".`) + const secret = await this.deps.store.readSecret(key.id) + if (secret === null) { + throw new Error(`Key "${key.name}" cannot be decrypted (Keychain reset or corrupted blob). Re-enter the value to fix it.`) + } + return secret + } + + async createProvider(name: string): Promise { + const trimmed = name.trim() + if (!trimmed) throw new Error('Provider name cannot be empty.') + const snapshot = await this.deps.store.loadIndex() + if (snapshot.providers.some(p => p.name.toLowerCase() === trimmed.toLowerCase())) { + throw new Error(`Provider "${trimmed}" already exists.`) + } + const now = this.deps.now?.() ?? Date.now() + const provider: KeyVaultProvider = { id: newVaultId(), name: trimmed, createdAt: now, updatedAt: now } + snapshot.providers.push(provider) + await this.deps.store.saveIndex(snapshot) + return provider + } + + async renameProvider(id: string, name: string): Promise { + const trimmed = name.trim() + if (!trimmed) throw new Error('Provider name cannot be empty.') + const snapshot = await this.deps.store.loadIndex() + const provider = snapshot.providers.find(p => p.id === id) + if (!provider) throw new Error('Provider not found.') + if (snapshot.providers.some(p => p.id !== id && p.name.toLowerCase() === trimmed.toLowerCase())) { + throw new Error(`Provider "${trimmed}" already exists.`) + } + provider.name = trimmed + provider.updatedAt = this.deps.now?.() ?? Date.now() + await this.deps.store.saveIndex(snapshot) + } + + async deleteProvider(id: string): Promise { + const snapshot = await this.deps.store.loadIndex() + const remainingKeys = snapshot.keys.filter(k => k.providerId !== id) + // Index-first ordering: a crash mid-delete leaves an orphan blob + // (harmless, invisible) rather than an index entry whose secret is + // already gone (reads as corrupt). + await this.deps.store.saveIndex({ providers: snapshot.providers.filter(p => p.id !== id), keys: remainingKeys }) + for (const key of snapshot.keys) { + if (key.providerId === id) await this.deps.store.deleteSecret(key.id) + } + } + + async putKey(input: KeyVaultKeyInput): Promise { + const name = input.name.trim() + if (!name) throw new Error('Key name cannot be empty.') + const snapshot = await this.deps.store.loadIndex() + if (!snapshot.providers.some(p => p.id === input.providerId)) { + throw new Error('Provider not found.') + } + const now = this.deps.now?.() ?? Date.now() + const existing = input.id ? snapshot.keys.find(k => k.id === input.id) : undefined + if (input.id && !existing) throw new Error('Key not found.') + if (snapshot.keys.some(k => k.id !== existing?.id && k.providerId === input.providerId && k.name === name)) { + throw new Error(`Key "${name}" already exists for this provider.`) + } + + // Secret-first ordering on update: write the new blob BEFORE the + // index references it, so a crash never produces an index entry with + // a missing/stale blob. + const value = input.value.trim() + let hint = existing?.hint ?? '' + if (existing) { + if (value.length > 0) { + await this.deps.store.writeSecret(existing.id, value) + hint = value.slice(-4) + } else if (await this.deps.store.readSecret(existing.id) === null) { + // Editing metadata cannot resurrect an unreadable secret; the + // user must re-enter the value. Surface that now, not at reveal. + throw new Error(`Key "${existing.name}" has no readable stored value — re-enter the value.`) + } + existing.name = name + existing.note = input.note.trim() + existing.hint = hint + existing.updatedAt = now + await this.deps.store.saveIndex(snapshot) + return existing + } + + if (!value) throw new Error('Key value cannot be empty.') + const key: KeyVaultKey = { + id: newVaultId(), + providerId: input.providerId, + name, + note: input.note.trim(), + hint: value.slice(-4), + createdAt: now, + updatedAt: now, + } + await this.deps.store.writeSecret(key.id, value) + snapshot.keys.push(key) + await this.deps.store.saveIndex(snapshot) + return key + } + + async deleteKey(providerId: string, keyId: string): Promise { + const snapshot = await this.deps.store.loadIndex() + const key = snapshot.keys.find(k => k.id === keyId && k.providerId === providerId) + if (!key) throw new Error('Key not found.') + await this.deps.store.saveIndex({ + providers: snapshot.providers, + keys: snapshot.keys.filter(k => k.id !== keyId), + }) + await this.deps.store.deleteSecret(keyId) + } + + private async findKey(providerId: string, keyId: string): Promise { + const snapshot = await this.deps.store.loadIndex() + const key = snapshot.keys.find(k => k.id === keyId && k.providerId === providerId) + if (!key) throw new Error('Key not found.') + return key + } +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npm run test:unit -- src/main/keyVault` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add src/main/keyVault/VaultService.ts src/main/keyVault/VaultService.test.ts +git commit -m "feat(vault): add touch-id gated vault service + +Every secret-leaving path funnels through one once-per-run gate backed by +an injected promptAuth (systemPreferences.promptTouchID in production), +failing closed when the platform cannot prompt. Snapshot metadata never +carries secrets; template references resolve by provider/key name. + +Refs #831" +``` + +--- + +### Task 3: IPC handlers + preload API + main wiring + +**Files:** +- Create: `src/main/ipc/keyVault.ts` +- Modify: `src/main/ipc/index.ts` (import + deps entry + register call) +- Create: `src/preload/api/keyVault.ts` +- Modify: `src/preload/api/index.ts` (import + spread) +- Modify: `src/main/index.ts` (construct service, pass to registerAllIpc) + +- [ ] **Step 1: Create the IPC module** + +Create `src/main/ipc/keyVault.ts` (mirrors `src/main/ipc/caffeinate.ts` — thin handlers, service owns behavior): + +```ts +import { ipcMain } from 'electron' + +import type { VaultService } from '@main/keyVault/VaultService.js' +import type { KeyVaultKeyInput } from '@shared/types/keyVault' + +// Thin IPC surface for the key vault (#831). Handlers validate nothing — +// the service owns all rules — so behavior stays testable without +// spinning up ipcMain. Secrets cross only on the reveal/copy/resolve-ref +// return paths, all of which sit behind the service's unlock gate. +export function registerKeyVaultIpc({ vaultService }: { vaultService: VaultService }): void { + ipcMain.handle('key-vault:status', () => vaultService.getStatus()) + ipcMain.handle('key-vault:list', () => vaultService.list()) + ipcMain.handle('key-vault:unlock', () => vaultService.unlock()) + ipcMain.handle('key-vault:lock', () => vaultService.lock()) + ipcMain.handle('key-vault:reveal', (_event, providerId: string, keyId: string) => + vaultService.reveal(providerId, keyId)) + ipcMain.handle('key-vault:copy-key', (_event, providerId: string, keyId: string) => + vaultService.copyKey(providerId, keyId)) + ipcMain.handle('key-vault:resolve-ref', (_event, providerName: string, keyName: string) => + vaultService.resolveReference(providerName, keyName)) + ipcMain.handle('key-vault:create-provider', (_event, name: string) => vaultService.createProvider(name)) + ipcMain.handle('key-vault:rename-provider', (_event, id: string, name: string) => + vaultService.renameProvider(id, name)) + ipcMain.handle('key-vault:delete-provider', (_event, id: string) => vaultService.deleteProvider(id)) + ipcMain.handle('key-vault:put-key', (_event, input: KeyVaultKeyInput) => vaultService.putKey(input)) + ipcMain.handle('key-vault:delete-key', (_event, providerId: string, keyId: string) => + vaultService.deleteKey(providerId, keyId)) +} +``` + +- [ ] **Step 2: Register it in the IPC aggregator** + +In `src/main/ipc/index.ts`: + +1. Add imports next to the caffeinate import block: + +```ts +import { registerKeyVaultIpc } from '@main/ipc/keyVault.js' +import type { VaultService } from '@main/keyVault/VaultService.js' +``` + +2. Add `vaultService: VaultService` to the `registerAllIpc(deps)` parameter type (mirror how `caffeinateController: CaffeinateController` appears). + +3. Add the call inside `registerAllIpc` next to `registerCaffeinateIpc(deps)`: + +```ts +registerKeyVaultIpc(deps) +``` + +- [ ] **Step 3: Create the preload API module** + +Create `src/preload/api/keyVault.ts`: + +```ts +import { ipcRenderer } from 'electron' + +import type { KeyVaultKeyInput, KeyVaultSnapshot, KeyVaultStatus } from '@shared/types/keyVault' + +// Flat key-vault surface merged into window.api. Rejected promises carry +// the service's Error message (cancel, fail-closed, not-found) so the UI +// can toast it verbatim. +export const keyVaultApi = { + keyVaultStatus: (): Promise => ipcRenderer.invoke('key-vault:status'), + keyVaultList: (): Promise => ipcRenderer.invoke('key-vault:list'), + keyVaultUnlock: (): Promise => ipcRenderer.invoke('key-vault:unlock'), + keyVaultLock: (): Promise => ipcRenderer.invoke('key-vault:lock'), + keyVaultReveal: (providerId: string, keyId: string): Promise => + ipcRenderer.invoke('key-vault:reveal', providerId, keyId), + keyVaultCopyKey: (providerId: string, keyId: string): Promise => + ipcRenderer.invoke('key-vault:copy-key', providerId, keyId), + keyVaultResolveReference: (providerName: string, keyName: string): Promise => + ipcRenderer.invoke('key-vault:resolve-ref', providerName, keyName), + keyVaultCreateProvider: (name: string): Promise => + ipcRenderer.invoke('key-vault:create-provider', name), + keyVaultRenameProvider: (id: string, name: string): Promise => + ipcRenderer.invoke('key-vault:rename-provider', id, name), + keyVaultDeleteProvider: (id: string): Promise => + ipcRenderer.invoke('key-vault:delete-provider', id), + keyVaultPutKey: (input: KeyVaultKeyInput): Promise => + ipcRenderer.invoke('key-vault:put-key', input), + keyVaultDeleteKey: (providerId: string, keyId: string): Promise => + ipcRenderer.invoke('key-vault:delete-key', providerId, keyId), +} +``` + +- [ ] **Step 4: Merge into the preload api object** + +In `src/preload/api/index.ts`, add the import with the other api module imports: + +```ts +import { keyVaultApi } from './keyVault.js' +``` + +and add to the composed flat object (spread-merge; a name collision is a compile error by design): + +```ts + ...keyVaultApi, +``` + +- [ ] **Step 5: Wire the service in main** + +In `src/main/index.ts`, add imports near the other service imports: + +```ts +import { clipboard, systemPreferences } from 'electron' +import { createFileVaultStore } from '@main/keyVault/vaultStore.js' +import { VaultService } from '@main/keyVault/VaultService.js' +``` + +(If `electron` is already imported there, extend that import instead of adding a second one.) + +Construct the service near the `CaffeinateController` construction, before `registerAllIpc`: + +```ts +// API Key Vault (#831). promptTouchID presents Touch ID with the user's +// login password as fallback, which is the "mac password" gate. Unsigned +// dev builds may skip the biometric option but the password path still +// works; if neither is available the service fails closed on unlock. +const vaultService = new VaultService({ + store: createFileVaultStore(), + promptAuth: reason => systemPreferences.promptTouchID(reason), + canPromptAuth: () => systemPreferences.canPromptTouchID(), + copyToClipboard: text => clipboard.writeText(text), +}) +``` + +Then add `vaultService,` to the `registerAllIpc({ ... })` call's deps argument. + +- [ ] **Step 6: Typecheck** + +Run: `npm run typecheck` +Expected: exits 0. (The `electron` import in `main/index.ts` must not duplicate an existing import — merge if flagged.) + +- [ ] **Step 7: Commit** + +```bash +git add src/main/ipc/keyVault.ts src/main/ipc/index.ts src/preload/api/keyVault.ts src/preload/api/index.ts src/main/index.ts +git commit -m "feat(vault): expose vault over ipc and preload + +Thin ipcMain handlers delegating to VaultService, a flat preload surface, +and production wiring of promptTouchID/clipboard in the composition +root. Secrets only cross on gated reveal/copy/resolve-ref returns. + +Refs #831" +``` + +--- + +### Task 4: Session text delivery helper + +**Files:** +- Create: `src/renderer/src/features/session-text-delivery/deliverTextToSession.ts` +- Test: `src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts` + +- [ ] **Step 1: Write the failing tests** + +Create `src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts`: + +```ts +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' +import type { Workspace } from '@renderer/workspace/workspaceStore' +import type { SessionRuntime } from '@renderer/session-runtime/state' +import type { SessionId } from '@renderer/workspace/types' + +// The dispatch matrix (#830): composer draft for rendered agent panes, +// bracketed paste without Enter for every focused PTY (plain terminals +// and agent panes in terminal view share window.api.sendInput). +// window.api is stubbed because the real bridge only exists in the +// packaged app. + +const sendInput = vi.fn(async () => {}) +const ensureSessionLive = vi.fn(async () => {}) + +function makeRuntime(overrides: Partial = {}): SessionRuntime { + return { draftInput: '', processStatus: 'started', ...overrides } as unknown as SessionRuntime +} + +function makeWorkspace(sessions: Record, runtimes: Record): Workspace { + return { + state: { + sessions: Object.fromEntries( + Object.entries(sessions).map(([id, s]) => [id, { id, kind: s.kind, agentViewModeOverride: s.override }]), + ), + }, + getRuntime: (id: SessionId) => runtimes[id], + setDraftInput: vi.fn(), + ensureSessionLive, + } as unknown as Workspace +} + +beforeEach(() => { + sendInput.mockClear() + ensureSessionLive.mockClear() + ;(globalThis as { window?: unknown }).window = { api: { sendInput } } +}) + +describe('deliverTextToSession', () => { + it('appends to the composer draft for a rendered agent pane', async () => { + const workspace = makeWorkspace( + { a: { kind: 'claude', override: 'agent' } }, + { a: makeRuntime({ draftInput: 'existing' }) }, + ) + const result = await deliverTextToSession(workspace, 'a', 'new text', { insertMode: 'append' }) + expect(result).toEqual({ delivered: true, surface: 'composer' }) + expect(workspace.setDraftInput).toHaveBeenCalledWith('a', 'existing\n\nnew text') + expect(sendInput).not.toHaveBeenCalled() + }) + + it('bracket-pastes without Enter into a plain terminal pane', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime() }) + const result = await deliverTextToSession(workspace, 't', 'line1\nline2') + expect(result).toEqual({ delivered: true, surface: 'pty' }) + expect(sendInput).toHaveBeenCalledWith('t', '\x1b[200~line1\nline2\x1b[201~') + }) + + it('bracket-pastes into an agent pane in terminal view', async () => { + const workspace = makeWorkspace( + { a: { kind: 'claude', override: 'terminal' } }, + { a: makeRuntime() }, + ) + const result = await deliverTextToSession(workspace, 'a', 'key') + expect(result).toEqual({ delivered: true, surface: 'pty' }) + expect(sendInput).toHaveBeenCalledWith('a', '\x1b[200~key\x1b[201~') + }) + + it('wakes a sleeping backend before writing to a PTY', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime({ processStatus: 'stopped' }) }) + await deliverTextToSession(workspace, 't', 'x') + expect(ensureSessionLive).toHaveBeenCalledWith('t', 'session-text-delivery', { awaitInputReady: false }) + expect(sendInput).toHaveBeenCalled() + }) + + it('does not wake an already-started backend', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime() }) + await deliverTextToSession(workspace, 't', 'x') + expect(ensureSessionLive).not.toHaveBeenCalled() + }) + + it('reports no-session for an unknown target', async () => { + const workspace = makeWorkspace({}, {}) + const result = await deliverTextToSession(workspace, 'gone' as SessionId, 'x') + expect(result).toEqual({ delivered: false, reason: 'no-session' }) + }) +}) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npm run test:renderer -- src/renderer/src/features/session-text-delivery` +Expected: FAIL — cannot resolve the module. + +- [ ] **Step 3: Implement the helper** + +Create `src/renderer/src/features/session-text-delivery/deliverTextToSession.ts`: + +```ts +import { useAppStore } from '@renderer/app-state/hooks' +import { applyPromptTemplateInsertMode } from '@renderer/features/prompt-templates/interpolate' +import { getEffectiveAgentSurfaceForSession } from '@renderer/workspace/agentDisplayMode' +import { isSessionExited } from '@renderer/workspace/providerSessionIdentity' +import type { SessionId } from '@renderer/workspace/types' +import type { Workspace } from '@renderer/workspace/workspaceStore' + +// Session text delivery (#830): the ONE routing point for programmatic +// text insertion into a pane. Prompt Templates and the API Key Vault +// both call this instead of hand-rolling per-feature paths. +// +// Dispatch rule — mirror what the user sees: +// * rendered agent surface → composer draft edit (never submits; the +// draft stays visible and editable, matching template insertion's +// "prefill, don't replay" contract) +// * anything with a visible PTY (plain terminal pane, or agent pane +// in terminal view) → bracketed paste via window.api.sendInput, +// which is the SAME channel both surfaces use for keystrokes +// +// WHY no Enter on the PTY path: the user must review what landed before +// it executes. Bracketed paste markers additionally keep shells like +// zsh from executing multi-line payloads line-by-line. Trade-off: a +// program that never enabled bracketed paste mode will print the marker +// bytes — acceptable versus the alternative of raw newlines, which a +// shell would run immediately. + +export type DeliverTextResult = + | { delivered: true; surface: 'composer' | 'pty' } + | { delivered: false; reason: 'no-session' } + +export async function deliverTextToSession( + workspace: Workspace, + sessionId: SessionId, + text: string, + opts?: { insertMode?: 'replace' | 'append' }, +): Promise { + const session = workspace.state.sessions[sessionId] + if (!session) return { delivered: false, reason: 'no-session' } + + if (session.kind !== 'terminal') { + const surface = getEffectiveAgentSurfaceForSession({ + kind: session.kind, + providerRuntime: session.providerRuntime, + globalMode: useAppStore.getState().settings.agentViewMode, + override: session.agentViewModeOverride, + runtime: workspace.getRuntime(sessionId), + }) + if (surface === 'rendered') { + const currentDraft = workspace.getRuntime(sessionId).draftInput + workspace.setDraftInput( + sessionId, + applyPromptTemplateInsertMode(currentDraft, text, opts?.insertMode ?? 'append'), + ) + return { delivered: true, surface: 'composer' } + } + } + return deliverPtyText(workspace, sessionId, text) +} + +async function deliverPtyText( + workspace: Workspace, + sessionId: SessionId, + text: string, +): Promise { + const runtime = workspace.getRuntime(sessionId) + // WHY wake first: lazily-woken restored sessions may have no main-side + // backend yet, and sendInput into a missing backend is silently + // dropped. Same predicate and no input-ready wait as AgentTerminalLeaf + // (#772): readiness is a composer concept, not a PTY one. + if (runtime.processStatus !== 'started' || isSessionExited(runtime)) { + await workspace.ensureSessionLive(sessionId, 'session-text-delivery', { awaitInputReady: false }) + } + await window.api.sendInput(sessionId, `\x1b[200~${text}\x1b[201~`) + return { delivered: true, surface: 'pty' } +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npm run test:renderer -- src/renderer/src/features/session-text-delivery` +Expected: PASS (6 tests). If the `agentViewModeOverride` or `providerRuntime` field names mismatch `SessionMeta`, fix the fake workspace in the test to the real field names — the helper's source is the contract. + +- [ ] **Step 5: Commit** + +```bash +git add src/renderer/src/features/session-text-delivery +git commit -m "feat(session-text): deliver text to any focused pane + +One routing point for programmatic text insertion: composer draft edits +for rendered agent panes, bracketed paste without Enter over sendInput +for plain terminals and agent terminal views, with lazy-wake before PTY +writes. Consumers: prompt templates and the api key vault. + +Refs #830" +``` + +--- + +### Task 5: Prompt templates over terminals + +**Files:** +- Modify: `src/renderer/src/features/prompt-templates/targetSession.ts` +- Modify: `src/renderer/src/features/prompt-templates/commands/promptTemplateCommands.ts` +- Modify: `src/renderer/src/features/command-palette/ui/CommandPalette.tsx` (both insertion sites) +- Test: `src/renderer/src/features/prompt-templates/targetSession.test.ts` (update expectations) + +- [ ] **Step 1: Update targetSession** + +Replace the body of `promptTemplateTargetSessionIdForState` in `src/renderer/src/features/prompt-templates/targetSession.ts` and its doc comment: + +```ts +/** + * Resolves the pane that may receive a prompt template. + * + * WHY this is the shared command target and not narrower (#830): + * terminal panes became valid template targets when insertion learned to + * bracket-paste into any focused PTY via deliverTextToSession. The old + * agent-only predicate hid the command from exactly the panes where + * users run unsupported agent harnesses in a raw terminal. + */ +export function promptTemplateTargetSessionId(workspace: Workspace): string | null { + return promptTemplateTargetSessionIdForState(workspace.state) +} + +export function promptTemplateTargetSessionIdForState(state: WorkspaceState): string | null { + return commandTargetSessionIdForState(state) +} +``` + +- [ ] **Step 2: Update the commands** + +In `src/renderer/src/features/prompt-templates/commands/promptTemplateCommands.ts`: + +1. On the `prompt-template` command, remove the `renderedViewPolicy: { kind: 'requires-rendered-feed' }` line (a terminal-surface pane has no rendered feed; the policy would hide the command where #830 just made it work) and update its description: + +```ts + description: '**What it does:** Inserts a saved **prompt template** into the focused pane.\n\n**Use when:** You want reusable prompt text without retyping it.\n\n**Notes:** Rendered panes insert into the composer; terminal panes receive a bracketed paste without submitting.', +``` + +2. The `save-composer-as-prompt-template` command must stay composer-only — replace its `when` guard: + +```ts + when: ({ workspace }) => { + const sessionId = promptTemplateTargetSessionId(workspace) + if (!sessionId) return false + // Saving reads the composer draft; terminal panes have none. + if ((workspace.state.sessions[sessionId]?.kind ?? 'claude') === 'terminal') return false + return workspace.getRuntime(sessionId).draftInput.trim().length > 0 + }, +``` + +- [ ] **Step 3: Route palette insertion through the helper** + +In `src/renderer/src/features/command-palette/ui/CommandPalette.tsx`: + +1. Add the import: + +```ts +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' +``` + +2. In `executePromptTemplate`, replace the direct-draft insertion block: + +```ts + const currentDraft = workspace.getRuntime(sessionId).draftInput + workspace.setDraftInput(sessionId, applyPromptTemplateInsertMode(currentDraft, body, template.insertMode)) + workspace.showPaneToast(sessionId, `Inserted template: ${template.title}`) + onClose() +``` + +with: + +```ts + const result = await deliverTextToSession(workspace, sessionId, body, { insertMode: template.insertMode }) + if (result.delivered) { + workspace.showPaneToast(sessionId, `Inserted template: ${template.title}`) + onClose() + } else { + workspace.showPaneToast(sessionId, 'Template target pane is gone') + } +``` + +3. In `insertFilledPromptTemplate`, replace: + +```ts + const currentDraft = workspace.getRuntime(sessionId).draftInput + workspace.setDraftInput( + sessionId, + applyPromptTemplateInsertMode(currentDraft, resolved, fill.insertMode), + ) + workspace.showPaneToast(sessionId, `Inserted template: ${fill.template.title}`) + onClose() +``` + +with (and make the callback body async — change `const insertFilledPromptTemplate = useCallback(() => {` to `useCallback(async () => {`): + +```ts + const result = await deliverTextToSession(workspace, sessionId, resolved, { insertMode: fill.insertMode }) + if (result.delivered) { + workspace.showPaneToast(sessionId, `Inserted template: ${fill.template.title}`) + onClose() + } else { + workspace.showPaneToast(sessionId, 'Template target pane is gone') + } +``` + +If callers invoke `insertFilledPromptTemplate` synchronously, wrap their call sites with `void insertFilledPromptTemplate()` — do not leave a floating promise. + +4. Check whether `applyPromptTemplateInsertMode` is still used in this file after both replacements; if not, remove it from the import (the delivery helper owns that logic now). Keep `fillPromptTemplateBody` — it still resolves variables. + +- [ ] **Step 4: Update targetSession tests** + +In `src/renderer/src/features/prompt-templates/targetSession.test.ts`, update/extend the cases: terminal-kind sessions are now VALID targets; keep any non-session rejection cases. Example additions: + +```ts + it('accepts terminal panes as targets (bracket-paste insertion)', () => { + expect(promptTemplateTargetSessionIdForState(stateWithFocusedTerminal())).toBe('t1') + }) +``` + +(match the file's existing fixture helpers; the behavioral flip is the point). + +- [ ] **Step 5: Run tests** + +```bash +npm run test:unit -- src/renderer/src/features/prompt-templates +npm run test:renderer -- src/renderer/src/features/command-palette +npm run check:keybindings +``` + +Expected: all PASS. If palette tests assert the old draft-only insertion, update them to expect `window.api.sendInput` for terminal targets / `setDraftInput` for composer targets. + +- [ ] **Step 6: Commit** + +```bash +git add src/renderer/src/features/prompt-templates src/renderer/src/features/command-palette +git commit -m "feat(session-text): route template insertion through the delivery helper + +Terminal panes and agent terminal views become valid template targets; +insertion bracket-pastes without submitting instead of silently doing +nothing. Composer behavior is unchanged (insert modes, no submit). + +Refs #830" +``` + +--- + +### Task 6: `{{key:Provider/Key}}` template references + +**Files:** +- Create: `src/renderer/src/features/prompt-templates/keyReferences.ts` +- Modify: `src/renderer/src/features/command-palette/ui/CommandPalette.tsx` (resolve refs in `executePromptTemplate`) +- Test: `src/renderer/src/features/prompt-templates/keyReferences.test.ts` + +- [ ] **Step 1: Write the failing tests** + +Create `src/renderer/src/features/prompt-templates/keyReferences.test.ts`: + +```ts +import { describe, expect, it } from 'vitest' + +import { collectKeyReferences, resolveKeyReferences } from '@renderer/features/prompt-templates/keyReferences' + +describe('collectKeyReferences', () => { + it('collects and dedupes references', () => { + const refs = collectKeyReferences('Use {{key:Brave/main}} and again {{ key:Brave/main }}, plus {{key:OpenAI/prod key}}') + expect(refs).toEqual([ + { providerName: 'Brave', keyName: 'main' }, + { providerName: 'OpenAI', keyName: 'prod key' }, + ]) + }) + + it('ignores ordinary template variables', () => { + expect(collectKeyReferences('{{name}} and {{ date }}')).toEqual([]) + }) +}) + +describe('resolveKeyReferences', () => { + it('substitutes resolved values', async () => { + const resolved = await resolveKeyReferences( + 'Brave key: {{key:Brave/main}}', + async ref => (ref.providerName === 'Brave' ? 'BSA-1' : null), + ) + expect(resolved).toBe('Brave key: BSA-1') + }) + + it('aborts loudly on an unresolved reference', async () => { + await expect( + resolveKeyReferences('{{key:Brave/nope}}', async () => null), + ).rejects.toThrow(/Unresolved key reference/) + }) +}) +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `npm run test:unit -- src/renderer/src/features/prompt-templates` +Expected: FAIL — module missing. + +- [ ] **Step 3: Implement** + +Create `src/renderer/src/features/prompt-templates/keyReferences.ts`: + +```ts +// API Key Vault references inside prompt template bodies (#831). +// +// Syntax: {{key:Provider Name/Key Name}} — matched case-sensitively +// against vault metadata at execution time. WHY names and not ids: +// templates are user-authored text that must stay readable and +// portable; uuids would make every template an opaque pile. Renaming a +// provider or key therefore breaks its references — resolution aborts +// loudly with a toast instead of silently inserting nothing. +// +// WHY this pattern is separate from the ordinary {{variable}} pattern: +// the placeholder grammar is [A-Za-z0-9_]+ only, so these refs never +// collide with or surface as form fields in the fill pane; they are +// resolved BEFORE variable fill. + +export type KeyReference = { providerName: string; keyName: string } + +const KEY_REF_PATTERN = /\{\{\s*key:([^/{}]+?)\/([^/{}]+?)\s*\}\}/g + +export function collectKeyReferences(body: string): KeyReference[] { + const seen = new Set() + const ordered: KeyReference[] = [] + for (const match of body.matchAll(KEY_REF_PATTERN)) { + const ref = { providerName: match[1].trim(), keyName: match[2].trim() } + const dedupeKey = `${ref.providerName}\u0000${ref.keyName}` + if (seen.has(dedupeKey)) continue + seen.add(dedupeKey) + ordered.push(ref) + } + return ordered +} + +export async function resolveKeyReferences( + body: string, + resolve: (ref: KeyReference) => Promise, +): Promise { + // Resolve every distinct reference out of band first (a synchronous + // String.replace callback cannot await, and each ref may cross the + // vault gate), collecting ALL failures so one error message tells the + // user everything that needs fixing. + const refs = collectKeyReferences(body) + const values = new Map() + const failures: string[] = [] + for (const ref of refs) { + const value = await resolve(ref) + if (value === null || value.length === 0) { + failures.push(`{{key:${ref.providerName}/${ref.keyName}}}`) + continue + } + values.set(`${ref.providerName}\u0000${ref.keyName}`, value) + } + if (failures.length > 0) { + throw new Error(`Unresolved key reference: ${failures.join(', ')}`) + } + return body.replace(KEY_REF_PATTERN, (_match, rawProvider: string, rawKey: string) => { + return values.get(`${rawProvider.trim()}\u0000${rawKey.trim()}`) ?? '' + }) +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `npm run test:unit -- src/renderer/src/features/prompt-templates` +Expected: PASS. + +- [ ] **Step 5: Integrate into template execution** + +In `src/renderer/src/features/command-palette/ui/CommandPalette.tsx`, inside `executePromptTemplate` immediately after the body is resolved (`const body = template.buildBody ? ... : template.body`) and before the variables check, add: + +```ts + // Vault key references (#831): resolve before variable fill so + // the fill pane never displays secret values, and abort with a + // toast on missing refs instead of pasting placeholder text. + const keyRefs = collectKeyReferences(body) + if (keyRefs.length > 0) { + const resolvedRefs = await resolveKeyReferences(body, async ref => { + try { + return await window.api.keyVaultResolveReference(ref.providerName, ref.keyName) + } catch { + // Locked/canceled/missing all mean "cannot resolve now" — + // the aggregate error below names the reference. + return null + } + }) + body = resolvedRefs + } +``` + +(If `body` is `const`, change its declaration to `let`.) Add the imports: + +```ts +import { collectKeyReferences, resolveKeyReferences } from '@renderer/features/prompt-templates/keyReferences' +``` + +- [ ] **Step 6: Commit** + +```bash +git add src/renderer/src/features/prompt-templates src/renderer/src/features/command-palette +git commit -m "feat(vault): resolve key references in prompt templates + +{{key:Provider/Key}} refs resolve against the gated vault before +variable fill; missing or locked refs abort insertion with a toast +naming every failure. Names over ids keep templates readable. + +Refs #831" +``` + +--- + +### Task 7: Vault UI (uiShell state, command, surface, modal) + +**Files:** +- Modify: `src/renderer/src/app-state/uiShell/types.ts` (+ `keyVaultOpen`) +- Modify: `src/renderer/src/app-state/uiShell/slice.ts` (+ default + `openKeyVault`/`closeKeyVault`) +- Modify: `src/renderer/src/features/command-palette/types.ts` (+ `openKeyVault` on the ui context type, next to `openUsageModal`) +- Modify: `src/renderer/src/features/command-palette/ui/CommandPalette.tsx` (selector + two ui object entries) +- Create: `src/renderer/src/features/key-vault/commands/keyVaultCommands.ts` +- Modify: `src/renderer/src/features/command-palette/catalog.ts` (import + spread) +- Create: `src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx` +- Create: `src/renderer/src/features/key-vault/surfaces/KeyVaultModalSurface.tsx` +- Modify: `src/renderer/src/app/surfaces/registry.tsx` (append modal entry) + +- [ ] **Step 1: uiShell state** + +In `src/renderer/src/app-state/uiShell/types.ts`, add to `UiShellState` (near `usageModalOpen`): + +```ts + /** When true, the API Key Vault modal is open (#831). Transient command + * chrome, not workspace data — same rationale as usageModalOpen. */ + keyVaultOpen: boolean +``` + +In `src/renderer/src/app-state/uiShell/slice.ts`, add the default next to `usageModalOpen: false,`: + +```ts + keyVaultOpen: false, +``` + +and the actions next to `openUsageModal`/`closeUsageModal` (mirror their exact shape): + +```ts + openKeyVault: () => + set({ keyVaultOpen: true }, false, 'uiShell/openKeyVault'), + closeKeyVault: () => + set({ keyVaultOpen: false }, false, 'uiShell/closeKeyVault'), +``` + +- [ ] **Step 2: Command palette ui plumbing** + +In `src/renderer/src/features/command-palette/types.ts`, next to `openUsageModal: () => void` (line ~210), add: + +```ts + openKeyVault: () => void +``` + +In `src/renderer/src/features/command-palette/ui/CommandPalette.tsx`: + +1. Next to `const openUsageModal = useAppStore(state => state.openUsageModal)` (~line 294): + +```ts + const openKeyVault = useAppStore(state => state.openKeyVault) +``` + +2. Add `openKeyVault,` to BOTH ui context object literals that contain `openUsageModal,` (~lines 630 and 738). + +- [ ] **Step 3: The command** + +Create `src/renderer/src/features/key-vault/commands/keyVaultCommands.ts`: + +```ts +import type { CommandDef } from '@renderer/features/command-palette/types' + +export const keyVaultCommands: CommandDef[] = [ + { + id: 'api-key-vault', + category: 'workspace-tools', + surface: 'app', + title: 'API Key Vault…', + description: + '**What it does:** Opens the **API Key Vault** — manage provider API keys, insert them into the focused pane, copy to clipboard, and reference them from prompt templates (`{{key:Provider/Key}}`).\n\n**Use when:** You regularly paste API keys (Brave, OpenAI, …) into agent prompts.\n\n**Notes:** Encrypted with the OS keyring; one Touch ID / password unlock per app launch.', + keywords: ['api', 'key', 'vault', 'secret', 'credential', 'token', 'password'], + run: ({ ui }) => { + ui.openKeyVault() + }, + }, +] +``` + +In `src/renderer/src/features/command-palette/catalog.ts`, add the import next to `promptTemplateCommands`: + +```ts +import { keyVaultCommands } from '@renderer/features/key-vault/commands/keyVaultCommands' +``` + +and spread it into the command list next to `...promptTemplateCommands,`: + +```ts + ...keyVaultCommands, +``` + +- [ ] **Step 4: The modal** + +Create `src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx`: + +```tsx +import { useCallback, useEffect, useState } from 'react' + +import { useAppStore } from '@renderer/app-state/hooks' +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' +import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/commandTargetSessionId' +import { useWorkspace } from '@renderer/workspace/workspaceStore' +import type { KeyVaultKey, KeyVaultStatus } from '@shared/types/keyVault' + +// API Key Vault modal (#831). Revealed plaintext lives ONLY in this +// component's state — never in the persisted app store, never in a +// journal — and is dropped on close. Metadata comes and goes through +// window.api.keyVault* calls; every secret fetch crosses the main-side +// unlock gate (one OS prompt per app run). + +type KeyForm = { id?: string; name: string; value: string; note: string } | null + +export function KeyVaultModal() { + const closeKeyVault = useAppStore(state => state.closeKeyVault) + const workspace = useWorkspace() + const [status, setStatus] = useState(null) + const [providers, setProviders] = useState<{ id: string; name: string }[]>([]) + const [keys, setKeys] = useState([]) + const [selectedProviderId, setSelectedProviderId] = useState(null) + // keyId -> revealed plaintext. Ephemeral by design. + const [revealed, setRevealed] = useState>(new Map()) + const [newProviderName, setNewProviderName] = useState('') + const [keyForm, setKeyForm] = useState(null) + const [error, setError] = useState(null) + + const refresh = useCallback(async () => { + try { + const [nextStatus, snapshot] = await Promise.all([ + window.api.keyVaultStatus(), + window.api.keyVaultList(), + ]) + setStatus(nextStatus) + setProviders(snapshot.providers) + setKeys(snapshot.keys) + setSelectedProviderId(current => { + if (current && snapshot.providers.some(p => p.id === current)) return current + return snapshot.providers[0]?.id ?? null + }) + setError(null) + } catch (err) { + setError(err instanceof Error ? err.message : String(err)) + } + }, []) + + useEffect(() => { void refresh() }, [refresh]) + useEffect(() => { + // Drop all plaintext the moment the modal closes. + return () => setRevealed(new Map()) + }, []) + + const runVaultAction = async (action: () => Promise) => { + try { + await action() + await refresh() + setError(null) + } catch (err) { + // Canceled OS prompt, fail-closed gate, duplicate name, … — the + // service messages are written for direct display. + setError(err instanceof Error ? err.message : String(err)) + } + } + + const addProvider = () => { + const name = newProviderName.trim() + if (!name) return + setNewProviderName('') + void runVaultAction(async () => { + const provider = await window.api.keyVaultCreateProvider(name) + setSelectedProviderId((await window.api.keyVaultList()).providers.find(p => p.name === name)?.id ?? provider.id) + }) + } + + const saveKeyForm = () => { + const form = keyForm + if (!form || !selectedProviderId) return + setKeyForm(null) + void runVaultAction(() => window.api.keyVaultPutKey({ + providerId: selectedProviderId, + id: form.id, + name: form.name, + value: form.value, + note: form.note, + })) + } + + const toggleReveal = async (key: KeyVaultKey) => { + if (revealed.has(key.id)) { + const next = new Map(revealed) + next.delete(key.id) + setRevealed(next) + return + } + try { + const value = await window.api.keyVaultReveal(key.providerId, key.id) + setRevealed(prev => new Map(prev).set(key.id, value)) + setError(null) + } catch (err) { + setError(err instanceof Error ? err.message : String(err)) + } + } + + const insertKey = async (key: KeyVaultKey) => { + const sessionId = commandTargetSessionId(workspace) + if (!sessionId) { + setError('No focused pane to insert into') + return + } + try { + const value = revealed.get(key.id) ?? await window.api.keyVaultReveal(key.providerId, key.id) + const result = await deliverTextToSession(workspace, sessionId, value) + if (result.delivered) { + workspace.showPaneToast(sessionId, `Inserted key: ${key.name}`) + closeKeyVault() + } else { + setError('Focused pane is no longer available') + } + } catch (err) { + setError(err instanceof Error ? err.message : String(err)) + } + } + + const selectedKeys = keys.filter(k => k.providerId === selectedProviderId) + const selectedProvider = providers.find(p => p.id === selectedProviderId) ?? null + + return ( +
{ if (e.target === e.currentTarget) closeKeyVault() }} + > +
+
+
+ API Key Vault + {status && ( + + {status.unlocked ? 'unlocked' : 'locked'} + + )} +
+
+ {status?.unlocked && ( + + )} + +
+
+ + {status && !status.encryptionAvailable && ( +
+ OS keyring (safeStorage) is unavailable on this machine — keys cannot be stored. +
+ )} + {error && ( +
{error}
+ )} + +
+
+ {providers.map(provider => ( + + ))} +
+ setNewProviderName(e.target.value)} + onKeyDown={e => { if (e.key === 'Enter') addProvider() }} + /> +
+
+ +
+ {!selectedProvider &&
Create a provider to get started.
} + {selectedProvider && ( + <> +
+ {selectedProvider.name} +
+ + + +
+
+ + {selectedKeys.map(key => ( +
+
+ {key.name} + ••••{key.hint} + {revealed.has(key.id) && ( + + {revealed.get(key.id)} + + )} + + + + + + +
+ {key.note &&
{key.note}
} +
+ ))} + + {keyForm && ( +
+
{keyForm.id ? `Edit key` : 'New key'}
+ setKeyForm({ ...keyForm, name: e.target.value })} + /> + setKeyForm({ ...keyForm, value: e.target.value })} + /> + setKeyForm({ ...keyForm, note: e.target.value })} + /> +
+ + +
+
+ )} + + )} +
+
+ +
+ Reference keys from prompt templates with {'{{key:Provider/Key}}'} · Encrypted with the OS keyring · One unlock per app launch +
+
+
+ ) +} +``` + +Note: match the styling vocabulary of neighboring modals (check `UsageModal.tsx` / `BuryPanePrompt.tsx`) and adjust class names to whatever the codebase's current primitives are — the structure and behaviors above are the contract. + +- [ ] **Step 5: The surface wrapper + registry** + +Create `src/renderer/src/features/key-vault/surfaces/KeyVaultModalSurface.tsx`: + +```tsx +import { useAppStore } from '@renderer/app-state/hooks' +import { KeyVaultModal } from '@renderer/features/key-vault/ui/KeyVaultModal' + +export function KeyVaultModalSurface() { + const open = useAppStore(state => state.keyVaultOpen) + if (!open) return null + return +} +``` + +In `src/renderer/src/app/surfaces/registry.tsx`, add the import: + +```ts +import { KeyVaultModalSurface } from '@renderer/features/key-vault/surfaces/KeyVaultModalSurface' +``` + +and append at the END of `modalSurfaces` (per the registry contract — new modals append so sibling order cannot move an established surface): + +```ts + { id: 'key-vault', Component: KeyVaultModalSurface }, +``` + +- [ ] **Step 6: Verify** + +```bash +npm run typecheck +npm run test:renderer -- src/renderer/src/features/command-palette +npm run check:keybindings +``` + +Expected: PASS (new command may extend the keybinding baseline; if `check:keybindings` reports the new command, follow its printed remediation). + +- [ ] **Step 7: Commit** + +```bash +git add src/renderer/src/app-state src/renderer/src/features/key-vault src/renderer/src/features/command-palette src/renderer/src/app/surfaces +git commit -m "feat(vault): add api key vault command and modal + +App-surface palette command opens the vault modal: provider/key CRUD, +reveal/copy/insert actions, lock-now, and an explicit unavailable-keyring +notice. Revealed plaintext stays in ephemeral component state only. + +Refs #831" +``` + +--- + +### Task 8: Final verification pass + +**Files:** none (verification only). + +- [ ] **Step 1: Full check suite** + +```bash +npm run typecheck +npm test +npm run check:keybindings +npm run test:contract +``` + +Expected: all green. Fix anything that surfaces; each fix is its own commit (`fix(vault): …` / `fix(session-text): …`). + +- [ ] **Step 2: Manual smoke test** + +```bash +npm run dev +``` + +1. Cmd+P → “API Key Vault” → create provider “Brave” → add key “main” → confirm the Touch ID / password prompt appears on first Reveal. +2. Focus a Claude pane → Insert → key lands in the composer draft unsubmitted. +3. Open a plain terminal pane → Insert → key lands as one bracketed paste, no execution. +4. Create a custom template `My Brave key is {{key:Brave/main}}` → run Prompt Template in the terminal pane → resolved key pastes; a bad ref toasts the failure. +5. Cancel the OS prompt once → confirm nothing is revealed and the error line explains it. + +- [ ] **Step 3: Report state** + +Per conventions: report final state, leave branch clean, open the PR fully built out (implementation + tests + spec), link `Refs #830` / `Refs #831`, and wait for explicit confirmation before any merge. diff --git a/docs/superpowers/specs/2026-09-07-api-key-vault-design.md b/docs/superpowers/specs/2026-09-07-api-key-vault-design.md new file mode 100644 index 00000000..4c23946d --- /dev/null +++ b/docs/superpowers/specs/2026-09-07-api-key-vault-design.md @@ -0,0 +1,56 @@ +# API Key Vault + Session Text Delivery — Design + +**Status:** approved design, 2026-09-07 +**Issues:** #830 (session text delivery), #831 (API key vault) +**Plan:** `docs/superpowers/plans/2026-09-07-api-key-vault.md` + +## Problem + +1. Models routinely need third-party API keys (Brave Search, OpenAI, …). Users paste them straight into prompts because they are low-stakes, but every use means a round-trip to the provider dashboard to copy the key again. +2. Prompt templates only target agent composers (`targetSession.ts` returns null for terminal-kind sessions), so templates — and any future key-insertion feature — are useless in plain terminal panes where users run unsupported agent harnesses, and in agent panes switched to terminal view. + +## Requirements (confirmed with user) + +- Vault: user-created providers (empty start, no seed list), each holding keys with name + secret + note. +- Encryption at rest via the existing safeStorage discipline (`src/main/dictation/apiKeyStore.ts`). +- Unlock gate: Touch ID / Mac login-password prompt once per app launch; "Lock now" re-arms; cancel/unavailable fails closed. +- Consumption: insert into focused pane (composer or any PTY), copy to clipboard, and `{{key:Provider/Key}}` references in prompt templates. +- Terminal delivery is always paste-without-submit (review then Enter), mirroring the templates' "prefill, don't replay" contract. + +## Decisions + +### D1 — Per-key safeStorage blobs + plaintext metadata index (chosen over whole-vault blob / native Keychain items) + +`STATE_DIR/key-vault/index.json` carries providers, key names, notes, timestamps, and last-4 hints (non-secret). Each secret lives in `keys/.bin` as its own safeStorage ciphertext. Rationale identical to `apiKeyStore.ts:27-35`: a corrupt blob after a Keychain reset costs exactly one key, never the vault; the provider list renders without touching decryption; edits rewrite one blob, not a monolith. Whole-vault encryption hides metadata but bricks everything on one decrypt failure. Native Keychain items per key were rejected: ACL prompts fire unpredictably and conflict with the once-per-launch gate. + +### D2 — Unlock gate in main via `systemPreferences.promptTouchID`, once per run + +Every secret-leaving path (reveal, copy, template ref resolution) funnels through one `ensureUnlocked()`. Electron 43.1.0's `promptTouchID` uses `SecAccessControlUserPresence`; unlike `canPromptTouchID`, it is not a biometric-only capability check. Attempt the supported macOS API and fail closed on rejection. The OS authentication UI still needs manual verification on signed/unsigned builds and password-only Macs. Concurrent requests share one prompt. Lock invalidates pending auth and disk reads and broadcasts cache invalidation to all windows. Metadata CRUD/list IPC is gated too. + +The vault encrypts stored key values, not the prompt pipeline. Inserted keys follow normal draft autosave into plaintext `workspace.json`, terminal scrollback, and provider transcript retention after submission; copying puts the value on the system clipboard. The modal explicitly discloses this boundary. Names and notes are plaintext metadata; short values do not get a hint that would expose the entire secret. No export, transparent secret-token draft system, or general transcript redaction is promised. + +### D3 — Renderer-owned `deliverTextToSession` (chosen over main-owned paste IPC) + +One helper dispatches by normalized session kind and effective surface. Rendered panes edit the existing draft; bare key insertion concatenates text, while templates retain their append/replace policy. Terminal insertion uses the mounted leaf's paste target, respecting visibility, attach/replay state and xterm's current bracketed-paste mode. Single-line text works without bracketed-paste support; multiline text is refused unless the program has enabled that mode. Embedded control sequences are refused. No Enter is added, and rejected writes keep the picker open rather than silently retrying into a replacement backend. Wake never follows a disposed or changed terminal target. + +### D4 — Template key references by name, resolved at insertion + +`{{key:Provider Name/Key Name}}` is disjoint from ordinary `{{variable}}` fields. The fill pane retains the dynamic or saved body with unresolved key references, then `prepareTemplateText` fills ordinary variables and resolves keys only at final insertion. Cancellation or a missing key aborts insertion. Capture the original session, reject changes during asynchronous preparation, and never deliver after closing the palette. Provider/key names exclude syntax delimiters. Renaming breaks references loudly. The external control template API remains a draft-only API and does not expose the vault. + +## Rejected / out of scope + +- Auto-submit after terminal paste (user review required before execution). +- Remote phone client vault access; MCP/agent access to keys; export/import or cross-device migration (safeStorage blobs are device-scoped by design). +- Seeded provider lists (go stale; user creates what they need). + +## Testing + +- Unit: vault store (file discipline, corrupt-blob isolation), service (gate semantics, CRUD rules, fail-closed), key-reference collect/resolve/abort. +- Renderer: delivery helper dispatch matrix with stubbed `window.api.sendInput`. +- Manual smoke: Touch ID cancel path, insert into composer vs terminal, template ref resolution in a raw terminal. + +## Risks + +- `promptTouchID` behavior on unsigned builds: password fallback expected to work; if a platform ever cannot prompt, the vault degrades to unusable-but-safe (fail closed), surfaced in the modal status line. +- Programs without bracketed-paste support cannot safely receive multiline templates; the helper refuses rather than risking shell execution. +- Renaming vault providers/keys breaks template references by design; failures are loud, never silent. diff --git a/src/main/index.ts b/src/main/index.ts index 24cdbf5d..b2e97955 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -8,7 +8,7 @@ import { createExternalControlSettings } from './settings/externalControl' import { createExternalCodexIntegration } from './settings/externalCodexIntegration' import operatorSkillSource from '../../operator-skills/agent-code-computer-execution/SKILL.md?raw' -import { app, clipboard, crashReporter, dialog, Menu } from 'electron' +import { app, clipboard, crashReporter, dialog, Menu, systemPreferences } from 'electron' import { existsSync } from 'fs' import { readFile } from 'fs/promises' import { join } from 'path' @@ -95,6 +95,9 @@ import { AgentManagementBridge } from '@main/agentManagement/AgentManagementBrid import { AiWorkspaceRegistry } from '@main/aiWorkspace/AiWorkspaceRegistry.js' import { RemoteController } from '@main/remote/RemoteController.js' import { CaffeinateController } from '@main/caffeinate/CaffeinateController.js' +import { createFileVaultStore } from '@main/keyVault/vaultStore.js' +import { createSafeStorageCodec } from '@main/keyVault/safeStorageCodec.js' +import { VaultService } from '@main/keyVault/VaultService.js' import { buildAppMenu } from '@main/menu/appMenu.js' import { AppRunJournal } from '@main/incident/AppRunJournal.js' import { installProcessCrashHooks } from '@main/incident/installCrashHooks.js' @@ -194,6 +197,22 @@ const aiWorkspaceRegistry = new AiWorkspaceRegistry() aiWorkspaceRegistry.on('changed', event => broadcastToWindows('ai-workspace:changed', event)) const caffeinateController = new CaffeinateController() +// API Key Vault (#831). promptTouchID presents Touch ID with the user's +// login password as fallback, which is the "mac password" gate. Unsigned +// dev builds may skip the biometric option but the password path still +// works; if neither is available the service fails closed on unlock. +// Constructed top-level (like caffeinate) because it owns no window and +// no async boot step — only the per-run unlock boolean. +const vaultService = new VaultService({ + store: createFileVaultStore(join(STATE_DIR, 'key-vault'), createSafeStorageCodec()), + promptAuth: reason => systemPreferences.promptTouchID(reason), + // canPromptTouchID checks biometrics, not user-presence/password auth. + // Electron 43's promptTouchID uses SecAccessControlUserPresence; attempt + // that supported macOS API and let rejection keep the vault locked. + canPromptAuth: () => process.platform === 'darwin' && typeof systemPreferences.promptTouchID === 'function', + copyToClipboard: text => clipboard.writeText(text), +}) + // SessionManager is constructed inside whenReady so we can await // TmuxRegistry.detectAvailability() first — terminal sessions need // to know during spawn whether a tmux backend is available, and @@ -944,6 +963,7 @@ async function startApp(): Promise { agentManagementBridge, aiWorkspaceRegistry, caffeinateController, + vaultService, appRunJournal, cliUpdateOrchestrator, workflowBridge: activeWorkflowBridge, diff --git a/src/main/ipc/index.ts b/src/main/ipc/index.ts index 600e5b49..629f3b4c 100644 --- a/src/main/ipc/index.ts +++ b/src/main/ipc/index.ts @@ -33,6 +33,8 @@ import { registerAgentManagementIpc } from '@main/ipc/agentManagement.js' import { registerAiWorkspaceIpc } from '@main/ipc/aiWorkspace.js' import { registerRenderedContentIpc } from '@main/ipc/renderedContent.js' import { registerCaffeinateIpc } from '@main/ipc/caffeinate.js' +import { registerKeyVaultIpc } from '@main/ipc/keyVault.js' +import type { VaultService } from '@main/keyVault/VaultService.js' import { registerRemoteIpc } from '@main/ipc/remote.js' import type { OrchestrationBridge } from '@main/orchestration/OrchestrationBridge.js' import type { AgentManagementBridge } from '@main/agentManagement/AgentManagementBridge.js' @@ -75,6 +77,7 @@ export type IpcDeps = { agentManagementBridge: AgentManagementBridge aiWorkspaceRegistry: AiWorkspaceRegistry caffeinateController: CaffeinateController + vaultService: VaultService remoteController: RemoteController appRunJournal: AppRunJournal cliUpdateOrchestrator: CliUpdateOrchestrator @@ -111,6 +114,7 @@ export function registerAllIpc(deps: IpcDeps): void { registerAiWorkspaceIpc(deps.aiWorkspaceRegistry) registerRenderedContentIpc() registerCaffeinateIpc(deps.caffeinateController) + registerKeyVaultIpc({ vaultService: deps.vaultService }) registerRemoteIpc(deps.remoteController) registerIncidentIpc(deps.appRunJournal) // Debug export needs the lifecycle limiter's monotonic completeness state. diff --git a/src/main/ipc/keyVault.ts b/src/main/ipc/keyVault.ts new file mode 100644 index 00000000..103ecf5f --- /dev/null +++ b/src/main/ipc/keyVault.ts @@ -0,0 +1,44 @@ +import { ipcMain } from 'electron' + +import type { VaultService } from '@main/keyVault/VaultService.js' +import type { KeyVaultKeyInput } from '@shared/types/keyVault' +import { broadcastToWindows } from '@main/window/windowRegistry.js' +import { z } from 'zod' + +// IPC validates wire shapes and gates metadata operations; the service owns +// secret-read authorization and file transactions. Clipboard copy stays in +// main, while reveal and template resolution return a gated secret to the UI. +export function registerKeyVaultIpc({ vaultService }: { vaultService: VaultService }): void { + vaultService.on('locked', () => broadcastToWindows('key-vault:locked', {})) + // Metadata mutations are protected too. TypeScript types cannot validate + // IPC input, and schema errors must not echo submitted secrets to diagnostics. + const text = z.string().max(65536) + const key = z.object({ providerId: text, id: text.optional(), name: text, value: text, note: text }).strict() + function parse(schema: z.ZodType, value: unknown): T { + const result = schema.safeParse(value) + if (!result.success) throw new Error('Invalid vault request.') + return result.data + } + async function unlocked(operation: () => Promise): Promise { + await vaultService.unlock() + if (!vaultService.getStatus().unlocked) throw new Error('Vault was locked.') + return operation() + } + ipcMain.handle('key-vault:status', () => vaultService.getStatus()) + ipcMain.handle('key-vault:list', () => unlocked(() => vaultService.list())) + ipcMain.handle('key-vault:unlock', () => vaultService.unlock()) + ipcMain.handle('key-vault:lock', () => vaultService.lock()) + ipcMain.handle('key-vault:reveal', (_event, providerId: string, keyId: string) => + vaultService.reveal(providerId, keyId)) + ipcMain.handle('key-vault:copy-key', (_event, providerId: string, keyId: string) => + vaultService.copyKey(providerId, keyId)) + ipcMain.handle('key-vault:resolve-ref', (_event, providerName: string, keyName: string) => + vaultService.resolveReference(providerName, keyName)) + ipcMain.handle('key-vault:create-provider', (_event, name: string) => unlocked(() => vaultService.createProvider(parse(text, name)))) + ipcMain.handle('key-vault:rename-provider', (_event, id: string, name: string) => + unlocked(() => vaultService.renameProvider(parse(text, id), parse(text, name)))) + ipcMain.handle('key-vault:delete-provider', (_event, id: string) => unlocked(() => vaultService.deleteProvider(parse(text, id)))) + ipcMain.handle('key-vault:put-key', (_event, input: KeyVaultKeyInput) => unlocked(() => vaultService.putKey(parse(key, input)))) + ipcMain.handle('key-vault:delete-key', (_event, providerId: string, keyId: string) => + unlocked(() => vaultService.deleteKey(parse(text, providerId), parse(text, keyId)))) +} diff --git a/src/main/keyVault/VaultService.test.ts b/src/main/keyVault/VaultService.test.ts new file mode 100644 index 00000000..432d2ec6 --- /dev/null +++ b/src/main/keyVault/VaultService.test.ts @@ -0,0 +1,258 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { VaultService, type VaultServiceDeps } from '@main/keyVault/VaultService.js' +import type { VaultStore } from '@main/keyVault/vaultStore.js' +import type { KeyVaultSnapshot } from '@shared/types/keyVault' + +function deferred() { + let resolve!: (value: T | PromiseLike) => void + const promise = new Promise(done => { resolve = done }) + return { promise, resolve } +} + +// In-memory store: the file layer has its own tests (vaultStore.test.ts); +// these tests pin the SERVICE rules — gate semantics, ordering, and +// fail-closed behavior. +function makeStore(): VaultStore { + const snapshot: KeyVaultSnapshot = { providers: [], keys: [] } + const secrets = new Map() + return { + encryptionAvailable: () => true, + loadIndex: async () => ({ providers: [...snapshot.providers], keys: [...snapshot.keys] }), + saveIndex: async next => { + snapshot.providers = [...next.providers] + snapshot.keys = [...next.keys] + }, + readSecret: async id => secrets.get(id) ?? null, + writeSecret: async (id, value) => void secrets.set(id, value), + deleteSecret: async id => void secrets.delete(id), + } +} + +function makeDeps(overrides: Partial = {}): VaultServiceDeps { + return { + store: makeStore(), + promptAuth: vi.fn(async () => {}), + canPromptAuth: () => true, + copyToClipboard: vi.fn(), + now: () => 1_000, + ...overrides, + } +} + +describe('VaultService unlock gate', () => { + it('prompts exactly once per run for repeated reveals', async () => { + const deps = makeDeps() + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'BSA-xyz', note: '' }) + + await service.reveal(provider.id, key.id) + await service.copyKey(provider.id, key.id) + await service.reveal(provider.id, key.id) + + expect(deps.promptAuth).toHaveBeenCalledTimes(1) + expect(service.getStatus().unlocked).toBe(true) + }) + + it('fails closed when the user cancels the OS prompt', async () => { + const deps = makeDeps({ promptAuth: vi.fn(async () => { throw new Error('user canceled') }) }) + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'BSA-xyz', note: '' }) + + await expect(service.reveal(provider.id, key.id)).rejects.toThrow('user canceled') + expect(service.getStatus().unlocked).toBe(false) + }) + + it('fails closed when the OS prompt itself rejects (no capability pre-filter)', async () => { + // WHY no canPromptAuth pre-gate (review finding): canPromptTouchID() + // reports biometrics only and locked out password-only Macs. The + // service must ATTEMPT the prompt and fail closed on rejection. + const deps = makeDeps({ + promptAuth: vi.fn(async () => { throw new Error('Could not authenticate') }), + canPromptAuth: () => false, + }) + const service = new VaultService(deps) + await expect(service.unlock()).rejects.toThrow('Could not authenticate') + expect(service.getStatus().unlocked).toBe(false) + // canPromptAuth stays metadata-only: it must not block the attempt. + expect(deps.promptAuth).toHaveBeenCalledTimes(1) + }) + + it('shares one OS prompt across concurrent reveals', async () => { + const deps = makeDeps() + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'v', note: '' }) + await Promise.all([ + service.reveal(provider.id, key.id), + service.reveal(provider.id, key.id), + service.reveal(provider.id, key.id), + ]) + expect(deps.promptAuth).toHaveBeenCalledTimes(1) + }) + + it('a lock issued during a pending prompt wins over its completion', async () => { + const prompt = deferred() + const deps = makeDeps({ + promptAuth: () => prompt.promise, + }) + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'v', note: '' }) + + const pending = service.reveal(provider.id, key.id) + const rejected = expect(pending).rejects.toThrow(/locked/i) + service.lock() // user hits "Lock now" while the prompt is on screen + prompt.resolve() + await rejected + expect(service.getStatus().unlocked).toBe(false) + }) + + it('does not release a secret if locked during the disk read', async () => { + const store = makeStore() + const deps = makeDeps({ store }) + const service = new VaultService(deps) + const provider = await service.createProvider('Brave') + const key = await service.putKey({ providerId: provider.id, name: 'main', value: 'secret', note: '' }) + const read = deferred() + const started = deferred() + store.readSecret = () => { started.resolve(); return read.promise } + const pending = service.copyKey(provider.id, key.id) + const rejected = expect(pending).rejects.toThrow(/locked/i) + await started.promise + service.lock() + read.resolve('secret') + await rejected + expect(deps.copyToClipboard).not.toHaveBeenCalled() + }) + + it('can retry unlocking after the keyring becomes available', async () => { + const store = makeStore() + const available = vi.fn(() => false) + store.encryptionAvailable = available + const service = new VaultService(makeDeps({ store })) + await expect(service.unlock()).rejects.toThrow(/keyring/i) + available.mockReturnValue(true) + await service.unlock() + expect(service.getStatus().unlocked).toBe(true) + }) + + it('serializes concurrent provider creations without losing entries', async () => { + const service = new VaultService(makeDeps()) + await Promise.all([ + service.createProvider('Brave'), + service.createProvider('OpenAI'), + service.createProvider('Anthropic'), + ]) + const names = (await service.list()).providers.map(p => p.name).sort() + expect(names).toEqual(['Anthropic', 'Brave', 'OpenAI']) + }) + + it('omits hints for secrets too short to survive slicing', async () => { + const service = new VaultService(makeDeps()) + const provider = await service.createProvider('Brave') + const short = await service.putKey({ providerId: provider.id, name: 'tiny', value: 'abc', note: '' }) + const long = await service.putKey({ providerId: provider.id, name: 'real', value: 'BSA-abcdef1234', note: '' }) + expect(short.hint).toBe('') + expect(long.hint).toBe('1234') + }) + + it('fails closed when the OS keyring is unavailable', async () => { + const store = makeStore() + const service = new VaultService(makeDeps({ + store: Object.assign(store, { encryptionAvailable: () => false }), + })) + await expect(service.unlock()).rejects.toThrow(/keyring unavailable/i) + }) + + it('lock() re-arms the gate within the same run', async () => { + const deps = makeDeps() + const service = new VaultService(deps) + await service.unlock() + service.lock() + await service.unlock() + expect(deps.promptAuth).toHaveBeenCalledTimes(2) + }) +}) + +describe('VaultService CRUD', () => { + let service: VaultService + let providerId: string + + beforeEach(async () => { + service = new VaultService(makeDeps()) + const provider = await service.createProvider('Brave') + providerId = provider.id + }) + + it('round-trips providers and keys with hints', async () => { + const key = await service.putKey({ providerId, name: 'main', value: 'BSA-abcdef1234', note: 'prod' }) + expect(key.hint).toBe('1234') + const list = await service.list() + expect(list.providers.map(p => p.name)).toEqual(['Brave']) + expect(list.keys.map(k => k.name)).toEqual(['main']) + expect(await service.reveal(providerId, key.id)).toBe('BSA-abcdef1234') + }) + + it('updating with an empty value keeps the existing secret', async () => { + const key = await service.putKey({ providerId, name: 'main', value: 'BSA-one', note: '' }) + await service.putKey({ providerId, id: key.id, name: 'renamed', value: '', note: 'n' }) + const list = await service.list() + expect(list.keys[0].name).toBe('renamed') + // Hint unchanged: the stored secret is unchanged. + expect(list.keys[0].hint).toBe('-one') + expect(await service.reveal(providerId, key.id)).toBe('BSA-one') + }) + + it('rejects duplicate provider names case-insensitively', async () => { + await expect(service.createProvider('brave')).rejects.toThrow(/already exists/i) + }) + + it('rejects duplicate key names within a provider', async () => { + await service.putKey({ providerId, name: 'main', value: 'a', note: '' }) + await expect(service.putKey({ providerId, name: 'main', value: 'b', note: '' })).rejects.toThrow(/already exists/i) + }) + + it('cannot edit a key by pairing its id with a different provider', async () => { + const other = await service.createProvider('Other') + const key = await service.putKey({ providerId, name: 'main', value: 'original', note: '' }) + await expect(service.putKey({ providerId: other.id, id: key.id, name: 'main', value: 'changed', note: '' })) + .rejects.toThrow(/not found/i) + expect(await service.reveal(providerId, key.id)).toBe('original') + }) + + it('deleting a provider removes its keys from the index', async () => { + await service.putKey({ providerId, name: 'main', value: 'x', note: '' }) + await service.deleteProvider(providerId) + const list = await service.list() + expect(list.providers).toEqual([]) + expect(list.keys).toEqual([]) + }) + + it('resolveReference matches by provider/key name after gating', async () => { + await service.putKey({ providerId, name: 'main', value: 'BSA-ref', note: '' }) + await expect(service.resolveReference('Brave', 'main')).resolves.toBe('BSA-ref') + await expect(service.resolveReference('Brave', 'missing')).rejects.toThrow(/not found/i) + }) + + it('corrupt secret blob surfaces as a readable-key error', async () => { + // Simulate the Keychain-reset corruption: metadata present, secret + // unreadable. Contract: reveal names the key instead of returning + // null or throwing something opaque. The secret breaks AFTER + // creation via a wrapping store, so putKey still sees a real write. + const base = makeStore() + const breakingStore: typeof base = { + ...base, + readSecret: async id => (id === 'known-key' ? null : base.readSecret(id)), + } + const service2 = new VaultService(makeDeps({ store: breakingStore })) + const provider = await service2.createProvider('Brave') + const key = await service2.putKey({ providerId: provider.id, name: 'main', value: 'y', note: '' }) + // Put a marker id whose blob "rot" the store models as unreadable. + ;(breakingStore as unknown as { readSecret: (id: string) => Promise }).readSecret = + async (id: string) => (id === key.id ? null : base.readSecret(id)) + await expect(service2.reveal(provider.id, key.id)).rejects.toThrow(/cannot be decrypted/i) + }) +}) diff --git a/src/main/keyVault/VaultService.ts b/src/main/keyVault/VaultService.ts new file mode 100644 index 00000000..83447dcb --- /dev/null +++ b/src/main/keyVault/VaultService.ts @@ -0,0 +1,319 @@ +import { EventEmitter } from 'node:events' +import type { + KeyVaultKey, + KeyVaultKeyInput, + KeyVaultProvider, + KeyVaultSnapshot, + KeyVaultStatus, +} from '@shared/types/keyVault' +import { newVaultId, type VaultStore } from '@main/keyVault/vaultStore.js' + +// Vault orchestration (issue #831): CRUD over the store, the once-per-run +// unlock gate, and clipboard copies. +// +// WHY the gate lives here and not in the IPC layer: every secret-leaving +// path (reveal, copy, resolveReference) must pass through ONE gate. The +// gate is a plain boolean per app run — `promptAuth` is injected so unit +// tests never trigger a real OS dialog, and production wires it to +// systemPreferences.promptTouchID, which presents Touch ID with the +// login-password fallback (the "mac password" requirement). If the +// platform cannot prompt at all, we fail closed: no secret leaves main. +// +// WHY secrets never appear in the snapshot: the renderer list view is +// built entirely from non-secret metadata; plaintext crosses the bridge +// only as the return value of gated calls. Inserting it deliberately leaves +// vault protection: ordinary composer drafts and transcripts can persist it. + +export type VaultServiceDeps = { + store: VaultStore + /** Presents the OS auth prompt. Rejects when the user cancels. */ + promptAuth: (reason: string) => Promise + /** Whether the OS can present the prompt at all. */ + canPromptAuth: () => boolean + copyToClipboard: (text: string) => void + /** Injectable clock for deterministic timestamps. */ + now?: () => number +} + +/** WHY a hint policy function: slice(-4) on a 1-4 character value stores + * the ENTIRE secret in the plaintext metadata index (review finding). + * Short values get no hint at all; the identity convenience of a hint is + * not worth leaking the whole key through the ungated list(). */ +function hintFor(value: string): string { + return value.length >= 5 ? value.slice(-4) : '' +} + +function validateName(name: string): string { + const trimmed = name.trim() + // These delimiters belong to {{key:Provider/Key}}, not provider/key names. + // Rejecting them on creation keeps every saved key referenceable. + if (!trimmed || trimmed.length > 200 || /[\/{}\x00-\x1f\x7f]/.test(trimmed)) { + throw new Error('Use a name of 1-200 characters without /, braces or control characters.') + } + return trimmed +} + +export class VaultService extends EventEmitter { + private unlocked = false + // WHY single-flight + generation (review finding): without it, two + // concurrent reveals on a locked vault opened TWO OS prompts, and a + // prompt that was already in flight when lock() ran would complete + // afterwards and silently re-unlock the vault. The pending promise + // dedupes the prompts; the generation token makes a lock() that lands + // mid-prompt win over the prompt's completion. + private pendingUnlock: Promise | null = null + private lockGeneration = 0 + // WHY a mutation queue: every CRUD method is a load-modify-write of the + // whole index. Two concurrent createProvider calls interleaved and one + // entry silently vanished (reproduced in review). Serializing the + // transactions through one promise chain is the smallest correct fix; + // the vault is human-edit-frequency, so there is no throughput concern. + private mutationQueue: Promise = Promise.resolve() + + constructor(private readonly deps: VaultServiceDeps) { super() } + + getStatus(): KeyVaultStatus { + return { + encryptionAvailable: this.deps.store.encryptionAvailable(), + authPromptAvailable: this.deps.canPromptAuth(), + unlocked: this.unlocked, + } + } + + lock(): void { + this.unlocked = false + // Invalidate any prompt still on screen: its completion must not + // resurrect the unlocked state the user just revoked. + this.lockGeneration += 1 + this.emit('locked') + } + + async unlock(): Promise { + await this.ensureUnlocked() + } + + private ensureUnlocked(): Promise { + if (this.unlocked) return Promise.resolve() + if (this.pendingUnlock) return this.pendingUnlock + const generation = this.lockGeneration + const pending = Promise.resolve().then(async () => { + if (!this.deps.store.encryptionAvailable()) { + throw new Error('System keyring unavailable — the vault cannot read or store keys on this machine.') + } + // WHY attempt instead of pre-gating on canPromptAuth (review + // finding): Electron's canPromptTouchID() reports BIOMETRIC + // capability only. Pre-gating locked out every password-only Mac + // (mini/Studio/Pro, clamshell laptops) from the login-password + // path the feature promises. promptTouchID itself presents the + // password fallback; a platform that truly cannot prompt rejects + // and we fail closed right here. + await this.deps.promptAuth('unlock the Agent Code API key vault') + if (generation !== this.lockGeneration) throw new Error('Vault was locked during authentication.') + this.unlocked = true + }).finally(() => { + if (this.pendingUnlock === pending) this.pendingUnlock = null + }) + this.pendingUnlock = pending + return pending + } + + private assertUnlocked(generation: number): void { + // Lock must fence the result, not just the start of an async operation. + // Otherwise a disk read or an OS prompt can return plaintext after revocation. + if (!this.unlocked || generation !== this.lockGeneration) { + throw new Error('Vault was locked. Unlock it and try again.') + } + } + + /** Serialize an index read-modify-write transaction (see mutationQueue). */ + private enqueueMutation(operation: () => Promise): Promise { + const next = this.mutationQueue.then(operation, operation) + // Keep the chain alive even if a transaction rejects: one failed CRUD + // call must not poison every later one. + this.mutationQueue = next.catch(() => {}) + return next + } + + async list(): Promise { + return this.deps.store.loadIndex() + } + + async reveal(providerId: string, keyId: string): Promise { + const generation = this.lockGeneration + await this.ensureUnlocked() + const key = await this.findKey(providerId, keyId) + const secret = await this.deps.store.readSecret(keyId) + this.assertUnlocked(generation) + if (secret === null) { + throw new Error( + `Key "${key.name}" cannot be decrypted (Keychain reset or corrupted blob). Re-enter the value to fix it.`, + ) + } + return secret + } + + async copyKey(providerId: string, keyId: string): Promise { + const generation = this.lockGeneration + const value = await this.reveal(providerId, keyId) + this.assertUnlocked(generation) + this.deps.copyToClipboard(value) + } + + /** Resolve a `{{key:Provider/Key}}` template reference by NAME (#831). + * Names, not ids, so user-authored templates stay readable; renaming + * breaks references loudly rather than silently. */ + async resolveReference(providerName: string, keyName: string): Promise { + const generation = this.lockGeneration + await this.ensureUnlocked() + const snapshot = await this.deps.store.loadIndex() + const provider = snapshot.providers.find(p => p.name === providerName) + if (!provider) throw new Error(`Key vault provider "${providerName}" not found.`) + const key = snapshot.keys.find(k => k.providerId === provider.id && k.name === keyName) + if (!key) throw new Error(`Key "${keyName}" not found for provider "${providerName}".`) + const secret = await this.deps.store.readSecret(key.id) + this.assertUnlocked(generation) + if (secret === null) { + throw new Error( + `Key "${key.name}" cannot be decrypted (Keychain reset or corrupted blob). Re-enter the value to fix it.`, + ) + } + return secret + } + + async createProvider(name: string): Promise { + return this.enqueueMutation(() => this.createProviderTransaction(name)) + } + + private async createProviderTransaction(name: string): Promise { + const trimmed = validateName(name) + const snapshot = await this.deps.store.loadIndex() + if (snapshot.providers.some(p => p.name.toLowerCase() === trimmed.toLowerCase())) { + throw new Error(`Provider "${trimmed}" already exists.`) + } + const now = this.deps.now?.() ?? Date.now() + const provider: KeyVaultProvider = { id: newVaultId(), name: trimmed, createdAt: now, updatedAt: now } + snapshot.providers.push(provider) + await this.deps.store.saveIndex(snapshot) + return provider + } + + async renameProvider(id: string, name: string): Promise { + return this.enqueueMutation(() => this.renameProviderTransaction(id, name)) + } + + private async renameProviderTransaction(id: string, name: string): Promise { + const trimmed = validateName(name) + const snapshot = await this.deps.store.loadIndex() + const provider = snapshot.providers.find(p => p.id === id) + if (!provider) throw new Error('Provider not found.') + if (snapshot.providers.some(p => p.id !== id && p.name.toLowerCase() === trimmed.toLowerCase())) { + throw new Error(`Provider "${trimmed}" already exists.`) + } + provider.name = trimmed + provider.updatedAt = this.deps.now?.() ?? Date.now() + await this.deps.store.saveIndex(snapshot) + } + + async deleteProvider(id: string): Promise { + return this.enqueueMutation(() => this.deleteProviderTransaction(id)) + } + + private async deleteProviderTransaction(id: string): Promise { + const snapshot = await this.deps.store.loadIndex() + if (!snapshot.providers.some(p => p.id === id)) throw new Error('Provider not found.') + // Index-first ordering: a crash mid-delete leaves an orphan blob + // (harmless, invisible) rather than an index entry whose secret is + // already gone (reads as corrupt). + await this.deps.store.saveIndex({ + providers: snapshot.providers.filter(p => p.id !== id), + keys: snapshot.keys.filter(k => k.providerId !== id), + }) + for (const key of snapshot.keys) { + if (key.providerId === id) await this.deps.store.deleteSecret(key.id) + } + } + + async putKey(input: KeyVaultKeyInput): Promise { + return this.enqueueMutation(() => this.putKeyTransaction(input)) + } + + private async putKeyTransaction(input: KeyVaultKeyInput): Promise { + const name = validateName(input.name) + if (input.note.length > 4000 || input.value.length > 65536 || /[\x00-\x1f\x7f]/.test(input.value)) { + throw new Error('Key values must be single-line text up to 64 KiB; notes may be up to 4000 characters.') + } + const snapshot = await this.deps.store.loadIndex() + if (!snapshot.providers.some(p => p.id === input.providerId)) { + throw new Error('Provider not found.') + } + const now = this.deps.now?.() ?? Date.now() + const existing = input.id ? snapshot.keys.find(k => k.id === input.id && k.providerId === input.providerId) : undefined + if (input.id && !existing) throw new Error('Key not found.') + if ( + snapshot.keys.some( + k => k.id !== existing?.id && k.providerId === input.providerId && k.name === name, + ) + ) { + throw new Error(`Key "${name}" already exists for this provider.`) + } + + const value = input.value.trim() + if (existing) { + let hint = existing.hint + if (value.length > 0) { + // Secret-first ordering on update: write the new blob BEFORE the + // index references it, so a crash never produces an index entry + // with a missing/stale blob. + await this.deps.store.writeSecret(existing.id, value) + hint = hintFor(value) + } else if ((await this.deps.store.readSecret(existing.id)) === null) { + // Editing metadata cannot resurrect an unreadable secret; the + // user must re-enter the value. Surface that now, not at reveal. + throw new Error(`Key "${existing.name}" has no readable stored value — re-enter the value.`) + } + existing.name = name + existing.note = input.note.trim() + existing.hint = hint + existing.updatedAt = now + await this.deps.store.saveIndex(snapshot) + return existing + } + + if (!value) throw new Error('Key value cannot be empty.') + const key: KeyVaultKey = { + id: newVaultId(), + providerId: input.providerId, + name, + note: input.note.trim(), + hint: hintFor(value), + createdAt: now, + updatedAt: now, + } + await this.deps.store.writeSecret(key.id, value) + snapshot.keys.push(key) + await this.deps.store.saveIndex(snapshot) + return key + } + + async deleteKey(providerId: string, keyId: string): Promise { + return this.enqueueMutation(() => this.deleteKeyTransaction(providerId, keyId)) + } + + private async deleteKeyTransaction(providerId: string, keyId: string): Promise { + const snapshot = await this.deps.store.loadIndex() + const key = snapshot.keys.find(k => k.id === keyId && k.providerId === providerId) + if (!key) throw new Error('Key not found.') + await this.deps.store.saveIndex({ + providers: snapshot.providers, + keys: snapshot.keys.filter(k => k.id !== keyId), + }) + await this.deps.store.deleteSecret(keyId) + } + + private async findKey(providerId: string, keyId: string): Promise { + const snapshot = await this.deps.store.loadIndex() + const key = snapshot.keys.find(k => k.id === keyId && k.providerId === providerId) + if (!key) throw new Error('Key not found.') + return key + } +} diff --git a/src/main/keyVault/safeStorageCodec.ts b/src/main/keyVault/safeStorageCodec.ts new file mode 100644 index 00000000..1e3bea53 --- /dev/null +++ b/src/main/keyVault/safeStorageCodec.ts @@ -0,0 +1,23 @@ +import { safeStorage } from 'electron' + +import type { SecretCodec } from '@main/keyVault/vaultStore.js' + +// Production codec: thin adapter over Electron safeStorage, which derives +// its key from the OS Keychain (macOS) without prompting. Kept in its own +// module — separate from vaultStore.ts — purely so the store's unit tests +// never need the `electron` module, which only exists inside the packaged +// app. isEncryptionAvailable is consulted before every read so a Keychain +// that disappears mid-run reads as "unreadable key" instead of a crash. +export function createSafeStorageCodec(): SecretCodec { + return { + isEncryptionAvailable: () => { + try { + return safeStorage.isEncryptionAvailable() + } catch { + return false + } + }, + encrypt: plain => safeStorage.encryptString(plain), + decrypt: cipher => safeStorage.decryptString(cipher), + } +} diff --git a/src/main/keyVault/vaultStore.test.ts b/src/main/keyVault/vaultStore.test.ts new file mode 100644 index 00000000..61f63501 --- /dev/null +++ b/src/main/keyVault/vaultStore.test.ts @@ -0,0 +1,125 @@ +import { mkdtemp, readFile, stat, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { beforeEach, describe, expect, it } from 'vitest' + +import { createFileVaultStore, type SecretCodec } from '@main/keyVault/vaultStore.js' + +// Deterministic fake codec: real safeStorage cannot run under vitest +// (the electron module is the packaged app), and the store's job here is +// file discipline, not cryptography — safeStorage itself is exercised by +// the packaged app and by Electron upstream. +const fakeCodec: SecretCodec = { + isEncryptionAvailable: () => true, + encrypt: plain => Buffer.from(`enc:${plain}`, 'utf8'), + decrypt: cipher => { + // Real safeStorage throws on bytes it cannot decrypt; the fake must + // too, or the corrupt-blob test would "decrypt" garbage into text. + const text = cipher.toString('utf8') + if (!text.startsWith('enc:')) throw new Error('invalid ciphertext') + return text.slice(4) + }, +} + +let root: string + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'agent-code-vault-')) +}) + +describe('vaultStore', () => { + it('round-trips index edits and secrets', async () => { + const store = createFileVaultStore(root, fakeCodec) + const snapshot = await store.loadIndex() + const provider = { id: 'p1', name: 'Brave', createdAt: 1, updatedAt: 1 } + snapshot.providers.push(provider) + snapshot.keys.push({ + id: 'k1', providerId: 'p1', name: 'main', note: '', hint: '_key', + createdAt: 1, updatedAt: 1, + }) + await store.saveIndex(snapshot) + await store.writeSecret('k1', 'BSA-secret-value') + + const reloaded = createFileVaultStore(root, fakeCodec) + expect((await reloaded.loadIndex()).providers).toEqual([provider]) + expect(await reloaded.readSecret('k1')).toBe('BSA-secret-value') + }) + + it('starts with an empty snapshot on a fresh directory', async () => { + const store = createFileVaultStore(root, fakeCodec) + const snapshot = await store.loadIndex() + expect(snapshot.providers).toEqual([]) + expect(snapshot.keys).toEqual([]) + }) + + it('refuses to treat a corrupt or future index as an empty writable vault', async () => { + const store = createFileVaultStore(root, fakeCodec) + for (const content of ['{broken', '{"version":2,"providers":[],"keys":[]}', '{"version":1,"providers":{}}']) { + await writeFile(join(root, 'index.json'), content) + await expect(store.loadIndex()).rejects.toThrow(/index/i) + expect(await readFile(join(root, 'index.json'), 'utf8')).toBe(content) + } + }) + + it('rejects path traversal instead of reading or writing outside the vault', async () => { + const store = createFileVaultStore(root, fakeCodec) + await expect(store.writeSecret('../outside', 'value')).rejects.toThrow(/id/i) + await expect(store.readSecret('../outside')).rejects.toThrow(/id/i) + await expect(store.deleteSecret('../outside')).rejects.toThrow(/id/i) + }) + + it('refuses encryption writes when the system keyring is unavailable', async () => { + const store = createFileVaultStore(root, { ...fakeCodec, isEncryptionAvailable: () => false }) + await expect(store.writeSecret('key', 'secret')).rejects.toThrow(/keyring/i) + }) + + it('isolates a corrupt blob to a single key', async () => { + const store = createFileVaultStore(root, fakeCodec) + const snapshot = await store.loadIndex() + snapshot.providers.push({ id: 'p1', name: 'Brave', createdAt: 1, updatedAt: 1 }) + snapshot.keys.push( + { id: 'good', providerId: 'p1', name: 'a', note: '', hint: 'aaaa', createdAt: 1, updatedAt: 1 }, + { id: 'bad', providerId: 'p1', name: 'b', note: '', hint: 'bbbb', createdAt: 1, updatedAt: 1 }, + ) + await store.saveIndex(snapshot) + await store.writeSecret('good', 'one') + await store.writeSecret('bad', 'two') + // Corrupt exactly one blob on disk — the Keychain-reset scenario from + // apiKeyStore.ts: a decrypt failure must cost one key, not the vault. + await writeFile(join(root, 'keys', 'bad.bin'), Buffer.from('garbage')) + + expect(await store.readSecret('good')).toBe('one') + expect(await store.readSecret('bad')).toBeNull() + }) + + it('reports encryption availability through the codec', async () => { + const unavailable = createFileVaultStore(root, { ...fakeCodec, isEncryptionAvailable: () => false }) + expect(unavailable.encryptionAvailable()).toBe(false) + }) + + it('deletes secret blobs without touching the index', async () => { + const store = createFileVaultStore(root, fakeCodec) + await store.writeSecret('k1', 'v') + await store.deleteSecret('k1') + expect(await store.readSecret('k1')).toBeNull() + // Index file still parses after secret deletion. + await expect(store.loadIndex()).resolves.toBeTruthy() + }) + + it('writes secret blobs with 0600 permissions', async () => { + const store = createFileVaultStore(root, fakeCodec) + await store.writeSecret('k1', 'v') + const stats = await stat(join(root, 'keys', 'k1.bin')) + // WHY the explicit mask (review finding): this test used to assert + // only file length — the 0600 discipline was never actually pinned. + expect(stats.mode & 0o777).toBe(0o600) + // A replacement write through the same path must keep the mode. + await store.writeSecret('k1', 'longer-value') + const rewritten = await stat(join(root, 'keys', 'k1.bin')) + expect(rewritten.mode & 0o777).toBe(0o600) + // The index gets the same treatment. + await store.saveIndex(await store.loadIndex()) + const index = await stat(join(root, 'index.json')) + expect(index.mode & 0o777).toBe(0o600) + }) +}) diff --git a/src/main/keyVault/vaultStore.ts b/src/main/keyVault/vaultStore.ts new file mode 100644 index 00000000..32450691 --- /dev/null +++ b/src/main/keyVault/vaultStore.ts @@ -0,0 +1,158 @@ +import { randomUUID } from 'node:crypto' +import { chmod, mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import { z } from 'zod' + +import { STATE_DIR } from '@main/storage/paths.js' +import type { KeyVaultSnapshot } from '@shared/types/keyVault' + +// Vault persistence: one plaintext index + one encrypted blob per key +// secret (issue #831). +// +// WHY per-key blobs instead of one encrypted vault document: the same +// reasoning as src/main/dictation/apiKeyStore.ts — a malformed cipher +// blob (safeStorage refusing to decrypt after a macOS Keychain reset) +// must cost exactly one key, never the whole vault. The index carries +// only non-secret metadata (names, notes, timestamps, last-4 hints), so +// the provider list renders without touching decryption at all. +// +// WHY the codec is injected: unit tests run outside the packaged app +// where the `electron` module (and therefore safeStorage) does not +// exist. The store's own responsibilities are file discipline; the real +// codec is wired in src/main/index.ts. +// +// Write discipline mirrors apiKeyStore / remote/auth/secret.ts: mkdir +// recursive, temp file + atomic rename, 0o600 on secret blobs, and a +// decrypt failure reads back as null instead of throwing so callers can +// degrade per key. + +export type SecretCodec = { + isEncryptionAvailable(): boolean + encrypt(plain: string): Buffer + decrypt(cipher: Buffer): string +} + +export function newVaultId(): string { + return randomUUID() +} + +type IndexFile = KeyVaultSnapshot & { version: 1 } + +const identifier = z.string().regex(/^[a-zA-Z0-9_-]{1,128}$/) +const providerSchema = z.object({ + id: identifier, name: z.string().min(1).max(200), + createdAt: z.number().finite(), updatedAt: z.number().finite(), +}).strict() +const indexSchema = z.object({ + version: z.literal(1), + providers: z.array(providerSchema).max(1000), + keys: z.array(providerSchema.extend({ + providerId: identifier, note: z.string().max(4000), hint: z.string().max(4), + }).strict()).max(10000), +}).strict() + +export type VaultStore = { + encryptionAvailable(): boolean + loadIndex(): Promise + saveIndex(snapshot: KeyVaultSnapshot): Promise + readSecret(keyId: string): Promise + writeSecret(keyId: string, value: string): Promise + deleteSecret(keyId: string): Promise +} + +export function createFileVaultStore( + rootDir: string = join(STATE_DIR, 'key-vault'), + codec: SecretCodec, +): VaultStore { + const indexFile = join(rootDir, 'index.json') + const keysDir = join(rootDir, 'keys') + + function secretPath(keyId: string): string { + if (!identifier.safeParse(keyId).success) throw new Error('Invalid vault key id.') + return join(keysDir, `${keyId}.bin`) + } + + async function atomicWrite( + path: string, + data: string | Buffer, + mode: 0o600 | undefined, + ): Promise { + await mkdir(dirname(path), { recursive: true, mode: 0o700 }) + // Unique across store instances too; exclusive creation avoids following + // an existing temporary symlink. Never publish a permissions failure. + const tmp = `${path}.${randomUUID()}.tmp` + try { + await writeFile(tmp, data, { mode: mode ?? 0o600, flag: 'wx' }) + await chmod(tmp, mode ?? 0o600) + await rename(tmp, path) + } finally { + await rm(tmp, { force: true }) + } + } + + return { + encryptionAvailable: () => codec.isEncryptionAvailable(), + + async loadIndex() { + let raw: string + try { + raw = await readFile(indexFile, 'utf8') + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return { providers: [], keys: [] } + throw new Error('Vault index cannot be read; the existing vault has not been changed.') + } + try { + const parsed = indexSchema.parse(JSON.parse(raw)) + const providers = new Set(parsed.providers.map(p => p.id)) + if (providers.size !== parsed.providers.length || + new Set(parsed.keys.map(k => k.id)).size !== parsed.keys.length || + parsed.keys.some(k => !providers.has(k.providerId))) throw new Error('Invalid references') + return { providers: parsed.providers, keys: parsed.keys } + } catch { + // Returning an empty index here would let the next CRUD operation + // overwrite recoverable data. Only the vault UI fails, not app boot. + throw new Error('Vault index is damaged or unsupported; it has not been changed.') + } + }, + + async saveIndex(snapshot) { + const file: IndexFile = { + version: 1, + providers: snapshot.providers, + keys: snapshot.keys, + } + // Index is non-secret metadata; 0o600 is harmless belt-and-braces. + await atomicWrite(indexFile, `${JSON.stringify(file, null, 2)}\n`, 0o600) + }, + + async readSecret(keyId) { + const path = secretPath(keyId) + if (!codec.isEncryptionAvailable()) return null + let cipher: Buffer + try { + cipher = await readFile(path) + } catch { + return null + } + try { + const plain = codec.decrypt(cipher) + return plain.length > 0 ? plain : null + } catch { + // Decrypt failure after a Keychain reset: report unreadable, + // leave the blob in place (matches apiKeyStore's reasoning that + // future safeStorage recovery should stay possible). + return null + } + }, + + async writeSecret(keyId, value) { + const path = secretPath(keyId) + if (!codec.isEncryptionAvailable()) throw new Error('System keyring unavailable; cannot store a key.') + await atomicWrite(path, codec.encrypt(value), 0o600) + }, + + async deleteSecret(keyId) { + await rm(secretPath(keyId), { force: true }) + }, + } +} diff --git a/src/preload/api/index.ts b/src/preload/api/index.ts index eba5ec86..330f52dd 100644 --- a/src/preload/api/index.ts +++ b/src/preload/api/index.ts @@ -22,6 +22,7 @@ import { agentManagementApi } from '@preload/api/agentManagement.js' import { aiWorkspaceApi } from '@preload/api/aiWorkspace.js' import { renderedContentApi } from '@preload/api/renderedContent.js' import { caffeinateApi } from '@preload/api/caffeinate.js' +import { keyVaultApi } from '@preload/api/keyVault.js' import { menuApi } from '@preload/api/menu.js' import { incidentApi } from '@preload/api/incident.js' import { lifecycleApi } from '@preload/api/lifecycle.js' @@ -73,6 +74,7 @@ export const api = { ...aiWorkspaceApi, ...renderedContentApi, ...caffeinateApi, + ...keyVaultApi, ...menuApi, ...incidentApi, ...lifecycleApi, diff --git a/src/preload/api/keyVault.ts b/src/preload/api/keyVault.ts new file mode 100644 index 00000000..61da48b4 --- /dev/null +++ b/src/preload/api/keyVault.ts @@ -0,0 +1,31 @@ +import { ipcRenderer } from 'electron' +import { subscribe } from '@preload/api/ipc.js' + +import type { KeyVaultKeyInput, KeyVaultSnapshot, KeyVaultStatus } from '@shared/types/keyVault' + +// Flat key-vault surface merged into window.api. Rejected promises carry +// the service's Error message (cancel, fail-closed, not-found) so the UI +// can toast it verbatim. +export const keyVaultApi = { + onKeyVaultLocked: (callback: () => void) => subscribe('key-vault:locked', callback), + keyVaultStatus: (): Promise => ipcRenderer.invoke('key-vault:status'), + keyVaultList: (): Promise => ipcRenderer.invoke('key-vault:list'), + keyVaultUnlock: (): Promise => ipcRenderer.invoke('key-vault:unlock'), + keyVaultLock: (): Promise => ipcRenderer.invoke('key-vault:lock'), + keyVaultReveal: (providerId: string, keyId: string): Promise => + ipcRenderer.invoke('key-vault:reveal', providerId, keyId), + keyVaultCopyKey: (providerId: string, keyId: string): Promise => + ipcRenderer.invoke('key-vault:copy-key', providerId, keyId), + keyVaultResolveReference: (providerName: string, keyName: string): Promise => + ipcRenderer.invoke('key-vault:resolve-ref', providerName, keyName), + keyVaultCreateProvider: (name: string): Promise => + ipcRenderer.invoke('key-vault:create-provider', name), + keyVaultRenameProvider: (id: string, name: string): Promise => + ipcRenderer.invoke('key-vault:rename-provider', id, name), + keyVaultDeleteProvider: (id: string): Promise => + ipcRenderer.invoke('key-vault:delete-provider', id), + keyVaultPutKey: (input: KeyVaultKeyInput): Promise => + ipcRenderer.invoke('key-vault:put-key', input), + keyVaultDeleteKey: (providerId: string, keyId: string): Promise => + ipcRenderer.invoke('key-vault:delete-key', providerId, keyId), +} diff --git a/src/renderer/src/app-state/types.ts b/src/renderer/src/app-state/types.ts index 5ed3624b..cede9243 100644 --- a/src/renderer/src/app-state/types.ts +++ b/src/renderer/src/app-state/types.ts @@ -120,6 +120,8 @@ export type UiShellSlice = UiShellState & { closeProviderSwitchPicker: () => void openUsageModal: () => void closeUsageModal: () => void + openKeyVault: () => void + closeKeyVault: () => void openRewindPrompt: (sessionId: SessionId) => void closeRewindPrompt: () => void openAgentViewModePicker: (sessionId: SessionId) => void diff --git a/src/renderer/src/app-state/uiShell/slice.ts b/src/renderer/src/app-state/uiShell/slice.ts index 1a012dca..c2e857e1 100644 --- a/src/renderer/src/app-state/uiShell/slice.ts +++ b/src/renderer/src/app-state/uiShell/slice.ts @@ -50,6 +50,7 @@ export const createUiShellSlice: StateCreator< closeOldAgentsOpen: false, bulkProviderSwitchOpen: false, usageModalOpen: false, + keyVaultOpen: false, providerSwitchPickerSessionId: null, rewindPromptSessionId: null, agentViewModePickerSessionId: null, @@ -328,6 +329,10 @@ export const createUiShellSlice: StateCreator< set({ usageModalOpen: true }, false, 'uiShell/openUsageModal'), closeUsageModal: () => set({ usageModalOpen: false }, false, 'uiShell/closeUsageModal'), + openKeyVault: () => + set({ keyVaultOpen: true }, false, 'uiShell/openKeyVault'), + closeKeyVault: () => + set({ keyVaultOpen: false }, false, 'uiShell/closeKeyVault'), openRewindPrompt: sessionId => set({ rewindPromptSessionId: sessionId }, false, 'uiShell/openRewindPrompt'), diff --git a/src/renderer/src/app-state/uiShell/types.ts b/src/renderer/src/app-state/uiShell/types.ts index 291c5053..0d314496 100644 --- a/src/renderer/src/app-state/uiShell/types.ts +++ b/src/renderer/src/app-state/uiShell/types.ts @@ -296,6 +296,9 @@ export type UiShellState = { * and persisting it in WorkspaceState would make a quota inspection look * like durable workspace data. */ usageModalOpen: boolean + /** When true, the API Key Vault modal is open (#831). Transient command + * chrome, not workspace data — same rationale as usageModalOpen above. */ + keyVaultOpen: boolean /** * Session captured when the single-agent Switch Provider command ran. * diff --git a/src/renderer/src/app/surfaces/registry.tsx b/src/renderer/src/app/surfaces/registry.tsx index 3f1ba7de..d6737a22 100644 --- a/src/renderer/src/app/surfaces/registry.tsx +++ b/src/renderer/src/app/surfaces/registry.tsx @@ -29,6 +29,7 @@ import { KeyboardShortcutsSurface } from '@renderer/features/settings/surfaces/K import { RewindToPromptSurface } from '@renderer/features/workspace/surfaces/RewindToPromptSurface' import { AgentTitlePromptSurface } from '@renderer/features/workspace/surfaces/AgentTitlePromptSurface' import { ProviderSwitchPickerSurface } from '@renderer/features/workspace/surfaces/ProviderSwitchPickerSurface' +import { KeyVaultModalSurface } from '@renderer/features/key-vault/surfaces/KeyVaultModalSurface' // The surface registry (issue #494). Adding a surface = write a wrapper // in the owning feature's surfaces/ folder + add ONE import + ONE array @@ -87,6 +88,7 @@ export const modalSurfaces: SurfaceEntry[] = [ // New modals append so their z-50 sibling order cannot accidentally move an // established surface below one it used to cover; see the registry contract. { id: 'provider-switch-picker', Component: ProviderSwitchPickerSurface }, + { id: 'key-vault', Component: KeyVaultModalSurface }, ] /** diff --git a/src/renderer/src/control/featureReference.ts b/src/renderer/src/control/featureReference.ts index 1812eeaf..db13e4ea 100644 --- a/src/renderer/src/control/featureReference.ts +++ b/src/renderer/src/control/featureReference.ts @@ -16,6 +16,7 @@ import { controlReference as reference14 } from '@renderer/features/global-edito import { controlReference as reference15 } from '@renderer/features/path-picker/controlReference' import { controlReference as reference16 } from '@renderer/features/performance/controlReference' import { controlReference as reference17 } from '@renderer/features/prompt-templates/controlReference' +import { controlReference as reference32 } from '@renderer/features/key-vault/controlReference' import { controlReference as reference18 } from '@renderer/features/reader/controlReference' import { controlReference as reference19 } from '@renderer/features/remote/controlReference' import { controlReference as reference20 } from '@renderer/features/rendered-content/controlReference' @@ -64,6 +65,7 @@ export const featureReferences = [ ...reference29, ...reference30, ...reference31, + ...reference32, ] // These directories implement shared infrastructure rather than separate UI @@ -94,6 +96,10 @@ export const referenceOwnership = { "copy-code-block": "copy-code-block", "performance": "performance", "prompt-templates": "prompt-templates", + "key-vault": "key-vault", + // Shared insertion path behind templates + the vault (#830): no UI of + // its own; its behavior is documented on the prompt-templates page. + "session-text-delivery": "prompt-templates", "reply-to-selection": "reply-to-selection", "agent-status": "agent-status", "ai-workspace": "ai-workspace", diff --git a/src/renderer/src/features/command-palette/catalog.test.ts b/src/renderer/src/features/command-palette/catalog.test.ts index ce46f657..d888becc 100644 --- a/src/renderer/src/features/command-palette/catalog.test.ts +++ b/src/renderer/src/features/command-palette/catalog.test.ts @@ -11,8 +11,8 @@ import type { CommandDef } from '@renderer/features/command-palette/types' // // This file pinned the exact 102-id before-state, then the 106-id governance // after-state (102 - 5 retired + 9 added), then 112 with Grid Dispatch's six -// row commands (#681), 113 with New Window (#688), and now 114 with Clear -// Agent Composer (#683). Keeping ONE snapshot that moved — rather +// row commands (#681), 113 with New Window (#688), 114 with Clear +// Agent Composer (#683), and 115 with API Key Vault (#831). Keeping ONE snapshot that moved — rather // than a "baseline" file and an "after" file — is what makes the plan's // headline count an assertion anyone can check against running code instead of // prose. @@ -147,10 +147,11 @@ const BASELINE_COMMAND_IDS: readonly string[] = [ // copy-assistant / copy-code-block (2) 'copy-assistant-message', 'copy-code-block', - // prompt templates + reply to selection (4) + // prompt templates + api key vault + reply to selection (5) 'manage-prompt-templates', 'prompt-template', 'save-composer-as-prompt-template', + 'api-key-vault', 'reply-to-selection', // agent status / remote (2) 'show-agent-status', @@ -188,12 +189,12 @@ const NAVIGATION_COMMAND_GROUP: readonly string[] = [ const ids = (): string[] => builtInCommandCatalog.map(c => c.id) describe('built-in command catalog — baseline characterization', () => { - it('contains exactly the 114 governed commands in registration order', () => { + it('contains exactly the 115 governed commands in registration order', () => { // Order matters: this is the palette's empty-query browse order. expect(ids()).toEqual([...BASELINE_COMMAND_IDS]) }) - it('has exactly 114 commands', () => { + it('has exactly 115 commands', () => { // Stated separately from the order assertion because this number is the // thing that moves, and a bare count failure is a clearer signal than a // 99-line array diff. @@ -202,10 +203,11 @@ describe('built-in command catalog — baseline characterization', () => { // `open-keyboard-shortcuts` → 102 with the three composer commands → 104 // with the two lane-removal commands → 105 with Set Agent Title → 106 with // New Lane → 112 with Grid Dispatch's six row commands → 113 with New - // Window → 114 with Clear Agent Composer (#683). Each + // Window → 114 with Clear Agent Composer (#683) → 115 with API Key + // Vault (#831). Each // step of that arithmetic was a deliberate edit to this line, which is the // entire point of pinning it. - expect(builtInCommandCatalog).toHaveLength(114) + expect(builtInCommandCatalog).toHaveLength(115) }) it('reports no structural defects', () => { @@ -236,11 +238,11 @@ describe('generated per-provider split commands', () => { }) it('accounts for the difference between literal and total command count', () => { - // 114 total - 4 generated = 110 literal `id:` fields across the command + // 115 total - 4 generated = 111 literal `id:` fields across the command // modules. At the original baseline this read 102 - 4 = 98; it moved down by // the five retirements, then back up by the nine additions, Grid Dispatch's // six row commands, and New Window. - expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(110) + expect(builtInCommandCatalog.length - nonDefaultProviders.length * 2).toBe(111) }) it('emits both directions for every non-default provider', () => { @@ -339,7 +341,7 @@ describe('governance targets', () => { }) it('lands on the arithmetic the plan predicted', () => { - // 102 baseline - 5 retirements + 17 additions = 114, checked against the + // 102 baseline - 5 retirements + 18 additions = 115, checked against the // real catalog rather than trusted as prose. // // The subtracted term is the count of APPROVED ADDITIONS and the expected @@ -357,9 +359,9 @@ describe('governance targets', () => { // `new-dispatch-row`, `remove-dispatch-row`, `dispatch-row-project`, // `dispatch-row-child-cap`, `dispatch-focus-row-up`, // `dispatch-focus-row-down`, `new-window` (#688), and - // `clear-agent-composer` (#683). - expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 17).toBe(102) - expect(builtInCommandCatalog).toHaveLength(114) + // `clear-agent-composer` (#683), and `api-key-vault` (#831). + expect(builtInCommandCatalog.length + RETIRED_COMMAND_IDS.length - 18).toBe(102) + expect(builtInCommandCatalog).toHaveLength(115) }) }) diff --git a/src/renderer/src/features/command-palette/catalog.ts b/src/renderer/src/features/command-palette/catalog.ts index b7585560..5418d710 100644 --- a/src/renderer/src/features/command-palette/catalog.ts +++ b/src/renderer/src/features/command-palette/catalog.ts @@ -11,6 +11,7 @@ import { readerCommands } from '@renderer/features/reader/commands/readerCommand import { copyAssistantCommands } from '@renderer/features/copy-assistant/commands/copyAssistantCommands' import { copyCodeBlockCommands } from '@renderer/features/copy-code-block/commands/copyCodeBlockCommands' import { promptTemplateCommands } from '@renderer/features/prompt-templates/commands/promptTemplateCommands' +import { keyVaultCommands } from '@renderer/features/key-vault/commands/keyVaultCommands' import { replyToSelectionCommands } from '@renderer/features/reply-to-selection/commands/replyToSelectionCommands' import { agentStatusCommands } from '@renderer/features/agent-status/commands/agentStatusCommands' import { dispatchColorFlagCommands } from '@renderer/features/workspace/commands/dispatchColorFlagCommands' @@ -78,8 +79,9 @@ export const builtInCommandCatalog: readonly CommandDef[] = Object.freeze([ ...copyAssistantCommands, ...copyCodeBlockCommands, ...promptTemplateCommands, - // Grouped with the prompt-template commands because it is the other - // composer-insertion command — registry order is the palette's + ...keyVaultCommands, + // Grouped with the prompt-template and vault commands because those are + // the other insertion commands — registry order is the palette's // empty-query browse order, so like things stay adjacent. ...replyToSelectionCommands, ...agentStatusCommands, diff --git a/src/renderer/src/features/command-palette/types.ts b/src/renderer/src/features/command-palette/types.ts index a4dbc1e0..fa93307d 100644 --- a/src/renderer/src/features/command-palette/types.ts +++ b/src/renderer/src/features/command-palette/types.ts @@ -208,6 +208,7 @@ export type CommandContext = { /** Open the title editor for the captured command-target agent. */ openAgentTitlePrompt: (sessionId: string) => void openUsageModal: () => void + openKeyVault: () => void toggleGitBar: () => void toggleWorktreesBar: () => void toggleDebugPanel: () => void diff --git a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx index 59b86ace..1d30e52d 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -55,10 +55,9 @@ import type { PendingCommandInvocation } from '@renderer/app-state/uiShell/types import { allPromptTemplates, } from '@renderer/features/prompt-templates/templates' -import { - applyPromptTemplateInsertMode, - fillPromptTemplateBody, -} from '@renderer/features/prompt-templates/interpolate' +import { prepareTemplateText } from '@renderer/features/prompt-templates/keyReferences' +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' + import { createSavedPromptTemplate, duplicatePromptTemplate, @@ -119,6 +118,7 @@ type BuriedPaneInfo = { } type PromptTemplateFillState = { + sessionId: string template: PromptTemplate values: PromptTemplateVariableValueMap insertMode: PromptTemplateInsertMode @@ -253,6 +253,12 @@ function OpenCommandPalette({ // are untouched, and the future provider-enumeration rewrite (#394 §7) // rebuilds command CONTENT, not this assembly. const workspace = useWorkspaceContext() + const templateActive = useRef(visible) + const templateBusy = useRef(false) + useEffect(() => { + templateActive.current = visible + return () => { templateActive.current = false } + }, [visible]) // Injected into the execution gateway so an async command failure is // visible. Before the gateway, every call site was `void command.run(ctx)` // and a rejected promise vanished with no user-facing signal at all. @@ -292,6 +298,7 @@ function OpenCommandPalette({ const closePinAgents = useAppStore(state => state.closePinAgents) const closePathPicker = useAppStore(state => state.closePathPicker) const openUsageModal = useAppStore(state => state.openUsageModal) + const openKeyVault = useAppStore(state => state.openKeyVault) const toggleGitBar = useAppStore(state => state.toggleGitBar) const toggleWorktreesBar = useAppStore(state => state.toggleWorktreesBar) const toggleDebugPanel = useAppStore(state => state.toggleDebugPanel) @@ -628,6 +635,7 @@ function OpenCommandPalette({ closePinAgents, closePathPicker, openUsageModal, + openKeyVault, toggleGitBar, toggleWorktreesBar, toggleDebugPanel, @@ -736,6 +744,7 @@ function OpenCommandPalette({ closePinAgents, closePathPicker, openUsageModal, + openKeyVault, toggleGitBar, toggleWorktreesBar, toggleDebugPanel, @@ -1281,14 +1290,26 @@ function OpenCommandPalette({ async (template: PromptTemplate, originSelectedIndex = selectedIndex) => { const sessionId = promptTemplateSessionId if (!sessionId) return + if (templateBusy.current) return + templateBusy.current = true + const originalSession = workspace.state.sessions[sessionId] + const originalDraft = workspace.getRuntime(sessionId).draftInput try { const body = template.buildBody ? await template.buildBody({ workspace, sessionId }) : template.body + if (!templateActive.current) return if (template.variables.length > 0) { + // The fill pane stores the ORIGINAL body: `{{key:…}}` references + // stay as visible names and are resolved only at final insertion + // (see resolveVaultKeyReferences) — a secret must never render + // in the pane, and resolving early was ALSO wrong for combined + // ref+variable templates because the resolved body was discarded + // here (review finding). setPromptTemplateFillState({ - template: template.buildBody ? { ...template, body } : template, + sessionId, + template: { ...template, body }, values: {}, insertMode: template.insertMode, returnTo: promptTemplateFillReturnState(mode, query, originSelectedIndex), @@ -1298,18 +1319,40 @@ function OpenCommandPalette({ setSelectedIndex(0) return } - // Template insertion deliberately stops at the draft boundary. - // The user's next action is still visible and editable in the - // composer; nothing is sent to Claude/Codex until they press - // Enter themselves. This mirrors rewind-to-prompt's "prefill, - // don't replay" contract. - const currentDraft = workspace.getRuntime(sessionId).draftInput - workspace.setDraftInput(sessionId, applyPromptTemplateInsertMode(currentDraft, body, template.insertMode)) - workspace.showPaneToast(sessionId, `Inserted template: ${template.title}`) - onClose() + // Template insertion deliberately stops at the delivery boundary + // (#830): the user's next action is still visible and editable — + // composer draft for rendered panes, an unsubmitted bracketed + // paste for any PTY surface. Nothing is sent until they press + // Enter themselves, mirroring rewind-to-prompt's "prefill, don't + // replay" contract. + const text = await prepareTemplateText({ ...template, body }, {}, ref => + window.api.keyVaultResolveReference(ref.providerName, ref.keyName)) + if (!templateActive.current) return + if (useAppStore.getState().workspaceState.sessions[sessionId] !== originalSession || + workspace.getRuntime(sessionId).draftInput !== originalDraft) { + throw new Error('Target pane or draft changed while preparing the template. Try again.') + } + const result = await deliverTextToSession( + workspace, + sessionId, + text, + { insertMode: template.insertMode, isCurrent: () => templateActive.current && + useAppStore.getState().workspaceState.sessions[sessionId] === originalSession && + workspace.getRuntime(sessionId).draftInput === originalDraft }, + ) + if (result.delivered) { + workspace.showPaneToast(sessionId, `Inserted template: ${template.title}`) + onClose() + } else if (result.reason === 'write-rejected') { + workspace.showPaneToast(sessionId, 'Terminal write was rejected — pane is not ready') + } else { + workspace.showPaneToast(sessionId, 'Template target pane is gone') + } } catch (err) { const message = err instanceof Error ? err.message : String(err) workspace.showPaneToast(sessionId, `Template failed: ${message}`) + } finally { + templateBusy.current = false } }, [mode, onClose, promptTemplateSessionId, query, selectedIndex, workspace], @@ -1440,27 +1483,48 @@ function OpenCommandPalette({ [customPromptTemplates, setSettings], ) - const insertFilledPromptTemplate = useCallback(() => { + const insertFilledPromptTemplate = useCallback(async () => { const fill = promptTemplateFillState if (!fill) return - const sessionId = commandTargetSessionId(workspace) + const sessionId = fill.sessionId if (!sessionId) return + if (templateBusy.current) return + templateBusy.current = true + const originalSession = workspace.state.sessions[sessionId] + const originalDraft = workspace.getRuntime(sessionId).draftInput try { - const resolved = fillPromptTemplateBody({ - body: fill.template.body, - variables: fill.template.variables, - values: fill.values, - }) - const currentDraft = workspace.getRuntime(sessionId).draftInput - workspace.setDraftInput( + // Key references resolve HERE, after variable fill and immediately + // before delivery: the fill pane showed only names, and an + // unresolved reference aborts with a toast naming every failure + // instead of pasting literal `{{key:…}}` text (review finding). + const text = await prepareTemplateText(fill.template, fill.values, ref => + window.api.keyVaultResolveReference(ref.providerName, ref.keyName)) + if (!templateActive.current) return + if (useAppStore.getState().workspaceState.sessions[sessionId] !== originalSession || + workspace.getRuntime(sessionId).draftInput !== originalDraft) { + throw new Error('Target pane or draft changed while preparing the template. Try again.') + } + const result = await deliverTextToSession( + workspace, sessionId, - applyPromptTemplateInsertMode(currentDraft, resolved, fill.insertMode), + text, + { insertMode: fill.insertMode, isCurrent: () => templateActive.current && + useAppStore.getState().workspaceState.sessions[sessionId] === originalSession && + workspace.getRuntime(sessionId).draftInput === originalDraft }, ) - workspace.showPaneToast(sessionId, `Inserted template: ${fill.template.title}`) - onClose() + if (result.delivered) { + workspace.showPaneToast(sessionId, `Inserted template: ${fill.template.title}`) + onClose() + } else if (result.reason === 'write-rejected') { + workspace.showPaneToast(sessionId, 'Terminal write was rejected — pane is not ready') + } else { + workspace.showPaneToast(sessionId, 'Template target pane is gone') + } } catch (error) { const message = error instanceof Error ? error.message : String(error) workspace.showPaneToast(sessionId, `Template failed: ${message}`) + } finally { + templateBusy.current = false } }, [onClose, promptTemplateFillState, workspace]) @@ -1510,7 +1574,7 @@ function OpenCommandPalette({ if (mode === 'save-prompt-template' || mode === 'edit-prompt-template') { savePromptTemplateForm() } else if (mode === 'fill-prompt-template') { - insertFilledPromptTemplate() + void insertFilledPromptTemplate() } else if (mode === 'ai-workspace-create') { void createAiWorkspace() } else if (mode === 'ai-workspace-open') { diff --git a/src/renderer/src/features/key-vault/commands/keyVaultCommands.ts b/src/renderer/src/features/key-vault/commands/keyVaultCommands.ts new file mode 100644 index 00000000..9f84248b --- /dev/null +++ b/src/renderer/src/features/key-vault/commands/keyVaultCommands.ts @@ -0,0 +1,16 @@ +import type { CommandDef } from '@renderer/features/command-palette/types' + +export const keyVaultCommands: CommandDef[] = [ + { + id: 'api-key-vault', + category: 'workspace-tools', + surface: 'app', + title: 'API Key Vault…', + description: + '**What it does:** Opens the **API Key Vault** — manage provider API keys, insert them into the focused pane, copy to clipboard, and reference them from prompt templates (`{{key:Provider/Key}}`).\n\n**Use when:** You regularly paste API keys (Brave, OpenAI, …) into agent prompts.\n\n**Notes:** Encrypted with the OS keyring; one Touch ID / password unlock per app launch.', + keywords: ['api', 'key', 'vault', 'secret', 'credential', 'token', 'password'], + run: ({ ui }) => { + ui.openKeyVault() + }, + }, +] diff --git a/src/renderer/src/features/key-vault/controlReference.ts b/src/renderer/src/features/key-vault/controlReference.ts new file mode 100644 index 00000000..6a8912ae --- /dev/null +++ b/src/renderer/src/features/key-vault/controlReference.ts @@ -0,0 +1,23 @@ +import type { FeatureReference } from '@control-sdk' + +// Keep purpose, UI routes and limitations beside this feature. The assembled +// control reference adds current commands/bindings instead of copying them. +export const controlReference = [ + { + "id": "key-vault", + "title": "API Key Vault", + "purpose": "Store provider API keys encrypted at rest and insert them into any focused pane without re-copying from provider dashboards.", + "ui": "Command palette: API Key Vault modal (provider/key management, reveal, copy, insert).", + "prerequisites": "macOS keyring (safeStorage) available; Touch ID / login password for the once-per-launch unlock.", + "workflow": [ + "Create a provider (e.g. Brave)", + "add named keys with optional notes", + "insert into the focused pane, copy to clipboard, or reference from a template ({{key:Provider/Key}})" + ], + "outcome": "The selected key's value is delivered to the focused composer or terminal pane without submitting, or placed on the clipboard.", + "cautions": "Secrets are safeStorage-encrypted per key at rest in the vault and the modal keeps revealed values only in ephemeral component state. Reveal/copy/resolve cross a once-per-app-launch OS auth gate and fail closed on cancel. Template references resolve by provider/key NAME; renaming breaks references loudly. An INSERTED key leaves the vault's protection by design: composer drafts autosave to workspace.json in plaintext until sent or cleared, PTY pastes land in scrollback, and submitting puts the key in the provider transcript like any manual paste. Insertion into terminals is a bracketed paste WITHOUT Enter — review before submitting.", + "commandIds": [ + "api-key-vault" + ] + } +] satisfies FeatureReference[] diff --git a/src/renderer/src/features/key-vault/surfaces/KeyVaultModalSurface.tsx b/src/renderer/src/features/key-vault/surfaces/KeyVaultModalSurface.tsx new file mode 100644 index 00000000..9e03c2be --- /dev/null +++ b/src/renderer/src/features/key-vault/surfaces/KeyVaultModalSurface.tsx @@ -0,0 +1,8 @@ +import { useAppStore } from '@renderer/app-state/hooks' +import { KeyVaultModal } from '@renderer/features/key-vault/ui/KeyVaultModal' + +export function KeyVaultModalSurface() { + const open = useAppStore(state => state.keyVaultOpen) + if (!open) return null + return +} diff --git a/src/renderer/src/features/key-vault/ui/KeyVaultModal.renderer.test.tsx b/src/renderer/src/features/key-vault/ui/KeyVaultModal.renderer.test.tsx new file mode 100644 index 00000000..09180a7a --- /dev/null +++ b/src/renderer/src/features/key-vault/ui/KeyVaultModal.renderer.test.tsx @@ -0,0 +1,65 @@ +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { WorkspaceProvider } from '@renderer/workspace/WorkspaceContext' +import type { Workspace } from '@renderer/workspace/workspaceStore' +import { KeyVaultModal } from './KeyVaultModal' + +// The app, not a modal, owns workspace boot/subscriptions. This catches an +// accidental call to useWorkspace() while exercising the real context seam. +vi.mock('@renderer/workspace/workspaceStore', () => ({ + useWorkspace: () => { throw new Error('Modal started another workspace controller') }, +})) +const api = { + keyVaultStatus: vi.fn(async () => ({ encryptionAvailable: true, authPromptAvailable: true, unlocked: true })), + keyVaultList: vi.fn(async () => ({ + providers: [{ id: 'p', name: 'Brave' }], + keys: [{ id: 'k', providerId: 'p', name: 'main', note: '', hint: '1234' }], + })), + keyVaultUnlock: vi.fn(async () => {}), + keyVaultLock: vi.fn(async () => {}), + keyVaultReveal: vi.fn(async () => 'test-secret-1234'), + keyVaultRenameProvider: vi.fn(async () => {}), + keyVaultPutKey: vi.fn(async () => {}), + onKeyVaultLocked: vi.fn((_callback: () => void) => () => {}), +} +beforeEach(() => { + vi.clearAllMocks() + vi.stubGlobal('api', api) + Object.assign(window, { api }) +}) +afterEach(() => { cleanup(); vi.unstubAllGlobals() }) +function open() { + render( + + ) +} + +it('reuses the app workspace and renames a provider through an inline form', async () => { + open() + fireEvent.click(await screen.findByRole('button', { name: 'Rename' })) + fireEvent.change(screen.getByPlaceholderText('Provider name'), { target: { value: 'Brave Search' } }) + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + await waitFor(() => expect(api.keyVaultRenameProvider).toHaveBeenCalledWith('p', 'Brave Search')) +}) + +it('drops a pending reveal after a lock broadcast instead of displaying stale plaintext', async () => { + let resolve!: (value: string) => void + const promise = new Promise(done => { resolve = done }) + api.keyVaultReveal.mockReturnValueOnce(promise) + open() + fireEvent.click(await screen.findByRole('button', { name: 'Reveal' })) + await waitFor(() => expect(api.keyVaultReveal).toHaveBeenCalled()) + act(() => api.onKeyVaultLocked.mock.calls.at(-1)![0]()) + await act(async () => { resolve('test-secret-1234'); await promise }) + expect(screen.queryByText('test-secret-1234')).toBeNull() +}) + +it('keeps a failed key edit available for retry', async () => { + api.keyVaultPutKey.mockRejectedValueOnce(new Error('Disk unavailable')) + open() + fireEvent.click(await screen.findByRole('button', { name: 'Edit' })) + fireEvent.change(screen.getByPlaceholderText('Note (optional)'), { target: { value: 'keep my edits' } }) + fireEvent.click(screen.getByRole('button', { name: 'Save' })) + await screen.findByText('Disk unavailable') + expect(screen.getByPlaceholderText('Note (optional)')).toHaveValue('keep my edits') +}) diff --git a/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx new file mode 100644 index 00000000..953b29cf --- /dev/null +++ b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx @@ -0,0 +1,442 @@ +import { useCallback, useEffect, useRef, useState } from 'react' + +import { Button } from '@renderer/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from '@renderer/components/ui/dialog' +import { useAppStore } from '@renderer/app-state/hooks' +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' +import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/commandTargetSessionId' +import { useWorkspaceLayoutContext } from '@renderer/workspace/WorkspaceContext' +import type { KeyVaultKey, KeyVaultStatus } from '@shared/types/keyVault' + +// API Key Vault modal (#831). Revealed plaintext lives only in this +// component's ephemeral state — the VAULT never persists it — and is +// cleared on lock/close/deletion. Metadata flows through the +// window.api.keyVault* calls; every secret fetch crosses the main-side +// unlock gate (one OS prompt per app run), so "Reveal" on a locked vault +// is what triggers Touch ID / the login-password prompt. +// +// DISCLOSURE (review finding): once a key is INSERTED, it leaves the +// vault's protection by design — a composer draft autosaves to +// workspace.json in plaintext until sent or cleared, and a PTY paste +// lands in scrollback/tmux history. Submitting the prompt puts the key +// in the provider transcript, plaintext, exactly like a manual paste. +// The vault encrypts STORAGE, not the prompt pipeline. + +type KeyForm = { id?: string; name: string; value: string; note: string } | null + +export function KeyVaultModal() { + const closeKeyVault = useAppStore(state => state.closeKeyVault) + // Only App owns useWorkspace(): mounting another controller here would + // duplicate recovery, subscriptions and persistence every time the vault opens. + const workspace = useWorkspaceLayoutContext() + const targetSessionId = useRef(commandTargetSessionId(workspace)) + const generation = useRef(0) + const mounted = useRef(true) + const actionInFlight = useRef(false) + const [status, setStatus] = useState(null) + const [providers, setProviders] = useState<{ id: string; name: string }[]>([]) + const [keys, setKeys] = useState([]) + const [selectedProviderId, setSelectedProviderId] = useState(null) + // keyId -> revealed plaintext. Ephemeral by design; never persisted. + const [revealed, setRevealed] = useState>(new Map()) + const [newProviderName, setNewProviderName] = useState('') + // Inline rename (review finding): window.prompt does not exist in + // Electron — it throws — so Rename lived in a dead click handler. An + // inline input row follows the same pattern as "New provider…". + const [providerRename, setProviderRename] = useState<{ id: string; name: string } | null>(null) + const [keyForm, setKeyForm] = useState(null) + const [error, setError] = useState(null) + + const refresh = useCallback(async () => { + const started = generation.current + try { + await window.api.keyVaultUnlock() + if (!mounted.current || started !== generation.current) return + const [nextStatus, snapshot] = await Promise.all([ + window.api.keyVaultStatus(), + window.api.keyVaultList(), + ]) + if (!mounted.current || started !== generation.current) return + setStatus(nextStatus) + setProviders(snapshot.providers) + setKeys(snapshot.keys) + // Never keep plaintext for metadata that just disappeared (key or + // provider deleted elsewhere in the modal while revealed). + setRevealed(prev => { + const live = new Set(snapshot.keys.map(k => k.id)) + let changed = false + for (const id of prev.keys()) if (!live.has(id)) changed = true + return changed ? new Map([...prev].filter(([id]) => live.has(id))) : prev + }) + setSelectedProviderId(current => { + if (current && snapshot.providers.some(p => p.id === current)) return current + return snapshot.providers[0]?.id ?? null + }) + setError(null) + } catch (err) { + if (!mounted.current || started !== generation.current) return + setError(err instanceof Error ? err.message : String(err)) + } + }, []) + + const clearSecrets = useCallback(() => { + generation.current += 1 + setRevealed(new Map()) + setKeyForm(null) + setStatus(prev => prev ? { ...prev, unlocked: false } : prev) + }, []) + + useEffect(() => { + mounted.current = true + const off = window.api.onKeyVaultLocked(clearSecrets) + void refresh() + return () => { mounted.current = false; generation.current += 1; off() } + }, [refresh, clearSecrets]) + + const runVaultAction = async (action: () => Promise) => { + if (actionInFlight.current) return + actionInFlight.current = true + const started = generation.current + try { + await action() + if (!mounted.current || started !== generation.current) return + await refresh() + } catch (err) { + // Canceled OS prompt, fail-closed gate, duplicate name, … — the + // service messages are written for direct display. + setError(err instanceof Error ? err.message : String(err)) + } finally { + actionInFlight.current = false + } + } + + const addProvider = () => { + const name = newProviderName.trim() + if (!name) return + setNewProviderName('') + void runVaultAction(() => window.api.keyVaultCreateProvider(name)) + } + + const saveKeyForm = () => { + const form = keyForm + if (!form || !selectedProviderId) return + const started = generation.current + void runVaultAction(async () => { + await window.api.keyVaultPutKey({ + providerId: selectedProviderId, + id: form.id, + name: form.name, + value: form.value, + note: form.note, + }) + if (mounted.current && generation.current === started) { + setKeyForm(current => current === form ? null : current) + setRevealed(new Map()) + } + }) + } + + const toggleReveal = async (key: KeyVaultKey) => { + const started = generation.current + if (revealed.has(key.id)) { + const next = new Map(revealed) + next.delete(key.id) + setRevealed(next) + return + } + if (actionInFlight.current) return + actionInFlight.current = true + try { + const value = await window.api.keyVaultReveal(key.providerId, key.id) + if (!mounted.current || started !== generation.current) return + setRevealed(prev => new Map(prev).set(key.id, value)) + setStatus(prev => (prev ? { ...prev, unlocked: true } : prev)) + setError(null) + } catch (err) { + setError(err instanceof Error ? err.message : String(err)) + } finally { + actionInFlight.current = false + } + } + + const insertKey = async (key: KeyVaultKey) => { + const started = generation.current + const sessionId = targetSessionId.current + if (!sessionId) { + setError('No focused pane to insert into') + return + } + if (actionInFlight.current) return + actionInFlight.current = true + const owner = workspace.state.sessions[sessionId] + try { + // A display cache is not authorization; always re-enter the main gate. + const value = await window.api.keyVaultReveal(key.providerId, key.id) + if (!mounted.current || started !== generation.current) return + if (useAppStore.getState().workspaceState.sessions[sessionId] !== owner) { + throw new Error('Target pane changed while unlocking. Close the vault and choose the pane again.') + } + const result = await deliverTextToSession(workspace, sessionId, value, { + isCurrent: () => mounted.current && started === generation.current && + useAppStore.getState().workspaceState.sessions[sessionId] === owner, + }) + if (result.delivered) { + workspace.showPaneToast(sessionId, `Inserted key: ${key.name}`) + closeKeyVault() + } else if (result.reason === 'write-rejected') { + setError('Terminal write was rejected — pane is not ready; try again') + } else { + setError('Focused pane is no longer available') + } + } catch (err) { + setError(err instanceof Error ? err.message : String(err)) + } finally { + actionInFlight.current = false + } + } + + const selectedKeys = keys.filter(k => k.providerId === selectedProviderId) + const selectedProvider = providers.find(p => p.id === selectedProviderId) ?? null + + return ( + { if (!nextOpen) closeKeyVault() }}> + + +
+ API Key Vault + + {status + ? status.unlocked + ? 'unlocked for this app launch' + : 'locked — revealing a key prompts once per launch' + : '…'} + +
+ {status?.unlocked && ( + + )} + + + {status && !status.encryptionAvailable && ( +
+ OS keyring (safeStorage) is unavailable on this machine — keys cannot be stored. +
+ )} + {error && ( +
{error}
+ )} + + {!status?.unlocked && } + {status?.unlocked &&
+
+ {providers.map(provider => ( + + ))} + setNewProviderName(e.target.value)} + onKeyDown={e => { if (e.key === 'Enter') addProvider() }} + /> +
+ +
+ {!selectedProvider && ( +
Create a provider to get started.
+ )} + {selectedProvider && ( + <> +
+ {selectedProvider.name} +
+ + + +
+
+ + {providerRename && providerRename.id === selectedProvider.id && ( +
+ setProviderRename({ ...providerRename, name: e.target.value })} + onKeyDown={e => { + if (e.key !== 'Enter') return + const name = providerRename.name.trim() + if (!name) return + const id = providerRename.id + setProviderRename(null) + void runVaultAction(() => window.api.keyVaultRenameProvider(id, name)) + }} + /> + + +
+ )} + + {selectedKeys.map(key => ( +
+
+ {key.name} + ••••{key.hint} + {revealed.has(key.id) && ( + + {revealed.get(key.id)} + + )} + + + + + + +
+ {key.note &&
{key.note}
} +
+ ))} + + {keyForm && ( +
+
{keyForm.id ? 'Edit key' : 'New key'}
+ setKeyForm({ ...keyForm, name: e.target.value })} + /> + setKeyForm({ ...keyForm, value: e.target.value })} + /> + setKeyForm({ ...keyForm, note: e.target.value })} + /> +
+ + +
+
+ )} + + )} +
+
+ + } +
+ Reference keys from prompt templates with {'{{key:Provider/Key}}'} · Encrypted with the OS + keyring · One unlock per app launch · An inserted key sits in the saved draft (or terminal + scrollback) until sent or cleared +
+ + + ) +} diff --git a/src/renderer/src/features/prompt-templates/commands/promptTemplateCommands.ts b/src/renderer/src/features/prompt-templates/commands/promptTemplateCommands.ts index c3d46f46..79977ab2 100644 --- a/src/renderer/src/features/prompt-templates/commands/promptTemplateCommands.ts +++ b/src/renderer/src/features/prompt-templates/commands/promptTemplateCommands.ts @@ -1,5 +1,8 @@ import type { CommandDef } from '@renderer/features/command-palette/types' -import { promptTemplateTargetSessionId } from '@renderer/features/prompt-templates/targetSession' +import { + promptTemplateComposerSessionIdForState, + promptTemplateTargetSessionId, +} from '@renderer/features/prompt-templates/targetSession' export const promptTemplateCommands: CommandDef[] = [ { @@ -27,10 +30,13 @@ export const promptTemplateCommands: CommandDef[] = [ category: 'session', surface: 'session', title: 'Prompt Template…', - description: '**What it does:** Inserts a saved **prompt template** into the focused composer.\n\n**Use when:** You want reusable prompt text without retyping it.\n\n**Notes:** Agent panes only.', + description: '**What it does:** Inserts a saved **prompt template** into the focused pane.\n\n**Use when:** You want reusable prompt text without retyping it.\n\n**Notes:** Rendered panes insert into the composer; terminal panes receive a bracketed paste without submitting.', keywords: ['prompt', 'template', 'snippet', 'insert', 'draft'], keepPaletteOpen: true, - renderedViewPolicy: { kind: 'opens-rendered-feed' }, + // No renderedViewPolicy since #830: terminal-surface panes have no + // rendered feed, and "requires-rendered-feed" would hide this command + // from exactly the panes (raw terminals, agent terminal view) where + // bracket-paste insertion just became possible. when: ({ workspace }) => promptTemplateTargetSessionId(workspace) !== null, run: ({ ui, flags }) => { // Already showing this mode? Dismiss. A mode-entering command whose @@ -54,7 +60,8 @@ export const promptTemplateCommands: CommandDef[] = [ keepPaletteOpen: true, renderedViewPolicy: { kind: 'requires-rendered-feed' }, when: ({ workspace }) => { - const sessionId = promptTemplateTargetSessionId(workspace) + // Saving reads the composer draft; terminal panes have none. + const sessionId = promptTemplateComposerSessionIdForState(workspace.state) if (!sessionId) return false return workspace.getRuntime(sessionId).draftInput.trim().length > 0 }, diff --git a/src/renderer/src/features/prompt-templates/controlReference.ts b/src/renderer/src/features/prompt-templates/controlReference.ts index d6ac8eb5..e0f45877 100644 --- a/src/renderer/src/features/prompt-templates/controlReference.ts +++ b/src/renderer/src/features/prompt-templates/controlReference.ts @@ -16,7 +16,7 @@ export const controlReference = [ "submit deliberately." ], "outcome": "The intended prompt text is available for the target agent.", - "cautions": "templates.list/read describes stored or dynamic bodies. templates.insert requires template/draft revisions and an explicit project, preserves attachments and never sends. templates.save/delete changes custom templates only. Inspect the resulting draft before delivery.", + "cautions": "templates.list/read describes stored or dynamic bodies. templates.insert requires template/draft revisions and an explicit project, preserves attachments and never sends. templates.save/delete changes custom templates only. Insertion targets any focused pane (#830): rendered panes edit the composer draft, terminal panes receive an unsubmitted bracketed paste. {{key:Provider/Key}} vault references resolve at insertion time and abort loudly when unresolved; the MCP control path does not resolve them. Inspect the resulting draft before delivery.", "commandIds": [ "prompt-template", "manage-prompt-templates", diff --git a/src/renderer/src/features/prompt-templates/keyReferences.test.ts b/src/renderer/src/features/prompt-templates/keyReferences.test.ts new file mode 100644 index 00000000..e071d721 --- /dev/null +++ b/src/renderer/src/features/prompt-templates/keyReferences.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest' + +import { collectKeyReferences, resolveKeyReferences, prepareTemplateText } from '@renderer/features/prompt-templates/keyReferences' + +describe('collectKeyReferences', () => { + it('collects and dedupes references', () => { + const refs = collectKeyReferences( + 'Use {{key:Brave/main}} and again {{ key:Brave/main }}, plus {{key:OpenAI/prod key}}', + ) + expect(refs).toEqual([ + { providerName: 'Brave', keyName: 'main' }, + { providerName: 'OpenAI', keyName: 'prod key' }, + ]) + }) + + it('ignores ordinary template variables', () => { + expect(collectKeyReferences('{{name}} and {{ date }}')).toEqual([]) + }) +}) + +describe('resolveKeyReferences', () => { + it('preserves source-code braces in a template without declared variables', async () => { + const body = 'Example: {{ count }} and {{foo}}. Key: {{key:Brave/main}}' + expect(await prepareTemplateText({ body, variables: [] }, {}, async () => 'credential')) + .toBe('Example: {{ count }} and {{foo}}. Key: credential') + }) + it('fills ordinary variables before resolving vault references without mutating the saved template', async () => { + const template = { + body: 'Use {{key:Brave/main}} for {{task}}', + variables: [{ name: 'task', label: 'Task', description: '', defaultValue: '', required: true }], + } + expect(await prepareTemplateText(template, { task: 'search' }, async () => 'credential')).toBe('Use credential for search') + expect(template.body).toBe('Use {{key:Brave/main}} for {{task}}') + }) + it('substitutes resolved values', async () => { + const resolved = await resolveKeyReferences( + 'Brave key: {{key:Brave/main}}', + async ref => (ref.providerName === 'Brave' ? 'BSA-1' : null), + ) + expect(resolved).toBe('Brave key: BSA-1') + }) + + it('aborts loudly on an unresolved reference', async () => { + await expect( + resolveKeyReferences('{{key:Brave/nope}}', async () => null), + ).rejects.toThrow(/Unresolved key reference/) + }) + + it('names every failure when multiple refs are broken', async () => { + await expect( + resolveKeyReferences('{{key:Brave/a}} {{key:OpenAI/b}}', async () => null), + ).rejects.toThrow(/Brave\/a.*OpenAI\/b/) + }) +}) diff --git a/src/renderer/src/features/prompt-templates/keyReferences.ts b/src/renderer/src/features/prompt-templates/keyReferences.ts new file mode 100644 index 00000000..1d46da22 --- /dev/null +++ b/src/renderer/src/features/prompt-templates/keyReferences.ts @@ -0,0 +1,74 @@ +// API Key Vault references inside prompt template bodies (#831). +// +// Syntax: {{key:Provider Name/Key Name}} — matched case-sensitively +// against vault metadata at execution time. WHY names and not ids: +// templates are user-authored text that must stay readable and +// portable; uuids would make every template an opaque pile. Renaming a +// provider or key therefore breaks its references — resolution aborts +// loudly with a toast instead of silently inserting nothing. +// +// WHY this pattern is separate from the ordinary {{variable}} grammar: +// the placeholder pattern is [A-Za-z0-9_]+ only, so these refs never +// collide with or surface as form fields in the fill pane; they are +// resolved only after variable fill, at insertion, so the pane never sees a secret. + +import { fillPromptTemplateBody } from './interpolate' +import type { PromptTemplate, PromptTemplateVariableValueMap } from './types' + +// Both picker paths share this order. Resolving before the fill pane either +// exposed credentials in its preview or discarded the resolved body entirely. +export function prepareTemplateText( + template: Pick, + values: PromptTemplateVariableValueMap, + resolve: (ref: KeyReference) => Promise, +): Promise { + // Dynamic worktree/transcript dumps can contain Vue/Jinja/Handlebars braces. + // With no declared variables the old picker inserted that source verbatim; + // running the filler would silently erase every unrelated {{word}}. + const body = template.variables.length ? fillPromptTemplateBody({ ...template, values }) : template.body + return resolveKeyReferences(body, resolve) +} + +export type KeyReference = { providerName: string; keyName: string } + +const KEY_REF_PATTERN = /\{\{\s*key:([^/{}]+?)\/([^/{}]+?)\s*\}\}/g + +export function collectKeyReferences(body: string): KeyReference[] { + const seen = new Set() + const ordered: KeyReference[] = [] + for (const match of body.matchAll(KEY_REF_PATTERN)) { + const ref = { providerName: match[1].trim(), keyName: match[2].trim() } + const dedupeKey = `${ref.providerName}\u0000${ref.keyName}` + if (seen.has(dedupeKey)) continue + seen.add(dedupeKey) + ordered.push(ref) + } + return ordered +} + +export async function resolveKeyReferences( + body: string, + resolve: (ref: KeyReference) => Promise, +): Promise { + // Resolve every distinct reference out of band first (a synchronous + // String.replace callback cannot await, and each ref may cross the + // vault gate), collecting ALL failures so one error message tells the + // user everything that needs fixing. + const refs = collectKeyReferences(body) + const values = new Map() + const failures: string[] = [] + for (const ref of refs) { + const value = await resolve(ref) + if (value === null || value.length === 0) { + failures.push(`{{key:${ref.providerName}/${ref.keyName}}}`) + continue + } + values.set(`${ref.providerName}\u0000${ref.keyName}`, value) + } + if (failures.length > 0) { + throw new Error(`Unresolved key reference: ${failures.join(', ')}`) + } + return body.replace(KEY_REF_PATTERN, (_match, rawProvider: string, rawKey: string) => { + return values.get(`${rawProvider.trim()}\u0000${rawKey.trim()}`) ?? '' + }) +} diff --git a/src/renderer/src/features/prompt-templates/targetSession.test.ts b/src/renderer/src/features/prompt-templates/targetSession.test.ts index 08598480..41d2ab49 100644 --- a/src/renderer/src/features/prompt-templates/targetSession.test.ts +++ b/src/renderer/src/features/prompt-templates/targetSession.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from 'vitest' -import { promptTemplateTargetSessionIdForState } from '@renderer/features/prompt-templates/targetSession' +import { + promptTemplateComposerSessionIdForState, + promptTemplateTargetSessionIdForState, +} from '@renderer/features/prompt-templates/targetSession' import type { WorkspaceState } from '@renderer/workspace/types' function stateWithFocusedSession(kind: 'claude' | 'terminal'): WorkspaceState { @@ -21,10 +24,19 @@ function stateWithFocusedSession(kind: 'claude' | 'terminal'): WorkspaceState { } describe('promptTemplateTargetSessionIdForState', () => { - it('offers template insertion only when the command target owns an agent composer', () => { + it('accepts agent panes and terminal panes (bracket-paste insertion, #830)', () => { expect(promptTemplateTargetSessionIdForState(stateWithFocusedSession('claude'))) .toBe('session-1') + // Terminals are valid targets: deliverTextToSession routes them to a + // bracketed paste over sendInput instead of a composer draft edit. expect(promptTemplateTargetSessionIdForState(stateWithFocusedSession('terminal'))) + .toBe('session-1') + }) + + it('restricts composer-draft commands to agent panes', () => { + expect(promptTemplateComposerSessionIdForState(stateWithFocusedSession('claude'))) + .toBe('session-1') + expect(promptTemplateComposerSessionIdForState(stateWithFocusedSession('terminal'))) .toBeNull() }) diff --git a/src/renderer/src/features/prompt-templates/targetSession.ts b/src/renderer/src/features/prompt-templates/targetSession.ts index 0f006d3c..99f8a5e9 100644 --- a/src/renderer/src/features/prompt-templates/targetSession.ts +++ b/src/renderer/src/features/prompt-templates/targetSession.ts @@ -4,19 +4,28 @@ import type { WorkspaceState } from '@renderer/workspace/types' import type { Workspace } from '@renderer/workspace/workspaceStore' /** - * Resolves the composer that may receive a prompt template. + * Resolves the pane that may receive a prompt template. * - * WHY this is narrower than the shared command target: terminal panes are real - * sessions and therefore valid targets for lifecycle commands, but they do not - * own the agent composer that template insertion edits. Keeping this predicate - * feature-owned and shared by both command visibility and execution prevents a - * picker from advertising an action that can only return silently. + * WHY this is the shared command target and not narrower (#830): + * terminal panes became valid template targets when insertion learned to + * bracket-paste into any focused PTY via deliverTextToSession. The old + * agent-only predicate hid the command from exactly the panes where + * users run unsupported agent harnesses in a raw terminal. */ export function promptTemplateTargetSessionId(workspace: Workspace): string | null { return promptTemplateTargetSessionIdForState(workspace.state) } export function promptTemplateTargetSessionIdForState(state: WorkspaceState): string | null { + return commandTargetSessionIdForState(state) +} + +/** + * Whether the target pane owns an agent composer whose DRAFT a command + * can read. Terminal panes are valid delivery targets (#830) but have no + * composer; commands like "save composer as template" stay hidden there. + */ +export function promptTemplateComposerSessionIdForState(state: WorkspaceState): string | null { const sessionId = commandTargetSessionIdForState(state) if (!sessionId) return null const kind = state.sessions[sessionId]?.kind ?? DEFAULT_PROVIDER diff --git a/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts b/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts new file mode 100644 index 00000000..b81d463a --- /dev/null +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts @@ -0,0 +1,157 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { encodeTerminalPaste, registerTerminalPasteTarget } from '@renderer/workspace/terminal/textPasteTarget' + +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' +import type { Workspace } from '@renderer/workspace/workspaceStore' +import type { SessionRuntime } from '@renderer/session-runtime/state' +import type { SessionId } from '@renderer/workspace/types' + +// The dispatch matrix (#830): composer draft for rendered agent panes, +// bracketed paste without Enter for every focused PTY (plain terminals +// and agent panes in terminal view share window.api.sendInput). +// window.api is stubbed because the real bridge only exists in the +// packaged app. + +// Typed boolean: main's sendInput resolves false when the write is dropped +// (missing backend / reserved) — the helper must react to that. +const sendInput = vi.fn(async (_id: string, _data: string) => true) +const ensureSessionLive = vi.fn(async () => {}) +const setDraftInput = vi.fn() +const registrations: (() => void)[] = [] +afterEach(() => { registrations.splice(0).forEach(dispose => dispose()) }) + +function makeRuntime(overrides: Partial = {}): SessionRuntime { + return { draftInput: '', processStatus: 'started', ...overrides } as unknown as SessionRuntime +} + +type SessionFixture = { kind: string; override?: 'agent' | 'terminal' } + +function makeWorkspace( + sessions: Record, + runtimes: Record, +): Workspace { + for (const id of Object.keys(sessions)) { + registrations.push(registerTerminalPasteTarget(id, { + isActive: () => true, + paste: text => sendInput(id, encodeTerminalPaste(text, true)), + })) + } + return { + state: { + sessions: Object.fromEntries( + Object.entries(sessions).map(([id, s]) => [ + id, + { id, kind: s.kind, agentViewModeOverride: s.override }, + ]), + ), + }, + getRuntime: (id: SessionId) => runtimes[id], + setDraftInput, + ensureSessionLive, + } as unknown as Workspace +} + +beforeEach(() => { + sendInput.mockReset().mockResolvedValue(true) + ensureSessionLive.mockClear() + setDraftInput.mockClear() +}) + +describe('deliverTextToSession', () => { + it('appends to the composer draft for a rendered agent pane', async () => { + const workspace = makeWorkspace( + { a: { kind: 'claude', override: 'agent' } }, + { a: makeRuntime({ draftInput: 'existing' }) }, + ) + const result = await deliverTextToSession(workspace, 'a', 'new text', { insertMode: 'append' }) + expect(result).toEqual({ delivered: true, surface: 'composer' }) + expect(setDraftInput).toHaveBeenCalledWith('a', 'existing\n\nnew text') + expect(sendInput).not.toHaveBeenCalled() + }) + + it('honors replace insert mode on the composer path', async () => { + const workspace = makeWorkspace( + { a: { kind: 'claude', override: 'agent' } }, + { a: makeRuntime({ draftInput: 'existing' }) }, + ) + await deliverTextToSession(workspace, 'a', 'replacement', { insertMode: 'replace' }) + expect(setDraftInput).toHaveBeenCalledWith('a', 'replacement') + }) + + it('bracket-pastes without Enter into a plain terminal pane', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime() }) + const result = await deliverTextToSession(workspace, 't', 'line1\nline2') + expect(result).toEqual({ delivered: true, surface: 'pty' }) + expect(sendInput).toHaveBeenCalledWith('t', '\x1b[200~line1\nline2\x1b[201~') + expect(sendInput.mock.calls[0][1].endsWith('\r')).toBe(false) + }) + + it('bracket-pastes into an agent pane in terminal view', async () => { + const workspace = makeWorkspace( + { a: { kind: 'claude', override: 'terminal' } }, + { a: makeRuntime() }, + ) + const result = await deliverTextToSession(workspace, 'a', 'key') + expect(result).toEqual({ delivered: true, surface: 'pty' }) + expect(sendInput).toHaveBeenCalledWith('a', '\x1b[200~key\x1b[201~') + }) + + it('wakes a sleeping backend before writing to a PTY', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime({ processStatus: 'exited' }) }) + await deliverTextToSession(workspace, 't', 'x') + expect(ensureSessionLive).toHaveBeenCalledWith('t', 'session-text-delivery', { awaitInputReady: false }) + expect(sendInput).toHaveBeenCalled() + }) + + it('does not wake an already-started backend', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime() }) + await deliverTextToSession(workspace, 't', 'x') + expect(ensureSessionLive).not.toHaveBeenCalled() + }) + + it('reports no-session for an unknown target', async () => { + const workspace = makeWorkspace({}, {}) + const result = await deliverTextToSession(workspace, 'gone' as SessionId, 'x') + expect(result).toEqual({ delivered: false, reason: 'no-session' }) + }) + + it('reports a refused write without retrying into a potentially changed process', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime() }) + sendInput.mockResolvedValueOnce(false).mockResolvedValueOnce(true) + const result = await deliverTextToSession(workspace, 't', 'x') + expect(result).toEqual({ delivered: false, reason: 'write-rejected' }) + expect(sendInput).toHaveBeenCalledTimes(1) + expect(ensureSessionLive).not.toHaveBeenCalled() + }) + + it('surfaces write-rejected when the write is dropped', async () => { + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime() }) + sendInput.mockResolvedValue(false) + const result = await deliverTextToSession(workspace, 't', 'x') + expect(result).toEqual({ delivered: false, reason: 'write-rejected' }) + }) + + it('normalizes a legacy session without kind like TileTree does', async () => { + // Legacy persisted sessions can lack `kind`; undefined must not read + // as "rendered" (review finding). With a terminal override the text + // must reach the PTY. + const workspace = makeWorkspace( + { a: { kind: undefined as unknown as string, override: 'terminal' } }, + { a: makeRuntime() }, + ) + const result = await deliverTextToSession(workspace, 'a', 'x') + expect(result).toEqual({ delivered: true, surface: 'pty' }) + }) + + it('cancels a pending wake when the picker closes or the vault locks', async () => { + let finishWake!: () => void + let valid = true + ensureSessionLive.mockImplementationOnce(() => new Promise(resolve => { finishWake = resolve })) + const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime({ processStatus: 'idle' }) }) + const pending = deliverTextToSession(workspace, 't', 'credential', { isCurrent: () => valid }) + valid = false + finishWake() + expect(await pending).toEqual({ delivered: false, reason: 'cancelled' }) + expect(sendInput).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/features/session-text-delivery/deliverTextToSession.ts b/src/renderer/src/features/session-text-delivery/deliverTextToSession.ts new file mode 100644 index 00000000..d1e1d0e6 --- /dev/null +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.ts @@ -0,0 +1,102 @@ +import { useAppStore } from '@renderer/app-state/hooks' +import { applyPromptTemplateInsertMode } from '@renderer/features/prompt-templates/interpolate' +import { getEffectiveAgentSurfaceForSession } from '@renderer/workspace/agentDisplayMode' +import { isSessionExited } from '@renderer/workspace/providerSessionIdentity' +import { DEFAULT_PROVIDER } from '@shared/types/providerKind' +import { getTerminalPasteTarget } from '@renderer/workspace/terminal/textPasteTarget' +import type { SessionId } from '@renderer/workspace/types' +import type { Workspace } from '@renderer/workspace/workspaceStore' + +// Session text delivery (#830): the ONE routing point for programmatic +// text insertion into a pane. Prompt Templates and the API Key Vault +// both call this instead of hand-rolling per-feature paths. +// +// Dispatch rule — mirror what the user sees: +// * rendered agent surface → composer draft edit (never submits; the +// draft stays visible and editable, matching template insertion's +// "prefill, don't replay" contract) +// * anything with a visible PTY (plain terminal pane, or agent pane +// in terminal view) -> paste through that mounted terminal owner, +// which checks its live paste mode and attach/replay state. +// +// WHY no Enter on the PTY path: the user must review what landed before +// it executes. Bracketed paste markers additionally keep shells like +// zsh from executing multi-line payloads line-by-line. A program that has +// not enabled bracketed paste receives single-line text only. Multiline +// input is refused rather than risking accidental command execution. +// +// SECRET DISCLOSURE (review finding): text inserted into a composer +// draft follows the SAME persistence rules as any draft — it autosaves +// to workspace.json in plaintext until sent or cleared. Inserted into a +// PTY it lands in scrollback (and tmux history). That is inherent to +// insertion itself, not this helper: once the user submits, the secret +// reaches the provider transcript in plaintext anyway. The VAULT's +// encryption contract covers storage, not the prompt pipeline. + +export type DeliverTextResult = + | { delivered: true; surface: 'composer' | 'pty' } + | { delivered: false; reason: 'no-session' | 'write-rejected' | 'cancelled' } + +export async function deliverTextToSession( + workspace: Workspace, + sessionId: SessionId, + text: string, + opts?: { insertMode?: 'replace' | 'append'; isCurrent?: () => boolean }, +): Promise { + if (opts?.isCurrent && !opts.isCurrent()) return { delivered: false, reason: 'cancelled' } + const session = workspace.state.sessions[sessionId] + if (!session) return { delivered: false, reason: 'no-session' } + + // WHY normalize kind (review finding): legacy persisted sessions may + // lack `kind`. TileTree normalizes missing kinds to the default agent + // provider before asking the surface policy; doing the same here keeps + // this helper's dispatch identical to what the pane actually renders — + // an undefined kind must not silently mean "rendered". + const kind = session.kind ?? DEFAULT_PROVIDER + + if (kind !== 'terminal') { + const surface = getEffectiveAgentSurfaceForSession({ + kind, + providerRuntime: session.providerRuntime, + globalMode: useAppStore.getState().settings.agentViewMode, + override: session.agentViewModeOverride, + runtime: workspace.getRuntime(sessionId), + }) + if (surface === 'rendered') { + const currentDraft = workspace.getRuntime(sessionId).draftInput + workspace.setDraftInput( + sessionId, + opts?.insertMode ? applyPromptTemplateInsertMode(currentDraft, text, opts.insertMode) : currentDraft + text, + ) + return { delivered: true, surface: 'composer' } + } + } + return deliverPtyText(workspace, sessionId, text, opts?.isCurrent) +} + +async function deliverPtyText( + workspace: Workspace, + sessionId: SessionId, + text: string, + isCurrent?: () => boolean, +): Promise { + const runtime = workspace.getRuntime(sessionId) + const target = getTerminalPasteTarget(sessionId) + if (!target) return { delivered: false, reason: 'write-rejected' } + // WHY wake first: lazily-woken restored sessions may have no main-side + // backend yet, and sendInput into a missing backend is silently + // dropped. Same predicate and no input-ready wait as AgentTerminalLeaf + // (#772): readiness is a composer concept, not a PTY one. + if (runtime.processStatus !== 'started' || isSessionExited(runtime)) { + await workspace.ensureSessionLive(sessionId, 'session-text-delivery', { awaitInputReady: false }) + } + // Auth/picker lifetime can end DURING wake. Checking only in the caller + // would still paste after Escape or Lock now if the backend starts late. + if (isCurrent && !isCurrent()) return { delivered: false, reason: 'cancelled' } + // Do not follow a changed/mirrored target after wake or retry into a + // replacement process. A refused write keeps the picker open for the user. + if (getTerminalPasteTarget(sessionId) === target && await target.paste(text)) { + return { delivered: true, surface: 'pty' } + } + return { delivered: false, reason: 'write-rejected' } +} diff --git a/src/renderer/src/workspace/terminal/textPasteTarget.test.ts b/src/renderer/src/workspace/terminal/textPasteTarget.test.ts new file mode 100644 index 00000000..34bbad1a --- /dev/null +++ b/src/renderer/src/workspace/terminal/textPasteTarget.test.ts @@ -0,0 +1,25 @@ +import { describe, expect, it } from 'vitest' +import { encodeTerminalPaste, registerTerminalPasteTarget, getTerminalPasteTarget } from './textPasteTarget' + +describe('terminal text insertion', () => { + it('wraps multiline text only when the application enables bracketed paste', () => { + expect(encodeTerminalPaste('one\r\ntwo', true)).toBe('\x1b[200~one\ntwo\x1b[201~') + expect(() => encodeTerminalPaste('one\ntwo', false)).toThrow(/multiline/i) + expect(encodeTerminalPaste('single line', false)).toBe('single line') + }) + it('refuses control bytes that could terminate a paste or execute a command', () => { + expect(() => encodeTerminalPaste('safe\x1b[201~\rcommand', true)).toThrow(/control/i) + expect(() => encodeTerminalPaste('text\x03', false)).toThrow(/control/i) + expect(() => encodeTerminalPaste('text\t', false)).toThrow(/control/i) + }) + it('selects only the active view and cannot revive a disposed target', () => { + const target = { isActive: () => true, paste: async () => true } + const hidden = { isActive: () => false, paste: async () => true } + const remove = registerTerminalPasteTarget('a', target) + const removeHidden = registerTerminalPasteTarget('a', hidden) + expect(getTerminalPasteTarget('a')).toBe(target) + remove() + expect(getTerminalPasteTarget('a')).toBeNull() + removeHidden() + }) +}) diff --git a/src/renderer/src/workspace/terminal/textPasteTarget.ts b/src/renderer/src/workspace/terminal/textPasteTarget.ts new file mode 100644 index 00000000..76d2ed67 --- /dev/null +++ b/src/renderer/src/workspace/terminal/textPasteTarget.ts @@ -0,0 +1,37 @@ +export type TerminalPasteTarget = { + isActive(): boolean + paste(text: string): Promise +} + +// Only the mounted xterm knows whether the application enabled bracketed +// paste, and only its leaf knows whether attach/replay has finished. A raw +// sendInput from a command bypasses both. Register that existing owner rather +// than adding a second terminal parser or inferring readiness from agent state. +const targets = new Map>() + +export function registerTerminalPasteTarget(sessionId: string, target: TerminalPasteTarget): () => void { + const group = targets.get(sessionId) ?? new Set() + group.add(target) + targets.set(sessionId, group) + return () => { + group.delete(target) + if (!group.size && targets.get(sessionId) === group) targets.delete(sessionId) + } +} + +export function getTerminalPasteTarget(sessionId: string): TerminalPasteTarget | null { + return [...(targets.get(sessionId) ?? [])].find(target => target.isActive()) ?? null +} + +export function encodeTerminalPaste(text: string, bracketed: boolean): string { + const normalized = text.replace(/\r\n?/g, '\n') + // Embedded ESC can close a bracketed paste; other control bytes can invoke + // terminal actions. Refuse rather than silently change a credential/prompt. + if (/[\x00-\x08\x0b-\x1f\x7f-\x9f]/.test(normalized) || (!bracketed && normalized.includes('\t'))) { + throw new Error('Paste contains terminal control characters; no input was sent.') + } + if (!bracketed && normalized.includes('\n')) { + throw new Error('This terminal program has not enabled bracketed paste; multiline input was not sent.') + } + return bracketed ? `\x1b[200~${normalized}\x1b[201~` : normalized +} diff --git a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx index 5d854a0e..573caaac 100644 --- a/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/AgentTerminalLeaf.tsx @@ -21,6 +21,7 @@ import { subscribeToAgentPtyData } from '@renderer/workspace/terminal/sessionDat import { attachXtermWebglRenderer } from '@renderer/workspace/terminal/xtermWebglRenderer' import { AgentTitleHeader } from '@renderer/workspace/tile-tree/AgentTitleHeader' import { createTerminalInputForwarder } from '@renderer/workspace/tile-tree/terminalInputForwarder' +import { encodeTerminalPaste, registerTerminalPasteTarget } from '@renderer/workspace/terminal/textPasteTarget' import { AgentTerminalActions } from '@renderer/workspace/tile-tree/AgentTerminalActions' import { useAgentTerminalFollow } from '@renderer/workspace/tile-tree/agentTerminalFollow' @@ -142,6 +143,7 @@ export function AgentTerminalLeaf({ // Nullable like the disposables above: xterm init can throw before the // follow wiring ever runs, and cleanup must survive that path. let offFollowAttach: (() => void) | null = null + let offTextPaste: (() => void) | null = null let resizeObserver: ResizeObserver | null = null let resizeFrame: number | null = null let disposed = false @@ -275,6 +277,13 @@ export function AgentTerminalLeaf({ const forwarder = createTerminalInputForwarder(data => { void window.api.sendInput(sessionId, data) }) + offTextPaste = registerTerminalPasteTarget(sessionId, { + isActive: () => !disposed && focusedRef.current && dimensionActiveRef.current, + paste: async text => { + if (disposed || !dimensionActiveRef.current || !attachedBackfillDone || forwarder.replaying || !term) return false + return window.api.sendInput(sessionId, encodeTerminalPaste(text, term.modes.bracketedPasteMode)) + }, + }) // WHY the Submit button reuses the keypress pipeline instead of calling // window.api.sendInput directly: the leaf only forwards keystrokes AFTER // attach (pendingInput) and only outside the replay window (the @@ -499,6 +508,7 @@ export function AgentTerminalLeaf({ onDataDisposable?.dispose() offFollowAttach?.() offPtyData?.() + offTextPaste?.() webglRenderer?.dispose() if (onThemeChangedListener) { window.removeEventListener(THEME_CHANGED_EVENT, onThemeChangedListener) diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx index 2db8ab3b..ce693d94 100644 --- a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx @@ -14,6 +14,7 @@ import { } from '@renderer/app-state/settings/theme' import { readXtermTheme, syncXtermTheme } from '@renderer/workspace/tile-tree/xtermTheme' import { createTerminalInputForwarder } from '@renderer/workspace/tile-tree/terminalInputForwarder' +import { encodeTerminalPaste, registerTerminalPasteTarget } from '@renderer/workspace/terminal/textPasteTarget' import { subscribeToTerminalData } from '@renderer/workspace/terminal/sessionDataDispatcher' import { attachXtermWebglRenderer } from '@renderer/workspace/terminal/xtermWebglRenderer' @@ -147,6 +148,7 @@ export function TerminalLeaf({ let webglRenderer: ReturnType | null = null let onDataDisposable: { dispose(): void } | null = null let offTerminalData: (() => void) | null = null + let offTextPaste: (() => void) | null = null let resizeObserver: ResizeObserver | null = null let resizeFrame: number | null = null let disposed = false @@ -278,6 +280,13 @@ export function TerminalLeaf({ const forwarder = createTerminalInputForwarder(data => { void window.api.sendInput(sessionId, data) }) + offTextPaste = registerTerminalPasteTarget(sessionId, { + isActive: () => !disposed && focusedRef.current && ownerVisibleRef.current, + paste: async text => { + if (disposed || !ownerVisibleRef.current || !attachedBackfillDone || forwarder.replaying || !term) return false + return window.api.sendInput(sessionId, encodeTerminalPaste(text, term.modes.bracketedPasteMode)) + }, + }) onDataDisposable = term.onData(data => { if (forwarder.replaying) return if (!attachedBackfillDone) { @@ -434,6 +443,7 @@ export function TerminalLeaf({ resizeObserver?.disconnect() onDataDisposable?.dispose() offTerminalData?.() + offTextPaste?.() webglRenderer?.dispose() if (onThemeChangedListenerRef) { window.removeEventListener(THEME_CHANGED_EVENT, onThemeChangedListenerRef) diff --git a/src/shared/lifecycle/events.ts b/src/shared/lifecycle/events.ts index 2fc56db0..348134f6 100644 --- a/src/shared/lifecycle/events.ts +++ b/src/shared/lifecycle/events.ts @@ -522,6 +522,11 @@ export const WAKE_CALLERS = [ 'orchestration.read-agent', 'orchestration.send-prompt', 'control.send-prompt', + // Programmatic text delivery (#830: prompt templates / API key vault + // inserting into a PTY surface). Same family as the MCP-driven callers + // above — not a direct human gesture, but always downstream of one + // (a palette action), and rare enough that a storm means a stuck loop. + 'session-text-delivery', ] as const export type WakeCaller = (typeof WAKE_CALLERS)[number] diff --git a/src/shared/types/keyVault.ts b/src/shared/types/keyVault.ts new file mode 100644 index 00000000..7ca65fe4 --- /dev/null +++ b/src/shared/types/keyVault.ts @@ -0,0 +1,57 @@ +// Wire contract for the API Key Vault (issue #831). +// +// WHY a shared module: vault data crosses the preload bridge in both +// directions (renderer edits metadata, main returns snapshots), so the +// renderer must type its UI against something importable from both +// processes without dragging main-process storage code into the bundle. +// Mirrors the other @shared/types contracts. + +/** A user-created provider bucket (e.g. "Brave", "OpenAI"). The vault + * starts empty by design (#831): no seed list of providers to go stale + * as services appear and disappear. */ +export type KeyVaultProvider = { + id: string + name: string + createdAt: number + updatedAt: number +} + +/** Non-secret metadata for one stored key. The secret value NEVER crosses + * this type: it lives in an encrypted per-key blob on disk and is only + * returned by the gated reveal/copy calls. `hint` is the last four + * characters of the value, captured at write time, so the vault UI can + * confirm identity without decrypting anything. */ +export type KeyVaultKey = { + id: string + providerId: string + name: string + note: string + hint: string + createdAt: number + updatedAt: number +} + +export type KeyVaultSnapshot = { + providers: KeyVaultProvider[] + keys: KeyVaultKey[] +} + +export type KeyVaultStatus = { + /** Electron safeStorage reports the OS keyring usable. */ + encryptionAvailable: boolean + /** macOS can present the Touch ID / login-password prompt. */ + authPromptAvailable: boolean + /** The once-per-app-run unlock gate has been passed. */ + unlocked: boolean +} + +export type KeyVaultKeyInput = { + providerId: string + /** Omit to create a new key; include to update an existing one. An + * empty `value` on update means "keep the existing secret" so users + * can rename/re-note without re-pasting the key. */ + id?: string + name: string + value: string + note: string +}