diff --git a/changelog.d/fixes/12643-messages-entry-guard.md b/changelog.d/fixes/12643-messages-entry-guard.md new file mode 100644 index 0000000000..a7412f9380 --- /dev/null +++ b/changelog.d/fixes/12643-messages-entry-guard.md @@ -0,0 +1 @@ +- **fix(chat):** requests with null/non-object entries in `messages[]` are now rejected with a clear 400 instead of crashing translators with an HTTP 500 ([#12643](https://github.com/diegosouzapw/OmniRoute/issues/12643)) — thanks @soroush5 diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 994a212281..ee72d15446 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -1,4 +1,5 @@ { + "_rebaseline_2026_09_15_12643_messages_entry_guard": "#12643 own growth: src/sse/handlers/chat.ts +10 (2490->2500 after syncing the 09-16 base, which itself moved the frozen value). A `messages` array containing a null/non-object entry (e.g. `[null]`) passed every existing entry guard (#5110/#6402/#6407/#6412) and crashed downstream translators/session helpers (openai-to-claude.ts, sessionManager.ts, contextManager.ts's fixToolPairs) reading `.role`/`.content` off the raw entry, surfacing as an HTTP 500 instead of a clean 400. Adds one more entry-shape guard clause to the same chokepoint, extending the existing guard family — same pattern, irreducible call-site wiring (the check itself is a one-line `.some()` predicate, not extractable into its own leaf without hiding the chokepoint). Covered by tests/unit/chat-messages-entry-objects-12643.test.ts (3/3) plus the sibling guard suites (chat-messages-validation-6402.test.ts, chat-non-string-model-6407.test.ts, 22/22, no regression). ATUALIZADO 2026-09-17: o teto foi refixado em 2519 ao mergear o tip atual. O tip sozinho ja esta em 2509 (acima do teto 2500 que esta PR havia fixado contra um tip anterior); o +10 desta PR e o proprio guard de entrada. O excedente do tip (2509>2500) e base-red herdado, nao introduzido aqui.", "_rebaseline_2026_09_17_13185_claude_oauth_sticky_refresh": "PR #13185 (@RaviTharuma): soft-fail do refresh do Claude para CredentialHealth nao ficar sticky-dead. src/lib/tokenHealthCheck.ts 1214 (tip) -> 1220 na branch e 1221 na arvore combinada com #13426; teto fixado em 1221. O teto anterior (1218) tinha apenas 4 linhas de folga. O crescimento e o proprio fix: preservar o refresh_token e distinguir falha transitoria de credencial morta exige estado extra no caminho de sweep, que nao pode sair do modulo sem quebrar a API interna. Coberto por tests/unit/tokenHealthCheck-claude-refresh-token-preserved.test.ts; os 14 arquivos irmaos de tokenHealthCheck/credentialHealth foram rodados juntos (72/72).", "_rebaseline_2026_09_16_jxnlexn_wave_growth": "Combined growth of the 2026-09-15 maxmad64bis uplift batch (each PR rebaselined its own growth; the merged sum is larger): src/sse/handlers/chat.ts->2498; open-sse/handlers/chatCore.ts->6181. Every hunk is flag-gated or a verified fix covered by that PR's tests; see the batch report.", "_rebaseline_2026_09_16_wave22_growth": "Combined growth of the 2026-09-15 maxmad64bis uplift batch (each PR rebaselined its own growth; the merged sum is larger): src/sse/handlers/chatHelpers.ts->1231; open-sse/executors/cursor.ts->1808. Every hunk is flag-gated or a verified fix covered by that PR's tests; see the batch report.", @@ -504,7 +505,7 @@ "src/shared/components/RequestLoggerV2.tsx": 1718, "src/shared/constants/providers/apikey/gateways.ts": 1502, "src/shared/services/cliRuntime.ts": 1296, - "src/sse/handlers/chat.ts": 2500, + "src/sse/handlers/chat.ts": 2520, "src/sse/services/auth.ts": 3557, "tests/unit/account-fallback-service.test.ts": 2453, "tests/unit/provider-validation-specialty.test.ts": 4656, diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index a377853d1b..26c7b42e0a 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -486,6 +486,16 @@ async function handleChatImplementation( log.warn("CHAT", "Rejecting request with empty messages array"); return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required"); } + // Reject non-object entries before they reach code that reads `msg.role` / + // `msg.content` off them (crash-then-500 in translators — #12643). The + // route schema accepts `z.array(z.unknown())`, so `[null]` gets this far. + if ( + Array.isArray(msgBody.messages) && + msgBody.messages.some((m) => m === null || typeof m !== "object" || Array.isArray(m)) + ) { + log.warn("CHAT", "Rejecting request with non-object message entries"); + return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: Expected array of objects"); + } if (!("messages" in msgBody) && !("input" in msgBody) && sourceFormat !== "antigravity") { log.warn("CHAT", "Rejecting request with missing messages"); return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: Expected array, received undefined"); diff --git a/tests/unit/chat-messages-entry-objects-12643.test.ts b/tests/unit/chat-messages-entry-objects-12643.test.ts new file mode 100644 index 0000000000..412bfac5ac --- /dev/null +++ b/tests/unit/chat-messages-entry-objects-12643.test.ts @@ -0,0 +1,69 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { createChatPipelineHarness } from "../integration/_chatPipelineHarness.ts"; + +// Regression tests for #12643 — a `messages` array containing `null` (or any +// non-object entry) passed every entry guard and crashed translators with a +// raw TypeError (`msg.role` off null), surfacing as HTTP 500. +// +// The guard at src/sse/handlers/chat.ts now rejects non-object entries with a +// clear OmniRoute-level 400 before any routing or upstream call, extending the +// #5110/#6402/#6407/#6412 guard family. + +const harness = await createChatPipelineHarness("chat-messages-entry-objects-12643"); +const { handleChat, buildRequest, resetStorage, seedConnection } = harness; + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + await harness.cleanup(); +}); + +async function postMessages(messages: unknown) { + await seedConnection("anthropic", { apiKey: "sk-ant" }); + + 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: "anthropic/claude-haiku-4-5", + messages, + }, + }) + ); + const body = (await response.json()) as { error?: { message?: string } }; + return { response, body, upstreamCalled }; +} + +test("#12643: messages: [null] is rejected with a clear 400", async () => { + const { response, body, upstreamCalled } = await postMessages([null]); + + assert.equal(response.status, 400, "null entry must be a 400, not a 500 crash"); + assert.match(body.error?.message ?? "", /Expected array of objects/i); + assert.equal(upstreamCalled, false, "must not forward upstream"); +}); + +test("#12643: messages with string/number entries are rejected with a clear 400", async () => { + for (const messages of [[{ role: "user", content: "hi" }, "oops"], [42]]) { + const { response, body, upstreamCalled } = await postMessages(messages); + + assert.equal(response.status, 400, `must be a 400: ${JSON.stringify(messages)}`); + assert.match(body.error?.message ?? "", /Expected array of objects/i); + assert.equal(upstreamCalled, false, "must not forward upstream"); + } +}); + +test("#12643: well-formed messages pass the entry guard", async () => { + const { response, body } = await postMessages([{ role: "user", content: "hi" }]); + + const msg = body.error?.message ?? ""; + assert.ok(!(response.status === 400 && /Expected array of objects/i.test(msg))); +});