From 9296d3dc31702392f308dc6d5857b53378c9f8b7 Mon Sep 17 00:00:00 2001 From: ebreen Date: Wed, 2 Sep 2026 22:43:23 +0200 Subject: [PATCH] fix(auth): stop the API key placeholder from shadowing Oh My Pi /login credentials (#78) Oh My Pi kept the unresolved $COMMAND_CODE_API_KEY placeholder as a literal config API key that shadowed its /login credential store and was sent as the Bearer token (401). The placeholder is now registered only on pi, where it keeps the API-key auth method and --api-key working next to OAuth; on OMP the provider omits apiKey unless a real key is configured. Host-supplied placeholders are resolved or stripped on every stream path, and the legacy generate transport uses the same rule. Stored /login OAuth and API-key credentials, --api-key, and env keys are now covered end to end on both pi and Oh My Pi, and CI runs the pi suite against a real binary. Co-authored-by: ebreen --- .github/workflows/ci.yml | 12 ++- CHANGELOG.md | 3 + CONTRIBUTING.md | 4 + README.md | 2 + index.ts | 54 +++++++++--- src/converters.ts | 25 +++++- src/core.ts | 17 ++-- tests/test-omp-compat.mjs | 155 ++++++++++++++++++++++++++++++++--- tests/test-pi-local.mjs | 131 +++++++++++++++++++++++++---- tests/test-pure-functions.ts | 37 +++++++++ tests/test-stream.ts | 15 ++++ 11 files changed, 399 insertions(+), 56 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68b1346..f5dabb4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,11 +17,21 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm - run: npm ci - run: npm run typecheck + # `npm test` includes tests/test-pi-local.mjs, which drives a real pi + # binary against the mock API and otherwise skips silently. + - name: Install pi + run: | + npm install -g @earendil-works/pi-coding-agent@latest + echo "PI_BIN=$(npm prefix -g)/bin/pi" >> "$GITHUB_ENV" + - name: Verify pi starts + run: '"$PI_BIN" --version' - run: npm test + env: + PI_LOCAL_REQUIRED: "1" format: runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 99b58b0..1a751ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Fix Oh My Pi chat returning `401 Invalid 'Authorization' header` after `/login`: OMP kept the unresolved `$COMMAND_CODE_API_KEY` placeholder as a literal config API key that shadowed its stored credentials and was sent as the Bearer token. The placeholder is now registered only on pi, where it keeps the API-key login method and `--api-key` working next to OAuth; on OMP the provider omits `apiKey` unless a real key is configured. Placeholders passed by the host are also resolved or stripped on the Provider API and compat stream paths, matching the generate transport and `/commandcode-quota`. +- Cover stored `/login` OAuth and API-key credentials, `--api-key`, and env keys end to end on both pi and Oh My Pi, asserting the exact Bearer token the mock API receives. + ## 0.6.2 - 2026-09-02 - Fix `omp plugin install` on Oh My Pi 18.x, which rejected 0.6.1 because its pi-ai lacks the `registerApiProvider` export; the compat registration now resolves at runtime and is skipped on hosts that register custom APIs themselves. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 32a9171..53ee9e4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,6 +52,10 @@ COMMANDCODE_E2E_PROVIDER_API_KEY_FILE=/path/to/provider-key npm run test:e2e:liv Use `npm run test:e2e:live:all` with the Go and GOAT file variables to run both subscription transports sequentially. Store keys in a secret manager and export each one to a new mode-`0600` temporary file for the test; never add key files to the repository. Direct `*_API_KEY` variables are intended primarily for protected CI secrets. +### pi end-to-end + +`tests/test-pi-local.mjs` runs the extension inside a real `pi` binary against a mock Command Code API, including every credential source (`/login` OAuth and API-key credentials, `--api-key`, env keys). It skips locally when `pi` is not on `PATH`; CI installs pi and runs it as part of `npm test` with `PI_LOCAL_REQUIRED=1`. Point `PI_BIN` at another pi executable to test against a specific version. + ### Oh My Pi compatibility `tests/test-omp-compat.mjs` runs the extension inside a real `omp` binary against a mock Command Code API. It skips locally when `omp` is not on `PATH`; CI installs Oh My Pi and runs it as a required check with `OMP_COMPAT_REQUIRED=1`, so a change that only loads on pi fails CI instead of the next `omp plugin install`. diff --git a/README.md b/README.md index c064dad..6e2abab 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,8 @@ Run `/login` in pi or OMP. Select **Use a subscription**, then **Command Code**. If automatic transfer from the browser fails, copy the API key shown by Command Code and paste it into the terminal prompt. +On Oh My Pi, `/login` stores those credentials in OMP's credential store and chat uses them directly. If chat still returns `401 Invalid 'Authorization' header`, restart OMP after `/login` and confirm `/commandcode-quota` shows your account. + ### Environment variable ```sh diff --git a/index.ts b/index.ts index 89a9816..3253d82 100644 --- a/index.ts +++ b/index.ts @@ -17,6 +17,7 @@ import { import { join } from "node:path" import { getConfiguredApiKey } from "./src/api-key.ts" +import { pickCommandCodeApiKey, withResolvedCommandCodeApiKey } from "./src/converters.ts" import { createStreamCommandCode } from "./src/core.ts" import { calculateCommandCodeCost } from "./src/cost.ts" import { @@ -54,10 +55,40 @@ type CompatStreamFunction = ( * and registers custom APIs itself inside `registerProvider`. Resolve the * function at runtime so the extension loads on both hosts. */ -function registerCompatApiProvider(stream: CompatStreamFunction): void { +function compatApiProviderRegistrar(): ((...args: unknown[]) => unknown) | undefined { const register = (piAiCompat as { registerApiProvider?: unknown }).registerApiProvider - if (typeof register !== "function") return - register({ api: COMMAND_CODE_API, stream, streamSimple: stream }, COMPAT_SOURCE_ID) + return typeof register === "function" ? (register as (...args: unknown[]) => unknown) : undefined +} + +function registerCompatApiProvider(stream: CompatStreamFunction): void { + compatApiProviderRegistrar()?.( + { api: COMMAND_CODE_API, stream, streamSimple: stream }, + COMPAT_SOURCE_ID, + ) +} + +/** + * The `apiKey` handed to `registerProvider` means different things per host. + * + * pi parses `$COMMAND_CODE_API_KEY` as an env template: unresolved means + * "not configured", so `/login` credentials and `--api-key` take over, and + * the entry keeps the API-key auth method registered next to OAuth. Without + * it pi composes an OAuth-only provider and drops stored `api_key` + * credentials and `--api-key`. + * + * Oh My Pi has no template notion: an unresolved value stays a literal config + * override that shadows its `/login` credential store and is sent verbatim as + * `Authorization: Bearer $COMMAND_CODE_API_KEY`. There, omit `apiKey` unless + * a real key is configured; OMP then reads env keys and stored credentials + * itself. + * + * Hosts are told apart by the same `registerApiProvider` probe used for the + * compat registry: pi exports it, OMP does not. + */ +function providerApiKey(): string | undefined { + const configured = pickCommandCodeApiKey(getConfiguredApiKey(), undefined) + if (configured) return configured + return compatApiProviderRegistrar() ? "$COMMAND_CODE_API_KEY" : undefined } function commandCodeHeaders(): Record | undefined { @@ -76,7 +107,7 @@ function createProviderConfig( return { name: "Command Code", baseUrl: apiBase, - apiKey: getConfiguredApiKey() ?? "$COMMAND_CODE_API_KEY", + apiKey: providerApiKey(), api: COMMAND_CODE_API, streamSimple: streamCommandCode, headers, @@ -132,15 +163,18 @@ export default async function (pi: ExtensionAPI) { calculateCost: calculateCommandCodeCost, apiBase: legacyApiBase(apiBase), }) + const resolveStreamOptions = (options?: Parameters[2]) => + withResolvedCommandCodeApiKey(options, getConfiguredApiKey()) const transport = createCommandCodeTransportRouter({ createStream: () => new AssistantMessageEventStream(), streamProvider: (model, context, options) => streamNativeProvider( { ...model, api: apiForModelId(model.id), compat: model.compatConfig ?? model.compat }, context, - options, + resolveStreamOptions(options), ), - streamGenerate, + streamGenerate: (model, context, options) => + streamGenerate(model, context, resolveStreamOptions(options)), }) // pi dispatches the main chat through the registered provider, but sibling @@ -149,13 +183,9 @@ export default async function (pi: ExtensionAPI) { // api-registry, which knows nothing about extension providers. Register the // custom api there so those calls reach the same transport. The registry // resolves no credentials for extension providers, so fall back to the - // configured key when the caller passes none. + // configured key when the caller passes none or a placeholder. const compatStream: CompatStreamFunction = (model, context, options) => - transport.stream( - model, - context, - options?.apiKey ? options : { ...options, apiKey: getConfiguredApiKey() }, - ) as AssistantMessageEventStream + transport.stream(model, context, resolveStreamOptions(options)) as AssistantMessageEventStream registerCompatApiProvider(compatStream) pi.on("message_end", async (event, ctx) => { diff --git a/src/converters.ts b/src/converters.ts index 0df5434..d5e7d91 100644 --- a/src/converters.ts +++ b/src/converters.ts @@ -147,6 +147,13 @@ export const COMMAND_CODE_PLACEHOLDER_KEYS = new Set([ "COMMANDCODE_API_KEY", ]) +function usableCommandCodeApiKey(value: string | undefined): string | undefined { + const trimmed = typeof value === "string" ? value.trim() : undefined + if (!trimmed) return undefined + if (COMMAND_CODE_PLACEHOLDER_KEYS.has(trimmed)) return undefined + return trimmed +} + /** * Pick the real API key from a host registry value and/or the env/auth-file * fallback, never returning a literal placeholder or an empty/whitespace value. @@ -156,10 +163,20 @@ export function pickCommandCodeApiKey( registryKey: string | undefined, hostKey: string | undefined, ): string | undefined { - const trimmed = typeof registryKey === "string" ? registryKey.trim() : undefined - if (!trimmed) return hostKey - if (COMMAND_CODE_PLACEHOLDER_KEYS.has(trimmed)) return hostKey - return trimmed + return usableCommandCodeApiKey(registryKey) ?? usableCommandCodeApiKey(hostKey) +} + +/** + * Replace a host-supplied placeholder (or missing key) with the configured + * fallback. Used for both registerProvider and the Provider API stream path. + */ +export function withResolvedCommandCodeApiKey( + options: T | undefined, + configuredKey: string | undefined, +): T | { apiKey?: string } { + const apiKey = pickCommandCodeApiKey(options?.apiKey, configuredKey) + if (options && apiKey === options.apiKey) return options + return { ...options, apiKey } } export function textContent(message: { content?: unknown }): string { diff --git a/src/core.ts b/src/core.ts index 44d3fa7..0d16e70 100644 --- a/src/core.ts +++ b/src/core.ts @@ -19,6 +19,7 @@ import { messagesToCC, numberValue, parseStreamEventLine, + pickCommandCodeApiKey, recordOrEmpty, stringValue, toolsToJson, @@ -237,22 +238,14 @@ export function createStreamCommandCode(deps: CoreDependencies) { async function run() { // Some hosts pass a literal env-var reference instead of resolving it. - const PLACEHOLDER_API_KEYS = new Set([ - "$COMMAND_CODE_API_KEY", - "COMMAND_CODE_API_KEY", - "$COMMANDCODE_API_KEY", - "COMMANDCODE_API_KEY", - ]) - const hostKey = - options?.apiKey && !PLACEHOLDER_API_KEYS.has(options.apiKey) ? options.apiKey : undefined - - const apiKey = - hostKey ?? + const apiKey = pickCommandCodeApiKey( + options?.apiKey, getApiKey({ env: deps.env, authPaths: deps.authPaths, homeDir: deps.homeDir, - }) + }), + ) if (!apiKey) { const msg: AssistantMessageLike = { diff --git a/tests/test-omp-compat.mjs b/tests/test-omp-compat.mjs index 4dc4cce..3ab296e 100644 --- a/tests/test-omp-compat.mjs +++ b/tests/test-omp-compat.mjs @@ -9,7 +9,7 @@ */ import assert from "node:assert/strict" -import { spawn } from "node:child_process" +import { spawn, spawnSync } from "node:child_process" import { accessSync, constants, mkdtempSync, rmSync } from "node:fs" import { createServer } from "node:http" import { tmpdir } from "node:os" @@ -176,19 +176,93 @@ const address = server.address() const port = typeof address === "object" && address ? address.port : 0 const apiBase = `http://127.0.0.1:${port}` -function runOmp(args, timeoutMs = 30_000) { +const agentDir = join(tempHome, ".omp", "agent") + +function ompEnv(overrides = {}) { + const env = { + ...process.env, + HOME: tempHome, + USERPROFILE: tempHome, + PI_CODING_AGENT_DIR: agentDir, + COMMAND_CODE_API_KEY: "mock-key", + COMMANDCODE_API_BASE: `${apiBase}/provider/v1`, + COMMANDCODE_MODELS_URL: `${apiBase}/provider/v1/models`, + } + for (const [key, value] of Object.entries(overrides)) { + if (value === undefined) delete env[key] + else env[key] = value + } + return env +} + +// Same DDL OMP 18 runs for its credential store; OMP's own migration is +// `CREATE TABLE IF NOT EXISTS`, so creating it first is safe. +const OMP_AUTH_CREDENTIALS_DDL = `CREATE TABLE IF NOT EXISTS auth_credentials ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + provider TEXT NOT NULL, + credential_type TEXT NOT NULL, + data TEXT NOT NULL, + disabled_cause TEXT DEFAULT NULL, + identity_key TEXT DEFAULT NULL, + created_at INTEGER NOT NULL DEFAULT (CAST(strftime('%s','now') AS INTEGER)), + updated_at INTEGER NOT NULL DEFAULT (CAST(strftime('%s','now') AS INTEGER)) +)` + +/** + * Store a credential the way OMP's `/login` does: in the `auth_credentials` + * table of `agent.db`. OMP is a Bun binary, so `bun:sqlite` is always + * available next to it and matches the SQLite build OMP itself uses. + * Pass `undefined` to remove any stored Command Code credential. + */ +function seedOmpCredential(credential) { + const rows = + credential === undefined + ? [] + : [ + [ + credential.type, + JSON.stringify( + credential.type === "oauth" + ? { + access: credential.access, + refresh: credential.refresh, + expires: credential.expires, + } + : { key: credential.key, source: "login" }, + ), + ], + ] + const script = ` + import { Database } from "bun:sqlite" + // \`bun -e\` has no script slot: argv is [bun, ...args]. + const [dbPath, ddl, rowsJson] = process.argv.slice(1) + const db = new Database(dbPath) + db.run(ddl) + db.run("DELETE FROM auth_credentials WHERE provider = ?", ["commandcode"]) + for (const [type, data] of JSON.parse(rowsJson)) { + db.run( + "INSERT INTO auth_credentials (provider, credential_type, data) VALUES (?, ?, ?)", + ["commandcode", type, data], + ) + } + db.close() + ` + const result = spawnSync( + "bun", + ["-e", script, join(agentDir, "agent.db"), OMP_AUTH_CREDENTIALS_DDL, JSON.stringify(rows)], + { env: ompEnv(), encoding: "utf-8" }, + ) + assert.equal(result.status, 0, result.stderr) +} + +function runOmp(args, timeoutOrOptions = 30_000) { + const options = + typeof timeoutOrOptions === "number" ? { timeoutMs: timeoutOrOptions } : timeoutOrOptions + const timeoutMs = options.timeoutMs ?? 30_000 return new Promise((resolve) => { const child = spawn(OMP_BIN, args, { cwd: PROJECT_DIR, - env: { - ...process.env, - HOME: tempHome, - USERPROFILE: tempHome, - PI_CODING_AGENT_DIR: join(tempHome, ".omp", "agent"), - COMMAND_CODE_API_KEY: "mock-key", - COMMANDCODE_API_BASE: `${apiBase}/provider/v1`, - COMMANDCODE_MODELS_URL: `${apiBase}/provider/v1/models`, - }, + env: ompEnv(options.env), stdio: ["ignore", "pipe", "pipe"], }) let stdout = "" @@ -269,6 +343,65 @@ try { assert.equal(lastRequestBody?.model, TEST_MODEL) assert.ok(Array.isArray(lastRequestBody?.messages)) + // OMP stores `/login` credentials in agent.db and consults them only when + // the extension does not install a config API key. main registered the + // unresolved `$COMMAND_CODE_API_KEY` placeholder, which OMP kept as a + // literal config override and sent as the Bearer token, so stored + // credentials never reached the request (401). + const chatArgs = ["-e", EXT_PATH, "-p", "say mock token", "--model", `commandcode/${TEST_MODEL}`] + const noEnvKey = { COMMAND_CODE_API_KEY: undefined, COMMANDCODE_API_KEY: undefined } + + console.log("[omp-compat] stored /login OAuth credential is used when no env key exists") + seedOmpCredential({ + type: "oauth", + access: "stored-oauth-token", + refresh: "stored-oauth-token", + expires: Date.now() + 24 * 60 * 60 * 1000, + }) + requestCount = 0 + lastRequestHeaders = {} + const oauthChat = await runOmp(chatArgs, { env: noEnvKey }) + assert.equal(oauthChat.code, 0, oauthChat.stderr) + assert.match(oauthChat.stdout, /mock-omp-ok/) + assert.equal(requestCount, 1) + assert.equal(lastRequestHeaders.authorization, "Bearer stored-oauth-token") + + console.log("[omp-compat] stored /login API key credential is used when no env key exists") + seedOmpCredential({ type: "api_key", key: "stored-api-key" }) + requestCount = 0 + lastRequestHeaders = {} + const apiKeyChat = await runOmp(chatArgs, { env: noEnvKey }) + assert.equal(apiKeyChat.code, 0, apiKeyChat.stderr) + assert.match(apiKeyChat.stdout, /mock-omp-ok/) + assert.equal(requestCount, 1) + assert.equal(lastRequestHeaders.authorization, "Bearer stored-api-key") + + console.log("[omp-compat] --api-key wins over a stored credential") + requestCount = 0 + lastRequestHeaders = {} + const cliKeyChat = await runOmp([...chatArgs, "--api-key", "cli-key"], { env: noEnvKey }) + assert.equal(cliKeyChat.code, 0, cliKeyChat.stderr) + assert.match(cliKeyChat.stdout, /mock-omp-ok/) + assert.equal(requestCount, 1) + assert.equal(lastRequestHeaders.authorization, "Bearer cli-key") + + console.log("[omp-compat] COMMAND_CODE_API_KEY still works alongside a stored credential") + requestCount = 0 + lastRequestHeaders = {} + const envKeyChat = await runOmp(chatArgs) + assert.equal(envKeyChat.code, 0, envKeyChat.stderr) + assert.match(envKeyChat.stdout, /mock-omp-ok/) + assert.equal(requestCount, 1) + assert.match(lastRequestHeaders.authorization ?? "", /^Bearer (mock-key|stored-api-key)$/) + + console.log("[omp-compat] no credential at all never sends the placeholder") + seedOmpCredential(undefined) + requestCount = 0 + lastRequestHeaders = {} + const noKeyChat = await runOmp(chatArgs, { env: noEnvKey }) + assert.equal(requestCount, 0, JSON.stringify(lastRequestHeaders)) + assert.doesNotMatch(noKeyChat.stdout + noKeyChat.stderr, /\$COMMAND_CODE_API_KEY/) + console.log("[omp-compat] developer advisory reaches the legacy generate request body") requestCount = 0 requestBodies = [] diff --git a/tests/test-pi-local.mjs b/tests/test-pi-local.mjs index 416742b..aee3495 100644 --- a/tests/test-pi-local.mjs +++ b/tests/test-pi-local.mjs @@ -44,12 +44,20 @@ function findPiBinary() { const PI_BIN = findPiBinary() if (!PI_BIN) { + if (process.env.PI_LOCAL_REQUIRED === "1") { + console.error("[pi-local] FAIL - pi is required but not on PATH and PI_BIN is unset") + process.exit(1) + } console.log("[pi-local] SKIP — pi is not on PATH") process.exit(0) } const piCheck = spawnSync(PI_BIN, ["--help"], { stdio: "ignore" }) if (piCheck.error) { + if (process.env.PI_LOCAL_REQUIRED === "1") { + console.error(`[pi-local] FAIL - pi failed to start: ${piCheck.error.message}`) + process.exit(1) + } console.log(`[pi-local] SKIP — pi failed to start: ${piCheck.error.message}`) process.exit(0) } @@ -226,11 +234,19 @@ const env = { COMMANDCODE_MODELS_URL: `${apiBase}/provider/v1/models`, } -function runPi(args, timeoutMs = 30_000) { +function runPi(args, timeoutOrOptions = 30_000) { + const options = + typeof timeoutOrOptions === "number" ? { timeoutMs: timeoutOrOptions } : timeoutOrOptions + const timeoutMs = options.timeoutMs ?? 30_000 + const childEnv = { ...env } + for (const [key, value] of Object.entries(options.env ?? {})) { + if (value === undefined) delete childEnv[key] + else childEnv[key] = value + } return new Promise((resolve) => { const child = spawn(PI_BIN, args, { cwd: PROJECT_DIR, - env, + env: childEnv, stdio: ["ignore", "pipe", "pipe"], }) let stdout = "" @@ -421,9 +437,9 @@ async function runRpcExtensionCommands(timeoutMs = 30_000) { stderr += chunk.toString("utf-8") }) - const waitFor = (predicate) => + const waitFor = (predicate, fromIndex = 0) => new Promise((resolve, reject) => { - const existing = events.find(predicate) + const existing = events.slice(fromIndex).find(predicate) if (existing) { resolve(existing) return @@ -445,17 +461,27 @@ async function runRpcExtensionCommands(timeoutMs = 30_000) { ) const commandNames = commandsResponse.data?.commands?.map((command) => command.name) ?? [] - send({ id: "status-before", type: "prompt", message: "/commandcode-status" }) - await waitFor( - (event) => event.type === "response" && event.id === "status-before" && event.success, - ) - const statusBefore = await waitFor( - (event) => - event.type === "extension_ui_request" && - event.method === "notify" && - typeof event.message === "string" && - event.message.includes("model count: 3"), - ) + // The cached catalog registers immediately and refreshes in the + // background. `/commandcode-refresh` coalesces with an in-flight refresh, + // so the status must report the startup refresh as finished before the + // catalog is changed; otherwise the command reports the old catalog. + let statusBefore + for (let attempt = 0; attempt < 20; attempt += 1) { + const id = `status-before-${attempt}` + const fromIndex = events.length + send({ id, type: "prompt", message: "/commandcode-status" }) + await waitFor((event) => event.type === "response" && event.id === id && event.success) + statusBefore = await waitFor( + (event) => + event.type === "extension_ui_request" && + event.method === "notify" && + typeof event.message === "string" && + event.message.includes("model count: 3"), + fromIndex, + ) + if (/source: live[\s\S]*refresh: idle/.test(statusBefore.message)) break + await new Promise((resolve) => setTimeout(resolve, 100)) + } includeRefreshedModel = true send({ id: "refresh", type: "prompt", message: "/commandcode-refresh" }) @@ -713,7 +739,11 @@ try { const offlineListOutput = offlineList.stdout || offlineList.stderr assert.match(offlineListOutput, /gpt-5\.4/) assert.match(offlineListOutput, /cc-second-model/) - assert.match(offlineList.stderr, /Using the cached catalog/) + // The cached catalog is registered before the background refresh fails, and + // `--list-models` exits as soon as the list is printed. Whether the refresh + // warning reaches stderr first depends on the host runtime (Bun flushes it, + // Node does not), so the warning is asserted on the print run below, which + // waits for the response. console.log("[pi-local] use a cached model while model discovery is offline") requestCount = 0 @@ -824,6 +854,75 @@ try { "string", ) + // pi resolves `/login` credentials, `--api-key`, and env keys through the + // provider's registered auth methods. Stored credentials and `--api-key` + // only reach the request when the provider keeps an API-key auth method + // next to OAuth, so every credential source is checked without an env key. + const authArgs = [ + "--no-extensions", + "-e", + EXT_PATH, + "-p", + "say mock token", + "--provider", + "commandcode", + "--model", + TEST_MODEL, + ] + const noEnvKey = { COMMAND_CODE_API_KEY: undefined, COMMANDCODE_API_KEY: undefined } + const authPath = join(agentDir, "auth.json") + + console.log("[pi-local] stored /login OAuth credential is used when no env key exists") + writeFileSync( + authPath, + JSON.stringify({ + commandcode: { + type: "oauth", + access: "stored-oauth-token", + refresh: "stored-oauth-token", + expires: Date.now() + 24 * 60 * 60 * 1000, + }, + }), + ) + requestCount = 0 + lastRequestHeaders = {} + const oauthPrint = await runPi(authArgs, { env: noEnvKey }) + assert.equal(oauthPrint.code, 0, oauthPrint.stderr) + assert.match(oauthPrint.stdout, /mock-pi-ok/) + assert.equal(requestCount, 1) + assert.equal(lastRequestHeaders.authorization, "Bearer stored-oauth-token") + + console.log("[pi-local] stored /login API key credential is used when no env key exists") + writeFileSync( + authPath, + JSON.stringify({ commandcode: { type: "api_key", key: "stored-api-key" } }), + ) + requestCount = 0 + lastRequestHeaders = {} + const apiKeyPrint = await runPi(authArgs, { env: noEnvKey }) + assert.equal(apiKeyPrint.code, 0, apiKeyPrint.stderr) + assert.match(apiKeyPrint.stdout, /mock-pi-ok/) + assert.equal(requestCount, 1) + assert.equal(lastRequestHeaders.authorization, "Bearer stored-api-key") + + console.log("[pi-local] --api-key is used when no env key or stored credential exists") + rmSync(authPath, { force: true }) + requestCount = 0 + lastRequestHeaders = {} + const cliKeyPrint = await runPi([...authArgs, "--api-key", "cli-key"], { env: noEnvKey }) + assert.equal(cliKeyPrint.code, 0, cliKeyPrint.stderr) + assert.match(cliKeyPrint.stdout, /mock-pi-ok/) + assert.equal(requestCount, 1) + assert.equal(lastRequestHeaders.authorization, "Bearer cli-key") + + console.log("[pi-local] no credential at all never sends the placeholder") + requestCount = 0 + lastRequestHeaders = {} + const noKeyPrint = await runPi(authArgs, { env: noEnvKey }) + assert.notEqual(noKeyPrint.code, 0) + assert.equal(requestCount, 0, JSON.stringify(lastRequestHeaders)) + assert.doesNotMatch(noKeyPrint.stdout + noKeyPrint.stderr, /\$COMMAND_CODE_API_KEY/) + console.log("[pi-local] Claude request through Anthropic Messages endpoint") requestCount = 0 const claudePrint = await runPi( diff --git a/tests/test-pure-functions.ts b/tests/test-pure-functions.ts index bc320d4..6a1a650 100644 --- a/tests/test-pure-functions.ts +++ b/tests/test-pure-functions.ts @@ -17,6 +17,7 @@ import { messagesToCC, parseStreamEventLine, pickCommandCodeApiKey, + withResolvedCommandCodeApiKey, projectSlugFromPath, textContent, toJsonSchema, @@ -148,6 +149,42 @@ describe("pickCommandCodeApiKey()", () => { it("trims a real registry key", () => { assert.equal(pickCommandCodeApiKey(" real-registry-key ", "file-key"), "real-registry-key") }) + + it("never returns a placeholder as the host fallback", () => { + assert.equal(pickCommandCodeApiKey(undefined, "$COMMAND_CODE_API_KEY"), undefined) + assert.equal(pickCommandCodeApiKey("$COMMAND_CODE_API_KEY", "$COMMANDCODE_API_KEY"), undefined) + assert.equal(pickCommandCodeApiKey("COMMAND_CODE_API_KEY", "COMMANDCODE_API_KEY"), undefined) + }) + + it("omits a placeholder from registerProvider when no real key is configured", () => { + assert.equal(pickCommandCodeApiKey(undefined, undefined), undefined) + assert.equal(pickCommandCodeApiKey("$COMMAND_CODE_API_KEY", undefined), undefined) + assert.equal(pickCommandCodeApiKey("user_real-key", undefined), "user_real-key") + }) +}) + +describe("withResolvedCommandCodeApiKey()", () => { + it("replaces a host placeholder with the configured key", () => { + assert.deepEqual( + withResolvedCommandCodeApiKey({ apiKey: "$COMMAND_CODE_API_KEY", extra: true }, "file-key"), + { apiKey: "file-key", extra: true }, + ) + }) + + it("drops a placeholder when no configured key exists", () => { + assert.deepEqual( + withResolvedCommandCodeApiKey({ apiKey: "$COMMAND_CODE_API_KEY" }, undefined), + { + apiKey: undefined, + }, + ) + }) + + it("keeps a real host key and injects a configured key when the host omitted one", () => { + const options = { apiKey: "host-key" } + assert.equal(withResolvedCommandCodeApiKey(options, "file-key"), options) + assert.deepEqual(withResolvedCommandCodeApiKey(undefined, "file-key"), { apiKey: "file-key" }) + }) }) describe("projectSlugFromPath()", () => { diff --git a/tests/test-stream.ts b/tests/test-stream.ts index 040acb2..61f4b13 100644 --- a/tests/test-stream.ts +++ b/tests/test-stream.ts @@ -108,6 +108,21 @@ describe("streamCommandCode — auth", () => { assert.equal(server.lastRequestHeaders().authorization, "Bearer option-key") }) + + it("treats a blank options.apiKey like a missing one", async () => { + server.mockResponse({ + type: "success", + events: [JSON.stringify({ type: "finish", finishReason: "stop" })], + }) + const { streamCommandCode } = createTestDeps({ + apiBase: server.baseUrl(), + env: { COMMAND_CODE_API_KEY: "env-key" }, + }) + + await collectEvents(streamCommandCode(makeModel(), makeContext(), { apiKey: " " })) + + assert.equal(server.lastRequestHeaders().authorization, "Bearer env-key") + }) }) describe("streamCommandCode — successful streams", () => {