Skip to content

fix(egress): strip _omniroute* markers at shared pre-executor boundary - #14252

Merged
diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
alfred-rootson:fix/strip-omniroute-markers-pre-executor
Sep 22, 2026
Merged

diegosouzapw merged 1 commit into
diegosouzapw:release/v3.8.51from
alfred-rootson:fix/strip-omniroute-markers-pre-executor

Conversation

@alfred-rootson

Copy link
Copy Markdown
Contributor

Description

Internal routing control markers (e.g. _omnirouteSkipContextRelay, _omnirouteInternalRequest) are stamped onto internal summarizer request bodies during universal/context handoff.

While applyFingerprint() in cliFingerprints.ts already strips internal fields, executors that serialize their request bodies independently bypass that boundary, causing _omniroute* keys to reach strict-schema upstreams (e.g. Bedrock/Anthropic and OpenAI-compatible gateways) and fail with [400] Extra inputs are not permitted / Unknown parameter.

This change adds stripInternalBodyFields(bodyToSend) inside normalizeAttemptBody() in open-sse/handlers/chatCore/upstreamBody.ts — the shared egress boundary that all request bodies pass through before executor dispatch.

Validation

  • tests/unit/strip-internal-omniroute-markers.test.ts extended with an end-to-end regression test driving maybeGenerateUniversalHandoff with a custom-serializing executor.
  • All 8 tests in the suite pass.
  • Pre-commit hooks, linting, and docs-sync checks passed cleanly.

Ensure internal control markers (e.g. _omnirouteSkipContextRelay,
_omnirouteInternalRequest) injected during universal/context handoff
are stripped at normalizeAttemptBody before reaching any executor.
Prevents leaks on custom executors that serialize request bodies
independently.

Includes cross-layer regression test asserting that universal handoff
dispatches contain no _omniroute* keys.
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks @alfred-rootson — merging via the release merge-train. Validated in local merge-train (mt-train10c) on the devbox @ train tip 4d841aa1c740bbaa03868dc0a403c62099a99a42 with the 72 sibling PRs of this batch: typecheck:core, file-size, complexity, cognitive-complexity, changelog-integrity green; changed-area node:test 831/831 (0 failing) and vitest 480/482 — the two reds are autoCombo/provider-family-combos.test.ts timing out at 20s, which reproduces on the PURE release tip under the full vitest suite (and is already tracked by the Release-Green issue #13866), so it is inherited, not this batch's. Merged --admin per merge-gates §3/§4/§7.

@diegosouzapw
diegosouzapw merged commit 1fb7c9d into diegosouzapw:release/v3.8.51 Sep 22, 2026
3 checks passed
diegosouzapw added a commit that referenced this pull request Sep 22, 2026
…strip

#14252 added stripInternalBodyFields() at the shared pre-executor boundary so
_omniroute* routing markers cannot leak upstream. The same helper also removes
the four _native*Passthrough markers — but those are read INSIDE the executor
(codex.ts:1259, xai.ts:130), which deletes them itself. Stripping them a layer
early turned every Responses-native request into a translated one, which then
lost client fields to the #2608 allowlist; 'metadata' is how it surfaced.

Executor-consumed markers are now a named list the boundary keeps and the
serialization strip (applyFingerprint) still removes, so the leak fix stands.

Bisected to 1fb7c9d on a clean checkout. Also aligns opencode-executor's auth
expectation with #14230, which routed Zen GPT-5.6 through /v1/responses where
#12633's x-api-key rule applies.

Refs #14496.
diegosouzapw added a commit that referenced this pull request Sep 22, 2026
#14252 ("strip _omniroute* markers at shared pre-executor boundary") wired
stripInternalBodyFields() into normalizeAttemptBody(). That helper removes two
classes of marker: the `_omniroute*` prefix class (consumed by routing before
dispatch — the ones #14252 was actually about) AND the four keys in
INTERNAL_BODY_FIELDS, which are consumed by the EXECUTORS inside
transformRequest():

  _nativeCodexPassthrough                      CodexExecutor.transformRequest
  _nativeXaiResponsesPassthrough               XaiExecutor.transformRequest
  _nativeOpenAICompatibleResponsesPassthrough  passthrough dispatch
  _claudeCodeRequiresLowercaseToolNames        BaseExecutor

normalizeAttemptBody() runs BEFORE transformRequest(), so every native Responses
passthrough was silently demoted to the translated path: CodexExecutor no longer
saw its marker, fell through to the RESPONSES_API_ALLOWLIST, and dropped the
client's top-level `metadata` (plus the passthrough instructions handling). No
error — just a quietly different upstream payload. Red on the release tip:
"chatCore keeps Responses-native Codex payloads in native passthrough mode".

Fix: split out stripInternalOmnirouteMarkers() (prefix-only) and call that at
the pre-executor boundary, preserving #14252's intent. The executor-consumed
markers keep being removed at the executor-egress boundary (base.ts, dario.ts,
ninerouter.ts, applyFingerprint), which runs after transformRequest has read
them. stripInternalBodyFields() itself is unchanged in behavior.

Tests: two regressions in strip-internal-omniroute-markers.test.ts — the helper
level (prefix strip keeps the executor markers) and the real boundary
(prepareUpstreamBody preserves _nativeCodexPassthrough and the client metadata
while still dropping _omnirouteSkipContextRelay).
diegosouzapw added a commit that referenced this pull request Sep 24, 2026
Drains the base-red accumulated on release/v3.8.51 (#14496, #14547). Production fixes: auggie spawn typing, projectCombo type imports, Responses passthrough markers surviving the egress strip (#14252 regression), compression effective-pipeline preview following the runtime lossy policy (#14529 regression, #12063), CLIProxyAPI account-health host default (#14544 regression), DEEP_HEALTH_CHECK_ENABLED env contract. The remaining test guards were realigned to merged design changes, each traced to its commit; the subtitle-runtime fixtures moved to a per-run temp dir; documented file-size/stryker rebaselines absorb the 2026-09-23/24 merge-wave drift.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants