Skip to content

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

Closed

Description

@Tardfyou

Summary

ADK's tool-confirmation feature is designed so that a sensitive tool call pauses for explicit human approval before any side effect occurs. The human's decision is delivered back to the framework as a ToolConfirmation object whose confirmed boolean carries the approve/deny verdict.

The framework, however, never reads confirmed. The resume path (google/adk/flows/llm_flows/request_confirmation.py:: _resolve_confirmation_targets) validates that the tool call is registered, requires confirmation, and that the arguments match — but never checks the verdict — and then hands the call to functions.py:: _execute_single_function_call_async, which attaches the ToolConfirmation to the ToolContext as advisory data and unconditionally invokes tool.run_async(...).

Whether "Decline" actually prevents the side effect is left to each individual tool implementation. Built-in tools (FunctionTool.run_async at function_tool.py:350, plus BashTool, MCPTool, ComputerUseTool) each re-implement the deny check inside their own run_async. But BaseTool subclassing is the documented extension point for custom/enterprise tools. Any custom BaseTool that opts into confirmation via the documented check_require_confirmation / request_confirmation API but does not itself honor tool_confirmation.confirmed (by bug, omission, or intent) executes its side effect after the user explicitly declined.

Root cause

  1. google/adk/flows/llm_flows/request_confirmation.py — _resolve_confirmation_targets(...): validates registration / requires-confirmation / argument match, and re-executes confirmed tools — but there is no branch that inspects ToolConfirmation.confirmed. A confirmed=False response takes exactly the same code path as confirmed=True.
  2. google/adk/flows/llm_flows/functions.py — _execute_single_function_call_async: passes tool_confirmation into ToolContext and calls tool.run_async(...) unconditionally; no central check of .confirmed.
  3. Deny enforcement exists only inside four built-in tools' own run_async implementations (function_tool.py:350 etc.). BaseTool subclassing is the documented extension point, so every custom tool must independently re-implement deny handling — an easy-to-miss, unenforced contract.

Reproduction (fully offline, deterministic; no LLM API needed)

Save as poc.py and run with python poc.py against google-adk==2.9.1:

import asyncio, json, os, tempfile
from google.adk.agents import LlmAgent
from google.adk.models import BaseLlm
from google.adk.models.llm_response import LlmResponse
from google.adk.runners import Runner
from google.adk.sessions import InMemorySessionService
from google.adk.tools import BaseTool
from google.adk.tools.tool_context import ToolContext
from google.genai import types

MARKER = None
EXECUTIONS = []

class FakeLlm(BaseLlm):
  def __init__(self):
    super().__init__(model='fake-llm')
    object.__setattr__(self, 'calls', 0)
  async def generate_content_async(self, llm_request, stream=False):
    n = self.calls
    object.__setattr__(self, 'calls', n + 1)
    if n == 0:
      yield LlmResponse(content=types.Content(role='model', parts=[
          types.Part.from_function_call(name='dangerous_custom',
                                        args={'target': 'prod-db'})]))
    else:
      yield LlmResponse(content=types.Content(role='model',
          parts=[types.Part.from_text(text=f'turn {n} done')]))

class DangerousCustomTool(BaseTool):
  """Custom BaseTool that opts into confirmation."""
  def __init__(self):
    super().__init__(name='dangerous_custom', description='side-effecting tool')
  def _get_declaration(self):
    return types.FunctionDeclaration(name=self.name, description=self.description,
        parameters=types.Schema(type=types.Type.OBJECT,
            properties={'target': types.Schema(type=types.Type.STRING)}))
  async def check_require_confirmation(self, args, tool_context) -> bool:
    return True
  async def run_async(self, *, args, tool_context):
    if not tool_context.tool_confirmation:
      tool_context.request_confirmation(hint='Allow destructive action?')
      return {'status': 'awaiting_user_confirmation'}
    EXECUTIONS.append(tool_context.tool_confirmation.confirmed)
    with open(MARKER, 'w') as f:
      f.write('EFFECT HAPPENED. user_confirmed=%s\n'
              % tool_context.tool_confirmation.confirmed)
    return {'status': 'executed',
            'user_confirmed': tool_context.tool_confirmation.confirmed}

