diff --git a/apps/desktop/src/main/__tests__/credential-store-contract.test.ts b/apps/desktop/src/main/__tests__/credential-store-contract.test.ts deleted file mode 100644 index 5dc4bfcf4a..0000000000 --- a/apps/desktop/src/main/__tests__/credential-store-contract.test.ts +++ /dev/null @@ -1,58 +0,0 @@ -import assert from 'node:assert/strict'; -import { readFileSync } from 'node:fs'; -import { join } from 'node:path'; -import { describe, it } from 'node:test'; - -const source = readFileSync(join(process.cwd(), 'src/main/credential-store.ts'), 'utf8'); - -describe('credential store secret-kind expansion contract', () => { - it('keeps legacy connection secret key names backward compatible', () => { - assert.match(source, /case 'api_key':\s*return 'apiKey';/); - assert.match(source, /case 'oauth_token':\s*return 'oauthToken';/); - }); - - it('declares the Phase 1 settings secret kinds', () => { - for (const kind of [ - 'bot_token', - 'app_secret', - 'proxy_password', - 'gateway_token', - 'tavily_api_key', - ]) { - assert.match(source, new RegExp(`'${kind}'`), `${kind} must be a public CredentialKind`); - } - }); - - it('exposes typed helpers for future settings migration without changing settings consumers', () => { - for (const method of [ - 'getBotToken', - 'setBotToken', - 'deleteBotToken', - 'getBotAppSecret', - 'setBotAppSecret', - 'deleteBotAppSecret', - 'getProxyPassword', - 'setProxyPassword', - 'deleteProxyPassword', - 'getGatewayToken', - 'setGatewayToken', - 'deleteGatewayToken', - 'getTavilyApiKey', - 'setTavilyApiKey', - 'deleteTavilyApiKey', - ]) { - assert.match(source, new RegExp(`${method}\\(`), `${method} must be present`); - } - }); - - it('uses deterministic non-secret slugs for provider-scoped and global settings secrets', () => { - assert.match(source, /const BOT_SECRET_SLUG_PREFIX = 'settings:bot';/); - assert.match(source, /function botSecretSlug\(provider: BotProvider\): string \{\s*return `\$\{BOT_SECRET_SLUG_PREFIX\}:\$\{provider\}`;\s*\}/); - assert.match(source, /const GLOBAL_PROXY_SECRET_SLUG = 'settings:network-proxy';/); - assert.match(source, /const GLOBAL_GATEWAY_SECRET_SLUG = 'settings:open-gateway';/); - assert.match(source, /const GLOBAL_TAVILY_SECRET_SLUG = 'settings:web-search:tavily';/); - assert.doesNotMatch(source, /token\}:/, 'raw bot tokens must not be interpolated into key names'); - assert.doesNotMatch(source, /password\}:/, 'raw proxy passwords must not be interpolated into key names'); - assert.doesNotMatch(source, /secret\}:/, 'raw app secrets must not be interpolated into key names'); - }); -}); diff --git a/apps/desktop/src/main/__tests__/credential-store-migration.test.ts b/apps/desktop/src/main/__tests__/credential-store-migration.test.ts new file mode 100644 index 0000000000..31201b6b3d --- /dev/null +++ b/apps/desktop/src/main/__tests__/credential-store-migration.test.ts @@ -0,0 +1,80 @@ +import assert from 'node:assert/strict'; +import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, it } from 'node:test'; +import { migrateLegacyCredentials, type SafeStorageLike } from '../credential-store.js'; + +// The migration ORCHESTRATION — shared lock, version gate, fail-closed aborts, +// atomic 0600 rewrite, idempotency, malformed/missing handling — is tested in +// @maka/storage against migrateLegacyCredentialFile. This file covers only the +// desktop GLUE: that safeStorage is wired through correctly (base64-decode then +// decryptString) and that its availability is propagated. + +/** Fake safeStorage: "decrypt" strips an `enc:` prefix so every value is proven + * to round-trip through decryptString. */ +function fakeSafeStorage(available: boolean): SafeStorageLike { + return { + isEncryptionAvailable: () => available, + decryptString: (buf) => buf.toString('utf8').replace(/^enc:/, ''), + }; +} + +/** The legacy on-disk encoding: base64(safeStorage.encryptString(value)). */ +function encrypted(value: string): string { + return Buffer.from(`enc:${value}`).toString('base64'); +} + +async function withWorkspace(fn: (root: string, path: string) => Promise): Promise { + const root = await mkdtemp(join(tmpdir(), 'maka-cred-mig-')); + try { + return await fn(root, join(root, 'credentials.json')); + } finally { + await rm(root, { recursive: true, force: true }); + } +} + +describe('migrateLegacyCredentials (desktop safeStorage glue)', () => { + it('base64-decodes and safeStorage-decrypts every kind to v1 plaintext-0600', async () => { + await withWorkspace(async (root, path) => { + // Full scope: API key + bot token + proxy password all migrate. + await writeFile( + path, + JSON.stringify({ + values: { + 'openai:apiKey': encrypted('sk-1'), + 'settings:bot:telegram:botToken': encrypted('tok-2'), + 'settings:network-proxy:proxyPassword': encrypted('pw-3'), + }, + }), + 'utf8', + ); + + await migrateLegacyCredentials(root, fakeSafeStorage(true)); + + const after = JSON.parse(await readFile(path, 'utf8')) as { + version: number; + values: Record; + }; + assert.equal(after.version, 1); + assert.deepEqual(after.values, { + 'openai:apiKey': 'sk-1', + 'settings:bot:telegram:botToken': 'tok-2', + 'settings:network-proxy:proxyPassword': 'pw-3', + }); + if (process.platform !== 'win32') { + assert.equal((await stat(path)).mode & 0o777, 0o600); // owner-only at rest + } + }); + }); + + it('propagates safeStorage unavailability: aborts and leaves the file intact', async () => { + await withWorkspace(async (root, path) => { + const original = JSON.stringify({ values: { 'openai:apiKey': encrypted('sk-1') } }); + await writeFile(path, original, 'utf8'); + + await assert.rejects(migrateLegacyCredentials(root, fakeSafeStorage(false)), /unavailable/); + assert.equal(await readFile(path, 'utf8'), original); // untouched — no data loss + }); + }); +}); diff --git a/apps/desktop/src/main/__tests__/credential-store-secret-kinds-contract.test.ts b/apps/desktop/src/main/__tests__/credential-store-secret-kinds-contract.test.ts index 5baf452349..c0fda03d7c 100644 --- a/apps/desktop/src/main/__tests__/credential-store-secret-kinds-contract.test.ts +++ b/apps/desktop/src/main/__tests__/credential-store-secret-kinds-contract.test.ts @@ -2,7 +2,6 @@ import assert from 'node:assert/strict'; import { readFile } from 'node:fs/promises'; import { join, resolve } from 'node:path'; import { describe, it } from 'node:test'; -import type { BotProvider } from '@maka/core'; import type { CredentialKind, CredentialStore } from '../credential-store.js'; const repoRoot = process.cwd().endsWith(join('apps', 'desktop')) @@ -13,6 +12,11 @@ async function readRepo(relativePath: string): Promise { return readFile(join(repoRoot, relativePath), 'utf8'); } +// Compile-time guard: the desktop entrypoint still consumes the shared +// CredentialStore contract (re-exported from @maka/storage). The store +// implementation and its kind/slug invariants — legacy stored-key names, +// no raw secret in key names, fail-closed reads — are now tested +// behaviourally in packages/storage/src/__tests__/credential-store.test.ts. const credentialKinds: CredentialKind[] = [ 'api_key', 'oauth_token', @@ -22,113 +26,33 @@ const credentialKinds: CredentialKind[] = [ 'gateway_token', 'tavily_api_key', ]; - -type Phase4CredentialStoreMethods = Pick< - CredentialStore, - | 'getBotToken' - | 'setBotToken' - | 'deleteBotToken' - | 'getBotAppSecret' - | 'setBotAppSecret' - | 'deleteBotAppSecret' - | 'getProxyPassword' - | 'setProxyPassword' - | 'deleteProxyPassword' - | 'getGatewayToken' - | 'setGatewayToken' - | 'deleteGatewayToken' - | 'getTavilyApiKey' - | 'setTavilyApiKey' - | 'deleteTavilyApiKey' ->; - -type BotScopedSetter = (provider: BotProvider, secret: string) => Promise; -const botScopedSetter: BotScopedSetter = null as unknown as Phase4CredentialStoreMethods['setBotToken']; - +const secretReader = null as unknown as Pick; void credentialKinds; -void botScopedSetter; - -describe('credential-store secret kind expansion contract', () => { - it('exposes the Phase 4 credential kinds without changing legacy stored kind names', async () => { - const source = await readRepo('apps/desktop/src/main/credential-store.ts'); - - for (const kind of credentialKinds) { - assert.match(source, new RegExp(`'${kind}'`), `CredentialKind must include ${kind}`); - } - - assert.match(source, /case 'api_key':\s*return 'apiKey';/, 'api_key must keep the legacy stored key suffix'); - assert.match(source, /case 'oauth_token':\s*return 'oauthToken';/, 'oauth_token must keep the legacy stored key suffix'); - assert.match(source, /getApiKey\(slug: string\)[\s\S]*?return this\.get\(slug, 'apiKey'\);/); - assert.match(source, /setApiKey\(slug: string, apiKey: string\)[\s\S]*?return this\.set\(slug, 'apiKey', apiKey\);/); - assert.match(source, /getOAuthToken\(slug: string\)[\s\S]*?return this\.get\(slug, 'oauthToken'\);/); - assert.match(source, /setOAuthToken\(slug: string, token: string\)[\s\S]*?return this\.set\(slug, 'oauthToken', token\);/); - assert.match(source, /private key\(slug: string, kind: StoredCredentialKind\): string \{\s*return `\$\{slug\}:\$\{kind\}`;\s*\}/); - }); - - it('scopes bot token and app-secret helpers by provider and kind', async () => { - const source = await readRepo('apps/desktop/src/main/credential-store.ts'); +void secretReader; - assert.match(source, /const BOT_SECRET_SLUG_PREFIX = 'settings:bot';/); - assert.match(source, /function botSecretSlug\(provider: BotProvider\): string \{\s*return `\$\{BOT_SECRET_SLUG_PREFIX\}:\$\{provider\}`;\s*\}/); - assert.match(source, /getBotToken\(provider: BotProvider\)[\s\S]*return this\.get\(botSecretSlug\(provider\), 'botToken'\);/); - assert.match(source, /setBotToken\(provider: BotProvider, token: string\)[\s\S]*return this\.set\(botSecretSlug\(provider\), 'botToken', token\);/); - assert.match(source, /deleteBotToken\(provider: BotProvider\)[\s\S]*return this\.deleteSecret\(botSecretSlug\(provider\), 'bot_token'\);/); - assert.match(source, /getBotAppSecret\(provider: BotProvider\)[\s\S]*return this\.get\(botSecretSlug\(provider\), 'botAppSecret'\);/); - assert.match(source, /setBotAppSecret\(provider: BotProvider, secret: string\)[\s\S]*return this\.set\(botSecretSlug\(provider\), 'botAppSecret', secret\);/); - assert.match(source, /deleteBotAppSecret\(provider: BotProvider\)[\s\S]*return this\.deleteSecret\(botSecretSlug\(provider\), 'app_secret'\);/); - assert.doesNotMatch(source, /botSecretSlug\([^)]*(token|secret|value|apiKey|password|key)[^)]*\)/); - }); - - it('uses isolated singleton slugs for global secret helpers', async () => { - const source = await readRepo('apps/desktop/src/main/credential-store.ts'); - - assert.match(source, /const GLOBAL_PROXY_SECRET_SLUG = 'settings:network-proxy';/); - assert.match(source, /const GLOBAL_GATEWAY_SECRET_SLUG = 'settings:open-gateway';/); - assert.match(source, /const GLOBAL_TAVILY_SECRET_SLUG = 'settings:web-search:tavily';/); - assert.match(source, /getProxyPassword\(\)[\s\S]*return this\.get\(GLOBAL_PROXY_SECRET_SLUG, 'proxyPassword'\);/); - assert.match(source, /setProxyPassword\(password: string\)[\s\S]*return this\.set\(GLOBAL_PROXY_SECRET_SLUG, 'proxyPassword', password\);/); - assert.match(source, /deleteProxyPassword\(\)[\s\S]*return this\.deleteSecret\(GLOBAL_PROXY_SECRET_SLUG, 'proxy_password'\);/); - assert.match(source, /getGatewayToken\(\)[\s\S]*return this\.get\(GLOBAL_GATEWAY_SECRET_SLUG, 'gatewayToken'\);/); - assert.match(source, /setGatewayToken\(token: string\)[\s\S]*return this\.set\(GLOBAL_GATEWAY_SECRET_SLUG, 'gatewayToken', token\);/); - assert.match(source, /deleteGatewayToken\(\)[\s\S]*return this\.deleteSecret\(GLOBAL_GATEWAY_SECRET_SLUG, 'gateway_token'\);/); - assert.match(source, /getTavilyApiKey\(\)[\s\S]*return this\.get\(GLOBAL_TAVILY_SECRET_SLUG, 'tavilyApiKey'\);/); - assert.match(source, /setTavilyApiKey\(key: string\)[\s\S]*return this\.set\(GLOBAL_TAVILY_SECRET_SLUG, 'tavilyApiKey', key\);/); - assert.match(source, /deleteTavilyApiKey\(\)[\s\S]*return this\.deleteSecret\(GLOBAL_TAVILY_SECRET_SLUG, 'tavily_api_key'\);/); - }); - - it('keeps credential writes encrypted, fail-closed, and in the existing file shape', async () => { +describe('credential store migration off safeStorage (#32)', () => { + it('uses the pure-Node @maka/storage backend as the live store, not a safeStorage store', async () => { const source = await readRepo('apps/desktop/src/main/credential-store.ts'); - const setBlock = source.match(/private async set\([^)]*\): Promise \{[\s\S]*?\n \}/); - const getBlock = source.match(/private async get\([^)]*\): Promise \{[\s\S]*?\n \}/); - const readBlock = source.match(/private async readUnlocked\(\): Promise \{[\s\S]*?\n \}/); + // Contract + implementation are re-exported from the shared package. + assert.match(source, /from '@maka\/storage'/); + assert.match(source, /createFileCredentialStore/); + // No live get/set store class remains in the desktop file — safeStorage + // is used ONLY by the one-time importer below. + assert.doesNotMatch(source, /class \w*CredentialStore/); - assert.ok(setBlock, 'set helper must exist'); - assert.ok(getBlock, 'get helper must exist'); - assert.ok(readBlock, 'readUnlocked helper must exist'); - assert.match(setBlock![0], /if \(!safeStorage\.isEncryptionAvailable\(\)\) \{[\s\S]*throw new Error/); - assert.match(setBlock![0], /file\.values\[this\.key\(slug, kind\)\] = safeStorage\.encryptString\(value\)\.toString\('base64'\);/); - assert.match(getBlock![0], /if \(!encrypted\) return null;/); - assert.match(getBlock![0], /safeStorage\.decryptString\(Buffer\.from\(encrypted, 'base64'\)\)/); - assert.match(readBlock![0], /ENOENT'[\s\S]*return \{ values: \{\} \};/); - assert.match(source, /interface CredentialFile \{\s*values: Record;\s*\}/); - assert.match(source, /JSON\.stringify\(file, null, 2\) \+ '\\n'/); - assert.doesNotMatch(source, /writeFile\([^)]*value/); + const main = await readRepo('apps/desktop/src/main/main.ts'); + assert.match(main, /createFileCredentialStore\(workspaceRoot\)/); + assert.doesNotMatch(main, /createSafeStorageCredentialStore/); }); - it('keeps delete helpers idempotent and targeted', async () => { - const source = await readRepo('apps/desktop/src/main/credential-store.ts'); - const deleteSecretBlock = source.match(/async deleteSecret\([^)]*\): Promise \{[\s\S]*?\n \}/); - const deleteBlock = source.match(/async delete\(slug: string\): Promise \{[\s\S]*?\n \}/); - - assert.ok(deleteSecretBlock, 'deleteSecret helper must exist'); - assert.ok(deleteBlock, 'delete helper must exist'); - assert.match(deleteSecretBlock![0], /delete file\.values\[this\.key\(slug, toStoredKind\(kind\)\)\];/); - assert.doesNotMatch(deleteSecretBlock![0], /if \([^)]*values\[this\.key/, 'targeted delete should remain idempotent when missing'); - assert.match(deleteBlock![0], /for \(const kind of STORED_CREDENTIAL_KINDS\)/); - assert.match(source, /'apiKey'[\s\S]*'oauthToken'[\s\S]*'botToken'[\s\S]*'botAppSecret'[\s\S]*'proxyPassword'[\s\S]*'gatewayToken'[\s\S]*'tavilyApiKey'/); + it('runs the migration before any credential use, non-fatally', async () => { + const main = await readRepo('apps/desktop/src/main/main.ts'); + // Inside the whenReady handler, in a try/catch so a migration failure + // doesn't crash startup (later reads fail closed with guidance). + assert.match(main, /try \{\s*await migrateLegacyCredentials\(workspaceRoot, safeStorage\);\s*\} catch/); }); - it('does not start settings migration or change renderer-facing settings masking in this phase', async () => { + it('keeps renderer-facing settings masked and does not read bot/global secrets in main', async () => { const main = await readRepo('apps/desktop/src/main/main.ts'); const helpers = await readRepo('apps/desktop/src/main/settings-ipc-helpers.ts'); diff --git a/apps/desktop/src/main/credential-store.ts b/apps/desktop/src/main/credential-store.ts index 1627d79150..671fec330a 100644 --- a/apps/desktop/src/main/credential-store.ts +++ b/apps/desktop/src/main/credential-store.ts @@ -1,248 +1,50 @@ -import { safeStorage } from 'electron'; -import { mkdir, readFile, rename, writeFile } from 'node:fs/promises'; -import { dirname, join } from 'node:path'; -import type { BotProvider } from '@maka/core'; - -type StoredCredentialKind = - | 'apiKey' - | 'oauthToken' - | 'botToken' - | 'botAppSecret' - | 'proxyPassword' - | 'gatewayToken' - | 'tavilyApiKey'; -export type CredentialKind = - | 'api_key' - | 'oauth_token' - | 'bot_token' - | 'app_secret' - | 'proxy_password' - | 'gateway_token' - | 'tavily_api_key'; - -interface CredentialFile { - values: Record; -} - -export interface CredentialStore { - getSecret(slug: string, kind: CredentialKind): Promise; - setSecret(slug: string, kind: CredentialKind, value: string): Promise; - deleteSecret(slug: string, kind?: CredentialKind): Promise; - getBotToken(provider: BotProvider): Promise; - setBotToken(provider: BotProvider, token: string): Promise; - deleteBotToken(provider: BotProvider): Promise; - getBotAppSecret(provider: BotProvider): Promise; - setBotAppSecret(provider: BotProvider, secret: string): Promise; - deleteBotAppSecret(provider: BotProvider): Promise; - getProxyPassword(): Promise; - setProxyPassword(password: string): Promise; - deleteProxyPassword(): Promise; - getGatewayToken(): Promise; - setGatewayToken(token: string): Promise; - deleteGatewayToken(): Promise; - getTavilyApiKey(): Promise; - setTavilyApiKey(key: string): Promise; - deleteTavilyApiKey(): Promise; - getApiKey(slug: string): Promise; - getOAuthToken(slug: string): Promise; - setApiKey(slug: string, apiKey: string): Promise; - setOAuthToken(slug: string, token: string): Promise; - delete(slug: string): Promise; -} - -export function createSafeStorageCredentialStore(workspaceRoot: string): CredentialStore { - return new SafeStorageCredentialStore(join(workspaceRoot, 'credentials.json')); -} - -class SafeStorageCredentialStore implements CredentialStore { - private queue: Promise = Promise.resolve(); - - constructor(private readonly path: string) {} - - getSecret(slug: string, kind: CredentialKind): Promise { - return this.get(slug, toStoredKind(kind)); - } - - setSecret(slug: string, kind: CredentialKind, value: string): Promise { - return this.set(slug, toStoredKind(kind), value); - } - - async deleteSecret(slug: string, kind?: CredentialKind): Promise { - if (!kind) { - await this.delete(slug); - return; - } - await this.withQueue(async () => { - const file = await this.readUnlocked(); - delete file.values[this.key(slug, toStoredKind(kind))]; - await this.write(file); - }); - } - - getBotToken(provider: BotProvider): Promise { - return this.get(botSecretSlug(provider), 'botToken'); - } - - setBotToken(provider: BotProvider, token: string): Promise { - return this.set(botSecretSlug(provider), 'botToken', token); - } - - deleteBotToken(provider: BotProvider): Promise { - return this.deleteSecret(botSecretSlug(provider), 'bot_token'); - } - - getBotAppSecret(provider: BotProvider): Promise { - return this.get(botSecretSlug(provider), 'botAppSecret'); - } - - setBotAppSecret(provider: BotProvider, secret: string): Promise { - return this.set(botSecretSlug(provider), 'botAppSecret', secret); - } - - deleteBotAppSecret(provider: BotProvider): Promise { - return this.deleteSecret(botSecretSlug(provider), 'app_secret'); - } - - getProxyPassword(): Promise { - return this.get(GLOBAL_PROXY_SECRET_SLUG, 'proxyPassword'); - } - - setProxyPassword(password: string): Promise { - return this.set(GLOBAL_PROXY_SECRET_SLUG, 'proxyPassword', password); - } - - deleteProxyPassword(): Promise { - return this.deleteSecret(GLOBAL_PROXY_SECRET_SLUG, 'proxy_password'); - } - - getGatewayToken(): Promise { - return this.get(GLOBAL_GATEWAY_SECRET_SLUG, 'gatewayToken'); - } - - setGatewayToken(token: string): Promise { - return this.set(GLOBAL_GATEWAY_SECRET_SLUG, 'gatewayToken', token); - } - - deleteGatewayToken(): Promise { - return this.deleteSecret(GLOBAL_GATEWAY_SECRET_SLUG, 'gateway_token'); - } - - getTavilyApiKey(): Promise { - return this.get(GLOBAL_TAVILY_SECRET_SLUG, 'tavilyApiKey'); - } - - setTavilyApiKey(key: string): Promise { - return this.set(GLOBAL_TAVILY_SECRET_SLUG, 'tavilyApiKey', key); - } - - deleteTavilyApiKey(): Promise { - return this.deleteSecret(GLOBAL_TAVILY_SECRET_SLUG, 'tavily_api_key'); - } - - getApiKey(slug: string): Promise { - return this.get(slug, 'apiKey'); - } - - getOAuthToken(slug: string): Promise { - return this.get(slug, 'oauthToken'); - } - - setApiKey(slug: string, apiKey: string): Promise { - return this.set(slug, 'apiKey', apiKey); - } - - setOAuthToken(slug: string, token: string): Promise { - return this.set(slug, 'oauthToken', token); - } - - async delete(slug: string): Promise { - await this.withQueue(async () => { - const file = await this.readUnlocked(); - for (const kind of STORED_CREDENTIAL_KINDS) { - delete file.values[this.key(slug, kind)]; - } - await this.write(file); - }); - } - - private async get(slug: string, kind: StoredCredentialKind): Promise { - const encrypted = (await this.readUnlocked()).values[this.key(slug, kind)]; - if (!encrypted) return null; - return safeStorage.decryptString(Buffer.from(encrypted, 'base64')); - } - - private async set(slug: string, kind: StoredCredentialKind, value: string): Promise { - await this.withQueue(async () => { - if (!safeStorage.isEncryptionAvailable()) { - throw new Error('Electron safeStorage encryption is not available on this system.'); - } - const file = await this.readUnlocked(); - file.values[this.key(slug, kind)] = safeStorage.encryptString(value).toString('base64'); - await this.write(file); - }); - } - - private key(slug: string, kind: StoredCredentialKind): string { - return `${slug}:${kind}`; - } - - private async readUnlocked(): Promise { - try { - return JSON.parse(await readFile(this.path, 'utf8')) as CredentialFile; - } catch (error) { - if ((error as { code?: string }).code === 'ENOENT') return { values: {} }; - throw error; - } - } - - private async write(file: CredentialFile): Promise { - await mkdir(dirname(this.path), { recursive: true }); - const tempPath = `${this.path}.${process.pid}.${Date.now()}.tmp`; - await writeFile(tempPath, JSON.stringify(file, null, 2) + '\n', 'utf8'); - await rename(tempPath, this.path); - } - - private withQueue(operation: () => Promise): Promise { - const next = this.queue.then(operation, operation); - this.queue = next.catch(() => {}); - return next; - } -} - -const STORED_CREDENTIAL_KINDS = [ - 'apiKey', - 'oauthToken', - 'botToken', - 'botAppSecret', - 'proxyPassword', - 'gatewayToken', - 'tavilyApiKey', -] as const satisfies readonly StoredCredentialKind[]; - -const BOT_SECRET_SLUG_PREFIX = 'settings:bot'; -const GLOBAL_PROXY_SECRET_SLUG = 'settings:network-proxy'; -const GLOBAL_GATEWAY_SECRET_SLUG = 'settings:open-gateway'; -const GLOBAL_TAVILY_SECRET_SLUG = 'settings:web-search:tavily'; - -function botSecretSlug(provider: BotProvider): string { - return `${BOT_SECRET_SLUG_PREFIX}:${provider}`; +import { join } from 'node:path'; +import { createFileCredentialStore, migrateLegacyCredentialFile } from '@maka/storage'; + +/** + * The slice of Electron `safeStorage` the importer needs, injected so the + * migration can be tested without Electron — and so this module no longer + * imports `electron` at load time. main.ts passes the real `safeStorage`. + */ +export interface SafeStorageLike { + isEncryptionAvailable(): boolean; + decryptString(encrypted: Buffer): string; } -function toStoredKind(kind: CredentialKind): StoredCredentialKind { - switch (kind) { - case 'api_key': - return 'apiKey'; - case 'oauth_token': - return 'oauthToken'; - case 'bot_token': - return 'botToken'; - case 'app_secret': - return 'botAppSecret'; - case 'proxy_password': - return 'proxyPassword'; - case 'gateway_token': - return 'gatewayToken'; - case 'tavily_api_key': - return 'tavilyApiKey'; - } +// The credential store and its migration are pure-Node and live in +// @maka/storage so the headless runtime can read the same file (issue #32). +// Re-exported here so existing `./credential-store.js` importers keep their path. +export type { CredentialKind, CredentialStore } from '@maka/storage'; +export { CREDENTIAL_SCHEMA_VERSION, createFileCredentialStore } from '@maka/storage'; + +/** + * One-time migration off Electron `safeStorage` (issue #32). Desktop-only glue: + * it supplies the crypto — the legacy file stored each secret as + * `base64(safeStorage.encryptString(value))`, so here we base64-decode and + * `safeStorage.decryptString` each value — while @maka/storage owns the + * orchestration: the shared cross-process lock, the version gate, fail-closed + * aborts, the atomic 0600 rewrite, and the tombstone. A successful run leaves + * the file as shared v1 plaintext-0600, which the headless `FileCredentialStore` + * then reads. + * + * Scope — this migrates EVERY secret in `credentials.json`, not just API keys: + * bot tokens, bot app secrets, the proxy password, the gateway token, and the + * Tavily key all decrypt to plaintext too. That is required, not incidental — + * the desktop abandons `safeStorage` entirely (the live store is now the + * pure-Node `FileCredentialStore`), so any value left encrypted would become + * permanently unreadable. The accepted at-rest posture for all of them is + * plaintext behind 0600 (SECURITY.md / file-first, #32). + * + * If `safeStorage` is unavailable the migration aborts and leaves the encrypted + * file untouched (a later run migrates once it is available). main.ts runs this + * before any credential use, non-fatally, in `whenReady`. + */ +export function migrateLegacyCredentials( + workspaceRoot: string, + safeStorage: SafeStorageLike, +): Promise { + return migrateLegacyCredentialFile(join(workspaceRoot, 'credentials.json'), { + isAvailable: () => safeStorage.isEncryptionAvailable(), + decrypt: (storedValue) => safeStorage.decryptString(Buffer.from(storedValue, 'base64')), + }); } diff --git a/apps/desktop/src/main/main.ts b/apps/desktop/src/main/main.ts index bf6d998235..5ebe35fd49 100644 --- a/apps/desktop/src/main/main.ts +++ b/apps/desktop/src/main/main.ts @@ -1,4 +1,4 @@ -import { app, BrowserWindow, dialog, ipcMain, Menu, nativeTheme, screen, shell } from 'electron'; +import { app, BrowserWindow, dialog, ipcMain, Menu, nativeTheme, safeStorage, screen, shell } from 'electron'; import { isExternalUrl } from './external-link-guard.js'; import { readSavedBounds, writeSavedBounds, type SavedBounds } from './window-state.js'; import { createHash, randomUUID } from 'node:crypto'; @@ -166,7 +166,7 @@ import { errorReason, requireReadyConnection, } from './chat-readiness.js'; -import { createSafeStorageCredentialStore } from './credential-store.js'; +import { createFileCredentialStore, migrateLegacyCredentials } from './credential-store.js'; import { bindOnboardingDeps, createOnboardingService } from './onboarding-service.js'; import { handleQuickChatStart as runQuickChatStart, type QuickChatResult } from './quick-chat.js'; import { connectionTestStatusPatch } from './connection-test-status.js'; @@ -248,7 +248,7 @@ const settingsStore = createSettingsStore(workspaceRoot); const telemetryRepo = createTelemetryRepo(workspaceRoot); const artifactStore = createArtifactStore(workspaceRoot); const attachmentApprovals = createAttachmentApprovalRegistry(); -const credentialStore = createSafeStorageCredentialStore(workspaceRoot); +const credentialStore = createFileCredentialStore(workspaceRoot); // PR-OAUTH-SUBSCRIPTION-0: Claude subscription OAuth service. // Lives in main process only; renderer accesses via IPC. Tokens // never cross the IPC boundary (xuan G-X3). Cloak path is dynamic- @@ -4061,6 +4061,15 @@ async function ensureBootstrapConnection(): Promise { registerIpc(); app.whenReady().then(async () => { + // One-time migration of credentials.json off Electron safeStorage so + // the pure-Node runtime can read it (issue #32). Runs before any + // credential read/write below; failure is non-fatal (legacy file is + // left intact and later credential reads fail closed with guidance). + try { + await migrateLegacyCredentials(workspaceRoot, safeStorage); + } catch (error) { + console.error('[credentials] migration off safeStorage failed; legacy file left intact:', error); + } if (visualSmokeFixture) { console.log(`[visual-smoke] scenario=${visualSmokeFixture.scenario} workspace=${workspaceRoot}`); await seedVisualSmokeFixture({ workspaceRoot, fixture: visualSmokeFixture, credentialStore }); diff --git a/apps/desktop/src/main/onboarding-service.ts b/apps/desktop/src/main/onboarding-service.ts index 81c280a2d0..123eab1263 100644 --- a/apps/desktop/src/main/onboarding-service.ts +++ b/apps/desktop/src/main/onboarding-service.ts @@ -214,7 +214,7 @@ export function bindOnboardingDeps(input: { list(): Promise; getDefault(): Promise; }; - credentialStore: Pick; + credentialStore: Pick; listSessions(): Promise; }): OnboardingServiceDeps { return { @@ -225,7 +225,7 @@ export function bindOnboardingDeps(input: { upsertMilestone: (id, status) => input.settingsStore.upsertOnboardingMilestone(id, status), clearMilestone: (id) => input.settingsStore.clearOnboardingMilestone(id), hasApiKey: async (slug) => { - const key = await input.credentialStore.getApiKey(slug); + const key = await input.credentialStore.getSecret(slug, 'api_key'); return typeof key === 'string' && key.length > 0; }, }; diff --git a/packages/storage/src/__tests__/credential-migration.test.ts b/packages/storage/src/__tests__/credential-migration.test.ts new file mode 100644 index 0000000000..fa3a186422 --- /dev/null +++ b/packages/storage/src/__tests__/credential-migration.test.ts @@ -0,0 +1,160 @@ +import assert from 'node:assert/strict'; +import { mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, it } from 'node:test'; +import { migrateLegacyCredentialFile, type LegacyCredentialDecryptor } from '../credential-store.js'; + +// Behavioral tests over real temp files for the one-time migration. The +// decryptor is injected (the crypto is the caller's concern), so these run +// under plain `node --test`. No fixed sleeps: concurrency is proven by racing +// two real migrations, not by waiting a guessed number of milliseconds. + +/** Fake decryptor: "decrypt" strips an `enc:` prefix so every value is proven + * to round-trip through decrypt(). */ +function fakeDecryptor(available: boolean): LegacyCredentialDecryptor { + return { + isAvailable: () => available, + decrypt: (stored) => stored.replace(/^enc:/, ''), + }; +} + +function legacyFile(values: Record): string { + return JSON.stringify({ values }); +} + +async function withWorkspace(fn: (path: string) => Promise): Promise { + const root = await mkdtemp(join(tmpdir(), 'maka-cred-mig-')); + try { + return await fn(join(root, 'credentials.json')); + } finally { + await rm(root, { recursive: true, force: true }); + } +} + +const isPosix = process.platform !== 'win32'; + +describe('migrateLegacyCredentialFile', () => { + it('decrypts ALL secret kinds to v1 plaintext-0600 in place', async () => { + await withWorkspace(async (path) => { + // Full scope: not just API keys — bot/proxy/etc. secrets migrate too. + await writeFile( + path, + legacyFile({ + 'openai:apiKey': 'enc:sk-1', + 'settings:bot:telegram:botToken': 'enc:tok-2', + 'settings:network-proxy:proxyPassword': 'enc:pw-3', + }), + 'utf8', + ); + + await migrateLegacyCredentialFile(path, fakeDecryptor(true)); + + const after = JSON.parse(await readFile(path, 'utf8')) as { + version: number; + values: Record; + }; + assert.equal(after.version, 1); + assert.deepEqual(after.values, { + 'openai:apiKey': 'sk-1', + 'settings:bot:telegram:botToken': 'tok-2', + 'settings:network-proxy:proxyPassword': 'pw-3', + }); + if (isPosix) { + assert.equal((await stat(path)).mode & 0o777, 0o600); // owner-only at rest + } + }); + }); + + it('aborts and leaves the legacy file intact when the decryptor is unavailable', async () => { + await withWorkspace(async (path) => { + const original = legacyFile({ 'openai:apiKey': 'enc:sk-1' }); + await writeFile(path, original, 'utf8'); + + await assert.rejects(migrateLegacyCredentialFile(path, fakeDecryptor(false)), /unavailable/); + assert.equal(await readFile(path, 'utf8'), original); // untouched — no data loss + }); + }); + + it('migrates an EMPTY legacy file even when the decryptor is unavailable', async () => { + await withWorkspace(async (path) => { + // A user who deleted their last secret under the old store left `values: {}`. + // There is nothing to decrypt, so an unavailable decryptor must NOT block + // the stamp — otherwise the v1 store (which refuses unversioned files) + // would be permanently unusable for that workspace. + await writeFile(path, legacyFile({}), 'utf8'); + + await migrateLegacyCredentialFile(path, fakeDecryptor(false)); + + const after = JSON.parse(await readFile(path, 'utf8')) as { + version: number; + values: Record; + }; + assert.equal(after.version, 1); + assert.deepEqual(after.values, {}); + }); + }); + + it('is a no-op on an already-migrated v1 file', async () => { + await withWorkspace(async (path) => { + const v1 = JSON.stringify({ version: 1, values: { 'openai:apiKey': 'sk-1' } }); + await writeFile(path, v1, 'utf8'); + + await migrateLegacyCredentialFile(path, fakeDecryptor(true)); + assert.equal(await readFile(path, 'utf8'), v1); // unchanged + }); + }); + + it('refuses a malformed legacy file rather than tombstone it empty', async () => { + await withWorkspace(async (path) => { + const malformed = JSON.stringify({ foo: 1 }); // no version, no values + await writeFile(path, malformed, 'utf8'); + + await assert.rejects(migrateLegacyCredentialFile(path, fakeDecryptor(true)), /malformed/); + assert.equal(await readFile(path, 'utf8'), malformed); // untouched + }); + }); + + it('refuses a legacy file whose values are not all strings, leaving it untouched', async () => { + // A number, null, or nested object can't be a safeStorage-encrypted string; + // it must fail closed BEFORE reaching the decryptor (mirrors the v1 reader), + // not be fed garbage to decrypt. + for (const badValue of ['123', 'null', '{ "nested": "no" }']) { + await withWorkspace(async (path) => { + const original = `{ "values": { "x:apiKey": ${badValue} } }`; + await writeFile(path, original, 'utf8'); + + await assert.rejects(migrateLegacyCredentialFile(path, fakeDecryptor(true)), /not a string/); + assert.equal(await readFile(path, 'utf8'), original); // byte-for-byte untouched + }); + } + }); + + it('is a no-op when there is no credentials file', async () => { + await withWorkspace(async (path) => { + await migrateLegacyCredentialFile(path, fakeDecryptor(true)); + await assert.rejects(stat(path)); // still absent, no file created + }); + }); + + it('serializes two racing migrations: one migrates, the other no-ops on the re-read', async () => { + await withWorkspace(async (path) => { + await writeFile(path, legacyFile({ 'openai:apiKey': 'enc:sk-1' }), 'utf8'); + + // The shared lock serializes the two; the loser re-reads inside the lock, + // sees v1, and no-ops instead of decrypting a stale snapshot again. Both + // resolve cleanly and the result is the single correct v1 file. + await Promise.all([ + migrateLegacyCredentialFile(path, fakeDecryptor(true)), + migrateLegacyCredentialFile(path, fakeDecryptor(true)), + ]); + + const after = JSON.parse(await readFile(path, 'utf8')) as { + version: number; + values: Record; + }; + assert.equal(after.version, 1); + assert.deepEqual(after.values, { 'openai:apiKey': 'sk-1' }); + }); + }); +}); diff --git a/packages/storage/src/__tests__/credential-store.test.ts b/packages/storage/src/__tests__/credential-store.test.ts new file mode 100644 index 0000000000..68e26cb7b7 --- /dev/null +++ b/packages/storage/src/__tests__/credential-store.test.ts @@ -0,0 +1,239 @@ +import assert from 'node:assert/strict'; +import { chmod, mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, test } from 'node:test'; +import { + CREDENTIAL_SCHEMA_VERSION, + createFileCredentialStore, + withCredentialFileLock, + type CredentialKind, +} from '../credential-store.js'; + +const isPosix = process.platform !== 'win32'; + +async function withTempDir(fn: (dir: string) => Promise): Promise { + const dir = await mkdtemp(join(tmpdir(), 'maka-cred-')); + try { + return await fn(dir); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +describe('FileCredentialStore', () => { + test('round-trips secrets and returns null for missing ones', async () => { + await withTempDir(async (dir) => { + const store = createFileCredentialStore(dir); + assert.equal(await store.getSecret('openai-prod', 'api_key'), null); + + await store.setSecret('openai-prod', 'api_key', 'sk-test-123'); + assert.equal(await store.getSecret('openai-prod', 'api_key'), 'sk-test-123'); + + await store.deleteSecret('openai-prod', 'api_key'); + assert.equal(await store.getSecret('openai-prod', 'api_key'), null); + }); + }); + + test('deleteSecret(slug) with no kind clears every kind for that slug only', async () => { + await withTempDir(async (dir) => { + const store = createFileCredentialStore(dir); + await store.setSecret('a', 'api_key', 'key-a'); + await store.setSecret('a', 'oauth_token', 'tok-a'); + await store.setSecret('b', 'api_key', 'key-b'); + + await store.deleteSecret('a'); + + assert.equal(await store.getSecret('a', 'api_key'), null); + assert.equal(await store.getSecret('a', 'oauth_token'), null); + assert.equal(await store.getSecret('b', 'api_key'), 'key-b'); + }); + }); + + test('writes a versioned, plaintext (file-first) file', async () => { + await withTempDir(async (dir) => { + const store = createFileCredentialStore(dir); + await store.setSecret('openai-prod', 'api_key', 'sk-plain'); + + const raw = JSON.parse(await readFile(join(dir, 'credentials.json'), 'utf8')) as { + version: number; + values: Record; + }; + assert.equal(raw.version, CREDENTIAL_SCHEMA_VERSION); + // File-first: the value is stored as plaintext, not encoded. + assert.equal(raw.values['openai-prod:apiKey'], 'sk-plain'); + }); + }); + + test('reading an unknown / pre-migration schema fails closed', async () => { + await withTempDir(async (dir) => { + const path = join(dir, 'credentials.json'); + + // Legacy file: no `version` field (the safeStorage-era shape). + await writeFile(path, JSON.stringify({ values: { 'x:apiKey': 'enc' } }), 'utf8'); + const legacy = createFileCredentialStore(dir); + await assert.rejects(legacy.getSecret('x', 'api_key'), /schema version/); + + // Future version we don't understand. + await writeFile(path, JSON.stringify({ version: 999, values: {} }), 'utf8'); + const future = createFileCredentialStore(dir); + await assert.rejects(future.getSecret('x', 'api_key'), /schema version/); + }); + }); + + test('leaves no temp file behind after a write', async () => { + await withTempDir(async (dir) => { + const store = createFileCredentialStore(dir); + await store.setSecret('a', 'api_key', 'k'); + const entries = await readdir(dir); + assert.deepEqual(entries, ['credentials.json']); + }); + }); + + test('creates the file 0600 on POSIX', { skip: !isPosix }, async () => { + await withTempDir(async (dir) => { + const store = createFileCredentialStore(dir); + await store.setSecret('a', 'api_key', 'k'); + const mode = (await stat(join(dir, 'credentials.json'))).mode & 0o777; + assert.equal(mode, 0o600); + }); + }); + + test('re-chmods a pre-existing world-readable file to 0600 on write', { skip: !isPosix }, async () => { + await withTempDir(async (dir) => { + const path = join(dir, 'credentials.json'); + // A valid v1 file that was created with a loose mode. + await writeFile(path, JSON.stringify({ version: CREDENTIAL_SCHEMA_VERSION, values: {} }), { + encoding: 'utf8', + mode: 0o644, + }); + const store = createFileCredentialStore(dir); + await store.setSecret('a', 'api_key', 'k'); + const mode = (await stat(path)).mode & 0o777; + assert.equal(mode, 0o600); + }); + }); + + test('hardens a pre-existing world-accessible workspace dir to 0700 on write', { skip: !isPosix }, async () => { + await withTempDir(async (dir) => { + await chmod(dir, 0o777); // a loose dir that predates the hardening + const store = createFileCredentialStore(dir); + await store.setSecret('a', 'api_key', 'k'); + // ensureSecretDir re-chmods an existing dir (mkdir's mode only applies on + // creation); the writer and the lock share it, so the lock can't leave + // the dir loose either. + assert.equal((await stat(dir)).mode & 0o777, 0o700); + }); + }); + + test('serializes concurrent writes across slugs without clobbering', async () => { + await withTempDir(async (dir) => { + // With no in-instance queue, these contend directly on the file lock — + // this proves the lock alone serializes a read-modify-write so no slug is + // dropped. A handful is enough; more just adds lock-poll wall-clock. + const store = createFileCredentialStore(dir); + const count = 8; + await Promise.all( + Array.from({ length: count }, (_unused, i) => store.setSecret(`conn-${i}`, 'api_key', `key-${i}`)), + ); + for (let i = 0; i < count; i++) { + assert.equal(await store.getSecret(`conn-${i}`, 'api_key'), `key-${i}`); + } + }); + }); + + test('two independent store instances writing concurrently both survive (cross-process lock)', async () => { + await withTempDir(async (dir) => { + // Separate instances => separate in-instance queues; only the file + // lock can stop a read-modify-write lost update between them. + const a = createFileCredentialStore(dir); + const b = createFileCredentialStore(dir); + await Promise.all([ + a.setSecret('slug-a', 'api_key', 'AAA'), + b.setSecret('slug-b', 'api_key', 'BBB'), + ]); + + const reader = createFileCredentialStore(dir); + assert.equal(await reader.getSecret('slug-a', 'api_key'), 'AAA'); + assert.equal(await reader.getSecret('slug-b', 'api_key'), 'BBB'); + }); + }); + + test('a held lock is waited on, never stolen (no lost update)', async () => { + await withTempDir(async (dir) => { + const path = join(dir, 'credentials.json'); + // Hold the lock as another process (or a crashed one) would: the lock is + // the `${path}.lock` directory. The store must wait for it, never steal it. + const lockPath = `${path}.lock`; + await mkdir(lockPath); + + const store = createFileCredentialStore(dir); + let settled = false; + const write = store.setSecret('a', 'api_key', 'V').then(() => { + settled = true; + }); + + // The lock is held, so the write is blocked before its critical section: + // it must not steal the lock and must not have written the file yet. + await assert.rejects(stat(path)); // file absent — proven blocked, not stolen + assert.equal(settled, false); + + await rm(lockPath, { recursive: true, force: true }); // release + await write; + assert.equal(await store.getSecret('a', 'api_key'), 'V'); // proceeds only once the lock frees + }); + }); + + test('a never-released lock fails loud with the lock path and recovery hint', async () => { + await withTempDir(async (dir) => { + const path = join(dir, 'credentials.json'); + await mkdir(`${path}.lock`); // a crashed holder's lock that never releases + // A small timeout drives the fail-loud path without the production wait. + // The error must name the lock dir AND how to recover, so the guidance + // can't silently regress. + await assert.rejects( + withCredentialFileLock(path, async () => 'unreachable', 60), + (error: Error) => + error.message.includes(`${path}.lock`) + && /remove that directory and retry/.test(error.message), + ); + }); + }); +}); + +describe('FileCredentialStore secret-kind + slug contract', () => { + // The on-disk stored-kind suffixes are a backward-compat contract: a + // migrated legacy file keeps the same `slug:kind` keys, so the suffix + // names must not drift. + const kindToStoredSuffix: Array<[CredentialKind, string]> = [ + ['api_key', 'apiKey'], + ['oauth_token', 'oauthToken'], + ['bot_token', 'botToken'], + ['app_secret', 'botAppSecret'], + ['proxy_password', 'proxyPassword'], + ['gateway_token', 'gatewayToken'], + ['tavily_api_key', 'tavilyApiKey'], + ]; + + test('preserves the legacy stored-key suffix for every kind', async () => { + await withTempDir(async (dir) => { + const store = createFileCredentialStore(dir); + // The generic API expresses every kind — including the bot/proxy/gateway/ + // tavily secrets the migration carries — as `${slug}:${suffix}`. The + // caller owns the slug, so a key never derives from a secret value. + for (const [kind] of kindToStoredSuffix) { + await store.setSecret('settings:bot:telegram', kind, `val-${kind}`); + } + const raw = JSON.parse(await readFile(join(dir, 'credentials.json'), 'utf8')) as { + values: Record; + }; + for (const [kind, suffix] of kindToStoredSuffix) { + assert.equal( + raw.values[`settings:bot:telegram:${suffix}`], + `val-${kind}`, + `${kind} -> settings:bot:telegram:${suffix}`, + ); + } + }); + }); +}); diff --git a/packages/storage/src/credential-store.ts b/packages/storage/src/credential-store.ts new file mode 100644 index 0000000000..0ddb8434dd --- /dev/null +++ b/packages/storage/src/credential-store.ts @@ -0,0 +1,391 @@ +import { randomUUID } from 'node:crypto'; +import { chmod, mkdir, readFile, rename, rm, writeFile } from 'node:fs/promises'; +import { dirname, join } from 'node:path'; + +/** + * Pure-Node credential store. Shared by the desktop app and any + * headless consumer (CLI / eval harness / third party) that runs the + * runtime outside Electron. + * + * At rest this is plaintext JSON behind 0600 file perms (file-first; + * see issue #32). The OS user account is the security boundary + * (SECURITY.md). At-rest encryption (an OS keychain via a pure-Node + * binding, or a passphrase) is a later addition — deliberately deferred + * until there is a real backend, so its sync/async shape is designed + * against that backend instead of guessed now. + * + * Writes are serialized across processes by an atomic-mkdir lockfile that is + * never stolen (see withCredentialFileLock), so two store instances (or + * processes) sharing one file can't lose each other's update through a + * read-modify-write race. + * + * Secret VALUES are never logged. Callers expose the typed + * `CredentialStore` API to third parties — never the raw file format, + * which stays an internal implementation detail. + */ + +type StoredCredentialKind = + | 'apiKey' + | 'oauthToken' + | 'botToken' + | 'botAppSecret' + | 'proxyPassword' + | 'gatewayToken' + | 'tavilyApiKey'; +export type CredentialKind = + | 'api_key' + | 'oauth_token' + | 'bot_token' + | 'app_secret' + | 'proxy_password' + | 'gateway_token' + | 'tavily_api_key'; + +/** Current on-disk schema version. Unknown versions fail closed on read. */ +export const CREDENTIAL_SCHEMA_VERSION = 1; + +interface CredentialFile { + version: number; + values: Record; +} + +export interface CredentialStore { + getSecret(slug: string, kind: CredentialKind): Promise; + setSecret(slug: string, kind: CredentialKind, value: string): Promise; + /** Delete one kind, or — with no kind — every kind for the slug (e.g. a + * connection being removed). */ + deleteSecret(slug: string, kind?: CredentialKind): Promise; +} + +export function createFileCredentialStore(workspaceRoot: string): CredentialStore { + return new FileCredentialStore(join(workspaceRoot, 'credentials.json')); +} + +/** + * Injected decryptor for the one-time legacy migration. The legacy + * credentials.json stored each secret as an opaque, externally-encrypted string + * (Electron `safeStorage`, base64-wrapped); only the desktop main process can + * decrypt it. The migration lives here so it shares the live store's lock and + * atomic writer, but the crypto stays the caller's: desktop passes a decryptor + * backed by `safeStorage`, while a headless caller has none and never runs it. + */ +export interface LegacyCredentialDecryptor { + /** Whether decryption is currently possible. If false, the migration aborts + * and leaves the encrypted file untouched (never destroy unrecoverable + * secrets). */ + isAvailable(): boolean; + /** Decrypt one legacy stored value to plaintext. */ + decrypt(storedValue: string): string; +} + +/** + * One-time migration of a legacy (pre-version, externally-encrypted) + * credentials.json to the shared v1 plaintext-0600 shape, in place. + * + * Runs under the SAME cross-process lock as the live store and re-reads inside + * it, so a racing process that already migrated (and a live writer that added a + * newer secret) is never clobbered by a stale snapshot. Idempotent: a no-op + * when the file is missing or already v1. Fails closed: an unexpected version, + * a malformed `values`, or a decryptor that is unavailable while there are + * values to decrypt throws and leaves the file untouched rather than risk + * tombstoning unrecoverable secrets. + * + * Tombstone, not dual-active: a successful run rewrites every value as + * plaintext, so no decryptable copy survives. + */ +export async function migrateLegacyCredentialFile( + path: string, + decryptor: LegacyCredentialDecryptor, +): Promise { + await withCredentialFileLock(path, async () => { + let raw: string; + try { + raw = await readFile(path, 'utf8'); + } catch (error) { + if ((error as { code?: string }).code === 'ENOENT') return; // nothing to migrate + throw error; + } + + const parsed = JSON.parse(raw) as { version?: number; values?: Record }; + if (parsed.version === CREDENTIAL_SCHEMA_VERSION) return; // already migrated (possibly by a racing process) + if (parsed.version !== undefined) { + throw new Error(`Cannot migrate credentials.json: unexpected schema version ${parsed.version}.`); + } + + const legacy = parsed.values; + if (legacy === null || typeof legacy !== 'object' || Array.isArray(legacy)) { + throw new Error('Cannot migrate credentials.json: missing or malformed `values`. Leaving it untouched.'); + } + const entries = Object.entries(legacy); + // Every legacy value must be a string we can hand to the decryptor. A + // non-string entry means a corrupt or foreign file, so fail closed and + // leave it untouched rather than feed garbage to decrypt — the same + // per-value guarantee the v1 reader enforces. + for (const [key, storedValue] of entries) { + if (typeof storedValue !== 'string') { + throw new Error( + `Cannot migrate credentials.json: value for "${key}" is not a string. Leaving it untouched.`, + ); + } + } + // Only the actual decryption needs the decryptor. An empty legacy file has + // nothing to decrypt, so it must still migrate to the v1 empty shape even + // when the decryptor is unavailable — otherwise a user who deleted their + // last secret under the old store, on a box where safeStorage is now + // unavailable, would be stuck: the v1 store refuses the unversioned file and + // the migration would refuse to stamp it. + if (entries.length > 0 && !decryptor.isAvailable()) { + throw new Error( + 'Cannot migrate credentials.json: the legacy decryptor is unavailable. Leaving the encrypted file untouched.', + ); + } + + // Decrypt EVERY legacy value to plaintext. Keys are preserved verbatim + // (slugs can contain ':', so we never parse them apart). + const migrated: Record = {}; + for (const [key, storedValue] of entries) { + migrated[key] = decryptor.decrypt(storedValue); + } + + await writeSecretFileAtomic( + path, + JSON.stringify({ version: CREDENTIAL_SCHEMA_VERSION, values: migrated }, null, 2) + '\n', + ); + }); +} + +class FileCredentialStore implements CredentialStore { + constructor(private readonly path: string) {} + + getSecret(slug: string, kind: CredentialKind): Promise { + return this.get(slug, toStoredKind(kind)); + } + + setSecret(slug: string, kind: CredentialKind, value: string): Promise { + return this.set(slug, toStoredKind(kind), value); + } + + async deleteSecret(slug: string, kind?: CredentialKind): Promise { + await this.mutate((values) => { + if (kind) { + delete values[this.key(slug, toStoredKind(kind))]; + return; + } + // No kind: clear every kind for the slug in one read-modify-write. + for (const storedKind of STORED_CREDENTIAL_KINDS) { + delete values[this.key(slug, storedKind)]; + } + }); + } + + private async get(slug: string, kind: StoredCredentialKind): Promise { + const value = (await this.readUnlocked()).values[this.key(slug, kind)]; + return value === undefined ? null : value; + } + + private set(slug: string, kind: StoredCredentialKind, value: string): Promise { + return this.mutate((values) => { + values[this.key(slug, kind)] = value; + }); + } + + /** + * Read-modify-write the whole file under the cross-process lockfile. The lock + * serializes concurrent calls on this instance and a second store instance / + * process alike, so one mechanism covers both — no separate in-instance queue. + */ + private mutate(apply: (values: Record) => void): Promise { + return withCredentialFileLock(this.path, async () => { + const file = await this.readUnlocked(); + apply(file.values); + await this.write(file); + }); + } + + private key(slug: string, kind: StoredCredentialKind): string { + return `${slug}:${kind}`; + } + + private async readUnlocked(): Promise { + let raw: string; + try { + raw = await readFile(this.path, 'utf8'); + } catch (error) { + if ((error as { code?: string }).code === 'ENOENT') { + return { version: CREDENTIAL_SCHEMA_VERSION, values: {} }; + } + throw error; + } + const parsed = JSON.parse(raw) as Partial; + // Fail closed on an unknown / pre-migration schema. A legacy file + // (safeStorage-encrypted, no `version`) lands here as `undefined` + // and must be migrated by the desktop importer before use — we do + // not silently start a parallel plaintext store next to it. + if (parsed.version !== CREDENTIAL_SCHEMA_VERSION) { + throw new Error( + `Unsupported credentials.json schema version: ${String(parsed.version)} ` + + `(expected ${CREDENTIAL_SCHEMA_VERSION}). Open the desktop app once to migrate, ` + + `or re-authenticate. If migration keeps failing, a stale lock may be blocking it — ` + + `remove ${this.path}.lock and retry.`, + ); + } + // A v1 file must carry a well-formed `values` map. Treat a missing or + // malformed `values` as corruption and fail closed rather than silently + // serving an empty store (which would read as "no credentials"). + const values = parsed.values; + if (values === null || typeof values !== 'object' || Array.isArray(values)) { + throw new Error('Corrupt credentials.json: `values` is missing or not an object.'); + } + for (const [k, v] of Object.entries(values)) { + if (typeof v !== 'string') { + throw new Error(`Corrupt credentials.json: value for "${k}" is not a string.`); + } + } + return { version: CREDENTIAL_SCHEMA_VERSION, values: values as Record }; + } + + private write(file: CredentialFile): Promise { + return writeSecretFileAtomic(this.path, JSON.stringify(file, null, 2) + '\n'); + } +} + +/** + * Create (or harden) the directory that holds a secret file: 0700, and + * re-chmod a pre-existing looser dir so neither the secret nor the lock can sit + * world-readable. mkdir's mode only applies on creation, so the chmod is what + * fixes an existing dir. On POSIX a chmod failure fails closed (we must not + * write plaintext credentials into a dir we couldn't lock down); no-op on + * Windows. Shared by the writer and the lock so their dir hardening can't drift. + */ +async function ensureSecretDir(dir: string): Promise { + await mkdir(dir, { recursive: true, mode: 0o700 }); + await chmodStrict(dir, 0o700); +} + +/** + * Owner-only atomic write for a credentials file: a 0700 dir, an exclusive + * 0600 temp ('wx'/O_EXCL so we never follow a pre-planted symlink at a + * predictable path), 0600 re-enforced, an atomic rename, and temp cleanup on + * failure. Shared by the live store and the one-time migration so the hardening + * can't drift between the two write paths. + */ +async function writeSecretFileAtomic(path: string, contents: string): Promise { + await ensureSecretDir(dirname(path)); + const tempPath = `${path}.${randomUUID()}.tmp`; + try { + await writeFile(tempPath, contents, { encoding: 'utf8', mode: 0o600, flag: 'wx' }); + await chmodStrict(tempPath, 0o600); + await rename(tempPath, path); + } catch (error) { + await rm(tempPath, { force: true }); + throw error; + } +} + +/** + * chmod that fails loud on POSIX and is best-effort on Windows. A secret file + * or its directory left looser than intended breaks the 0600/0700 boundary, so + * on POSIX we surface the failure rather than write plaintext into it; Windows + * has no POSIX mode, so a failure there is a no-op. One policy for both the + * secret file (0600) and its directory (0700) so they can't drift apart. + */ +async function chmodStrict(path: string, mode: number): Promise { + if (process.platform === 'win32') { + await chmod(path, mode).catch(() => {}); + return; + } + await chmod(path, mode); +} + +// A contended acquire polls this often, then fails loud after the timeout. +const LOCK_POLL_MS = 25; +const LOCK_TIMEOUT_MS = 10_000; +const delay = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +/** + * Serialize a read-modify-write across processes / store instances that share + * one credentials.json, so two writers can't lose each other's update through a + * read, read, write, write race. + * + * Acquire is an atomic `mkdir` of `${targetPath}.lock` (POSIX mkdir is atomic + * and fails EEXIST if it already exists); release deletes it. The lock is NEVER + * stolen — a held or leftover lock is waited on, then we fail loud. That is the + * whole design, and the reason it is correct. Every "detect a crashed holder's + * stale lock, then remove it and re-acquire" scheme — the earlier hand-rolled + * ones AND proper-lockfile — is a TOCTOU race: between judging a lock stale and + * deleting it, another contender can reclaim it, so the delete drops a live + * lock and both writers enter the critical section. There is no safe userspace + * compare-and-steal, so we do not steal at all. + * + * The cost: a hard crash (SIGKILL / power loss) mid-write leaves the lock + * directory behind, and the next writer fails loud until it is removed — an + * explicit, one-command recovery, never a silent lost update. A clean exit or a + * completed write releases it via the finally. credentials.json is written + * rarely and is local, so this is the right trade for credential data. Used by + * both the live store and the one-time migration so they serialize on one lock. + * + * `timeoutMs` defaults to LOCK_TIMEOUT_MS; it is a parameter only so a test can + * drive the fail-loud path with a small value. Exported for that test — it is + * deliberately NOT re-exported from index.ts, so the package's public surface + * stays the typed store + migration and callers can't drive the lock directly. + */ +export async function withCredentialFileLock( + targetPath: string, + fn: () => Promise, + timeoutMs: number = LOCK_TIMEOUT_MS, +): Promise { + const lockPath = `${targetPath}.lock`; + // mkdir (the acquire below) is atomic but needs its parent to exist; harden it + // to 0700 the same way the writer does so the lock dir never sits loose. + await ensureSecretDir(dirname(targetPath)); + const deadline = Date.now() + timeoutMs; + for (;;) { + try { + await mkdir(lockPath); + break; + } catch (error) { + if ((error as { code?: string }).code !== 'EEXIST') throw error; + if (Date.now() >= deadline) { + throw new Error( + `credentials.json is locked by another process (${lockPath}). ` + + 'If no other process is using it, remove that directory and retry.', + ); + } + await delay(LOCK_POLL_MS); + } + } + try { + return await fn(); + } finally { + await rm(lockPath, { recursive: true, force: true }); + } +} + +const STORED_CREDENTIAL_KINDS = [ + 'apiKey', + 'oauthToken', + 'botToken', + 'botAppSecret', + 'proxyPassword', + 'gatewayToken', + 'tavilyApiKey', +] as const satisfies readonly StoredCredentialKind[]; + +function toStoredKind(kind: CredentialKind): StoredCredentialKind { + switch (kind) { + case 'api_key': + return 'apiKey'; + case 'oauth_token': + return 'oauthToken'; + case 'bot_token': + return 'botToken'; + case 'app_secret': + return 'botAppSecret'; + case 'proxy_password': + return 'proxyPassword'; + case 'gateway_token': + return 'gatewayToken'; + case 'tavily_api_key': + return 'tavilyApiKey'; + } +} diff --git a/packages/storage/src/index.ts b/packages/storage/src/index.ts index 568f85620a..ce0674b2a6 100644 --- a/packages/storage/src/index.ts +++ b/packages/storage/src/index.ts @@ -1,6 +1,19 @@ export * from './session-store.js'; export * from './agent-run-store.js'; export * from './connection-store.js'; +// Narrow public surface: only the typed store + the one-time migration. The +// file lock and atomic writer stay internal so callers can't bypass the +// CredentialStore contract and drive the low-level lock directly. +export { + CREDENTIAL_SCHEMA_VERSION, + createFileCredentialStore, + migrateLegacyCredentialFile, +} from './credential-store.js'; +export type { + CredentialKind, + CredentialStore, + LegacyCredentialDecryptor, +} from './credential-store.js'; export * from './settings-store.js'; export * from './telemetry-repo.js'; export * from './artifact-store.js';