diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index ba59f9091f..52ec5a86e2 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -258,7 +258,7 @@ "src/shared/services/cliRuntime.ts": 1100, "src/shared/validation/schemas.ts": 2523, "_rebaseline_2026_06_28_5275_correlation_id_extract": "Extraction of the safe CorrelationId subset of #5275 (hartmark) — request correlation id stored in call_logs (migration 109) and returned via the X-Correlation-Id response header, WITHOUT the combo/resilience or build/lazy-loading changes (those stay in #5275). Own growth: callLogs.ts 975->985 (correlation_id column on CallLogSummaryRow + read/map), usageHistory.ts 983->988 (correlationId metadata normalize), chat.ts 1575->1632 (withCorrelationId response wiring + combo-failure log carrying correlationId), chatHelpers.ts new 811 (withCorrelationId helper + reqId threading; was 791951 (+9), EditConnectionModal.tsx ->1277 (+18). Absorbs the pre-existing session base-red on these frozen modals; release captain rebaseline-at-release supersedes.", "_rebaseline_2026_07_06_6499_unique_default_name": "PR #6499 own growth: AddApiKeyModal.tsx 952->959 (+7 = a unique default connection name so a second API key for the same provider does not reuse 'main' and trigger the backend name-based upsert that silently overwrote the first connection). The pure name derivation was extracted to computeConnectionDefaultName.ts (unit-tested) to keep the growth minimal; the contributor's original full-form-reset rewrite was trimmed to a spread reset to avoid dropping the GLM team-quota fields #6351 added and to hold the frozen god-file growth down. Release captain rebaseline-at-release supersedes.", "_rebaseline_2026_07_07_6523_chirag_cooldown_body": "PR #6523 (@chirag127, #6460) own growth: chatHelpers.ts 860->866 (+6 = retryAfterAt/credentialsCoolingCount fields on modelCooldownResponse) and auth.ts 2447->2448 (+1 = connectionsCount threaded through no-credentials fallback). Owner-approved rebaseline (file-size cap for contributor PR). Frozen (cannot grow further); release captain's rebaseline-at-release supersedes.", - "_rebaseline_2026_07_07_6526_chirag_modal_1080p": "PR #6526 (@chirag127, #6265): AddApiKeyModal.tsx ->961 (1080p sizing). Owner-approved. Frozen." + "_rebaseline_2026_07_07_6526_chirag_modal_1080p": "PR #6526 (@chirag127, #6265): AddApiKeyModal.tsx ->961 (1080p sizing). Owner-approved. Frozen.", + "_rebaseline_2026_07_07_6515_chirag": "PR #6515 (@chirag127) own growth: src/sse/handlers/chat.ts ->1763. Owner-approved rebaseline. Frozen." } \ No newline at end of file diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index b2539ad822..6a713e4727 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -240,18 +240,30 @@ export async function handleChat( // reasoning_effort / reasoning / object-shaped thinking always wins (backward compatible). body = normalizeReasoningRequest(body); - // Early guard: an explicitly empty `messages` array is invalid for every - // upstream (Anthropic/OpenAI both reject "at least one message is required"). - // Forwarding it produced a confusing raw upstream 400/502; reject it here with - // a clear OmniRoute-level error before any routing or upstream call (#5110). - // Responses-API requests use `input` (not `messages`) so they are unaffected, - // and an absent `messages` field is left to downstream validation. - if ( - Array.isArray((body as { messages?: unknown }).messages) && - (body as { messages: unknown[] }).messages.length === 0 - ) { - log.warn("CHAT", "Rejecting request with empty messages array"); - return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required"); + // Early guard: an invalid `messages` field is rejected here with a clear + // OmniRoute-level 400 before any routing or upstream call (#5110, #6402). + // Without this guard, schema-invalid bodies fell through to model resolution + // and surfaced as a misleading 404 `model_not_found` from chatHelpers.ts (#6402). + // Cases covered: + // - present-but-non-array (null, number, string, object) → 400 (#6402) + // - empty array → 400 ("at least one message is required") (#5110) + // - missing entirely, when the Responses-API `input` discriminator is also + // absent → 400 (#6402). Responses-API requests use `input` (not `messages`) + // and are still unaffected. + { + const b = body as { messages?: unknown; input?: unknown }; + if ("messages" in b && !Array.isArray(b.messages)) { + log.warn("CHAT", "Rejecting request with non-array messages"); + return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: Expected array"); + } + if (Array.isArray(b.messages) && b.messages.length === 0) { + log.warn("CHAT", "Rejecting request with empty messages array"); + return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required"); + } + if (!("messages" in b) && !("input" in b)) { + log.warn("CHAT", "Rejecting request with missing messages"); + return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: Expected array, received undefined"); + } } // Reject non-string `model` before it reaches downstream code that calls diff --git a/tests/unit/chat-messages-validation-6402.test.ts b/tests/unit/chat-messages-validation-6402.test.ts new file mode 100644 index 0000000000..4b58f5d4ec --- /dev/null +++ b/tests/unit/chat-messages-validation-6402.test.ts @@ -0,0 +1,124 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { createChatPipelineHarness } from "../integration/_chatPipelineHarness.ts"; + +// Regression tests for #6402 — schema-invalid `messages` fields fell through +// the existing empty-array guard and reached model resolution, where an +// unresolvable model surfaced as a misleading 404 `model_not_found` from +// chatHelpers.ts (`No active credentials for provider:

`). +// +// The guard at src/sse/handlers/chat.ts now rejects three additional cases with +// a clear OmniRoute-level 400 before any routing or upstream call: +// - present-but-null messages +// - present-but-non-array messages (number, string, object) +// - missing messages when the Responses-API `input` discriminator is also +// absent. + +const harness = await createChatPipelineHarness("chat-messages-validation-6402"); +const { handleChat, buildRequest, resetStorage, seedConnection } = harness; + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + await harness.cleanup(); +}); + +test("#6402: messages: null is rejected with a clear 400 before model resolution", async () => { + 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: null, + }, + }) + ); + + assert.equal(response.status, 400, "null messages must be a 400, not a 404 model_not_found"); + const body = (await response.json()) as { error?: { message?: string } }; + assert.match(body.error?.message ?? "", /messages.*Expected array/i); + assert.equal(upstreamCalled, false, "must not forward a null-messages request upstream"); +}); + +test("#6402: messages: is rejected with a clear 400 before model resolution", async () => { + 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: 123 as unknown as [], + }, + }) + ); + + assert.equal(response.status, 400, "non-array messages must be a 400, not a 404 model_not_found"); + const body = (await response.json()) as { error?: { message?: string } }; + assert.match(body.error?.message ?? "", /messages.*Expected array/i); + assert.equal(upstreamCalled, false, "must not forward a non-array-messages request upstream"); +}); + +test("#6402: missing messages (and no Responses-API input) is rejected with a clear 400", async () => { + 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", + }, + }) + ); + + assert.equal(response.status, 400, "missing messages must be a 400, not a 404 model_not_found"); + const body = (await response.json()) as { error?: { message?: string } }; + assert.match(body.error?.message ?? "", /messages.*Expected array/i); + assert.equal(upstreamCalled, false, "must not forward a missing-messages request upstream"); +}); + +test("#6402: Responses-API input passes the guard (messages discriminator preserved)", async () => { + await seedConnection("openai", { apiKey: "sk-openai" }); + + const response = await handleChat( + buildRequest({ + url: "http://localhost/v1/responses", + body: { + model: "openai/gpt-4.1", + input: [{ role: "user", content: "Hello" }], + }, + }) + ); + + // The guard must not fire for Responses-API requests; whatever downstream + // status this produces (200, 404, etc.), it MUST NOT be the guard's + // "messages: Expected array" 400. + if (response.status === 400) { + const body = (await response.json()) as { error?: { message?: string } }; + assert.doesNotMatch( + body.error?.message ?? "", + /messages.*Expected array/i, + "Responses-API request must not be caught by the messages guard" + ); + } +});