diff --git a/.gitignore b/.gitignore index 7380fcc..da59067 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,10 @@ node_modules/ # Belongs in ~/.config/cli-tools/blog.json, never in the repository. blog.config.json +# API keys. These belong in ~/.config/cli-tools/credentials.json, 0600, and +# never in a repository — see `cli-tools config`. +credentials.json + # Local environment and credentials, in every form they usually turn up in. .env .env.* diff --git a/README.md b/README.md index 26c5f42..1fdcd75 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ Check what landed, and wire up the pit aliases: ```sh cli-tools list # * runs from here, ! is shadowed by another copy cli-tools aliases --install # /blog /free /merge /prs /whois +cli-tools config # API keys: what is set, and where it came from cli-tools update # git pull, reinstall, relink ``` @@ -90,6 +91,45 @@ pnpm unlink:bin # remove ours ln -sf ~/scripts/bin/gh-prs-merge ~/.local/bin/gh-prs-merge # and so on ``` +## API keys + +`generate-names` needs an OpenAI or Anthropic key. Store one once, and nothing +has to carry it in an environment again: + +```sh +cli-tools config set openai # prompts; the value is never echoed +cli-tools config # what is set, and which source is winning +cli-tools config unset openai +``` + +Keys live in `~/.config/cli-tools/credentials.json`, written `0600` in a `0700` +directory (`$CLI_TOOLS_CREDENTIALS` overrides the path). Nothing prints a whole +key back — `config` shows a masked preview and a length, which is enough to tell +two keys apart and not enough to use one. `--json` is machine-readable and +carries the same masked previews, not the values. + +| Key | Variable | Used by | +| --- | --- | --- | +| `openai` | `OPENAI_API_KEY` | `generate-names` | +| `anthropic` | `ANTHROPIC_API_KEY` | `generate-names` | + +**The environment wins over the file.** A key exported in your shell or injected +by CI overrides a stored one, so a one-off `OPENAI_API_KEY=… generate-names …` +still behaves. Because that is otherwise invisible — you store a key, and the +old one keeps being used — `cli-tools config` reports the *source* of each key +rather than only whether one exists, and says so explicitly when a stored value +is being shadowed. + +A value can be passed inline (`cli-tools config set openai sk-…`) for scripts, +and piped (`… | cli-tools config set openai`) when there is no TTY. Inline is +the worst of the three: it lands in shell history and in `ps`, so the command +warns when you use it interactively. + +This is a machine-local credential store, the same kind of thing as +`~/.aws/credentials` — not a `.env`, not something to copy between machines, and +not where a production secret belongs. A secret that a deployed service needs +goes on that service, with your vault as the record. + ## Usage ### `gh-prs` @@ -201,9 +241,11 @@ and shuffled. Asking a model for 1,000 names directly repeats itself within a few hundred, drifts off-brief, and costs far more — and the call count here is the same whether you ask for 10 names or 10,000. -Needs `OPENAI_API_KEY` or `ANTHROPIC_API_KEY`. Whichever is set is used; -OpenAI wins if both are. Defaults are the cheap tier on each side -(`gpt-4.1-mini` / `claude-haiku-4-5`) and are overridable with `--model`. +Needs a key — `cli-tools config set openai` stores one (see [API +keys](#api-keys)), and `OPENAI_API_KEY` / `ANTHROPIC_API_KEY` still work and +take precedence. Whichever provider has a key is used; OpenAI wins if both do. +Defaults are the cheap tier on each side (`gpt-4.1-mini` / `claude-haiku-4-5`) +and are overridable with `--model`. | Flag | Effect | | --- | --- | diff --git a/bin/cli-tools.ts b/bin/cli-tools.ts index 001ed38..24251d1 100755 --- a/bin/cli-tools.ts +++ b/bin/cli-tools.ts @@ -20,6 +20,15 @@ import { mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { parseArgs, UsageError } from '../src/args.ts'; +import { + credentialsPath, + keyStates, + keyVariable, + KNOWN_KEYS, + loadStored, + mask, + saveStored, +} from '../src/credentials.ts'; import { isMain } from '../src/is-main.ts'; import { aliasesPath, @@ -36,6 +45,7 @@ const USAGE = `Usage: cli-tools link [--force] cli-tools unlink cli-tools aliases [--install] + cli-tools config [set [value] | unset ] cli-tools [args…] Commands: @@ -44,12 +54,17 @@ Commands: link Symlink the commands into ~/.local/bin unlink Remove the symlinks we own aliases Print the moshcode pit aliases, or write them with --install + config API keys: what is set, where it came from, and how to change it where Print the checkout this command is running from +Keys (config set ): + openai OPENAI_API_KEY generate-names + anthropic ANTHROPIC_API_KEY generate-names + Options: --force link: take over a symlink owned by another checkout --install aliases: merge them into ~/.moshcode/aliases.json - --json list/aliases: machine-readable + --json list/aliases/config: machine-readable (config never prints a key) -h, --help `; @@ -127,6 +142,157 @@ function writeAliases(): number { return 0; } +/** + * Read one line without echoing it. + * + * A key typed at a visible prompt ends up in the scrollback of whatever + * terminal, screen share or recording happens to be running, which is most of + * the reason to have this command rather than telling people to edit the file. + * Piped input is read as-is, so `… | cli-tools config set openai` works in a + * script without a TTY. + */ +async function promptSecret(label: string): Promise { + if (!process.stdin.isTTY) { + const chunks: Buffer[] = []; + for await (const chunk of process.stdin) chunks.push(Buffer.from(chunk)); + return Buffer.concat(chunks).toString('utf8').trim(); + } + + process.stderr.write(label); + process.stdin.setRawMode(true); + process.stdin.resume(); + + return new Promise((resolve) => { + let value = ''; + const onData = (chunk: Buffer) => { + for (const byte of chunk) { + // Enter, or EOF/interrupt. + if (byte === 0x0d || byte === 0x0a || byte === 0x04) { + finish(); + return; + } + if (byte === 0x03) { + process.stderr.write('\n'); + process.exit(130); + } + // Backspace / delete. + if (byte === 0x7f || byte === 0x08) { + value = value.slice(0, -1); + continue; + } + value += String.fromCharCode(byte); + } + }; + const finish = () => { + process.stdin.off('data', onData); + process.stdin.setRawMode(false); + process.stdin.pause(); + process.stderr.write('\n'); + resolve(value.trim()); + }; + process.stdin.on('data', onData); + }); +} + +async function configCommand(rest: readonly string[], json: boolean): Promise { + const [verb, name, ...more] = rest; + + if (!verb) { + const states = keyStates(); + if (json) { + process.stdout.write(`${JSON.stringify({ path: credentialsPath(), keys: states }, null, 2)}\n`); + return 0; + } + + process.stdout.write(`${credentialsPath()}\n\n`); + for (const state of states) { + const where = + state.source === 'env' + ? 'environment (overrides the file)' + : state.source === 'file' + ? 'stored' + : 'not set'; + process.stdout.write( + ` ${state.name.padEnd(10)} ${state.variable.padEnd(18)} ${where}\n` + + (state.preview ? `${' '.repeat(13)}${state.preview}\n` : ''), + ); + } + + const shadowed = states.filter((state) => state.source === 'env'); + if (shadowed.length > 0) { + // The failure this heads off: storing a key, still getting the old one, + // and having nothing on screen explain why. + process.stdout.write( + `\nNote: ${shadowed.map((s) => s.variable).join(', ')} ${shadowed.length === 1 ? 'is' : 'are'} set in your environment,\n` + + 'so a stored value would be ignored. Unset the variable to use the stored one.\n', + ); + } + if (states.every((state) => state.source === 'unset')) { + process.stdout.write('\nNothing set. Add one with:\n cli-tools config set openai\n'); + } + return 0; + } + + if (verb !== 'set' && verb !== 'unset') { + process.stderr.write(`config: unknown verb "${verb}" (expected set or unset)\n`); + return 1; + } + + if (!name) { + process.stderr.write(`config ${verb}: name a key — ${Object.keys(KNOWN_KEYS).join(', ')}\n`); + return 1; + } + + const variable = keyVariable(name); + if (!variable) { + process.stderr.write( + `config: unknown key "${name}". Known keys: ${Object.keys(KNOWN_KEYS).join(', ')}\n`, + ); + return 1; + } + + const stored = loadStored(); + + if (verb === 'unset') { + if (!Object.hasOwn(stored, variable)) { + process.stdout.write(`config: ${variable} was not stored — nothing to remove.\n`); + return 0; + } + delete stored[variable]; + process.stdout.write(`config: removed ${variable} from ${saveStored(stored)}\n`); + return 0; + } + + // An inline value is accepted because scripts need it, but it lands in shell + // history and the process list, so the prompt is the default and this says so. + let value = more.length > 0 ? more.join(' ').trim() : ''; + if (!value) { + value = await promptSecret(`${variable}: `); + } else if (process.stdin.isTTY) { + process.stderr.write( + 'config: a value on the command line is visible in shell history and `ps`.\n' + + ` Prefer \`cli-tools config set ${name}\` and type it at the prompt.\n`, + ); + } + + if (!value) { + process.stderr.write('config: no value given — nothing stored.\n'); + return 1; + } + + stored[variable] = value; + const path = saveStored(stored); + process.stdout.write(`config: stored ${variable} (${mask(value)}) in ${path}\n`); + + if (process.env[variable]) { + process.stdout.write( + `\nNote: ${variable} is also set in your environment, which wins.\n` + + ` Unset it for the stored value to take effect.\n`, + ); + } + return 0; +} + export async function run(argv: readonly string[]): Promise { // The first word is the command, and everything after it belongs to that // command — parsed here only for our own verbs, and passed through untouched @@ -145,7 +311,7 @@ export async function run(argv: readonly string[]): Promise { // Anything that is not one of ours is one of the commands: pass it straight // through, arguments and streams untouched, so `cli-tools gh-prs --orgs x` // behaves exactly as `gh-prs --orgs x` does. - const known = new Set(['list', 'update', 'link', 'unlink', 'aliases', 'where']); + const known = new Set(['list', 'update', 'link', 'unlink', 'aliases', 'config', 'where']); if (!known.has(command)) { const match = commands(root).find((entry) => entry.name === command); if (!match) { @@ -172,6 +338,12 @@ export async function run(argv: readonly string[]): Promise { process.stdout.write(`${root}\n`); return 0; + // positional, so `--json` is a flag here rather than part of a key's value. + // A value that begins with a dash cannot be passed inline for the same + // reason; type it at the prompt, which is the better habit anyway. + case 'config': + return configCommand(options.positional, options.flags.has('--json')); + case 'list': { const binDir = join(root, 'bin'); const all = commands(root).map((entry) => ({ diff --git a/bin/generate-names.ts b/bin/generate-names.ts index df6a199..0c399f7 100755 --- a/bin/generate-names.ts +++ b/bin/generate-names.ts @@ -8,6 +8,7 @@ */ import { UsageError, integer, parseArgs } from '../src/args.ts'; +import { resolveCredentials } from '../src/credentials.ts'; import { isMain } from '../src/is-main.ts'; import { DEFAULT_COUNT, @@ -37,8 +38,15 @@ Options: --timeout MS API timeout (default: 60000) -h, --help show this help -Needs OPENAI_API_KEY or ANTHROPIC_API_KEY. Names go to stdout and nothing -else does, so the output pipes cleanly. +Needs an OpenAI or Anthropic key. Store one once: + + cli-tools config set openai # prompts, nothing echoed or logged + cli-tools config # what is set, and where it came from + +kept 0600 in ~/.config/cli-tools/credentials.json. OPENAI_API_KEY and +ANTHROPIC_API_KEY still work and take precedence over a stored key. + +Names go to stdout and nothing else does, so the output pipes cleanly. `; if (isMain(import.meta.url)) { @@ -70,16 +78,20 @@ if (isMain(import.meta.url)) { const tld = (values.get('--tld') ?? DEFAULT_TLD).replace(/^\./, ''); if (!/^[a-z]{2,}$/i.test(tld)) throw new UsageError(`--tld must be letters, got "${tld}"`); + // Stored keys first, environment on top — see src/credentials.ts. Shaped + // as an environment record so resolveProvider needs no change. + const credentials = resolveCredentials(process.env); + let provider; try { - provider = resolveProvider(process.env, values.get('--provider')); + provider = resolveProvider(credentials, values.get('--provider')); } catch (error) { // A bad --provider is a typo and a missing key is a setup problem; both // are the caller's to fix, so report them like any other usage error. throw new UsageError(error instanceof Error ? error.message : String(error)); } const model = values.get('--model') ?? DEFAULT_MODELS[provider]; - const apiKey = process.env[provider === 'openai' ? 'OPENAI_API_KEY' : 'ANTHROPIC_API_KEY']!; + const apiKey = credentials[provider === 'openai' ? 'OPENAI_API_KEY' : 'ANTHROPIC_API_KEY']!; const call = provider === 'openai' ? openaiCaller(apiKey, model, timeout) diff --git a/plugins/tools/commands/config.md b/plugins/tools/commands/config.md new file mode 100644 index 0000000..c15f270 --- /dev/null +++ b/plugins/tools/commands/config.md @@ -0,0 +1,64 @@ +--- +description: Store the API keys the cli-tools commands need, and see which source is winning. +allowed-tools: Bash(cli-tools:*), Read +--- + +## Task + +Set up, inspect or clear the API keys `cli-tools` commands use. + +```bash +cli-tools config # what is set, and where each key came from +cli-tools config set openai # prompts; the value is never echoed +cli-tools config set anthropic +cli-tools config unset openai +cli-tools config --json # machine-readable, still masked +``` + +Keys live in `~/.config/cli-tools/credentials.json`, written `0600` inside a +`0700` directory. `$CLI_TOOLS_CREDENTIALS` overrides the path. + +| Key | Variable | Used by | +| --- | --- | --- | +| `openai` | `OPENAI_API_KEY` | `generate-names` | +| `anthropic` | `ANTHROPIC_API_KEY` | `generate-names` | + +## Never print a key + +Nothing here prints a whole key, and neither should you. `config` shows a masked +preview and a character count — enough to tell two keys apart, not enough to use +one — and `--json` carries the same previews rather than the values. + +If someone needs the real value, it is in the file; read it deliberately rather +than by running a command that dumps it into a transcript. + +## The environment wins + +An exported `OPENAI_API_KEY` overrides a stored one, so a one-off +`OPENAI_API_KEY=… generate-names …` and a CI-injected key both still work. + +That precedence is the thing that confuses people: you store a key, and the old +one keeps being used, with nothing on screen to say why. So `config` reports the +**source** of each key — `environment (overrides the file)`, `stored`, or +`not set` — and states plainly when a stored value is being shadowed. If a key +looks stored but is not taking effect, that line is the answer; unset the +variable. + +## Setting one non-interactively + +```bash +printf '%s' "$KEY" | cli-tools config set openai # piped, no TTY needed +cli-tools config set openai sk-… # inline — see below +``` + +Prefer the pipe. An inline value lands in shell history and is visible in `ps` +to every process on the box for as long as the command runs, which is why the +command warns about it when used interactively. + +## What this is not + +A machine-local credential store, the same kind of thing as `~/.aws/credentials` +or `gh auth` — one machine's own copy. It is not a `.env`: nothing loads it into +an environment wholesale, nothing syncs it, and it is not how a key travels +between machines. A secret a deployed service needs belongs on that service, +with the vault as the record. diff --git a/src/credentials.ts b/src/credentials.ts new file mode 100644 index 0000000..fb19c44 --- /dev/null +++ b/src/credentials.ts @@ -0,0 +1,140 @@ +import { chmodSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { dirname, join } from 'node:path'; + +/** + * API keys the commands here need, kept on this machine. + * + * This is a credential store in the sense `~/.aws/credentials` or `gh auth` is + * one: a single machine's own copy, 0600, never committed and never handed to + * anyone. It is deliberately not a `.env` — nothing reads it into a process + * environment wholesale, nothing syncs it, and it is not how a secret travels + * between machines. A production secret still belongs on the service that runs + * it, with the vault as the record. + * + * The environment always wins over the file. A key exported in the shell, or + * injected by CI, has to be able to override a stale stored one — and because + * that is invisible when it happens, `cli-tools config` reports which source + * each key is coming from rather than only whether one exists. + */ + +/** Friendly name → the environment variable the tools already read. */ +export const KNOWN_KEYS: Record = { + openai: 'OPENAI_API_KEY', + anthropic: 'ANTHROPIC_API_KEY', +}; + +export type Source = 'env' | 'file' | 'unset'; + +export interface KeyState { + name: string; + variable: string; + source: Source; + /** Masked, never the whole value. */ + preview: string | null; +} + +function xdgConfigHome(env: NodeJS.ProcessEnv): string { + return env.XDG_CONFIG_HOME || join(homedir(), '.config'); +} + +export function credentialsPath(env: NodeJS.ProcessEnv = process.env): string { + return env.CLI_TOOLS_CREDENTIALS || join(xdgConfigHome(env), 'cli-tools', 'credentials.json'); +} + +/** Resolve a friendly name or an env var name to the env var name, or null. */ +export function keyVariable(name: string): string | null { + const key = String(name ?? '') + .trim() + .toLowerCase() + .replace(/[-_]?(api[-_]?)?key$/, ''); + if (Object.hasOwn(KNOWN_KEYS, key)) return KNOWN_KEYS[key]!; + + const upper = String(name ?? '') + .trim() + .toUpperCase(); + return Object.values(KNOWN_KEYS).includes(upper) ? upper : null; +} + +/** + * Read the stored keys. + * + * A missing file is the normal first-run case. Malformed JSON is an error, + * because falling back to "no keys" would surface as a confusing "set + * OPENAI_API_KEY" message pointing at the environment rather than at the file + * that is actually broken. + */ +export function loadStored(env: NodeJS.ProcessEnv = process.env): Record { + const path = credentialsPath(env); + let text: string; + try { + text = readFileSync(path, 'utf8'); + } catch { + return {}; + } + + let parsed: unknown; + try { + parsed = JSON.parse(text); + } catch (error) { + throw new Error(`${path}: not valid JSON — ${(error as Error).message}`); + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return {}; + + const stored: Record = {}; + for (const [name, value] of Object.entries(parsed as Record)) { + if (typeof value === 'string' && value.trim()) stored[name] = value.trim(); + } + return stored; +} + +/** Write the store, readable only by its owner. */ +export function saveStored( + stored: Record, + env: NodeJS.ProcessEnv = process.env, +): string { + const path = credentialsPath(env); + // 0700 on the directory as well: a 0600 file inside a world-readable new + // directory is only half the protection, and this may be creating both. + mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); + writeFileSync(path, `${JSON.stringify(stored, null, 2)}\n`, { mode: 0o600 }); + // writeFileSync's mode applies only when it creates the file, so an existing + // one keeps whatever it had — including a permissive mode from a hand edit. + chmodSync(path, 0o600); + return path; +} + +/** + * The keys as the tools should see them: stored first, environment on top. + * + * Shaped as an environment record on purpose, so callers that already read + * `process.env` take it without further change. + */ +export function resolveCredentials( + env: NodeJS.ProcessEnv = process.env, +): Record { + const merged: Record = { ...loadStored(env) }; + for (const variable of Object.values(KNOWN_KEYS)) { + if (env[variable]) merged[variable] = env[variable]; + } + return merged; +} + +/** Show enough of a key to recognise it, never enough to use it. */ +export function mask(value: string): string { + const text = String(value ?? ''); + if (text.length <= 8) return '*'.repeat(text.length); + return `${text.slice(0, 5)}…${text.slice(-4)} (${text.length} chars)`; +} + +/** Where each known key is coming from, for `cli-tools config`. */ +export function keyStates(env: NodeJS.ProcessEnv = process.env): KeyState[] { + const stored = loadStored(env); + return Object.entries(KNOWN_KEYS).map(([name, variable]) => { + const fromEnv = env[variable]; + if (fromEnv) return { name, variable, source: 'env' as const, preview: mask(fromEnv) }; + const fromFile = stored[variable]; + if (fromFile) return { name, variable, source: 'file' as const, preview: mask(fromFile) }; + return { name, variable, source: 'unset' as const, preview: null }; + }); +} diff --git a/src/generate-names.ts b/src/generate-names.ts index 961af6e..e9289c3 100644 --- a/src/generate-names.ts +++ b/src/generate-names.ts @@ -48,7 +48,10 @@ export function resolveProvider( } if (env['OPENAI_API_KEY']) return 'openai'; if (env['ANTHROPIC_API_KEY']) return 'anthropic'; - throw new Error('set OPENAI_API_KEY or ANTHROPIC_API_KEY (or pass --provider)'); + throw new Error( + 'no API key — run `cli-tools config set openai` (or anthropic), ' + + 'or export OPENAI_API_KEY / ANTHROPIC_API_KEY', + ); } export function buildPrompt(description: string, words: 1 | 2): string { diff --git a/test/credentials.test.ts b/test/credentials.test.ts new file mode 100644 index 0000000..4255ccb --- /dev/null +++ b/test/credentials.test.ts @@ -0,0 +1,219 @@ +import { mkdtemp, rm, stat, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { + credentialsPath, + keyStates, + keyVariable, + loadStored, + mask, + resolveCredentials, + saveStored, +} from '../src/credentials.ts'; +import { resolveProvider } from '../src/generate-names.ts'; + +const dirs: string[] = []; + +afterEach(async () => { + await Promise.all(dirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); +}); + +async function sandbox(): Promise { + const dir = await mkdtemp(join(tmpdir(), 'credentials-')); + dirs.push(dir); + return { XDG_CONFIG_HOME: dir } as NodeJS.ProcessEnv; +} + +describe('credentialsPath', () => { + it('lives under the XDG config dir', () => { + expect(credentialsPath({ XDG_CONFIG_HOME: '/xdg' } as NodeJS.ProcessEnv)).toBe( + '/xdg/cli-tools/credentials.json', + ); + }); + + it('falls back to ~/.config', () => { + expect(credentialsPath({} as NodeJS.ProcessEnv)).toMatch( + /\.config\/cli-tools\/credentials\.json$/, + ); + }); + + it('honours an explicit override', () => { + expect(credentialsPath({ CLI_TOOLS_CREDENTIALS: '/tmp/k.json' } as NodeJS.ProcessEnv)).toBe( + '/tmp/k.json', + ); + }); +}); + +describe('keyVariable', () => { + it('accepts the friendly name, with or without a key suffix', () => { + expect(keyVariable('openai')).toBe('OPENAI_API_KEY'); + expect(keyVariable('openai-key')).toBe('OPENAI_API_KEY'); + expect(keyVariable('openai_api_key')).toBe('OPENAI_API_KEY'); + expect(keyVariable('Anthropic')).toBe('ANTHROPIC_API_KEY'); + }); + + it('accepts the environment variable name itself', () => { + expect(keyVariable('OPENAI_API_KEY')).toBe('OPENAI_API_KEY'); + expect(keyVariable('anthropic_api_key')).toBe('ANTHROPIC_API_KEY'); + }); + + it('rejects anything else rather than inventing a variable', () => { + expect(keyVariable('gemini')).toBeNull(); + expect(keyVariable('')).toBeNull(); + }); +}); + +describe('mask', () => { + it('shows enough to recognise a key and not enough to use it', () => { + const masked = mask('sk-proj-abcdefghijklmnop1234'); + expect(masked).toContain('sk-pr'); + expect(masked).toContain('1234'); + expect(masked).not.toContain('abcdefghijkl'); + }); + + it('reveals nothing at all from a short value', () => { + expect(mask('short')).toBe('*****'); + }); +}); + +describe('saveStored / loadStored', () => { + it('round-trips', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-test-value-1234' }, env); + expect(loadStored(env)).toEqual({ OPENAI_API_KEY: 'sk-test-value-1234' }); + }); + + // A key readable by every account on the box is not stored, it is published. + it('writes the file 0600 and the directory 0700', async () => { + const env = await sandbox(); + const path = saveStored({ OPENAI_API_KEY: 'sk-test-value-1234' }, env); + + expect((await stat(path)).mode & 0o777).toBe(0o600); + expect((await stat(join(env.XDG_CONFIG_HOME!, 'cli-tools'))).mode & 0o777).toBe(0o700); + }); + + // writeFileSync's mode applies only when it creates the file, so a file left + // permissive by a hand edit would otherwise stay that way forever. + it('tightens the mode of a file that already exists', async () => { + const env = await sandbox(); + const path = saveStored({ OPENAI_API_KEY: 'first-value-here' }, env); + await writeFile(path, '{}', { mode: 0o644 }); + + saveStored({ OPENAI_API_KEY: 'second-value-here' }, env); + expect((await stat(path)).mode & 0o777).toBe(0o600); + }); + + it('treats a missing file as no keys', async () => { + expect(loadStored(await sandbox())).toEqual({}); + }); + + // Falling back to "no keys" would surface as a message about the environment, + // pointing away from the file that is actually broken. + it('refuses malformed JSON, naming the file', async () => { + const env = await sandbox(); + const path = saveStored({}, env); + await writeFile(path, '{ not json'); + + expect(() => loadStored(env)).toThrow(path); + }); + + it('drops blank and non-string values', async () => { + const env = await sandbox(); + const path = saveStored({}, env); + await writeFile(path, JSON.stringify({ OPENAI_API_KEY: ' ', ANTHROPIC_API_KEY: 42 })); + + expect(loadStored(env)).toEqual({}); + }); +}); + +describe('resolveCredentials', () => { + it('returns the stored key when the environment has none', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-stored-value-1' }, env); + + expect(resolveCredentials(env).OPENAI_API_KEY).toBe('sk-stored-value-1'); + }); + + // CI and a one-off `KEY=… command` both depend on this precedence. + it('lets the environment override the stored key', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-stored-value-1' }, env); + env.OPENAI_API_KEY = 'sk-env-value-2'; + + expect(resolveCredentials(env).OPENAI_API_KEY).toBe('sk-env-value-2'); + }); + + it('leaves a stored key alone when a different variable is exported', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-stored-value-1' }, env); + env.ANTHROPIC_API_KEY = 'sk-ant-value-2'; + + const resolved = resolveCredentials(env); + expect(resolved.OPENAI_API_KEY).toBe('sk-stored-value-1'); + expect(resolved.ANTHROPIC_API_KEY).toBe('sk-ant-value-2'); + }); +}); + +// The wiring generate-names depends on: resolveCredentials produces an +// environment-shaped record, so resolveProvider consumes it unchanged and a +// stored key selects a provider exactly as an exported one does. +describe('resolveCredentials feeding resolveProvider', () => { + it('selects a provider from a stored key alone', async () => { + const env = await sandbox(); + saveStored({ ANTHROPIC_API_KEY: 'sk-ant-stored-1234' }, env); + + expect(resolveProvider(resolveCredentials(env))).toBe('anthropic'); + }); + + it('honours --provider against a stored key', async () => { + const env = await sandbox(); + saveStored({ ANTHROPIC_API_KEY: 'sk-ant-stored-1234' }, env); + + expect(resolveProvider(resolveCredentials(env), 'anthropic')).toBe('anthropic'); + expect(() => resolveProvider(resolveCredentials(env), 'openai')).toThrow(/OPENAI_API_KEY/); + }); + + it('still reports no key when neither source has one', async () => { + const env = await sandbox(); + expect(() => resolveProvider(resolveCredentials(env))).toThrow(/cli-tools config set openai/); + }); +}); + +describe('keyStates', () => { + it('reports each key as env, file or unset', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-stored-value-1' }, env); + env.ANTHROPIC_API_KEY = 'sk-ant-value-2'; + + const byName = Object.fromEntries(keyStates(env).map((state) => [state.name, state])); + expect(byName.openai!.source).toBe('file'); + expect(byName.anthropic!.source).toBe('env'); + }); + + it('reports a stored key shadowed by the environment as env, not file', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-stored-value-1' }, env); + env.OPENAI_API_KEY = 'sk-env-value-2'; + + const openai = keyStates(env).find((state) => state.name === 'openai')!; + expect(openai.source).toBe('env'); + // This is what makes "I stored it and it still uses the old one" visible. + expect(openai.preview).toContain('sk-en'); + }); + + it('never puts a whole key in the state', async () => { + const env = await sandbox(); + saveStored({ OPENAI_API_KEY: 'sk-secret-value-abcdefgh' }, env); + + for (const state of keyStates(env)) { + expect(state.preview ?? '').not.toContain('sk-secret-value-abcdefgh'); + } + }); + + it('reports unset when nothing is configured', async () => { + const env = await sandbox(); + expect(keyStates(env).every((state) => state.source === 'unset')).toBe(true); + }); +}); diff --git a/test/generate-names.test.ts b/test/generate-names.test.ts index 02ce740..526a599 100644 --- a/test/generate-names.test.ts +++ b/test/generate-names.test.ts @@ -34,8 +34,13 @@ describe('resolveProvider', () => { expect(() => resolveProvider({ OPENAI_API_KEY: 'x' }, 'gemini')).toThrow(/expected openai or anthropic/); }); + // The message names the command that fixes it, not just the variable: a key + // can now be stored as well as exported, and "set OPENAI_API_KEY" sent people + // to their shell profile when `cli-tools config set openai` is the better + // answer and the one that does not leave the key in a dotfile. it('explains what to set when there is no key at all', () => { - expect(() => resolveProvider({})).toThrow(/OPENAI_API_KEY or ANTHROPIC_API_KEY/); + expect(() => resolveProvider({})).toThrow(/cli-tools config set openai/); + expect(() => resolveProvider({})).toThrow(/OPENAI_API_KEY/); }); });