Repository navigation
fix(chat): reject null/non-object entries in messages[] (#12643) - #13755
Merged
diegosouzapw merged 5 commits intoSep 17, 2026
Merged
Conversation
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 <mrsoroushahmadi@gmail.com>
…null-non-object-message-entries # Conflicts: # config/quality/file-size-baseline.json
…null-non-object-message-entries # Conflicts: # config/quality/file-size-baseline.json
# Conflicts: # config/quality/file-size-baseline.json
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.
This was referenced Sep 17, 2026
Merged
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…rence#12643) (cheaper-inference#13755) * fix(chat): reject null/non-object entries in messages[] (cheaper-inference#12643) A messages array containing null (or any non-object entry, e.g. [null] or [42]) passed every existing entry guard in chat.ts (cheaper-inference#5110/cheaper-inference#6402/cheaper-inference#6407/cheaper-inference#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 cheaper-inference#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 cheaper-inference#12644 Co-authored-by: soroush5 <mrsoroushahmadi@gmail.com> * chore(quality): refix the chat.ts ceiling for the merged tree 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. --------- Co-authored-by: soroush5 <mrsoroushahmadi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #12643
Root cause (short)
handleChatImplementationinsrc/sse/handlers/chat.tshas a guard family for themessagesfield (built up across #5110/#6402/#6407/#6412) that checks array-ness, non-emptiness, and
presence — but never the shape of the individual entries. The route's Zod schema is
intentionally wide (
z.array(z.unknown()), per #6402), somessages: [null]ormessages: [42]sailed straight through the guard chain into translation. Two confirmed unguarded crash sites:
open-sse/translator/request/openai-to-claude.ts— reads.roledirectly off each entry.open-sse/services/sessionManager.ts(extractSystemPrompt) — same, reached fromgenerateSessionIdearlier in the dispatch path than the translator.open-sse/services/contextManager.ts(fixToolPairs) — same unguarded shape assumption.Any of these turns a malformed client request into an unhandled
TypeError, surfacing as a rawHTTP 500 instead of a clean 400.
Fix
Adds one more entry-shape guard clause to the existing chokepoint in
chat.ts, right after theempty-array check and before the missing-messages check: rejects the request with a 400
(
"messages: Expected array of objects") when any entry isnull, a non-object primitive, or anarray (arrays are
typeof "object"but not valid message objects).Regression test
tests/unit/chat-messages-entry-objects-12643.test.ts(built on the existingtests/integration/_chatPipelineHarness.tsharness).RED (on unfixed
release/v3.8.51tip):GREEN (with the fix):
Gates run
npx eslint --suppressions-location config/quality/eslint-suppressions.json src/sse/handlers/chat.ts tests/unit/chat-messages-entry-objects-12643.test.ts— clean, no new warnings.npm run typecheck:core— clean.node scripts/check/check-file-size.mjs—src/sse/handlers/chat.tsis a frozen file; thisirreducible +10-line guard at the existing chokepoint bumps it 2462→2472 with a matching
rebaseline note in
config/quality/file-size-baseline.json(same pattern as prior small-guardgrowths in this file, e.g. fix(responses-continuation): chain off the effective post-reconstruction input, not the pre-reconstruction client bytes #12641, fix(combo): treat a pin-only step as implicit connection allowlist #12697, feat(providers): add weekly quota tracking for grok-web #8127).
open-sse/utils/stream.tsremains red —pre-existing base drift untouched by this PR (already 3115>3098 on the pure tip per its own
baseline note; base-red 🔴 Release branch not green: release/v3.8.51 #12732, owned by
/sweep-reds).node scripts/check/check-complexity.mjs— OK (2824 violations vs baseline 3218).node scripts/check/check-cognitive-complexity.mjs— OK (1276 violations vs baseline 1437).node scripts/check/check-test-discovery.mjs— OK, new test file discovered.Existing tests aligned
None — no existing assertion encoded the old buggy contract. Ran the sibling guard suites for
regression:
tests/unit/chat-messages-validation-6402.test.ts+tests/unit/chat-non-string-model-6407.test.ts— 22/22 pass, no regression.
Credit
PR #12644 (@soroush5) proposed this exact fix — same guard shape, same error message — but was
closed on 2026-09-12 without merging, so the bug never actually shipped. This PR re-implements it
fresh against the current tip.
Originally-proposed-by/Co-authored-bytrailers added on thecommit.