diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 0dc5a2e00b6..8b6d2b2454b 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -1360,6 +1360,15 @@ def read_only_tools_auto_approval_rule(function_call: Content) -> bool: auto-approved, even when their name matches a file-access tool, so the rule stays scoped to this provider's local tools. + .. warning:: + **Security — avoid tool-name collisions.** This rule approves local + tool calls by tool name only (``file_access_read``, + ``file_access_ls``, and ``file_access_grep``). Any other local tool + registered under one of these names — for example a tool with a + caller-configurable name such as the shell tool — may also be + auto-approved, bypassing the human approval boundary. Ensure no other + tool collides with these reserved names. + Args: function_call: The pending ``function_call`` content. @@ -1387,6 +1396,17 @@ def all_tools_auto_approval_rule(function_call: Content) -> bool: auto-approved, even when their name matches a file-access tool, so the rule stays scoped to this provider's local tools. + .. warning:: + **Security — avoid tool-name collisions.** This rule approves local + tool calls by tool name only (``file_access_write``, + ``file_access_read``, ``file_access_delete``, ``file_access_ls``, + ``file_access_grep``, ``file_access_replace``, and + ``file_access_replace_lines``). Any other local tool registered under + one of these names — for example a tool with a caller-configurable + name such as the shell tool — may also be auto-approved, bypassing + the human approval boundary. Ensure no other tool collides with these + reserved names. + Args: function_call: The pending ``function_call`` content. diff --git a/python/packages/core/agent_framework/_harness/_tool_approval.py b/python/packages/core/agent_framework/_harness/_tool_approval.py index 56595278d04..289a3bccbcf 100644 --- a/python/packages/core/agent_framework/_harness/_tool_approval.py +++ b/python/packages/core/agent_framework/_harness/_tool_approval.py @@ -364,6 +364,19 @@ def __init__( auto_approval_rules: Optional callbacks that can auto-approve a ``function_call``. Each callback receives the function-call content and returns ``True`` to approve it. + + .. warning:: + **Security.** Auto-approval rules may match tool calls by + name (each callback receives the full ``function_call`` + content, including arguments, and can implement stricter, + argument-aware matching). A rule provided for one feature + (for example a provider's read-only rule) may auto-approve + **any** local tool whose name matches, not just the tool the + rule was designed for. When adding rules here, ensure no + unrelated tools you register collide with a name approved by + any rule in this list, otherwise that tool may be + auto-approved without a human prompt, bypassing the approval + boundary. """ self.source_id = source_id self.auto_approval_rules = tuple(auto_approval_rules or ()) diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index ba709768091..41f69ff8899 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -1969,6 +1969,15 @@ def read_only_tools_auto_approval_rule(function_call: Content) -> bool: auto-approved, even when their name matches a skill tool, so the rule stays scoped to this provider's local tools. + .. warning:: + **Security — avoid tool-name collisions.** This rule approves local + tool calls by tool name only (``load_skill`` and + ``read_skill_resource``). Any other local tool registered under one + of these names — for example a tool with a caller-configurable name + such as the shell tool — may also be auto-approved, bypassing the + human approval boundary. Ensure no other tool collides with these + reserved names. + Args: function_call: The pending ``function_call`` content. @@ -1997,6 +2006,15 @@ def all_tools_auto_approval_rule(function_call: Content) -> bool: auto-approved, even when their name matches a skill tool, so the rule stays scoped to this provider's local tools. + .. warning:: + **Security — avoid tool-name collisions.** This rule approves local + tool calls by tool name only (``load_skill``, + ``read_skill_resource``, and ``run_skill_script``). Any other local + tool registered under one of these names — for example a tool with a + caller-configurable name such as the shell tool — may also be + auto-approved, bypassing the human approval boundary. Ensure no other + tool collides with these reserved names. + Args: function_call: The pending ``function_call`` content. diff --git a/python/packages/tools/agent_framework_tools/shell/_docker.py b/python/packages/tools/agent_framework_tools/shell/_docker.py index 8afc3338785..f718c0d6d58 100644 --- a/python/packages/tools/agent_framework_tools/shell/_docker.py +++ b/python/packages/tools/agent_framework_tools/shell/_docker.py @@ -680,7 +680,23 @@ def as_function( name: str = "run_shell", description: str | None = None, ) -> FunctionTool: - """Return a :class:`~agent_framework.FunctionTool` bound to this instance.""" + """Return a :class:`~agent_framework.FunctionTool` bound to this instance. + + Args: + name: Function name surfaced to the model. Defaults to ``run_shell``. + + .. warning:: + **Security — avoid tool-name collisions.** This applies only + when the shell tool requires approval (constructed with + ``approval_mode="always_require"``, the default). Auto-approval + rules may match tool calls by name, so setting ``name`` to a + value approved by an auto-approval rule for another feature + may cause this shell tool to also be auto-approved, bypassing + the human approval boundary. Choose a unique name that no + other registered tool uses. + description: Optional description surfaced to the model. When + ``None`` a mode-appropriate default is used. + """ async def _run_shell(command: str) -> str: try: diff --git a/python/packages/tools/agent_framework_tools/shell/_tool.py b/python/packages/tools/agent_framework_tools/shell/_tool.py index 38edb932d49..920639c0737 100644 --- a/python/packages/tools/agent_framework_tools/shell/_tool.py +++ b/python/packages/tools/agent_framework_tools/shell/_tool.py @@ -285,6 +285,21 @@ def as_function( The returned tool has ``kind="shell"`` so provider-specific ``get_shell_tool(func=...)`` factories recognise it as a local shell. + + Args: + name: Function name surfaced to the model. Defaults to ``run_shell``. + + .. warning:: + **Security — avoid tool-name collisions.** This applies only + when the shell tool requires approval (constructed with + ``approval_mode="always_require"``, the default). Auto-approval + rules may match tool calls by name, so setting ``name`` to a + value approved by an auto-approval rule for another feature + may cause this shell tool to also be auto-approved, bypassing + the human approval boundary. Choose a unique name that no + other registered tool uses. + description: Optional description surfaced to the model. When + ``None`` a mode-appropriate default is used. """ async def _run_shell(command: str) -> str: diff --git a/python/samples/02-agents/harness/README.md b/python/samples/02-agents/harness/README.md index 4bda12e4032..2167db5ed45 100644 --- a/python/samples/02-agents/harness/README.md +++ b/python/samples/02-agents/harness/README.md @@ -159,6 +159,13 @@ for vetting the external service, agent, skill source, or provider before enabli out to an LLM whose output permanently becomes chat history. A compromised summarization service could inject unsafe, persistent instructions. Only use a service you trust as much as the primary model. +- **Auto-approval rules** (`FileAccessProvider.read_only_tools_auto_approval_rule` / + `all_tools_auto_approval_rule`, and the equivalent `SkillsProvider` rules, passed to + `ToolApprovalMiddleware`) — the built-in rules approve local tools by tool name only (e.g. + `file_access_read`, `file_access_ls`, `file_access_grep`). Auto-approval rules may match by name, + so any other local tool registered under one of these names — for example the shell tool given a + caller-configurable name — may also be auto-approved, bypassing the human approval boundary. Ensure + no other tool collides with these reserved names. - **Telemetry** — when observability is enabled, telemetry destinations are developer-configured. Default telemetry is metadata only; enabling sensitive data additionally emits raw message content, tool arguments, and tool results. See the [observability samples](../observability/README.md). diff --git a/python/samples/02-agents/harness/build_your_own_claw/README.md b/python/samples/02-agents/harness/build_your_own_claw/README.md index 28ca1aaf38d..b1c90ac784e 100644 --- a/python/samples/02-agents/harness/build_your_own_claw/README.md +++ b/python/samples/02-agents/harness/build_your_own_claw/README.md @@ -75,6 +75,14 @@ Teaches the assistant to work with *your* data safely. deleting still pause for approval. The `place_trade` tool is marked `approval_mode="always_require"`, so the harness asks you to approve or deny before any trade runs. The trade is simulated. + + > ⚠️ **Security — avoid tool-name collisions:** `read_only_tools_auto_approval_rule` + > approves local file-access tools by tool name only (`file_access_read`, + > `file_access_ls`, `file_access_grep`). Auto-approval rules may match by name, + > so any other local tool registered under one of these names — for example a + > tool with a caller-configurable name such as the shell tool — may also be + > auto-approved, bypassing the human approval boundary. Ensure no other tool + > collides with these reserved names. - **Durable memory, two ways:** - **File memory** (coarse-grained, explicit) — the agent reads/writes files such as `watchlist.md`. File memory is on by default; its files live on disk under diff --git a/python/samples/02-agents/skills/skills_auto_approval/README.md b/python/samples/02-agents/skills/skills_auto_approval/README.md index ba324ee349a..31895ace81c 100644 --- a/python/samples/02-agents/skills/skills_auto_approval/README.md +++ b/python/samples/02-agents/skills/skills_auto_approval/README.md @@ -22,6 +22,14 @@ prompt for safe operations. Both rules reject any call carrying a `server_label`, so they stay scoped to this provider's local tools and never auto-approve a same-named hosted tool. +> ⚠️ **Security — avoid tool-name collisions:** these rules approve local skill +> tools by tool name only (`load_skill`, `read_skill_resource`, and — for +> `all_tools_auto_approval_rule` — `run_skill_script`). Auto-approval rules may +> match by name, so any other local tool registered under one of these names — +> for example a tool with a caller-configurable name such as the shell tool — may +> also be auto-approved, bypassing the human approval boundary. Ensure no other +> tool collides with these reserved names. + > **Note:** To use auto-approval rules, the agent must have `ToolApprovalMiddleware` in its middleware stack. ## Key Components