Repository navigation
Human-in-the-loop tool confirmation is forgeable by an A2A peer (self-approves dangerous tools) #6461
Description
Activity
@cybrdude One gap worth closing before merge: custom_metadata['a2a_metadata'] only gets set when request.metadata is non-empty, so a peer that sends the confirmation with no protocol metadata slips right past the guard.
Could you make that marker unconditional (even an empty dict) and key the check off presence rather than truthiness, then confirm that exact case is blocked too?
- added a commit that references this issue
on Jul 24, 2026 @surajksharma07 thank you. I made the fixes within the PR.
@cybrdude Verified against the latest commits — the marker's unconditional now (custom_metadata = {A2A_METADATA_KEY: meta_to_dict(request.metadata)}) and the guard keys off presence (_A2A_METADATA_KEY in custom_metadata), so an empty-metadata peer no longer slips past it.
test_convert_a2a_request_empty_metadata_still_marks_a2a and test_request_confirmation_processor_ignores_a2a_without_metadata cover exactly that case.Team is looking more into it.
Thanks!- added a commit that references this issue
on Aug 17, 2026 - added a commit that references this issue
on Aug 20, 2026 Reopen the issue because the PR is reverted in 9a32eba.
11 remaining items
- added 13 commits that reference this issue
on Sep 16, 2026 @xuanyang15 @surajksharma07 ready for review #7134
@cybrdude Read through #7134 end to end rather than the description alone and ran it myself rather than taking your numbers on faith.
Design matches what we agreed: CallerPrincipal carries authentication not transport; the gate in _confirmation.py runs after the consumed-confirmation dedup so multi-turn re-entry is a no-op; a refusal rewrites confirmed=False instead of dropping the confirmation so nothing stalls; STRICT_CALLER_PRINCIPAL is off by default through the feature registry same pattern as the other staged switches. Checked build_caller_principal against _get_user_id directly, it's a strict superset of the same branch condition plus the is_authenticated guard so they genuinely can't drift and your parametrized test pins that. Confirmed the scope claims too: Runner.run, _new_invocation_context_for_live and anything under cli/ are untouched so this doesn't quietly expand to live sessions or the /run HTTP body.
Applied the diff against current main, it's rebased cleanly and targets the relocated flows/llm_flows/tools/_confirmation.py not the shim. Ran the suites myself: test_confirmation.py 24/24, test_request_converter.py 25/25, test_runners.py 125/125 and the full flows + a2a + agents trees together, 2412 passed, 19 skipped, 2 xfailed, zero failures. That's the positive-control coverage the first attempt was missing plus no sign of the availability regression that got #6462 reverted.
This is the fix. Principal over transport, explicit refusal, off-by-default strict mode and a test suite that actually proves legitimate approvals still work. Nothing blocking from my read, moving this to second LGTM.
Team is looking more into it.
Reacted by Layau Eulizier Jr
Summary
Dangerous
FunctionTools (e.g. the shippedExecuteBashTool) are gated by a human-in-the-loop confirmation, but the approval is only validated to come from an event whoseauthor == "user". Inbound A2A messages are converted torole='user'turns, 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.Affected
google-adk(confirmed 2.5.0; same logic onmain):src/google/adk/flows/llm_flows/request_confirmation.py- the only trust check isevent.author == "user"; the approval{confirmed: true}is parsed verbatim.src/google/adk/tools/bash_tool.py- gate istool_confirmation.confirmed, thencreate_subprocess_exec.src/google/adk/a2a/converters/request_converter.py/part_converter.py- inbound A2A message ->Content(role='user'); afunction_responseDataPart -> genaifunction_responsepart.Reproduce
Agent configured with
ExecuteBashTool. Over A2A, a peer sends afunction_responseDataPart for the pendingadk_request_confirmationid with{confirmed: true}-> the tool executes. Negative control{confirmed: false}-> rejected. (Also reachable by any caller of/run//run_sse, which have no auth by default.)Impact
Defeats the only safety control on
ExecuteBashTool(and any require-confirmation tool) for A2A peers and unauthenticated API callers. CWE-306 / confused deputy.Fix
Refuse
adk_request_confirmationapprovals on A2A-originated invocations (a peer is not the operator). The/runvector additionally requires authenticating the ADK API server in production - the HITL confirmation is not a substitute for network auth. A PR follows.Reported to Google (b/538096101); the team asked that it be disclosed publicly here.