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

未关闭
#7,148 1 条评论 0 个 reaction 已指派 1 人 在 GitHub 查看

@surajksharma07 已经在做这个了。

开始于 2026年9月17日。

评估

这个 Issue 还没有评估数据。

描述

request clarification tools

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 — yimou@hust.edu.cn — GitHub: @Tardfyou
Happy to provide the full PoC files, control harness, and any additional details.

主要语言
Python
星标
21.6k
派生
4k
平均合并
13 小时 49 分钟
30 天内合并 PR
10

贡献指南

打开贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

google/adk-python 的其他 Issue

查看 google/adk-python 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。