fix(sse): reject non-string model with 400 before resolver (#6407) (#6433)

reject non-string model with 400 before resolver (#6407, 6/6). Reconciled with #6437 early schema validation on chat.ts. Integrated into release/v3.8.46.

CHANGELOG.md

@@ -21,6 +21,7 @@
 
 ### 🐛 Bug Fixes
 
+- **fix(sse):** a non-string `model` field is now rejected with a `400` before the resolver, instead of crashing downstream `.toLowerCase()`/`.split()` calls into an empty-body `500` that escapes the error sanitizer ([#6407](https://github.com/diegosouzapw/OmniRoute/issues/6407)). Regression guard: `tests/unit/chat-non-string-model-6407.test.ts`. (thanks @chirag127)
 - **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)

src/sse/handlers/chat.ts

@@ -245,6 +245,20 @@ export async function handleChat(
     return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required");
   }
 
+  // Reject non-string `model` before it reaches downstream code that calls
+  // `.toLowerCase()` / `.split()` / `.startsWith()` on it (crash-then-500 with an
+  // empty body, escaping the error sanitizer — #6407). An explicit `null`/`undefined`
+  // stays permitted here because the existing `Missing model` guard below returns a
+  // clean 400 for those; anything else that is not a string is a client type error.
+  const rawModel = (body as { model?: unknown }).model;
+  if (rawModel !== undefined && rawModel !== null && typeof rawModel !== "string") {
+    log.warn("CHAT", `Rejecting non-string model (typeof=${typeof rawModel})`);
+    return errorResponse(
+      HTTP_STATUS.BAD_REQUEST,
+      `model: Expected string, received ${Array.isArray(rawModel) ? "array" : typeof rawModel}`
+    );
+  }
+
   // 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

tests/unit/chat-non-string-model-6407.test.ts

@@ -0,0 +1,123 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { createChatPipelineHarness } from "../integration/_chatPipelineHarness.ts";
+
+// Regression test for #6407 — a `model` field of a non-string type
+// (`number`/`boolean`/`array`/`object`) crashed downstream string ops
+// (`.toLowerCase()`/`.split()`/`.startsWith()`) and returned HTTP 500 with an
+// empty body, bypassing the error sanitizer (Hard Rule #12).
+//
+// The guard rejects non-string `model` with a clean 400 + typed error message
+// BEFORE the model resolver runs, matching the #5110 empty-messages precedent.
+
+const harness = await createChatPipelineHarness("chat-non-string-model-6407");
+const { handleChat, buildRequest, resetStorage, seedConnection } = harness;
+
+test.beforeEach(async () => {
+  await resetStorage();
+});
+
+test.after(async () => {
+  await harness.cleanup();
+});
+
+for (const [label, value, expectedType] of [
+  ["number", 123, "number"],
+  ["boolean", true, "boolean"],
+  ["array", [], "array"],
+  ["object", {}, "object"],
+] as const) {
+  test(`#6407: model as ${label} → 400 with typed error, no upstream call`, async () => {
+    await seedConnection("openai", { apiKey: "sk-openai" });
+
+    let upstreamCalled = false;
+    globalThis.fetch = async () => {
+      upstreamCalled = true;
+      return new Response("{}", {
+        status: 200,
+        headers: { "content-type": "application/json" },
+      });
+    };
+
+    const response = await handleChat(
+      buildRequest({
+        body: {
+          model: value,
+          messages: [{ role: "user", content: "hi" }],
+        },
+      })
+    );
+
+    assert.equal(response.status, 400, `${label} model must be a 400, not 500`);
+    const body = (await response.json()) as { error?: { message?: string } };
+    assert.match(
+      body.error?.message ?? "",
+      /model:\s*Expected string, received/i,
+      `error should say "model: Expected string, received ${expectedType}"`
+    );
+    assert.ok(
+      body.error?.message?.includes(expectedType),
+      `error should include the received type "${expectedType}"`
+    );
+    // Sanitized: no leaked stack frames per Hard Rule #12 / #6407 impact 3.
+    assert.ok(
+      !(body.error?.message ?? "").includes("at /"),
+      "error must not leak stack trace frames"
+    );
+    assert.equal(upstreamCalled, false, "must not forward a non-string-model request upstream");
+  });
+}
+
+test("#6407: string model still routes normally (guard is not over-broad)", async () => {
+  await seedConnection("openai", { apiKey: "sk-openai" });
+
+  let upstreamCalled = false;
+  globalThis.fetch = async () => {
+    upstreamCalled = true;
+    return Response.json({
+      id: "x",
+      object: "chat.completion",
+      choices: [
+        { index: 0, message: { role: "assistant", content: "hi" }, finish_reason: "stop" },
+      ],
+    });
+  };
+
+  const response = await handleChat(
+    buildRequest({
+      body: {
+        model: "openai/gpt-4.1",
+        stream: false,
+        messages: [{ role: "user", content: "Hello" }],
+      },
+    })
+  );
+
+  assert.notEqual(response.status, 400, "a valid string model must not be caught by the guard");
+  assert.equal(upstreamCalled, true, "a valid request must still reach upstream");
+});
+
+test("#6407: null model still routed to the existing 'Missing model' 400 (not the new guard)", async () => {
+  await seedConnection("openai", { apiKey: "sk-openai" });
+
+  globalThis.fetch = async () =>
+    new Response("{}", { status: 200, headers: { "content-type": "application/json" } });
+
+  const response = await handleChat(
+    buildRequest({
+      body: {
+        model: null,
+        messages: [{ role: "user", content: "hi" }],
+      },
+    })
+  );
+
+  assert.equal(response.status, 400, "null model stays a 400");
+  const body = (await response.json()) as { error?: { message?: string } };
+  assert.match(
+    body.error?.message ?? "",
+    /missing model/i,
+    "null model keeps the existing 'Missing model' message (not the new type guard)"
+  );
+});