From 1aadb02eafd3420499ce30ce3154507433439c2d Mon Sep 17 00:00:00 2001 From: Mason Tanguay Date: Sat, 18 Jul 2026 01:04:45 -0700 Subject: [PATCH] fix(delegation): stop mixed platform bundles from re-exposing blocked tools to leaf children A leaf subagent is meant to be denied delegate_task, execute_code, memory, clarify, cronjob, and send_message. _strip_blocked_tools() only drops a toolset when EVERY tool in it is blocked, so mixed platform bundles (hermes-cli, hermes-telegram, and every other gateway bundle) survived stripping and re-exposed the blocked tools after composite expansion. A leaf child spawned from any gateway platform could recursively delegate, run code, and write memory. Pass exact one-tool deny toolsets into the child's disabled_toolsets so model_tools subtracts the blocked names AFTER composite expansion, and the restriction survives later registry/MCP refreshes. Orchestrators regain only delegate_task. Salvaged from #66036 by Mason Tanguay (@DictatorBacon); scoped to the authority fix + its regressions (docs/interrupt changes dropped). Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com> --- tests/tools/test_delegate.py | 90 ++++++++++++++++++++++++++++++++++++ tools/delegate_tool.py | 44 ++++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/tests/tools/test_delegate.py b/tests/tools/test_delegate.py index 0ac285b5302..96d89503826 100644 --- a/tests/tools/test_delegate.py +++ b/tests/tools/test_delegate.py @@ -200,6 +200,96 @@ class TestStripBlockedTools(unittest.TestCase): f"but was not stripped", ) + def test_mixed_composite_is_subtracted_at_child_assembly(self): + """A mixed platform bundle must not re-expose blocked leaf tools. + + ``hermes-cli`` contains both allowed tools and every sensitive + delegate tool, so it cannot be dropped wholesale. Child construction + must instead pass exact one-tool deny toolsets to AIAgent, where + model_tools applies them after resolving the composite. + """ + import model_tools + + parent = _make_mock_parent() + parent.enabled_toolsets = ["hermes-cli"] + parent.disabled_toolsets = ["browser"] + + with patch("run_agent.AIAgent") as MockAgent: + MockAgent.return_value = MagicMock() + _build_child_agent( + task_index=0, + goal="Inspect safely", + context=None, + toolsets=None, + model=None, + max_iterations=10, + parent_agent=parent, + task_count=1, + role="leaf", + ) + + _, kwargs = MockAgent.call_args + disabled = kwargs["disabled_toolsets"] + self.assertIn("browser", disabled) + for toolset_name in ( + "clarify", + "cronjob", + "delegation", + "code_execution", + "memory", + ): + self.assertIn(toolset_name, disabled) + + definitions = model_tools.get_tool_definitions( + enabled_toolsets=kwargs["enabled_toolsets"], + disabled_toolsets=disabled, + quiet_mode=True, + skip_tool_search_assembly=True, + ) + names = {item["function"]["name"] for item in definitions} + self.assertTrue(names & {"terminal", "read_file", "web_search"}) + self.assertTrue(DELEGATE_BLOCKED_TOOLS.isdisjoint(names)) + + def test_orchestrator_composite_regains_only_delegate_task(self): + import model_tools + + parent = _make_mock_parent() + parent.enabled_toolsets = ["hermes-cli"] + parent.disabled_toolsets = ["delegation", "browser"] + + with ( + patch("run_agent.AIAgent") as MockAgent, + patch("tools.delegate_tool._get_orchestrator_enabled", return_value=True), + patch("tools.delegate_tool._get_max_spawn_depth", return_value=2), + ): + MockAgent.return_value = MagicMock() + _build_child_agent( + task_index=0, + goal="Coordinate safely", + context=None, + toolsets=None, + model=None, + max_iterations=10, + parent_agent=parent, + task_count=1, + role="orchestrator", + ) + + _, kwargs = MockAgent.call_args + disabled = kwargs["disabled_toolsets"] + self.assertNotIn("delegation", disabled) + definitions = model_tools.get_tool_definitions( + enabled_toolsets=kwargs["enabled_toolsets"], + disabled_toolsets=disabled, + quiet_mode=True, + skip_tool_search_assembly=True, + ) + names = {item["function"]["name"] for item in definitions} + self.assertIn("delegate_task", names) + self.assertTrue( + (DELEGATE_BLOCKED_TOOLS - {"delegate_task"}).isdisjoint(names) + ) + class TestDelegateTask(unittest.TestCase): def test_no_parent_agent(self): diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 632fa762db0..12f94a180a8 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -784,6 +784,27 @@ def _strip_blocked_tools(toolsets: List[str]) -> List[str]: return [t for t in toolsets if t not in blocked_toolset_names] +def _blocked_toolsets_for_role(role: str) -> List[str]: + """Return one-tool deny toolsets for a delegated child role. + + ``_strip_blocked_tools`` can remove fully blocked toolsets, but it must keep + mixed platform bundles such as ``hermes-cli`` because those also contain + useful tools. Passing these exact deny toolsets to AIAgent lets + ``model_tools`` subtract blocked names *after* composite expansion, and the + restriction survives later registry/MCP refreshes through the agent's + stored ``disabled_toolsets``. + """ + blocked_names = set(DELEGATE_BLOCKED_TOOLS) + if role == "orchestrator": + blocked_names.discard("delegate_task") + return sorted( + name + for name, defn in TOOLSETS.items() + if defn.get("tools") + and set(defn.get("tools", ())).issubset(blocked_names) + ) + + def _emit_parent_console(parent_agent, line: str) -> None: """Emit a human-readable progress line to the parent's console. @@ -1137,6 +1158,28 @@ def _build_child_agent( else: child_toolsets = _strip_blocked_tools(DEFAULT_TOOLSETS) + # Blocked tools also live inside mixed platform bundles (hermes-cli, + # hermes-telegram, etc.) that _strip_blocked_tools must keep because they + # carry useful tools too. Pass exact one-tool deny toolsets through to the + # child so model_tools subtracts the blocked names AFTER composite + # expansion, and the restriction survives later registry/MCP refreshes. + raw_parent_disabled = getattr(parent_agent, "disabled_toolsets", None) + if isinstance(raw_parent_disabled, (list, tuple, set)): + inherited_disabled = [str(name) for name in raw_parent_disabled] + else: + inherited_disabled = [] + if effective_role == "orchestrator": + # Role grants delegate_task explicitly, matching the unconditional + # delegation toolset re-add below. + inherited_disabled = [ + name for name in inherited_disabled if name != "delegation" + ] + child_disabled_toolsets = list( + dict.fromkeys( + inherited_disabled + _blocked_toolsets_for_role(effective_role) + ) + ) + # Orchestrators retain the 'delegation' toolset that _strip_blocked_tools # removed. The re-add is unconditional on parent-toolset membership because # orchestrator capability is granted by role, not inherited — see the @@ -1332,6 +1375,7 @@ def _build_child_agent( prefill_messages=getattr(parent_agent, "prefill_messages", None), fallback_model=parent_fallback, enabled_toolsets=child_toolsets, + disabled_toolsets=child_disabled_toolsets, quiet_mode=True, ephemeral_system_prompt=child_prompt, log_prefix=f"[subagent-{task_index}]",