From f69cfb3f814afe44843524aeedd0ea3a5fe3010c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:22:00 -0700 Subject: [PATCH 01/11] docs(vault): plan api key vault and session text delivery Implementation plan for #830/#831: encrypted per-key vault store with a touch-id once-per-run gate, renderer-owned session text delivery with bracket-paste-without-submit over any focused PTY, and named key references in prompt templates. First file on the branch per conventions. Refs #830, Refs #831 --- .../plans/2026-09-07-api-key-vault.md | 1957 +++++++++++++++++ 1 file changed, 1957 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-07-api-key-vault.md 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..44626254 --- /dev/null +++ b/docs/superpowers/plans/2026-09-07-api-key-vault.md @@ -0,0 +1,1957 @@ +# API Key Vault + Session Text Delivery Implementation Plan + +> **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. From 56f1d5220b092e4772dbf58d4d4e86df4640ceb3 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:22:00 -0700 Subject: [PATCH 02/11] docs(vault): record api key vault design decisions Approved design: per-key safeStorage blobs over whole-vault encryption, promptTouchID gate in main failing closed, renderer-owned text delivery, name-based template refs resolved pre-fill. Refs #830, Refs #831 --- .../specs/2026-09-07-api-key-vault-design.md | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 docs/superpowers/specs/2026-09-07-api-key-vault-design.md 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..541aaa48 --- /dev/null +++ b/docs/superpowers/specs/2026-09-07-api-key-vault-design.md @@ -0,0 +1,54 @@ +# 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()`. Production wires `promptAuth` to `systemPreferences.promptTouchID`, which presents Touch ID with the login-password fallback (the "mac password" requirement). Unsigned dev builds may skip biometry; the password path still works. If prompting is impossible, the vault fails closed — no secret leaves main. Snapshots never contain secrets; revealed plaintext exists only in ephemeral renderer component state. + +### D3 — Renderer-owned `deliverTextToSession` (chosen over main-owned paste IPC) + +One helper dispatches by session kind + effective surface (`getEffectiveAgentSurfaceForSession`): rendered agent panes append to the composer draft via `setDraftInput`; plain terminals and agent terminal views write a bracketed paste (`ESC[200~ … ESC[201~`, no Enter) through `window.api.sendInput` — the same channel both surfaces already use for keystrokes — after a lazy-wake when the backend is missing. The renderer owns focus/workspace/surface context; main stays provider-agnostic. Bracketed-paste markers keep shells from executing multi-line payloads; the trade-off (programs that never enabled the mode print the marker bytes) is accepted over raw newlines, which shells would run immediately. + +### D4 — Template key references by name, resolved pre-fill + +`{{key:Provider Name/Key Name}}` (grammar deliberately disjoint from `{{variable}}`, so refs never become fill-pane fields). Names over ids keep templates readable; renames break references loudly. Resolution happens before variable fill so secrets never render in the fill pane; any unresolved ref aborts insertion with a toast naming all failures. Renames of providers/keys are uniqueness-checked to keep references unambiguous. + +## 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. +- Bracketed paste into programs that never enable the mode shows marker bytes — documented trade-off (D3). +- Renaming vault providers/keys breaks template references by design; failures are loud, never silent. From 40a923cb5c91c4b934145a6e70c65ec8f59810eb Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:40:56 -0700 Subject: [PATCH 03/11] 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 --- src/main/keyVault/vaultStore.test.ts | 94 +++++++++++++++++++ src/main/keyVault/vaultStore.ts | 131 +++++++++++++++++++++++++++ src/shared/types/keyVault.ts | 57 ++++++++++++ 3 files changed, 282 insertions(+) create mode 100644 src/main/keyVault/vaultStore.test.ts create mode 100644 src/main/keyVault/vaultStore.ts create mode 100644 src/shared/types/keyVault.ts diff --git a/src/main/keyVault/vaultStore.test.ts b/src/main/keyVault/vaultStore.test.ts new file mode 100644 index 00000000..9c14a5dc --- /dev/null +++ b/src/main/keyVault/vaultStore.test.ts @@ -0,0 +1,94 @@ +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 => { + // 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('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) + }) +}) diff --git a/src/main/keyVault/vaultStore.ts b/src/main/keyVault/vaultStore.ts new file mode 100644 index 00000000..b64d2e5e --- /dev/null +++ b/src/main/keyVault/vaultStore.ts @@ -0,0 +1,131 @@ +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 treated the + // same way below: the vault degrades to empty rather than + // bricking startup. Secret blobs on disk become orphans, which + // is the safe direction — the metadata loss already happened + // when the index corrupted, and a hard failure here would make + // the whole app unusable over data the user cannot recover + // through this path anyway. + return { providers: [], keys: [] } + } + try { + const parsed = JSON.parse(raw) as IndexFile + return { providers: parsed.providers ?? [], keys: parsed.keys ?? [] } + } catch { + 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 }) + }, + } +} 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 +} From d738ce5cb8fbf291279a8e626ce52c10b15f9828 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:41:55 -0700 Subject: [PATCH 04/11] 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 --- src/main/keyVault/VaultService.test.ts | 150 ++++++++++++++++ src/main/keyVault/VaultService.ts | 228 +++++++++++++++++++++++++ 2 files changed, 378 insertions(+) create mode 100644 src/main/keyVault/VaultService.test.ts create mode 100644 src/main/keyVault/VaultService.ts diff --git a/src/main/keyVault/VaultService.test.ts b/src/main/keyVault/VaultService.test.ts new file mode 100644 index 00000000..61a85d5c --- /dev/null +++ b/src/main/keyVault/VaultService.test.ts @@ -0,0 +1,150 @@ +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 { + 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 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 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. + 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) + }) +}) diff --git a/src/main/keyVault/VaultService.ts b/src/main/keyVault/VaultService.ts new file mode 100644 index 00000000..7db292f2 --- /dev/null +++ b/src/main/keyVault/VaultService.ts @@ -0,0 +1,228 @@ +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() + 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 { + 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.`) + } + + 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 = 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 + } +} From 607ae9181542e3f9299dd4cfcdf0c4d53e3e2825 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:45:45 -0700 Subject: [PATCH 05/11] 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. The safeStorage codec lives in its own module so vaultStore stays unit-testable without the packaged electron module. Secrets only cross on gated reveal/copy/resolve-ref returns. Refs #831 --- src/main/index.ts | 19 +++++++++++++++++- src/main/ipc/index.ts | 4 ++++ src/main/ipc/keyVault.ts | 28 ++++++++++++++++++++++++++ src/main/keyVault/safeStorageCodec.ts | 23 +++++++++++++++++++++ src/preload/api/index.ts | 2 ++ src/preload/api/keyVault.ts | 29 +++++++++++++++++++++++++++ 6 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 src/main/ipc/keyVault.ts create mode 100644 src/main/keyVault/safeStorageCodec.ts create mode 100644 src/preload/api/keyVault.ts diff --git a/src/main/index.ts b/src/main/index.ts index 24cdbf5d..b5390934 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,19 @@ 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), + canPromptAuth: () => systemPreferences.canPromptTouchID(), + 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 +960,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..ca8f89fc --- /dev/null +++ b/src/main/ipc/keyVault.ts @@ -0,0 +1,28 @@ +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)) +} 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/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..3129d8f9 --- /dev/null +++ b/src/preload/api/keyVault.ts @@ -0,0 +1,29 @@ +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), +} From 17f759e533ccd78048b5d78fbb28fb5712770c8d Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:46:24 -0700 Subject: [PATCH 06/11] 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 --- .../deliverTextToSession.renderer.test.ts | 107 ++++++++++++++++++ .../deliverTextToSession.ts | 75 ++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts create mode 100644 src/renderer/src/features/session-text-delivery/deliverTextToSession.ts 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..ace817c8 --- /dev/null +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts @@ -0,0 +1,107 @@ +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 () => {}) +const setDraftInput = vi.fn() + +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 { + 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.mockClear() + ensureSessionLive.mockClear() + setDraftInput.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(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: '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' }) + }) +}) 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..ee329675 --- /dev/null +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.ts @@ -0,0 +1,75 @@ +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' } +} From d44c69061800b34ab163e0386546c947a8148f57 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:48:46 -0700 Subject: [PATCH 07/11] 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), and composer-draft commands (save-as-template) stay agent-only. Refs #830 --- .../command-palette/ui/CommandPalette.tsx | 42 ++++++++++--------- .../commands/promptTemplateCommands.ts | 15 +++++-- .../prompt-templates/targetSession.test.ts | 16 ++++++- .../prompt-templates/targetSession.ts | 21 +++++++--- 4 files changed, 63 insertions(+), 31 deletions(-) diff --git a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx index 59b86ace..67f0eaa3 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -56,9 +56,9 @@ import { allPromptTemplates, } from '@renderer/features/prompt-templates/templates' import { - applyPromptTemplateInsertMode, fillPromptTemplateBody, } from '@renderer/features/prompt-templates/interpolate' +import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' import { createSavedPromptTemplate, duplicatePromptTemplate, @@ -1298,15 +1298,19 @@ 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 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') + } } catch (err) { const message = err instanceof Error ? err.message : String(err) workspace.showPaneToast(sessionId, `Template failed: ${message}`) @@ -1440,7 +1444,7 @@ function OpenCommandPalette({ [customPromptTemplates, setSettings], ) - const insertFilledPromptTemplate = useCallback(() => { + const insertFilledPromptTemplate = useCallback(async () => { const fill = promptTemplateFillState if (!fill) return const sessionId = commandTargetSessionId(workspace) @@ -1451,13 +1455,13 @@ function OpenCommandPalette({ variables: fill.template.variables, values: fill.values, }) - const currentDraft = workspace.getRuntime(sessionId).draftInput - workspace.setDraftInput( - sessionId, - applyPromptTemplateInsertMode(currentDraft, resolved, fill.insertMode), - ) - workspace.showPaneToast(sessionId, `Inserted template: ${fill.template.title}`) - onClose() + 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') + } } catch (error) { const message = error instanceof Error ? error.message : String(error) workspace.showPaneToast(sessionId, `Template failed: ${message}`) @@ -1510,7 +1514,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/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/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 From 9f932fab396e02c9d39c348b77e4ac7a96828a0b Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:53:13 -0700 Subject: [PATCH 08/11] 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. Adds the session-text-delivery wake caller to the lifecycle allowlist. Refs #831 --- .../command-palette/ui/CommandPalette.tsx | 18 +++++- .../prompt-templates/keyReferences.test.ts | 41 +++++++++++++ .../prompt-templates/keyReferences.ts | 57 +++++++++++++++++++ .../deliverTextToSession.renderer.test.ts | 4 +- src/shared/lifecycle/events.ts | 5 ++ 5 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/features/prompt-templates/keyReferences.test.ts create mode 100644 src/renderer/src/features/prompt-templates/keyReferences.ts diff --git a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx index 67f0eaa3..21596f06 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -58,6 +58,7 @@ import { import { fillPromptTemplateBody, } from '@renderer/features/prompt-templates/interpolate' +import { collectKeyReferences, resolveKeyReferences } from '@renderer/features/prompt-templates/keyReferences' import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' import { createSavedPromptTemplate, @@ -1283,9 +1284,24 @@ function OpenCommandPalette({ if (!sessionId) return try { - const body = template.buildBody + let body = template.buildBody ? await template.buildBody({ workspace, sessionId }) : template.body + // Vault key references (#831): resolve BEFORE the variables check + // 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) { + body = 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 + } + }) + } if (template.variables.length > 0) { setPromptTemplateFillState({ template: template.buildBody ? { ...template, body } : template, 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..5ea242d9 --- /dev/null +++ b/src/renderer/src/features/prompt-templates/keyReferences.test.ts @@ -0,0 +1,41 @@ +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/) + }) + + 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..99a4b8c2 --- /dev/null +++ b/src/renderer/src/features/prompt-templates/keyReferences.ts @@ -0,0 +1,57 @@ +// 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 BEFORE variable fill and the fill pane never sees a secret. + +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/session-text-delivery/deliverTextToSession.renderer.test.ts b/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts index ace817c8..3e30007d 100644 --- a/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts @@ -11,7 +11,7 @@ import type { SessionId } from '@renderer/workspace/types' // window.api is stubbed because the real bridge only exists in the // packaged app. -const sendInput = vi.fn(async () => {}) +const sendInput = vi.fn(async (_id: string, _data: string) => {}) const ensureSessionLive = vi.fn(async () => {}) const setDraftInput = vi.fn() @@ -87,7 +87,7 @@ describe('deliverTextToSession', () => { }) it('wakes a sleeping backend before writing to a PTY', async () => { - const workspace = makeWorkspace({ t: { kind: 'terminal' } }, { t: makeRuntime({ processStatus: 'stopped' }) }) + 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() 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] From db813163c13e3abd638e0a7e0f567bd07af2a839 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 15:57:53 -0700 Subject: [PATCH 09/11] 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 and is dropped on close. Refs #831 --- src/renderer/src/app-state/types.ts | 2 + src/renderer/src/app-state/uiShell/slice.ts | 5 + src/renderer/src/app-state/uiShell/types.ts | 3 + src/renderer/src/app/surfaces/registry.tsx | 2 + .../src/features/command-palette/catalog.ts | 2 + .../src/features/command-palette/types.ts | 1 + .../command-palette/ui/CommandPalette.tsx | 3 + .../key-vault/commands/keyVaultCommands.ts | 16 + .../surfaces/KeyVaultModalSurface.tsx | 8 + .../features/key-vault/ui/KeyVaultModal.tsx | 336 ++++++++++++++++++ 10 files changed, 378 insertions(+) create mode 100644 src/renderer/src/features/key-vault/commands/keyVaultCommands.ts create mode 100644 src/renderer/src/features/key-vault/surfaces/KeyVaultModalSurface.tsx create mode 100644 src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx 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/features/command-palette/catalog.ts b/src/renderer/src/features/command-palette/catalog.ts index b7585560..2496ecdf 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,6 +79,7 @@ export const builtInCommandCatalog: readonly CommandDef[] = Object.freeze([ ...copyAssistantCommands, ...copyCodeBlockCommands, ...promptTemplateCommands, + ...keyVaultCommands, // Grouped with the prompt-template commands because it is the other // composer-insertion command — registry order is the palette's // empty-query browse order, so like things stay adjacent. 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 21596f06..48b7fc97 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -293,6 +293,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) @@ -629,6 +630,7 @@ function OpenCommandPalette({ closePinAgents, closePathPicker, openUsageModal, + openKeyVault, toggleGitBar, toggleWorktreesBar, toggleDebugPanel, @@ -737,6 +739,7 @@ function OpenCommandPalette({ closePinAgents, closePathPicker, openUsageModal, + openKeyVault, toggleGitBar, toggleWorktreesBar, toggleDebugPanel, 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/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.tsx b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx new file mode 100644 index 00000000..06e0007b --- /dev/null +++ b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx @@ -0,0 +1,336 @@ +import { useCallback, useEffect, 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 { 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 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. + +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; never persisted. + 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 unmounts. + 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(() => window.api.keyVaultCreateProvider(name)) + } + + 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)) + setStatus(prev => (prev ? { ...prev, unlocked: true } : prev)) + 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 (!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}
+ )} + +
+
+ {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 +
+
+
+ ) +} From 5052db1fbe824819ee6767490df44f6cdffac276 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 17:07:17 -0700 Subject: [PATCH 10/11] fix(vault): update catalog baseline and feature references The api-key-vault command is the 115th governed command; bump the characterization arithmetic (102 - 5 retired + 18 additions). Document the key-vault feature reference and map session-text-delivery as infrastructure owned by the prompt-templates page. Refs #830, Refs #831 --- src/renderer/src/control/featureReference.ts | 6 ++++ .../features/command-palette/catalog.test.ts | 28 ++++++++++--------- .../features/key-vault/controlReference.ts | 23 +++++++++++++++ 3 files changed, 44 insertions(+), 13 deletions(-) create mode 100644 src/renderer/src/features/key-vault/controlReference.ts 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/key-vault/controlReference.ts b/src/renderer/src/features/key-vault/controlReference.ts new file mode 100644 index 00000000..152fbb07 --- /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 and never persisted renderer-side. 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. Insertion into terminals is a bracketed paste WITHOUT Enter — review before submitting.", + "commandIds": [ + "api-key-vault" + ] + } +] satisfies FeatureReference[] From 003951efdba9159773c5dd876de5dba57232294d Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Mon, 7 Sep 2026 19:25:18 -0700 Subject: [PATCH 11/11] fix(vault): enforce lock revocation and safe text insertion Resolve Claude/Codex review findings with cancellation-safe secret reads, serialized vault writes, damaged-index preservation, mounted-terminal paste ownership, and post-fill template resolution. Reuse the app workspace controller and clear plaintext caches on lock across windows. Document vault-only at-rest protection and downstream plaintext draft/transcript retention rather than claiming end-to-end prompt secrecy. Add regression coverage for races, failed writes, control bytes, source braces, and modal flows. Refs #830, #831 --- .../plans/2026-09-07-api-key-vault.md | 16 ++ .../specs/2026-09-07-api-key-vault-design.md | 12 +- src/main/index.ts | 5 +- src/main/ipc/keyVault.ts | 36 ++-- src/main/keyVault/VaultService.test.ts | 126 ++++++++++++- src/main/keyVault/VaultService.ts | 139 ++++++++++++--- src/main/keyVault/vaultStore.test.ts | 37 +++- src/main/keyVault/vaultStore.ts | 69 ++++--- src/preload/api/keyVault.ts | 2 + .../src/features/command-palette/catalog.ts | 4 +- .../command-palette/ui/CommandPalette.tsx | 99 ++++++++--- .../features/key-vault/controlReference.ts | 2 +- .../ui/KeyVaultModal.renderer.test.tsx | 65 +++++++ .../features/key-vault/ui/KeyVaultModal.tsx | 168 ++++++++++++++---- .../prompt-templates/controlReference.ts | 2 +- .../prompt-templates/keyReferences.test.ts | 15 +- .../prompt-templates/keyReferences.ts | 19 +- .../deliverTextToSession.renderer.test.ts | 58 +++++- .../deliverTextToSession.ts | 55 ++++-- .../terminal/textPasteTarget.test.ts | 25 +++ .../src/workspace/terminal/textPasteTarget.ts | 37 ++++ .../workspace/tile-tree/AgentTerminalLeaf.tsx | 10 ++ .../src/workspace/tile-tree/TerminalLeaf.tsx | 10 ++ 23 files changed, 854 insertions(+), 157 deletions(-) create mode 100644 src/renderer/src/features/key-vault/ui/KeyVaultModal.renderer.test.tsx create mode 100644 src/renderer/src/workspace/terminal/textPasteTarget.test.ts create mode 100644 src/renderer/src/workspace/terminal/textPasteTarget.ts diff --git a/docs/superpowers/plans/2026-09-07-api-key-vault.md b/docs/superpowers/plans/2026-09-07-api-key-vault.md index 44626254..cc8e2d7e 100644 --- a/docs/superpowers/plans/2026-09-07-api-key-vault.md +++ b/docs/superpowers/plans/2026-09-07-api-key-vault.md @@ -1,5 +1,21 @@ # 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. 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 index 541aaa48..4c23946d 100644 --- a/docs/superpowers/specs/2026-09-07-api-key-vault-design.md +++ b/docs/superpowers/specs/2026-09-07-api-key-vault-design.md @@ -25,15 +25,17 @@ ### D2 — Unlock gate in main via `systemPreferences.promptTouchID`, once per run -Every secret-leaving path (reveal, copy, template ref resolution) funnels through one `ensureUnlocked()`. Production wires `promptAuth` to `systemPreferences.promptTouchID`, which presents Touch ID with the login-password fallback (the "mac password" requirement). Unsigned dev builds may skip biometry; the password path still works. If prompting is impossible, the vault fails closed — no secret leaves main. Snapshots never contain secrets; revealed plaintext exists only in ephemeral renderer component state. +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 session kind + effective surface (`getEffectiveAgentSurfaceForSession`): rendered agent panes append to the composer draft via `setDraftInput`; plain terminals and agent terminal views write a bracketed paste (`ESC[200~ … ESC[201~`, no Enter) through `window.api.sendInput` — the same channel both surfaces already use for keystrokes — after a lazy-wake when the backend is missing. The renderer owns focus/workspace/surface context; main stays provider-agnostic. Bracketed-paste markers keep shells from executing multi-line payloads; the trade-off (programs that never enabled the mode print the marker bytes) is accepted over raw newlines, which shells would run immediately. +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 pre-fill +### D4 — Template key references by name, resolved at insertion -`{{key:Provider Name/Key Name}}` (grammar deliberately disjoint from `{{variable}}`, so refs never become fill-pane fields). Names over ids keep templates readable; renames break references loudly. Resolution happens before variable fill so secrets never render in the fill pane; any unresolved ref aborts insertion with a toast naming all failures. Renames of providers/keys are uniqueness-checked to keep references unambiguous. +`{{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 @@ -50,5 +52,5 @@ One helper dispatches by session kind + effective surface (`getEffectiveAgentSur ## 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. -- Bracketed paste into programs that never enable the mode shows marker bytes — documented trade-off (D3). +- 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 b5390934..b2e97955 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -206,7 +206,10 @@ const caffeinateController = new CaffeinateController() const vaultService = new VaultService({ store: createFileVaultStore(join(STATE_DIR, 'key-vault'), createSafeStorageCodec()), promptAuth: reason => systemPreferences.promptTouchID(reason), - canPromptAuth: () => systemPreferences.canPromptTouchID(), + // 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), }) diff --git a/src/main/ipc/keyVault.ts b/src/main/ipc/keyVault.ts index ca8f89fc..103ecf5f 100644 --- a/src/main/ipc/keyVault.ts +++ b/src/main/ipc/keyVault.ts @@ -2,14 +2,30 @@ 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' -// 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. +// 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', () => vaultService.list()) + 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) => @@ -18,11 +34,11 @@ export function registerKeyVaultIpc({ vaultService }: { vaultService: VaultServi 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:create-provider', (_event, name: string) => unlocked(() => vaultService.createProvider(parse(text, 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)) + 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) => - vaultService.deleteKey(providerId, keyId)) + 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 index 61a85d5c..432d2ec6 100644 --- a/src/main/keyVault/VaultService.test.ts +++ b/src/main/keyVault/VaultService.test.ts @@ -4,6 +4,12 @@ import { VaultService, type VaultServiceDeps } from '@main/keyVault/VaultService 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. @@ -59,9 +65,98 @@ describe('VaultService unlock gate', () => { 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 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 () => { @@ -120,6 +215,14 @@ describe('VaultService CRUD', () => { 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) @@ -137,14 +240,19 @@ describe('VaultService CRUD', () => { 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 })) + // 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: '' }) - // 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 + // 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 index 7db292f2..83447dcb 100644 --- a/src/main/keyVault/VaultService.ts +++ b/src/main/keyVault/VaultService.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import type { KeyVaultKey, KeyVaultKeyInput, @@ -20,8 +21,8 @@ import { newVaultId, type VaultStore } from '@main/keyVault/vaultStore.js' // // 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. +// 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 @@ -34,10 +35,42 @@ export type VaultServiceDeps = { now?: () => number } -export class VaultService { +/** 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) {} + constructor(private readonly deps: VaultServiceDeps) { super() } getStatus(): KeyVaultStatus { return { @@ -49,24 +82,56 @@ export class VaultService { 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 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.)', - ) + 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.') } - await this.deps.promptAuth('unlock the Agent Code API key vault') - this.unlocked = true + } + + /** 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 { @@ -74,9 +139,11 @@ export class VaultService { } 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.`, @@ -86,7 +153,9 @@ export class VaultService { } 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) } @@ -94,6 +163,7 @@ export class VaultService { * 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) @@ -101,6 +171,7 @@ export class VaultService { 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.`, @@ -110,8 +181,11 @@ export class VaultService { } async createProvider(name: string): Promise { - const trimmed = name.trim() - if (!trimmed) throw new Error('Provider name cannot be empty.') + 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.`) @@ -124,8 +198,11 @@ export class VaultService { } async renameProvider(id: string, name: string): Promise { - const trimmed = name.trim() - if (!trimmed) throw new Error('Provider name cannot be empty.') + 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.') @@ -138,6 +215,10 @@ export class VaultService { } 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 @@ -153,14 +234,20 @@ export class VaultService { } async putKey(input: KeyVaultKeyInput): Promise { - const name = input.name.trim() - if (!name) throw new Error('Key name cannot be empty.') + 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) : undefined + 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( @@ -178,7 +265,7 @@ export class VaultService { // 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 = value.slice(-4) + 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. @@ -198,7 +285,7 @@ export class VaultService { providerId: input.providerId, name, note: input.note.trim(), - hint: value.slice(-4), + hint: hintFor(value), createdAt: now, updatedAt: now, } @@ -209,6 +296,10 @@ export class VaultService { } 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.') diff --git a/src/main/keyVault/vaultStore.test.ts b/src/main/keyVault/vaultStore.test.ts index 9c14a5dc..61f63501 100644 --- a/src/main/keyVault/vaultStore.test.ts +++ b/src/main/keyVault/vaultStore.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, readFile, writeFile } from 'node:fs/promises' +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' @@ -52,6 +52,27 @@ describe('vaultStore', () => { 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() @@ -88,7 +109,17 @@ describe('vaultStore', () => { 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) + 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 index b64d2e5e..32450691 100644 --- a/src/main/keyVault/vaultStore.ts +++ b/src/main/keyVault/vaultStore.ts @@ -1,6 +1,7 @@ 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' @@ -37,6 +38,19 @@ export function newVaultId(): string { 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 @@ -53,17 +67,27 @@ export function createFileVaultStore( 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 }) - 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(() => {}) + 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 { @@ -73,21 +97,21 @@ export function createFileVaultStore( let raw: string try { raw = await readFile(indexFile, 'utf8') - } catch { - // Absent index = fresh vault. A CORRUPT index is treated the - // same way below: the vault degrades to empty rather than - // bricking startup. Secret blobs on disk become orphans, which - // is the safe direction — the metadata loss already happened - // when the index corrupted, and a hard failure here would make - // the whole app unusable over data the user cannot recover - // through this path anyway. - return { providers: [], keys: [] } + } 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 = JSON.parse(raw) as IndexFile - return { providers: parsed.providers ?? [], keys: parsed.keys ?? [] } + 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 { - return { providers: [], keys: [] } + // 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.') } }, @@ -102,10 +126,11 @@ export function createFileVaultStore( }, async readSecret(keyId) { + const path = secretPath(keyId) if (!codec.isEncryptionAvailable()) return null let cipher: Buffer try { - cipher = await readFile(join(keysDir, `${keyId}.bin`)) + cipher = await readFile(path) } catch { return null } @@ -121,11 +146,13 @@ export function createFileVaultStore( }, async writeSecret(keyId, value) { - await atomicWrite(join(keysDir, `${keyId}.bin`), codec.encrypt(value), 0o600) + 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(join(keysDir, `${keyId}.bin`), { force: true }) + await rm(secretPath(keyId), { force: true }) }, } } diff --git a/src/preload/api/keyVault.ts b/src/preload/api/keyVault.ts index 3129d8f9..61da48b4 100644 --- a/src/preload/api/keyVault.ts +++ b/src/preload/api/keyVault.ts @@ -1,4 +1,5 @@ import { ipcRenderer } from 'electron' +import { subscribe } from '@preload/api/ipc.js' import type { KeyVaultKeyInput, KeyVaultSnapshot, KeyVaultStatus } from '@shared/types/keyVault' @@ -6,6 +7,7 @@ import type { KeyVaultKeyInput, KeyVaultSnapshot, KeyVaultStatus } from '@shared // 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'), diff --git a/src/renderer/src/features/command-palette/catalog.ts b/src/renderer/src/features/command-palette/catalog.ts index 2496ecdf..5418d710 100644 --- a/src/renderer/src/features/command-palette/catalog.ts +++ b/src/renderer/src/features/command-palette/catalog.ts @@ -80,8 +80,8 @@ export const builtInCommandCatalog: readonly CommandDef[] = Object.freeze([ ...copyCodeBlockCommands, ...promptTemplateCommands, ...keyVaultCommands, - // Grouped with the prompt-template commands because it is the other - // composer-insertion command — registry order is the palette's + // 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/ui/CommandPalette.tsx b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx index 48b7fc97..1d30e52d 100644 --- a/src/renderer/src/features/command-palette/ui/CommandPalette.tsx +++ b/src/renderer/src/features/command-palette/ui/CommandPalette.tsx @@ -55,11 +55,9 @@ import type { PendingCommandInvocation } from '@renderer/app-state/uiShell/types import { allPromptTemplates, } from '@renderer/features/prompt-templates/templates' -import { - fillPromptTemplateBody, -} from '@renderer/features/prompt-templates/interpolate' -import { collectKeyReferences, resolveKeyReferences } from '@renderer/features/prompt-templates/keyReferences' +import { prepareTemplateText } from '@renderer/features/prompt-templates/keyReferences' import { deliverTextToSession } from '@renderer/features/session-text-delivery/deliverTextToSession' + import { createSavedPromptTemplate, duplicatePromptTemplate, @@ -120,6 +118,7 @@ type BuriedPaneInfo = { } type PromptTemplateFillState = { + sessionId: string template: PromptTemplate values: PromptTemplateVariableValueMap insertMode: PromptTemplateInsertMode @@ -254,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. @@ -1285,29 +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 { - let body = template.buildBody + const body = template.buildBody ? await template.buildBody({ workspace, sessionId }) : template.body - // Vault key references (#831): resolve BEFORE the variables check - // 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) { - body = 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 - } - }) - } + 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), @@ -1323,16 +1325,34 @@ function OpenCommandPalette({ // paste for any PTY surface. Nothing is sent until they press // Enter themselves, mirroring rewind-to-prompt's "prefill, don't // replay" contract. - const result = await deliverTextToSession(workspace, sessionId, body, { insertMode: template.insertMode }) + 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], @@ -1466,24 +1486,45 @@ function OpenCommandPalette({ 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 result = await deliverTextToSession(workspace, sessionId, resolved, { insertMode: fill.insertMode }) + // 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, + text, + { insertMode: fill.insertMode, isCurrent: () => templateActive.current && + useAppStore.getState().workspaceState.sessions[sessionId] === originalSession && + workspace.getRuntime(sessionId).draftInput === originalDraft }, + ) 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]) diff --git a/src/renderer/src/features/key-vault/controlReference.ts b/src/renderer/src/features/key-vault/controlReference.ts index 152fbb07..6a8912ae 100644 --- a/src/renderer/src/features/key-vault/controlReference.ts +++ b/src/renderer/src/features/key-vault/controlReference.ts @@ -15,7 +15,7 @@ export const controlReference = [ "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 and never persisted renderer-side. 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. Insertion into terminals is a bracketed paste WITHOUT Enter — review before submitting.", + "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" ] 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 index 06e0007b..953b29cf 100644 --- a/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx +++ b/src/renderer/src/features/key-vault/ui/KeyVaultModal.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { Button } from '@renderer/components/ui/button' import { @@ -11,21 +11,34 @@ import { 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 { 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 state — never in the persisted app store, never in a -// journal — and is dropped on close. Metadata flows through the +// 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) - const workspace = useWorkspace() + // 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([]) @@ -33,43 +46,73 @@ export function KeyVaultModal() { // 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)) } }, []) - useEffect(() => { void refresh() }, [refresh]) - useEffect(() => { - // Drop all plaintext the moment the modal unmounts. - return () => setRevealed(new Map()) + 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() - 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)) + } finally { + actionInFlight.current = false } } @@ -83,50 +126,78 @@ export function KeyVaultModal() { const saveKeyForm = () => { const form = keyForm if (!form || !selectedProviderId) return - setKeyForm(null) - void runVaultAction(() => window.api.keyVaultPutKey({ + 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 sessionId = commandTargetSessionId(workspace) + 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 { - const value = revealed.get(key.id) ?? await window.api.keyVaultReveal(key.providerId, key.id) - const result = await deliverTextToSession(workspace, sessionId, value) + // 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 } } @@ -151,7 +222,12 @@ export function KeyVaultModal() { @@ -167,8 +243,9 @@ export function KeyVaultModal() {
{error}
)} -
-
+ {!status?.unlocked && } + {status?.unlocked &&
+
{providers.map(provider => ( @@ -236,9 +308,41 @@ export function KeyVaultModal() {
+ {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) && ( @@ -326,9 +430,11 @@ export function KeyVaultModal() {
+ }
Reference keys from prompt templates with {'{{key:Provider/Key}}'} · Encrypted with the OS - keyring · One unlock per app launch + 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/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 index 5ea242d9..e071d721 100644 --- a/src/renderer/src/features/prompt-templates/keyReferences.test.ts +++ b/src/renderer/src/features/prompt-templates/keyReferences.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest' -import { collectKeyReferences, resolveKeyReferences } from '@renderer/features/prompt-templates/keyReferences' +import { collectKeyReferences, resolveKeyReferences, prepareTemplateText } from '@renderer/features/prompt-templates/keyReferences' describe('collectKeyReferences', () => { it('collects and dedupes references', () => { @@ -19,6 +19,19 @@ describe('collectKeyReferences', () => { }) 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}}', diff --git a/src/renderer/src/features/prompt-templates/keyReferences.ts b/src/renderer/src/features/prompt-templates/keyReferences.ts index 99a4b8c2..1d46da22 100644 --- a/src/renderer/src/features/prompt-templates/keyReferences.ts +++ b/src/renderer/src/features/prompt-templates/keyReferences.ts @@ -10,7 +10,24 @@ // 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 BEFORE variable fill and the fill pane never sees a secret. +// 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 } 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 index 3e30007d..b81d463a 100644 --- a/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.renderer.test.ts @@ -1,4 +1,5 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +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' @@ -11,9 +12,13 @@ import type { SessionId } from '@renderer/workspace/types' // window.api is stubbed because the real bridge only exists in the // packaged app. -const sendInput = vi.fn(async (_id: string, _data: string) => {}) +// 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 @@ -25,6 +30,12 @@ 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( @@ -41,10 +52,9 @@ function makeWorkspace( } beforeEach(() => { - sendInput.mockClear() + sendInput.mockReset().mockResolvedValue(true) ensureSessionLive.mockClear() setDraftInput.mockClear() - ;(globalThis as { window?: unknown }).window = { api: { sendInput } } }) describe('deliverTextToSession', () => { @@ -104,4 +114,44 @@ describe('deliverTextToSession', () => { 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 index ee329675..d1e1d0e6 100644 --- a/src/renderer/src/features/session-text-delivery/deliverTextToSession.ts +++ b/src/renderer/src/features/session-text-delivery/deliverTextToSession.ts @@ -2,6 +2,8 @@ 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' @@ -14,32 +16,47 @@ import type { Workspace } from '@renderer/workspace/workspaceStore' // 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 +// 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. 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. +// 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' } + | { delivered: false; reason: 'no-session' | 'write-rejected' | 'cancelled' } export async function deliverTextToSession( workspace: Workspace, sessionId: SessionId, text: string, - opts?: { insertMode?: 'replace' | 'append' }, + 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' } - if (session.kind !== 'terminal') { + // 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: session.kind, + kind, providerRuntime: session.providerRuntime, globalMode: useAppStore.getState().settings.agentViewMode, override: session.agentViewModeOverride, @@ -49,20 +66,23 @@ export async function deliverTextToSession( const currentDraft = workspace.getRuntime(sessionId).draftInput workspace.setDraftInput( sessionId, - applyPromptTemplateInsertMode(currentDraft, text, opts?.insertMode ?? 'append'), + opts?.insertMode ? applyPromptTemplateInsertMode(currentDraft, text, opts.insertMode) : currentDraft + text, ) return { delivered: true, surface: 'composer' } } } - return deliverPtyText(workspace, sessionId, text) + 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 @@ -70,6 +90,13 @@ async function deliverPtyText( 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' } + // 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)