diff --git a/bin/cli/commands/setup-opencode.mjs b/bin/cli/commands/setup-opencode.mjs index f6039fb1a9..e7b8986c4b 100644 --- a/bin/cli/commands/setup-opencode.mjs +++ b/bin/cli/commands/setup-opencode.mjs @@ -35,16 +35,24 @@ export function resolveOpencodeTarget(opts = {}) { baseUrl = `http://localhost:${Number(opts.port ?? process.env.PORT ?? 20128) || 20128}`; } + // Precedence: explicit --api-key flag > OMNIROUTE_API_KEY env var > active + // context's management token. A context's accessToken/apiKey is a CLI + // management credential (oma_live_...) with no /v1/* inference scope — it + // must never silently outrank a real inference key the caller supplied + // either as a flag or via the ambient env var (mirrors the explicit > + // ambient-env > context precedence documented in bin/cli/api.mjs's + // buildHeaders()). Only fall back to the context token when neither an + // explicit flag nor the env var is set. let apiKey = opts.apiKey ?? opts["api-key"]; + if (!apiKey) apiKey = process.env.OMNIROUTE_API_KEY || ""; if (!apiKey) { try { const c = resolveActiveContext(opts.context ?? process.env.OMNIROUTE_CONTEXT); - apiKey = c?.accessToken || c?.apiKey; + apiKey = c?.accessToken || c?.apiKey || ""; } catch { /* no context auth */ } } - if (!apiKey) apiKey = process.env.OMNIROUTE_API_KEY || ""; return { baseUrl: baseUrl.replace(/\/+$/, ""), apiKey }; } @@ -177,8 +185,17 @@ export function registerSetupOpencode(program) { "--allow-container-write", "Write even when the target is inside a container and not mounted from the host" ) - .action(async (opts) => { - const code = await runSetupOpencodeCommand(opts); + .action(async (opts, cmd) => { + // Commander parses the ancestor program's own global --api-key option + // (bin/cli/program.mjs, bound to .env("OMNIROUTE_API_KEY")) against any + // occurrence of the flag in argv, so it wins the value even when the + // user typed --api-key AFTER `setup-opencode` — this local option's own + // `opts.apiKey` never sees it. cmd.optsWithGlobals() resolves to the + // correct value either way ("globals overwrite locals" is exactly the + // outcome we want here, since the global option is where the value + // always actually lands). + const resolvedOpts = { ...opts, apiKey: cmd.optsWithGlobals().apiKey ?? opts.apiKey }; + const code = await runSetupOpencodeCommand(resolvedOpts); if (code !== 0) process.exit(code); }); } diff --git a/changelog.d/fixes/12783-setup-opencode-api-key-precedence.md b/changelog.d/fixes/12783-setup-opencode-api-key-precedence.md new file mode 100644 index 0000000000..0cd25a8908 --- /dev/null +++ b/changelog.d/fixes/12783-setup-opencode-api-key-precedence.md @@ -0,0 +1 @@ +- fix(cli): setup-opencode no longer sends an active context's management token to `/v1/models` when `--api-key`/`OMNIROUTE_API_KEY` is supplied — an explicit flag or the env var now always outranks the context's token, and the flag itself is no longer swallowed by the parent program's global `--api-key` option (#12783) diff --git a/tests/unit/repro-12783-setup-opencode-apikey.test.ts b/tests/unit/repro-12783-setup-opencode-apikey.test.ts new file mode 100644 index 0000000000..fac035cb79 --- /dev/null +++ b/tests/unit/repro-12783-setup-opencode-apikey.test.ts @@ -0,0 +1,121 @@ +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; + +import { resolveOpencodeTarget } from "../../bin/cli/commands/setup-opencode.mjs"; + +/** Point OMNIROUTE_CONTEXT config resolution at an isolated, throwaway DATA_DIR. */ +function withIsolatedContext(contextConfig, fn) { + const dir = mkdtempSync(join(tmpdir(), "omniroute-setup-opencode-test-")); + const originalDataDir = process.env.DATA_DIR; + process.env.DATA_DIR = dir; + writeFileSync( + join(dir, "config.json"), + JSON.stringify({ + version: 1, + currentContext: "remote", + contexts: { remote: contextConfig }, + }) + ); + try { + return fn(); + } finally { + if (originalDataDir === undefined) delete process.env.DATA_DIR; + else process.env.DATA_DIR = originalDataDir; + rmSync(dir, { recursive: true, force: true }); + } +} + +function withEnvApiKey(value, fn) { + const original = process.env.OMNIROUTE_API_KEY; + if (value === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = value; + try { + return fn(); + } finally { + if (original === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = original; + } +} + +test("setup-opencode: --api-key typed AFTER the subcommand name is not stolen by the parent program's global option", async () => { + const { createProgram } = await import("../../bin/cli/program.mjs"); + const program = createProgram(); + const setupOpencode = program.commands.find((c) => c.name() === "setup-opencode"); + assert.ok(setupOpencode, "setup-opencode subcommand must be registered"); + + let capturedApiKey; + setupOpencode._actionHandler = null; // avoid the real network-calling action + setupOpencode.action((opts, cmd) => { + capturedApiKey = cmd.optsWithGlobals().apiKey ?? opts.apiKey; + }); + + await program.parseAsync( + [ + "node", + "omniroute", + "setup-opencode", + "--remote", + "http://100.64.0.1:20128", + "--api-key", + "sk-TESTKEY123", + ], + { from: "node" } + ); + + assert.equal( + capturedApiKey, + "sk-TESTKEY123", + "the CLI-supplied --api-key value must reach the setup-opencode action handler" + ); +}); + +test("resolveOpencodeTarget: (a) explicit --api-key flag wins over an active context's management token", () => { + withEnvApiKey(undefined, () => { + withIsolatedContext( + { baseUrl: "http://100.64.0.1:20128", accessToken: "oma_live_CONTEXT_TOKEN" }, + () => { + const { apiKey } = resolveOpencodeTarget({ apiKey: "sk-FLAG", context: "remote" }); + assert.equal(apiKey, "sk-FLAG"); + } + ); + }); +}); + +test("resolveOpencodeTarget: (b) OMNIROUTE_API_KEY env wins over an active context's management token when no flag is passed", () => { + withEnvApiKey("sk-ENVKEY", () => { + withIsolatedContext( + { baseUrl: "http://100.64.0.1:20128", accessToken: "oma_live_CONTEXT_TOKEN" }, + () => { + const { apiKey } = resolveOpencodeTarget({ context: "remote" }); + assert.equal(apiKey, "sk-ENVKEY"); + } + ); + }); +}); + +test("resolveOpencodeTarget: (c) the context's token is used only when neither a flag nor the env var is set", () => { + withEnvApiKey(undefined, () => { + withIsolatedContext( + { baseUrl: "http://100.64.0.1:20128", accessToken: "oma_live_CONTEXT_TOKEN" }, + () => { + const { apiKey } = resolveOpencodeTarget({ context: "remote" }); + assert.equal(apiKey, "oma_live_CONTEXT_TOKEN"); + } + ); + }); +}); + +test("resolveOpencodeTarget: falls back to '' when neither a flag, env var, nor a resolvable context is present", () => { + withEnvApiKey(undefined, () => { + withIsolatedContext({ baseUrl: "http://100.64.0.1:20128" }, () => { + const { apiKey } = resolveOpencodeTarget({ + remote: "http://100.64.0.1:20128", + context: "__no-such-context__", + }); + assert.equal(apiKey, ""); + }); + }); +});