Skip to content

Commit 517d0da

Browse files
committed
Mcp(fix[safety]): Name the tier a gated tool needs
Calling a tool above the server's `LIBTMUX_SAFETY` tier reported `Unknown tool: 'kill_pane'` — the server denying that its own gated tool exists. An agent told a tool is absent reports the capability as missing instead of naming the setting that enables it. Two gates were intended and only one worked. FastMCP's native `disable()` enforces the tier; `SafetyMiddleware` was meant to explain it. `get_tool()` answers `None` for a disabled tool, so the guard reading `if tool and not allowed` never ran for precisely the tools it was written for, and dispatch raised `NotFoundError` instead. Off-tier names now resolve against `_list_tools()`, which retains disabled tools with their tags. The batch wrappers carried the same defect through a second call path: `_get_allowed_tool_tier` also read `get_tool` and raised "Unknown tool" on `None`, so a gated tool and a misspelled one produced byte-identical rows. It hands the operation on instead of duplicating the lookup — nested calls already run with `run_middleware=True`, so one source of truth decides which of the two it is. The message also hardcoded `LIBTMUX_SAFETY=destructive` for every denial, so a readonly server answered a `send_keys` call by advising the strongest tier — telling a user to grant `kill_server` rights in order to type into a pane. Denials now name the required tier and the tier in force. Restores an audit property the middleware ordering is built around: a tier denial must raise inside `SafetyMiddleware` so `AuditMiddleware`, sitting outside it, records it as a denial. While the denial never fired, blocked calls were audited as unknown-tool errors. `on_call_tool` now fails closed with no FastMCP context. It previously fell through to the tool, which is fail-open in a gate whose top tier includes `kill_server`, and was masked only because the native gate made the dispatch fail anyway. The existing suite documented the dead path rather than testing it, so 921 passing tests never called a gated tool end to end. Adds that coverage per tier and through the batch wrapper, plus a contract test pinning the private `_list_tools()` behavior the explanation depends on.
1 parent c201727 commit 517d0da

6 files changed

Lines changed: 528 additions & 16 deletions

File tree

CHANGES

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,55 @@
66
_Notes on upcoming releases will be added here_
77
<!-- END PLACEHOLDER - ADD NEW CHANGELOG ENTRIES BELOW THIS LINE -->
88

9+
### What's new
10+
11+
#### A gated tool now says which tier it needs
12+
13+
Calling a tool above the server's `LIBTMUX_SAFETY` tier reported
14+
`Unknown tool: 'kill_pane'` — the server denying that its own gated tool
15+
exists. An agent told a tool is absent reports the capability as missing
16+
rather than naming the setting that enables it.
17+
18+
Off-tier calls now name both the tier the tool requires and the tier in
19+
force:
20+
21+
```
22+
Tool 'kill_pane' requires safety level 'destructive', but this server is
23+
running at 'mutating'. Restart it with LIBTMUX_SAFETY=destructive to
24+
enable it.
25+
```
26+
27+
The message previously hardcoded `LIBTMUX_SAFETY=destructive` for every
28+
denial, so a `readonly` server answered a `send_keys` call by advising
29+
the strongest tier — telling a user to grant `kill_server` rights in
30+
order to type into a pane. The tier named is now the one the tool
31+
actually needs.
32+
33+
Two gates were always intended here, and only one of them worked.
34+
FastMCP's native `disable()` enforces the tier; `SafetyMiddleware` was
35+
meant to explain it. Because `get_tool()` answers `None` for a disabled
36+
tool, the middleware's `if tool and not allowed` guard never ran for the
37+
tools it was written for. Off-tier names now resolve against the full
38+
registry, which retains disabled tools and their tags.
39+
40+
Denials also reach the audit log as denials again. The server's
41+
middleware ordering is built so that a tier denial raises inside
42+
`SafetyMiddleware` and is recorded by `AuditMiddleware` outside it;
43+
while the denial never fired, a blocked call was audited as an
44+
unknown-tool error instead.
45+
46+
The batch wrappers carried the same defect through a second call path:
47+
`_get_allowed_tool_tier` also read `get_tool` and raised "Unknown tool"
48+
on `None`, so a gated tool and a misspelled one produced byte-identical
49+
rows. It now hands the operation on instead of duplicating the lookup —
50+
the nested call runs through the middleware, which names the tier for a
51+
gated tool while a real typo still gets FastMCP's own error.
52+
53+
`SafetyMiddleware.on_call_tool` additionally fails closed when no
54+
FastMCP context is present. It previously fell through to the tool,
55+
which was fail-open in a gate whose top tier includes `kill_server`, and
56+
was masked only because the native gate made the dispatch fail anyway.
57+
958
### Documentation
1059

