fix(init): stop persisting env-var API keys into config.json

src/commands/init.ts

@@ -186,21 +186,45 @@ async function promptProvider(existingConfig: Config | null): Promise<ProviderSe
   return { providerKey, baseUrl: info.baseUrl, apiKeyEnv: info.apiKeyEnv, needsApiKey: info.needsApiKey };
 }
 
+export interface ApiKeySelection {
+  effectiveKey: string;
+  persistKey: string | undefined;
+}
+
+/**
+ * Decide which API key is used for this run (`effectiveKey`) and which key, if
+ * any, may be written back to config.json (`persistKey`).
+ *
+ * Provenance comes from the raw config file read, never from comparing values:
+ * `storedKey` is set only when config.json explicitly holds a key for the
+ * selected provider. Environment variables therefore cannot smuggle a key onto
+ * disk, and a stored key that happens to equal an env var is still preserved.
+ */
+export function resolveApiKeySelection(
+  userInput: string,
+  storedKey: string | undefined,
+  envKey: string,
+): ApiKeySelection {
+  return {
+    effectiveKey: userInput || envKey || storedKey || '',
+    persistKey: userInput || storedKey,
+  };
+}
+
 async function promptApiKey(
   provider: ProviderSetup,
   storedConfig: Pick<Partial<Config>, 'provider' | 'apiKey' | 'baseUrl'> | null,
-): Promise<string | undefined | null> {
-  if (!provider.needsApiKey) return undefined;
-
-  const existingKey = getExistingApiKeyForProvider(
-    provider.providerKey,
-    provider.baseUrl,
-    storedConfig,
-    provider.apiKeyEnv,
-  );
-  const keyResult = await password(buildApiKeyPrompt(existingKey, provider.apiKeyEnv));
+): Promise<ApiKeySelection | null> {
+  if (!provider.needsApiKey) return { effectiveKey: '', persistKey: undefined };
+
+  const storedKey = getStoredApiKeyForProvider(provider.providerKey, provider.baseUrl, storedConfig);
+  const envKey = process.env['COMMIT_ECHO_API_KEY']?.trim() || process.env[provider.apiKeyEnv]?.trim() || '';
+  const promptKey = storedKey || envKey || '';
+
+  const keyResult = await password(buildApiKeyPrompt(promptKey, provider.apiKeyEnv));
   if (isCancel(keyResult)) return null;
-  return keyResult || existingKey || '';
+
+  return resolveApiKeySelection(keyResult, storedKey, envKey);
 }
 
 interface PromptModelDependencies {
@@ -371,11 +395,11 @@ async function persistSetup(
   }
 }
 
-async function testConfiguration(config: Config, provider: ProviderSetup): Promise<boolean> {
+async function testConfiguration(config: Config, provider: ProviderSetup, effectiveKey?: string): Promise<boolean> {
   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 +428,7 @@ function buildTemplateInfo(config: Config): string {
 interface CollectedSetup {
   config: Config;
   provider: ProviderSetup;
+  effectiveKey: string;
 }
 
 async function collectConfig(
@@ -413,10 +438,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 +452,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,17 +494,17 @@ 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.`);
     outro(warn);
     return;
   }
 
-  if (!(await testConfiguration(config, provider))) {
+  if (!(await testConfiguration(config, provider, effectiveKey))) {
     outro('Setup cancelled.');
     return;
   }

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');
+});