Skip to content

fix(security): scan the text a tool_result carries - #13101

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/sanitizer-tool-result-carrier
Sep 10, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
ntdatt812:fix/sanitizer-tool-result-carrier

Conversation

@ntdatt812

Copy link
Copy Markdown
Contributor

The bug

extractMessageContents() reads a content part's text. A tool_result block does not have one — it carries its payload on content, as a string or as a nested block list:

// src/lib/providers/xai/translators/claude.ts
if (b?.type === "tool_result") {
  input.push({
    type: "function_call_output",
    call_id: b.tool_use_id,
    output: typeof b.content === "string" ? b.content : JSON.stringify(b.content ?? ""),
  });
}

So nothing a tool handed back to the model was ever scanned. detectInjection() runs on the joined output of that extractor, and the extractor returned an empty list.

Measured on release/v3.8.51

One payload, three carriers, same request otherwise:

carrier extracted detections
{ type: "text", text } the payload 2 (system_override, system_prompt_leak, both high)
{ type: "tool_result", content: "…" } [] 0
{ type: "tool_result", content: [{ type: "text", text }] } [] 0

This is the carrier that matters most for this particular guard. User text is written by the caller; tool output is fetched from outside the conversation, which is where indirect prompt injection arrives from. With INPUT_SANITIZER_MODE=block, the same sentence is a 400 in a user turn and a pass-through in tool output.

The file already believed a part can carry text there

redactBody(), sixty lines down in the same file, rewrites it:

if (typeof next.content === "string") {
  next.content = processPII(next.content, true).text;
}

Only the extractor never looked. And because redaction runs only after detection fires, that branch could not do anything on its own: PII inside a tool result was neither detected nor redacted.

The change

  • collectPartText() — one helper, so the extractor and redactBody() agree on what a part carries. It takes text and content (string, or a nested list of strings/{text} blocks).
  • Used for message content parts and for system blocks — redactBody() already rewrote content on both.
  • redactBody() now also rewrites a nested block list, so detection and redaction reach the same bytes. Without that half, the new detection would report PII, log it, and still forward it.

No pattern changes, no new carriers beyond the one shape, and MAX_INJECTION_SCAN_BYTES still caps the scan.

Tests

tests/unit/guardrails/injection-extraction-tool-result.test.ts, 10 tests, extending the coverage style of injection-extraction.test.ts. They run the real pipeline through sanitizeRequest, not just the extractor, and include a no-duplicate test and a "part with neither field" test so the helper cannot pass by over-collecting.

Three mutations, each killing a different set:

mutation result
extractor stops reading a part's string content 4 fail
extractor stops walking a nested block list 4 fail
redactBody stops rewriting a nested block list 1 fail (the redaction-symmetry test)

Existing suites — guardrails/injection-extraction, guardrails/injection-route-coverage, env-docs-input-sanitizer-8093, chatcore-sanitization, guardrails-registry, injection-guard-nonchat-route-logging: 45 passed. eslint clean; the pre-commit gates (docs-sync, any-budget, tracked-artifacts) all pass.

Relation to earlier work

Issue #8094 closed the redactBody() coverage holes it listed — prompt, instructions, query, documents. The tool_result carrier was not among them, and it is the only one whose bytes originate outside the conversation.

extractMessageContents() reads a content part's `text`. A tool_result block
does not have one: it carries its payload on `content`, as a string or as a
nested block list. The repo's own Claude translator reads exactly that
(`providers/xai/translators/claude.ts`), and redactBody() in this same file
already rewrites the string form -- only the extractor never looked.

So nothing the model was handed back by a tool was ever scanned. Measured on
release/v3.8.51 with one payload in three carriers:

  plain text block      -> 2 detections
  tool_result (string)  -> 0, nothing extracted
  tool_result (blocks)  -> 0, nothing extracted

That is the carrier that matters most for this guard: user text is written by
the caller, while tool output is fetched from outside the conversation, which
is where indirect prompt injection arrives from.

Collect a part's `content` alongside its `text`, in messages and in system
blocks, and let redactBody() reach a nested block list too -- redaction only
runs once detection has fired, so the two have to see the same bytes or PII
would be detected, logged, and forwarded anyway.

Issue diegosouzapw#8094 closed the coverage holes it listed (prompt, instructions, query,
documents); the tool_result carrier was not among them.
@ntdatt812

Copy link
Copy Markdown
Contributor Author

One note on the check:changelog-integrity gate, measured rather than assumed: it is already failing on release/v3.8.51.

[changelog-integrity] 1 invalid changelog fragment(s):
  ✗ changelog.d/fixes/reset-aware-model-family.md: fragment must start with a markdown bullet ("- ")

That file arrived with #12637 (2a6eff0ae, 3 Sep) and starts with prose instead of - . It is not touched by this branch, and I left it alone rather than widen the diff — say the word if you would rather it were fixed here.

The fragment this PR adds (changelog.d/fixes/13101-sanitizer-tool-result-carrier.md) is the only other one on the branch and validates.

@diegosouzapw
diegosouzapw merged commit 567abb5 into diegosouzapw:release/v3.8.51 Sep 10, 2026
9 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
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.
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