1160
#### opencode joins the install picker

src/libtmux_mcp/middleware.py

Lines changed: 109 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,25 +51,53 @@
5151
ExpectedToolError,
5252
)
5353

54+
logger = logging.getLogger(__name__)
55+
5456
_TIER_LEVELS: dict[str, int] = {
5557
TAG_READONLY: 0,
5658
TAG_MUTATING: 1,
5759
TAG_DESTRUCTIVE: 2,
5860
}
5961

62+
#: Reverse of :data:`_TIER_LEVELS`, so a middleware configured with an
63+
#: unrecognized tier can still *name* the tier it fell back to.
64+
_LEVEL_TIERS: dict[int, str] = {level: tier for tier, level in _TIER_LEVELS.items()}
65+
66+
67+
def _highest_tier(tags: t.Collection[str]) -> str | None:
68+
"""Return the highest safety tier named in *tags*, or None if untagged."""
69+
found = [tier for tier in _TIER_LEVELS if tier in tags]
70+
if not found:
71+
return None
72+
return max(found, key=lambda tier: _TIER_LEVELS[tier])
73+
6074

6175
class SafetyMiddleware(Middleware):
62-
"""Gate tools by safety tier.
76+
"""Explain tier denials that ``_enable_allowed_tools`` enforces.
77+
78+
FastMCP's native ``disable()`` is the enforcement gate and holds
79+
even for a call that skips the middleware chain. It also makes
80+
``get_tool()`` answer **None**, so FastMCP reports an off-tier call
81+
as ``Unknown tool`` -- the server denying its own gated tool exists.
82+
This gate names the required tier instead, resolving such names
83+
against :meth:`_tier_snapshot` (the registry keeps disabled tools).
84+
85+
Denials must raise *here* so :class:`AuditMiddleware`, which sits
86+
outside, records them as denials rather than unknown-tool errors.
6387
6488
Parameters
6589
----------
6690
max_tier : str
67-
Maximum allowed tier. One of ``TAG_READONLY``, ``TAG_MUTATING``,
68-
or ``TAG_DESTRUCTIVE``.
91+
Maximum allowed tier. Unrecognized values fall back to
92+
``TAG_READONLY``.
6993
"""
7094

7195
def __init__(self, max_tier: str = TAG_MUTATING) -> None:
7296
self.max_level = _TIER_LEVELS.get(max_tier, 0)
97+
#: Normalized tier name, so a denial reports where the server
98+
#: stands rather than echoing an unrecognized env value back.
99+
self.max_tier = _LEVEL_TIERS[self.max_level]
100+
self._tier_by_tool: dict[str, str] | None = None
73101

74102
def _is_allowed(self, tags: set[str]) -> bool:
75103
"""Return True if the tool's tags fall within the allowed tier.
@@ -84,6 +112,55 @@ def _is_allowed(self, tags: set[str]) -> bool:
84112
return False
85113
return found_tier
86114

