fix(cli): active-context credential must win over the ambient OMNIROUTE_API_KEY (#4364)

Found by end-to-end testing of remote mode against a live VPS: after
`omniroute connect <remote>` 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 <x>` 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.
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-20 06:05:33 -03:00
committed by GitHub
parent 708d77616c
commit cdfd71c173
2 changed files with 84 additions and 5 deletions

View File

@@ -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 <remote>` 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 <x>` 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}`);
}

View File

@@ -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 <remote>` 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 <x>` 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 () => {