fix(security): enforce allowedEndpoints on the alias rewrites (#13685) (#13741)

Co-authored-by: Goni Sulaiman <gonisulaimann@users.noreply.github.com>

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))

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;
+}

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(

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");
+});

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"]);