From cdfd71c173f5d8d78d1f5f01c48a6a79f1cc4525 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 20 Jun 2026 06:05:33 -0300 Subject: [PATCH] fix(cli): active-context credential must win over the ambient OMNIROUTE_API_KEY (#4364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by end-to-end testing of remote mode against a live VPS: after `omniroute connect ` saved the scoped admin token as the active context, every remote *management* command (`tokens list/create/revoke`, etc.) failed with "Invalid management token". Root cause: the global `--api-key` option is bound to the env var (.env("OMNIROUTE_API_KEY")), and users keep OMNIROUTE_API_KEY (their inference key) in the shell. Commands that spread `optsWithGlobals()` into apiFetch therefore carry `opts.apiKey` === the env value, which buildHeaders treated as an explicit override that outranked the active context — so the local inference key was sent to the remote instead of the scoped token, defeating remote mode. Fix (single chokepoint in buildHeaders): an `opts.apiKey` that merely mirrors the ambient OMNIROUTE_API_KEY is treated as ambient (a fallback), not as an explicit override; only a DISTINCT key — a real `--api-key ` flag or a command-supplied token like `connect --key` — counts as explicit and wins. Precedence becomes: explicit distinct key -> active-context credential -> ambient env key. This keeps `connect --key`, local/default usage, and explicit overrides working while making `connect` actually route management commands to the remote. Regression tests added to cli-remote-mode.test.ts (RED before, GREEN after): context token wins over an opts.apiKey echoing the env; a distinct explicit key still wins; ambient env remains the no-context fallback. --- bin/cli/api.mjs | 24 ++++++++--- tests/unit/cli-remote-mode.test.ts | 65 ++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/bin/cli/api.mjs b/bin/cli/api.mjs index ca2e09fa37..52e4bbaf85 100644 --- a/bin/cli/api.mjs +++ b/bin/cli/api.mjs @@ -58,18 +58,32 @@ export async function buildHeaders(opts) { if (opts.body && !headers.has("content-type") && typeof opts.body !== "string") { headers.set("content-type", "application/json"); } - // Auth precedence: explicit opts/env → active context. Within a context the - // scoped accessToken wins over the legacy apiKey. This routes the active - // context's credential to the (possibly remote) server automatically. - let auth = opts.apiKey ?? process.env.OMNIROUTE_API_KEY; + // Auth precedence: explicit key → active-context credential → ambient env key. + // + // The active context's scoped token MUST win over the ambient OMNIROUTE_API_KEY: + // `omniroute connect ` saves the context's token, but users keep + // OMNIROUTE_API_KEY in their shell. The global `--api-key` option is bound to + // that env var (.env("OMNIROUTE_API_KEY")), so commands that spread + // `optsWithGlobals()` into apiFetch carry opts.apiKey === the env value. If that + // echoed value outranked the context, every remote management command would send + // the local inference key and fail with "Invalid management token" — defeating + // remote mode. So an opts.apiKey that merely mirrors the ambient env var is + // treated as ambient (a fallback), NOT as an explicit override; only a DISTINCT + // key — a real `--api-key ` flag or a command-supplied token like + // `connect --key` — counts as explicit and wins. Within a context the scoped + // accessToken wins over the legacy apiKey. + const ambientKey = process.env.OMNIROUTE_API_KEY || null; + const explicitKey = opts.apiKey && opts.apiKey !== ambientKey ? opts.apiKey : null; + let auth = explicitKey; if (!auth) { try { const ctx = resolveActiveContext(opts.context ?? process.env.OMNIROUTE_CONTEXT); auth = ctx?.accessToken || ctx?.apiKey || null; } catch { - // No context credential available — continue unauthenticated. + // No context credential available — fall through to the ambient fallback. } } + if (!auth) auth = opts.apiKey || ambientKey || null; if (auth && !headers.has("authorization")) { headers.set("authorization", `Bearer ${auth}`); } diff --git a/tests/unit/cli-remote-mode.test.ts b/tests/unit/cli-remote-mode.test.ts index e368fc7d69..a9c894665e 100644 --- a/tests/unit/cli-remote-mode.test.ts +++ b/tests/unit/cli-remote-mode.test.ts @@ -144,6 +144,71 @@ test("buildHeaders: explicit opts.apiKey wins over the context credential", asyn assert.equal(headers.get("authorization"), "Bearer sk-explicit"); }); +test("buildHeaders: active-context token wins over an opts.apiKey echoing the ambient env", async () => { + // Regression: users keep OMNIROUTE_API_KEY (their inference key) in the shell. + // The global --api-key option is bound to that env var, so commands that spread + // optsWithGlobals() into apiFetch carry opts.apiKey === the env value. After + // `omniroute connect ` the active context holds the scoped token; an + // opts.apiKey that merely mirrors the ambient env must NOT outrank it, or every + // remote management command sends the local inference key ("Invalid management + // token"). This reproduces the exact failing shape: opts.apiKey === env value. + writeConfig({ + version: 1, + currentContext: "vps", + contexts: { vps: { baseUrl: "https://vps.example.com", accessToken: "oma_live_scoped" } }, + }); + const prev = process.env.OMNIROUTE_API_KEY; + process.env.OMNIROUTE_API_KEY = "sk-ambient-inference-key"; + try { + const { buildHeaders } = await import("../../bin/cli/api.mjs"); + // opts.apiKey mirrors the ambient env (commander's .env binding + spread). + const headers = await buildHeaders({ cliToken: "", apiKey: "sk-ambient-inference-key" }); + assert.equal(headers.get("authorization"), "Bearer oma_live_scoped"); + } finally { + if (prev === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = prev; + } +}); + +test("buildHeaders: a DISTINCT explicit key still wins over the context even when env is set", async () => { + // A real `--api-key ` flag or a command-supplied token (e.g. connect --key) + // differs from the ambient env value, so it counts as explicit and overrides. + writeConfig({ + version: 1, + currentContext: "vps", + contexts: { vps: { baseUrl: "https://vps.example.com", accessToken: "oma_live_scoped" } }, + }); + const prev = process.env.OMNIROUTE_API_KEY; + process.env.OMNIROUTE_API_KEY = "sk-ambient-inference-key"; + try { + const { buildHeaders } = await import("../../bin/cli/api.mjs"); + const headers = await buildHeaders({ cliToken: "", apiKey: "oma_explicit_token" }); + assert.equal(headers.get("authorization"), "Bearer oma_explicit_token"); + } finally { + if (prev === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = prev; + } +}); + +test("buildHeaders: ambient OMNIROUTE_API_KEY is the fallback when no context credential", async () => { + // Local/default usage with no stored context credential still works off the env. + writeConfig({ + version: 1, + currentContext: "default", + contexts: { default: { baseUrl: "http://localhost:20128", apiKey: null } }, + }); + const prev = process.env.OMNIROUTE_API_KEY; + process.env.OMNIROUTE_API_KEY = "sk-ambient-inference-key"; + try { + const { buildHeaders } = await import("../../bin/cli/api.mjs"); + const headers = await buildHeaders({ cliToken: "", apiKey: "sk-ambient-inference-key" }); + assert.equal(headers.get("authorization"), "Bearer sk-ambient-inference-key"); + } finally { + if (prev === undefined) delete process.env.OMNIROUTE_API_KEY; + else process.env.OMNIROUTE_API_KEY = prev; + } +}); + // ── context current command ───────────────────────────────────────────────────── test("commands/contexts.mjs registers a `current` subcommand", async () => {