fix(a2a): reject tool confirmations arriving over A2A (HITL confused deputy)#6462
Open
cybrdude wants to merge 6 commits into
Open
fix(a2a): reject tool confirmations arriving over A2A (HITL confused deputy)#6462cybrdude wants to merge 6 commits into
cybrdude wants to merge 6 commits into
Conversation
…deputy)
A human-in-the-loop tool confirmation (ToolContext.request_confirmation -> ToolConfirmation.confirmed) is enforced only by checking the last user-authored event's function_response for {confirmed: true}. Inbound A2A messages are converted to author='user' turns (request_converter.py), so a remote A2A peer can supply the approval and self-approve a pending dangerous tool call (e.g. ExecuteBashTool) - a confused deputy across the A2A trust boundary. Refuse to honor confirmations on A2A-originated invocations (detected via run_config.custom_metadata['a2a_metadata']); approval must come from the operator's own (non-A2A) channel. Note: this does not remove the need to authenticate the ADK API server (/run) in production; HITL confirmation is not a substitute for network auth.
Ensure invocation is marked as A2A-originated regardless of peer metadata.
Checking truthiness of custom_metadata meant an A2A peer that sent no protocol metadata produced an empty dict and skipped the guard. Check for the key itself instead.
This test asserted the behavior that caused the bypass: that empty peer metadata omits the a2a_metadata marker. Invert it, since the marker is a trust signal a peer must not be able to suppress.
Adds regression coverage for google#6461, including the no-metadata case that bypassed the guard. Non-A2A behavior stays covered by the existing success test, which uses the default RunConfig.
Author
|
@xuanyang15 fixes applied per @surajksharma07 |
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.
Closes #6461
Summary
A dangerous
FunctionTool(e.g. the shippedExecuteBashTool) is gated by a human-in-the-loop confirmation:ToolContext.request_confirmation()emits a syntheticadk_request_confirmationcall, and the tool only runs once afunction_responsefor it with{confirmed: true}appears. The only check on that approval is that it comes from an event whoseauthor == "user"(flows/llm_flows/request_confirmation.py).Inbound A2A messages are converted to
Content(role='user', ...)(a2a/converters/request_converter.py), and afunction_responseDataPart round-trips to a genaifunction_responsepart (part_converter.py). So a remote A2A peer can supply the{confirmed: true}approval and self-approve a pending dangerous tool call - a confused deputy across the A2A trust boundary. The confirmation is meant to require the human operator; a peer is not the operator.Change
Refuse to honor
adk_request_confirmationapprovals on an A2A-originated invocation, detected viarun_config.custom_metadata['a2a_metadata'](the marker set by the A2A request converter). Approvals must come from the operator's own (non-A2A) channel. Additive: it does not affect the local CLI / Dev UI path (noa2a_metadata), nor legitimate A2A tool use - only a peer's ability to satisfy a human confirmation.Verified
Against
google-adkwith a mock-model harness: with the A2A marker on the approval turn the pendingExecuteBashToolis not executed; without it (human-operator turn) it executes as before.Scope / non-goals
This closes the A2A-peer vector. It does not address an unauthenticated caller of the ADK API server:
/runand/run_ssehave no auth by default, so a caller who can reach them can already both trigger the tool and post the approval. That is a deployment concern - the API server must be authenticated in production; the human-in-the-loop confirmation is not a substitute for network authentication. Happy to add a docs/security note and unit tests in review.Reported to Google (b/538096101); the team asked that it be disclosed publicly on GitHub.