fix(gateway): neutralise untrusted reply text in the reply prefix - #87229
Closed
iainlane wants to merge 1 commit into
Closed
fix(gateway): neutralise untrusted reply text in the reply prefix#87229iainlane wants to merge 1 commit into
iainlane wants to merge 1 commit into
Conversation
_prepare_inbound_message_text builds the [Replying to: "..."] pointer by interpolating event.reply_to_text[:500] raw into the per-turn message. The quoted text is another participant's content on every platform that populates reply_to_text, and the prefix is prepended verbatim to the turn the model reads, so an embedded newline let a crafted quote break out of the bracketed line and pose as a fresh markdown section (a fake "## SYSTEM" heading) or forge framing of its own. Run the snippet through neutralize_untrusted_inline_text() before building the prefix, the same treatment NousResearch#5961/dbad6d47 gave other attacker-controllable prompt metadata. The helper collapses newlines and control characters to spaces, keeping a well-behaved quote byte-identical while making a hostile one visually inert. The existing 500-char cap is the intended bound, so the helper's own truncation is disabled (max_chars=0). Because the fix sits in the shared gateway path, it covers every platform that supplies reply context. Adds regression tests: framing in the quoted text cannot escape the prefix (for both the own-message and other-message variants), the snippet is truncated to 500 chars, and no prefix is emitted without reply text.
alt-glitch
added
type/security
Collaborator
Duplicate of #65184: both patch the same reply-prefix interpolation sink with the same inline-text neutralization mechanism. |
Author
|
Closing as a duplicate of #65184, which predates this PR by a month and applies the same neutralisation at the same call site; the triage flag is correct and I had missed it when searching. One small delta that may be worth folding into #65184: my version also parametrises the breakout test over the reply_to_is_own_message branch, which formats through a second f-string at the same sink. Happy to see #65184 land. |
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
2 participants
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.
What does this PR do?
_prepare_inbound_message_textbuilds the[Replying to: "..."]pointer byinterpolating
event.reply_to_text[:500]raw into the per-turn message. Thequoted text is another participant's content on every platform that populates
reply_to_text, and the prefix is prepended verbatim to the turn the modelreads. An embedded newline therefore let a crafted quote break out of the
bracketed line and pose as a fresh markdown section (a fake
## SYSTEMheading) or forge framing of its own.
This PR runs the snippet through
neutralize_untrusted_inline_text()beforebuilding the prefix, the same treatment dbad6d4 (#5961) gave other
attacker-controllable prompt metadata. The helper collapses newlines and
control characters to spaces, so a well-behaved quote is unchanged while a
hostile one becomes visually inert. The existing 500-char cap is the intended
bound, so the helper's own truncation is disabled (
max_chars=0). Because thefix sits in the shared gateway path, it covers every platform that supplies
reply context.
One behaviour change to be aware of: a multi-line quote now renders as a
single space-joined line in the prefix. That is the helper's established
semantics.
Extracted from #51803, where review flagged this surface; the platform-wide
chokepoint fix stands alone, and #51803/#62088 build on it.
Related Issue
No existing issue; the surface was identified during review of #51803.
Type of Change
Changes Made
gateway/run.py: neutralise the reply snippet before interpolating it intothe
[Replying to ...]prefix (one call site, both the own-message andother-message variants).
tests/gateway/test_reply_to_injection.py: regression tests, writtenfailing-first: framing in the quoted text cannot escape the prefix
(parametrised over both variants), the snippet is truncated to 500 chars,
and no prefix is emitted without reply text.
How to Test
pytest tests/gateway/test_reply_to_injection.py -q(7 tests).main: revert thegateway/run.pyhunk and re-run; thetwo
test_framing_in_reply_text_cannot_break_out_of_the_prefixparametrisations fail, with the injected
## SYSTEMheading landing atline start in the composed message.
_prepare_inbound_message_text: 280 passed.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests pass (targeted suites runlocally: 7/7 in the changed test file and 280 passed across the twelve
files covering this code path; opened as draft so CI runs the full suite)
Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A