security(self-hosted): entry divert skips enforceApiKeyPolicy; shared key compared without timing-safe check (#14485)

changelog.d/fixes/14485-selfhosted-entry-policy-bypass.md

@@ -0,0 +1 @@
+- security(self-hosted): enforce enforceApiKeyPolicy (schedule/rate-limit/allowedModels/quota) on the self-hosted unified-entry divert — only when the divert is configured, so cloud requests still run the policy exactly once inside handleChat() — and compare the optional shared self-hosted API key with a constant-time comparison instead of `!==` (#14485)

open-sse/services/selfHostedEntry.ts

@@ -14,15 +14,23 @@
  *  - Auto-route = header override -> model-prefix match -> deterministic strategy
  *    (`routingStrategies.ts`, M2/RIC-740). Every decision is explainable via the
  *    `x-omniroute-route-decision` response header — no predictive model.
- *  - The API-key check is a scaffold reserved for the D5 quota-key system: when
- *    `OMNIROUTE_SELF_HOSTED_API_KEY` is unset the route is open (loopback /
- *    trusted-network deployment), exactly like the existing self-hosted local
- *    providers.
+ *  - The optional `OMNIROUTE_SELF_HOSTED_API_KEY` shared-secret check here is a
+ *    scaffold reserved for the D5 quota-key system: when unset the route is open
+ *    (loopback / trusted-network deployment), exactly like the existing
+ *    self-hosted local providers; when set, it is compared with a constant-time
+ *    comparison (`timingSafeCompare`), never `===` (#14485, CWE-208).
+ *  - Every request through the unified `/v1/chat/completions` entry — including
+ *    this self-hosted divert — is gated by `enforceApiKeyPolicy()` (schedule,
+ *    rate limit, quota, allowedModels) exactly once: the route runs it right
+ *    before this divert only when `isSelfHostedEntryConfigured()`, and the cloud
+ *    path runs it inside handleChat() (#14485). This module's own optional shared-key check is a *separate*,
+ *    additive gate for the self-hosted config itself, not a substitute for it.
  */
 
 import * as yaml from "js-yaml";
 import { readFile } from "node:fs/promises";
 import { errorResponse, buildErrorBody, parseUpstreamError } from "../utils/error.ts";
+import { timingSafeCompare } from "@/shared/utils/timingSafeCompare";
 import { stripSensitiveResponseHeaders } from "../utils/upstreamResponseHeaders.ts";
 import type { ChatRequest, ProviderConfig } from "./providerAdapters.ts";
 import { ProviderRouter } from "./providerAdapters.ts";
@@ -397,18 +405,27 @@ export async function completeViaSelfHostedRouter(
  * failed to load/parse, returns a 500 error instead — a misconfigured entry
  * must never silently fall through to cloud routing.
  */
-export async function handleSelfHostedCompletions(
-  request: Request,
-  body: Record<string, unknown> | null,
-  options: SelfHostedOptions = {}
-): Promise<Response | null> {
-  const isConfigured = Boolean(
+/**
+ * True when a self-hosted provider config is present (inline, file, or env).
+ * `handleSelfHostedCompletions()` answers EVERY request once this is true, so
+ * the route uses it to scope work that must run only on the divert path (e.g.
+ * the #14485 key-policy gate, which the cloud path runs inside handleChat()).
+ */
+export function isSelfHostedEntryConfigured(options: SelfHostedOptions = {}): boolean {
+  return Boolean(
     options.providers ??
     options.providersFile ??
     process.env[CONFIG_ENV] ??
     process.env[CONFIG_FILE_ENV]
   );
-  if (!isConfigured) return null;
+}
+
+export async function handleSelfHostedCompletions(
+  request: Request,
+  body: Record<string, unknown> | null,
+  options: SelfHostedOptions = {}
+): Promise<Response | null> {
+  if (!isSelfHostedEntryConfigured(options)) return null;
 
   const runtime = await loadSelfHostedRuntime(options);
   if (!runtime) {
@@ -423,7 +440,7 @@ export async function handleSelfHostedCompletions(
   if (apiKey) {
     const authHeader = request.headers.get("authorization") ?? "";
     const expected = `Bearer ${apiKey}`;
-    if (authHeader !== expected) {
+    if (!timingSafeCompare(authHeader, expected)) {
       return errorResponse(401, "Invalid API key", { type: "authentication_error" });
     }
   }

src/app/api/v1/chat/completions/route.ts

@@ -5,7 +5,10 @@ import { handleChat } from "@/sse/handlers/chat";
 import { generateRequestId } from "@/shared/utils/requestId";
 import { resolveIncomingCorrelationId } from "@/shared/utils/correlationPreserve.ts";
 import { errorResponse } from "@omniroute/open-sse/utils/error.ts";
-import { handleSelfHostedCompletions } from "@omniroute/open-sse/services/selfHostedEntry.ts";
+import {
+  handleSelfHostedCompletions,
+  isSelfHostedEntryConfigured,
+} from "@omniroute/open-sse/services/selfHostedEntry.ts";
 import { initTranslators } from "@omniroute/open-sse/translator/index.ts";
 import { createInjectionGuard } from "@/middleware/promptInjectionGuard";
 import { acceptHeaderForcesStream } from "@omniroute/open-sse/utils/aiSdkCompat.ts";
@@ -29,6 +32,7 @@ import {
   withCompressionHeaderEcho,
 } from "@/shared/utils/compressionHeaderEcho";
 import { resolveModelAliasWithSeedFallbackOnBody } from "@/lib/modelAliasResolver";
+import { enforceApiKeyPolicy } from "@/shared/utils/apiKeyPolicy";
 import {
   assertRuntimeModelProviderAvailable,
   isRuntimeProviderRetirementError,
@@ -166,9 +170,28 @@ export async function POST(request) {
           // self-hosted model ids (`local/llama3`, `ollama/qwen2`, ...) never trip
           // cloud-peer 410s or alias rewrites. Config-absent requests proceed to the
           // normal cloud pipeline unchanged.
-          const selfHostedResponse = await handleSelfHostedCompletions(request, parsedBody);
-          if (selfHostedResponse) {
-            return finishAdmission(selfHostedResponse);
+          //
+          // #14485: the divert must still run the same key-policy enforcement as
+          // the normal cloud pipeline (enforceApiKeyPolicy, called deep inside
+          // handleChat() on that path) — otherwise a disabled/rate-limited/
+          // schedule-restricted OmniRoute API key reaches the self-hosted upstream
+          // unchecked. Run it ONLY when the divert is configured (it then answers
+          // every request): the cloud path already runs it once in handleChat(),
+          // and a second run would consume the rate-limit window twice, apply
+          // throttleDelayMs twice and check allowedModels before alias resolution.
+          if (isSelfHostedEntryConfigured()) {
+            const keyPolicy = await enforceApiKeyPolicy(
+              request,
+              typeof parsedBody.model === "string" ? parsedBody.model : null
+            );
+            if (keyPolicy.rejection) {
+              return finishAdmission(keyPolicy.rejection);
+            }
+
+            const selfHostedResponse = await handleSelfHostedCompletions(request, parsedBody);
+            if (selfHostedResponse) {
+              return finishAdmission(selfHostedResponse);
+            }
           }
 
           try {

tests/unit/self-hosted-entry-policy-bypass-14485.test.ts

@@ -0,0 +1,225 @@
+/**
+ * Repro for #14485 (part 1): the self-hosted unified-entry divert in
+ * `/v1/chat/completions` calls `handleSelfHostedCompletions()` and returns its
+ * response BEFORE `enforceApiKeyPolicy()` ever runs (that call lives deep
+ * inside `handleChat()`, which the divert never reaches). A disabled/banned
+ * OmniRoute API key — normally rejected with 403 "This API key is disabled" —
+ * sails straight through to the self-hosted provider once
+ * OMNIROUTE_SELF_HOSTED_PROVIDERS is configured.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { createServer, type Server } from "node:http";
+import type { AddressInfo } from "node:net";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-selfhosted-policy-14485-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "task-14485-api-key-secret";
+
+const coreDb = await import("../../src/lib/db/core.ts");
+const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
+const rateLimiter = await import("../../src/shared/utils/rateLimiter.ts");
+rateLimiter.setRateLimiterTestMode(true);
+
+let upstreamCalls = 0;
+const upstream: Server = createServer((req, res) => {
+  upstreamCalls++;
+  let raw = "";
+  req.on("data", (chunk) => (raw += chunk));
+  req.on("end", () => {
+    // Force the client (the self-hosted executor's fetch) to close its socket
+    // after this response instead of keeping it alive for reuse — an idle
+    // keep-alive connection otherwise left `http.Server#close()` hanging in
+    // this test's teardown (observed during the original repro run).
+    res.writeHead(200, { "content-type": "application/json", connection: "close" });
+    res.end(
+      JSON.stringify({
+        id: "chatcmpl-stub",
+        object: "chat.completion",
+        model: "gpt-oss",
+        choices: [
+          { index: 0, message: { role: "assistant", content: "ok" }, finish_reason: "stop" },
+        ],
+      })
+    );
+  });
+});
+
+await new Promise<void>((resolve) => upstream.listen(0, "127.0.0.1", resolve));
+const upstreamPort = (upstream.address() as AddressInfo).port;
+const upstreamBaseUrl = `http://127.0.0.1:${upstreamPort}/v1`;
+
+process.env.OMNIROUTE_SELF_HOSTED_PROVIDERS = [
+  "providers:",
+  "  - id: stub",
+  "    kind: openai",
+  `    baseUrl: ${upstreamBaseUrl}`,
+  "    model: gpt-oss",
+].join("\n");
+
+const chatRoute = await import("../../src/app/api/v1/chat/completions/route.ts");
+
+test.after(async () => {
+  // The stub upstream keeps its keep-alive sockets open by default, which left
+  // node's test runner hanging in post-assertion cleanup (observed during the
+  // original repro run). Force-close any still-open connections before closing
+  // the server so the process can exit promptly.
+  if (typeof upstream.closeAllConnections === "function") {
+    upstream.closeAllConnections();
+  }
+  await new Promise((resolve) => upstream.close(resolve));
+  apiKeysDb.resetApiKeyState();
+  coreDb.resetDbInstance();
+  try {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  } catch {
+    // best-effort cleanup
+  }
+});
+
+test("#14485: self-hosted divert must not bypass enforceApiKeyPolicy for a disabled OmniRoute key", async () => {
+  const created = await apiKeysDb.createApiKey("Disabled Key 14485", "machine-14485");
+  await apiKeysDb.updateApiKeyPermissions(created.id, { isActive: false });
+
+  const request = new Request("http://localhost/v1/chat/completions", {
+    method: "POST",
+    headers: {
+      "content-type": "application/json",
+      Authorization: `Bearer ${created.key}`,
+    },
+    body: JSON.stringify({
+      model: "local/gpt-oss",
+      messages: [{ role: "user", content: "hi" }],
+    }),
+  });
+
+  const response = await chatRoute.POST(request);
+
+  assert.notEqual(
+    response.status,
+    200,
+    "BUG #14485: a DISABLED OmniRoute API key reached the self-hosted upstream " +
+      `(status=${response.status}, upstream received ${upstreamCalls} call(s)) — ` +
+      "the divert at src/app/api/v1/chat/completions/route.ts skips enforceApiKeyPolicy entirely"
+  );
+  assert.equal(
+    response.status,
+    403,
+    "expected the standard disabled-key rejection (403) from enforceApiKeyPolicy"
+  );
+});
+
+test("#14485: self-hosted divert still respects an active key's allowedModels restriction", async () => {
+  const created = await apiKeysDb.createApiKey("Restricted Key 14485", "machine-14485-restricted");
+  await apiKeysDb.updateApiKeyPermissions(created.id, {
+    allowedModels: ["openai/gpt-4.1"], // deliberately excludes the self-hosted model below
+  });
+
+  const before = upstreamCalls;
+  const request = new Request("http://localhost/v1/chat/completions", {
+    method: "POST",
+    headers: {
+      "content-type": "application/json",
+      Authorization: `Bearer ${created.key}`,
+    },
+    body: JSON.stringify({
+      model: "local/gpt-oss",
+      messages: [{ role: "user", content: "hi" }],
+    }),
+  });
+
+  const response = await chatRoute.POST(request);
+
+  assert.notEqual(
+    response.status,
+    200,
+    "an allowedModels-restricted key must not reach the self-hosted upstream for a " +
+      `disallowed model (status=${response.status})`
+  );
+  assert.equal(upstreamCalls, before, "the self-hosted upstream must not have been called");
+});
+
+test("#14485: an active key with no restrictions still reaches the self-hosted upstream", async () => {
+  const created = await apiKeysDb.createApiKey("Unrestricted Key 14485", "machine-14485-ok");
+
+  const before = upstreamCalls;
+  const request = new Request("http://localhost/v1/chat/completions", {
+    method: "POST",
+    headers: {
+      "content-type": "application/json",
+      Authorization: `Bearer ${created.key}`,
+    },
+    body: JSON.stringify({
+      model: "local/gpt-oss",
+      messages: [{ role: "user", content: "hi" }],
+    }),
+  });
+
+  const response = await chatRoute.POST(request);
+
+  assert.equal(response.status, 200, "a policy-compliant key must still reach self-hosted");
+  assert.equal(upstreamCalls, before + 1, "the self-hosted upstream must have been called once");
+});
+
+test("#14485: a cloud request (no self-hosted config) runs the key policy ONCE, not twice", async () => {
+  // A key allowed exactly one request per minute. The cloud pipeline already
+  // enforces the policy inside handleChat(); if the route ALSO enforced it
+  // ahead of the (unconfigured) self-hosted divert, this single request would
+  // consume the window twice and be rejected with 429 by the second check.
+  const created = await apiKeysDb.createApiKey("One-per-minute Key 14485", "machine-14485-rl");
+  await apiKeysDb.updateApiKeyPermissions(created.id, {
+    rateLimits: [{ limit: 1, window: 60 }],
+  });
+
+  const savedConfig = process.env.OMNIROUTE_SELF_HOSTED_PROVIDERS;
+  delete process.env.OMNIROUTE_SELF_HOSTED_PROVIDERS;
+  const before = upstreamCalls;
+  try {
+    const request = new Request("http://localhost/v1/chat/completions", {
+      method: "POST",
+      headers: {
+        "content-type": "application/json",
+        Authorization: `Bearer ${created.key}`,
+      },
+      body: JSON.stringify({
+        model: "openai/gpt-4.1",
+        messages: [{ role: "user", content: "hi" }],
+      }),
+    });
+
+    const response = await chatRoute.POST(request);
+    const text = await response.text();
+
+    assert.notEqual(
+      response.status,
+      429,
+      "a single cloud request must not trip a 1-req/min limit — the key policy ran twice " +
+        `(route pre-divert + handleChat): ${text.slice(0, 200)}`
+    );
+    assert.doesNotMatch(text, /Request limit exceeded/);
+    assert.equal(upstreamCalls, before, "a cloud request must never reach the self-hosted stub");
+  } finally {
+    process.env.OMNIROUTE_SELF_HOSTED_PROVIDERS = savedConfig;
+  }
+});
+
+test("#14485: the shared self-hosted API key is compared with the constant-time helper, not `!==`", async () => {
+  const source = fs.readFileSync(
+    new URL("../../open-sse/services/selfHostedEntry.ts", import.meta.url),
+    "utf8"
+  );
+  assert.match(
+    source,
+    /timingSafeCompare\(\s*authHeader\s*,\s*expected\s*\)/,
+    "expected the shared self-hosted API key comparison to go through timingSafeCompare(), " +
+      "not a raw `!==` (CWE-208 timing side-channel)"
+  );
+  assert.doesNotMatch(
+    source,
+    /authHeader\s*!==\s*expected/,
+    "the raw non-constant-time comparison must be fully replaced, not just supplemented"
+  );
+});