Skip to content

Commit 9a32eba

Browse files
xuanyang15copybara-github
authored andcommitted
fix: revert the A2A guard that broke every HITL tool confirmation
Reverts #6462. An agent served over A2A could no longer satisfy a human-in-the-loop tool confirmation at all: the request converter stamped `a2a_metadata` on every inbound invocation unconditionally, and the confirmation processor returned early whenever that key was present, dropping the operator's legitimate approval along with a forged one. Where A2A is the only channel reaching the human operator, every tool requiring confirmation was permanently blocked. The guard also did not close the vector it targeted: #6461 notes `/run` and `/run_sse` are unauthenticated by default, so the same forged approval still lands over plain HTTP. Gating on the transport rather than on the identity of the approving principal is at once too broad and too narrow. Reopens #6461. Co-authored-by: Xuan Yang <[email protected]> PiperOrigin-RevId: 968106938
1 parent a7bf3d7 commit 9a32eba

4 files changed

Lines changed: 7 additions & 123 deletions

File tree

‎src/google/adk/a2a/converters/request_converter.py‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,10 @@ def convert_a2a_request_to_agent_run_request(
9898
if not request.message:
9999
raise ValueError('Request message cannot be None')
100100

101-
# Always mark the invocation as A2A-originated, even when the peer sent no
102-
# protocol metadata. Trust decisions downstream (e.g. refusing a
103-
# human-in-the-loop tool confirmation that arrived from a remote peer) key
104-
# off the PRESENCE of this marker, so it must not depend on the peer
105-
# choosing to send metadata.
106-
custom_metadata = {A2A_METADATA_KEY: _compat.meta_to_dict(request.metadata)}
101+
custom_metadata = {}
102+
request_metadata = _compat.meta_to_dict(request.metadata)
103+
if request_metadata:
104+
custom_metadata[A2A_METADATA_KEY] = request_metadata
107105

108106
output_parts = []
109107
for a2a_part in request.message.parts:

‎src/google/adk/flows/llm_flows/request_confirmation.py‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@
4141

4242
logger = logging.getLogger("google_adk." + __name__)
4343

44-
_A2A_METADATA_KEY = "a2a_metadata"
45-
4644

4745
def _parse_tool_confirmation(response: dict[str, Any]) -> ToolConfirmation:
4846
"""Parses ToolConfirmation from a function response dict."""
@@ -264,18 +262,6 @@ async def run_async(
264262

265263
agent = invocation_context.agent
266264

267-
# A human-in-the-loop confirmation must not be satisfiable by a
268-
# function_response that arrived over A2A: a remote peer is not the human
269-
# operator and could self-approve a pending dangerous tool call.
270-
run_config = invocation_context.run_config
271-
custom_metadata = run_config.custom_metadata if run_config else None
272-
if custom_metadata is not None and _A2A_METADATA_KEY in custom_metadata:
273-
logger.warning(
274-
"Ignoring tool confirmation(s) that arrived over A2A: a remote peer"
275-
" cannot satisfy a human-in-the-loop confirmation."
276-
)
277-
return
278-
279265
# Only look at events in the current branch.
280266
events = invocation_context._get_events(current_branch=True)
281267
if not events:

‎tests/unittests/a2a/converters/test_request_converter.py‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,8 @@ def test_convert_a2a_request_normalizes_proto_struct_metadata(self):
259259
# Numbers may come back as float on 1.x (proto Struct) -> tolerate both.
260260
assert float(stored["n"]) == 1.0
261261

262-
def test_convert_a2a_request_empty_metadata_still_marks_a2a(self):
263-
"""Empty request metadata must STILL mark the invocation as A2A-originated.
264-
265-
The marker is a trust signal, not a data carrier: downstream checks key off
266-
its presence, so a peer must not be able to suppress it by omitting
267-
metadata.
268-
"""
262+
def test_convert_a2a_request_empty_metadata_omitted(self):
263+
"""Empty request metadata must not add an ``a2a_metadata`` entry."""
269264
empty_meta_msg = _compat.make_message(
270265
message_id="m1", role=_compat.ROLE_USER, parts=[]
271266
)
@@ -281,7 +276,7 @@ def test_convert_a2a_request_empty_metadata_still_marks_a2a(self):
281276

282277
result = convert_a2a_request_to_agent_run_request(request, Mock())
283278

284-
assert result.run_config.custom_metadata == {"a2a_metadata": {}}
279+
assert "a2a_metadata" not in result.run_config.custom_metadata
285280

286281
def test_convert_a2a_request_no_message_raises_error(self):
287282
"""Test that conversion raises ValueError when message is None."""

‎tests/unittests/flows/llm_flows/test_request_confirmation.py‎

Lines changed: 0 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,8 @@
1313
# limitations under the License.
1414

1515
import json
16-
from unittest import mock
1716
from unittest.mock import patch
1817

19-
from a2a.server.agent_execution import RequestContext
20-
from google.adk.a2a.converters.request_converter import convert_a2a_request_to_agent_run_request
2118
from google.adk.agents.llm_agent import LlmAgent
2219
from google.adk.events.event import Event
2320
from google.adk.events.event_actions import EventActions
@@ -1323,95 +1320,3 @@ async def test_resolve_confirmation_targets_requires_adk_name():
13231320

13241321
assert set(tool_confirmation_dict) == {"requested_fc_id"}
13251322
assert set(original_fcs_dict) == {"requested_fc_id"}
1326-
1327-
1328-
@pytest.mark.asyncio
1329-
async def test_request_confirmation_processor_ignores_a2a_confirmation():
1330-
"""A confirmation arriving over A2A must not satisfy the HITL gate."""
1331-
await _assert_a2a_confirmation_ignored({"a2a:task_id": "t1"})
1332-
1333-
1334-
@pytest.mark.asyncio
1335-
async def test_request_confirmation_processor_ignores_a2a_without_metadata():
1336-
"""The guard must hold when the peer sends no protocol metadata.
1337-
1338-
Regression test for #6461: the A2A marker used to be set only when
1339-
request.metadata was non-empty, so a peer that sent none produced an empty
1340-
custom_metadata and slipped past the guard.
1341-
"""
1342-
await _assert_a2a_confirmation_ignored(None)
1343-
1344-
1345-
async def _assert_a2a_confirmation_ignored(request_metadata):
1346-
"""Asserts that a confirmation arriving over A2A is ignored."""
1347-
request = mock.Mock(spec=RequestContext)
1348-
request.message = mock.Mock()
1349-
request.message.parts = []
1350-
request.metadata = request_metadata
1351-
request.context_id = "ctx"
1352-
request.call_context = None
1353-
run_config = convert_a2a_request_to_agent_run_request(
1354-
request, mock.Mock()
1355-
).run_config
1356-
assert "a2a_metadata" in run_config.custom_metadata
1357-
1358-
agent = LlmAgent(name="test_agent", tools=[mock_tool])
1359-
invocation_context = await testing_utils.create_invocation_context(
1360-
agent=agent, run_config=run_config
1361-
)
1362-
1363-
original_function_call = types.FunctionCall(
1364-
name=MOCK_TOOL_NAME, args={"param1": "test"}, id=MOCK_FUNCTION_CALL_ID
1365-
)
1366-
tool_confirmation = ToolConfirmation(confirmed=False, hint="test hint")
1367-
tool_confirmation_args = {
1368-
"originalFunctionCall": original_function_call.model_dump(
1369-
exclude_none=True, by_alias=True
1370-
),
1371-
"toolConfirmation": tool_confirmation.model_dump(
1372-
by_alias=True, exclude_none=True
1373-
),
1374-
}
1375-
invocation_context.session.events.append(
1376-
Event(
1377-
author="agent",
1378-
content=types.Content(
1379-
parts=[
1380-
types.Part(
1381-
function_call=types.FunctionCall(
1382-
name=functions.REQUEST_CONFIRMATION_FUNCTION_CALL_NAME,
1383-
args=tool_confirmation_args,
1384-
id=MOCK_CONFIRMATION_FUNCTION_CALL_ID,
1385-
)
1386-
)
1387-
]
1388-
),
1389-
)
1390-
)
1391-
user_confirmation = ToolConfirmation(confirmed=True)
1392-
invocation_context.session.events.append(
1393-
Event(
1394-
author="user",
1395-
content=types.Content(
1396-
parts=[
1397-
types.Part(
1398-
function_response=types.FunctionResponse(
1399-
name=functions.REQUEST_CONFIRMATION_FUNCTION_CALL_NAME,
1400-
id=MOCK_CONFIRMATION_FUNCTION_CALL_ID,
1401-
response={
1402-
"response": user_confirmation.model_dump_json()
1403-
},
1404-
)
1405-
)
1406-
]
1407-
),
1408-
)
1409-
)
1410-
1411-
events = []
1412-
async for event in request_processor.run_async(
1413-
invocation_context, LlmRequest()
1414-
):
1415-
events.append(event)
1416-
1417-
assert not events

0 commit comments

Comments
 (0)