diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d7c61f4d6..a65ecebcc7 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/app/api/[...omnirouteApiCatchAll]/route.ts b/src/app/api/[...omnirouteApiCatchAll]/route.ts new file mode 100644 index 0000000000..5025f06194 --- /dev/null +++ b/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); +} diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index af00b793eb..5be9a25842 100644 --- a/src/sse/handlers/chat.ts +++ b/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) { diff --git a/tests/unit/api/api-catchall-json-404.test.ts b/tests/unit/api/api-catchall-json-404.test.ts new file mode 100644 index 0000000000..c401970537 --- /dev/null +++ b/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 Promise>)[ + 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", + ); +}); diff --git a/tests/unit/chat-early-schema-validation-6412.test.ts b/tests/unit/chat-early-schema-validation-6412.test.ts new file mode 100644 index 0000000000..8176366082 --- /dev/null +++ b/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 +});