mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
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>
This commit is contained in:
parent
95b09d3f78
commit
1aadb02eaf
2 changed files with 134 additions and 0 deletions
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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}]",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue