diff --git a/CHANGELOG.md b/CHANGELOG.md index 000ac8d250..1c07a7efa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,6 +98,14 @@ This section accumulates work toward the **2.3.0** milestone only; ordinary chat keeps them and `/connect` is unchanged. Mirrors the Feishu and DingTalk handling. ([#8118]) +- **subagents:** Apply tool allowlists and denylists to middleware-declared + tools even when global authorization is disabled, including the default + denial of `task`. Restricted subagents reject non-Sequence declarations + explicitly, and invalid declarations report middleware-specific configuration + guidance. Final graph assembly checks both explicit and middleware tools; + framework-generated discovery helpers retain their filtered catalogs. + ([#8136](https://github.com/bytedance/deer-flow/pull/8136)) + - **frontend:** **Share** in a sidebar chat's menu now copies a link to the deployment you are using. On `localhost` and `127.0.0.1`, the default `make dev` and Docker addresses, it copied a link to the hosted demo diff --git a/README.md b/README.md index e5655b0443..3ca12eb365 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,7 @@ Each guide is the full reference for its topic. Open the one you need. | Sandbox modes, file system, uploads and outputs | [Sandbox](docs/guide/sandbox.md) | | Skills, built-in tools, script workflows, Claude Code integration | [Skills & Tools](docs/guide/skills-and-tools.md) | | MCP servers | [MCP Server](docs/guide/mcp.md) | -| Sub-agents and task delegation | [Sub-Agents](docs/guide/subagents.md) | +| Sub-agents and task delegation | [Sub-Agents](docs/guide/subagents.md), [Tool restrictions](docs/guide/subagents.md#tool-restrictions) | | Long-term memory | [Long-Term Memory](docs/guide/memory.md) | | Session goals, context compaction, task notes, evidence-based decisions | [Context, Goals and Agent Decisions](docs/guide/context.md) | | Private knowledge retrieval (RAGFlow) | [Private Knowledge Retrieval](docs/guide/knowledge-retrieval.md) | diff --git a/backend/packages/harness/deerflow/agents/middlewares/tool_declarations.py b/backend/packages/harness/deerflow/agents/middlewares/tool_declarations.py index f643c3ca6b..29408a2682 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/tool_declarations.py +++ b/backend/packages/harness/deerflow/agents/middlewares/tool_declarations.py @@ -53,9 +53,13 @@ shallow copy shares, so that shape is refused *before any write* and the build fails with the caller's instance fully intact. When ``authorization.enabled`` is not true the ordinary pass returns -``provider=None`` and every step here is skipped: nothing is collected, +``provider=None`` and the authorization steps above are skipped: nothing is collected, nothing is narrowed, and the stack handed to ``create_agent`` is the original list object (:func:`narrow_declared_tools` returns it unchanged). + +Subagent name restrictions are independent: :func:`apply_declared_tool_policy` +also narrows declarations when provider authorization is disabled. It preserves +allowed callable declarations and leaves unrestricted stacks untouched. """ from __future__ import annotations @@ -68,6 +72,7 @@ from dataclasses import dataclass from typing import Any from langchain_core.tools import BaseTool +from langchain_core.tools import tool as create_tool from deerflow.authz.provider import AuthorizationProvider from deerflow.authz.tool_filter import apply_tool_authorization @@ -116,16 +121,22 @@ def _middleware_name(middleware: object) -> str: return getattr(middleware, "name", None) or type(middleware).__name__ -def _iter_declared(middleware: object) -> tuple[Any, ...]: +def _iter_declared(middleware: object, *, allow_sequences: bool = False) -> tuple[Any, ...]: tools = getattr(middleware, "tools", None) if tools is None: return () - if not isinstance(tools, (list, tuple)): + if allow_sequences: + # Match AgentMiddleware.tools' Sequence contract without consuming a + # shared one-shot iterator. Provider authorization keeps its stricter + # list/tuple contract below. + if not isinstance(tools, Sequence) or isinstance(tools, (str, bytes, bytearray)): + raise DeclaredToolViewError(f"Middleware {_middleware_name(middleware)} ({type(middleware).__name__}) must declare tools as a Sequence of tools (for example, a list or tuple), not {type(tools).__name__}.") + elif not isinstance(tools, (list, tuple)): # Fail loudly, not a warning-and-skip: LangChain's factory iterates # the attribute as-is, so a non-sequence container (set, generator, - # dict_values, ...) would still bind with no Layer-1 decision. Only - # reachable on the authorization-enabled paths — when authorization - # is disabled this module never inspects the stack. + # dict_values, ...) would still bind with no name-based decision. + # Provider authorization requires list/tuple declarations. Subagent + # name policies opt into the Sequence contract above. raise DeclaredToolViewError( f"Middleware {_middleware_name(middleware)} ({type(middleware).__name__}) declares 'tools' as {type(tools).__name__}, not a list/tuple; LangChain would iterate and bind it unchecked. Declare tools as a list or tuple." ) @@ -274,6 +285,125 @@ def apply_declared_tool_view( return view +class ToolPolicyNameResolver: + """Reuse callable name conversion within one build, never policy verdicts. + + Keep declarations alive and match by identity, including unhashable callable + objects. Read BaseTool names on every pass; a renamed callable is resolved + again so the final audit still checks the names LangChain will bind. + """ + + def __init__(self) -> None: + self._resolved: dict[int, tuple[Any, str | None, str]] = {} + + def name(self, declaration: Any, *, owner: str) -> str: + if isinstance(declaration, BaseTool): + return declaration.name + try: + callable_name = getattr(declaration, "__name__", None) + cached = self._resolved.get(id(declaration)) + if cached is not None and cached[0] is declaration and cached[1] == callable_name: + return cached[2] + name = create_tool(declaration).name + except Exception as exc: + raise DeclaredToolViewError(f"{owner} declares a {type(declaration).__name__} tool entry that LangChain cannot convert. Declare tools as BaseTool instances or callables with a valid name, signature and docstring.") from exc + self._resolved[id(declaration)] = (declaration, callable_name, name) + return name + + +def _tool_policy_denial(name: str, allowed_names: frozenset[str] | None, denied_names: frozenset[str]) -> str | None: + if name in denied_names: + return "listed in disallowed_tools" + if allowed_names is not None and name not in allowed_names: + return "missing from the tools allowlist" + return None + + +def apply_declared_tool_policy( + middlewares: Sequence[object], + *, + allowed: Sequence[str] | None, + disallowed: Sequence[str] | None, + name_resolver: ToolPolicyNameResolver | None = None, +) -> list[object]: + """Apply subagent name restrictions without changing allowed declarations. + + Unlike provider authorization, name policies also support plain callables: + resolve their names exactly as ToolNode does, while retaining the original + entries. Only a middleware losing a tool needs an independent copy. This + pass handles declarations only; generated discovery helpers in the explicit + tool list already carry a policy-filtered catalog. Declaration containers + follow AgentMiddleware's Sequence contract; one-shot iterators are rejected + before consumption so a shared extension does not silently lose tools. + """ + if allowed is None and not disallowed: + return middlewares if isinstance(middlewares, list) else list(middlewares) + + allowed_names = frozenset(allowed) if allowed is not None else None + denied_names = frozenset(disallowed or ()) + resolver = name_resolver if name_resolver is not None else ToolPolicyNameResolver() + + def denial_reason(middleware: object, declaration: Any) -> str | None: + name = resolver.name(declaration, owner=f"Middleware {_middleware_name(middleware)} ({type(middleware).__name__})") + return _tool_policy_denial(name, allowed_names, denied_names) + + view = middlewares if isinstance(middlewares, list) else list(middlewares) + for index, middleware in enumerate(view): + tools = _iter_declared(middleware, allow_sequences=True) + kept = tuple(tool for tool in tools if denial_reason(middleware, tool) is None) + if len(kept) == len(tools): + continue + clone = _independent_copy(middleware) + _assign_narrowed_tools(clone, middleware, kept) + if view is middlewares: + view = list(middlewares) + view[index] = clone + + # Re-read the exact stack handed to create_agent, including copied wrappers. + for middleware in view: + for declaration in _iter_declared(middleware, allow_sequences=True): + reason = denial_reason(middleware, declaration) + if reason is not None: + raise DeclaredToolViewError(f"Middleware {_middleware_name(middleware)} still declares a tool {reason} after build-local narrowing.") + return view + + +def verify_subagent_tool_policy( + tools: Sequence[Any], + middlewares: Sequence[object], + *, + allowed: Sequence[str] | None, + disallowed: Sequence[str] | None, + discovery_tools: Sequence[BaseTool] = (), + name_resolver: ToolPolicyNameResolver | None = None, +) -> None: + """Audit the final explicit and middleware tools immediately before graph creation. + + Never refilter or mutate the build. Exempt only the exact discovery helpers + generated from admitted tools/skills for this assembly, and only in the + explicit list; matching a helper's name does not grant an exemption. + """ + if allowed is None and not disallowed: + return + allowed_names = frozenset(allowed) if allowed is not None else None + denied_names = frozenset(disallowed or ()) + resolver = name_resolver if name_resolver is not None else ToolPolicyNameResolver() + + def verify(declaration: Any, *, owner: str) -> None: + name = resolver.name(declaration, owner=owner) + reason = _tool_policy_denial(name, allowed_names, denied_names) + if reason is not None: + raise DeclaredToolViewError(f"{owner} exposes tool {name!r}, {reason}, in the final tool set. Apply the subagent name policy to every tool source before graph creation.") + + for tool in tools: + if any(tool is helper for helper in discovery_tools): + continue + verify(tool, owner="Subagent explicit tool list") + for middleware in middlewares: + for declaration in _iter_declared(middleware, allow_sequences=True): + verify(declaration, owner=f"Middleware {_middleware_name(middleware)} ({type(middleware).__name__})") + + def verify_declared_tool_view( middlewares: Sequence[object], *, diff --git a/backend/packages/harness/deerflow/subagents/AGENTS.md b/backend/packages/harness/deerflow/subagents/AGENTS.md index 145b48eb68..293d0d84cb 100644 --- a/backend/packages/harness/deerflow/subagents/AGENTS.md +++ b/backend/packages/harness/deerflow/subagents/AGENTS.md @@ -54,6 +54,16 @@ set, against an isolated PostgreSQL schema. They need no paid model. ## Isolation from the parent +- **Apply name policies to middleware tools after assembly**, even without global + authorization. Preserve shared middleware and allowed callables; require + Sequence declarations and reject one-shot iterators unconsumed. Discovery + helpers keep filtered catalogs; provider validation remains independent. +- **Audit immediately before `create_agent`.** Check explicit and middleware + tools, failing on unexpected names rather than silently re-filtering. Exempt + only actual generated discovery helpers in the explicit list, never name + matches or middleware tools. Reuse callable name conversion within this build; + recheck new or renamed tools. Errors identify `disallowed_tools` hits or missing + `tools` allowlist entries; denylists win. - **A subagent inherits scope, not state.** It gets the parent's identity, the user's skill storage, the knowledge execution scope and a deep copy of the uploaded-file list at dispatch. It does not share the parent's message history. diff --git a/backend/packages/harness/deerflow/subagents/executor.py b/backend/packages/harness/deerflow/subagents/executor.py index f9b7446285..dbc82ac598 100644 --- a/backend/packages/harness/deerflow/subagents/executor.py +++ b/backend/packages/harness/deerflow/subagents/executor.py @@ -1116,6 +1116,9 @@ class SubagentExecutor: self.assembly_descriptor: Any | None = None self._assembled_system_prompt = self.config.system_prompt or "" self._assembled_skills: list[Any] = [] + # Only these exact, build-generated instances receive the discovery + # exemption in the final name-policy audit; names alone are insufficient. + self._policy_discovery_tools: tuple[BaseTool, ...] = () logger.info(f"[trace={self.trace_id}] SubagentExecutor initialized: {config.name} with {len(self.tools)} tools") @@ -1268,6 +1271,14 @@ class SubagentExecutor: from deerflow.agents.lead_agent.agent import _insert_hitl_approval_middleware _insert_hitl_approval_middleware(middlewares, bound_tools, app_config, supports_resume=False, model_name=self.model_name) + # Extensions can add tools through middleware after the inherited list + # was filtered. Apply the same subagent policy to those declarations, + # independently of global authorization, without changing shared stacks + # or re-filtering generated discovery helpers in bound_tools. + from deerflow.agents.middlewares.tool_declarations import ToolPolicyNameResolver, apply_declared_tool_policy, verify_subagent_tool_policy + + policy_names = ToolPolicyNameResolver() + middlewares = apply_declared_tool_policy(middlewares, allowed=self.config.tools, disallowed=self.config.disallowed_tools, name_resolver=policy_names) # Collect every guard middleware that exposes ``consume_stop_reason`` # (TokenBudgetMiddleware, LoopDetectionMiddleware, # ModelLengthFinishReasonMiddleware) so _aexecute can read @@ -1280,6 +1291,14 @@ class SubagentExecutor: # system_prompt is included in initial state messages (see _build_initial_state) # to avoid multiple SystemMessages which some LLM APIs don't support. + verify_subagent_tool_policy( + bound_tools, + middlewares, + allowed=self.config.tools, + disallowed=self.config.disallowed_tools, + discovery_tools=getattr(self, "_policy_discovery_tools", ()), + name_resolver=policy_names, + ) agent = create_agent( model=model, tools=bound_tools, @@ -1607,6 +1626,7 @@ class SubagentExecutor: enabled=resolved_app_config.tool_search.enabled, ) final_tools.extend(late_tools) + self._policy_discovery_tools = tuple(helper for helper in (deferred_setup.tool_search_tool, skill_setup.describe_skill_tool) if helper is not None and any(helper is tool for tool in final_tools)) # Combine the system prompt and skill discovery metadata into a single # SystemMessage. Full SKILL.md bodies are loaded only when activated. diff --git a/backend/tests/test_skills_authorization.py b/backend/tests/test_skills_authorization.py index dc9f5d737d..4a1d1477ed 100644 --- a/backend/tests/test_skills_authorization.py +++ b/backend/tests/test_skills_authorization.py @@ -1079,7 +1079,7 @@ def test_subagent_executor_resolves_skill_authorization_for_chain(monkeypatch): executor = SubagentExecutor.__new__(SubagentExecutor) executor.input_gate = None - executor.config = SimpleNamespace(name="sub", skills=None, max_turns=25) + executor.config = SimpleNamespace(name="sub", skills=None, max_turns=25, tools=None, disallowed_tools=["task"]) executor.model_name = "m" executor.thinking_enabled = False executor.reasoning_effort = None @@ -1446,7 +1446,7 @@ def test_subagent_executor_shares_one_skill_authorization_instance(monkeypatch): executor = SubagentExecutor.__new__(SubagentExecutor) executor.input_gate = None - executor.config = SimpleNamespace(name="sub", skills=None, max_turns=25) + executor.config = SimpleNamespace(name="sub", skills=None, max_turns=25, tools=None, disallowed_tools=["task"]) executor.model_name = "m" executor.thinking_enabled = False executor.reasoning_effort = None diff --git a/backend/tests/test_subagent_declared_tool_policy.py b/backend/tests/test_subagent_declared_tool_policy.py new file mode 100644 index 0000000000..18d1fc73c4 --- /dev/null +++ b/backend/tests/test_subagent_declared_tool_policy.py @@ -0,0 +1,583 @@ +"""Subagent tool restrictions cover extension declarations in the real graph.""" + +import importlib.util +import sys +from collections import UserList +from pathlib import Path +from types import ModuleType + +import pytest +from deerflow_extension_api import AgentScope, MiddlewarePlacement, Placement, extension +from langchain.agents.middleware import AgentMiddleware +from langchain_core.language_models.fake_chat_models import GenericFakeChatModel +from langchain_core.messages import AIMessage +from langchain_core.tools import StructuredTool + +from deerflow.config.app_config import AppConfig +from deerflow.extensions.loader import ExtensionSpec, load_extensions +from deerflow.subagents.config import SubagentConfig + + +def _callable_without_description() -> str: + # Missing documentation is intentional: LangChain must reject this callable. + return "report" + + +@pytest.mark.parametrize( + "declaration", + [ + pytest.param("read_report", id="string"), + pytest.param(None, id="none"), + pytest.param(object(), id="non-callable"), + pytest.param(_callable_without_description, id="invalid-callable"), + ], +) +def test_invalid_declaration_reports_middleware_context(declaration): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError, apply_declared_tool_policy + + middleware = _middleware([declaration]) + declarations = middleware.tools + + with pytest.raises(DeclaredToolViewError) as raised: + apply_declared_tool_policy([middleware], allowed=None, disallowed=["task"]) + + assert middleware.name in str(raised.value) + assert "BaseTool" in str(raised.value) + assert "callable" in str(raised.value) + assert raised.value.__cause__ is not None + assert middleware.tools is declarations + assert middleware.tools[0] is declaration + + +@pytest.fixture +def executor_module(monkeypatch, tmp_path): + # Use the real module despite conftest's circular-import stub. + path = Path(__file__).parents[1] / "packages/harness/deerflow/subagents/executor.py" + spec = importlib.util.spec_from_file_location("_declared_policy_executor", path) + module = importlib.util.module_from_spec(spec) + monkeypatch.setitem(sys.modules, spec.name, module) + spec.loader.exec_module(module) + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path)) + yield module + module._shutdown_isolated_subagent_loop() + + +class RecordingModel(GenericFakeChatModel): + bound_tool_names: list[list[str]] = [] + + def bind_tools(self, tools, **kwargs): + self.bound_tool_names.append([tool.name for tool in tools]) + return self + + +def _load_extension(monkeypatch, middleware): + class Contributor: + def contribute_middlewares(self, app_store, context): + return [MiddlewarePlacement(middleware, Placement.TOOL_VISIBLE, AgentScope.SUBAGENT)] + + @extension(api="0.2.0", name="declared-policy-test") + def install(registry, config): + registry.middlewares(Contributor()) + + module = ModuleType("_declared_policy_extension") + module.install = install + monkeypatch.setitem(sys.modules, module.__name__, module) + loaded, diagnostics = load_extensions([ExtensionSpec(use=f"{module.__name__}:install", required=True)]) + assert not [item for item in diagnostics if item.level == "error"] + return loaded + + +def _middleware(tools): + class ReportMiddleware(AgentMiddleware): + pass + + middleware = ReportMiddleware() + middleware.tools = tools + return middleware + + +def _report_tool(name, executed): + def report() -> str: + """Return a synthetic report without external side effects.""" + executed.append(name) + return name + + return StructuredTool.from_function(report, name=name, return_direct=True) + + +def _app_config(): + return AppConfig.model_validate( + { + "models": [{"name": "offline", "use": "langchain_openai:ChatOpenAI", "model": "offline"}], + "sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider"}, + "summarization": {"enabled": False}, + } + ) + + +async def _run(executor_module, monkeypatch, extensions, config, responses, *, tools=(), app_config=None): + model = RecordingModel(messages=iter(responses)) + monkeypatch.setattr(executor_module, "create_chat_model", lambda **kwargs: model) + executor = executor_module.SubagentExecutor( + config=config, + tools=list(tools), + extensions=extensions, + app_config=app_config or _app_config(), + parent_model="offline", + thread_id="policy-thread", + user_id="policy-user", + ) + result = await executor._aexecute("Review the synthetic report") + assert result.status == executor_module.SubagentStatus.COMPLETED, result.error + return result, model + + +def _call(name, identifier): + return AIMessage(content="", tool_calls=[{"name": name, "args": {}, "id": identifier}]) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("container", [list, tuple, UserList], ids=["list", "tuple", "sequence"]) +@pytest.mark.parametrize( + "allowed, disallowed, denied_name", + [ + (["read_report"], ["task"], "publish_report"), + (None, ["task", "publish_report"], "publish_report"), + (["read_report", "publish_report"], ["publish_report"], "publish_report"), + (None, ["task"], "task"), + ], + ids=["allowlist", "denylist", "deny-wins", "default-task-denial"], +) +async def test_restricted_extension_tool_cannot_execute(executor_module, monkeypatch, allowed, disallowed, denied_name, container): + executed = [] + declarations = container([_report_tool(denied_name, executed), _report_tool("read_report", executed)]) + middleware = _middleware(declarations) + extensions = _load_extension(monkeypatch, middleware) + config = SubagentConfig(name="reviewer", description="Read reports", skills=[], tools=allowed, disallowed_tools=disallowed) + + result, model = await _run(executor_module, monkeypatch, extensions, config, [_call(denied_name, "denied-call"), _call("read_report", "allowed-call")]) + + assert executed == ["read_report"] + assert result.result == "read_report" + assert model.bound_tool_names + assert all(denied_name not in names and "read_report" in names for names in model.bound_tool_names) + # The extension may reuse this same middleware for another subagent. + assert middleware.tools is declarations + assert [tool.name for tool in declarations] == [denied_name, "read_report"] + + +@pytest.mark.asyncio +async def test_empty_allowlist_excludes_all_extension_declarations(executor_module, monkeypatch): + executed = [] + extensions = _load_extension(monkeypatch, _middleware([_report_tool("publish_report", executed)])) + config = SubagentConfig(name="reviewer", description="No tools", skills=[], tools=[]) + + result, model = await _run(executor_module, monkeypatch, extensions, config, [AIMessage(content="Reviewed without tools")]) + + assert result.result == "Reviewed without tools" + assert not any(model.bound_tool_names) + assert executed == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("container", [list, tuple, UserList], ids=["list", "tuple", "sequence"]) +async def test_shared_extension_can_publish_for_an_unrestricted_subagent(executor_module, monkeypatch, container): + executed = [] + declarations = container([_report_tool("publish_report", executed), _report_tool("read_report", executed)]) + middleware = _middleware(declarations) + extensions = _load_extension(monkeypatch, middleware) + restricted = SubagentConfig(name="reviewer", description="Read reports", skills=[], tools=["read_report"]) + await _run(executor_module, monkeypatch, extensions, restricted, [_call("read_report", "read-call")]) + unrestricted = SubagentConfig(name="publisher", description="Publish reports", skills=[], tools=None, disallowed_tools=[]) + + result, model = await _run(executor_module, monkeypatch, extensions, unrestricted, [_call("publish_report", "publish-call")]) + + assert result.result == "publish_report" + assert executed == ["read_report", "publish_report"] + assert "publish_report" in model.bound_tool_names[0] + assert middleware.tools is declarations + + +@pytest.mark.asyncio +@pytest.mark.parametrize("allowed", [["read_report"], ["read_report", "publish_report"]], ids=["callable-denied", "callable-allowed"]) +async def test_callable_declarations_keep_their_names_and_shared_identity(executor_module, monkeypatch, allowed): + executed = [] + + def publish_report() -> str: + """Publish a synthetic report.""" + executed.append("publish_report") + return "published" + + declarations = [publish_report, _report_tool("read_report", executed)] + middleware = _middleware(declarations) + extensions = _load_extension(monkeypatch, middleware) + config = SubagentConfig(name="reviewer", description="Read reports", skills=[], tools=allowed) + permitted = "publish_report" in allowed + responses = [_call("publish_report", "publish-call"), _call("read_report", "read-call")] + + result, model = await _run(executor_module, monkeypatch, extensions, config, responses) + + assert result.result == "read_report" + assert executed == (["publish_report", "read_report"] if permitted else ["read_report"]) + assert all(("publish_report" in names) is permitted for names in model.bound_tool_names) + assert middleware.tools is declarations + assert declarations[0] is publish_report + + +@pytest.mark.asyncio +async def test_subagent_policy_intersects_global_authorization(executor_module, monkeypatch): + executed = [] + declarations = [_report_tool(name, executed) for name in ("publish_report", "archive_report", "read_report")] + extensions = _load_extension(monkeypatch, _middleware(declarations)) + config = SubagentConfig(name="reviewer", description="Read reports", skills=[], disallowed_tools=["publish_report"]) + app_config = _app_config() + app_config.authorization = type(app_config.authorization).model_validate( + { + "enabled": True, + "default_role": "user", + "provider": { + "use": "deerflow.authz.rbac:RbacAuthorizationProvider", + "config": {"roles": {"user": {"tools": {"allow": ["publish_report", "read_report"]}}}}, + }, + } + ) + + result, model = await _run( + executor_module, + monkeypatch, + extensions, + config, + [_call("publish_report", "config-denied"), _call("archive_report", "provider-denied"), _call("read_report", "both-allow")], + app_config=app_config, + ) + + assert result.result == "read_report" + assert executed == ["read_report"] + assert all(names == ["read_report"] for names in model.bound_tool_names) + + +@pytest.mark.asyncio +async def test_generated_discovery_helper_retains_its_filtered_catalog(executor_module, monkeypatch): + from deerflow.tools.mcp_metadata import tag_mcp_tool + + executed = [] + mcp_tool = tag_mcp_tool(_report_tool("mcp_report", executed)) + extensions = _load_extension(monkeypatch, _middleware([_report_tool("publish_report", executed)])) + config = SubagentConfig(name="reviewer", description="Read MCP reports", skills=[], tools=["mcp_report"], disallowed_tools=["task", "tool_search"]) + app_config = _app_config() + app_config.tool_search.enabled = True + discover = AIMessage(content="", tool_calls=[{"name": "tool_search", "args": {"query": "select:mcp_report"}, "id": "discover"}]) + + result, model = await _run( + executor_module, + monkeypatch, + extensions, + config, + [discover, _call("mcp_report", "read-call")], + tools=[mcp_tool], + app_config=app_config, + ) + + assert result.result == "mcp_report" + assert executed == ["mcp_report"] + assert "tool_search" in model.bound_tool_names[0] + assert "mcp_report" not in model.bound_tool_names[0] + assert "mcp_report" in model.bound_tool_names[-1] + assert all("publish_report" not in names for names in model.bound_tool_names) + + +def test_unrestricted_policy_does_not_inspect_middleware_declarations(): + from deerflow.agents.middlewares.tool_declarations import apply_declared_tool_policy + + class UninspectedMiddleware(AgentMiddleware): + @property + def tools(self): + raise AssertionError("An unrestricted build must not read this property") + + stack = [UninspectedMiddleware()] + + assert apply_declared_tool_policy(stack, allowed=None, disallowed=[]) is stack + + +@pytest.mark.parametrize("container", [list, UserList], ids=["list", "sequence"]) +def test_allowed_callable_property_keeps_the_original_middleware(container): + from deerflow.agents.middlewares.tool_declarations import apply_declared_tool_policy + + def read_report() -> str: + """Return a synthetic report.""" + return "report" + + declarations = container([read_report]) + + class PropertyMiddleware(AgentMiddleware): + @property + def tools(self): + return declarations + + original = PropertyMiddleware() + + stack = [original] + view = apply_declared_tool_policy(stack, allowed=["read_report"], disallowed=["task"]) + + assert view is stack + assert view[0] is original + assert original.tools is declarations + assert original.tools[0] is read_report + + +def test_restricted_policy_rejects_iterator_without_consuming_shared_declarations(): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError, apply_declared_tool_policy + + tool = _report_tool("read_report", []) + declarations = iter([tool]) + original = _middleware(declarations) + + with pytest.raises(DeclaredToolViewError, match="Sequence"): + apply_declared_tool_policy([original], allowed=None, disallowed=["task"]) + + assert original.tools is declarations + assert next(declarations) is tool + + +def _audit_executor(executor_module, monkeypatch, config, *, tools=(), extensions=None, app_config=None): + model = RecordingModel(messages=iter([AIMessage(content="Reviewed")])) + monkeypatch.setattr(executor_module, "create_chat_model", lambda **kwargs: model) + executor = executor_module.SubagentExecutor(config=config, tools=list(tools), extensions=extensions, app_config=app_config or _app_config(), parent_model="offline", thread_id="audit-thread", user_id="audit-user") + builds = [] + real_create_agent = executor_module.create_agent + + def record_build(**kwargs): + builds.append(kwargs) + return real_create_agent(**kwargs) + + monkeypatch.setattr(executor_module, "create_agent", record_build) + return executor, builds + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "allowed, disallowed, denied_name", + [(["read_report"], ["task"], "publish_report"), (None, ["publish_report"], "publish_report"), (None, ["task"], "task")], + ids=["allowlist", "denylist", "default-task-denial"], +) +async def test_final_audit_rejects_explicit_tool_injection(executor_module, monkeypatch, allowed, disallowed, denied_name): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError + + config = SubagentConfig(name="reviewer", description="Audit tools", skills=[], tools=allowed, disallowed_tools=disallowed) + read = _report_tool("read_report", []) + executor, builds = _audit_executor(executor_module, monkeypatch, config, tools=[read]) + + # An assembly caller supplies a new explicit list after constructor filtering. + with pytest.raises(DeclaredToolViewError, match=denied_name): + await executor._create_agent(tools=[read, _report_tool(denied_name, [])]) + + assert builds == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("denied_name", ["publish_report", "tool_search"]) +async def test_final_audit_rejects_late_middleware_declarations(executor_module, monkeypatch, denied_name): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError + + config = SubagentConfig(name="reviewer", description="Audit tools", skills=[], tools=["read_report"], disallowed_tools=["task", "tool_search"]) + executor, builds = _audit_executor(executor_module, monkeypatch, config) + real_resolve = executor._resolve_recursion_limit + injections = [] + + def introduce_late_declaration(middlewares): + # Fault injection: simulate a later assembly pass adding a new tool + # channel after the existing declaration filter has already run. + middleware = _middleware([_report_tool(denied_name, [])]) + middlewares.append(middleware) + injections.append(middleware) + return real_resolve(middlewares) + + monkeypatch.setattr(executor, "_resolve_recursion_limit", introduce_late_declaration) + + with pytest.raises(DeclaredToolViewError, match=denied_name): + await executor._create_agent() + + assert len(injections) == 1 + assert builds == [] + + +@pytest.mark.asyncio +async def test_final_audit_does_not_exempt_a_tool_search_lookalike(executor_module, monkeypatch): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError + from deerflow.tools.mcp_metadata import tag_mcp_tool + + config = SubagentConfig(name="reviewer", description="Audit discovery", skills=[], tools=["mcp_report"], disallowed_tools=["task", "tool_search"]) + app_config = _app_config() + app_config.tool_search.enabled = True + executor, builds = _audit_executor(executor_module, monkeypatch, config, tools=[tag_mcp_tool(_report_tool("mcp_report", []))], app_config=app_config) + _, _, setup = await executor._build_initial_state("Review reports") + lookalike = _report_tool("tool_search", []) + assert setup.tool_search_tool is not None and setup.tool_search_tool is not lookalike + + with pytest.raises(DeclaredToolViewError, match="tool_search"): + await executor._create_agent(tools=[lookalike], deferred_setup=setup) + + assert builds == [] + + +@pytest.mark.asyncio +async def test_generated_skill_discovery_survives_final_audit(executor_module, monkeypatch, tmp_path): + from deerflow.skills.types import Skill, SkillCategory + + skill = Skill(name="demo", description="Demo skill", license=None, skill_dir=tmp_path, skill_file=tmp_path / "SKILL.md", relative_path=Path("demo"), category=SkillCategory.PUBLIC, enabled=True) + + class StaticSkillStorage: + def load_skills(self, **kwargs): + return [skill] + + monkeypatch.setattr("deerflow.skills.storage.get_or_new_user_skill_storage", lambda *args, **kwargs: StaticSkillStorage()) + config = SubagentConfig(name="reviewer", description="Audit discovery", skills=["demo"], tools=[], disallowed_tools=["task", "describe_skill"]) + app_config = _app_config() + app_config.skills.deferred_discovery = True + executor, builds = _audit_executor(executor_module, monkeypatch, config, app_config=app_config) + _, tools, setup = await executor._build_initial_state("Review skills") + assert [tool.name for tool in tools] == ["describe_skill"] + + await executor._create_agent(tools=tools, deferred_setup=setup) + + assert len(builds) == 1 + assert builds[0]["tools"][0] is tools[0] + + +def test_final_audit_is_a_noop_for_an_unrestricted_policy(): + from deerflow.agents.middlewares.tool_declarations import verify_subagent_tool_policy + + class UninspectedMiddleware: + @property + def tools(self): + raise AssertionError("Unrestricted builds must not inspect declarations") + + verify_subagent_tool_policy([None], [UninspectedMiddleware()], allowed=None, disallowed=[]) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("container", [list, tuple, UserList], ids=["list", "tuple", "sequence"]) +async def test_callable_names_are_resolved_once_per_build(executor_module, monkeypatch, container): + from deerflow.agents.middlewares import tool_declarations + + executed = [] + + def read_report() -> str: + """Read a synthetic report.""" + executed.append("callable") + return "report" + + def publish_report() -> str: + """Publish a synthetic report.""" + executed.append("denied") + return "published" + + declarations = container([read_report, publish_report]) + middleware = _middleware(declarations) + extensions = _load_extension(monkeypatch, middleware) + config = SubagentConfig(name="reviewer", description="Read reports", skills=[], tools=["read_report", "finish_report"]) + converted = [] + real_create_tool = tool_declarations.create_tool + + def record_conversion(declaration): + converted.append(declaration) + return real_create_tool(declaration) + + monkeypatch.setattr(tool_declarations, "create_tool", record_conversion) + for build in range(2): + converted.clear() + await _run( + executor_module, + monkeypatch, + extensions, + config, + [_call("read_report", f"read-{build}"), _call("finish_report", f"finish-{build}")], + tools=[_report_tool("finish_report", executed)], + ) + assert converted.count(read_report) == 1 + assert converted.count(publish_report) == 1 + assert middleware.tools is declarations + assert declarations[0] is read_report + assert executed == ["callable", "finish_report", "callable", "finish_report"] + + +@pytest.mark.asyncio +async def test_final_audit_resolves_a_new_callable_after_narrowing(executor_module, monkeypatch): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError + + def publish_report() -> str: + """Publish a synthetic report.""" + return "published" + + config = SubagentConfig(name="reviewer", description="Read reports", skills=[], tools=["read_report"]) + executor, builds = _audit_executor(executor_module, monkeypatch, config) + real_resolve = executor._resolve_recursion_limit + injections = [] + + def inject_callable(middlewares): + injections.append(publish_report) + middlewares.append(_middleware([publish_report])) + return real_resolve(middlewares) + + monkeypatch.setattr(executor, "_resolve_recursion_limit", inject_callable) + with pytest.raises(DeclaredToolViewError, match="publish_report"): + await executor._create_agent() + assert injections == [publish_report] + assert builds == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind", ["callable", "base-tool"]) +async def test_final_audit_rechecks_a_renamed_declaration(executor_module, monkeypatch, kind): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError + + def read_report() -> str: + """Read a synthetic report.""" + return "report" + + declaration = read_report if kind == "callable" else _report_tool("read_report", []) + extensions = _load_extension(monkeypatch, _middleware([declaration])) + config = SubagentConfig(name="reviewer", description="Read reports", skills=[], tools=["read_report"]) + executor, builds = _audit_executor(executor_module, monkeypatch, config, extensions=extensions) + real_resolve = executor._resolve_recursion_limit + mutations = [] + + def rename_declaration(middlewares): + # Extension/schema normalization may copy declarations. Mutate the + # actual final stack rather than the contributor's original object. + name_attribute = "__name__" if kind == "callable" else "name" + live_declaration = next(tool for middleware in middlewares for tool in getattr(middleware, "tools", ()) if getattr(tool, name_attribute, None) == "read_report") + mutations.append(live_declaration) + setattr(live_declaration, name_attribute, "publish_report") + return real_resolve(middlewares) + + monkeypatch.setattr(executor, "_resolve_recursion_limit", rename_declaration) + with pytest.raises(DeclaredToolViewError, match="publish_report"): + await executor._create_agent(extensions=extensions) + assert len(mutations) == 1 + assert builds == [] + + +@pytest.mark.parametrize("source", ["explicit", "middleware"]) +@pytest.mark.parametrize( + "allowed, disallowed, reason", + [ + (["read_report"], [], "missing from the tools allowlist"), + (None, ["publish_report"], "listed in disallowed_tools"), + (["read_report"], ["publish_report"], "listed in disallowed_tools"), + ], + ids=["allowlist-miss", "denylist-hit", "deny-precedence"], +) +def test_final_audit_reports_the_policy_check_that_failed(source, allowed, disallowed, reason): + from deerflow.agents.middlewares.tool_declarations import DeclaredToolViewError, verify_subagent_tool_policy + + declaration = _report_tool("publish_report", []) + middleware = _middleware([declaration]) + tools, middlewares = ([declaration], []) if source == "explicit" else ([], [middleware]) + with pytest.raises(DeclaredToolViewError) as raised: + verify_subagent_tool_policy(tools, middlewares, allowed=allowed, disallowed=disallowed) + message = str(raised.value) + assert "publish_report" in message + assert reason in message + assert ("Subagent explicit tool list" if source == "explicit" else middleware.name) in message + assert "before graph creation" in message diff --git a/docs/guide/subagents.md b/docs/guide/subagents.md index de28feb615..8dab8dcad1 100644 --- a/docs/guide/subagents.md +++ b/docs/guide/subagents.md @@ -263,3 +263,22 @@ When Gateway workers share file storage, concurrent managed-subagent creates for Native workers support per-subagent `thinking_enabled` and `reasoning_effort` through YAML configuration and the managed-subagent API. Unconfigured workers keep the existing thinking-off request; the selected model's required-thinking policy, supported effort values and aliases still apply. These settings do not inherit the lead agent's thinking preferences. For example, independent read-only research can run concurrently when the wall-clock savings outweigh duplicated discovery and synthesis cost, while a repository refactor with shared files and sequential test feedback remains with the lead agent. When `max_concurrent_subagents` is `1`, parallel and multi-batch routing guidance is disabled; delegation remains available only for material specialist or context-isolation benefit. + +## Tool restrictions + +Subagent `tools` allowlists and `disallowed_tools` denylists apply to middleware-declared tools as well as inherited tools, even when global authorization is disabled. Denylists take precedence; an empty allowlist excludes all middleware declarations. + +Framework-generated discovery helpers (`tool_search` and `describe_skill`) are +an exception to these name-level allowlists and denylists. Adding either name to +`disallowed_tools` does not remove the generated helper. Each helper's catalog +remains limited to admitted tools or skills. + +Before creating the agent graph, a final check verifies the explicit tool list +and middleware declarations against the subagent's name policy. A tool added +after the earlier filtering passes stops the build if it violates that policy. +The error identifies whether the tool is listed in `disallowed_tools` or missing +from the `tools` allowlist, so the corresponding configuration can be corrected. +Discovery exemptions apply only to the actual framework-generated helper +instances; an ordinary tool with the same name still follows the name policy. + +Middleware tool declarations follow LangChain's `Sequence` contract, including lists and tuples. Restricted subagents reject non-sequence containers such as sets and generators before consuming them; the default `disallowed_tools=["task"]` also counts as a restriction. Global authorization retains its existing list/tuple declaration requirement. Extensions remain trusted operator code; tool configuration is not an isolation boundary for extension code.