From 7aa17fd0ed7765d40a11397141e9e5917a257575 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 15 Sep 2026 13:39:30 -0300 Subject: [PATCH 1/2] fix(chat): reject null/non-object entries in messages[] (#12643) A messages array containing null (or any non-object entry, e.g. [null] or [42]) passed every existing entry guard in chat.ts (#5110/#6402/#6407/#6412) and reached downstream translators/session helpers that read `.role` / `.content` directly off each entry (openai-to-claude.ts, sessionManager.ts, contextManager.ts's fixToolPairs), crashing with a raw TypeError and surfacing as an HTTP 500 instead of a clean 400. The route's Zod schema is intentionally wide (z.array(z.unknown())), so this shape check belongs in the handler's guard chain. Adds one more entry-shape guard clause to the same chokepoint, rejecting the request with a clear 400 before any routing or upstream call. Regression test: tests/unit/chat-messages-entry-objects-12643.test.ts PR #12644 (@soroush5) proposed this exact fix but was closed without merging on 2026-09-12; this re-implements it fresh against the current tip using the same guard shape and error message. Originally-proposed-by: @soroush5 in #12644 Co-authored-by: soroush5 --- .../fixes/12643-messages-entry-guard.md | 1 + config/quality/file-size-baseline.json | 3 +- src/sse/handlers/chat.ts | 20 +++++- .../chat-messages-entry-objects-12643.test.ts | 69 +++++++++++++++++++ 4 files changed, 91 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/12643-messages-entry-guard.md create mode 100644 tests/unit/chat-messages-entry-objects-12643.test.ts 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 000000000000..a7412f9380eb --- /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 fef8ea6cfee7..9e8e3aabac3f 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 2462->2472 (+10). 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).", "_rebaseline_2026_09_14_13547_noauth_model_lockout": "PR #13547 own growth: src/sse/services/auth.ts 3542->3556 (+14). The synthetic noauth connection short-circuited before the per-connection status pass, so a recorded model-only lockout was never enforced and the locked model was retried on every request. Irreducible at the early-return site. Covered by tests/unit/noauth-model-lockout.test.ts (carried from #13527).", "_rebaseline_2026_09_14_13404_healthcheck_backup_prune": "PR #13404 own growth: src/lib/db/core.ts 1745->1767 (+22). createManagedDbBackup (the health-check-repair snapshot path) never ran retention, so every restart of a healthy DB added a full-size copy to db_backups/; it now prunes with the same env-driven limits as backup.ts, importing backupRetention directly to avoid the backup.ts cycle. Covered by tests/unit/db-backup-healthcheck-prune-13308.test.ts.", "_rebaseline_2026_09_14_13349_virtualfactory_custom_models_guard": "PR #13349 own growth: open-sse/services/autoCombo/virtualFactory.ts 1219->1230 (+11). The customModels key_value blob is operator-writable raw JSON, so a null or non-object row null-derefed every read and no auto/* pool could materialize; the builder now filters rows the same way catalog.ts already does. Irreducible at the read site. Covered by tests/unit/combo-auto-pool-visible-only.test.ts.", @@ -477,7 +478,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": 2462, + "src/sse/handlers/chat.ts": 2472, "src/sse/services/auth.ts": 3556, "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 d3ff15d4444c..8f4961395ca2 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -476,6 +476,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"); @@ -1014,7 +1024,15 @@ async function handleChatImplementation( if (isComboLiveTest) return true; // #12886: combo-name allow-list must not skip inner targets (#9057 still // checks auto/* / disableNonPublic via comboTargetPassesKeyModelPolicy). - if (!(await comboTargetPassesKeyModelPolicy({ apiKey, apiKeyInfo, requestedModelStr: resolvedModelStr, targetModelStr: modelString, isModelAllowedForKey }))) { + if ( + !(await comboTargetPassesKeyModelPolicy({ + apiKey, + apiKeyInfo, + requestedModelStr: resolvedModelStr, + targetModelStr: modelString, + isModelAllowedForKey, + })) + ) { return false; } 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 000000000000..412bfac5ac0f --- /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))); +}); From 99026039b02648d00c2c5fc3f9c4309289648b8d Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 17 Sep 2026 13:15:18 -0300 Subject: [PATCH 2/2] chore(quality): refix the chat.ts ceiling for the merged tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This branch rebaselined src/sse/handlers/chat.ts against an older tip. After merging the current release tip the combined file is 2520 lines, so the 2500 ceiling no longer covers it. The tip alone is already at 2509 — above the 2500 this PR had frozen — so most of the gap is inherited, not introduced here. This PR's own contribution is the +10 of the messages-entry guard itself. Ceiling refixed at the value the gate reports for the merged tree. --- config/quality/file-size-baseline.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 8d645b023aac..ee72d1544654 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -1,5 +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).", + "_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.", @@ -505,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,