Skip to content

Commit 125099c

Browse files
committed
feat: redo tool filtering for better tests
Filtering tools at server startup made it hard to test the effects of CONFIG.toolset. Switch to a system where tools are completely filtered in the middleware. (Doing it at the session level would be possible, but the direction for future protocol versions of MCP is to remove sessions entirely) This allows making the tests for server.py more comprehensive and greatly reduces the amount of mocking necessary. * All tests for server.py are moved into a new test_server.py. * Instead of making "fixed" the default toolset for tools without a tag, fixed tools are tagged just like run_script tools are. * toolset.Toolset is modified to only have tags for inclusion rather than exclusion tags as well. * A new setup_client fixture is added that is more flexible than the old mcp_client fixture - it's callable and the desired toolset and mcp-app support are passed in. * Our mcp-app HTML resource is properly filtered along with the tools.
1 parent c74f0dc commit 125099c

14 files changed

Lines changed: 649 additions & 489 deletions

File tree

src/linux_mcp_server/server.py

Lines changed: 133 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,20 @@
33
import logging
44
import sys
55

6+
from dataclasses import dataclass
67
from importlib import resources
78
from pathlib import Path
89

10+
from fastmcp import Context
911
from fastmcp import FastMCP
12+
from fastmcp.exceptions import NotFoundError
1013
from fastmcp.resources import ResourceContent
1114
from fastmcp.resources import ResourceResult
1215
from fastmcp.server.dependencies import get_access_token
1316
from fastmcp.server.middleware import Middleware
1417
from fastmcp.server.middleware import MiddlewareContext
1518
from fastmcp.server.middleware.middleware import CallNext
19+
from fastmcp.utilities.components import FastMCPComponent
1620
from mcp.types import InitializeRequest
1721
from mcp.types import InitializeRequestParams
1822
from mcp.types import InitializeResult
@@ -29,6 +33,7 @@
2933
from linux_mcp_server.mcp_app import MCP_UI_EXTENSION
3034
from linux_mcp_server.mcp_app import RUN_SCRIPT_APP_URI
3135
from linux_mcp_server.toolset import get_toolset
36+
from linux_mcp_server.toolset import Toolset as ToolsetInfo
3237

3338