async def main():
  global MARKER
  MARKER = os.path.join(tempfile.mkdtemp(prefix='poc_'), 'effect_marker.txt')
  agent = LlmAgent(name='root', model=FakeLlm(), tools=[DangerousCustomTool()])
  runner = Runner(app_name='poc', agent=agent, session_service=InMemorySessionService())
  s = await runner.session_service.create_session(app_name='poc', user_id='u')

  fc_id = None
  async for ev in runner.run_async(user_id='u', session_id=s.id,
      new_message=types.Content(role='user', parts=[types.Part.from_text(text='do it')])):
    for fc in ev.get_function_calls():
      if fc.name == 'adk_request_confirmation':
        fc_id = fc.id
  print('turn1 marker exists (expect False):', os.path.exists(MARKER))

  deny = types.Content(role='user', parts=[types.Part(function_response=
      types.FunctionResponse(name='adk_request_confirmation', id=fc_id,
                             response={'confirmed': False}))])
  async for ev in runner.run_async(user_id='u', session_id=s.id, new_message=deny):
    for fr in ev.get_function_responses():
      print('turn2 function_response:', fr.name, '->', fr.response)

  print('EFFECT EXECUTED DESPITE DENY:', os.path.exists(MARKER))
  print('run_async verdicts seen:', EXECUTIONS)

asyncio.run(main())

Observed output (google-adk 2.9.1, Python 3.12)

turn1 marker exists (expect False): False
turn2 function_response: dangerous_custom -> {'status': 'executed', 'user_confirmed': False}
EFFECT EXECUTED DESPITE DENY: True
marker content: EFFECT HAPPENED. user_confirmed=False args={"target": "prod-db"}
run_async verdicts seen: [False]

Expected output

The framework should refuse to resume the tool when confirmed is False: it should
return a denial function response to the model and never invoke run_async — or at
minimum provide a framework-level default-deny hook so custom tools cannot forget it.

Impact

  • Direct verdict-to-effect break: the human explicitly declined, yet the sensitive
    operation executed (user_confirmed=False inside the executed effect). The
    confirmation dialog is decorative for any tool that does not self-enforce.
  • Documented extension point affected: BaseTool subclassing is the standard way
    enterprises wrap internal sensitive operations (DB mutations, deployments, payments).
    An implementation that forgets the deny check turns the approval dialog into a no-op.
  • Prompt-injection amplification: an injected model instruction that triggers a
    confirmation-gated tool now needs only a distracted user clicking "Decline" for the
    action to execute anyway; the denial is even echoed to the model inside the executed
    result payload (user_confirmed: False).

Suggested fix

In _resolve_confirmation_targets (request_confirmation.py), split re-execution
targets by verdict: only confirmed is True calls proceed to
_execute_single_function_call_async; confirmed=False calls should receive a
framework-generated denial function response, never re-entering run_async.
Additionally, expose a BaseTool.on_confirmation_denied(...) default implementation
so tools can customize the denial message without being responsible for stopping
execution.


Disclosure note

This finding was reported through Google's Vulnerability Reward Program
(issue 562236334). The Google Bug
Hunter team reviewed the report and responded that it is a "design choice or framework
ergonomics issue rather than a security vulnerability," and suggested: "Feel free to
disclose this on the project's GitHub issues page as a public issue."
This issue is
filed per that guidance.


Reporter: Chengzhi Yi — [email protected] — GitHub: @Tardfyou
Happy to provide the full PoC files, control harness, and any additional details.

