From 1fd19c45ecf9ef11b7faee5e54ab1145ed5842fa Mon Sep 17 00:00:00 2001 From: Randi <55005611+rdself@users.noreply.github.com> Date: Sun, 28 Jun 2026 21:17:38 -0400 Subject: [PATCH] fix(authz): require auth for v1beta client API (#5274) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: close /v1beta authz bypass — classify /v1beta + /api/v1beta aliases as CLIENT_API so Bearer auth is enforced. Validated authz TDD suite (classify/proxy-contract/pipeline/inventory). Integrated into release/v3.8.40. --- docs/architecture/AUTHZ_GUIDE.md | 18 +++++------ src/app/api/settings/authz-inventory/route.ts | 7 +++- src/proxy.ts | 2 ++ src/server/authz/classify.ts | 13 ++++++++ tests/unit/api/authz-inventory.test.ts | 7 ++++ tests/unit/authz/classify.test.ts | 19 +++++++++++ tests/unit/authz/pipeline.test.ts | 32 +++++++++++++++++++ tests/unit/authz/proxy-contract.test.ts | 1 + 8 files changed, 89 insertions(+), 10 deletions(-) diff --git a/docs/architecture/AUTHZ_GUIDE.md b/docs/architecture/AUTHZ_GUIDE.md index 15fd5f5e30..3944c007e4 100644 --- a/docs/architecture/AUTHZ_GUIDE.md +++ b/docs/architecture/AUTHZ_GUIDE.md @@ -43,11 +43,11 @@ Some management routes accept **either** mode: cookie OR `Bearer ` when the `src/server/authz/types.ts` defines three classes; any route that cannot be classified deterministically falls back to `MANAGEMENT`. -| Class | Description | Auth required | -| ------------ | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------- | -| `PUBLIC` | Explicitly safe routes — login, logout, status, init, health, onboarding bootstrap. | None | -| `CLIENT_API` | Model-serving endpoints — `/api/v1/*`, plus aliases `/v1/*`, `/chat/completions`, `/responses`, `/models`, `/codex/*`. | Bearer key when the effective `REQUIRE_API_KEY` feature flag is enabled | -| `MANAGEMENT` | Dashboard pages, settings, providers, keys, admin and diagnostics endpoints. | Dashboard session OR Bearer with `manage` scope | +| Class | Description | Auth required | +| ------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------- | +| `PUBLIC` | Explicitly safe routes — login, logout, status, init, health, onboarding bootstrap. | None | +| `CLIENT_API` | Model-serving endpoints — `/api/v1/*`, `/api/v1beta/*`, plus aliases `/v1/*`, `/v1beta/*`, `/chat/completions`, `/responses`, `/models`, `/codex/*`. | Bearer key when the effective `REQUIRE_API_KEY` feature flag is enabled | +| `MANAGEMENT` | Dashboard pages, settings, providers, keys, admin and diagnostics endpoints. | Dashboard session OR Bearer with `manage` scope | ## Pipeline @@ -73,7 +73,7 @@ Trusted internal headers (defined in `src/server/authz/headers.ts`) are **stripp Each route class has a policy in `src/server/authz/policies/`: - **`publicPolicy`** (`policies/public.ts`) — always returns `allow({ kind: "anonymous", id: "anonymous" })`. -- **`clientApiPolicy`** (`policies/clientApi.ts`) — extracts Bearer, validates via `validateApiKey()`. Falls through to anonymous only when the effective `REQUIRE_API_KEY` feature flag is disabled. The effective flag is resolved through `isRequireApiKeyEnabled()` (`DB feature flag override > process.env.REQUIRE_API_KEY > default`) so Dashboard Feature Flags and environment variables govern `/api/v1/*` and aliases consistently; resolver failures fail closed. Allows dashboard-session requests on client API routes (including `/api/v1/models`, used by the dashboard model catalog). +- **`clientApiPolicy`** (`policies/clientApi.ts`) — extracts Bearer, validates via `validateApiKey()`. Falls through to anonymous only when the effective `REQUIRE_API_KEY` feature flag is disabled. The effective flag is resolved through `isRequireApiKeyEnabled()` (`DB feature flag override > process.env.REQUIRE_API_KEY > default`) so Dashboard Feature Flags and environment variables govern `/api/v1/*`, `/api/v1beta/*`, and aliases consistently; resolver failures fail closed. Allows dashboard-session requests on client API routes (including `/api/v1/models`, used by the dashboard model catalog). - **`managementPolicy`** (`policies/management.ts`) — accepts dashboard session, internal model-sync requests (matched against `/api/providers/[name]/(sync-models|models)`), or skips entirely if `isAuthRequired()` returns false. Returns 403 (`AUTH_001`) when a Bearer token is present but invalid, 401 otherwise. Also enforces the route-guard tiers (LOCAL_ONLY / ALWAYS_PROTECTED) before any auth branch — see [Route Guard Tiers](../security/ROUTE_GUARD_TIERS.md). LOCAL_ONLY paths in `LOCAL_ONLY_MANAGE_SCOPE_BYPASS_PREFIXES` (today: `/api/mcp/`) may be accessed from non-loopback when the Bearer key carries the `manage` scope; all other LOCAL_ONLY paths remain strict-loopback regardless of scope. A successful policy returns `AuthSubject` with `kind ∈ { client_api_key, dashboard_session, management_key, anonymous }`. Downstream handlers can read it via `assertAuth(request, "CLIENT_API")` in `src/server/authz/assertAuth.ts` instead of re-running auth logic. @@ -99,13 +99,13 @@ PUBLIC_READONLY_API_ROUTE_PREFIXES = ["/api/monitoring/health", "/api/settings/r PUBLIC_READONLY_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); ``` -Read-only prefixes are public **only** for safe methods. Note: `classifyRoute()` excludes `/api/v1/*` from the PUBLIC fall-through — those are always `CLIENT_API` so the Bearer-key policy still applies. +Read-only prefixes are public **only** for safe methods. Note: `classifyRoute()` excludes `/api/v1/*` and `/api/v1beta/*` from the PUBLIC fall-through — those are always `CLIENT_API` so the Bearer-key policy still applies. ## Adding a New Route ### Pattern 1 — Public client API endpoint (Bearer-auth) -Routes under `/api/v1/` are classified `CLIENT_API` automatically. The middleware enforces the Bearer check; route handlers don't need to redo it but can read the subject if useful. +Routes under `/api/v1/` and `/api/v1beta/` are classified `CLIENT_API` automatically. The middleware enforces the Bearer check; route handlers don't need to redo it but can read the subject if useful. ```typescript // src/app/api/v1/your-route/route.ts @@ -173,7 +173,7 @@ Preset bundles (`MCP_SCOPE_PRESETS`): `readonly`, `full`, `monitor`, `agent`. Us - No password configured **and** no `INITIAL_PASSWORD` env var → bootstrap mode allows the onboarding wizard and loopback requests, but exposed network requests still need credentials. - Any DB error → fails closed (secure-by-default). -Client API key enforcement uses `isRequireApiKeyEnabled()` in `src/shared/utils/featureFlags.ts`, not a direct `process.env.REQUIRE_API_KEY` read. This matters for deployed instances: toggling `REQUIRE_API_KEY` in Dashboard → Feature Flags stores a DB override and immediately affects `/v1/*`, `/models`, `/responses`, `/chat/completions`, `/codex/*`, and other client-API auth checks that share this helper. If the feature flag store cannot be read, client API auth fails closed and requires a key. +Client API key enforcement uses `isRequireApiKeyEnabled()` in `src/shared/utils/featureFlags.ts`, not a direct `process.env.REQUIRE_API_KEY` read. This matters for deployed instances: toggling `REQUIRE_API_KEY` in Dashboard → Feature Flags stores a DB override and immediately affects `/v1/*`, `/v1beta/*`, `/models`, `/responses`, `/chat/completions`, `/codex/*`, and other client-API auth checks that share this helper. If the feature flag store cannot be read, client API auth fails closed and requires a key. ## Breaking Change — v3.8.0 diff --git a/src/app/api/settings/authz-inventory/route.ts b/src/app/api/settings/authz-inventory/route.ts index a9bc671f5f..880f1af9cb 100644 --- a/src/app/api/settings/authz-inventory/route.ts +++ b/src/app/api/settings/authz-inventory/route.ts @@ -22,7 +22,12 @@ const MANAGEMENT_TIER_PREFIXES: ReadonlyArray = [ "/api/api-keys", ]; -const CLIENT_API_TIER_PREFIXES: ReadonlyArray = ["/v1/", "/api/v1/"]; +const CLIENT_API_TIER_PREFIXES: ReadonlyArray = [ + "/v1/", + "/api/v1/", + "/v1beta/", + "/api/v1beta/", +]; const PUBLIC_TIER_PREFIXES: ReadonlyArray = ["/api/health", "/api/version", "/_next/"]; diff --git a/src/proxy.ts b/src/proxy.ts index eaa312e091..93de5e149f 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -14,6 +14,8 @@ export const config = { "/api/:path*", "/v1/:path*", "/v1", + "/v1beta/:path*", + "/v1beta", "/chat/:path*", "/responses/:path*", "/responses", diff --git a/src/server/authz/classify.ts b/src/server/authz/classify.ts index fd5b9d4cd2..d0721dc5ae 100644 --- a/src/server/authz/classify.ts +++ b/src/server/authz/classify.ts @@ -25,6 +25,11 @@ function normalizePathname(rawPath: string): { path: string; reason?: Classifica return { path: "/api/v1" + tail, reason: "client_api_double_prefix" }; } + if (path === "/v1beta" || path.startsWith("/v1beta/")) { + const tail = path.slice("/v1beta".length) || ""; + return { path: "/api/v1beta" + tail, reason: "client_api_alias" }; + } + if (path === "/v1" || path.startsWith("/v1/")) { const tail = path.slice("/v1".length) || ""; return { path: "/api/v1" + tail, reason: "client_api_alias" }; @@ -87,6 +92,14 @@ export function classifyRoute(rawPath: string, method: string = "GET"): RouteCla }; } + if (normalizedPath === "/api/v1beta" || normalizedPath.startsWith("/api/v1beta/")) { + return { + routeClass: "CLIENT_API", + reason: aliasReason ?? "client_api_v1", + normalizedPath, + }; + } + if (normalizedPath.startsWith("/api/")) { if (isClassifiedAsPublic(normalizedPath, method)) { return { diff --git a/tests/unit/api/authz-inventory.test.ts b/tests/unit/api/authz-inventory.test.ts index e17d7bf797..5d37f44c11 100644 --- a/tests/unit/api/authz-inventory.test.ts +++ b/tests/unit/api/authz-inventory.test.ts @@ -77,6 +77,13 @@ test("AC-1: GET returns 5 tiers with prefixes + bypass state envelope", async () assert.ok(alwaysProtected!.prefixes.includes("/api/shutdown")); assert.equal(alwaysProtected!.bypassable, false); + const clientApi = body.tiers.find((t) => t.name === "CLIENT_API"); + assert.ok(clientApi); + assert.ok(clientApi!.prefixes.includes("/v1/")); + assert.ok(clientApi!.prefixes.includes("/api/v1/")); + assert.ok(clientApi!.prefixes.includes("/v1beta/")); + assert.ok(clientApi!.prefixes.includes("/api/v1beta/")); + // Every tier carries a non-empty description. for (const tier of body.tiers) { assert.ok(tier.description.length > 0, `tier ${tier.name} missing description`); diff --git a/tests/unit/authz/classify.test.ts b/tests/unit/authz/classify.test.ts index c32719cedd..ff3be47e21 100644 --- a/tests/unit/authz/classify.test.ts +++ b/tests/unit/authz/classify.test.ts @@ -49,6 +49,23 @@ const cases: Case[] = [ expectedClass: "CLIENT_API", expectedNormalized: "/api/v1/chat/completions", }, + { + name: "/v1beta alias", + path: "/v1beta", + expectedClass: "CLIENT_API", + expectedNormalized: "/api/v1beta", + }, + { + name: "/v1beta generateContent alias", + path: "/v1beta/models/gemini-pro:generateContent", + expectedClass: "CLIENT_API", + expectedNormalized: "/api/v1beta/models/gemini-pro:generateContent", + }, + { + name: "/api/v1beta generateContent", + path: "/api/v1beta/models/gemini-pro:generateContent", + expectedClass: "CLIENT_API", + }, { name: "/v1/v1 double-prefix", path: "/v1/v1", @@ -189,4 +206,6 @@ test("classifyRoute strips trailing slash on root only when not '/'", () => { test("classifyRoute treats /api/v1 prefix exactly", () => { assert.equal(classifyRoute("/api/v1abc").routeClass, "MANAGEMENT"); assert.equal(classifyRoute("/api/v1/x").routeClass, "CLIENT_API"); + assert.equal(classifyRoute("/api/v1betamax").routeClass, "MANAGEMENT"); + assert.equal(classifyRoute("/api/v1beta/models").routeClass, "CLIENT_API"); }); diff --git a/tests/unit/authz/pipeline.test.ts b/tests/unit/authz/pipeline.test.ts index 814ebb2fe8..1e94b90c30 100644 --- a/tests/unit/authz/pipeline.test.ts +++ b/tests/unit/authz/pipeline.test.ts @@ -18,6 +18,7 @@ const pipeline = await import("../../../src/server/authz/pipeline.ts"); const ORIGINAL_JWT = process.env.JWT_SECRET; const ORIGINAL_INITIAL = process.env.INITIAL_PASSWORD; const ORIGINAL_AUTH_COOKIE_SECURE = process.env.AUTH_COOKIE_SECURE; +const ORIGINAL_REQUIRE_API_KEY = process.env.REQUIRE_API_KEY; function resetEnvironment() { core.resetDbInstance(); @@ -26,6 +27,7 @@ function resetEnvironment() { fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); process.env.JWT_SECRET = "pipeline-jwt-secret"; process.env.INITIAL_PASSWORD = "pipeline-initial-password"; + process.env.REQUIRE_API_KEY = "true"; delete process.env.AUTH_COOKIE_SECURE; globalThis.__omnirouteShutdown = { init: false, shuttingDown: false, activeRequests: 0 }; } @@ -59,6 +61,8 @@ test.after(() => { else process.env.INITIAL_PASSWORD = ORIGINAL_INITIAL; if (ORIGINAL_AUTH_COOKIE_SECURE === undefined) delete process.env.AUTH_COOKIE_SECURE; else process.env.AUTH_COOKIE_SECURE = ORIGINAL_AUTH_COOKIE_SECURE; + if (ORIGINAL_REQUIRE_API_KEY === undefined) delete process.env.REQUIRE_API_KEY; + else process.env.REQUIRE_API_KEY = ORIGINAL_REQUIRE_API_KEY; globalThis.__omnirouteShutdown = { init: false, shuttingDown: false, activeRequests: 0 }; }); @@ -193,6 +197,34 @@ test("runAuthzPipeline rejects oversized rewritten alias API bodies before auth" assert.ok(response.headers.get("x-request-id")); }); +test("runAuthzPipeline rejects unauthenticated v1beta Gemini aliases as client API", async () => { + const response = await pipeline.runAuthzPipeline( + request("http://localhost/v1beta/models/gemini-pro:generateContent", { + method: "POST", + }), + { enforce: true } + ); + const body = await response.json(); + + assert.equal(response.status, 401); + assert.equal(response.headers.get("x-omniroute-route-class"), "CLIENT_API"); + assert.equal(body.error.code, "AUTH_002"); +}); + +test("runAuthzPipeline rejects unauthenticated internal api v1beta routes as client API", async () => { + const response = await pipeline.runAuthzPipeline( + request("http://localhost/api/v1beta/models/gemini-pro:generateContent", { + method: "POST", + }), + { enforce: true } + ); + const body = await response.json(); + + assert.equal(response.status, 401); + assert.equal(response.headers.get("x-omniroute-route-class"), "CLIENT_API"); + assert.equal(body.error.code, "AUTH_002"); +}); + test("runAuthzPipeline rejects new API requests during shutdown drain", async () => { globalThis.__omnirouteShutdown = { init: true, shuttingDown: true, activeRequests: 0 }; diff --git a/tests/unit/authz/proxy-contract.test.ts b/tests/unit/authz/proxy-contract.test.ts index 9989138fc4..93d44b5852 100644 --- a/tests/unit/authz/proxy-contract.test.ts +++ b/tests/unit/authz/proxy-contract.test.ts @@ -59,6 +59,7 @@ test("proxy.ts config.matcher covers every /api/* route plus dashboard and v1 al '"/api/:path*"', '"/dashboard/:path*"', '"/v1/:path*"', + '"/v1beta/:path*"', '"/chat/:path*"', '"/responses/:path*"', '"/codex/:path*"',