Fix unsafe A2A peer-supplied transferToAgent metadata - #596
Conversation
createAdkEventFromMetadata() restored the transferToAgent action field directly from A2A metadata supplied by a remote A2A peer, with no filtering. A malicious or compromised remote agent could set this metadata on any Message, Task, TaskStatusUpdateEvent, or TaskArtifactUpdateEvent it sends back, forcing the local orchestrator (llm_agent.ts, which reads functionResponseEvent.actions.transferToAgent) to redirect execution to a different agent within its own configured multi-agent tree. This mirrors the fix already merged in google/adk-python (0ba7d3cba7004c0fcc0a05f1f4cfb6ea78e38f91), which explicitly excludes transfer_to_agent from the set of fields a remote peer may set. Existing test event_converter_utils_test.ts (line ~203) previously asserted the vulnerable behavior as correct; that expectation needs updating alongside this fix.
AmaadMartin
left a comment
AmaadMartin
left a comment
There was a problem hiding this comment.
Verified the vulnerability rather than taking the description for it, and it holds. createAdkEventFromMetadata is reached from all five inbound paths (:121 message, :146/:170/:198 status- and artifact-updates, :243 task), so your "any message/task/status-update/artifact-update" is accurate, and llm_agent.ts:1014-1017 does resolve and run the named agent. The adk-python commit you cite is real and merged (0ba7d3cba700, 2026-07-30), and its scope matches yours. Your description is also careful to scope the impact to the local agent tree, which is correct — getAgentByName will not reach outside it.
Two things beyond the inline notes. The test at core/test/a2a/event_converter_utils_test.ts:203 asserts the behaviour you are removing, so this cannot go green without the update you offered — please include it here; details inline. And CI has not actually run on this PR: only check-changes and cla/google executed, everything else is parked at action_required pending a maintainer approving workflows for a fork-sourced PR. So the failing test is currently invisible rather than absent.
Per review feedback on this PR: the existing test at event_converter_utils_test.ts:203 asserted the old (vulnerable) behavior -- that a peer-supplied adk_transfer_to_agent value gets restored into event.actions.transferToAgent. Since the fix now drops that field, this assertion needs to change for CI to go green. Inverted rather than deleted: now asserts transferToAgent is undefined, so a future change that re-restores it from peer metadata fails this test instead of the test simply going silent. The adk_transfer_to_agent value stays in the fixture at :186, and the outbound toA2AMessage assertion at :92 is untouched -- only the inbound restoration assertion changes.
Per optional review feedback: mirrors the structural approach google/adk-python used for the same fix (0ba7d3cba700, PEER_SETTABLE_ACTION_FIELDS frozenset) instead of the implicit 'escalate survives because it's the one line left standing' shape. Functionally identical today -- escalate and transferToAgent are the only two action fields A2AMetadataKeys exposes, and transferToAgent is already excluded -- but a future action field is now unsafe-by-default: adding it to the candidate object alone does nothing until it's also added to PEER_SETTABLE_ACTION_FIELDS, rather than silently being restored because no one remembered to exclude it.
AmaadMartin
left a comment
AmaadMartin
left a comment
There was a problem hiding this comment.
Both comments addressed, and I re-verified rather than taking the replies for it. The inverted assertion at core/test/a2a/event_converter_utils_test.ts:209 pins the drop, the fixture at :186 and the outbound assertion at :92 are untouched, and a repo-wide search confirms :203 was the only test asserting the old inbound behaviour — nothing else reads transferToAgent off an inbound conversion (a2a_agent_test.ts:400, event_processor_utils_test.ts:103 and metadata_converter_utils_test.ts:81 are all the outbound direction). CI is green on 1ce9847 across all three runners, so the allowlist and the test change are consistent. Approving — the one nit below is a follow-up, not a blocker.
createAdkEventFromMetadata() restored `branch` straight from a remote
A2A peer's own response metadata (adk_branch), unlike `author` which is
always force-set by the caller. getContents() (content_processor_utils.ts)
uses an event's branch to keep sibling sub-agent conversation contexts
isolated from each other (a branch is visible in a given context only if
it is an ancestor of, or equal to, that context's current branch).
A malicious or compromised remote peer delegated a sub-task therefore
had two ways to break that isolation and inject its response into an
unrelated sibling sub-agent's LLM context:
- setting adk_branch to a shared ancestor branch (e.g. the parent
coordinator's branch instead of its own), or
- omitting adk_branch entirely, which the filter treats as "always
visible, in every branch".
This is the same class of bug fixed in #596 for actions.transferToAgent
(peer-controlled metadata able to corrupt local orchestrator state), on
a field that fix's allowlist didn't cover.
Fix: stop restoring `branch` in createAdkEventFromMetadata at all, and
thread it as an explicit parameter through toAdkEvent and its internal
per-event-type helpers instead, mirroring how `author` is already
force-set by the caller rather than trusted from peer metadata. The one
caller, A2ARemoteAgent.runAsyncImpl, now passes its own
InvocationContext.branch.
2 participants
A remote A2A peer could set
adk_transfer_to_agentmetadata on any message/task/status-update/artifact-update it sends back, andcreateAdkEventFromMetadata()incore/src/a2a/event_converter_utils.tsrestored it verbatim into the local event'sactions.transferToAgent, whichllm_agent.tsthen reads to select the next agent to run --letting a malicious or compromised remote peer redirect the local orchestrator's control flow within its own configured multi-agent tree.
This mirrors the fix already merged in
google/adk-python(0ba7d3cba7004c0fcc0a05f1f4cfb6ea78e38f91, "fix: ignore unsafe A2A peer-supplied event actions metadata"), which explicitly documentstransfer_to_agentas unsafe peer-controllable state.Verified with a standalone PoC test against this exact (pre-fix) source, confirming a peer-supplied
adk_transfer_to_agentvalue flows straight into the resulting event'sactions.transferToAgent.Note:
core/test/a2a/event_converter_utils_test.ts(line ~203) currently asserts the old (vulnerable) behavior as correct and will need its expectation updated to match this fix -- happy to include that update here if maintainers confirm the intended direction.