Repository navigation
fix(security): scan both ends of an oversized body, not just the front - #13104
Merged
diegosouzapw merged 3 commits intoSep 10, 2026
Merged
diegosouzapw merged 3 commits into
diegosouzapw merged 3 commits into
Conversation
detectInjection() caps the scan at the first 16 KB. The budget is deliberate (hot-path perf, cheaper-inference#3932 / cheaper-inference#4041) and this keeps it exactly. Which bytes it spends is the problem. extractMessageContents() appends `system`, `input`, `prompt`, `instructions`, `query` and `documents` after the message list. So one long message pushes all six past the cap, along with the newest turns. Measured on release/v3.8.51 with ~30 KB of ordinary conversation -- a small body for a coding agent -- and one payload moved between carriers: injection in system -> 0 detections injection in instructions -> 0 injection in query -> 0 injection in documents -> 0 injection in the newest turn -> 0 The same payload in a short body scores 2. The comment beside the slice says "injection directives sit near the top", but the extractor itself puts the highest-signal carriers at the bottom. Spend half the budget at each end. The tail is where content that has never been scanned before lives: those six carriers, and the turn just added -- an injection in an older turn was already the newest turn on an earlier request. A separator that breaks \s+ goes between the halves so no pattern can match across a boundary the two never shared, and it is charged to the budget so the loop still sees at most MAX_INJECTION_SCAN_BYTES characters. evaluatePromptInjection() re-derived the same prefix slice for custom patterns; it now shares the helper, so custom and built-in patterns cannot disagree about what was scanned.
The first version placed its two phrases at fixed offsets, so after the budget was split they both landed inside the head half and the test passed against a bare-newline separator too -- it proved nothing. Calibrate the head, gap and tail lengths from buildInjectionScanText itself, put the phrases flush against the seam, and keep the space that gives \b(ignore| its word boundary. Replacing the separator with a newline now fails this test.
diegosouzapw
merged commit Sep 10, 2026
751247a
into
cheaper-inference:release/v3.8.51
9 of 16 checks passed
diegosouzapw
added a commit
that referenced
this pull request
Sep 11, 2026
Four base-reds that appeared on the tip today, none owned by any open PR: - providers-constants-split / provider-node-reserved-prefix: GreenPT (#13024) and EURouter (#13025) move the totals 238 -> 240 and 408 -> 410. - provider-translate-path-golden: same two providers, regenerated with UPDATE_GOLDEN=1; the snapshot delta is purely additive. - glm-sse-transform-arity: #12925 declared the buffer-size slot GLM had been filling since #12179. The invariant is unchanged — the value must land in a real parameter, never a dropped positional — so the guard now checks it from the helper side and requires the named constant at the call site instead of a magic literal. - injection-guard-scan-bound-3932: #13104 split the 16 KB budget between head and tail, so 'outside the window' is now the middle of an oversized body, not its end. The 16 KB ceiling these cases protect is untouched. Co-authored-by: diegosouzapw <[email protected]>
7 tasks done
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
cheaper-inference#13104) Boarded with 13 sibling PRs into one worktree off release/v3.8.51 and validated as a set: 132 focused tests pass across all 15 test files in the batch, typecheck:core is clean, check-changelog-integrity reports no lost base bullets, and check-file-size is green. Your PR merged without conflict against its siblings. Thank you — the write-up made this reviewable: measuring the behaviour on the release tip and showing the before/after table meant the defect could be confirmed rather than taken on faith.
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.
The bug
detectInjection()caps the scan at the first 16 KB. The budget is deliberate (hot-path perf, #3932 / #4041) and this PR keeps it exactly. Which bytes it spends is the problem.extractMessageContents()appendssystem,input,prompt,instructions,queryanddocumentsafter the message list. So a single long message pushes all six past the cap — along with the newest turns.The comment beside the slice states the assumption:
The extractor puts the highest-signal carriers at the bottom.
Measured on
release/v3.8.51~30 KB of ordinary conversation — a small body for a coding agent — with one payload moved between carriers. Same payload scores 2 in a short body.
systeminstructionsquerydocumentsWith
INPUT_SANITIZER_MODE=block, the same sentence is a 400 in a fresh chat and a pass-through once the conversation passes 16 KB.The change
Spend half the budget at each end.
The tail is where content that has never been scanned before lives: those six carriers, and the turn that was just added. An injection in an older turn was the newest turn on an earlier request, and was scanned then. The head half keeps the front-of-conversation coverage the current slice provides.
Two details:
\s+sits between the halves. EveryINJECTION_PATTERNjoins its words with\s+, so a bare newline would let…ignore all previousat the end of the head andinstructions…at the start of the tail match as one phrase they never formed.MAX_INJECTION_SCAN_BYTEScharacters. No extra regex work, no extra bytes.evaluatePromptInjection()re-derived the same prefix slice for custom patterns; it now shares the helper, so custom and built-in patterns cannot disagree about what was scanned.This is a trade, and worth saying plainly: the second 8 KB of the oldest messages is no longer scanned. That region is conversation that earlier requests already scanned; the end of the body is not.
Tests
tests/unit/guardrails/injection-scan-window.test.ts, 12 tests, driven throughsanitizeRequestandevaluatePromptInjectionas well as the helper. The first test asserts the filler is both past the cap and clean, so no case below can pass for the wrong reason.Three mutations:
The seam test calibrates head/gap/tail lengths from
buildInjectionScanTextitself — the first version used fixed offsets, both phrases landed inside the head half, and it survived the newline mutation. It is in its own commit so that is visible.Existing suites —
guardrails/injection-extraction,guardrails/injection-route-coverage,env-docs-input-sanitizer-8093,chatcore-sanitization,guardrails-registry,guardrails-api-3496,guardrails-void-no-change-contract,injection-guard-nonchat-route-logging: 54 passed.eslint --suppressions-location config/quality/eslint-suppressions.jsonclean.Independent of #13101, which fixes a carrier the extractor never collected at all; these touch different parts of the file and either can land first.