fix(chat): validate scalar params before provider lookup (#6412) (#6437)

JSON 404 for unknown /api/* (#6424) + early scalar-param validation before provider lookup (#6412, 10/10). Integrated into release/v3.8.46.

CHANGELOG.md

@@ -21,6 +21,7 @@
 
 ### 🐛 Bug Fixes
 
+- **fix(api):** unknown `/api/*` routes now return a JSON `404` (instead of the dashboard HTML shell) and scalar chat params (`model`/`temperature`/etc.) are validated **before** the provider lookup so malformed requests fail fast with a clear `400` ([#6424](https://github.com/diegosouzapw/OmniRoute/issues/6424), [#6412](https://github.com/diegosouzapw/OmniRoute/issues/6412)). Regression guards: `tests/unit/api/api-catchall-json-404.test.ts`, `tests/unit/chat-early-schema-validation-6412.test.ts`. (thanks @chirag127)
 - **fix(api):** `/v1/chat/completions` now rejects a non-JSON `Content-Type` with a `400` before parsing the body ([#6414](https://github.com/diegosouzapw/OmniRoute/issues/6414)). Regression guard: `tests/unit/v1-chat-completions-content-type-6414.test.ts`. (thanks @chirag127)
 - **fix(api):** the `X-OmniRoute-Compression` response header is now echoed on `/v1/chat/completions` and `/v1/completions` ([#6422](https://github.com/diegosouzapw/OmniRoute/issues/6422)). Regression guard: `tests/unit/compression-header-echo-6422.test.ts`. (thanks @chirag127)
 - **fix(api):** concurrent `GET /v1/models` requests are coalesced into a single catalog build ([#6408](https://github.com/diegosouzapw/OmniRoute/issues/6408)). Regression guard: `tests/unit/v1-models-concurrent-6408.test.ts`. (thanks @chirag127)

src/app/api/[...omnirouteApiCatchAll]/route.ts

@@ -0,0 +1,56 @@
+import { CORS_HEADERS } from "@/shared/utils/cors";
+
+/**
+ * Catch-all fallback for /api/* — issue #6424.
+ *
+ * Extends the /v1/* catch-all (#6405) to the whole /api/ tree. Without this
+ * route, unknown paths under /api/context/*, /api/settings/*, /api/admin/*
+ * (etc.) fell through to the Next.js app-router `not-found.tsx`, which
+ * returned the dashboard HTML shell (~463 KB) to CLI/SDK callers instead of a
+ * JSON 404. Combined with the management-auth boundary, this made an
+ * "unknown route" indistinguishable from a "wrong scope" failure.
+ *
+ * Static / dynamic segments under /api/ take precedence over this catch-all
+ * in Next.js App Router matching, so real routes are unaffected.
+ */
+
+function notFoundResponse(request: Request): Response {
+  const url = new URL(request.url);
+  return Response.json(
+    {
+      error: {
+        message: `Unknown API route: ${url.pathname}`,
+        type: "not_found",
+        code: "unknown_route",
+        path: url.pathname,
+      },
+    },
+    {
+      status: 404,
+      headers: { "Content-Type": "application/json", ...CORS_HEADERS },
+    },
+  );
+}
+
+export async function OPTIONS() {
+  return new Response(null, { headers: CORS_HEADERS });
+}
+
+export async function GET(request: Request) {
+  return notFoundResponse(request);
+}
+export async function POST(request: Request) {
+  return notFoundResponse(request);
+}
+export async function PUT(request: Request) {
+  return notFoundResponse(request);
+}
+export async function PATCH(request: Request) {
+  return notFoundResponse(request);
+}
+export async function DELETE(request: Request) {
+  return notFoundResponse(request);
+}
+export async function HEAD(request: Request) {
+  return notFoundResponse(request);
+}

src/sse/handlers/chat.ts

@@ -245,6 +245,53 @@ export async function handleChat(
     return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required");
   }
 
+  // Early schema validation for scalar params BEFORE provider/model resolution (#6412).
+  // Previously, a bad `temperature: "not-a-number"` on an unknown provider returned
+  // 404 "model_not_found" — hiding the real schema error. Validate the param shape
+  // first so the client gets a 400 with the field name. Kept narrow to widely-supported
+  // OpenAI-spec params (temperature 0..2, top_p 0..1, max_tokens int >=1) so we don't
+  // reject legitimate provider-specific fields.
+  {
+    const b = body as {
+      temperature?: unknown;
+      top_p?: unknown;
+      max_tokens?: unknown;
+      n?: unknown;
+    };
+    const badParam = (name: string, msg: string) =>
+      errorResponse(HTTP_STATUS.BAD_REQUEST, `${name}: ${msg}`);
+    if (b.temperature !== undefined) {
+      if (typeof b.temperature !== "number" || Number.isNaN(b.temperature)) {
+        return badParam("temperature", "must be a number");
+      }
+      if (b.temperature < 0 || b.temperature > 2) {
+        return badParam("temperature", "must be between 0 and 2");
+      }
+    }
+    if (b.top_p !== undefined) {
+      if (typeof b.top_p !== "number" || Number.isNaN(b.top_p)) {
+        return badParam("top_p", "must be a number");
+      }
+      if (b.top_p < 0 || b.top_p > 1) {
+        return badParam("top_p", "must be between 0 and 1");
+      }
+    }
+    if (b.max_tokens !== undefined) {
+      if (
+        typeof b.max_tokens !== "number" ||
+        !Number.isInteger(b.max_tokens) ||
+        b.max_tokens < 1
+      ) {
+        return badParam("max_tokens", "must be a positive integer");
+      }
+    }
+    if (b.n !== undefined) {
+      if (typeof b.n !== "number" || !Number.isInteger(b.n) || b.n < 1) {
+        return badParam("n", "must be a positive integer");
+      }
+    }
+  }
+
   // buildClientRawRequest already deep-clones the body, so pass `body` directly — the
   // prior local clone was a redundant second full-body copy on the hot path (#5152).
   if (!clientRawRequest) {

tests/unit/api/api-catchall-json-404.test.ts

@@ -0,0 +1,55 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+/**
+ * Regression: issue #6424 — unknown paths under /api/* (outside /api/v1/*)
+ * returned the Next.js dashboard HTML 404 shell instead of JSON. This made
+ * management-auth failures indistinguishable from missing routes, and forced
+ * CLI/SDK callers to parse ~463 KB of HTML.
+ *
+ * Complements the /v1/* catch-all from #6405; asserts the app-router catch-all
+ * under `src/app/api/[...omnirouteApiCatchAll]/route.ts` returns JSON for
+ * every HTTP method.
+ */
+
+const catchAll = await import(
+  "../../../src/app/api/[...omnirouteApiCatchAll]/route.ts"
+);
+
+function makeReq(pathname: string, method = "GET"): Request {
+  return new Request(`http://localhost:20128${pathname}`, { method });
+}
+
+test("/api catchall returns application/json 404 with not_found type on GET", async () => {
+  const res = await catchAll.GET(makeReq("/api/context/rtk/does-not-exist"));
+  assert.equal(res.status, 404);
+  const ct = res.headers.get("content-type") || "";
+  assert.ok(ct.includes("application/json"), `expected JSON content-type, got: ${ct}`);
+  const body = (await res.json()) as {
+    error: { type: string; message: string; code?: string; path?: string };
+  };
+  assert.equal(body.error.type, "not_found");
+  assert.equal(body.error.path, "/api/context/rtk/does-not-exist");
+  assert.match(body.error.message, /Unknown API route/);
+});
+
+test("/api catchall returns JSON 404 on POST / PUT / PATCH / DELETE / HEAD", async () => {
+  for (const method of ["POST", "PUT", "PATCH", "DELETE", "HEAD"] as const) {
+    const res = await (catchAll as Record<string, (r: Request) => Promise<Response>>)[
+      method
+    ](makeReq(`/api/settings/nope/${method.toLowerCase()}`, method));
+    assert.equal(res.status, 404, `${method} status`);
+    assert.ok(
+      (res.headers.get("content-type") || "").includes("application/json"),
+      `${method} content-type`,
+    );
+  }
+});
+
+test("/api catchall OPTIONS preflight returns CORS headers", async () => {
+  const res = await catchAll.OPTIONS();
+  assert.ok(
+    res.headers.get("access-control-allow-methods"),
+    "OPTIONS must expose CORS methods header",
+  );
+});

tests/unit/chat-early-schema-validation-6412.test.ts

@@ -0,0 +1,97 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { createChatPipelineHarness } from "../integration/_chatPipelineHarness.ts";
+
+const harness = await createChatPipelineHarness("chat-early-schema-6412");
+const { buildRequest, handleChat, resetStorage } = harness;
+
+test.beforeEach(async () => {
+  await resetStorage();
+});
+
+test.after(async () => {
+  await harness.cleanup();
+});
+
+/**
+ * Regression guard for #6412 — schema validation of scalar params (temperature,
+ * top_p, max_tokens, n) MUST run BEFORE provider/model resolution. Previously,
+ * a bad `temperature: "not-a-number"` combined with an unknown provider
+ * returned 404 "model_not_found" — hiding the real schema error.
+ */
+
+async function postChat(body: any) {
+  const response = await handleChat(buildRequest({ body }));
+  const payload = (await response.json()) as any;
+  return { status: response.status, payload };
+}
+
+test("bad temperature (string) on unknown provider → 400, not 404", async () => {
+  const { status, payload } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+    temperature: "not-a-number",
+  });
+  assert.equal(status, 400);
+  assert.match(JSON.stringify(payload.error), /temperature/i);
+});
+
+test("out-of-range temperature (5.0) on unknown provider → 400, not 404", async () => {
+  const { status, payload } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+    temperature: 5.0,
+  });
+  assert.equal(status, 400);
+  assert.match(JSON.stringify(payload.error), /temperature/i);
+});
+
+test("bad top_p (string) → 400", async () => {
+  const { status, payload } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+    top_p: "bad",
+  });
+  assert.equal(status, 400);
+  assert.match(JSON.stringify(payload.error), /top_p/i);
+});
+
+test("bad max_tokens (negative) → 400", async () => {
+  const { status, payload } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+    max_tokens: -1,
+  });
+  assert.equal(status, 400);
+  assert.match(JSON.stringify(payload.error), /max_tokens/i);
+});
+
+test("bad n (0) → 400", async () => {
+  const { status, payload } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+    n: 0,
+  });
+  assert.equal(status, 400);
+  assert.match(JSON.stringify(payload.error), /n:/);
+});
+
+test("valid params (temperature=0.7) on unknown provider still 404 (provider lookup runs after schema ok)", async () => {
+  const { status, payload } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+    temperature: 0.7,
+    max_tokens: 100,
+  });
+  assert.equal(status, 404);
+  assert.match(JSON.stringify(payload.error), /model_not_found|No active credentials/i);
+});
+
+test("params omitted entirely → schema passes, no false 400", async () => {
+  const { status } = await postChat({
+    model: "nonexistent-provider/nonexistent-model",
+    messages: [{ role: "user", content: "hi" }],
+  });
+  assert.equal(status, 404); // model lookup still 404 after schema pass-through
+});