115+
def _denial_message(self, tool_name: str, required_tier: str | None) -> str:
116+
"""Name the required tier and the active one.
117+
118+
The message this replaced hardcoded ``destructive`` for every
119+
denial, so a readonly server answered ``send_keys`` by advising
120+
kill_server rights in order to type into a pane.
121+
"""
122+
if required_tier is None:
123+
return (
124+
f"Tool {tool_name!r} declares no safety tier and is blocked. "
125+
"This is a bug in the server, not a configuration problem; "
126+
"please report it."
127+
)
128+
return (
129+
f"Tool {tool_name!r} requires safety level {required_tier!r}, but "
130+
f"this server is running at {self.max_tier!r}. Restart it with "
131+
f"LIBTMUX_SAFETY={required_tier} to enable it."
132+
)
133+
134+
async def _tier_snapshot(self, fastmcp: t.Any) -> dict[str, str]:
135+
"""Map every registered tool name to its tier, disabled included.
136+
137+
Built once. ``_list_tools()`` is FastMCP-private, so a failure
138+
degrades to an empty map: the call then falls through to the
139+
stock ``NotFoundError``, losing the explanation but nothing
140+
else. ``tests/test_server.py`` pins the behavior so a fastmcp
141+
bump fails in CI rather than silently reverting this gate.
142+
"""
143+
if self._tier_by_tool is not None:
144+
return self._tier_by_tool
145+
146+
snapshot: dict[str, str] = {}
147+
try:
148+
registered = await fastmcp._list_tools()
149+
except Exception:
150+
logger.warning(
151+
"safety tier snapshot unavailable; off-tier calls will "
152+
"report 'unknown tool'",
153+
exc_info=True,
154+
)
155+
else:
156+
for tool in registered:
157+
tier = _highest_tier(tool.tags)
158+
if tier is not None:
159+
snapshot[tool.name] = tier
160+
161+
self._tier_by_tool = snapshot
162+
return snapshot
163+
87164
async def on_list_tools(
88165
self,
89166
context: MiddlewareContext,
@@ -98,16 +175,36 @@ async def on_call_tool(
98175
context: MiddlewareContext,
99176
call_next: t.Any,
100177
) -> t.Any:
101-
"""Block execution of tools above the safety tier."""
102-
if context.fastmcp_context:
103-
tool = await context.fastmcp_context.fastmcp.get_tool(context.message.name)
104-
if tool and not self._is_allowed(tool.tags):
105-
msg = (
106-
f"Tool '{context.message.name}' is not available at the "
107-
f"current safety level. Set LIBTMUX_SAFETY=destructive "
108-
f"to enable destructive tools."
178+
"""Block execution of tools above the safety tier.
179+
180+
Fail-closed except for a name the registry has never heard of,
181+
which is a typo and deserves FastMCP's own ``NotFoundError``.
182+
"""
183+
tool_name = context.message.name
184+
185+
if context.fastmcp_context is None:
186+
# No registry to consult: deny, since the top tier includes
187+
# kill_server.
188+
msg = (
189+
f"Tool {tool_name!r} was called without a FastMCP context, so "
190+
"its safety tier cannot be verified. Call it through MCP."
191+
)
192+
raise ExpectedToolError(msg)
193+
194+
fastmcp = context.fastmcp_context.fastmcp
195+
196+
tool = await fastmcp.get_tool(tool_name)
197+
if tool is not None:
198+
if not self._is_allowed(tool.tags):
199+
raise ExpectedToolError(
200+
self._denial_message(tool_name, _highest_tier(tool.tags))
109201
)
110-
raise ExpectedToolError(msg)
202+
return await call_next(context)
203+
204+
# Invisible to ``get_tool``: gated by tier, or nonexistent.
205+
gated_tier = (await self._tier_snapshot(fastmcp)).get(tool_name)
206+
if gated_tier is not None:
207+
raise ExpectedToolError(self._denial_message(tool_name, gated_tier))
111208
return await call_next(context)
112209

113210

src/libtmux_mcp/server.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,8 +436,11 @@ def _enable_allowed_tools() -> None:
436436
if _mcp_visibility_configured:
437437
return
438438

439-
# Use FastMCP's native visibility system as primary gate,
440-
# with the SafetyMiddleware as a secondary layer for clear error messages.
439+
# This is the ENFORCEMENT gate: it holds even for a call that skips
440+
# the middleware chain (``call_tool`` accepts
441+
# ``run_middleware=False``). ``SafetyMiddleware`` is the
442+
# EXPLANATION gate -- disabling makes ``get_tool`` answer None, so
443+
# FastMCP would otherwise report a gated tool as ``Unknown tool``.
441444
allowed_tags = {TAG_READONLY}
442445
if _safety_level in {TAG_MUTATING, TAG_DESTRUCTIVE}:
443446
allowed_tags.add(TAG_MUTATING)

src/libtmux_mcp/tools/batch_tools.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,14 @@ async def _get_allowed_tool_tier(
125125

126126
tool = await fastmcp.get_tool(operation.tool)
127127
if tool is None:
128-
msg = f"Unknown tool: {operation.tool!r}"
129-
raise ExpectedToolError(msg)
128+
# None means nonexistent OR disabled by tier, so raising
129+
# "Unknown tool" here denied that a gated tool exists. Hand it
130+
# on instead: the nested call runs with ``run_middleware=True``,
131+
# letting ``SafetyMiddleware`` name the tier and FastMCP still
132+
# raise ``NotFoundError`` for a typo. Nothing is skipped --
133+
# visibility follows tier tags, so an invisible tool is
134+
# off-tier by construction and is denied before these checks.
135+
return
130136

131137
# ``max_tier`` is a CEILING, so a readonly tool is reachable through
132138
# every batch wrapper, not only the readonly one. The batch loop is

0 commit comments

Comments
 (0)