diff --git a/tests/tools/test_mcp_multiplex_connection_keys.py b/tests/tools/test_mcp_multiplex_connection_keys.py index b77699635e..b7f80a13b9 100644 --- a/tests/tools/test_mcp_multiplex_connection_keys.py +++ b/tests/tools/test_mcp_multiplex_connection_keys.py @@ -38,7 +38,8 @@ def two_profiles(tmp_path, monkeypatch): ledgers = ("_servers", "_server_scope_keys", "_server_tool_scopes", "_server_connecting", "_server_connect_errors", "_server_connect_retry_after", "_server_connect_failures", "_server_error_counts", "_server_breaker_opened_at", "_lazy_server_configs", - "_mcp_tool_server_names", "_orphaned_adopters") + "_mcp_tool_server_names", "_orphaned_adopters", "_parallel_safe_servers", + "_server_trust_levels", "_tool_read_only_hints") saved = {n: type(getattr(core, n))(getattr(core, n)) for n in ledgers} for n in ledgers: getattr(core, n).clear() @@ -192,3 +193,30 @@ def test_owner_reload_reregisters_profiles_that_adopted_its_connection(two_profi two_profiles("b") assert registry.get_tool_names_for_toolset("mcp-x") == ["mcp__x__t"] assert disc.get_mcp_status({"x": cfg})[0]["status"] == "connected" + + +def test_untrusted_adopter_of_a_full_profiles_connection_keeps_its_own_trust_gate(two_profiles, monkeypatch): + """Trust is the consuming profile's policy: adopting A's ``trust: full`` connection must not let + B's ``trust: untrusted`` write-capable call skip approval.""" + from tools import mcp_tool_discovery as disc, mcp_tool_handlers as handlers + from tools import mcp_tool_registration as reg + import tools.approval_prompt as approval_prompt + + route = {"url": "https://mcp.example/x", "headers": {"Authorization": "Bearer shared"}} + cfg_a, cfg_b = dict(route, trust="full"), dict(route, trust="untrusted") + asked = [] + monkeypatch.setattr(approval_prompt, "request_elicitation_consent", + lambda *a, **k: asked.append(a) or "deny") + + two_profiles("a") + srv_a = _server("x", cfg_a) + disc._adopt_server("x", srv_a) + srv_a._registered_tool_names = reg._register_server_tools("x", srv_a, cfg_a) + + two_profiles("b") + assert reg.register_connected_into_current_scope({"x": cfg_b}) == 1 + assert handlers._trust_gate_check("x", "t") is not None and asked + + two_profiles("a") + assert handlers._trust_gate_check("x", "t") is None and len(asked) == 1 + diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 9868754f5e..c1ac8d83c2 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -468,7 +468,9 @@ _CIRCUIT_BREAKER_THRESHOLD, _CIRCUIT_BREAKER_COOLDOWN_SEC = 3, 60.0 # before the RPC fires. A lying readOnlyHint can only skip approval for calls the operator was # already warned about, never widen access. Missing trust = full; unrecognized = untrusted (a # typo must never disable the gate). Classified at CALL time from DISCOVERY data: no schema -# mutation, prompt cache intact. +# mutation, prompt cache intact. ``_server_trust_levels`` is keyed by the CONSUMING profile's own +# key (its policy for the name, even when it adopted another profile's connection); +# ``_tool_read_only_hints`` by the connection key (the server's own tool annotations). _server_trust_levels: Dict[Any, str] = {} _tool_read_only_hints: Dict[Any, Dict[str, bool]] = {} diff --git a/tools/mcp_tool_handlers.py b/tools/mcp_tool_handlers.py index 36ff99f4d6..e8df28d9a9 100644 --- a/tools/mcp_tool_handlers.py +++ b/tools/mcp_tool_handlers.py @@ -41,10 +41,12 @@ _STDIO_OUTCOME_UNCERTAIN_MSG = ( def _trust_gate_check(server_name: str, tool_name: str) -> Optional[str]: """Approval gate for write-capable tools on ``trust: untrusted`` servers. None to proceed, else a ``tool_error``. Fail-closed: approval-system errors block.""" - from tools.mcp_tool_scope import _resolve_server_key - key = _resolve_server_key(server_name) - if (_core._server_trust_levels.get(key, _core._TRUST_FULL) != _core._TRUST_UNTRUSTED - or _core._tool_read_only_hints.get(key, {}).get(tool_name) is True): + from tools.mcp_tool_scope import _resolve_server_key, _server_key + # Trust is the calling profile's own policy (an adopter of a shared connection keeps its own tier); + # readOnlyHint is a property of the connection's tools, so it lives under the connection key. + trust = _core._server_trust_levels.get(_server_key(server_name), _core._TRUST_FULL) + if (trust != _core._TRUST_UNTRUSTED + or _core._tool_read_only_hints.get(_resolve_server_key(server_name), {}).get(tool_name) is True): return None try: # lazy: tools.approval routes the prompt to whichever surface owns the session from tools.approval_prompt import request_elicitation_consent diff --git a/tools/mcp_tool_registration.py b/tools/mcp_tool_registration.py index a9a4d02280..a01d24c2c5 100644 --- a/tools/mcp_tool_registration.py +++ b/tools/mcp_tool_registration.py @@ -51,16 +51,27 @@ def _annotation_read_only_hint(mcp_tool: Any) -> bool: return hint is True -def _record_tool_trust_metadata(server_name: str, config: dict, tools: List[Any]) -> None: +def _record_tool_trust_metadata(server_name: str, config: dict, tools: List[Any], key=None) -> None: """Capture per-server trust and per-tool readOnlyHint at discovery — the security boundary: the call-time gate - classifies from data we control, never re-read server-supplied state.""" + classifies from data we control, never re-read server-supplied state. *key* is the connection (default: the + registering profile's own); the ``trust`` policy is recorded under it for the profile that owns it — an + adopting profile records its own policy in ``_record_scope_trust``.""" with _core._lock: - key = _resolve_server_key(server_name) + if key is None: + key = _server_key(server_name) _core._server_trust_levels[key] = _normalize_server_trust((config or {}).get("trust")) hints = _core._tool_read_only_hints.setdefault(key, {}) hints.update({t.name: _annotation_read_only_hint(t) for t in tools if getattr(t, "name", None)}) +def _record_scope_trust(server_name: str, config: dict, scope: str) -> None: + """``trust`` is the CONSUMING profile's policy, never the connection's: an ``untrusted`` profile that + adopts a ``full`` profile's live connection must still be asked before every write-capable call.""" + with _core._lock: + _core._server_trust_levels[_server_key(server_name, scope, current=False)] = _normalize_server_trust( + (config or {}).get("trust")) + + def _track_mcp_tool_server(tool_name: str, server_name: str) -> None: """Remember the exact raw MCP server that registered *tool_name*.""" with _core._lock: @@ -131,6 +142,7 @@ def _remove_server_scope(key, scope: str) -> None: _core._server_tool_scopes[key] = scopes else: _core._server_tool_scopes.pop(key, None) + _core._server_trust_levels.pop(_server_key(server_name, scope, current=False), None) _restore_server_toolset_alias(key) @@ -384,7 +396,7 @@ def _register_server_tools(name: str, server: "MCPServerTask", config: dict) -> ``toolsets.TOOLSETS``; lossy normalization collisions (``read-file``/``read_file``) fail closed.""" should_register = _make_tool_filter(name, config) key = _server_key_for_task(server) - _record_tool_trust_metadata(name, config, server._tools) + _record_tool_trust_metadata(name, config, server._tools, key) candidates = _tool_candidates(name, server._tools, should_register, server.tool_timeout) candidates += _utility_candidates(name, _select_utility_schemas(name, server, config), server.tool_timeout) registered = _register_candidates( @@ -485,6 +497,7 @@ def _register_connected_into_current_scope(servers: dict) -> int: # Visibility for this profile: the owner keeps teardown, this scope sees the connection. with _core._lock: _core._server_tool_scopes.setdefault(key, set()).add(scope) + _record_scope_trust(name, config, scope) if registry.get_tool_names_for_toolset(f"mcp-{name}"): continue candidates = _tool_candidates(name, server._tools, _make_tool_filter(name, config), server.tool_timeout)