From cd161eab7b7c42ac7369147bc9624364a8ae90d6 Mon Sep 17 00:00:00 2001 From: sai5880 Date: Thu, 22 Oct 2026 00:55:44 +0530 Subject: [PATCH] Require human confirmation for delete_files --- .../adk_agent_builder_assistant.py | 4 +- .../cli/built_in_agents/tools/delete_files.py | 12 +-- tests/unittests/cli/test_delete_files.py | 86 +++++++++++++++++++ .../cli/test_resolve_root_directory.py | 1 - 4 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 tests/unittests/cli/test_delete_files.py diff --git a/src/google/adk/cli/built_in_agents/adk_agent_builder_assistant.py b/src/google/adk/cli/built_in_agents/adk_agent_builder_assistant.py index 8d6fd7c967d..61aa5a338c4 100644 --- a/src/google/adk/cli/built_in_agents/adk_agent_builder_assistant.py +++ b/src/google/adk/cli/built_in_agents/adk_agent_builder_assistant.py @@ -96,7 +96,9 @@ def create_agent( # File management tools (multi-file support) FunctionTool(read_files), # Read multiple files FunctionTool(write_files), # Write multiple files - FunctionTool(delete_files), # Delete multiple files + FunctionTool( + delete_files, require_confirmation=True + ), # Delete multiple files FunctionTool(cleanup_unused_files), # ADK source code search (regex-based) FunctionTool(search_adk_source), # Search ADK source with regex diff --git a/src/google/adk/cli/built_in_agents/tools/delete_files.py b/src/google/adk/cli/built_in_agents/tools/delete_files.py index 61838dfbcb8..2ab339c42cd 100644 --- a/src/google/adk/cli/built_in_agents/tools/delete_files.py +++ b/src/google/adk/cli/built_in_agents/tools/delete_files.py @@ -31,20 +31,18 @@ async def delete_files( file_paths: List[str], tool_context: ToolContext, create_backup: bool = False, - confirm_deletion: bool = True, ) -> Dict[str, Any]: """Delete multiple files with optional backup creation. This tool safely deletes multiple files with validation and optional backup creation. It's designed for cleaning up unused tool files when agent configurations - change. + change. Every call requires human confirmation, which the framework collects + before the tool runs; the model cannot supply an argument to skip it. Args: file_paths: List of absolute or relative paths to files to delete create_backup: Whether to create a backup before deletion (default: False) - confirm_deletion: Whether deletion was confirmed by user (default: True for - safety) Returns: Dict containing deletion operation results: @@ -72,12 +70,6 @@ async def delete_files( "errors": [], } - # Safety check - only delete if user confirmed - if not confirm_deletion: - result["success"] = False - result["errors"].append("Deletion not confirmed by user") - return result - for resolved_path in resolved_paths: file_path_obj = resolved_path.resolve() file_info: Dict[str, Any] = { diff --git a/tests/unittests/cli/test_delete_files.py b/tests/unittests/cli/test_delete_files.py new file mode 100644 index 00000000000..7776dfe3fe5 --- /dev/null +++ b/tests/unittests/cli/test_delete_files.py @@ -0,0 +1,86 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests that delete_files is gated by framework human confirmation.""" + +from __future__ import annotations + +from pathlib import Path +from unittest import mock + +from google.adk.agents.invocation_context import InvocationContext +from google.adk.cli.built_in_agents.adk_agent_builder_assistant import AgentBuilderAssistant +from google.adk.sessions.session import Session +from google.adk.tools.tool_confirmation import ToolConfirmation +from google.adk.tools.tool_context import ToolContext + + +def _tool_context( + root: Path, + tool_confirmation: ToolConfirmation | None = None, +) -> ToolContext: + invocation_context = mock.MagicMock(spec=InvocationContext) + invocation_context._state_schema = None + invocation_context.session = mock.MagicMock(spec=Session) + invocation_context.session.state = {"root_directory": str(root)} + return ToolContext( + invocation_context=invocation_context, + function_call_id="call-1", + tool_confirmation=tool_confirmation, + ) + + +def _delete_files_tool(): + agent = AgentBuilderAssistant.create_agent(model="gemini-2.0-flash") + return next(tool for tool in agent.tools if tool.name == "delete_files") + + +async def test_delete_files_is_registered_with_require_confirmation(tmp_path): + tool = _delete_files_tool() + + assert await tool.check_require_confirmation({}, _tool_context(tmp_path)) + + +async def test_model_supplied_argument_cannot_bypass_confirmation(tmp_path): + victim = tmp_path / "orphan.py" + victim.write_text("x") + tool = _delete_files_tool() + + result = await tool.run_async( + args={ + "file_paths": [str(victim)], + # A stale argument from the old signature must not act as approval. + "confirm_deletion": True, + }, + tool_context=_tool_context(tmp_path), + ) + + assert result == { + "error": "This tool call requires confirmation, please approve or reject." + } + assert victim.exists() + + +async def test_approved_call_deletes_the_file(tmp_path): + victim = tmp_path / "orphan.py" + victim.write_text("x") + tool = _delete_files_tool() + + result = await tool.run_async( + args={"file_paths": [str(victim)]}, + tool_context=_tool_context(tmp_path, ToolConfirmation(confirmed=True)), + ) + + assert result["success"] + assert not victim.exists() diff --git a/tests/unittests/cli/test_resolve_root_directory.py b/tests/unittests/cli/test_resolve_root_directory.py index 9b7771e3260..092cdb24b0d 100644 --- a/tests/unittests/cli/test_resolve_root_directory.py +++ b/tests/unittests/cli/test_resolve_root_directory.py @@ -155,7 +155,6 @@ async def test_delete_files_blocks_relative_traversal( result = await delete_files( file_paths=[payload], tool_context=_tool_context(tmp_path), - confirm_deletion=True, ) assert not result["success"]