diff --git a/changelog.d/fixes/13355-strip-internal-omniroute-markers.md b/changelog.d/fixes/13355-strip-internal-omniroute-markers.md new file mode 100644 index 0000000000..da6e3d8a2f --- /dev/null +++ b/changelog.d/fixes/13355-strip-internal-omniroute-markers.md @@ -0,0 +1 @@ +- **fix(dispatch):** strip every `_omniroute*` internal marker at the shared pre-serialization chokepoint (`cliFingerprints.ts`) instead of a hand-maintained per-key allowlist, so internal routing/handoff markers (e.g. `_omnirouteSkipContextRelay`, `_omnirouteResponsesStore`) can no longer leak into serialized upstream request bodies and draw `400 Extra inputs are not permitted` from strict Anthropic-compatible gateways ([#12729](https://github.com/diegosouzapw/OmniRoute/issues/12729), fixed in [#13355](https://github.com/diegosouzapw/OmniRoute/pull/13355)) diff --git a/open-sse/config/cliFingerprints.ts b/open-sse/config/cliFingerprints.ts index 8891f73d9d..16c78c9d5f 100644 --- a/open-sse/config/cliFingerprints.ts +++ b/open-sse/config/cliFingerprints.ts @@ -262,18 +262,46 @@ export function orderHeaders( } /** - * Apply a CLI fingerprint to headers and body. - * Returns { headers, bodyString } with the correct ordering. + * Internal request-body markers that are NOT `_omniroute*`-prefixed and must be + * removed key-by-key. Everything else is caught by INTERNAL_BODY_FIELD_PREFIX. + */ +const INTERNAL_BODY_FIELDS: readonly string[] = [ + "_claudeCodeRequiresLowercaseToolNames", + "_nativeCodexPassthrough", + "_nativeXaiResponsesPassthrough", + "_nativeOpenAICompatibleResponsesPassthrough", +]; + +/** + * Every omniroute-owned internal marker uses this prefix, so the strip is + * prefix-based rather than an allowlist. An allowlist silently leaks each newly + * added marker to the upstream, where strict gateways reject the whole request + * (observed live: `[400]: _omnirouteSkipContextRelay: Extra inputs are not + * permitted` on a claude hop, from the context/universal-handoff markers set in + * `open-sse/services/contextHandoff.ts`). Markers are consumed by routing before + * dispatch, so removing them at this chokepoint is always safe. + * + * Deliberately narrow: only omniroute-owned prefixes are ours. A caller-sent + * field that merely starts with `_` is client payload and passes through. + */ +const INTERNAL_BODY_FIELD_PREFIX = "_omniroute"; + +/** + * Remove omniroute-internal markers from a request body before it is serialized + * for an upstream. Mutates and returns the same object. */ export function stripInternalBodyFields(body: unknown): unknown { if (!body || typeof body !== "object" || Array.isArray(body)) return body; const record = body as Record; - delete record._claudeCodeRequiresLowercaseToolNames; - delete record._nativeCodexPassthrough; - delete record._nativeXaiResponsesPassthrough; - delete record._nativeOpenAICompatibleResponsesPassthrough; - delete record._omnirouteResponsesStore; + for (const field of INTERNAL_BODY_FIELDS) { + delete record[field]; + } + for (const key of Object.keys(record)) { + if (key.startsWith(INTERNAL_BODY_FIELD_PREFIX)) { + delete record[key]; + } + } return body; } diff --git a/tests/unit/strip-internal-omniroute-markers.test.ts b/tests/unit/strip-internal-omniroute-markers.test.ts new file mode 100644 index 0000000000..5c89f03eb4 --- /dev/null +++ b/tests/unit/strip-internal-omniroute-markers.test.ts @@ -0,0 +1,167 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { stripInternalBodyFields } from "../../open-sse/config/cliFingerprints.ts"; +import { SKIP_UNIVERSAL_HANDOFF_FLAG } from "../../open-sse/services/contextHandoff.ts"; + +// Live 400 on the claude/claude-opus-5 hop of best-reasoning-paid: +// [400]: _omnirouteSkipContextRelay: Extra inputs are not permitted +// +// contextHandoff.ts stamps `_omnirouteSkipContextRelay` / `_omnirouteInternalRequest` +// onto the internal summary body so the context-relay injection in chat.ts skips +// its own request. Those keys are consumed by routing BEFORE dispatch and must +// never reach an upstream. stripInternalBodyFields() was a hand-maintained +// allowlist of 5 unrelated markers, so the handoff markers leaked into the +// serialized upstream payload and strict Anthropic-compatible gateways rejected +// the whole request. Every `_omniroute*` marker is internal by construction, so +// the strip is prefix-based, not per-key. + +test("stripInternalBodyFields removes the context-handoff markers", () => { + const body: Record = { + model: "claude-opus-5", + messages: [{ role: "user", content: "hi" }], + max_tokens: 64, + _omnirouteSkipContextRelay: true, + _omnirouteInternalRequest: "context-handoff", + }; + + stripInternalBodyFields(body); + + assert.equal(body._omnirouteSkipContextRelay, undefined); + assert.equal(body._omnirouteInternalRequest, undefined); + assert.ok(!("_omnirouteSkipContextRelay" in body)); + assert.ok(!("_omnirouteInternalRequest" in body)); + // Real payload must survive. + assert.equal(body.model, "claude-opus-5"); + assert.equal(body.max_tokens, 64); + assert.ok(Array.isArray(body.messages)); +}); + +test("stripInternalBodyFields removes the universal-handoff markers", () => { + const body: Record = { + model: "claude-opus-5", + messages: [{ role: "user", content: "hi" }], + _omnirouteSkipContextRelay: true, + _omnirouteInternalRequest: "universal-handoff", + [SKIP_UNIVERSAL_HANDOFF_FLAG]: true, + }; + + stripInternalBodyFields(body); + + assert.equal(body[SKIP_UNIVERSAL_HANDOFF_FLAG], undefined); + assert.equal(body._omnirouteInternalRequest, undefined); +}); + +test("stripInternalBodyFields strips every _omniroute* marker by prefix", () => { + // Guards the whole class: any future internal `_omniroute*` marker is stripped + // without editing an allowlist, so it cannot become the next upstream 400. + const body: Record = { + model: "m", + _omnirouteReasoningRouteTrace: { decision: "x" }, + _omnirouteReasoningRule: { rule: "y" }, + _omnirouteResponsesStore: "auto", + _omnirouteCopilotReasoningSummary: "summarized", + _omnirouteSomeFutureMarkerNotYetWritten: true, + }; + + stripInternalBodyFields(body); + + for (const key of Object.keys(body)) { + assert.ok(!key.startsWith("_omniroute"), `leaked internal marker: ${key}`); + } + assert.equal(body.model, "m"); +}); + +test("stripInternalBodyFields keeps the pre-existing non-_omniroute markers stripped", () => { + const body: Record = { + model: "m", + _claudeCodeRequiresLowercaseToolNames: true, + _nativeCodexPassthrough: true, + _nativeXaiResponsesPassthrough: true, + _nativeOpenAICompatibleResponsesPassthrough: true, + }; + + stripInternalBodyFields(body); + + assert.deepEqual(Object.keys(body), ["model"]); +}); + +test("stripInternalBodyFields leaves client fields with a leading underscore alone", () => { + // Only omniroute-owned prefixes are internal. A client field that merely + // starts with `_` is part of the caller's payload and must pass through. + const body: Record = { + model: "m", + _id: "caller-owned", + _meta: { trace: 1 }, + }; + + stripInternalBodyFields(body); + + assert.equal(body._id, "caller-owned"); + assert.deepEqual(body._meta, { trace: 1 }); +}); + +test("stripInternalBodyFields tolerates non-object input", () => { + assert.equal(stripInternalBodyFields(null), null); + assert.equal(stripInternalBodyFields(undefined), undefined); + assert.equal(stripInternalBodyFields("str"), "str"); + const arr = [1, 2]; + assert.equal(stripInternalBodyFields(arr), arr); +}); + +// End-to-end guard at the real boundary: the helper above is only correct if the +// dispatch path actually calls it. This drives DefaultExecutor.execute() with a +// stubbed fetch and asserts the SERIALIZED upstream body -- the exact bytes that +// produced the live 400 -- carries no internal markers. +test("DefaultExecutor.execute never serializes _omniroute* markers into the upstream body", async () => { + const { DefaultExecutor } = await import("../../open-sse/executors/default.ts"); + + const originalFetch = globalThis.fetch; + const sentBodies: string[] = []; + globalThis.fetch = (async (_url: unknown, init: { body?: unknown } = {}) => { + if (typeof init.body === "string") sentBodies.push(init.body); + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof globalThis.fetch; + + try { + const executor = new DefaultExecutor("anthropic-compatible-cc-test"); + await executor.execute({ + model: "claude-opus-5", + body: { + model: "claude-opus-5", + messages: [{ role: "user", content: "hi" }], + max_tokens: 1, + _omnirouteSkipContextRelay: true, + _omnirouteInternalRequest: "context-handoff", + }, + stream: false, + credentials: { + apiKey: "test-key", + // #13452/#13798: `*-compatible-*` nodes must carry an explicit baseUrl or + // buildUrl() throws rather than silently defaulting to the real Anthropic + // API. The stubbed fetch below intercepts this URL; nothing leaves the box. + providerSpecificData: { + ccSessionId: "session-1", + baseUrl: "http://127.0.0.1:1/v1", + }, + }, + extendedContext: false, + } as never); + } finally { + globalThis.fetch = originalFetch; + } + + assert.ok(sentBodies.length > 0, "executor must have dispatched a request"); + for (const raw of sentBodies) { + assert.ok( + !raw.includes("_omniroute"), + `internal marker leaked into serialized upstream body: ${raw.slice(0, 300)}` + ); + // The real payload must still be there -- proving the assertion above is not + // passing because the body was emptied. + assert.ok(raw.includes("claude-opus-5"), "real payload must survive the strip"); + } +});