diff --git a/bin/cli/commands/auth-export.mjs b/bin/cli/commands/auth-export.mjs index c968f0dab3..16ed31d872 100644 --- a/bin/cli/commands/auth-export.mjs +++ b/bin/cli/commands/auth-export.mjs @@ -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 ", t("authExport.idOpt")) .option("--format ", t("authExport.formatOpt"), "json") diff --git a/bin/cli/provider-test.mjs b/bin/cli/provider-test.mjs index 4ab68bcde0..13cf9644bc 100644 --- a/bin/cli/provider-test.mjs +++ b/bin/cli/provider-test.mjs @@ -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"), { diff --git a/open-sse/config/providers/registry/openrouter/index.ts b/open-sse/config/providers/registry/openrouter/index.ts index 1116badd4d..b8302c9580 100644 --- a/open-sse/config/providers/registry/openrouter/index.ts +++ b/open-sse/config/providers/registry/openrouter/index.ts @@ -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", diff --git a/tests/unit/cli-auth-export-wiring.test.ts b/tests/unit/cli-auth-export-wiring.test.ts new file mode 100644 index 0000000000..798b13c753 --- /dev/null +++ b/tests/unit/cli-auth-export-wiring.test.ts @@ -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; + } + ); +}); diff --git a/tests/unit/openrouter-key-validation-auth-endpoint.test.ts b/tests/unit/openrouter-key-validation-auth-endpoint.test.ts new file mode 100644 index 0000000000..f636bd6dee --- /dev/null +++ b/tests/unit/openrouter-key-validation-auth-endpoint.test.ts @@ -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(); + } + }); +});