diff --git a/changelog.d/fixes/13685-endpoint-alias-policy-fail-open.md b/changelog.d/fixes/13685-endpoint-alias-policy-fail-open.md new file mode 100644 index 0000000000..642315fb06 --- /dev/null +++ b/changelog.d/fixes/13685-endpoint-alias-policy-fail-open.md @@ -0,0 +1 @@ +- **fix(security):** a restricted API key is now enforced on the alias spellings `next.config.mjs` rewrites onto `/api/v1/…` — a route handler sees the client's original URL, so `POST /chat/completions`, `/responses`, `/responses/*`, `/models`, `/codex/*` and the doubled `/v1/v1/*` prefix all skipped the endpoint-category lookup and let a key allowed only on `search` reach chat or any other endpoint ([#13685](https://github.com/diegosouzapw/OmniRoute/issues/13685)) diff --git a/src/shared/constants/endpointCategories.ts b/src/shared/constants/endpointCategories.ts index fc3903d209..c71adc9e54 100644 --- a/src/shared/constants/endpointCategories.ts +++ b/src/shared/constants/endpointCategories.ts @@ -137,3 +137,39 @@ export function resolveEndpointCategory(pathname: string): string | null { } return null; } + +/** + * Short alias spellings `next.config.mjs` rewrites onto `/api/v1/…`, mapped to + * the `/v1/…` path the category table speaks. Mirrors that rewrite table. + */ +const ENDPOINT_ALIAS_REWRITES: readonly { source: string; canonical: string }[] = [ + { source: "/chat/completions", canonical: "/v1/chat/completions" }, + { source: "/responses", canonical: "/v1/responses" }, + { source: "/models", canonical: "/v1/models" }, + // `/codex/:path*` folds its sub-path onto the Responses route, which still + // resolves to the same category once the prefix is mapped. + { source: "/codex", canonical: "/v1/responses" }, +]; + +/** + * Rewrite a client-facing alias path onto the canonical `/v1/…` form that + * `resolveEndpointCategory()` understands. + * + * A route handler sees the client's original URL — Next never rewrites + * `request.url` — so a request that arrived as `/chat/completions`, `/models` + * or `/codex/…` matched no prefix at all and the endpoint check was skipped + * entirely (#13685). The `/api/v1/…` (App Router) and doubled `/v1/v1/…` + * spellings of the same endpoint fold onto `/v1/…` here as well. Anything + * already canonical, or belonging to no category, comes back unchanged. + */ +export function resolveCanonicalEndpointPath(pathname: string): string { + const path = pathname.replace(/^\/api(?=\/v1\/)/, "").replace(/^\/v1\/v1(?=\/|$)/, "/v1"); + + for (const { source, canonical } of ENDPOINT_ALIAS_REWRITES) { + if (path === source || path.startsWith(source + "/")) { + return canonical + path.slice(source.length); + } + } + + return path; +} diff --git a/src/shared/utils/apiKeyPolicy.ts b/src/shared/utils/apiKeyPolicy.ts index 1118d3b7c5..b100cbcc7f 100644 --- a/src/shared/utils/apiKeyPolicy.ts +++ b/src/shared/utils/apiKeyPolicy.ts @@ -23,7 +23,10 @@ import { import { HTTP_STATUS } from "@omniroute/open-sse/config/constants.ts"; import * as log from "@/sse/utils/logger"; import { checkRateLimit, RateLimitRule } from "./rateLimiter"; -import { resolveEndpointCategory } from "@/shared/constants/endpointCategories"; +import { + resolveCanonicalEndpointPath, + resolveEndpointCategory, +} from "@/shared/constants/endpointCategories"; import { resolveQuotaKeyScope } from "@/lib/quota/quotaKey"; import { isQuotaModelName, parseQuotaModelName } from "@/lib/quota/quotaModelNaming"; import { buildApiKeyUsageLimitPolicyRejection } from "@/lib/usage/apiKeyUsageLimits"; @@ -497,10 +500,12 @@ function validateEndpointAccess(context: PolicyContext): Response | null { if (!apiKeyInfo.allowedEndpoints?.length) return null; try { // A route handler sees the client's original URL: `/v1/…` when the - // `/v1/:path*` rewrite fired, but `/api/v1/…` when the client hit the App - // Router path directly (no rewrite). The category prefixes are `/v1/…`, so - // strip the `/api` shape or a restricted key silently passes on that path. - const pathname = new URL(request.url).pathname.replace(/^\/api(?=\/v1\/)/, ""); + // `/v1/:path*` rewrite fired, `/api/v1/…` when the client hit the App + // Router path directly (no rewrite), and the raw alias spelling + // (`/chat/completions`, `/models`, `/codex/…`, `/v1/v1/…`) in every case. + // The category prefixes are `/v1/…`, so canonicalize the path first or a + // restricted key silently passes on those spellings (#13685). + const pathname = resolveCanonicalEndpointPath(new URL(request.url).pathname); const category = resolveEndpointCategory(pathname); if (category && !apiKeyInfo.allowedEndpoints.includes(category)) { return errorResponse( diff --git a/tests/unit/endpoint-categories.test.ts b/tests/unit/endpoint-categories.test.ts index cb8179ab19..24fe45e95d 100644 --- a/tests/unit/endpoint-categories.test.ts +++ b/tests/unit/endpoint-categories.test.ts @@ -12,7 +12,7 @@ import assert from "node:assert/strict"; // ─── resolveEndpointCategory: pure function tests ───────────────────────── // Import the pure resolver without DB dependencies -const { resolveEndpointCategory } = +const { resolveCanonicalEndpointPath, resolveEndpointCategory } = await import("../../src/shared/constants/endpointCategories.ts"); test("resolveEndpointCategory: maps /v1/chat/completions to 'chat'", () => { @@ -136,3 +136,60 @@ test("resolveEndpointCategory: handles sub-paths under category", () => { test("resolveEndpointCategory: maps /v1/batches/delete-completed to 'batches' (bulk sweep is policy-gated)", () => { assert.equal(resolveEndpointCategory("/v1/batches/delete-completed"), "batches"); }); + +// ─── resolveCanonicalEndpointPath: alias spellings from next.config.mjs ─── +// A route handler sees the client's original URL, so an alias spelling has to +// be mapped onto the canonical `/v1/…` path or the category check is skipped +// altogether (#13685). + +test("resolveCanonicalEndpointPath: maps /chat/completions onto the chat endpoint", () => { + assert.equal(resolveCanonicalEndpointPath("/chat/completions"), "/v1/chat/completions"); + assert.equal(resolveEndpointCategory(resolveCanonicalEndpointPath("/chat/completions")), "chat"); +}); + +test("resolveCanonicalEndpointPath: maps /responses and its sub-paths onto the responses endpoint", () => { + assert.equal(resolveCanonicalEndpointPath("/responses"), "/v1/responses"); + assert.equal( + resolveCanonicalEndpointPath("/responses/input_tokens"), + "/v1/responses/input_tokens" + ); + assert.equal(resolveEndpointCategory(resolveCanonicalEndpointPath("/responses")), "chat"); + assert.equal( + resolveEndpointCategory(resolveCanonicalEndpointPath("/responses/input_tokens")), + "chat" + ); +}); + +test("resolveCanonicalEndpointPath: maps /models onto the models endpoint", () => { + assert.equal(resolveCanonicalEndpointPath("/models"), "/v1/models"); + assert.equal(resolveEndpointCategory(resolveCanonicalEndpointPath("/models")), "models"); +}); + +test("resolveCanonicalEndpointPath: /codex/… lands on the responses endpoint", () => { + assert.equal(resolveCanonicalEndpointPath("/codex"), "/v1/responses"); + assert.equal(resolveCanonicalEndpointPath("/codex/tasks/abc"), "/v1/responses/tasks/abc"); + assert.equal(resolveEndpointCategory(resolveCanonicalEndpointPath("/codex/tasks/abc")), "chat"); +}); + +test("resolveCanonicalEndpointPath: collapses the doubled /v1/v1 prefix", () => { + assert.equal(resolveCanonicalEndpointPath("/v1/v1/chat/completions"), "/v1/chat/completions"); + assert.equal(resolveCanonicalEndpointPath("/v1/v1/models"), "/v1/models"); + // `/v1/v1` alone maps onto the API root, which carries no category. + assert.equal(resolveEndpointCategory(resolveCanonicalEndpointPath("/v1/v1")), null); +}); + +test("resolveCanonicalEndpointPath: the /api/v1 App Router shape stays canonical", () => { + assert.equal(resolveCanonicalEndpointPath("/api/v1/chat/completions"), "/v1/chat/completions"); + assert.equal( + resolveCanonicalEndpointPath("/api/v1/images/generations"), + "/v1/images/generations" + ); +}); + +test("resolveCanonicalEndpointPath: canonical and uncategorised paths come back unchanged", () => { + assert.equal(resolveCanonicalEndpointPath("/v1/chat/completions"), "/v1/chat/completions"); + assert.equal(resolveCanonicalEndpointPath("/v1/search/analytics"), "/v1/search/analytics"); + assert.equal(resolveCanonicalEndpointPath("/"), "/"); + assert.equal(resolveCanonicalEndpointPath("/api/keys"), "/api/keys"); + assert.equal(resolveCanonicalEndpointPath("/dashboard"), "/dashboard"); +}); diff --git a/tests/unit/endpoint-restrictions-policy.test.ts b/tests/unit/endpoint-restrictions-policy.test.ts index dea603c947..6898a91cee 100644 --- a/tests/unit/endpoint-restrictions-policy.test.ts +++ b/tests/unit/endpoint-restrictions-policy.test.ts @@ -134,6 +134,99 @@ test("search-only key blocks /api/v1/chat/completions too — the App Router pat assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); }); +test("search-only key blocks the /chat/completions alias", async () => { + // `next.config.mjs` rewrites `/chat/completions` onto the chat route without + // touching `request.url`, so the policy sees the bare alias. It has to map + // onto the canonical `/v1/…` path or the allowlist fails open (#13685). + const policy = await loadPolicy("search-blocks-chat-alias"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/chat/completions", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the alias spelling"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); +}); + +test("search-only key blocks the /responses alias", async () => { + const policy = await loadPolicy("search-blocks-responses-alias"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/responses", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the /responses alias"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); +}); + +test("search-only key blocks a /responses sub-path alias", async () => { + const policy = await loadPolicy("search-blocks-responses-subpath"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/responses/input_tokens", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the /responses/* alias"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); +}); + +test("search-only key blocks the /codex/… alias", async () => { + const policy = await loadPolicy("search-blocks-codex-alias"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/codex/tasks/abc", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the /codex alias"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); +}); + +test("search-only key blocks the doubled /v1/v1 alias", async () => { + const policy = await loadPolicy("search-blocks-v1v1-alias"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/v1/v1/chat/completions", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the doubled /v1/v1 alias"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("chat"), `Error message should mention 'chat', got: ${msg}`); +}); + +test("search-only key blocks the /models alias", async () => { + const policy = await loadPolicy("search-blocks-models-alias"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/models", key.key); + const result = await policy.enforceApiKeyPolicy(request, "gpt-4"); + + assert.ok(result.rejection, "Should reject the /models alias"); + assert.equal(result.rejection.status, 403); + const msg = await readErrorMessage(result.rejection); + assert.ok(msg.includes("models"), `Error message should mention 'models', got: ${msg}`); +}); + +test("search-only key still reaches /v1/search through the canonical path", async () => { + // Control: the canonicalization must not turn an allowed endpoint into a + // rejection on the path that was already policed correctly. + const policy = await loadPolicy("search-allows-canonical"); + const key = await createKeyWithEndpoints(["search"]); + + const request = makeRequest("http://localhost/v1/search", key.key); + const result = await policy.enforceApiKeyPolicy(request, "search"); + + assert.equal(result.rejection, null); +}); + test("chat+embeddings key allows /v1/embeddings", async () => { const policy = await loadPolicy("chat-emb-allowed"); const key = await createKeyWithEndpoints(["chat", "embeddings"]);