Activity

  1. surajksharma07 commented on Sep 17, 2026

    @surajksharma07
    Collaborator

    @Tardfyou _resolve_confirmation_targets never checks .confirmed so a denied call still reaches run_async on our end too. As a workaround patching the shared _call_tool_async chokepoint in _caller.py to short-circuit and return a denial response whenever confirmed is False closes the gap in our testing. Could you try that on your setup and confirm it holds? #7151 already implements this same centralized-enforcement approach so once you've verified it thoroughly adding your confirmation there would help move it toward merge rather than opening a separate PR.

  2. sattyamjjain commented on Sep 22, 2026

    @sattyamjjain

    Agree on the chokepoint. I think the framing can go one step further, though: the problem is not a missing check in one place; it is that ToolConfirmation is advisory data rather than a gate.

    Today, confirmed=False gets handed to the tool, and the tool is trusted to act on it. The four built-in tools do. Every custom BaseTool is on its own, and a tool that forgets the check is indistinguishable from one that approves. BaseTool being the documented extension point is what makes that the default case rather than the edge case.

    If the verdict is enforced centrally before tool.run_async(...), a custom tool that forgets fails closed instead of open, and the behaviour gets tested once rather than once per tool.

    One suggestion for the test matrix if someone picks this up: a custom BaseTool subclass that ignores tool_confirmation entirely and records a side effect. Assert that the side effect never happens when confirmed=False. That test fails today, passes after the fix, and it is exactly the case the four built-in implementations cannot cover for you.

    Context on where I am coming from: I maintain agent-airlock, where this decision went the other way. A denied approval raises before the wrapped function is reached, so a tool never gets the chance to ignore it — https://github.com/sattyamjjain/agent-airlock/blob/main/docs/policies/human-oversight-decorator.md. Happy to send a PR with the central check and that test if maintainers want it.

  3. surajksharma07 commented on Sep 24, 2026

    @surajksharma07
    Collaborator

    Thanks @Tardfyou for the four-case check. Main now enforces this in the framework: 1d0daeb (plus 659e28f) adds a confirmation gate in _caller.py that answers "This tool call is rejected." before run_async when confirmed is False, with custom BaseTool tests for both the rejected case and the approved control. Your PoC on main gives 0 executions on deny and 1 on approve but 2.9.1/2.9.2 still run the tool so keep the caller workaround until the next release.

    @AnvitDevadiga #7151 still merges cleanly and blocks the deny but that part now overlaps with the gate on main and it changes the rejection text to "Tool execution not confirmed" (existing runner tests are edited to match) so might be worth narrowing the PR to the replay part.

    On replay though the claim set lives on InvocationContext which is rebuilt on every run_async and the Step 2 dedup only looks at responses after the latest user event. In my test sending the same confirmed=True twice (one after the other or at the same time) still ran the same function_call_id twice on both main and the PR head. The new test only claims twice on one context so it can't catch that.

    A durable check (looking for the original call's function_response anywhere in session history) would close it. Happy to open that as a separate issue if it's easier to track there.

  4. AnvitDevadiga commented on Sep 25, 2026

    @AnvitDevadiga

    Thanks for catching this. You’re right that an InvocationContext-local claim set cannot protect replays across separate run_async calls.

    I updated the deduplication logic to scan the complete persisted session history for an existing function response for the original function_call_id. This makes the replay check durable across rebuilt invocation contexts and process resumes.

    I also narrowed the PR by removing the denial-path changes that now overlap with the gate already present on main. The PR now focuses on confirmation replay protection.

    The fix is pushed in commit 4beee3a. Syntax and diff checks pass; the full test suite is blocked here because PyPI access is unavailable.

  5. surajksharma07 commented on Sep 28, 2026

    @surajksharma07
    Collaborator

    @Tardfyou the confirmation gate from 1d0daeb is out in v2.10.0. Your PoC on 2.10.0 gets "This tool call is rejected." on Decline and run_async is never called so you can drop the caller workaround after upgrading. Could you confirm on your side so we can close this?

    @AnvitDevadiga thanks for narrowing it down. Ran the same probe against b5bebd0 and replaying the same confirmed=True now gives one execution both serially and concurrently (2.10.0 still gives two). A few things before review: _claimed_tool_confirmations never drops entries so it keeps growing on a long-running server. _consumed_tool_confirmation_ids isn't read anywhere. The claim is only in memory so two replicas sharing a DatabaseSessionService can still both run the call. Could you add a runner-level test that sends the same confirmed=True twice (one after the other and with asyncio.gather) to a custom BaseTool that ignores the verdict and asserts a single execution? The current test only claims twice on one context.

  6. AnvitDevadiga commented on Sep 29, 2026

    @AnvitDevadiga

    Thanks for the review. I pushed 317b272f to PR #7151.

    Changes:

    • Removed the unused per-invocation _consumed_tool_confirmation_ids state.
    • Added public-Runner regression coverage for the same confirmed=True payload submitted serially and through asyncio.gather; both assert exactly one tool execution.
    • The claim registry remains process-local by design; cross-replica exactly-once semantics require a durable atomic claim in the session-service contract, which this PR does not pretend to provide.

    Validation: the two confirmation suites pass: 34 tests passed.

  7. surajksharma07 commented on Oct 6, 2026

    @surajksharma07
    Collaborator

    Checked 317b272/3f514e2 @AnvitDevadiga. On its own head both confirmation suites pass (36) and the new runner test is exactly the shape I was after. Merged onto current main (151 commits ahead, no conflicts) though test_repeated_confirmation_is_executed_once fails for both params on the invariant checker main added in 1f3b66b.

    It isn't just the checker. Dumping the session after a serial replay shows the tool still runs once but the replayed confirmed=True now gets a fresh adk_request_confirmation for the same already-executed call id and another "requires confirmation" response so the user is asked to approve it again. That doesn't happen on your old base and main alone runs the tool twice. 3d1d12d (gate moved into tools/_confirmation_utils.py, in 2.11.0) looks like the likely interaction. Could you rebase and have the test check the events not just the count?

    Still open from before: _claimed_tool_confirmations never drops entries. Since your history check already covers a replay after the response is persisted releasing the claim once the tool's response is appended would bound it to in-flight calls.

    @Tardfyou the deny gate is in both 2.10.0 and 2.11.0. If it holds on your side this can close once the replay part is settled.

  8. Tardfyou commented on Oct 7, 2026

    @Tardfyou
    Author

    @surajksharma07 Confirmed on our side. We re-ran our original PoC (a custom BaseTool that opts into confirmation and records side effects) against google-adk 2.10.0 from PyPI, unmodified:

    • user Decline → run_async is never called (0 executions), the function_response is {'error': 'This tool call is rejected.'}, and the side effect does not happen. On 2.9.1 the same harness executed the tool on deny, so the behavior change is attributable to the gate.
    • the caller workaround is no longer needed on 2.10.0 — we'll note that in our tracking.

    Thanks for landing the centralized enforcement; happy to see the confirm/deny contract move into the framework where it belongs. (We're following the replay work in #7151 separately — no further action needed from us on this issue.)

  9. DeanChensj commented on Oct 7, 2026

    @DeanChensj
    Collaborator

    Thank you so much @Tardfyou for the detailed vulnerability report, the clean offline reproduction script, and for taking the time to re-verify the fix on google-adk 2.10.0!

    As confirmed above, ToolConfirmation deny enforcement is now handled centrally by the framework in apply_confirmation_gate() (src/google/adk/tools/_confirmation_utils.py and src/google/adk/flows/llm_flows/tools/_caller.py) before tool.run_async() is ever invoked, ensuring that all built-in and custom BaseTool subclasses fail closed when confirmed=False.

    I will go ahead and close this issue as resolved (with confirmation replay protection continuing to be tracked separately in #7151). Please feel free to open a new issue or PR anytime if you run into any other edge cases!

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

Metadata

Metadata

Labels

request clarification[Status] The maintainer need clarification or more information from the authortools[Component] This issue is related to tools

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions