From deff3795b4c813488f58fd3bb7326e631e6fafc7 Mon Sep 17 00:00:00 2001 From: jxxghp Date: Wed, 9 Sep 2026 21:03:48 +0800 Subject: [PATCH] fix(agent): enforce read-only subagent tool actions --- app/agent/middleware/policy.py | 49 +++++ app/agent/middleware/subagents.py | 3 +- docs/agent.md | 2 + docs/architecture-overview.md | 2 +- docs/architecture/optimization-checklist.md | 2 +- .../architecture/dependency-baseline.json | 7 +- tests/test_agent_subagent_permissions.py | 185 ++++++++++++++++++ 7 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 tests/test_agent_subagent_permissions.py diff --git a/app/agent/middleware/policy.py b/app/agent/middleware/policy.py index 5ac1cc91a..5b992c1de 100644 --- a/app/agent/middleware/policy.py +++ b/app/agent/middleware/policy.py @@ -1,13 +1,16 @@ """LangChain 工具调用的 MoviePilot 宿主策略中间件。""" import asyncio +import json from collections.abc import Awaitable, Callable from typing import Any from langchain.agents.middleware import AgentMiddleware, ToolCallRequest, hook_config from langchain_core.messages import AIMessage, ToolMessage +from app.agent.policy.api import resolve_api_operation from app.agent.policy.contracts import ( + ActionEffect, ExecutionOutcome, ExecutionReceipt, ToolOrigin, @@ -22,11 +25,19 @@ from app.agent.policy.registry import requests_system_setting_secrets from app.agent.policy.sanitizer import stable_type_name from app.agent.tools.catalog import ToolCatalogSnapshot from app.agent.tools.impl.api import MoviePilotApiTool +from app.agent.tools.impl.mcp import McpExternalTool from app.agent.tools.result import EXECUTION_OUTCOME_KEY, ToolExecutionError, annotate_tool_result +from app.agent.tools.tags import ToolTag POLICY_DENIED_MESSAGE = "当前宿主策略不允许执行该工具。" POLICY_UNAVAILABLE_MESSAGE = "宿主策略暂时不可用，未执行该工具。" TOOL_TIMEOUT_MESSAGE = "工具执行超时，已停止等待结果；若工具包含外部写操作，操作可能仍在继续，请先确认实际状态再重试。" +SUBAGENT_READ_ACTIONS = { + "execute_command": frozenset({"read", "wait"}), + "browse_webpage": frozenset({"snapshot", "get_content", "screenshot", "wait", "list_tabs"}), + "persona": frozenset({"list"}), + "agent_task": frozenset({"list"}), +} # follow_imports=skip 下只在第三方中间件基类和装饰器边界忽略 misc。 @@ -35,6 +46,7 @@ class AgentPolicyMiddleware(AgentMiddleware): # type: ignore[misc] 模型供应商原生 server tools 在供应商侧执行，不经过本地 middleware， 因而不具备这里生成的 start/finish/fail 回执。 + 只读子代理的动作范围由宿主强制执行，不受主代理兼容观测模式影响。 """ def __init__( @@ -151,6 +163,17 @@ class AgentPolicyMiddleware(AgentMiddleware): # type: ignore[misc] enforce_decision: bool = True, ) -> tuple[bool, Any]: """执行一次本地工具调用，并复用 ToolNode 的策略生命周期。""" + if self.context.origin is ToolOrigin.SUBAGENT and not self._subagent_read_allowed(tool, arguments): + return False, ToolMessage( + content=json.dumps({ + "success": False, "error": "subagent_read_only", + "message": "子代理只允许已声明的只读操作；请由主代理核验并执行已获授权的写入。", + }, ensure_ascii=False), + tool_call_id=str(invocation_id or ""), + name=str(getattr(tool, "name", None) or "unknown"), + status="error", + additional_kwargs={EXECUTION_OUTCOME_KEY: ExecutionOutcome.FAILED.value}, + ) observation = call_policy_hook( "start", self.orchestrator.start, @@ -195,6 +218,32 @@ class AgentPolicyMiddleware(AgentMiddleware): # type: ignore[misc] ) return True, result + @staticmethod + def _subagent_read_allowed(tool: Any, arguments: dict[str, Any]) -> bool: + """按真实 API 操作限制子代理，MCP 的兼容 Read 标签不构成只读证明。""" + if isinstance(tool, MoviePilotApiTool): + operation = resolve_api_operation(str(arguments.get("operation_id") or "")) + if operation is None or operation.effect is not ActionEffect.SAFE_READ: + return False + try: + normalized = tool.canonical_arguments(arguments) + except (TypeError, ValueError): + return False + return not requests_system_setting_secrets(normalized) + if isinstance(tool, McpExternalTool): + return False + name = getattr(tool, "name", None) + if name in SUBAGENT_READ_ACTIONS: + action = arguments.get("action") + return bool( + isinstance(action, str) and action in SUBAGENT_READ_ACTIONS[name] + and not arguments.get("cookies") and not arguments.get("user_agent") + ) + tags = set(getattr(tool, "tags", None) or []) + return ToolTag.Read in tags and not tags.intersection({ + ToolTag.Write, ToolTag.Message, ToolTag.UserInteraction, + }) + __all__ = [ "AgentPolicyMiddleware", diff --git a/app/agent/middleware/subagents.py b/app/agent/middleware/subagents.py index a238e7320..e7dc2fd63 100644 --- a/app/agent/middleware/subagents.py +++ b/app/agent/middleware/subagents.py @@ -79,6 +79,7 @@ Rules: - Subagents must not send messages to the user, ask for interaction, or reveal their internal tool activity. - Give the user only your synthesized final answer and the minimum necessary next step. - If a task requires configuration changes, deletion, adding downloads, adding subscriptions, or any high-impact action, the main agent must handle it directly under the confirmation policy. +- Child tools enforce read-only operations. Perform command launches, browser navigation/interactions, and external MCP calls in the main agent; pass the resulting evidence to a child for analysis when useful. """ SUBAGENT_TASK_DESCRIPTION = ( @@ -103,7 +104,7 @@ Requirements: - Handle only the delegated subtask from the main agent. Do not converse with the user. - Do not send messages, request user interaction, or output progress updates. - Use tool results only for analysis, and return the final result only to the main agent. -- Unless the task explicitly requires it and your tool set permits it, limit yourself to read-only inspection and diagnosis. +- Limit yourself to the read-only operations permitted by the host. Command launches, browser navigation/interactions, external MCP calls, and writes must be handled by the main agent; request the resulting evidence instead of bypassing a denial. - If user confirmation or a high-impact change is needed, explain why the main agent must confirm it instead of executing it yourself. - Return a concise structured Chinese result with key evidence, judgment, and recommended next step. """ diff --git a/docs/agent.md b/docs/agent.md index 759760322..2e9208686 100644 --- a/docs/agent.md +++ b/docs/agent.md @@ -51,6 +51,8 @@ MoviePilot Agent 通过模型、Skills、工具和会话状态共同完成任务 ## 子代理连续工作 +默认子代理在宿主执行边界强制只读。多功能 API 网关按具体 `operation_id` 判断副作用，继承管理员身份也不能修改、删除、触发下载或服务操作；敏感设置原值读取同样不开放。终端仅允许读取/等待已有输出；浏览器仅允许观察已有页面，不允许导航、输入、点击、脚本、关闭或修改 Cookie。外部 MCP 的兼容 Read 标签没有动作级保证，需交由主代理处理。正常业务查询和声明只读的本地工具仍可使用。 + 一轮结束会回收未完成的子任务，正常收敛后同一会话下一轮仍可委派。会话关闭时永久停止接收子任务；无法在等待上限内收敛的任务仍由原 owner 持有并封闭新提交。 子代理沿用主 Agent 的最终请求预算和上下文压缩机制。长调查可以压缩旧消息；最新单条工具事务或固定提示本身超过模型窗口时仍会明确失败，不会静默删除关键内容。 diff --git a/docs/architecture-overview.md b/docs/architecture-overview.md index a2a1f04ba..35a9d3ebe 100644 --- a/docs/architecture-overview.md +++ b/docs/architecture-overview.md @@ -757,7 +757,7 @@ flowchart LR | 指标 | 当前值 | |---|---:| | Python 模块 | 995 | -| 内部导入边 | 8,455 | +| 内部导入边 | 8,458 | | 非平凡 SCC | 1（精确 containment 的 TMDB 移植包环） | | Application / Chain 具体 Adapter 直连 | 0 / 0 | | Direct egress | 53（债务已清零，53 条精确 containment） | diff --git a/docs/architecture/optimization-checklist.md b/docs/architecture/optimization-checklist.md index b29b040c1..817e92aad 100644 --- a/docs/architecture/optimization-checklist.md +++ b/docs/architecture/optimization-checklist.md @@ -94,7 +94,7 @@ ARCH-201 至 ARCH-204 均达到实现、验证、提交、推送和远端门禁 | 指标 | 当前值 | 解释 | |---|---:|---| -| 宿主 Python 模块 / 内部依赖边 | 995 / 8,455 | `dependency-baseline.json` 当前快照；分类、下载资源归类、订阅搜索、整理恢复与 Agent 计划模块的受控依赖 | +| 宿主 Python 模块 / 内部依赖边 | 995 / 8,458 | `dependency-baseline.json` 当前快照；分类、下载资源归类、订阅搜索、整理恢复与 Agent 计划模块的受控依赖 | | 非平凡 SCC | 1 | 仅保留精确 containment 的 29 模块 TMDB 移植包环 | | 跨层 DB 边界债务 | 0 | Application、Chain、API、Agent、Runtime、Workflow 到 DB 的受控债务均为零 | | Model/Oper 事务债务 | 0 | 自建 Session、自动事务装饰器、直接 commit/rollback 等基线均为零 | diff --git a/tests/fixtures/architecture/dependency-baseline.json b/tests/fixtures/architecture/dependency-baseline.json index b464f3015..eb3aac220 100644 --- a/tests/fixtures/architecture/dependency-baseline.json +++ b/tests/fixtures/architecture/dependency-baseline.json @@ -1074,8 +1074,8 @@ "runtime_only": true } }, - "edge_count": 8455, - "edge_sha256": "d989cabfc34fcff77f417e01f671915adb188f00d7beb00deb82fdff2d135bdf", + "edge_count": 8458, + "edge_sha256": "77a25667756a4b63d8487ac44252d77662044f1ea74d7c77dc0e4383a22d5f10", "edges": [ "app -> app.foundation", "app -> app.foundation.environment", @@ -1492,6 +1492,7 @@ "app.agent.middleware.plan -> app.agent.tools.tags", "app.agent.middleware.policy -> app.agent", "app.agent.middleware.policy -> app.agent.policy", + "app.agent.middleware.policy -> app.agent.policy.api", "app.agent.middleware.policy -> app.agent.policy.contracts", "app.agent.middleware.policy -> app.agent.policy.orchestrator", "app.agent.middleware.policy -> app.agent.policy.registry", @@ -1500,7 +1501,9 @@ "app.agent.middleware.policy -> app.agent.tools.catalog", "app.agent.middleware.policy -> app.agent.tools.impl", "app.agent.middleware.policy -> app.agent.tools.impl.api", + "app.agent.middleware.policy -> app.agent.tools.impl.mcp", "app.agent.middleware.policy -> app.agent.tools.result", + "app.agent.middleware.policy -> app.agent.tools.tags", "app.agent.middleware.selection -> app.agent", "app.agent.middleware.selection -> app.agent.llm", "app.agent.middleware.selection -> app.agent.llm.helper", diff --git a/tests/test_agent_subagent_permissions.py b/tests/test_agent_subagent_permissions.py new file mode 100644 index 000000000..9aafca495 --- /dev/null +++ b/tests/test_agent_subagent_permissions.py @@ -0,0 +1,185 @@ +"""以实际执行边界验证只读子代理不会获得父任务的写权限。""" + +import json +from types import SimpleNamespace +from unittest.mock import AsyncMock + +import pytest +from langchain.agents import create_agent +from langchain_core.language_models.fake_chat_models import FakeMessagesListChatModel +from langchain_core.messages import AIMessage, HumanMessage, ToolMessage + +from app.agent.mcp import AgentMcpToolSpec +from app.agent.middleware.policy import AgentPolicyMiddleware +from app.agent.middleware.subagents import _builtin_subagent_profiles, _SubAgentAgentProvider +from app.agent.policy.api import API_OPERATION_SPECS +from app.agent.policy.contracts import ActionEffect, AuthSource, PrincipalType, ToolOrigin, ToolPolicyContext +from app.agent.tools.impl.api import MoviePilotApiTool +from app.agent.tools.impl.mcp import McpExternalTool +from app.schemas.agent import AgentMcpServerConfig + + +class _Model(FakeMessagesListChatModel): + """脚本模型只用于向生产子图提交不应执行的动作，不作为智能评分。""" + + def bind_tools(self, tools, **kwargs): + """保留真实工具执行图，模型响应完全离线。""" + return self + + +def _context(admin=True, origin=ToolOrigin.SUBAGENT): + """创建不可由工具参数改写的宿主身份。""" + return ToolPolicyContext( + session_id="readonly", user_id="owner", origin=origin, + principal_type=PrincipalType.SUBAGENT if origin is ToolOrigin.SUBAGENT else PrincipalType.HUMAN, + auth_source=AuthSource.INTERNAL, agent_context={"is_admin": admin}, + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("admin", [False, True]) +@pytest.mark.parametrize("operation_id", [ + spec.operation_id for spec in API_OPERATION_SPECS + if spec.effect not in {ActionEffect.SAFE_READ, ActionEffect.SENSITIVE_READ} +]) +async def test_every_write_operation_is_denied_before_handler(admin, operation_id): + """全部登记的副作用操作均在 handler 前拒绝，普通角色也不能绕过。""" + gateway = MoviePilotApiTool(session_id="readonly", user_id="owner") + handler = AsyncMock(return_value="unexpected write") + middleware = AgentPolicyMiddleware(context=_context(admin)) + allowed, result = await middleware.execute_tool_call( + tool=gateway, arguments={"operation_id": operation_id}, handler=handler, + invocation_id="write", enforce_decision=False, + ) + assert allowed is False + assert result.status == "error" + assert json.loads(result.content)["error"] == "subagent_read_only" + handler.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_default_child_graph_denies_write_then_executes_valid_read(): + """真实默认子图必须零写入，并保留后续正常查询能力。""" + executor = AsyncMock() + executor.execute.return_value = '{"success":true,"data":[]}' + gateway = MoviePilotApiTool(session_id="readonly", user_id="owner", executor=executor) + gateway.set_agent_context({"is_admin": True}) + model = _Model(responses=[ + AIMessage(content="", tool_calls=[{ + "id": "write", "name": "moviepilot_api", "args": { + "operation_id": "config.system.update", "body": {"setting_key": "LLM_MAX_ITERATIONS", "value": 128}, + }, + }]), + AIMessage(content="", tool_calls=[{ + "id": "read", "name": "moviepilot_api", "args": {"operation_id": "site.list"}, + }]), + AIMessage(content="已读取站点状态，未修改设置"), + ]) + provider = _SubAgentAgentProvider(model=model, profiles=_builtin_subagent_profiles(), tools=[gateway]) + result = await provider.run_task(description="只读检查站点", subagent_type="general-purpose") + assert result == "已读取站点状态，未修改设置" + assert [call.args[0] for call in executor.execute.await_args_list] == ["site.list"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("show_secrets", [True, "true", "1", 1]) +async def test_child_secret_read_uses_canonical_query_semantics(show_secrets): + """不能利用布尔字符串或数字的 HTTP 转换请求原始敏感设置。""" + gateway = MoviePilotApiTool(session_id="readonly", user_id="owner") + handler = AsyncMock() + allowed, result = await AgentPolicyMiddleware(context=_context()).execute_tool_call( + tool=gateway, arguments={"operation_id": "config.system.get", "query": {"show_secrets": show_secrets}}, + handler=handler, invocation_id="secret", enforce_decision=False, + ) + assert allowed is False + assert result.status == "error" + handler.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_mcp_compatibility_read_tag_cannot_authorize_child_operation(): + """外部 MCP 的固定 Read 标签没有动作级语义，不能给只读子代理授权。""" + spec = AgentMcpToolSpec( + server=AgentMcpServerConfig(id="remote", name="Remote", transport="stdio", command="unused"), + name="erase", agent_tool_name="remote_erase", description="External operation", + input_schema={"type": "object"}, + ) + tool = McpExternalTool(spec, session_id="readonly", user_id="owner") + handler = AsyncMock() + allowed, result = await AgentPolicyMiddleware(context=_context()).execute_tool_call( + tool=tool, arguments={}, handler=handler, invocation_id="mcp", enforce_decision=False, + ) + assert allowed is False and result.status == "error" + handler.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("name,action,allowed", [ + ("execute_command", action, action in {"read", "wait"}) + for action in ("start", "run", "read", "wait", "write", "kill") +] + [ + ("browse_webpage", action, action in {"snapshot", "get_content", "screenshot", "wait", "list_tabs"}) + for action in ("goto", "snapshot", "get_content", "screenshot", "click", "click_ref", "fill", "fill_ref", + "select", "select_ref", "evaluate", "wait", "list_tabs", "open_tab", "focus_tab", "close_tab", "close_session") +] + [("persona", action, action == "list") for action in ("list", "switch", "update")] + + [("agent_task", action, action == "list") for action in ("list", "create", "update", "run", "delete")]) +async def test_mixed_tools_are_checked_by_action_despite_read_tag(name, action, allowed): + """终端和浏览器的 Read 标签不能给输入、脚本、导航或关闭动作授权。""" + tool = SimpleNamespace(name=name, tags=["read"]) + handler = AsyncMock(return_value="read result") + permitted, result = await AgentPolicyMiddleware(context=_context()).execute_tool_call( + tool=tool, arguments={"action": action}, handler=handler, + invocation_id="mixed", enforce_decision=False, + ) + assert permitted is allowed + assert handler.await_count == int(allowed) + if not allowed: + assert result.status == "error" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("extra", [{"cookies": "session=private"}, {"user_agent": "changed"}]) +async def test_browser_read_cannot_change_shared_session_identity(extra): + """截图等读取动作也不能顺带替换共享浏览器的 Cookie 或身份参数。""" + handler = AsyncMock() + permitted, _ = await AgentPolicyMiddleware(context=_context()).execute_tool_call( + tool=SimpleNamespace(name="browse_webpage", tags=["read"]), + arguments={"action": "snapshot", **extra}, handler=handler, + invocation_id="browser", enforce_decision=False, + ) + assert permitted is False + handler.assert_not_awaited() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("action", [None, {}, [], 1]) +async def test_invalid_mixed_action_is_denied_without_interrupting_graph(action): + """不合法的动作类型返回可恢复拒绝，不让集合查询异常打断子任务。""" + handler = AsyncMock() + permitted, result = await AgentPolicyMiddleware(context=_context()).execute_tool_call( + tool=SimpleNamespace(name="execute_command", tags=["read"]), + arguments={"action": action}, handler=handler, + invocation_id="invalid", enforce_decision=False, + ) + assert permitted is False and result.status == "error" + handler.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_parent_graph_keeps_authorized_api_write_path(): + """只读约束限定子代理，主代理已有写入路径继续实际执行。""" + executor = AsyncMock() + executor.execute.return_value = '{"success":true}' + gateway = MoviePilotApiTool(session_id="readonly", user_id="owner", executor=executor) + gateway.set_agent_context({"is_admin": True}) + model = _Model(responses=[AIMessage(content="", tool_calls=[{ + "id": "write", "name": "moviepilot_api", "args": { + "operation_id": "config.system.update", "body": {"setting_key": "LLM_MAX_ITERATIONS", "value": 128}, + }, + }]), AIMessage(content="已执行")]) + graph = create_agent(model=model, tools=[gateway], middleware=[ + AgentPolicyMiddleware(context=_context(origin=ToolOrigin.AGENT_INTERACTIVE), tools=[gateway]), + ]) + result = await graph.ainvoke({"messages": [HumanMessage(content="修改此设置")]}) + assert executor.execute.await_count == 1 + assert any(isinstance(message, ToolMessage) and message.status == "success" for message in result["messages"])