fix(config): reject non-string apiKeyCommand with ConfigError (#991)

* fix(config): reject non-string apiKeyCommand with ConfigError

An array-form apiKeyCommand (the more ergonomic shape users often reach
for) silently fell through resolveSecrets' typeof check and left apiKey
null with no explanation. Throw a ConfigError with the expected format
and example, let it propagate past loadConfig's JSON-parse try/catch,
and document the single-string contract on the type.

Impact: 3 functions changed, 106 affected

* docs(config): clarify apiKeyCommand runs without a shell (#991)

Impact: 1 functions changed, 0 affected

Author: carlos-alm

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;

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;
   };
 

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', () => {