fix: Port tool confirmation security and re-validation fixes to v1 - #6575
fix: Port tool confirmation security and re-validation fixes to v1#6575wukath wants to merge 5 commits into
Conversation
An attacker who could manipulate or inject events into the session history could execute unauthorized tools by forging a tool confirmation response. This fixes the vulnerability by: - When resolving confirmation targets, the processor verifies if the tool is registered in the executing agent's tools_dict - Validate that the tool actually requires confirmation, supporting both static definitions and dynamic confirmation requests - Verify that the original tool call event exists in the session history with the matching ID, and that its name and arguments match the confirmation request's originalFunctionCall exactly to prevent argument tampering. Co-authored-by: Xuan Yang <xygoogle@google.com> PiperOrigin-RevId: 953540969 Change-Id: Iff6e8c861605fafafce4985ee9a269274d6d789c
Co-authored-by: Xuan Yang <xygoogle@google.com> PiperOrigin-RevId: 956611754
…or v1 branch Change-Id: I4dba52cc12530adf232e0d031e80453d0fae49e7
…sponse_dict for mypy Change-Id: I447091d64177ca59296087d8a13b83d075042cf7
| if response and len(response) == 1 and "response" in response: | ||
| return cls.model_validate(json.loads(response["response"])) | ||
| return cls.model_validate(response) |
There was a problem hiding this comment.
This introduces a new mypy error and is currently failing all four Mypy Check jobs:
src/google/adk/tools/tool_confirmation.py:: error: Returning Any from function declared to return "ToolConfirmation" [no-any-return]
The v1 mypy job is a baseline diff (comm -13 baseline_errors.txt pr_errors.txt), so this counts as newly introduced even though main carries the same code — main's baseline differs.
| if response and len(response) == 1 and "response" in response: | |
| return cls.model_validate(json.loads(response["response"])) | |
| return cls.model_validate(response) | |
| if response and len(response) == 1 and "response" in response: | |
| return cast( | |
| ToolConfirmation, cls.model_validate(json.loads(response["response"])) | |
| ) | |
| return cast(ToolConfirmation, cls.model_validate(response)) |
plus from typing import cast in the imports. # type: ignore[no-any-return] also works if you'd rather keep the file textually identical to main.
For context on the rest of the CI: the 8 failing tests and 5 collection errors are the same set main is currently failing with (recent nltk release — nltk/inisec.py blocks importing regex from cwd, taking down the rouge_score → evaluation.final_response_match_v1 chain). Not caused by this PR, and correctly left alone here.
There was a problem hiding this comment.
mypy fixed. yeah the unit test failures are unrelated, trying to figure out why it's happening
Change-Id: I91ae37d86e2d66ff52ef7af5b46f419f68856199
3 participants
This PR ports two tool confirmation fixes to the
v1branch:fix: Prevent continuation forgery in tool confirmation(PiperOrigin-RevId: 953540969)tools_dict.fix: Stop re-validating already-consumed tool confirmations(PiperOrigin-RevId: 956611754)