diff --git a/src/infrastructure/config.ts b/src/infrastructure/config.ts index 4034db3e..523c16af 100644 --- a/src/infrastructure/config.ts +++ b/src/infrastructure/config.ts @@ -1,7 +1,7 @@ import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; -import { toErrorMessage } from '../shared/errors.js'; +import { ConfigError, toErrorMessage } from '../shared/errors.js'; import type { CodegraphConfig } from '../types.js'; import { debug, warn } from './logger.js'; @@ -179,6 +179,7 @@ export function loadConfig(cwd?: string): CodegraphConfig { _configCache.set(cwd, structuredClone(result)); return result; } catch (err: unknown) { + if (err instanceof ConfigError) throw err; debug(`Failed to parse config ${filePath}: ${toErrorMessage(err)}`); } } @@ -215,7 +216,16 @@ export function applyEnvOverrides(config: CodegraphConfig): CodegraphConfig { export function resolveSecrets(config: CodegraphConfig): CodegraphConfig { const cmd = config.llm.apiKeyCommand; - if (typeof cmd !== 'string' || cmd.trim() === '') return config; + if (cmd == null) return config; + if (typeof cmd !== 'string') { + const actual = Array.isArray(cmd) ? 'array' : typeof cmd; + throw new ConfigError( + `llm.apiKeyCommand must be a string (received ${actual}). ` + + 'The command is split on whitespace and executed without a shell. ' + + 'Example: "apiKeyCommand": "op read op://vault/openai/api-key"', + ); + } + if (cmd.trim() === '') return config; const parts = cmd.trim().split(/\s+/); const [executable, ...args] = parts; diff --git a/src/types.ts b/src/types.ts index d8d7b00c..09159205 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1104,6 +1104,13 @@ export interface CodegraphConfig { model: string | null; baseUrl: string | null; apiKey: string | null; + /** + * Command that prints the API key to stdout. Must be a single string — + * it is split on whitespace and executed via `execFileSync` with no shell, + * so shell features like `$(...)`, pipes, globs, or variable expansion are + * not supported. Example: `"op read op://vault/openai/api-key"`. Non-string + * values are rejected with a `ConfigError` at load time. + */ apiKeyCommand: string | null; }; diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index bba8c190..a2b50401 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -15,6 +15,7 @@ import { mergeConfig, resolveSecrets, } from '../../src/infrastructure/config.js'; +import { ConfigError } from '../../src/shared/errors.js'; vi.mock('node:child_process', async (importOriginal) => { const actual = await importOriginal(); @@ -430,13 +431,24 @@ describe('resolveSecrets', () => { expect(config.llm.apiKey).toBe('existing'); }); - it('skips when apiKeyCommand is not a string', () => { - const config = { + it('throws ConfigError when apiKeyCommand is not a string', () => { + const numberCfg = { llm: { provider: null, model: null, baseUrl: null, apiKey: 'existing', apiKeyCommand: 42 }, }; - resolveSecrets(config); + expect(() => resolveSecrets(numberCfg as never)).toThrow(ConfigError); + expect(() => resolveSecrets(numberCfg as never)).toThrow(/must be a string/); + + const arrayCfg = { + llm: { + provider: null, + model: null, + baseUrl: null, + apiKey: 'existing', + apiKeyCommand: ['op', 'read', 'op://vault/key'], + }, + }; + expect(() => resolveSecrets(arrayCfg as never)).toThrow(/received array/); expect(mockExecFile).not.toHaveBeenCalled(); - expect(config.llm.apiKey).toBe('existing'); }); it('warns and preserves existing apiKey on command failure', () => {