From 1c6b4db0d030af453f49045b4dddc82013e78a9c Mon Sep 17 00:00:00 2001 From: kragent66-glitch Date: Mon, 21 Sep 2026 01:51:17 +0000 Subject: [PATCH 1/3] fix(init): stop persisting env-var API keys into config.json --- src/commands/init.ts | 48 ++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/commands/init.ts b/src/commands/init.ts index 81e9b90..8305a6e 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -189,18 +189,20 @@ async function promptProvider(existingConfig: Config | null): Promise, 'provider' | 'apiKey' | 'baseUrl'> | null, -): Promise { - if (!provider.needsApiKey) return undefined; - - const existingKey = getExistingApiKeyForProvider( - provider.providerKey, - provider.baseUrl, - storedConfig, - provider.apiKeyEnv, - ); - const keyResult = await password(buildApiKeyPrompt(existingKey, provider.apiKeyEnv)); +): Promise<{ effectiveKey: string; persistKey: string | undefined } | null> { + if (!provider.needsApiKey) return { effectiveKey: '', persistKey: undefined }; + + const rawConfiguredKey = getStoredApiKeyForProvider(provider.providerKey, provider.baseUrl, storedConfig) || ''; + const envKey = process.env['COMMIT_ECHO_API_KEY']?.trim() || process.env[provider.apiKeyEnv]?.trim() || ''; + const promptKey = rawConfiguredKey || envKey || ''; + + const keyResult = await password(buildApiKeyPrompt(promptKey, provider.apiKeyEnv)); if (isCancel(keyResult)) return null; - return keyResult || existingKey || ''; + + const effectiveKey = keyResult || envKey || rawConfiguredKey || ''; + const persistKey = keyResult || rawConfiguredKey || undefined; + + return { effectiveKey, persistKey }; } interface PromptModelDependencies { @@ -371,11 +373,15 @@ async function persistSetup( } } -async function testConfiguration(config: Config, provider: ProviderSetup): Promise { +async function testConfiguration( + config: Config, + provider: ProviderSetup, + effectiveKey?: string, +): Promise { const testSpinner = spinner(); testSpinner.start('Testing connection...'); try { - const resolvedKey = config.apiKey ?? process.env[provider.apiKeyEnv] ?? ''; + const resolvedKey = effectiveKey ?? config.apiKey ?? process.env[provider.apiKeyEnv] ?? ''; const { testConnection } = await import('../llm/client.js'); const modelName = await testConnection({ ...config, apiKey: resolvedKey }); @@ -404,6 +410,7 @@ function buildTemplateInfo(config: Config): string { interface CollectedSetup { config: Config; provider: ProviderSetup; + effectiveKey: string; } async function collectConfig( @@ -413,10 +420,10 @@ async function collectConfig( const provider = await promptProvider(existingConfig); if (!provider) return null; - const apiKey = await promptApiKey(provider, storedConfig); - if (apiKey === null) return null; + const apiKeyResult = await promptApiKey(provider, storedConfig); + if (apiKeyResult === null) return null; - const selectedModel = await promptModel(provider, apiKey, existingConfig); + const selectedModel = await promptModel(provider, apiKeyResult.effectiveKey, existingConfig); if (!selectedModel) return null; const limits = await promptHistoryLimits(existingConfig); @@ -427,11 +434,12 @@ async function collectConfig( return { provider, + effectiveKey: apiKeyResult.effectiveKey, config: { provider: provider.providerKey, model: selectedModel, baseUrl: provider.providerKey === CUSTOM_PROVIDER_KEY ? provider.baseUrl : undefined, - apiKey: apiKey ?? undefined, + apiKey: apiKeyResult.persistKey, historySize: limits.historySize, maxDiffSize: limits.maxDiffSize, ...withTemplateFilePrecedence( @@ -468,9 +476,9 @@ async function runInteractiveSetup(options: { installHook?: boolean; uninstallHo return; } - const { config, provider } = setup; + const { config, provider, effectiveKey } = setup; - if (provider.needsApiKey && !config.apiKey && !process.env[provider.apiKeyEnv]) { + if (provider.needsApiKey && !effectiveKey) { await persistSetup(config, options); const apiKeyEnv = pc.cyan(`$${provider.apiKeyEnv}`); const warn = pc.yellow(`\n⚠ No API key provided. Make sure to set ${apiKeyEnv} before running suggestions.`); @@ -478,7 +486,7 @@ async function runInteractiveSetup(options: { installHook?: boolean; uninstallHo return; } - if (!(await testConfiguration(config, provider))) { + if (!(await testConfiguration(config, provider, effectiveKey))) { outro('Setup cancelled.'); return; } From 8b016e5651a1fcc94805f48c847c49471c4ca101 Mon Sep 17 00:00:00 2001 From: 404-Page-Found <139850808+404-Page-Found@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:24:57 +1000 Subject: [PATCH 2/3] Update src/commands/init.ts Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> --- src/commands/init.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/commands/init.ts b/src/commands/init.ts index 8305a6e..df7b112 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -200,7 +200,11 @@ async function promptApiKey( if (isCancel(keyResult)) return null; const effectiveKey = keyResult || envKey || rawConfiguredKey || ''; - const persistKey = keyResult || rawConfiguredKey || undefined; + const persistKey = keyResult + ? keyResult + : rawConfiguredKey !== process.env['COMMIT_ECHO_API_KEY'] + ? rawConfiguredKey + : undefined; return { effectiveKey, persistKey }; } From 9bbd4a221e7723396b6387cddacbed98b493d4f4 Mon Sep 17 00:00:00 2001 From: kragent66-glitch Date: Sat, 26 Sep 2026 12:04:53 +0000 Subject: [PATCH 3/3] fix(init): preserve API-key provenance from raw config Read the stored key straight from loadRawConfig() instead of inferring provenance by comparing it with COMMIT_ECHO_API_KEY, so a stored key that happens to equal the env var is no longer erased on reconfiguration. Also removes the nested ternary flagged by SonarCloud. Run prettier so npm run format:check passes in CI, and add a regression test covering the stored-key-equals-env-key case. --- src/commands/init.ts | 46 +++++++++++++-------- tests/init-api-key-selection.test.mjs | 58 +++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 16 deletions(-) create mode 100644 tests/init-api-key-selection.test.mjs diff --git a/src/commands/init.ts b/src/commands/init.ts index df7b112..fd7ed59 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -186,27 +186,45 @@ async function promptProvider(existingConfig: Config | null): Promise, 'provider' | 'apiKey' | 'baseUrl'> | null, -): Promise<{ effectiveKey: string; persistKey: string | undefined } | null> { +): Promise { if (!provider.needsApiKey) return { effectiveKey: '', persistKey: undefined }; - const rawConfiguredKey = getStoredApiKeyForProvider(provider.providerKey, provider.baseUrl, storedConfig) || ''; + const storedKey = getStoredApiKeyForProvider(provider.providerKey, provider.baseUrl, storedConfig); const envKey = process.env['COMMIT_ECHO_API_KEY']?.trim() || process.env[provider.apiKeyEnv]?.trim() || ''; - const promptKey = rawConfiguredKey || envKey || ''; + const promptKey = storedKey || envKey || ''; const keyResult = await password(buildApiKeyPrompt(promptKey, provider.apiKeyEnv)); if (isCancel(keyResult)) return null; - const effectiveKey = keyResult || envKey || rawConfiguredKey || ''; - const persistKey = keyResult - ? keyResult - : rawConfiguredKey !== process.env['COMMIT_ECHO_API_KEY'] - ? rawConfiguredKey - : undefined; - - return { effectiveKey, persistKey }; + return resolveApiKeySelection(keyResult, storedKey, envKey); } interface PromptModelDependencies { @@ -377,11 +395,7 @@ async function persistSetup( } } -async function testConfiguration( - config: Config, - provider: ProviderSetup, - effectiveKey?: string, -): Promise { +async function testConfiguration(config: Config, provider: ProviderSetup, effectiveKey?: string): Promise { const testSpinner = spinner(); testSpinner.start('Testing connection...'); try { diff --git a/tests/init-api-key-selection.test.mjs b/tests/init-api-key-selection.test.mjs new file mode 100644 index 0000000..71f9949 --- /dev/null +++ b/tests/init-api-key-selection.test.mjs @@ -0,0 +1,58 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { getStoredApiKeyForProvider, resolveApiKeySelection } from '../dist/commands/init.js'; + +const OPENAI_BASE_URL = 'https://api.openai.com/v1'; + +test('preserves an explicitly stored API key when blank input equals the generic env override', () => { + // Regression: provenance used to be inferred by comparing the raw config + // value against COMMIT_ECHO_API_KEY, so a config.json key that happened to + // equal the env var was treated as env-only and silently deleted on save. + const storedKey = getStoredApiKeyForProvider('openai', OPENAI_BASE_URL, { + provider: 'openai', + apiKey: 'sk-same', + }); + + assert.equal(storedKey, 'sk-same'); + + const selection = resolveApiKeySelection('', storedKey, 'sk-same'); + + assert.equal(selection.persistKey, 'sk-same'); + assert.equal(selection.effectiveKey, 'sk-same'); +}); + +test('never persists an API key that only came from the environment', () => { + const selection = resolveApiKeySelection('', undefined, 'sk-env-only'); + + assert.equal(selection.persistKey, undefined); + assert.equal(selection.effectiveKey, 'sk-env-only'); +}); + +test('never persists an API key when neither input nor stored key exists', () => { + const selection = resolveApiKeySelection('', undefined, ''); + + assert.equal(selection.persistKey, undefined); + assert.equal(selection.effectiveKey, ''); +}); + +test('keeps a stored key effective when the prompt is blank and no env var is set', () => { + const selection = resolveApiKeySelection('', 'sk-stored', ''); + + assert.equal(selection.persistKey, 'sk-stored'); + assert.equal(selection.effectiveKey, 'sk-stored'); +}); + +test('persists the key typed during reconfiguration', () => { + const selection = resolveApiKeySelection('sk-typed', 'sk-stored', 'sk-env'); + + assert.equal(selection.persistKey, 'sk-typed'); + assert.equal(selection.effectiveKey, 'sk-typed'); +}); + +test('prefers a typed key over the environment for the effective key', () => { + const selection = resolveApiKeySelection('sk-typed', undefined, 'sk-env'); + + assert.equal(selection.persistKey, 'sk-typed'); + assert.equal(selection.effectiveKey, 'sk-typed'); +});