diff --git a/src/commands/config.ts b/src/commands/config.ts index 9b3ed33..d9e0b05 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -117,13 +117,21 @@ function updateConfigField(config: Config, key: K, value function applyProviderChange(config: Config, provider: Config['provider']): Config { const nextConfig = updateConfigField(config, 'provider', provider); - if (provider === CUSTOM_PROVIDER_KEY) { + if (config.provider === provider) { return nextConfig; } + if (provider === CUSTOM_PROVIDER_KEY) { + return { + ...nextConfig, + apiKey: undefined, + }; + } + return { ...nextConfig, baseUrl: undefined, + apiKey: undefined, }; } diff --git a/src/commands/init.ts b/src/commands/init.ts index 8861e00..a0651a2 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -9,7 +9,7 @@ import { getProviderInfo, fetchModels, } from '../providers/index.js'; -import { saveConfig, configExists, loadConfig } from '../config/store.js'; +import { saveConfig, configExists, loadConfig, loadRawConfig } from '../config/store.js'; import type { Config } from '../types.js'; import { getAvailableTemplateVars } from '../llm/prompt.js'; import { installCommitHooks, uninstallCommitHooks } from '../git/hook.js'; @@ -30,6 +30,46 @@ export function buildApiKeyPrompt(existingKey: string, apiKeyEnv: string) { }; } +export function getStoredApiKeyForProvider( + selectedProvider: string, + selectedBaseUrl: string | undefined, + storedConfig: Pick, 'provider' | 'apiKey' | 'baseUrl'> | null, +): string | undefined { + if (storedConfig?.provider !== selectedProvider || typeof storedConfig.apiKey !== 'string') { + return undefined; + } + + if (selectedProvider === CUSTOM_PROVIDER_KEY) { + if (typeof storedConfig.baseUrl !== 'string' || typeof selectedBaseUrl !== 'string') { + return undefined; + } + + const storedUrl = normalizeBaseUrl(storedConfig.baseUrl); + const selectedUrl = normalizeBaseUrl(selectedBaseUrl); + if (storedUrl !== selectedUrl) { + return undefined; + } + } + + const storedKey = storedConfig.apiKey.trim(); + return storedKey || undefined; +} + +export function getExistingApiKeyForProvider( + provider: string, + baseUrl: string | undefined, + storedConfig: Pick, 'provider' | 'apiKey' | 'baseUrl'> | null, + apiKeyEnv: string, + env: NodeJS.ProcessEnv = process.env, +): string { + return ( + env['COMMIT_ECHO_API_KEY']?.trim() || + getStoredApiKeyForProvider(provider, baseUrl, storedConfig) || + env[apiKeyEnv]?.trim() || + '' + ); +} + /** * A template file takes precedence over inline templates at runtime, so drop * inline values when a templatePath is set to keep the saved config consistent @@ -146,11 +186,16 @@ async function promptProvider(existingConfig: Config | null): Promise, 'provider' | 'apiKey' | 'baseUrl'> | null, ): Promise { if (!provider.needsApiKey) return undefined; - const existingKey = existingConfig?.apiKey ?? process.env[provider.apiKeyEnv] ?? ''; + const existingKey = getExistingApiKeyForProvider( + provider.providerKey, + provider.baseUrl, + storedConfig, + provider.apiKeyEnv, + ); const keyResult = await text(buildApiKeyPrompt(existingKey, provider.apiKeyEnv)); if (isCancel(keyResult)) return null; return keyResult || existingKey || ''; @@ -339,11 +384,14 @@ interface CollectedSetup { provider: ProviderSetup; } -async function collectConfig(existingConfig: Config | null): Promise { +async function collectConfig( + existingConfig: Config | null, + storedConfig: Pick, 'provider' | 'apiKey' | 'baseUrl'> | null, +): Promise { const provider = await promptProvider(existingConfig); if (!provider) return null; - const apiKey = await promptApiKey(provider, existingConfig); + const apiKey = await promptApiKey(provider, storedConfig); if (apiKey === null) return null; const selectedModel = await promptModel(provider, apiKey, existingConfig); @@ -379,6 +427,7 @@ async function runInteractiveSetup(options: { installHook?: boolean; uninstallHo const isReconfig = configExists(); const existingConfig = isReconfig ? await loadConfig().catch(() => null) : null; + const storedConfig = isReconfig ? await loadRawConfig().catch(() => null) : null; if (isReconfig) { const reconfirm = await confirm({ @@ -391,7 +440,7 @@ async function runInteractiveSetup(options: { installHook?: boolean; uninstallHo } } - const setup = await collectConfig(existingConfig); + const setup = await collectConfig(existingConfig, storedConfig); if (!setup) { outro('Setup cancelled.'); return; diff --git a/tests/config-command.test.mjs b/tests/config-command.test.mjs index bd5cf89..5924768 100644 --- a/tests/config-command.test.mjs +++ b/tests/config-command.test.mjs @@ -6,6 +6,7 @@ import { join } from 'node:path'; import test from 'node:test'; import { promisify } from 'node:util'; import { maskApiKey } from '../dist/commands/config.js'; +import { resolveApiKey } from '../dist/llm/client.js'; import { assertFriendlyCommandError, assertJsonCommandError, writeInvalidConfig } from './cli-error-helpers.mjs'; const execFileAsync = promisify(execFile); @@ -358,7 +359,7 @@ test('config set rejects unknown provider keys and lists valid options', async ( }); }); -test('config set clears stale baseUrl when switching away from custom provider', async () => { +test('config set clears stale baseUrl and API key when switching away from custom provider', async () => { await withTempHome(async (homeDir) => { writeConfig(homeDir, { apiKey: 'sk-still-valid-for-next-provider', @@ -371,7 +372,34 @@ test('config set clears stale baseUrl when switching away from custom provider', assert.equal(config.provider, 'openai'); assert.equal(config.baseUrl, undefined); - assert.equal(config.apiKey, 'sk-still-valid-for-next-provider'); + assert.equal(config.apiKey, undefined); + }); +}); + +test('config set clears the API key when switching providers and uses the new provider env key', async () => { + await withTempHome(async (homeDir) => { + writeConfig(homeDir, { + provider: 'openai', + apiKey: 'sk-openai', + }); + + await runConfigWithArgs(homeDir, ['set', 'provider', 'anthropic']); + const config = readConfig(homeDir); + + assert.equal(config.provider, 'anthropic'); + assert.equal(config.apiKey, undefined); + + const original = process.env.ANTHROPIC_API_KEY; + process.env.ANTHROPIC_API_KEY = 'sk-anthropic'; + try { + assert.equal(resolveApiKey(config), 'sk-anthropic'); + } finally { + if (original === undefined) { + delete process.env.ANTHROPIC_API_KEY; + } else { + process.env.ANTHROPIC_API_KEY = original; + } + } }); }); diff --git a/tests/init-api-key-prompt.test.mjs b/tests/init-api-key-prompt.test.mjs index 2bac976..df08f91 100644 --- a/tests/init-api-key-prompt.test.mjs +++ b/tests/init-api-key-prompt.test.mjs @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { buildApiKeyPrompt } from '../dist/commands/init.js'; +import { buildApiKeyPrompt, getExistingApiKeyForProvider, getStoredApiKeyForProvider } from '../dist/commands/init.js'; test('does not prefill an existing API key in the init prompt', () => { const prompt = buildApiKeyPrompt('sk-live-secret', 'OPENAI_API_KEY'); @@ -16,3 +16,136 @@ test('leaves the API key prompt blank for new configs', () => { assert.equal(prompt.placeholder, ''); assert.equal(Object.hasOwn(prompt, 'initialValue'), false); }); + +test('reuses a trimmed stored API key only for the same provider', () => { + assert.equal( + getStoredApiKeyForProvider('openai', 'https://api.openai.com/v1', { + provider: 'openai', + apiKey: ' sk-openai ', + }), + 'sk-openai', + ); + assert.equal( + getStoredApiKeyForProvider('anthropic', 'https://api.anthropic.com/v1', { + provider: 'openai', + apiKey: 'sk-openai', + }), + undefined, + ); +}); + +test('reuses a custom API key only for the same normalized endpoint', () => { + const storedConfig = { + provider: '__custom__', + baseUrl: 'https://api.example.com/v1', + apiKey: ' sk-custom ', + }; + + assert.equal( + getStoredApiKeyForProvider('__custom__', 'https://api.example.com/v1/', storedConfig), + 'sk-custom', + ); + assert.equal( + getStoredApiKeyForProvider('__custom__', 'https://other.example.com/v1', storedConfig), + undefined, + ); +}); + +test('ignores a non-string stored API key for the selected provider', () => { + assert.equal( + getStoredApiKeyForProvider('openai', 'https://api.openai.com/v1', { provider: 'openai', apiKey: 123 }), + undefined, + ); +}); + +test('does not reuse a custom API key when either endpoint is missing or malformed', () => { + assert.equal( + getStoredApiKeyForProvider('__custom__', 'https://api.example.com/v1', { + provider: '__custom__', + baseUrl: 123, + apiKey: 'sk-custom', + }), + undefined, + ); + assert.equal( + getStoredApiKeyForProvider('__custom__', 'https://api.example.com/v1', { + provider: '__custom__', + apiKey: 'sk-custom', + }), + undefined, + ); + assert.equal( + getStoredApiKeyForProvider('__custom__', undefined, { + provider: '__custom__', + baseUrl: 'https://api.example.com/v1', + apiKey: 'sk-custom', + }), + undefined, + ); +}); + +test('prefers the trimmed generic API key env override during init reconfiguration', () => { + const env = { + COMMIT_ECHO_API_KEY: ' sk-generic ', + OPENAI_API_KEY: 'sk-openai', + }; + + assert.equal( + getExistingApiKeyForProvider( + 'openai', + 'https://api.openai.com/v1', + { provider: 'openai', apiKey: 'sk-stored' }, + 'OPENAI_API_KEY', + env, + ), + 'sk-generic', + ); +}); + +test('falls back to the provider-specific API key env when no generic or matching stored key exists', () => { + const env = { OPENAI_API_KEY: ' sk-openai ' }; + + assert.equal( + getExistingApiKeyForProvider( + 'openai', + 'https://api.openai.com/v1', + { provider: 'anthropic', apiKey: 'sk-anthropic' }, + 'OPENAI_API_KEY', + env, + ), + 'sk-openai', + ); + assert.equal( + getExistingApiKeyForProvider('openai', 'https://api.openai.com/v1', null, 'OPENAI_API_KEY', {}), + '', + ); +}); + +test('skips empty API-key candidates after trimming', () => { + assert.equal( + getExistingApiKeyForProvider( + 'openai', + 'https://api.openai.com/v1', + { provider: 'openai', apiKey: ' sk-stored ' }, + 'OPENAI_API_KEY', + { + COMMIT_ECHO_API_KEY: ' ', + OPENAI_API_KEY: 'sk-openai', + }, + ), + 'sk-stored', + ); + + assert.equal( + getExistingApiKeyForProvider( + 'openai', + 'https://api.openai.com/v1', + { provider: 'openai', apiKey: ' ' }, + 'OPENAI_API_KEY', + { + OPENAI_API_KEY: ' sk-openai ', + }, + ), + 'sk-openai', + ); +});