mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 20:32:25 +03:00
* 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 <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 commit is contained in:
committed by
GitHub
parent
d6f720bceb
commit
872376bdc1
1
changelog.d/fixes/12643-messages-entry-guard.md
Normal file
1
changelog.d/fixes/12643-messages-entry-guard.md
Normal file
@@ -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
|
||||
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
|
||||
69
tests/unit/chat-messages-entry-objects-12643.test.ts
Normal file
69
tests/unit/chat-messages-entry-objects-12643.test.ts
Normal file
@@ -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)));
|
||||
});
|
||||
Reference in New Issue
Block a user