Skip to content

fix: enforce denied tool confirmations centrally - #7151

Open
AnvitDevadiga wants to merge 1 commit into
google:mainfrom
AnvitDevadiga:enforce-tool-confirmation-denials
Open

AnvitDevadiga wants to merge 1 commit into
google:mainfrom
AnvitDevadiga:enforce-tool-confirmation-denials

Conversation

@AnvitDevadiga

Copy link
Copy Markdown

What changed

Enforce ToolConfirmation decisions at the ADK framework boundary.

  • Declined confirmations now return a framework-generated denial response.
  • Denied tools are never passed to BaseTool.run_async.
  • Approved tools retain the existing execution path.
  • Mixed approved and denied batches are handled independently.

Why

Custom BaseTool implementations can currently execute after a user clicks Decline unless each tool reimplements the denial check. This fixes the issue described in #7148 and makes the documented confirmation flow default-deny.

Testing

  • 39 targeted confirmation, tool, and runner tests pass.
  • Ruff checks pass.

Closes #7148

@google-cla

google-cla Bot commented Sep 17, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@AnvitDevadiga
AnvitDevadiga force-pushed the enforce-tool-confirmation-denials branch from 71c0be8 to 1c3bd19 Compare September 17, 2026 03:19
@babyblueviper1

Copy link
Copy Markdown

Good fix — moving denial enforcement to the framework boundary instead of trusting each BaseTool to reimplement it is the right shape (matches the general lesson: a verdict object is only as strong as the code path that's forced to consume it, not the code path that happens to check it).

One adjacent question worth asking before merge, not asserted as a bug since I haven't reproduced it against this exact tree: does the resume path guard against the same function_call_id/ToolConfirmation being submitted more than once? If a client can call the resume endpoint twice with an identical confirmed=True payload for a function_call_id that's already been executed, does _resolve_confirmation_targets re-execute it, or is there state elsewhere marking that confirmation as consumed?

Reason I ask: we hit the identical shape in our own verdict-issuing service — a pure function computing a decision reference had no state, so an identical still-valid verdict could be replayed to authorize a second dispatch (caught via a negative-control test explicitly checking for replay, not by inspection). The fix there was a small addition on top of the same "enforce at the boundary" pattern this PR already uses: a primary-keyed consumed-set with atomic check-and-mark, so a race between two identical resume calls has exactly one winner instead of two executions. If tools_to_resume_with_confirmation here is keyed by function_call_id and nothing marks an id as spent after this code path runs, the same gap likely exists — cheap to close now while the boundary-enforcement code is already being touched, much easier than adding it after the fact once callers start depending on today's semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ToolConfirmation deny verdict is never enforced by the framework: custom BaseTool executes after user clicks Decline (adk-python 2.9.1)

3 participants