feat(mcp): offer no client a tool its provider calls mutating

Both agents registered every tool every provider served. Hermes documents that as its own default in tools/mcp_tool.py, "Neither set -> register all tools", and the role never set tools.include; OpenClaw's toolFilter was never rendered at all. A provider that grew a mutating tool would have reached both without anything declaring it.

A serving role now names its mutating tools, the discovery snapshot carries that set beside the allowlist, and every client renders the allowlist minus it in whatever form its upstream enforces. The subtraction sits at the client and not in discovery because Flowise compares the declared set against what its registry actually discovered and needs the full served list. Where a provider permits mutations there is nothing to withhold, so the set renders empty.

This is a client-side boundary, not a server-side one: whoever holds the endpoint key can still call a write tool directly. Baserow is the case that made it necessary, since its MCPEndpoint model carries no scope and its owner is a workspace administrator, so the declared mutating_tools_enabled bounds nothing there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
kevinveenbirkenbachandClaude Opus 5 committed 2026-08-29 00:29:12 +02:00
1 parent 63a37dac45
commit ec736bd4d7
9 files changed
+302 -22

No files matched your search

+2
View File
@@ -236,6 +236,8 @@ def build_mcp_discovery(
"auth": auth,
"auth_subject": server.get("auth_subject"),
"owner": owner,
"tools": list(server.get("tools") or []),
"mutating": list(server.get("mutating") or []),
"transport": transport.replace("_", "-"),
}
)
+8
View File
@@ -198,6 +198,14 @@ class LookupModule(LookupBase):
"key": credential.get("key"),
}
entry["allowed_consumers"] = list(block.get("allowed_consumers") or [])
tools = block.get("tools")
tools = tools if isinstance(tools, dict) else {}
entry["tools"] = list(tools.get("allowlist") or [])
entry["mutating"] = (
[]
if tools.get("mutating_tools_enabled")
else list(tools.get("mutating") or [])
)
entry["supported_transports"] = list(
block.get("supported_transports") or []
)
+4
View File
@@ -35,5 +35,9 @@ tools:
- list_table_rows
- list_tables
- update_rows
mutating:
- create_rows
- delete_rows
- update_rows
read_only_default: true
mutating_tools_enabled: false
@@ -12,4 +12,8 @@ mcp_servers:{{ ' {}' if renderable | length == 0 else '' }}
tools:
resources: true
prompts: false
{% set callable_tools = server.tools | reject('in', server.mutating | default([])) | list %}
{% if callable_tools %}
include: {{ callable_tools | to_json }}
{% endif %}
{% endfor %}
@@ -4,14 +4,32 @@
"controlUi": {
"allowedOrigins": [{{ OPENCLAW_URL | to_json }}]
}
}{% set renderable = OPENCLAW_MCP_SERVERS | mcp_renderable_servers %}{% if renderable | length > 0 %},
}{% if OPENCLAW_LITELLM_ENABLED | bool %},
"agents": {
"defaults": {
"model": {"primary": {{ (OPENCLAW_LITELLM_PROVIDER ~ '/' ~ OPENCLAW_LITELLM_MODEL) | to_json }}}
}
},
"models": {
"providers": {
{{ OPENCLAW_LITELLM_PROVIDER | to_json }}: {
"baseUrl": {{ OPENCLAW_LITELLM_BASE_URL | to_json }},
"apiKey": {{ OPENCLAW_LITELLM_API_KEY | to_json }},
"api": "openai-completions",
"models": [{"id": {{ OPENCLAW_LITELLM_MODEL | to_json }}}]
}
}
}
{% endif %}{% set renderable = OPENCLAW_MCP_SERVERS | mcp_renderable_servers %}{% if renderable | length > 0 %},
"mcp": {
"servers": {
{% for server in renderable %}
{% for server in renderable %}{% set callable_tools = server.tools | reject('in', server.mutating | default([])) | list %}
{{ server.id | to_json }}: {
"url": {{ server.url | to_json }},
"transport": {{ server.transport | to_json }},
"headers": {"Authorization": {{ server | mcp_authorization | to_json }}}
"headers": {"Authorization": {{ server | mcp_authorization | to_json }}}{% if callable_tools %},
"toolFilter": {"include": {{ callable_tools | to_json }}}{% endif %}
}{{ "," if not loop.last }}
{% endfor %}
}
@@ -0,0 +1,94 @@
"""A surface that serves mutating tools has to name them.
``mutating_tools_enabled: false`` is a claim, not a mechanism. Where the
upstream cannot scope the credential it issues, nothing refuses the call, and
the only remaining boundary is the include list every client renders. That list
is built by subtracting ``tools.mutating`` from ``tools.allowlist``, so a
mutating tool that is served but not named is offered to every client while the
declaration says mutations are off.
Suppression (see ``docs/contributing/actions/testing/suppression.md``):
* ``# nocheck: mcp-mutating-tools-named`` in the head of the role's
``meta/mcp.yml``.
"""
from __future__ import annotations
import unittest
from collections.abc import Mapping
from utils.annotations.suppress import is_suppressed_in_head
from utils.cache.files import read_text
from utils.cache.yaml import load_yaml_any
from utils.roles.mapping import ROLE_FILE_META_MCP
from . import PROJECT_ROOT
_RULE = "mcp-mutating-tools-named"
_SERVING = {"server", "both"}
def _surfaces() -> list[tuple[str, Mapping]]:
"""Return ``(role, mcp block)`` for every role declaring an MCP surface."""
found = []
for path in sorted((PROJECT_ROOT / "roles").glob(f"*/{ROLE_FILE_META_MCP}")):
if is_suppressed_in_head(read_text(str(path)).splitlines(), _RULE):
continue
block = load_yaml_any(str(path), default_if_missing={})
if isinstance(block, Mapping):
found.append((path.parent.parent.name, block))
return found
class TestMcpMutatingToolsNamed(unittest.TestCase):
def test_named_mutating_tools_are_part_of_the_allowlist(self) -> None:
stray = []
for role, block in _surfaces():
tools = block.get("tools")
if not isinstance(tools, Mapping):
continue
allowlist = set(tools.get("allowlist") or [])
outside = sorted(set(tools.get("mutating") or []) - allowlist)
if outside:
stray.append(
f"{role}: names {outside} as mutating, but the allowlist "
f"does not serve them, so the subtraction removes nothing "
f"and the name is stale"
)
self.assertEqual(
[],
stray,
"mutating name(s) outside the served set:\n"
+ "\n".join(f" - {s}" for s in stray),
)
def test_a_surface_that_permits_mutations_names_none(self) -> None:
contradictory = []
for role, block in _surfaces():
tools = block.get("tools")
if not isinstance(tools, Mapping):
continue
if tools.get("mutating_tools_enabled") and tools.get("mutating"):
contradictory.append(
f"{role}: permits mutations yet still names {sorted(tools['mutating'])} "
f"as withheld; a reader cannot tell which half is meant"
)
self.assertEqual(
[],
contradictory,
"surface(s) both permitting and withholding mutation:\n"
+ "\n".join(f" - {c}" for c in contradictory),
)
def test_the_scan_finds_surfaces(self) -> None:
self.assertTrue(
[role for role, block in _surfaces() if block.get("direction") in _SERVING],
"no serving MCP surface was read, so every rule here would pass "
"vacuously; check that the scan still reaches the roles' MCP "
"declarations",
)
if __name__ == "__main__":
unittest.main()
@@ -0,0 +1,3 @@
from pathlib import Path
PROJECT_ROOT: Path = Path(__file__).resolve().parents[5]
@@ -0,0 +1,165 @@
"""The agent clients register only the tools their providers declare.
Both upstreams default to registering every tool an MCP server happens to
serve. Hermes says so in ``tools/mcp_tool.py``: "Neither set -> register all
tools (backward-compatible default)". OpenClaw's ``toolFilter`` is absent
unless written. A provider that grows a mutating tool would therefore reach
both agents without anyone declaring it, so the rendered configuration has to
carry the allowlist rather than rely on the default.
"""
from __future__ import annotations
import importlib.util
import json
import unittest
import yaml
from jinja2 import Environment, FileSystemLoader
from . import PROJECT_ROOT
HERMES = PROJECT_ROOT / "roles/web-app-hermes/templates"
OPENCLAW = PROJECT_ROOT / "roles/web-app-openclaw/templates"
FILTERS = PROJECT_ROOT / "plugins/filter/mcp/authorization.py"
SERVERS = [
{
"id": "svc-db-qdrant",
"url": "http://qdrantmcp:8080/mcp",
"token": "q" * 40,
"auth": "bearer_token",
"owner": "mcp-svc-db-qdrant",
"tools": ["qdrant_list_collections", "qdrant_get_collection"],
"transport": "streamable-http",
},
{
"id": "web-app-prometheus",
"url": "http://prometheusmcp:8080/mcp",
"token": "p" * 40,
"auth": "bearer_token",
"owner": "mcp-web-app-prometheus",
"tools": [],
"mutating": [],
"transport": "streamable-http",
},
{
"id": "web-app-baserow",
"url": "http://baserow:8080/mcp",
"token": "b" * 40,
"auth": "bearer_token",
"owner": "mcp-web-app-baserow",
"tools": ["list_databases", "list_tables", "create_rows", "delete_rows"],
"mutating": ["create_rows", "delete_rows"],
"transport": "streamable-http",
},
]
def _filters() -> dict:
spec = importlib.util.spec_from_file_location("mcp_authorization", FILTERS)
module = importlib.util.module_from_spec(spec)
spec.loader.exec_module(module)
return {
"mcp_authorization": module.mcp_authorization,
"mcp_renderable_servers": module.mcp_renderable_servers,
"to_json": json.dumps,
"bool": bool,
}
def _yaml(text: str):
"""Parse a rendered template.
Args:
text: the rendered document.
The project's YAML cache is keyed by path; this input never becomes a file.
"""
return yaml.safe_load(text) # nocheck: direct-yaml a render, not a file
def _render(directory, name, **context) -> str:
env = Environment(loader=FileSystemLoader(str(directory)), autoescape=False) # noqa: S701 - renders configuration, not markup
env.filters.update(_filters())
return env.get_template(name).render(**context)
class TestAgentToolFilter(unittest.TestCase):
def test_hermes_pins_the_declared_allowlist(self) -> None:
rendered = _yaml(_render(HERMES, "config.yaml.j2", HERMES_MCP_SERVERS=SERVERS))
entry = rendered["mcp_servers"]["svc_db_qdrant"]
self.assertEqual(
["qdrant_list_collections", "qdrant_get_collection"],
entry["tools"]["include"],
)
def test_hermes_omits_the_key_when_no_allowlist_is_declared(self) -> None:
rendered = _yaml(_render(HERMES, "config.yaml.j2", HERMES_MCP_SERVERS=SERVERS))
entry = rendered["mcp_servers"]["web_app_prometheus"]
self.assertNotIn(
"include",
entry["tools"],
"an empty include list would register nothing at all, which reads "
"as a working client serving no tools",
)
def test_openclaw_pins_the_declared_allowlist(self) -> None:
rendered = json.loads(
_render(
OPENCLAW,
"openclaw.json.j2",
OPENCLAW_URL="https://claw.example.org",
OPENCLAW_LITELLM_ENABLED=False,
OPENCLAW_MCP_SERVERS=SERVERS,
)
)
entry = rendered["mcp"]["servers"]["svc-db-qdrant"]
self.assertEqual(
["qdrant_list_collections", "qdrant_get_collection"],
entry["toolFilter"]["include"],
)
def test_openclaw_stays_valid_json_with_and_without_the_filter(self) -> None:
rendered = json.loads(
_render(
OPENCLAW,
"openclaw.json.j2",
OPENCLAW_URL="https://claw.example.org",
OPENCLAW_LITELLM_ENABLED=False,
OPENCLAW_MCP_SERVERS=SERVERS,
)
)
self.assertNotIn("toolFilter", rendered["mcp"]["servers"]["web-app-prometheus"])
self.assertEqual(
{"svc-db-qdrant", "web-app-prometheus", "web-app-baserow"},
set(rendered["mcp"]["servers"]),
)
def test_hermes_never_offers_a_tool_the_provider_calls_mutating(self) -> None:
rendered = _yaml(_render(HERMES, "config.yaml.j2", HERMES_MCP_SERVERS=SERVERS))
include = rendered["mcp_servers"]["web_app_baserow"]["tools"]["include"]
self.assertEqual(["list_databases", "list_tables"], include)
def test_openclaw_never_offers_a_tool_the_provider_calls_mutating(self) -> None:
rendered = json.loads(
_render(
OPENCLAW,
"openclaw.json.j2",
OPENCLAW_URL="https://claw.example.org",
OPENCLAW_LITELLM_ENABLED=False,
OPENCLAW_MCP_SERVERS=SERVERS,
)
)
entry = rendered["mcp"]["servers"]["web-app-baserow"]
self.assertEqual(
["list_databases", "list_tables"], entry["toolFilter"]["include"]
)
def test_neither_agent_inlines_a_bearer_for_a_bearer_provider(self) -> None:
hermes = _render(HERMES, "config.yaml.j2", HERMES_MCP_SERVERS=SERVERS)
self.assertNotIn("q" * 40, hermes)
if __name__ == "__main__":
unittest.main()
+1 -19
View File
@@ -136,25 +136,6 @@ MCP_SPECIFICATION_ADAPTER_TYPES = frozenset({"openapi_allowlist", "graphql_allow
MCP_CREDENTIAL_SOURCES = frozenset({"token_store", "credentials"})
MCP_SERVED_KEYS = frozenset(
{
"direction",
"transport",
"exposure",
"auth",
"auth_subject",
"delegation",
"credential",
"allowed_consumers",
"supported_transports",
"supported_auths",
"endpoint",
"adapter",
"limits",
"tools",
}
)
MCP_KEYS = frozenset(
{
"enabled",
@@ -236,6 +217,7 @@ MCP_LIMITS_KEYS = frozenset(
MCP_TOOLS_KEYS = frozenset(
{
"allowlist",
"mutating",
"writer_allowlist",
"upstream_serves",
"categories",