3439
def monkeypatch_fastmcp_for_app_visibility():
@@ -153,40 +158,42 @@ def monkeypatch_fastmcp_for_app_visibility():
153158
"""
154159

155160

156-
match CONFIG.toolset:
157-
case Toolset.FIXED:
158-
instructions = INSTRUCTIONS_FIXED
159-
case Toolset.RUN_SCRIPT:
160-
instructions = INSTRUCTIONS_RUN_SCRIPT
161-
case Toolset.BOTH:
162-
instructions = INSTRUCTIONS_BOTH
163-
case _:
164-
assert False, f"Unknown toolset configuration: {CONFIG.toolset}"
161+
def _get_instructions() -> str:
162+
match CONFIG.toolset:
163+
case Toolset.FIXED:
164+
return INSTRUCTIONS_FIXED
165+
case Toolset.RUN_SCRIPT:
166+
return INSTRUCTIONS_RUN_SCRIPT
167+
case Toolset.BOTH:
168+
return INSTRUCTIONS_BOTH
169+
case _: # pragma: no cover
170+
assert False, f"Unknown toolset configuration: {CONFIG.toolset}"
165171

166-
toolset = get_toolset(CONFIG.toolset.value)
167-
assert toolset is not None, f"Toolset not found in registry: {CONFIG.toolset}"
168172

169-
if CONFIG.toolset != Toolset.FIXED and CONFIG.gatekeeper_model is None:
170-
logger.error("LINUX_MCP_GATEKEEPER_MODEL not set, this is needed for run_script tools")
171-
sys.exit(1)
173+
def _current_toolset():
174+
toolset = get_toolset(CONFIG.toolset.value)
175+
assert toolset is not None, f"Toolset not found in registry: {CONFIG.toolset}"
172176

173-
# Create auth provider if configured
174-
auth_provider = create_auth_provider()
177+
return toolset
178+
179+
180+
def _check_gatekeeper_model():
181+
if CONFIG.toolset != Toolset.FIXED and CONFIG.gatekeeper_model is None:
182+
logger.error("LINUX_MCP_GATEKEEPER_MODEL not set, this is needed for run_script tools")
183+
sys.exit(1)
175184

176-
mcp = FastMCP("linux-mcp-server", instructions=instructions, version=linux_mcp_server.__version__, auth=auth_provider)
177185

178-
if toolset.include_tags:
179-
mcp.enable(tags=toolset.include_tags, only=True)
180-
else:
181-
mcp.enable()
186+
_check_gatekeeper_model()
187+
188+
# Create auth provider if configured
189+
auth_provider = create_auth_provider()
182190

183-
if toolset.exclude_tags:
184-
mcp.disable(tags=toolset.exclude_tags)
191+
mcp = FastMCP("linux-mcp-server", version=linux_mcp_server.__version__, auth=auth_provider)
185192

186193

187194
@mcp.resource(
188195
RUN_SCRIPT_APP_URI,
189-
tags={"run_script"},
196+
tags={"run_script", "mcp_apps_only"},
190197
)
191198
def run_script_app_html() -> ResourceResult:
192199
filename = "run-script-app.html"
@@ -238,22 +245,77 @@ def _use_mcp_app_for_client(client_params: InitializeRequestParams):
238245
return MCP_APP_MIME_TYPE in mime_types
239246

240247

248+
def _hide_app_tools_for_client(client_params: InitializeRequestParams):
249+
# Versions of goose before 1.29.0 don't understand _meta.ui.visiblity, so would
250+
# leak our app-only tools to the model. However, they also are happy to let the
251+
# model call tools that aren't listed as well. So if we see such an old version
252+
# of goose, we strip out the app-only tools.
253+
client_info = getattr(client_params, "clientInfo", None)
254+
if client_info and client_info.name and client_info.name.startswith("goose"):
255+
try:
256+
major, minor = client_info.version.split(".")[0:2]
257+
if (int(major), int(minor)) < (1, 29):
258+
return True
259+
except ValueError:
260+
return False
261+
262+
return False
263+
264+
265+
@dataclass
266+
class ComponentFilter:
267+
"""
268+
Determines what components (tools/resources) should be visible for a client
269+
given the current config.
270+
"""
271+
272+
mcp_apps: bool
273+
toolset: ToolsetInfo
274+
hide_app_tools: bool
275+
276+
def includes(self, component: FastMCPComponent):
277+
if not self.toolset.includes_tool(component.tags):
278+
return False
279+
280+
if self.mcp_apps:
281+
if "mcp_apps_exclude" in component.tags:
282+
return False
283+
else:
284+
if "mcp_apps_only" in component.tags:
285+
return False
286+
287+
if self.hide_app_tools and "hidden_from_model" in component.tags:
288+
return False
289+
290+
return True
291+
292+
@staticmethod
293+
def get(context: Context, *, is_list_tools=False):
294+
client_params = context.session.client_params
295+
assert client_params is not None
296+
mcp_apps = _use_mcp_app_for_client(client_params)
297+
hide_app_tools = mcp_apps and is_list_tools and _hide_app_tools_for_client(client_params)
298+
299+
return ComponentFilter(
300+
mcp_apps=mcp_apps,
301+
toolset=_current_toolset(),
302+
hide_app_tools=hide_app_tools,
303+
)
304+
305+
241306
# Middleware to enforce authorization policy
242307
class AuthorizationMiddleware(Middleware):
243308
async def on_call_tool(self, context: MiddlewareContext, call_next):
244309
# Extract tool metadata
245310
tool_args = context.message.arguments or {}
246311
target_host = tool_args.get("host")
247312

248-
# Get tool from FastMCP else fail closed if not found
249-
if context.fastmcp_context is None:
250-
logger.error("FastMCP context not available in middleware")
251-
raise ValueError("Internal error: FastMCP context not available")
313+
assert context.fastmcp_context
252314

253315
tool = await context.fastmcp_context.fastmcp.get_tool(context.message.name)
254-
if tool is None:
255-
logger.error(f"Tool not found: {context.message.name}")
256-
raise ValueError(f"Tool not found: {context.message.name}")
316+
if tool is None or not ComponentFilter.get(context.fastmcp_context).includes(tool):
317+
logger.error(f"Tool not found: '{context.message.name}'")
318+
raise NotFoundError(f"Tool not found: '{context.message.name}'")
257319

258320
# For stdio without policy configured, allow everything
259321
if CONFIG.transport == Transport.stdio and CONFIG.policy_path is None:
@@ -298,12 +360,26 @@ async def on_call_tool(self, context: MiddlewareContext, call_next):
298360
if action == PolicyAction.SSH_KEY:
299361
if not ssh_key_config:
300362
raise RuntimeError("Policy validation error: SSH_KEY action requires ssh_key configuration.")
301-
logger.debug(f"SSH key override: p`ath={ssh_key_config.path}, user={ssh_key_config.user}")
363+
logger.debug(f"SSH key override: path={ssh_key_config.path}, user={ssh_key_config.user}")
302364

303365
return await call_next(context)
304366

305367

306368
class DynamicDiscoveryMiddleware(Middleware):
369+
"""
370+
Implement a dynamic server that presents the right instructions, tools, and resources
371+
depending on the client and the current configuration.
372+
373+
Our configuration is logically static, but treating it dynamic makes it much
374+
easier to write the appropriate tests.
375+
376+
The dynamic behavior is done *per call*, not per session, since that's more future-proof.
377+
(Future MCP protocol versions will remove sessions entirely:
378+
https://modelcontextprotocol.io/seps/2575-stateless-mcp). This means doing it ourselves
379+
rather than using FastMCP's system for session-level enabling and disabling components,
380+
but it doesn't come out much worse.
381+
"""
382+
307383
async def on_initialize(
308384
self,
309385
context: MiddlewareContext[InitializeRequest],
@@ -315,51 +391,47 @@ async def on_initialize(
315391
# the client are fetched from the InitializationOptions object tucked
316392
# away in the ServerSession object, so we need to modify that based
317393
# on whether we'll use mcp-apps with the client making the InitializeRequest.
394+
#
395+
# For consistency and simplicity of testing, we always set the
396+
# instructions this way
318397

319398
assert context.fastmcp_context
320399
session = context.fastmcp_context.session
321400

322-
if _use_mcp_app_for_client(context.message.params):
323-
instructions = session._init_options.instructions
324-
if instructions:
325-
session._init_options.instructions = instructions.replace(
326-
"run_script_with_confirmation", "run_script_interactive"
327-
)
401+
instructions = _get_instructions()
402+
403+
toolset = _current_toolset()
404+
if "run_script" in toolset.tags and _use_mcp_app_for_client(context.message.params):
405+
instructions = instructions.replace("run_script_with_confirmation", "run_script_interactive")
406+
407+
session._init_options.instructions = instructions
328408

329409
return await call_next(context)
330410

331411
async def on_list_tools(self, context: MiddlewareContext, call_next):
332412
tools = await call_next(context)
333413

334-
# Eventually, the tagging of the tools via _meta.ui.visiblity as "app" or "model" will
335-
# hide this tool but Goose doesn't support this yet. On the other hand, goose is happy
336-
# if the app calls tools we don't list at all, so we just filter out the "app" tools
337-
filtered_tools = [t for t in tools if "hidden_from_model" not in (t.tags)]
414+
assert context.fastmcp_context
415+
filter = ComponentFilter.get(context.fastmcp_context, is_list_tools=True)
416+
return [t for t in tools if filter.includes(t)]
338417

339-
fastmcp_context = context.fastmcp_context
340-
assert fastmcp_context is not None, (
341-
"FastMCP framework error: context.fastmcp_context should not be None inside on_list_tools"
342-
)
418+
async def on_list_resources(self, context: MiddlewareContext, call_next):
419+
resources = await call_next(context)
343420

344-
request_ctx = fastmcp_context.request_context
345-
assert request_ctx is not None, (
346-
"FastMCP framework error: request context should not be None inside on_list_tools"
347-
)
421+
assert context.fastmcp_context
422+
filter = ComponentFilter.get(context.fastmcp_context)
423+
return [r for r in resources if filter.includes(r)]
348424

349-
client_params = request_ctx.session.client_params
350-
assert client_params is not None, (
351-
"FastMCP framework error: client_params should not be None inside on_list_tools"
352-
)
425+
# on_call_tool: the filtering for this is handled in AuthorizationMiddleware
426+
# (the two middlewares could be combined)
427+
#
428+
# on_read_resource: we consider it harmless if any app reads the static and
429+
# public mcp-app HTML, so we don't provide a on_read_resource() handler.
353430

354-
if _use_mcp_app_for_client(client_params):
355-
filtered_tools = [t for t in filtered_tools if "mcp_apps_exclude" not in t.tags]
356-
else:
357-
filtered_tools = [t for t in filtered_tools if "mcp_apps_only" not in t.tags]
358431

359-
return filtered_tools
432+
mcp.add_middleware(AuthorizationMiddleware())
433+
mcp.add_middleware(DynamicDiscoveryMiddleware())
360434

361435

362436
def main():
363-
mcp.add_middleware(AuthorizationMiddleware())
364-
mcp.add_middleware(DynamicDiscoveryMiddleware())
365437
mcp.run(show_banner=False, transport=CONFIG.transport.value, **CONFIG.transport_kwargs)

src/linux_mcp_server/tools/logs.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ async def _get_journal_logs(
8181
@mcp.tool(
8282
title="Get journal logs",
8383
description="Get systemd journal logs.",
84-
tags={"journal", "logs", "systemd", "troubleshooting"},
84+
tags={"fixed", "journal", "logs", "systemd", "troubleshooting"},
8585
annotations=ToolAnnotations(readOnlyHint=True),
8686
)
8787
@log_tool_call
@@ -143,7 +143,7 @@ async def get_journal_logs(
143143
@mcp.tool(
144144
title="Read log file",
145145
description="Read a specific log file.",
146-
tags={"files", "logs", "troubleshooting"},
146+
tags={"fixed", "files", "logs", "troubleshooting"},
147147
annotations=ToolAnnotations(readOnlyHint=True),
148148
)
149149
@log_tool_call

src/linux_mcp_server/tools/network.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
@mcp.tool(
2121
title="Get network interfaces",
2222
description="Get detailed information about network interfaces including address and traffic statistics.",
23-
tags={"connectivity", "interfaces", "network"},
23+
tags={"fixed", "connectivity", "interfaces", "network"},
2424
annotations=ToolAnnotations(readOnlyHint=True),
2525
)
2626
@log_tool_call
@@ -56,7 +56,7 @@ async def get_network_interfaces(
5656
@mcp.tool(
5757
title="Get network connections",
5858
description="Get detailed information about active network connections.",
59-
tags={"connections", "connectivity", "network"},
59+
tags={"fixed", "connections", "connectivity", "network"},
6060
annotations=ToolAnnotations(readOnlyHint=True),
6161
)
6262
@log_tool_call
@@ -82,7 +82,7 @@ async def get_network_connections(
8282
@mcp.tool(
8383
title="Get listening ports",
8484
description="Get details on listening port, protocols, and services.",
85-
tags={"connectivity", "network", "ports"},
85+
tags={"fixed", "connectivity", "network", "ports"},
8686
annotations=ToolAnnotations(readOnlyHint=True),
8787
)
8888
@log_tool_call

src/linux_mcp_server/tools/processes.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
@mcp.tool(
2121
title="List processes",
2222
description="List running processes",
23-
tags={"performance", "processes"},
23+
tags={"fixed", "performance", "processes"},
2424
annotations=ToolAnnotations(readOnlyHint=True),
2525
)
2626
@log_tool_call
@@ -45,7 +45,7 @@ async def list_processes(
4545
@mcp.tool(
4646
title="Process details",
4747
description="Get information about a specific process.",
48-
tags={"performance", "processes"},
48+
tags={"fixed", "performance", "processes"},
4949
annotations=ToolAnnotations(readOnlyHint=True),
5050
)
5151
@log_tool_call

src/linux_mcp_server/tools/run_script.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ class ExecuteScriptResult:
233233

234234

235235
@mcp.tool(
236-
tags={"run_script", "hidden_from_model"},
236+
tags={"run_script", "mcp_apps_only", "hidden_from_model"},
237237
description="Execute a script; this is only available to the our mcp-app",
238238
app=AppConfig(visibility=["app"]),
239239
)
@@ -270,7 +270,7 @@ async def execute_script(
270270

271271

272272
@mcp.tool(
273-
tags={"run_script", "hidden_from_model"},
273+
tags={"run_script", "mcp_apps_only", "hidden_from_model"},
274274
description="Reject a script; this is only available to the our mcp-app",
275275
app=AppConfig(visibility=["app"]),
276276
)
@@ -357,7 +357,7 @@ async def run_script_interactive(
357357

358358

359359
@mcp.tool(
360-
tags={"run_script", "hidden_from_model"},
360+
tags={"run_script", "mcp_apps_only", "hidden_from_model"},
361361
title="Get the execution state with request ID",
362362
description="Get the execution state with request ID",
363363
app=AppConfig(visibility=["app"]),

0 commit comments

Comments
 (0)