ben.carrasco/prowler
1
0
Fork 0
mirror of https://github.com/prowler-cloud/prowler.git synced 2026-08-19 09:30:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(mcp): add a single vocabulary for tool failures

Browse Source 
Adds lib/errors.py: the exception types the sub-servers raise for a
failure they can describe -- an API rejection, a task that never
finished, bad credentials, a Hub outage -- and render_tool_error, the
one place any of them becomes text a client reads.

Nothing imports it yet. The wrapper that turns these into an MCP error
on every tool, and the servers that raise them, follow in this stack.
This commit is contained in:
Rubén De la Torre Vico
2026-08-12 13:16:38 +02:00
parent 37ebd9b6fd
commit c37e2b90dc
3 changed files with 599 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mcp_server/prowler_mcp_server/lib/errors.py
+288
View File
@@ -0,0 +1,288 @@
"""What a Prowler MCP Server failure is, and the sentence it is described with.
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.
`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.
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
dependency on `prowler_app`.
"""
from __future__ import annotations
import json
from dataclasses import dataclass
from typing import Any
import httpx
# Upstream bodies are not ours and may be large or HTML; enough to diagnose, not enough
# to flood the model's context.
_MAX_UPSTREAM_BODY = 500
# The one thing a status code does not say. Appended only when a request that could have
# changed something did not come back with a verdict, because an agent reads a failure as
# "nothing happened" and will happily send the write again.
_UNKNOWN_OUTCOME = (
" It may have been carried out anyway, so check the current state before retrying."
)
@dataclass(frozen=True, slots=True)
class ApiErrorDetail:
"""One entry of a JSON:API `errors` array.
The API answers a rejected write with one error *per invalid field*, each naming the
field in `source`. Keeping the whole shape is what turns "the request was invalid"
into "these two fields were invalid, and here is which".
Field names are JSON:API's own. `source.pointer` and `source.parameter` are not two
spellings of one thing: a pointer is a JSON Pointer into the request *document*
(`/data/attributes/provider_id`, as `alerts/errors.py` and `tasks/beat.py` send),
while a parameter is a *query parameter* name (`page[size]`, `lookback_days`, as
`api/v1/views.py` sends). The spec has a third, `source.header`; the Prowler API
never emits one, so there is nothing here to read it into.
"""
detail: str | None = None
title: str | None = None
pointer: str | None = None
"""JSON:API `source.pointer`: a JSON Pointer into the request document."""
parameter: str | None = None
"""JSON:API `source.parameter`: the query parameter that caused the error."""
@classmethod
def from_jsonapi(cls, error: dict[str, Any]) -> ApiErrorDetail:
"""Build from a single member of a JSON:API `errors` array.
`source` is optional and most errors omit it, so it supplies the location only,
never whether there is a detail worth reporting.
"""
source = error.get("source", {})
return cls(
detail=error.get("detail"),
title=error.get("title"),
pointer=source.get("pointer"),
parameter=source.get("parameter"),
)
def render(self) -> str:
"""The error text, and where the API said it is.
A pointer is left as-is because a leading `/` already reads as a path into the
body. A parameter is labelled, since `(page[size])` on its own would read like
one.
"""
text = self.detail or self.title or ""
if not text:
return ""
if self.pointer:
return f"{text} ({self.pointer})"
if self.parameter:
return f"{text} (parameter {self.parameter})"
return text
def parse_jsonapi_errors(payload: Any) -> tuple[ApiErrorDetail, ...]:
"""Extract every error from a JSON:API error document.
Tolerant on purpose: this runs while handling a failure, and a body that is not the
document it should be must not turn a useful API error into a parsing traceback.
"""
if not isinstance(payload, dict):
return ()
errors = payload.get("errors")
if not isinstance(errors, list):
return ()
return tuple(
ApiErrorDetail.from_jsonapi(error)
for error in errors
if isinstance(error, dict)
)
class ProwlerAPIError(Exception):
"""An error response returned by the Prowler API.
Raised only when the API answered with an error status, which tells a caller
something no plain exception can: the request reached Prowler and was
rejected, so it changed nothing. A timeout or a dropped connection stays a
bare exception because the request may well have been processed.
"""
def __init__(
self,
message: str,
status_code: int,
*,
method: str | None = None,
path: str | None = None,
errors: tuple[ApiErrorDetail, ...] = (),
) -> None:
super().__init__(message)
self.status_code: int = status_code
# Stored as plain strings so this module stays independent of the API client's
# HTTPMethod enum; StrEnum members compare equal to their value either way.
self.method: str | None = str(method) if method is not None else None
self.path: str | None = path
self.errors: tuple[ApiErrorDetail, ...] = tuple(errors)
@property
def rejected(self) -> bool:
"""The request reached Prowler and was refused, so it changed nothing."""
return 400 <= self.status_code < 500
class ProwlerTaskError(Exception):
"""A background task this server was waiting on did not complete.
Separate from `ProwlerAPIError` because the API already accepted the work: the
task exists and may still be running, so the outcome is unknown rather than refused.
"""
def __init__(self, message: str, *, task_id: str, state: str) -> None:
super().__init__(message)
self.task_id: str = task_id
self.state: str = state
"""One of `timeout`, `failed` or `cancelled`."""
class ProwlerAuthError(ValueError):
"""The credentials are missing, malformed or expired.
Subclasses `ValueError` so that the handlers which already treat an
authentication failure as a refusal-before-send keep working. It is matched by name
in `render_tool_error` rather than by that base class, so it is described as the
credential problem it is instead of falling through to the bug branch.
"""
class ProwlerHubError(Exception):
"""The Prowler Hub answered with an error status.
The Hub is a separate public service with its own client, so its failures cannot be
`ProwlerAPIError`. Everything the Hub exposes is a read.
"""
def __init__(
self,
message: str,
*,
status_code: int,
path: str,
body: str | None = None,
) -> None:
super().__init__(message)
self.status_code: int = status_code
self.path: str = path
self.body: str | None = body
def _upstream_detail(body: str | None) -> str:
"""A readable line from an error body that is not JSON:API.
The Prowler API answers with a JSON:API document, which is parsed into
`ApiErrorDetail`. Every other host this server talks to has its own shape: the Hub
answers `{"error": "Not found"}`, GitHub answers plain text, a proxy in between may
answer HTML. Relaying any of those verbatim puts braces and markup in front of the
model, so the message is pulled out when there is one and truncated when there is
not.
"""
if not body:
return ""
text = body.strip()
try:
parsed = json.loads(text)
except ValueError:
parsed = None
if isinstance(parsed, dict):
for key in ("error", "message", "detail"):
value = parsed.get(key)
if isinstance(value, str) and value:
return value
return f"{text[:_MAX_UPSTREAM_BODY]}..." if len(text) > _MAX_UPSTREAM_BODY else text
def render_tool_error(error: Exception, *, warn: bool = True) -> str:
"""Describe an exception in the plainest sentence that keeps every useful detail.
Callers that surface a failure from anywhere other than a raised exception -- a
message they write themselves, a field of a structured result -- go through here
too, which is what keeps one failure from being described two different ways.
Pass `warn=False` when the caller already states the outcome, which a structured
result reporting `status="unknown"` does by definition. Otherwise the generic
warning lands next to a more specific one saying the same thing.
Ordered most specific first: `ProwlerAuthError` is a `ValueError` and
`httpx.TimeoutException` is a `RequestError`, so the general branches come last.
"""
unknown = _UNKNOWN_OUTCOME if warn else ""
if isinstance(error, ProwlerAPIError):
operation = (
" ".join(p for p in (error.method, error.path) if p) or "The request"
)
details = "; ".join(text for text in (d.render() for d in error.errors) if text)
message = f"{operation} failed with HTTP {error.status_code}."
if details:
message = f"{message} {details}"
# A 4xx is a refusal, so it changed nothing and needs no warning.
if error.rejected or error.method == "GET":
return message
return message + unknown
if isinstance(error, ProwlerTaskError):
# The API accepted the work before the wait failed, so the outcome is open
# whichever way the task ended.
return f"{error}{unknown}"
if isinstance(error, ProwlerAuthError):
return f"Prowler authentication failed: {error}"
if isinstance(error, ProwlerHubError):
# Same shape as the API branch, with the service named because the Hub can be
# down while the API is fine. Everything the Hub exposes is a GET.
message = f"Prowler Hub GET {error.path} failed with HTTP {error.status_code}."
detail = _upstream_detail(error.body)
return f"{message} {detail}" if detail else message
if isinstance(error, httpx.HTTPStatusError):
# An upstream that is not the Prowler API, such as the external-URL fetch.
request = error.request
message = (
f"{request.method} {request.url} failed with HTTP "
f"{error.response.status_code}."
)
detail = _upstream_detail(error.response.text)
return f"{message} {detail}" if detail else message
if isinstance(error, httpx.RequestError):
# No answer at all: a timeout, a dropped connection, a DNS failure.
try:
operation = f"{error.request.method} {error.request.url}"
method = error.request.method
except RuntimeError:
# httpx only attaches the request once it has one, and reading it before
# then raises. Never let that hide the failure being reported.
operation, method = "The request", None
suffix = "" if method == "GET" else unknown
return f"{operation} got no answer ({type(error).__name__}: {error}).{suffix}"
# No `ValueError` branch, deliberately. A message written for the caller is raised
# as a `ToolError`, which never reaches here. What is left -- a model factory
# rejecting an API payload, a pydantic `ValidationError`, an `int()` on something
# that is not a number -- is this server or the API breaking its own contract, and
# saying so is the only useful thing to tell a caller who cannot fix it.
return (
f"The Prowler MCP Server hit an unexpected {type(error).__name__}: {error}. "
"This is a bug in the server, not something you can fix by changing the "
"arguments."
)
mcp_server/tests/lib/__init__.py
View File
mcp_server/tests/lib/test_errors.py
+311
View File
@@ -0,0 +1,311 @@
"""Tests for the sentence a failure is described with.
These assert on the *text* a model reads, because that text is the whole contract: a
`ToolError` carries nothing else. What matters is that the API's own words survive
intact, and that a write whose outcome nobody can report says so.
How that sentence reaches a client -- and that no tool can escape it -- is
`test_server.py`.
"""
import httpx
import pytest
from prowler_mcp_server.lib.errors import (
ApiErrorDetail,
ProwlerAPIError,
ProwlerAuthError,
ProwlerHubError,
ProwlerTaskError,
parse_jsonapi_errors,
render_tool_error,
)
MAY_HAVE_LANDED = "It may have been carried out anyway"
# --------------------------------------------------------------------------- parsing
def test_every_error_of_the_document_is_preserved():
"""A rejected write names one error per invalid field; all of them matter."""
errors = parse_jsonapi_errors(
{
"errors": [
{"status": "400", "detail": "This field may not be blank."},
{"status": "400", "detail": "Enter a valid URL."},
]
}
)
assert [error.detail for error in errors] == [
"This field may not be blank.",
"Enter a valid URL.",
]
def test_a_query_parameter_error_is_not_dressed_up_as_a_body_path():
"""`source.parameter` and `source.pointer` are different places, per JSON:API.
The API sends a parameter for a bad query string (`api/v1/views.py` answers
`page[size]` and `lookback_days` that way) and a pointer for a bad body field.
Rendering a parameter bare would read as though `page[size]` were a path into the
document, which is somewhere the caller never put it.
"""
(error,) = parse_jsonapi_errors(
{
"errors": [
{
"detail": "invalid parameter 'page[size]'",
"source": {"parameter": "page[size]"},
}
]
}
)
assert error.parameter == "page[size]"
assert error.pointer is None
assert error.render() == "invalid parameter 'page[size]' (parameter page[size])"
def test_the_field_an_error_points_at_is_kept():
"""`source.pointer` is what turns "invalid" into "this field is invalid"."""
(error,) = parse_jsonapi_errors(
{
"errors": [
{
"detail": "This field may not be blank.",
"source": {"pointer": "/data/attributes/name"},
}
]
}
)
assert error.render() == ("This field may not be blank. (/data/attributes/name)")
def test_an_error_with_only_a_title_still_says_something():
"""`detail` is the useful field, but the API does not always send one."""
assert ApiErrorDetail(title="Not Found").render() == "Not Found"
@pytest.mark.parametrize(
"payload",
[None, "not a document", {}, {"errors": "not a list"}, {"errors": [None]}],
ids=["none", "text", "empty", "errors-not-a-list", "member-not-a-dict"],
)
def test_a_body_that_is_not_an_error_document_is_tolerated(payload):
"""Parsing runs while handling a failure; it must not become the failure."""
assert parse_jsonapi_errors(payload) == ()
def test_an_error_with_nothing_in_it_renders_empty():
"""Rendered to nothing rather than to punctuation, so composition can skip it."""
assert ApiErrorDetail().render() == ""
# --------------------------------------------------------------------- api failures
def test_a_failed_read_names_the_call_and_the_reason():
message = render_tool_error(
ProwlerAPIError(
"API request failed: 404 - Not found.",
404,
method="GET",
path="/findings/nope",
errors=parse_jsonapi_errors(
{"errors": [{"detail": "No Finding matches the given query."}]}
),
)
)
assert message == (
"GET /findings/nope failed with HTTP 404. No Finding matches the given query."
)
def test_every_error_of_a_rejected_write_reaches_the_client():
"""The API rejects a write with one error per invalid field, and all of them help."""
message = render_tool_error(
ProwlerAPIError(
"API request failed: 400 - blank",
400,
method="POST",
path="/integrations",
errors=parse_jsonapi_errors(
{
"errors": [
{
"detail": "This field may not be blank.",
"source": {"pointer": "/data/attributes/bucket_name"},
},
{"detail": "Enter a valid URL."},
]
}
),
)
)
assert message == (
"POST /integrations failed with HTTP 400. "
"This field may not be blank. (/data/attributes/bucket_name); "
"Enter a valid URL."
)
def test_a_rejected_write_gets_no_warning():
"""A 4xx changed nothing, so there is nothing to warn about."""
message = render_tool_error(
ProwlerAPIError("boom", 400, method="POST", path="/scans")
)
assert MAY_HAVE_LANDED not in message
def test_a_write_that_hit_a_server_error_warns_it_may_have_landed():
"""The API validates and queues before answering, so a 500 may have gone through."""
message = render_tool_error(
ProwlerAPIError("boom", 500, method="DELETE", path="/integrations/i1")
)
assert message.startswith("DELETE /integrations/i1 failed with HTTP 500.")
assert MAY_HAVE_LANDED in message
def test_a_failed_read_never_warns():
"""A read cannot have changed anything, whatever went wrong."""
message = render_tool_error(
ProwlerAPIError("boom", 500, method="GET", path="/scans")
)
assert MAY_HAVE_LANDED not in message
# ------------------------------------------------------------------ no answer at all
def test_a_write_that_got_no_answer_warns_it_may_have_landed():
"""A timeout is the case the warning exists for."""
request = httpx.Request("POST", "https://api.testing.invalid/api/v1/scans")
message = render_tool_error(httpx.ReadTimeout("timed out", request=request))
assert "POST https://api.testing.invalid/api/v1/scans got no answer" in message
assert "ReadTimeout" in message
assert MAY_HAVE_LANDED in message
def test_a_read_that_got_no_answer_does_not_warn():
request = httpx.Request("GET", "https://api.testing.invalid/api/v1/findings")
message = render_tool_error(httpx.ReadTimeout("timed out", request=request))
assert MAY_HAVE_LANDED not in message
def test_a_dropped_connection_names_what_went_wrong():
request = httpx.Request("POST", "https://api.testing.invalid/api/v1/scans")
message = render_tool_error(httpx.ConnectError("connection reset", request=request))
assert "ConnectError: connection reset" in message
def test_an_unfinished_task_warns_it_may_have_landed():
"""The API already accepted the work, so the outcome is open, not refused."""
message = render_tool_error(
ProwlerTaskError(
"Task t1 polling timed out after 60 seconds.", task_id="t1", state="timeout"
)
)
assert message.startswith("Task t1 polling timed out after 60 seconds.")
assert MAY_HAVE_LANDED in message
# -------------------------------------------------------------- refusals before send
def test_a_stray_value_error_is_reported_as_a_bug():
"""The distinction the previous passthrough branch could not make.
A model factory rejecting an API payload, or an `int()` on something that is not a
number, is not the caller's mistake. Describing it like a validation message sends
an agent off rewriting arguments that were never the problem.
"""
message = render_tool_error(
ValueError("Missing pagination metadata in API response")
)
assert "unexpected ValueError" in message
assert "bug in the server" in message
def test_an_authentication_failure_says_so():
"""A `ValueError` subclass, so it must be recognised before the generic branch."""
assert render_tool_error(ProwlerAuthError("Token has expired")) == (
"Prowler authentication failed: Token has expired"
)
# ---------------------------------------------------------------------- other hosts
def test_a_hub_failure_reads_like_an_api_failure():
"""Same sentence as the Prowler API, with the service named.
The Hub can be down while the API is fine, so which one failed is worth the two
extra words -- but the shape must not differ, or the two look like two contracts.
"""
message = render_tool_error(
ProwlerHubError(
"hub failed",
status_code=404,
path="/check/test",
body='{"error": "Not found"}',
)
)
assert message == "Prowler Hub GET /check/test failed with HTTP 404. Not found"
@pytest.mark.parametrize(
("body", "expected"),
[
('{"error": "Not found"}', "Not found"),
('{"message": "Bad gateway"}', "Bad gateway"),
('{"detail": "Rate limited"}', "Rate limited"),
("Service Unavailable", "Service Unavailable"),
('{"unexpected": "shape"}', '{"unexpected": "shape"}'),
],
ids=["error", "message", "detail", "plain-text", "unknown-json"],
)
def test_an_upstream_message_is_pulled_out_of_whatever_shape_it_came_in(body, expected):
"""Hosts that are not the Prowler API each have their own error shape.
Relaying the raw body puts JSON braces, or a whole HTML page, in front of the model.
"""
message = render_tool_error(
ProwlerHubError("hub failed", status_code=500, path="/checks", body=body)
)
assert message.endswith(expected)
def test_an_upstream_body_is_truncated():
"""An HTML error page must not flood the model's context."""
message = render_tool_error(
ProwlerHubError("hub failed", status_code=500, path="/checks", body="x" * 2000)
)
assert "x" * 500 in message
assert "x" * 501 not in message
# --------------------------------------------------------------------- server bugs
def test_a_bug_in_this_server_is_reported_as_a_bug():
"""Named as ours, so the caller stops trying to fix it by changing arguments."""
message = render_tool_error(KeyError("attributes"))
assert "unexpected KeyError" in message
assert "bug in the server" in message