diff --git a/docs/developer-guide/mcp-server.mdx b/docs/developer-guide/mcp-server.mdx index 186786651f..71e38cb4f3 100644 --- a/docs/developer-guide/mcp-server.mdx +++ b/docs/developer-guide/mcp-server.mdx @@ -44,6 +44,11 @@ The main server orchestrates three sub-servers with prefixed namespacing: mcp_server/prowler_mcp_server/ ├── server.py # Main orchestrator ├── main.py # CLI entry point +├── lib/ +│ ├── server.py # ProwlerMCP, the base class of every sub-server +│ ├── errors.py # Exception types and the one error renderer +│ ├── logger.py +│ └── analytics.py ├── prowler_hub/ ├── prowler_app/ │ ├── tools/ # Tool implementations @@ -59,6 +64,8 @@ The MCP Server uses two patterns for tool registration: 1. **Direct Decorators** (Prowler Hub/Docs): Tools are registered using `@mcp.tool()` decorators 2. **Auto-Discovery** (`prowler_app`): All public methods of `BaseTool` subclasses are auto-registered +Both funnel through `ProwlerMCP.tool` (`lib/server.py`), which is what applies the error contract to every tool no matter how it was registered. Build sub-servers with `ProwlerMCP`, never `FastMCP` directly. + ## Adding Tools to the `prowler_app` Sub-Server ### Step 1: Create the Tool Class @@ -120,12 +127,10 @@ class NewFeatureTools(BaseTool): Returns complete feature details including configuration and metadata. """ - try: - response = await self.api_client.get(f"/api/v1/features/{feature_id}") - return DetailedFeature.from_api_response(response["data"]).model_dump() - except Exception as e: - self.logger.error(f"Failed to get feature {feature_id}: {e}") - return {"error": str(e), "status": "failed"} + # No try/except: a failure here raises, and the tool wrapper turns it into a + # ToolError the client sees as `isError: true`. See "Error Handling" below. + response = await self.api_client.get(f"/api/v1/features/{feature_id}") + return DetailedFeature.from_api_response(response["data"]).model_dump() ``` ### Step 2: Create the Models @@ -369,18 +374,101 @@ async def search_items(self, status: str = Field(...)) -> dict: ### Error Handling -Return structured error responses instead of raising exceptions: +Let failures raise. Every sub-server is a `ProwlerMCP` (`prowler_mcp_server/lib/server.py`), +whose `tool()` wraps whatever it registers in `tool_errors`, turning any exception into a +`ToolError`. The client sees `isError: true` and a message it can act on. + +That wrapping is not something you apply — the two registration styles (the `@mcp.tool()` +decorators, and the direct `mcp.tool(fn)` call `BaseTool` uses) both funnel through +`ProwlerMCP.tool`. Build sub-servers with `ProwlerMCP`, never `FastMCP` directly: masking +is on everywhere, so a tool that escaped the funnel would answer `Error calling tool 'x'` +with no detail at all. + +Never `return {"error": ...}`: a returned payload is `isError: false` at the MCP protocol +level, so the client is told the call succeeded and only finds out otherwise if it happens +to inspect the right key. ```python async def get_item(self, item_id: str) -> dict: - try: - response = await self.api_client.get(f"/api/v1/items/{item_id}") - return DetailedItem.from_api_response(response["data"]).model_dump() - except Exception as e: - self.logger.error(f"Failed to get item {item_id}: {e}") - return {"error": str(e), "status": "failed"} + """A rejected request, a timeout and a malformed payload all raise from here. + + Each is rendered with the API's own words plus what it implies about retrying. + """ + response = await self.api_client.get(f"/api/v1/items/{item_id}") + return DetailedItem.from_api_response(response["data"]).model_dump() ``` +Raise `ToolError` whenever the message is one you wrote for the caller. Its text reaches +the client verbatim, so anything they need in order to recover has to be *in* the message +— an error carries nothing else: + +```python +from fastmcp.exceptions import ToolError + +if not data: + raise ToolError( + f"Item '{item_id}' was not found. Use prowler_list_items to find valid IDs." + ) +``` + +**Do not raise `ValueError` from a tool.** The two are not interchangeable: anything that +is not a `ToolError` is described as a bug in this server. That is right for a model +factory rejecting an API payload or a pydantic `ValidationError`, and wrong for a +refusal — so the exception type is what carries the distinction: + +```text +Date range cannot exceed 2 days. Requested range: 2025-01-01 to 2025-01-10 (10 days) + +The Prowler MCP Server hit an unexpected ValueError: Missing pagination metadata in API +response. This is a bug in the server, not something you can fix by changing the +arguments. +``` + +If you surface an exception yourself — into a `ToolError` you build, or into a field of a +structured result — pass it through `render_tool_error(e)` rather than `str(e)`, so the +same failure is never described two ways. Pass `warn=False` when the result already +reports the outcome. + +#### Deciding between an error and a result + +Ask two questions, in order: + +1. **Did the tool finish its own job?** `test_integration_connection`'s job is to run the + check and report what happened, so `connected: false` is the job finished. + `get_finding_details`' job is to return the finding, so no finding means it did not. +2. **Is the reported state a fact about the remote world or about our call?** The world + (Jira refused the credentials, 3 of 40 items failed, a discovery found nothing) is a + **result**. Our call (403, connection reset, invalid UUID, a bug in a model factory) is + an **error**. + +One rule overrides both: **if a write may have partially landed, that fact travels in a +successful structured result, never in an error.** An agent reads `isError: true` as +"nothing happened, safe to retry"; reporting "I may have created 17 Jira issues" that way +invites a duplicate dispatch. + +#### What the client reads + +`render_tool_error` describes the failure in one plain sentence: the call, the status and +whatever the API said, with the field named when it named one. + +```text +GET /findings/b1ca536c failed with HTTP 404. No Finding matches the given query. +POST /integrations failed with HTTP 400. This field may not be blank. (/data/attributes/configuration/bucket_name); Enter a valid URL. +Date range cannot exceed 2 days. Requested range: 2025-01-01 to 2025-01-10 (10 days) +``` + +Nothing is added that the status code already implies. The one exception is a request that +could have changed something and never came back with a verdict — a 5xx or a timeout on a +write — which gets a warning, because an agent otherwise reads any failure as "nothing +happened" and sends the write again: + +```text +DELETE /integrations/i1 failed with HTTP 500. A server error occurred. It may have been carried out anyway, so check the current state before retrying. +``` + +Every server sets `mask_error_details=True`. That costs nothing, because `ToolError` +bypasses masking; it only stops raw internals escaping from code paths outside a tool. + ### Parameter Descriptions Use Pydantic `Field()` with clear descriptions. This also helps LLMs understand diff --git a/mcp_server/AGENTS.md b/mcp_server/AGENTS.md index e786f9a5b7..f246a78122 100644 --- a/mcp_server/AGENTS.md +++ b/mcp_server/AGENTS.md @@ -26,6 +26,8 @@ The Prowler MCP Server provides AI agents access to the Prowler ecosystem throug ## CRITICAL RULES ### Tool Implementation +- ALWAYS: Build sub-servers with `ProwlerMCP`, never `FastMCP` directly. It is what + applies the error contract to every tool, whichever way it is registered - ALWAYS: Extend `BaseTool` ABC for Prowler tools (auto-registration) - ALWAYS: Use `@mcp.tool()` decorator for Hub/Docs tools - NEVER: Manually register BaseTool subclasses @@ -42,6 +44,37 @@ The Prowler MCP Server provides AI agents access to the Prowler ecosystem throug - ALWAYS: Use `build_filter_params()` for query parameters - NEVER: Create new httpx clients in tools +### Errors +One rule: **`ToolError` is a message you wrote for the caller. Any other exception is +a bug or an upstream failure**, and `render_tool_error` describes it. + +- ALWAYS: `raise ToolError(...)` for anything the caller can act on — a rejected + argument, a lookup that found nothing, a workflow step they must do first. Its text + reaches the client verbatim, past `mask_error_details` +- NEVER: `raise ValueError(...)` in a tool. It is reported as a bug in this server, + which is correct for a model factory rejecting an API payload and wrong for a + refusal +- ALWAYS: Let an upstream failure propagate untouched. `ProwlerMCP.tool` wraps every + registration, so it becomes a `ToolError` describing the call, the status and what + the API said. There is nothing to remember to apply +- NEVER: `return {"error": ...}` or `{"success": False}`. A returned payload is + `isError: false`, so the client is told the call succeeded +- NEVER: Raise a plain exception *after* a write has been accepted. `ToolError` is the + only kind whose message reaches the client exactly as written +- ALWAYS: `render_tool_error(e)` when you surface an exception yourself, so a failure + is never described two different ways. Use `warn=False` when embedding it in a + result that already reports the outcome +- ALWAYS: Return a structured result, not an error, when a write may have partially + landed (`status="unknown"`, `deleted="unknown"`, `safe_to_retry=False`). An agent + reads `isError: true` as "nothing happened, safe to retry" +- ALWAYS: Return a structured result for an outcome that *is* the tool's job to + report: `connected: false`, an empty list, an idempotent no-op +- NEVER: Wrap a whole tool body in `except Exception`. It reports bugs in this + server as API failures, and the wrapper already handles the rest + +See `prowler_mcp_server/lib/errors.py` and +`docs/developer-guide/mcp-server.mdx` for the message format. + --- ## ARCHITECTURE @@ -72,6 +105,9 @@ Python 3.12+ | FastMCP 3.4.4 | httpx (async) | Pydantic | uv | pytest ```text mcp_server/prowler_mcp_server/ ├── server.py # Main orchestration +├── lib/ +│ ├── server.py # ProwlerMCP: base class of every sub-server +│ └── errors.py # Exception types + render_tool_error ├── prowler_hub/server.py # Hub tools (no auth) ├── prowler_app/ │ ├── server.py @@ -113,7 +149,8 @@ make test-mcp # Run the MCP test suite exactly as CI does - [ ] Models use `MinimalSerializerMixin` - [ ] API responses transformed to simplified models - [ ] No hardcoded secrets -- [ ] Error handling returns structured responses +- [ ] Failures raise (never `return {"error": ...}`); outcomes that may have changed + something return a structured result - [ ] Parameter descriptions use Pydantic `Field()` - [ ] Tests added under `mcp_server/tests/`, mirroring the source path below the package root (`prowler_mcp_server/prowler_app/tools/` -> `tests/prowler_app/tools/`), diff --git a/mcp_server/prowler_mcp_server/lib/errors.py b/mcp_server/prowler_mcp_server/lib/errors.py index 1c11ebf63c..59e61d4820 100644 --- a/mcp_server/prowler_mcp_server/lib/errors.py +++ b/mcp_server/prowler_mcp_server/lib/errors.py @@ -1,14 +1,17 @@ -"""What a Prowler MCP Server failure is, and the sentence it is described with. +"""One way to fail: every tool failure reaches the client as a `ToolError`. MCP draws a line this server used to blur. A tool that *returns* `{"error": ...}` produces a successful result (`isError: false`) whose failure is only discoverable by guessing which key to look at; a tool that *raises* produces `isError: true`, which -every client and model already understands as "this call did not work". Moving the -server onto the second of those needs one vocabulary of failures and one way to render -them, which is what this module is; raising them is the sub-servers' job. +every client and model already understands as "this call did not work". -`render_tool_error` is the single place an exception becomes text a client reads, so -the same failure can never get described two different ways. +`ToolError` is a `FastMCPError`, and `FastMCP._call_tool` re-raises those untouched +(fastmcp/server/server.py:1241). So a message built here is what the client reads, +verbatim, past every mount and past `mask_error_details`. That is what lets the servers +mask by default while still telling the caller everything relevant. + +`render_tool_error` is the single place an exception becomes that text, and +`tool_errors` is what makes sure no tool can escape it. The exception types live here rather than next to the API client because the hub and the documentation sub-servers must be able to raise and render them without taking a @@ -17,11 +20,17 @@ dependency on `prowler_app`. from __future__ import annotations +import functools +import inspect import json +from collections.abc import Callable from dataclasses import dataclass from typing import Any import httpx +from fastmcp.exceptions import ToolError + +from prowler_mcp_server.lib.logger import logger # Upstream bodies are not ours and may be large or HTML; enough to diagnose, not enough # to flood the model's context. @@ -286,3 +295,53 @@ def render_tool_error(error: Exception, *, warn: bool = True) -> str: "This is a bug in the server, not something you can fix by changing the " "arguments." ) + + +def tool_errors(fn: Callable[..., Any]) -> Callable[..., Any]: + """Wrap a tool so that every failure leaves it as a `ToolError`. + + Applied by `ProwlerMCP.tool()` rather than by hand, so no registration can miss + it. It wraps the callable handed to `mcp.tool()`, not the class attribute, so only + the MCP boundary is normalised: a tool calling another tool internally still sees + the real, typed exception and can branch on it. + + Two constraints worth knowing before changing this: + + - Never register the result with `exclude_args=`. That path + (fastmcp/utilities/types.py) rebuilds the function from `__code__`, which on a + wrapper is the wrapper's own. Nothing in this server passes it today. + - `inspect.iscoroutinefunction`, not the `asyncio` one, which is deprecated from + Python 3.14 and would be an error under this project's warning filters. + """ + name = getattr(fn, "__qualname__", repr(fn)) + + def mark(wrapper: Callable[..., Any]) -> Callable[..., Any]: + """Flag the wrapper so a test can prove every registered tool went through it.""" + wrapper.__prowler_tool_errors__ = True # ty: ignore[unresolved-attribute] + return wrapper + + if inspect.iscoroutinefunction(fn): + + @functools.wraps(fn) + async def async_wrapper(*args: Any, **kwargs: Any) -> Any: + try: + return await fn(*args, **kwargs) + except ToolError: + raise + except Exception as error: + logger.exception(f"Tool {name} failed: {error}") + raise ToolError(render_tool_error(error)) from error + + return mark(async_wrapper) + + @functools.wraps(fn) + def sync_wrapper(*args: Any, **kwargs: Any) -> Any: + try: + return fn(*args, **kwargs) + except ToolError: + raise + except Exception as error: + logger.exception(f"Tool {name} failed: {error}") + raise ToolError(render_tool_error(error)) from error + + return mark(sync_wrapper) diff --git a/mcp_server/prowler_mcp_server/lib/server.py b/mcp_server/prowler_mcp_server/lib/server.py new file mode 100644 index 0000000000..d953e8d340 --- /dev/null +++ b/mcp_server/prowler_mcp_server/lib/server.py @@ -0,0 +1,37 @@ +"""The FastMCP subclass every Prowler sub-server is built from.""" + +from typing import Any + +from fastmcp import FastMCP + +from prowler_mcp_server.lib.errors import tool_errors + + +class ProwlerMCP(FastMCP): + """A FastMCP server whose tools all report failures the same way. + + `FastMCP.tool()` is the single funnel every registration goes through -- the + `@server.tool()` and bare `@server.tool` decorator forms, and the direct + `mcp.tool(fn)` call `BaseTool` uses to auto-register -- so applying + `tool_errors` here covers all of them at once. + + It is applied here rather than by hand because forgetting it is silent and + expensive: every server sets `mask_error_details=True`, so an unwrapped tool + answers `Error calling tool 'x'` and nothing else. Nothing about the tool looks + wrong, and the schema, the name and the description are all still correct. + """ + + def tool(self, name_or_fn: Any = None, **kwargs: Any) -> Any: + """Register a tool, wrapped so its failures reach the client as `ToolError`.""" + if callable(name_or_fn): + # Direct call: mcp.tool(fn), or the bare @mcp.tool decorator. + return super().tool(tool_errors(name_or_fn), **kwargs) + + # Parameterised decorator: @mcp.tool() or @mcp.tool(name="..."). FastMCP hands + # back the decorator that does the registering, so the wrap goes in front of it. + register = super().tool(name_or_fn, **kwargs) + + def decorator(fn: Any) -> Any: + return register(tool_errors(fn)) + + return decorator diff --git a/mcp_server/prowler_mcp_server/prowler_app/server.py b/mcp_server/prowler_mcp_server/prowler_app/server.py index e8e854144f..c64bfa7594 100644 --- a/mcp_server/prowler_mcp_server/prowler_app/server.py +++ b/mcp_server/prowler_mcp_server/prowler_app/server.py @@ -1,9 +1,8 @@ -from fastmcp import FastMCP - +from prowler_mcp_server.lib.server import ProwlerMCP from prowler_mcp_server.prowler_app.utils.tool_loader import load_all_tools # Initialize MCP server -app_mcp_server = FastMCP("prowler-app") +app_mcp_server = ProwlerMCP("prowler-app", mask_error_details=True) # Auto-discover and load all tools from the tools package load_all_tools(app_mcp_server) diff --git a/mcp_server/prowler_mcp_server/prowler_app/tools/base.py b/mcp_server/prowler_mcp_server/prowler_app/tools/base.py index ec54969ba2..fb03371e25 100644 --- a/mcp_server/prowler_mcp_server/prowler_app/tools/base.py +++ b/mcp_server/prowler_mcp_server/prowler_app/tools/base.py @@ -72,6 +72,9 @@ class BaseTool(ABC): async methods (not starting with '_') as tools. Subclasses do not need to override this method. + Failures need no handling here: `ProwlerMCP.tool` wraps whatever it is + given, so every tool reports them the same way. + Args: mcp: The FastMCP instance to register tools with """ diff --git a/mcp_server/prowler_mcp_server/prowler_documentation/server.py b/mcp_server/prowler_mcp_server/prowler_documentation/server.py index 7cd8825e4b..c87162f2d2 100644 --- a/mcp_server/prowler_mcp_server/prowler_documentation/server.py +++ b/mcp_server/prowler_mcp_server/prowler_documentation/server.py @@ -1,14 +1,14 @@ from typing import Any -from fastmcp import FastMCP from pydantic import Field +from prowler_mcp_server.lib.server import ProwlerMCP from prowler_mcp_server.prowler_documentation.search_engine import ( ProwlerDocsSearchEngine, ) -# Initialize FastMCP server -docs_mcp_server = FastMCP("prowler-docs") +# Initialize MCP server +docs_mcp_server = ProwlerMCP("prowler-docs", mask_error_details=True) prowler_docs_search_engine = ProwlerDocsSearchEngine() diff --git a/mcp_server/prowler_mcp_server/prowler_hub/server.py b/mcp_server/prowler_mcp_server/prowler_hub/server.py index 41e83eca90..d753b03473 100644 --- a/mcp_server/prowler_mcp_server/prowler_hub/server.py +++ b/mcp_server/prowler_mcp_server/prowler_hub/server.py @@ -5,13 +5,13 @@ Provides access to Prowler Hub API for security checks and compliance frameworks """ import httpx -from fastmcp import FastMCP from pydantic import Field from prowler_mcp_server import __version__ +from prowler_mcp_server.lib.server import ProwlerMCP -# Initialize FastMCP for Prowler Hub -hub_mcp_server = FastMCP("prowler-hub") +# Initialize MCP server for Prowler Hub +hub_mcp_server = ProwlerMCP("prowler-hub", mask_error_details=True) # API base URL BASE_URL = "https://hub.prowler.com/api" diff --git a/mcp_server/prowler_mcp_server/server.py b/mcp_server/prowler_mcp_server/server.py index 1b3271be38..fc182a7964 100644 --- a/mcp_server/prowler_mcp_server/server.py +++ b/mcp_server/prowler_mcp_server/server.py @@ -1,10 +1,10 @@ -from fastmcp import FastMCP from starlette.responses import JSONResponse from prowler_mcp_server import __version__ from prowler_mcp_server.lib.logger import logger +from prowler_mcp_server.lib.server import ProwlerMCP -prowler_mcp_server = FastMCP("prowler-mcp-server") +prowler_mcp_server = ProwlerMCP("prowler-mcp-server", mask_error_details=True) def setup_main_server(): diff --git a/mcp_server/tests/lib/test_server.py b/mcp_server/tests/lib/test_server.py new file mode 100644 index 0000000000..449776a64f --- /dev/null +++ b/mcp_server/tests/lib/test_server.py @@ -0,0 +1,170 @@ +"""Tests for the server class every sub-server is built from. + +These drive a real `ProwlerMCP` through an in-memory MCP client rather than calling +`tool_errors` directly, because applying that wrapper by hand is exactly what this +class exists to make unnecessary. What matters is that a tool registered *any* of the +ways this server registers them ends up with the error contract, and that it keeps the +name, description and schema FastMCP publishes. +""" + +import pytest +from fastmcp import Client +from fastmcp.exceptions import ToolError +from pydantic import Field + +from prowler_mcp_server.lib.errors import ProwlerAPIError +from prowler_mcp_server.lib.server import ProwlerMCP + + +async def call(server: ProwlerMCP, name: str, arguments: dict | None = None): + """Call a tool the way a client does, without raising on failure.""" + async with Client(server) as client: + return await client.call_tool(name, arguments or {}, raise_on_error=False) + + +async def test_the_parameterised_decorator_form_is_wrapped(): + """`@mcp.tool()` -- how the hub and documentation sub-servers register.""" + server = ProwlerMCP("test", mask_error_details=True) + + @server.tool() + async def failing() -> dict: + """A tool that fails.""" + raise ProwlerAPIError("boom", 404, method="GET", path="/x") + + result = await call(server, "failing") + + assert result.is_error + assert result.content[0].text == "GET /x failed with HTTP 404." + + +async def test_the_bare_decorator_form_is_wrapped(): + """`@mcp.tool` without parentheses is a different code path in FastMCP.""" + server = ProwlerMCP("test", mask_error_details=True) + + @server.tool + async def failing() -> dict: + """A tool that fails.""" + raise ProwlerAPIError("boom", 500, method="GET", path="/y") + + result = await call(server, "failing") + + assert result.is_error + assert "GET /y failed with HTTP 500." in result.content[0].text + + +async def test_the_direct_call_form_is_wrapped(): + """`mcp.tool(fn)` -- how `BaseTool` auto-registers its methods.""" + server = ProwlerMCP("test", mask_error_details=True) + + async def failing() -> dict: + """A tool that fails.""" + raise ProwlerAPIError("boom", 403, method="DELETE", path="/z") + + server.tool(failing) + + result = await call(server, "failing") + + assert result.is_error + assert "DELETE /z failed with HTTP 403." in result.content[0].text + + +async def test_a_synchronous_tool_is_wrapped(): + """The documentation sub-server registers plain `def` tools.""" + server = ProwlerMCP("test", mask_error_details=True) + + @server.tool() + def failing() -> dict: + """A synchronous tool that fails.""" + raise KeyError("attributes") + + result = await call(server, "failing") + + assert result.is_error + assert "unexpected KeyError" in result.content[0].text + + +async def test_a_refusal_reaches_the_caller_word_for_word(): + """A `ToolError` is passed through untouched, masking included. + + That is the whole reason refusals are raised as one: the tool already wrote the + sentence the caller needs, and nothing downstream improves on it. + """ + server = ProwlerMCP("test", mask_error_details=True) + + @server.tool() + async def refusing() -> dict: + """A tool that refuses its arguments.""" + raise ToolError( + "Date range cannot exceed 2 days. Requested range: 2025-01-01 to " + "2025-01-10 (10 days)" + ) + + result = await call(server, "refusing") + + assert result.is_error + assert result.content[0].text == ( + "Date range cannot exceed 2 days. Requested range: 2025-01-01 to " + "2025-01-10 (10 days)" + ) + + +async def test_a_result_is_passed_through_untouched(): + server = ProwlerMCP("test") + + @server.tool() + async def succeeding(value: int) -> dict: + """A tool that works.""" + return {"value": value} + + result = await call(server, "succeeding", {"value": 3}) + + assert not result.is_error + assert result.data == {"value": 3} + + +async def test_wrapping_does_not_disturb_the_published_tool(): + """The wrapper must be invisible to FastMCP's schema generation. + + A wrapper that loses the signature takes the parameters with it, and a tool with no + parameters and no description is unusable while still looking registered. + """ + server = ProwlerMCP("test") + + @server.tool() + async def search( + query: str = Field(description="What to search for"), + limit: int = Field(default=10, description="How many results"), + ) -> dict: + """Search for things.""" + return {"query": query, "limit": limit} + + async with Client(server) as client: + (tool,) = await client.list_tools() + + assert tool.name == "search" + assert tool.description == "Search for things." + properties = tool.inputSchema["properties"] + assert properties["query"]["description"] == "What to search for" + assert properties["limit"]["default"] == 10 + + +@pytest.mark.parametrize("name", ["decorated", "direct"]) +async def test_every_registration_carries_the_marker(name): + """The marker is what lets the contract test prove no tool slipped past.""" + server = ProwlerMCP("test") + + async def direct() -> dict: + """Registered by direct call.""" + return {} + + @server.tool() + async def decorated() -> dict: + """Registered by decorator.""" + return {} + + server.tool(direct) + + tool = await server.get_tool(name) + assert tool is not None, f"{name!r} was not registered at all" + # `get_tool` is typed as the base `Tool`; only `FunctionTool` carries `fn`. + assert getattr(getattr(tool, "fn", None), "__prowler_tool_errors__", False) diff --git a/mcp_server/tests/test_server.py b/mcp_server/tests/test_server.py index 30ab1fc92c..07dcdc6240 100644 --- a/mcp_server/tests/test_server.py +++ b/mcp_server/tests/test_server.py @@ -33,6 +33,34 @@ async def test_every_sub_server_contributes_tools(mcp_root_server): assert tools_in_namespace(tools, "prowler_"), "Prowler App registered no tools" +async def test_no_tool_disappears_between_registration_and_the_client(mcp_root_server): + """Every tool registered on a sub-server must still be reachable through the mount. + + `ProwlerMCP.tool` wraps every tool before handing it to FastMCP, whether it arrived + by decorator or by the direct call `BaseTool.register_tools` makes. A wrapper that + loses the signature, the name or the coroutine-ness of what it wraps drops the tool + silently: the mount still succeeds and the count is the only thing that moves. + """ + from prowler_mcp_server.prowler_app.server import app_mcp_server + from prowler_mcp_server.prowler_documentation.server import docs_mcp_server + from prowler_mcp_server.prowler_hub.server import hub_mcp_server + + async with Client(mcp_root_server) as client: + tools = await client.list_tools() + + for namespace, sub_server in ( + ("prowler_hub_", hub_mcp_server), + ("prowler_docs_", docs_mcp_server), + ("prowler_", app_mcp_server), + ): + expected = len(await sub_server.list_tools()) + published = len(tools_in_namespace(tools, namespace)) + assert published == expected, ( + f"'{namespace}' publishes {published} tools but its sub-server registered " + f"{expected}" + ) + + async def test_every_tool_is_namespaced(mcp_root_server): """Tool names are a published interface; nothing may escape the namespaces.""" async with Client(mcp_root_server) as client: