fix(cli): real OpenRouter key validation + auth export argument wiring (#11226) (#11264)

Two defects exposed by #11226 (the 401 "User not found." itself is upstream
OpenRouter's response to a bad key — not an OmniRoute bug):

1. OpenRouter key validation was vacuous. Both the dashboard Check (via
   validateProviderApiKey -> validateOpenAILikeProvider) and 'omniroute
   providers test' (bin/cli/provider-test.mjs) probed /api/v1/models, which is
   PUBLIC and answers 200 to any key — so invalid keys were saved/marked valid
   and only failed on real chat traffic. OpenRouter's authenticated key-info
   endpoint (GET /api/v1/auth/key, 200 = valid / 401 = invalid) is now the
   probe: registered as testKeyModelsUrl on the openrouter registry entry
   (same mechanism as perplexity) and as keyCheckPath in the CLI test configs.
   No other provider's probe changes; error results keep using canned strings,
   never the raw upstream body (Hard Rule #12).

2. 'omniroute auth export' crashed with "cmd.optsWithGlobals is not a
   function". .command("auth export") does not register a two-word command:
   commander parses the bare word 'export' as a required positional argument,
   so the action received (exportArgValue, options, command) while expecting
   (options, command). Registered 'export' as a proper nested subcommand of
   'auth' — the documented CLI surface 'omniroute auth export [--force]
   [--id] [--format] [--out]' is unchanged, and unknown positionals
   (e.g. 'omniroute auth bogus') are now rejected instead of silently running
   the export.

TDD: tests/unit/openrouter-key-validation-auth-endpoint.test.ts (stub mimics
the real OpenRouter: /models public-200, /auth/key 401 'User not found.') and
tests/unit/cli-auth-export-wiring.test.ts (real commander wiring via
createProgram) were RED before the fix and are GREEN after.

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-08-23 14:26:08 -03:00
committed by GitHub
parent b5e4c2c0ce
commit 5cf16028fe
5 changed files with 312 additions and 7 deletions

View File

@@ -22,8 +22,15 @@ const VALID_FORMATS = new Set(["json", "env"]);
const SECURE_FILE_MODE = 0o600;
export function registerAuthExport(program) {
// #11226: `.command("auth export")` does NOT register a two-word command — commander
// parses the bare word `export` as a required positional argument of `auth`, so the
// action received (exportArgValue, options, command) while expecting (options, command)
// and crashed with "cmd.optsWithGlobals is not a function". Register `export` as a
// proper nested subcommand instead; the CLI surface stays `omniroute auth export`.
program
.command("auth export")
.command("auth")
.description(t("authExport.description"))
.command("export")
.description(t("authExport.description"))
.option("--id <id>", t("authExport.idOpt"))
.option("--format <format>", t("authExport.formatOpt"), "json")

View File

@@ -10,6 +10,10 @@ const PROVIDER_TEST_CONFIGS = {
format: "openai",
baseUrl: "https://openrouter.ai/api/v1",
model: "openai/gpt-4o-mini",
// #11226: /models is public on OpenRouter (200 with any or no key) — probe the
// authenticated key-info endpoint instead so a bad key fails the test here
// instead of on the first real chat request.
keyCheckPath: "/auth/key",
},
groq: {
format: "openai",
@@ -101,13 +105,19 @@ async function testOpenAILikeProvider(input, config) {
"Content-Type": "application/json",
};
const modelsRes = await fetchWithTimeout(joinUrl(config.baseUrl, "/models"), {
method: "GET",
headers,
});
// Providers whose /models endpoint is public (e.g. OpenRouter) declare a
// keyCheckPath pointing at an authenticated endpoint so the probe actually
// exercises the key instead of the public catalog.
const probeRes = await fetchWithTimeout(
joinUrl(config.baseUrl, config.keyCheckPath || "/models"),
{
method: "GET",
headers,
}
);
if (modelsRes.ok || modelsRes.status === 401 || modelsRes.status === 403) {
return classifyResponse(modelsRes);
if (probeRes.ok || probeRes.status === 401 || probeRes.status === 403) {
return classifyResponse(probeRes);
}
const chatRes = await fetchWithTimeout(joinUrl(config.baseUrl, "/chat/completions"), {

View File

@@ -9,6 +9,11 @@ export const openrouterProvider: RegistryEntry = {
authType: "apikey",
authHeader: "bearer",
defaultContextLength: 128000,
// #11226: OpenRouter's /api/v1/models is PUBLIC (200 with any or no key), so the
// generic /models probe validated every key — even garbage ones — and bad keys
// only surfaced later as upstream 401 "User not found." on real chat traffic.
// /api/v1/auth/key is the authenticated key-info endpoint: 200 = valid, 401 = invalid.
testKeyModelsUrl: "https://openrouter.ai/api/v1/auth/key",
headers: {
"HTTP-Referer": "https://endpoint-proxy.local",
"X-Title": "Endpoint Proxy",

View File

@@ -0,0 +1,131 @@
// #11226 — `omniroute auth export` crashed with "cmd.optsWithGlobals is not a
// function" because the command was registered as `.command("auth export")`:
// commander parses the bare word `export` as a REQUIRED POSITIONAL ARGUMENT, so
// the action received ("export", options, command) while its signature expected
// (options, command) — the classic opts/cmd swap. The fix registers `export` as
// a proper nested subcommand of `auth`, restoring the documented CLI surface
// (docs/reference/CLI-TOOLS.md): `omniroute auth export [--force] [--id] [--format] [--out]`.
//
// These tests exercise the REAL commander wiring via createProgram() — no DB is
// touched on any of these paths (the no-force gate prints and returns before any
// DB access; an invalid --format fails validation before opening the DB).
import test from "node:test";
import assert from "node:assert/strict";
import { createProgram } from "../../bin/cli/program.mjs";
function captureConsole(): { captured: { logs: string[]; errors: string[] }; restore: () => void } {
const originalLog = console.log;
const originalError = console.error;
const captured = { logs: [] as string[], errors: [] as string[] };
console.log = (msg?: unknown) => {
captured.logs.push(String(msg ?? ""));
};
console.error = (msg?: unknown) => {
captured.errors.push(String(msg ?? ""));
};
return {
captured,
restore: () => {
console.log = originalLog;
console.error = originalError;
},
};
}
function stubProcessExit(): { exitCodes: number[]; restore: () => void } {
const originalExit = process.exit;
const exitCodes: number[] = [];
process.exit = ((code?: number) => {
exitCodes.push(code ?? 0);
}) as typeof process.exit;
return {
exitCodes,
restore: () => {
process.exit = originalExit;
},
};
}
test("auth command exposes 'export' as a subcommand, not a positional argument", () => {
const program = createProgram();
const auth = program.commands.find((c) => c.name() === "auth");
assert.ok(auth, "auth command exists");
const exportCmd = auth.commands.find((c) => c.name() === "export");
assert.ok(exportCmd, "export must be a nested subcommand of auth");
const registeredArgs = (auth as unknown as { registeredArguments?: unknown[] })
.registeredArguments;
assert.equal(
registeredArgs?.length ?? 0,
0,
"auth must not declare positional arguments (a bare word in .command() becomes one)"
);
});
test("auth export action receives (options, command): flags reach the handler end-to-end", async () => {
const program = createProgram();
const exitStub = stubProcessExit();
const { captured, restore } = captureConsole();
try {
// --format bogus makes runAuthExportCommand return 1 BEFORE any DB access;
// the action must then call process.exit(1). With the opts/cmd swap this
// parse rejects with "cmd.optsWithGlobals is not a function" instead.
await program.parseAsync([
"node",
"omniroute",
"auth",
"export",
"--force",
"--format",
"bogus",
]);
} finally {
restore();
exitStub.restore();
}
assert.deepEqual(
exitStub.exitCodes,
[1],
"handler must receive --format and exit 1 on bogus value"
);
assert.ok(
captured.errors.join("\n").includes("Invalid format"),
`expected the invalid-format error, got: ${captured.errors.join(" | ")}`
);
});
test("auth export without --force prints the confirmation gate (no crash, no DB)", async () => {
const program = createProgram();
const exitStub = stubProcessExit();
const { captured, restore } = captureConsole();
try {
await program.parseAsync(["node", "omniroute", "auth", "export"]);
} finally {
restore();
exitStub.restore();
}
assert.deepEqual(exitStub.exitCodes, [], "dry run exits 0 without calling process.exit");
assert.ok(
captured.logs.join("\n").includes("DECRYPTED"),
`expected the confirmation gate, got: ${captured.logs.join(" | ")}`
);
});
test("auth rejects an unknown positional (was silently accepted as the 'export' argument)", async () => {
const program = createProgram();
await assert.rejects(
program.parseAsync(["node", "omniroute", "auth", "bogus-word"]),
(err: unknown) => {
assert.ok(err instanceof Error);
assert.match(
(err as { code?: string }).code || "",
/commander\.(unknownCommand|helpDisplayed)/
);
return true;
}
);
});

View File

@@ -0,0 +1,152 @@
// #11226 — OpenRouter key validation was vacuous: the probe targeted the PUBLIC
// /api/v1/models endpoint, which answers 200 to any key (or no key at all), so a
// bad key was saved as "valid" and only failed later on real chat traffic with the
// upstream 401 "User not found.". The authenticated key-info endpoint
// (/api/v1/auth/key) is the correct probe: 200 = valid, 401 = invalid.
//
// The fetch stubs below mimic the REAL OpenRouter behavior verified live:
// GET /api/v1/models → 200 without any auth (public catalog)
// GET /api/v1/auth/key → 401 {"error":{"message":"User not found.","code":401}} for a bad key
import { describe, it } from "node:test";
import assert from "node:assert/strict";
const { getRegistryEntry } = await import("../../open-sse/config/providerRegistry.ts");
const { validateProviderApiKey } = await import("../../src/lib/providers/validation.ts");
const { testProviderApiKey } = await import("../../bin/cli/provider-test.mjs");
const AUTH_KEY_URL = "https://openrouter.ai/api/v1/auth/key";
const PUBLIC_MODELS_URL = "https://openrouter.ai/api/v1/models";
const BAD_KEY = "sk-or-v1-definitely-invalid-key";
const GOOD_KEY = "sk-or-v1-valid-key";
interface RecordedCall {
url: string;
authorization: string | null;
}
/**
* Stub fetch with the real OpenRouter behavior: /models is public (always 200),
* /auth/key requires a valid bearer (401 "User not found." otherwise).
*/
function stubRealOpenRouter() {
const calls: RecordedCall[] = [];
const originalFetch = globalThis.fetch;
globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => {
const url = typeof input === "string" ? input : input instanceof URL ? input.href : input.url;
const headers = new Headers(
init?.headers ?? (input instanceof Request ? input.headers : undefined)
);
calls.push({ url, authorization: headers.get("authorization") });
if (url.startsWith(AUTH_KEY_URL)) {
const bearer = headers.get("authorization") || "";
if (bearer === `Bearer ${GOOD_KEY}`) {
return new Response(JSON.stringify({ data: { label: "ok", is_free_tier: false } }), {
status: 200,
});
}
return new Response(JSON.stringify({ error: { message: "User not found.", code: 401 } }), {
status: 401,
});
}
if (url.includes("/models")) {
// Public catalog — answers 200 regardless of the Authorization header.
return new Response(JSON.stringify({ data: [] }), { status: 200 });
}
return new Response("{}", { status: 404 });
}) as typeof fetch;
return {
calls,
restore: () => {
globalThis.fetch = originalFetch;
},
};
}
describe("openrouter registry — authenticated key-validation endpoint (#11226)", () => {
it("declares the authenticated /auth/key probe as its key-test endpoint", () => {
const entry = getRegistryEntry("openrouter");
assert.ok(entry, "openrouter must be registered in the execution registry");
assert.equal(entry.testKeyModelsUrl, AUTH_KEY_URL);
});
it("marks a bad key INVALID even though the public /models endpoint answers 200", async () => {
const stub = stubRealOpenRouter();
try {
const result = await validateProviderApiKey({ provider: "openrouter", apiKey: BAD_KEY });
assert.equal(result.valid, false, "bad key must not validate against the public catalog");
assert.equal(result.error, "Invalid API key");
assert.deepEqual(
stub.calls.map((c) => c.url),
[AUTH_KEY_URL],
"must probe the authenticated key endpoint, not the public /models"
);
assert.equal(stub.calls[0].authorization, `Bearer ${BAD_KEY}`);
} finally {
stub.restore();
}
});
it("marks a good key VALID via /auth/key and never falls back to the chat probe", async () => {
const stub = stubRealOpenRouter();
try {
const result = await validateProviderApiKey({ provider: "openrouter", apiKey: GOOD_KEY });
assert.equal(result.valid, true);
assert.equal(result.error, null);
assert.deepEqual(
stub.calls.map((c) => c.url),
[AUTH_KEY_URL]
);
} finally {
stub.restore();
}
});
});
describe("omniroute providers test — openrouter probe (#11226)", () => {
it("marks a bad key INVALID even though the public /models endpoint answers 200", async () => {
const stub = stubRealOpenRouter();
try {
const result = await testProviderApiKey({ provider: "openrouter", apiKey: BAD_KEY });
assert.equal(result.valid, false, "CLI test must not trust the public /models endpoint");
assert.equal(result.error, "Invalid API key");
assert.deepEqual(
stub.calls.map((c) => c.url),
[AUTH_KEY_URL]
);
} finally {
stub.restore();
}
});
it("marks a good key VALID via /auth/key", async () => {
const stub = stubRealOpenRouter();
try {
const result = await testProviderApiKey({ provider: "openrouter", apiKey: GOOD_KEY });
assert.equal(result.valid, true);
assert.equal(result.error, null);
assert.deepEqual(
stub.calls.map((c) => c.url),
[AUTH_KEY_URL]
);
} finally {
stub.restore();
}
});
it("does not change the probe for other OpenAI-like providers (openai still uses /models)", async () => {
const stub = stubRealOpenRouter();
try {
const result = await testProviderApiKey({ provider: "openai", apiKey: GOOD_KEY });
assert.equal(result.valid, true);
assert.deepEqual(
stub.calls.map((c) => c.url),
["https://api.openai.com/v1/models"]
);
assert.ok(!stub.calls.some((c) => c.url === PUBLIC_MODELS_URL));
} finally {
stub.restore();
}
});
});