feat(ENG-1339): chunk oversized Tier-3 input instead of truncating by hiskudin · Pull Request #96 · StackOneHQ/defender

hiskudin · 2026-09-24T08:14:17Z

ENG-1339 — Tier-3 oversize input: chunk instead of truncate

Problem

In tier3_only ("Deep Scan"), a tool result whose serialized form exceeded tier3.maxTextLength
was truncated / spread-sampled before review (formatRecordsForTier3). The sampling order
(tier3SpreadOrder) is deterministic, so an attacker can precompute the dropped slot on an
over-budget list (a ~200-row list with text notes easily exceeds the old 10k budget), plant the
injection there, and the reviewer never sees it → allowed: true. Confirmed in adversarial review.
cascade mode uses maxSentence and is unaffected.

Fix — parallel, boundary-aware chunking

runTier3Only now serializes up to a coverage ceiling and reviews the whole thing across chunks
instead of truncating:

Serialize at the ceiling = maxChunks × maxTextLength.
Chunk on whole-line boundaries (chunkTier3Input), each ≤ maxTextLength, with an
~500-char overlap tail so an injection straddling a boundary lands in both chunks. An
over-long single line is word-split so it stays reviewable.
Review in parallel (Promise.all), each verdict validated with the existing
validateTier3Verdict / isTier3Block.
Union-of-blocks: any chunk blocks → overall block; the representative verdict is the blocker
(else the highest-score allow). A chunk whose classify throws/returns garbage is dropped
(per-chunk fail-open, matching the provider-outage policy) and flags coverageDegraded;
all-error → allow (unchanged fail-open). payloadError (serializer threw) still fails closed in
strict mode.
Genuine overflow (whole records dropped, or a string value cut — not a lone unfittable
field/key) routes to a new tier3.onOversize policy:
- skip (default) → allow per the blockHighRisk invariant, coverageDegraded: true.
- block → high risk (blocks in strict mode).
- scan_anyway → review the ceiling's chunks; block if any blocks, else fail closed because the
  overflow went unreviewed.

Config / API

tier3.maxTextLength now also means the per-chunk cap in tier3_only (the reviewer still
never sees more than this per call); unchanged in cascade.
New tier3.maxChunks (default 5) — sets the ceiling and the max parallel calls.
New tier3.onOversize (default skip).
New DefenseResult.tier3ChunkSummary?: { chunks, blocked, blockingChunk?, oversize? }
(present only when the chunk path ran; mirrors tier2Stats presence).
Default ceiling is 50KB (5 × 10k) — 5× the old 10k truncation point; raise maxChunks or
maxTextLength for more coverage at up to N× Tier-3 cost/latency (surfaced via
tier3ChunkSummary.chunks).

Cost / latency

Up to maxChunks parallel provider calls per oversized tool result (vs 1 before).

Tests

Retired the spread-sampling tests whose premise no longer holds (deterministic sampling replaced
by chunking).
Added chunking tests: the evasion repro is now blocked (late-slot injection reviewed →
block), chunk overlap, per-chunk cap, each onOversize branch, per-chunk provider error,
maxChunks/onOversize validation.
Full suite green (427), tsc --noEmit clean, biome check clean.

Notes

Dead-but-kept: tier3SpreadOrder / tier3ItemCap / the reserve pass are now only reached to
detect overflow and to feed scan_anyway; a follow-up can simplify.
Ship as a follow-up release (0.8.4) after 0.8.3.
Per-chunk size is conservative; e3 to tune it (token overhead) and re-check recall-by-position
(should be uniform now — everything sub-ceiling is reviewed, no sampling).

🤖 Generated with Claude Code

Tuning (e3, n=5,940 real payloads) + defaults

maxChunks: 5 — p99 payload needs 4 chunks; adequate.
maxTextLength: 10000 — raising to 18k gives no recall gain and worsens benign over-block; 10k is the tuned value.
Boundary overlap verified — a boundary-straddling injection survives whole in a chunk as often as a mid-chunk one (0.850 parity, identical recall); no split-induced blind spot.
onOversize default = skip — only ~0.59% of real payloads exceed the ~47.5KB ceiling. skip favors availability and fails open on that tail (consumers with a stricter threat model set block/scan_anyway to fail closed). Kept configurable per consumer.

Documented costs (inherent, not bugs)

Chunked union-of-blocks raises benign FPR ~+13pp on >10k payloads vs a single review — the price of full coverage; blockHighRisk consumers feel it on large tool results.
Chunking removes the sampling gap (everything sub-ceiling is reviewed) but recall stays position-biased — the reviewer model under-detects a single injection diluted among many benign records except when it's late. A model limit chunking can't fix.

