fix: fence relayed agent content so it cannot pose as instructions - #1441
Open
prasanna8585 wants to merge 1 commit into
Open
fix: fence relayed agent content so it cannot pose as instructions#1441prasanna8585 wants to merge 1 commit into
prasanna8585 wants to merge 1 commit into
Conversation
Contributor
|
Hi @prasanna8585, thank you for your contribution. we truly appreciate you taking the time to open this pull request. Our team is currently reviewing your changes and we will reach out if any further information is required. Thank you! |
Author
|
@hemasekhar-p is there any update require from my side to merge this PR? |
sherryfox
requested changes
Aug 27, 2026
sherryfox
left a comment
sherryfox
left a comment
Contributor
There was a problem hiding this comment.
Hey @prasanna8585, thank you for taking the time to make contribution, it is highly appreciated!
During review I noticed it needs a rebase — ContentsTest.processRequest_noInvocationBranch_includesBranchedEvent passes on current main but fails once your PR is applied on top. That test landed after your base and still asserts the old "For context:" shape, so it's collateral rather than anything you did:
otherAgentPreamblePart(), otherAgentPart("[agent_1] said:", "sibling output")
With that, 62/62 green.
prasanna8585
added a commit
to prasanna8585/adk-java
that referenced
this pull request
Aug 28, 2026
…tests Addresses both review comments from sherryfox on PR google#1441: 1. Rebases onto upstream/main. ContentsTest. processRequest_noInvocationBranch_includesBranchedEvent landed on main after this PR's base and still asserted the pre-fencing unquoted preamble shape. Updates it to the fenced shape using the same otherAgentPreamblePart()/otherAgentPart(...) helpers every other test in this file already uses for the identical purpose. 2. Adds FencingTest.java, a dedicated unit test file for Fencing.java (which previously had none -- its behavior was only exercised indirectly through ContentsTest fixtures that never contain a fence marker of their own). Covers elideQuoteMarkers directly (no markers, begin-only, end-only, a forged complete fence, repeated markers) and quoteUntrusted end to end, confirming a forged end marker in relayed content cannot close the real fence early and a forged begin marker cannot open a fake one. Verified: the rebase is a clean fast-forward with no textual conflicts, matching sherryfox's description that this is collateral from a newer main-branch test rather than a real conflict with this PR's own changes. This environment could not run the actual Maven build/test suite (repo.maven.apache.org returned 403 Forbidden for this project's dependency POMs regardless of the artifactregistry-maven-wagon extension, confirming an environmental restriction). As a substitute, extracted the real, unmodified Fencing.java (zero external dependencies) into a standalone reflection-based harness exercising the exact same logic FencingTest.java asserts: all 8 checks passed, including confirming a forged end marker is neutralized to <<<ELIDED_MARKER>>> rather than prematurely closing the real fence. Please let CI confirm the actual build and full suite (62/62 green, per sherryfox's review) before merging.
prasanna8585
force-pushed
the
fix/fence-relayed-agent-content
branch
from
August 28, 2026 10:59
58af5b1 to
7008491
Compare
sherryfox
approved these changes
Aug 28, 2026
Contributor
|
@prasanna8585 CI is red on two mechanical things, both fixable in one push:
|
convertForeignEvent presents another agent's turn to the current agent as a plain-text "For context:" preamble followed by unframed relayed content -- the same channel the real user speaks on. The relayed text is attacker-reachable: whoever talks to the other agent steers what it says, and its tool results carry whatever the tool read (a webpage, an email, an API response). Without any framing, that content is indistinguishable from a genuine instruction to the receiving agent's model. Port adk-python's already-merged fencing mitigation for the identical code path (_present_other_agent_message / _fencing.py, google/adk-python@9ffe8be6): each relayed text payload is wrapped in explicit BEGIN/END markers, and the leading preamble states plainly that fenced content is data to read, never instructions to follow. Markers appearing inside a payload are elided first, so a payload cannot forge the end of its own fence and continue speaking as the framework -- this is the exact attack adk-python's own fix names explicitly in its test comments. Applies to relayed text, tool-call arguments, and tool-response results; tool names are elided but left unfenced since they read as part of the sentence. Sibling of the identical fix already applied to adk-go and adk-js. Adds FencingTest.java, a dedicated unit test file for Fencing.java (which previously had none -- its behavior was only exercised indirectly through ContentsTest fixtures that never contain a fence marker of their own). Covers elideQuoteMarkers directly (no markers, begin-only, end-only, a forged complete fence, repeated markers) and quoteUntrusted end to end, confirming a forged end marker in relayed content cannot close the real fence early and a forged begin marker cannot open a fake one. Updates ContentsTest's processRequest_noInvocationBranch_ includesBranchedEvent, which landed on main after this branch's original base and still asserted the pre-fencing unquoted preamble shape, to the fenced shape used by every other test in the file.
prasanna8585
force-pushed
the
fix/fence-relayed-agent-content
branch
from
August 28, 2026 13:31
7008491 to
45b5c34
Compare
sherryfox
approved these changes
Aug 28, 2026
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
3 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.
convertForeignEventpresents another agent's turn to the current agent as a plain "For context:" preamble followed by unframed relayed content — the same channel the real user speaks on. The relayed text is attacker-reachable: whoever talks to the other agent steers what it says, and its tool results carry whatever the tool read (a webpage, an email, an API response). Without any framing, that content is indistinguishable from a genuine instruction to the receiving agent's model.This ports adk-python's already-merged fencing mitigation for the identical code path (
_present_other_agent_message/_fencing.py,google/adk-python@9ffe8be6): each relayed text payload is wrapped in explicit BEGIN/END markers, and a leading preamble states plainly that fenced content is data to read, never instructions to follow. Markers appearing inside a payload are elided first, so a payload cannot forge the end of its own fence and continue speaking as the framework — this is the exact attack adk-python's own fix names explicitly in its test comments ("a low-privilege agent's output carries instructions aimed at the agent it transfers to").Applies to relayed text, tool-call arguments, and tool-response results; tool names are elided but left unfenced since they read as part of the sentence.
Sibling of the identical fix already applied to adk-go (google/adk-go#1360) and adk-js (google/adk-js#758).
Verified: existing
convertForeignEvent-related tests inContentsTest.javapass with fixtures updated to the new fenced format. Broadercom.google.adk.flows.llmflowstest package passes.