Adversarial review

Two adversarial passes; 6 real bugs found and fixed (4 critical) — silent-drop signal gaps (mid-string, field-level, benign-summary over-trigger), zero-overlap on single-line chunks, chunk-count overshoot, and chunk-boundary gluing. Regression tests added for each. 433 tests pass.

tier3_only reviewed only a truncated/spread-sampled slice of an over-budget tool result. The sampling order was deterministic, so an attacker could precompute the dropped slot on a long list and hide an injection there, unreviewed → allowed. Replace truncation with parallel, boundary-aware chunking: - serialize up to a coverage ceiling (maxChunks × maxTextLength), - split into ≤maxChunks whole-line chunks with an overlap tail so a boundary-spanning injection lands in both, - review every chunk in parallel, union-of-blocks aggregation (any chunk blocks → block; representative verdict = the blocker), - route genuine overflow (whole records dropped / a string value cut) to a new tier3.onOversize policy: skip (default → allow), block, or scan_anyway (scan the ceiling, fail closed on the unseen overflow). maxTextLength now means the per-chunk cap in tier3_only (per provider call), so the reviewer still never sees more than that at once; add tier3.maxChunks (default 5) and tier3.onOversize (default skip) with validation. New DefenseResult.tier3ChunkSummary telemetry. A lone unfittable field/key no longer counts as oversize — only genuine content overflow does. Retire the spread-sampling tests whose premise no longer holds; add chunking tests (evasion repro fixed, overlap, per-chunk cap, each onOversize branch, per-chunk provider error). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…el overlap, chunk-count contract Adversarial review of PR #96 found three real issues: 1. (critical) Two content-drop sites (a field skipped because its key won't fit, a non-string scalar dropped) didn't flag oversize, so a genuinely over-budget single record fell through the normal path and its dropped (possibly injection-bearing) field was never reviewed while the result looked clean. Fix: derive oversize from the GREEDY pass's truncation (full budget, index order → truncates iff content genuinely exceeds the ceiling, at every drop site), captured before the reserve reset. Removes the fragile per-site budgetOverflow flag. 2. (critical) Unit-level chunk overlap was zero whenever a chunk held one line (common when a line approaches the per-chunk cap), so a token split at a chunk edge could evade every chunk. Fix: character-level overlap — each chunk carries the previous chunk's last ~overlap chars, concatenated with no separator so a hard-split token is reconstructed byte-contiguous. 3. (medium) Actual chunk/call count could exceed maxChunks because the ceiling didn't account for overlap stride. Fix: ceiling = maxChunks × (perChunk − overlap); documented maxChunks as a target (uneven line lengths can add a chunk). Tests: retire the oversized-key "review the rest" test (that path now correctly routes to onOversize); add byte-contiguous split-token overlap, character-overlap mechanism, and chunk-count-bound tests. 429 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…l gaps + chunk gluing Round-2 review of PR #96 found the oversize signal still had silent-drop holes and the byte-contiguous overlap corrupted framing. 1. (critical) A multi-line string truncated mid-value at an exact budget boundary hit the per-line loop's top `used >= limit` guard and dropped the remaining lines (the injection) with ZERO signal — no oversize, no coverage flag, looked like a clean single-chunk review. Fix: remove the top guard so a non-fitting line reaches the flagging branch. 2. (critical) The oversize signal (greedyTruncated) fired on ANY drop, including a benign non-string numeric-array summary line, so a common list-shaped payload with a numeric field skipped Tier-3 review entirely (default onOversize:skip → allow) even when the injection-bearing string fit. Fix: signal oversize only when a STRING leaf or a whole RECORD is dropped/truncated (new budgetOverflow flag), captured from the greedy pass; benign non-string drops still trigger the reserve retry but never force onOversize. Also flag the object/array field/element loop breaks and the field-level drop (a later string field dropped after an earlier field fills the ceiling) — closing the same silent-drop class at field granularity. 3. (medium) The no-separator carry glued one field's value onto the next field's key, corrupting the record-oriented framing and risking false-positive blocks. Fix: put the carry on its own line ("\n" join) and move hard-split-token reconstruction into splitLongLine, which now emits byte-overlapping pieces so a split token is whole inside a piece. Tests: regression tests for each (mid-string drop, field-drop, benign-summary non-skip, no-gluing). 433 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…t (e3 tuning) e3 tuning on 5,940 real payloads confirmed the defaults (maxChunks:5 covers p99=4; maxTextLength:10000 — raising it doesn't help recall and worsens over-block) and two costs worth stating: onOversize:skip fails open on the ~0.6% oversized tail, and chunked union-of-blocks raises benign FPR ~13pp on >10k results. Keep skip as the availability-favoring default; consumers set block/scan_anyway to fail closed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…p no longer discards it) Round-3 review found the oversize signal's precision was load-bearing in a way it can't be: a benign trailing sibling (e.g. a boolean) tripped the loop-break oversize flag, and under the default onOversize:skip the whole payload — including a fully-fitting injection — was discarded unreviewed (allowed:true). Fix the class architecturally instead of chasing signal precision: skip and scan_anyway now ALWAYS review the content that fit (union-of-blocks), so a fitting injection always blocks; only the genuinely-dropped overflow is governed by onOversize (skip allows it, scan_anyway fails closed, block blocks without reviewing). A false-positive oversize is now harmless — the fitting content is reviewed either way. With review no longer gated on it, the oversize signal reverts to the simple reliable greedyTruncated (greedy uses the full budget in index order, so it truncates iff content genuinely exceeds the ceiling — never under-flags, so no dropped content is ever silently unreviewed). Removes the fragile per-site budgetOverflow flag. Cost: skip now makes up to maxChunks provider calls on the ~0.6% oversized tail (was 0), for full review of the part that fit. Tests updated to the reviewed-what-fit semantics; added a round-3 regression (fitting injection + benign oversize-tipping sibling → reviewed and blocked under skip). 434 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Round-4 review disproved the "class is closed" claim with two reproduced bugs at production defaults: F1 (critical, silent bypass): runRecords charges the "\n\n" separator (`used += 2`) AFTER its `used >= maxChars` precheck but BEFORE calling serialize, so `used` could sit 2 below the ceiling, pass the precheck, then trip serialize's top guard (`if (used >= cap) return`) — which returned WITHOUT setting budgetTruncated. A huge first record then silently dropped every later record (e.g. a whole injected list), greedyTruncated stayed false, oversize was never flagged, coverageDegraded unset → allowed:true with zero signal, and the reserve anti-starvation retry never engaged. Fix: (a) serialize's guard now self-reports (sets budgetTruncated + coverageDegraded before returning) — closes the class at the guard against any caller; (b) the record-loop precheck accounts for the pending +2 so it breaks-and-flags instead of oscillating. F2 (high, policy defeated): the `joined.length === 0` branch preempted the onOversize handling, so block/scan_anyway did NOT fail closed when the whole payload's strings were dropped (e.g. a field whose key alone exceeds the ceiling → empty joined but oversize). Fix: handle onOversize:block before the empty-joined case, and fail closed under scan_anyway when empty+oversize. Regression tests for both (huge-first-record list, and empty+oversize block/scan_anyway/skip). 436 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Replace the drop-site-dependent oversize signal (greedyTruncated) with an independent, positive-accounting check that ends the recurring "content silently dropped from review" class: - `formatRecordsForTier3` now counts the STRING-leaf content it actually emits (`emittedStringChars`, excluding key prefixes and separators). - `sumInputStringChars(value)` independently sums the input's string-leaf content, traversing exactly as the serializer does (arrays/objects incl. non-plain, depth-capped, binary/non-string = 0). - oversize (`depthFlag.budgetExceeded`) ⟺ emitted < input, i.e. any string leaf was dropped or truncated — regardless of WHICH drop path lost it. Why this closes the class: it's positive accounting. A miscount can only UNDER-report emitted → over-flag oversize (harmless: under onOversize:skip the fitting content is still reviewed), never over-report → silent miss. It does not depend on every drop site remembering to set a flag, which is what failed in rounds 1-4. Also gives an ACCURATE signal — content the reserve pass rescues counts as emitted, so a greedy drop that reserve refits is no longer a false oversize. The round-4 self-reporting guard + record-loop precheck stay: they make budgetTruncated fire so the reserve retry engages and RESCUES dropped content into review (better than merely flagging it). 436 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…can't defeat the oversize check The definitive check re-derived the input total independently, which assumed idempotent reads. A non-idempotent getter ("malicious on first read, benign after") could show 100KB to the greedy pass and return "ok" to the reserve pass and to sumInputStringChars — so emitted == input, oversize never fired, and the injection was silently allowed under EVERY onOversize policy (including block/scan_anyway). Fix: materialize the payload once into a plain deep snapshot (each getter invoked a single time), then run greedy, reserve, and the input-sum off that snapshot. All passes see identical data, so a getter cannot reveal content to one pass and hide it from another. Getter throws still propagate to payloadError (fail closed), unchanged. Also removes sumStringContent's now- dead getter try/catch (the snapshot is plain) and its misleading comment. Regression: a non-idempotent getter is now flagged oversize (block fails closed; skip allows-but-flags), not silently allowed. 437 pass. Note: materialize adds one O(payload) deep copy per tier3_only call — a follow-up can add a no-getter fast path if the copy cost matters. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… cache, node budget) Round-6 review found the read-once snapshot itself was exploitable: F1 (critical): materializeForTier3 used `value.map()` on the untrusted top-level array — an own `map` property shadowing Array.prototype.map made it return [], producing an empty snapshot: injection never reviewed, allowed:true, and (worse than any prior round) NO signal at all. Fix: iterate the top level by index, like every nested array already does. F2 (high): the `seen` cache was keyed by object identity only, so a shared object first visited deep (its children truncated at the depth cap) cached a truncated copy that a later SHALLOW reference reused — silently omitting content the shallow path would have kept. Fix: key the cache by (object, depth); a shallow reference re-materializes fully. F3 (DoS regression): the snapshot deep-copied the whole payload with no budget (unlike the budget-bounded serializer), so a huge tool result (2M objects → ~3.4s) was an amplification vector. Fix: a node budget (TIER3_MAX_MATERIALIZE_NODES) — exceeding it flags oversize (onOversize governs the un-snapshotted remainder) and stops, bounding the copy. Regression tests for all three. 440 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… definitive check The snapshot (materializeForTier3) and its round-6 hardening (map-shadow fix, depth-keyed cache, node budget) defended hostile in-process JS objects — non-idempotent getters, prototype-method shadowing, reference aliasing, cycles — none of which are expressible in JSON. Tool results arrive as JSON.parse output (plain data), so this was out of the input contract, and it cost a deep copy on every tier3_only call plus ~80 lines of subtle traversal that itself needed a round of fixes. Kept: the definitive positive-accounting oversize check (emitted string chars < input string chars) — the real fix for the realistic "content silently dropped" class, cheap and independent of drop-site enumeration. It now runs directly on `value`; a throwing getter propagates → payloadError → fail closed (unchanged). Removed the 4 snapshot-specific tests; all data-class regressions (rounds 1-4) and the definitive-check tests stay. 436 pass. Follow-up: document that defender assumes JSON-serializable tool data (adversarial live JS objects out of scope). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…s it Scoped adversarial review (JSON-data only) found one real bug: content nested past MAX_TRAVERSAL_DEPTH is dropped by the serializer AND excluded from sumInputStringChars (both stop at the same depth), so it never registers as a drop in the emitted-vs-input check. `oversize` was derived only from budgetExceeded, so a JSON injection buried under ~100 levels of {"a":{"a":…}} was never flagged and block/scan_anyway never failed closed on it (allowed: true under every policy, only a soft coverageDegraded note). Fix: fold depthFlag.hit into the oversize signal — a depth cut is unreviewable content just like a ceiling overflow, so onOversize governs it (skip allows+flags, block/scan_anyway fail closed). Regression test added. 437 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings September 24, 2026 08:14

hiskudin requested a review from a team as a code owner September 24, 2026 08:14

Copilot started reviewing on behalf of hiskudin September 24, 2026 08:14 View session

Copilot AI reviewed Sep 24, 2026

hiskudin and others added 11 commits September 24, 2026 09:33

docs(ENG-1339): trim verbose comments and JSDoc to terse one-liners

aac61a6

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

hiskudin merged commit 7128877 into main Sep 25, 2026
3 checks passed

hiskudin deleted the hiskias/eng-1339-tier3-oversize-chunking branch September 25, 2026 10:18

stackone-devops-service-account mentioned this pull request Sep 25, 2026

chore(main): release defender 0.9.0 #97

Open

Navigation Menu

feat(ENG-1339): chunk oversized Tier-3 input instead of truncating - #96

feat(ENG-1339): chunk oversized Tier-3 input instead of truncating#96
hiskudin merged 12 commits into
mainfrom
hiskias/eng-1339-tier3-oversize-chunking

hiskudin commented Sep 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hiskudin commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ENG-1339 — Tier-3 oversize input: chunk instead of truncate

Problem

Fix — parallel, boundary-aware chunking

Config / API

Cost / latency

Tests

Notes

Tuning (e3, n=5,940 real payloads) + defaults

Documented costs (inherent, not bugs)

Adversarial review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hiskudin commented Sep 24, 2026 •

edited

Loading