mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-10-04 02:04:06 +00:00
feat(mcp): raise instead of returning error objects in the Prowler App tools (#12532)
This commit is contained in:
@@ -11,7 +11,11 @@ import pytest
|
||||
from fastmcp import Client
|
||||
from pydantic import BaseModel, ValidationError
|
||||
|
||||
from prowler_mcp_server.lib.errors import InvalidArgument, _describe_failure
|
||||
from prowler_mcp_server.lib.errors import (
|
||||
CredentialError,
|
||||
InvalidArgument,
|
||||
_describe_failure,
|
||||
)
|
||||
from prowler_mcp_server.prowler_app.utils.api_client import (
|
||||
ProwlerAPIError,
|
||||
ProwlerAPIInvalidResponse,
|
||||
@@ -99,6 +103,19 @@ def test_an_argument_this_server_rejected_is_repeated_verbatim():
|
||||
assert message == "page_size must be between 1 and 1000."
|
||||
|
||||
|
||||
def test_a_credential_caught_here_is_answered_like_the_401_it_would_have_got():
|
||||
"""It is not an argument problem, and saying so stops a pointless retry."""
|
||||
message = _describe_failure(CredentialError("the token has expired"))
|
||||
|
||||
assert "the token has expired" in message
|
||||
assert "changing the arguments will not help" in message
|
||||
|
||||
|
||||
def test_a_transport_this_server_cannot_serve_is_left_masked():
|
||||
"""No call caused a bad PROWLER_MCP_TRANSPORT_MODE and no call can fix it."""
|
||||
assert _describe_failure(RuntimeError("Invalid mode: websocket")) is None
|
||||
|
||||
|
||||
def test_a_pydantic_rejection_names_the_field_without_echoing_the_value():
|
||||
"""Pydantic quotes `input_value` back, and these tools take credentials."""
|
||||
|
||||
@@ -195,3 +212,18 @@ async def test_an_unreadable_api_answer_does_not_reach_the_agent_as_a_bad_argume
|
||||
assert result.isError is True
|
||||
assert "gateway timeout" not in result.content[0].text
|
||||
assert "argument" not in result.content[0].text
|
||||
|
||||
|
||||
async def test_a_tool_specific_message_survives_masking(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""A `ToolError` raised without a `from` clause is the final word."""
|
||||
mock_router.add("GET", "/api/v1/integrations/i1", json={"data": None})
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_get_integration", {"integration_id": "i1"}
|
||||
)
|
||||
|
||||
assert result.isError is True
|
||||
assert "prowler_list_integrations" in result.content[0].text
|
||||
|
||||
@@ -0,0 +1,125 @@
|
||||
"""Tests for the argument types every tool shares.
|
||||
|
||||
The bug these pin: "required" alone does not stop a blank identifier. A model
|
||||
that has no scan or query id to hand sends ``""`` rather than omitting the
|
||||
argument, and an unguarded empty string travels into a URL path or a request
|
||||
body -- where it comes back as a 404, or as an API rejection ("This field may
|
||||
not be blank") that names no argument and leaves the model with nothing to fix.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.jsonapi import jsonapi_collection, jsonapi_resource
|
||||
|
||||
SCAN_ID = "019ac0d6-90d5-73e9-9acf-c22e256f1bac"
|
||||
QUERIES = f"/api/v1/attack-paths-scans/{SCAN_ID}/queries"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("query_id", ["", " "], ids=["empty", "whitespace-only"])
|
||||
async def test_a_blank_identifier_is_rejected_before_any_request_goes_out(
|
||||
mcp_root_server, mock_api_client, mock_router, query_id
|
||||
):
|
||||
"""The reported failure: a blank `query_id` reached Prowler as a 400.
|
||||
|
||||
The message has to name the argument. Prowler's own answer to the blank value
|
||||
("This field may not be blank") does not say which field, so the model had no
|
||||
way to tell `scan_id` from `query_id` from the reply.
|
||||
"""
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_run_attack_paths_query",
|
||||
{"scan_id": SCAN_ID, "query_id": query_id},
|
||||
)
|
||||
|
||||
assert result.isError is True
|
||||
assert "query_id" in result.content[0].text
|
||||
assert mock_router.requests == []
|
||||
|
||||
|
||||
async def test_an_identifier_keeps_its_surrounding_whitespace_out_of_the_url(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""A padded id is the same id, and a raw one would build a URL-escaped path."""
|
||||
mock_router.add("GET", QUERIES, json=jsonapi_collection([]))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_list_attack_paths_queries", {"scan_id": f" {SCAN_ID} "}
|
||||
)
|
||||
|
||||
assert result.isError is False
|
||||
assert mock_router.paths() == [f"GET {QUERIES}"]
|
||||
|
||||
|
||||
async def test_a_blank_optional_value_is_rejected_rather_than_written(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""An omitted optional means "leave it alone"; a blank one would blank the field.
|
||||
|
||||
The API refuses it, so the only difference an unguarded blank makes is a
|
||||
round trip and an error that names nothing.
|
||||
"""
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_update_mute_rule", {"rule_id": SCAN_ID, "name": ""}
|
||||
)
|
||||
|
||||
assert result.isError is True
|
||||
assert "name" in result.content[0].text
|
||||
assert mock_router.requests == []
|
||||
|
||||
|
||||
async def test_an_omitted_optional_string_is_still_omitted(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""`NonBlankStr | None` must not turn "not provided" into a rejection."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
f"/api/v1/mute-rules/{SCAN_ID}",
|
||||
json={
|
||||
"data": jsonapi_resource(
|
||||
"mute-rules",
|
||||
SCAN_ID,
|
||||
{
|
||||
"name": "unchanged",
|
||||
"reason": "already reviewed",
|
||||
"enabled": True,
|
||||
"finding_uids": [],
|
||||
},
|
||||
)
|
||||
},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_update_mute_rule", {"rule_id": SCAN_ID}
|
||||
)
|
||||
|
||||
assert result.isError is False
|
||||
|
||||
|
||||
async def test_every_required_string_argument_is_guarded_against_a_blank(
|
||||
mcp_root_server,
|
||||
):
|
||||
"""A guard only one tool carries is one the next tool will be written without.
|
||||
|
||||
Declared as `minLength` rather than checked inside each tool, so a client sees
|
||||
the constraint in the schema before it calls.
|
||||
"""
|
||||
async with Client(mcp_root_server) as client:
|
||||
tools = await client.list_tools()
|
||||
|
||||
unguarded = [
|
||||
f"{tool.name}.{name}"
|
||||
for tool in tools
|
||||
for name, schema in tool.inputSchema.get("properties", {}).items()
|
||||
# Plain required strings only. A union such as `dict | str` takes a JSON
|
||||
# string, where a blank is a parse failure the classifier already
|
||||
# explains, and a blank filter is a filter that matches everything.
|
||||
if schema.get("type") == "string"
|
||||
and name in tool.inputSchema.get("required", [])
|
||||
and schema.get("minLength") != 1
|
||||
]
|
||||
|
||||
assert unguarded == []
|
||||
@@ -0,0 +1,225 @@
|
||||
"""Tests for the Attack Paths tools.
|
||||
|
||||
An Attack Paths scan is a separate resource from a regular scan, with IDs of its
|
||||
own, and Prowler only creates one for an AWS provider. So the 404 these tools get
|
||||
is almost always a regular scan ID passed where an Attack Paths one belongs --
|
||||
and Prowler's own reason for it, a bare "Not found.", names neither the resource
|
||||
it looked in nor the tool that returns the right ID.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.jsonapi import jsonapi_collection, jsonapi_resource
|
||||
|
||||
QUERIES = "/api/v1/attack-paths-scans/s1/queries"
|
||||
|
||||
|
||||
async def test_an_id_that_is_not_an_attack_paths_scan_says_which_tool_returns_one(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Relaying "Not found." sends an agent to re-check an ID it cannot fix.
|
||||
|
||||
The reply has to name the confusion it stands for: regular scan IDs do not
|
||||
resolve here, and only AWS providers have an Attack Paths scan at all.
|
||||
"""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
QUERIES,
|
||||
status=404,
|
||||
json={"errors": [{"status": "404", "detail": "Not found."}]},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="different resource from regular scans"):
|
||||
await client.call_tool(
|
||||
"prowler_list_attack_paths_queries", {"scan_id": "s1"}
|
||||
)
|
||||
|
||||
|
||||
async def test_the_answer_names_the_tool_that_returns_a_usable_id(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""An explanation with no next step leaves the agent guessing IDs."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
QUERIES,
|
||||
status=404,
|
||||
json={"errors": [{"status": "404", "detail": "Not found."}]},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="prowler_list_attack_paths_scans"):
|
||||
await client.call_tool(
|
||||
"prowler_list_attack_paths_queries", {"scan_id": "s1"}
|
||||
)
|
||||
|
||||
|
||||
async def test_a_failure_that_is_not_a_404_keeps_the_shared_message(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Only the 404 means a bad ID. A 403 is a permission the ID cannot fix."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
QUERIES,
|
||||
status=403,
|
||||
json={"errors": [{"status": "403", "detail": "Denied."}]},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="prowler_get_current_user"):
|
||||
await client.call_tool(
|
||||
"prowler_list_attack_paths_queries", {"scan_id": "s1"}
|
||||
)
|
||||
|
||||
|
||||
async def test_queries_come_back_as_a_list(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The success path is unchanged."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
QUERIES,
|
||||
json=jsonapi_collection(
|
||||
[
|
||||
jsonapi_resource(
|
||||
"attack-paths-queries",
|
||||
"aws-ec2-instances-internet-exposed",
|
||||
{
|
||||
"name": "Internet exposed EC2",
|
||||
"description": "Find internet-exposed EC2 instances",
|
||||
"provider": "aws",
|
||||
"parameters": [],
|
||||
},
|
||||
)
|
||||
]
|
||||
),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_list_attack_paths_queries", {"scan_id": "s1"}
|
||||
)
|
||||
|
||||
assert result.data[0]["id"] == "aws-ec2-instances-internet-exposed"
|
||||
|
||||
|
||||
# ------------------------------------------------------------- running a query
|
||||
|
||||
RUN = "/api/v1/attack-paths-scans/s1/queries/run"
|
||||
SCHEMA = "/api/v1/attack-paths-scans/s1/schema"
|
||||
|
||||
EMPTY_RESULT = {
|
||||
"data": {
|
||||
"type": "attack-paths-query-results",
|
||||
"id": "s1",
|
||||
"attributes": {"nodes": [], "relationships": []},
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
def _run_args(query_id: str = "aws-ec2-instances-internet-exposed") -> dict[str, str]:
|
||||
"""Arguments for a query run against the mocked scan."""
|
||||
return {"scan_id": "s1", "query_id": query_id}
|
||||
|
||||
|
||||
async def test_a_query_that_matched_nothing_is_an_answer_not_a_failure(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Prowler answers a query that matched nothing with 404 and the result body.
|
||||
|
||||
Raising on the status called a clean account a failed call and sent the agent
|
||||
off to re-check arguments that were right.
|
||||
"""
|
||||
mock_router.add("POST", RUN, status=404, json=EMPTY_RESULT)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_run_attack_paths_query", _run_args()
|
||||
)
|
||||
|
||||
assert result.isError is False
|
||||
assert "matched nothing" in result.structuredContent["message"]
|
||||
|
||||
|
||||
async def test_an_empty_result_does_not_come_back_as_an_empty_object(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The serializer drops empty lists, so `{}` is all that would be left."""
|
||||
mock_router.add("POST", RUN, status=404, json=EMPTY_RESULT)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool("prowler_run_attack_paths_query", _run_args())
|
||||
|
||||
assert result.data != {}
|
||||
|
||||
|
||||
async def test_a_null_graph_is_read_as_an_empty_one(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Prowler can spell the empty graph as `null` rather than as empty lists.
|
||||
|
||||
Reading `null` as if it were a graph crashed the parse, turning the same
|
||||
"nothing matched" answer into an error the agent could not act on.
|
||||
"""
|
||||
mock_router.add("POST", RUN, status=404, json={"data": {"attributes": None}})
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool_mcp(
|
||||
"prowler_run_attack_paths_query", _run_args()
|
||||
)
|
||||
|
||||
assert result.isError is False
|
||||
assert "matched nothing" in result.structuredContent["message"]
|
||||
|
||||
|
||||
async def test_a_run_against_an_unknown_scan_still_names_the_confusion(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""A 404 with no result body is the ID being wrong, not an empty answer."""
|
||||
mock_router.add(
|
||||
"POST",
|
||||
RUN,
|
||||
status=404,
|
||||
json={"errors": [{"status": "404", "detail": "Not found."}]},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="different resource from regular scans"):
|
||||
await client.call_tool("prowler_run_attack_paths_query", _run_args())
|
||||
|
||||
|
||||
async def test_a_scan_whose_graph_records_no_schema_says_so(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""This 404 is about the graph, not the ID, so it must not blame the ID."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
SCHEMA,
|
||||
status=404,
|
||||
json={"detail": "No cartography schema metadata found for this provider"},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="no Cartography schema recorded"):
|
||||
await client.call_tool(
|
||||
"prowler_get_attack_paths_cartography_schema", {"scan_id": "s1"}
|
||||
)
|
||||
|
||||
|
||||
async def test_a_schema_request_for_an_unknown_scan_names_the_confusion(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The other 404 here is the ID, and Prowler writes a JSON:API error for it."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
SCHEMA,
|
||||
status=404,
|
||||
json={"errors": [{"status": "404", "detail": "Not found."}]},
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="different resource from regular scans"):
|
||||
await client.call_tool(
|
||||
"prowler_get_attack_paths_cartography_schema", {"scan_id": "s1"}
|
||||
)
|
||||
@@ -0,0 +1,102 @@
|
||||
"""Tests for the compliance tools.
|
||||
|
||||
Both compliance tools answer for exactly one scan. ``scan_id`` names it
|
||||
directly; ``provider_id`` names it indirectly, as "the latest completed scan of
|
||||
this provider". Passing both is not a refinement of either -- the scan the
|
||||
caller named may belong to a different provider entirely -- so it is rejected
|
||||
rather than resolved by preferring one, which would answer confidently for a
|
||||
provider nobody asked about.
|
||||
|
||||
Tools are driven through an in-memory MCP client so FastMCP resolves the
|
||||
pydantic ``Field`` defaults and a raised failure arrives the way a client sees
|
||||
it.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.jsonapi import jsonapi_collection, jsonapi_resource
|
||||
|
||||
SCANS = "/api/v1/scans"
|
||||
OVERVIEWS = "/api/v1/compliance-overviews"
|
||||
REQUIREMENTS = f"{OVERVIEWS}/requirements"
|
||||
|
||||
TOOLS = [
|
||||
"prowler_get_compliance_overview",
|
||||
"prowler_get_compliance_framework_state_details",
|
||||
]
|
||||
|
||||
|
||||
def arguments(tool: str, **overrides) -> dict:
|
||||
"""Build the arguments for either tool, which differ only in compliance_id."""
|
||||
payload = dict(overrides)
|
||||
if tool.endswith("framework_state_details"):
|
||||
payload["compliance_id"] = "cis_1.5_aws"
|
||||
return payload
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tool", TOOLS)
|
||||
async def test_neither_a_scan_nor_a_provider_is_refused_before_any_request(
|
||||
mcp_root_server, mock_api_client, mock_router, tool
|
||||
):
|
||||
"""There is no scan to answer for, and no way to guess one."""
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="must be provided"):
|
||||
await client.call_tool(tool, arguments(tool))
|
||||
|
||||
assert mock_router.paths() == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tool", TOOLS)
|
||||
async def test_a_scan_and_a_provider_together_are_refused_rather_than_reconciled(
|
||||
mcp_root_server, mock_api_client, mock_router, tool
|
||||
):
|
||||
"""Silently keeping the scan would answer for whichever provider owns it.
|
||||
|
||||
That report names a scan the caller did ask for, so nothing about it looks
|
||||
wrong -- while the provider they also named went unread.
|
||||
"""
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="not both"):
|
||||
await client.call_tool(
|
||||
tool, arguments(tool, scan_id="s1", provider_id="p1")
|
||||
)
|
||||
|
||||
assert mock_router.paths() == []
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tool", TOOLS)
|
||||
async def test_a_provider_on_its_own_resolves_to_its_latest_completed_scan(
|
||||
mcp_root_server, mock_api_client, mock_router, tool
|
||||
):
|
||||
"""The indirection is the point of accepting a provider at all."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
SCANS,
|
||||
json=jsonapi_collection(
|
||||
[jsonapi_resource("scans", "s9", {"state": "completed"})]
|
||||
),
|
||||
)
|
||||
mock_router.add("GET", OVERVIEWS, json=jsonapi_collection([]))
|
||||
mock_router.add("GET", REQUIREMENTS, json=jsonapi_collection([]))
|
||||
# Each tool reads the compliance state from its own endpoint; both filter it
|
||||
# by the scan that had to be resolved first.
|
||||
read = OVERVIEWS if tool.endswith("overview") else REQUIREMENTS
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
await client.call_tool(tool, arguments(tool, provider_id="p1"))
|
||||
|
||||
assert mock_router.query_params("GET", SCANS)["filter[provider]"] == "p1"
|
||||
assert mock_router.query_params("GET", read)["filter[scan_id]"] == "s9"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("tool", TOOLS)
|
||||
async def test_a_provider_with_no_completed_scan_is_named_as_the_bad_argument(
|
||||
mcp_root_server, mock_api_client, mock_router, tool
|
||||
):
|
||||
"""Nothing has been scanned yet, so there is no compliance state to report."""
|
||||
mock_router.add("GET", SCANS, json=jsonapi_collection([]))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="No completed scans found for provider p1"):
|
||||
await client.call_tool(tool, arguments(tool, provider_id="p1"))
|
||||
@@ -312,17 +312,16 @@ async def test_creating_a_jira_integration_rejects_an_empty_domain(
|
||||
):
|
||||
"""A domain that normalizes to nothing is caught before the round trip."""
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_create_jira_integration",
|
||||
{
|
||||
"domain": "https://",
|
||||
"user_mail": "security@acme.com",
|
||||
"api_token": "fake-atlassian-token-for-testing",
|
||||
},
|
||||
)
|
||||
with pytest.raises(Exception, match="Invalid Jira domain"):
|
||||
await client.call_tool(
|
||||
"prowler_create_jira_integration",
|
||||
{
|
||||
"domain": "https://",
|
||||
"user_mail": "security@acme.com",
|
||||
"api_token": "fake-atlassian-token-for-testing",
|
||||
},
|
||||
)
|
||||
|
||||
assert result.data["status"] == "failed"
|
||||
assert "Invalid Jira domain" in result.data["error"]
|
||||
assert mock_router.requests == []
|
||||
|
||||
|
||||
@@ -334,14 +333,10 @@ async def test_creating_a_jira_integration_rejects_an_empty_domain(
|
||||
],
|
||||
ids=["security-hub", "amazon-s3"],
|
||||
)
|
||||
async def test_a_rejected_creation_is_reported_rather_than_raised(
|
||||
async def test_a_rejected_creation_fails_with_the_api_reason(
|
||||
mcp_root_server, mock_api_client, mock_router, tool, arguments
|
||||
):
|
||||
"""Write tools answer with an error object so the agent can act on it.
|
||||
|
||||
A raised exception reaches the model as a tool failure with no detail, and
|
||||
the API's message is exactly what tells it what to do next.
|
||||
"""
|
||||
"""A refused creation is a tool error, and it still carries the API's reason."""
|
||||
mock_router.add(
|
||||
"POST",
|
||||
INTEGRATIONS,
|
||||
@@ -350,10 +345,8 @@ async def test_a_rejected_creation_is_reported_rather_than_raised(
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(tool, arguments)
|
||||
|
||||
assert result.data["status"] == "failed"
|
||||
assert "already has this integration" in result.data["error"]
|
||||
with pytest.raises(Exception, match="already has this integration"):
|
||||
await client.call_tool(tool, arguments)
|
||||
|
||||
|
||||
async def test_a_creation_with_no_id_back_warns_before_a_blind_retry(
|
||||
@@ -367,12 +360,11 @@ async def test_a_creation_with_no_id_back_warns_before_a_blind_retry(
|
||||
mock_router.add("POST", INTEGRATIONS, json={"data": {}})
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_create_amazon_s3_integration", {"bucket_name": "my-reports"}
|
||||
)
|
||||
with pytest.raises(Exception, match="did not return its ID"):
|
||||
await client.call_tool(
|
||||
"prowler_create_amazon_s3_integration", {"bucket_name": "my-reports"}
|
||||
)
|
||||
|
||||
assert result.data["status"] == "failed"
|
||||
assert "did not return its ID" in result.data["error"]
|
||||
assert mock_router.paths() == [f"POST {INTEGRATIONS}"]
|
||||
|
||||
|
||||
@@ -395,12 +387,10 @@ async def test_a_creation_whose_read_back_fails_still_hands_over_the_id(
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_create_amazon_s3_integration", {"bucket_name": "my-reports"}
|
||||
)
|
||||
|
||||
assert result.data["status"] == "failed"
|
||||
assert "Integration i1 was created" in result.data["error"]
|
||||
with pytest.raises(Exception, match="Integration i1 was created"):
|
||||
await client.call_tool(
|
||||
"prowler_create_amazon_s3_integration", {"bucket_name": "my-reports"}
|
||||
)
|
||||
|
||||
|
||||
async def test_a_connection_check_that_cannot_run_is_not_reported_as_a_failure(
|
||||
@@ -542,13 +532,12 @@ async def test_a_configuration_that_is_not_an_object_is_rejected_before_the_writ
|
||||
stub_integration(mock_router, S3_ATTRIBUTES)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "configuration": configuration},
|
||||
)
|
||||
with pytest.raises(Exception, match=message):
|
||||
await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "configuration": configuration},
|
||||
)
|
||||
|
||||
assert result.data["status"] == "failed"
|
||||
assert message in result.data["error"]
|
||||
assert f"PATCH {INTEGRATION}" not in mock_router.paths()
|
||||
|
||||
|
||||
@@ -643,13 +632,12 @@ async def test_updating_a_jira_configuration_is_refused(
|
||||
stub_integration(mock_router, JIRA_ATTRIBUTES)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "configuration": {"domain": "other"}},
|
||||
)
|
||||
with pytest.raises(Exception, match="do not accept a configuration"):
|
||||
await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "configuration": {"domain": "other"}},
|
||||
)
|
||||
|
||||
assert result.data["status"] == "failed"
|
||||
assert "do not accept a configuration" in result.data["error"]
|
||||
assert f"PATCH {INTEGRATION}" not in mock_router.paths()
|
||||
|
||||
|
||||
@@ -660,12 +648,12 @@ async def test_attaching_a_jira_integration_to_a_provider_is_refused(
|
||||
stub_integration(mock_router, JIRA_ATTRIBUTES)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "provider_ids": ["p1"]},
|
||||
)
|
||||
with pytest.raises(Exception, match="tenant-wide"):
|
||||
await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "provider_ids": ["p1"]},
|
||||
)
|
||||
|
||||
assert "tenant-wide" in result.data["error"]
|
||||
assert f"PATCH {INTEGRATION}" not in mock_router.paths()
|
||||
|
||||
|
||||
@@ -683,12 +671,12 @@ async def test_security_hub_must_keep_exactly_one_provider(
|
||||
stub_integration(mock_router, SECURITY_HUB_ATTRIBUTES, provider_ids=("p1",))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "provider_ids": provider_ids},
|
||||
)
|
||||
with pytest.raises(Exception, match="exactly one AWS provider"):
|
||||
await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "provider_ids": provider_ids},
|
||||
)
|
||||
|
||||
assert "exactly one AWS provider" in result.data["error"]
|
||||
assert f"PATCH {INTEGRATION}" not in mock_router.paths()
|
||||
|
||||
|
||||
@@ -708,12 +696,12 @@ async def test_partial_jira_credentials_are_refused_to_protect_the_stored_ones(
|
||||
stub_integration(mock_router, JIRA_ATTRIBUTES)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "credentials": credentials},
|
||||
)
|
||||
with pytest.raises(Exception, match="replaced as a whole"):
|
||||
await client.call_tool(
|
||||
"prowler_update_integration",
|
||||
{"integration_id": "i1", "credentials": credentials},
|
||||
)
|
||||
|
||||
assert "replaced as a whole" in result.data["error"]
|
||||
assert f"PATCH {INTEGRATION}" not in mock_router.paths()
|
||||
|
||||
|
||||
@@ -751,13 +739,14 @@ async def test_replacing_jira_credentials_normalizes_the_domain(
|
||||
# ------------------------------------------------- delete and connection tools
|
||||
|
||||
|
||||
async def test_deleting_an_integration_reports_the_outcome_either_way(
|
||||
async def test_deleting_an_integration_confirms_it_happened(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Deletion is irreversible, so both outcomes are stated explicitly.
|
||||
"""Deletion is irreversible, so a success says so rather than staying silent.
|
||||
|
||||
A bare exception would leave the agent unsure whether the credentials are
|
||||
gone, and a retry of a delete that actually succeeded reads as a new failure.
|
||||
It says so in the message and nowhere else: a `deleted: true` flag could only
|
||||
ever be true, because an integration that was not deleted leaves the tool as
|
||||
an error.
|
||||
"""
|
||||
mock_router.add("DELETE", INTEGRATION, status=204)
|
||||
|
||||
@@ -766,24 +755,23 @@ async def test_deleting_an_integration_reports_the_outcome_either_way(
|
||||
"prowler_delete_integration", {"integration_id": "i1"}
|
||||
)
|
||||
|
||||
assert result.data["deleted"] is True
|
||||
assert "i1 deleted successfully" in result.data["message"]
|
||||
assert "deleted" not in result.data
|
||||
|
||||
|
||||
async def test_a_failed_deletion_says_it_did_not_happen(
|
||||
async def test_a_refused_deletion_fails_and_says_the_role_is_the_problem(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""`deleted: false` is the part the agent must not have to infer."""
|
||||
"""A 403 is the same answer for every tool, so `lib.errors` writes it."""
|
||||
mock_router.add(
|
||||
"DELETE", INTEGRATION, status=403, json=jsonapi_error(403, "Permission denied.")
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_delete_integration", {"integration_id": "i1"}
|
||||
)
|
||||
|
||||
assert result.data["deleted"] is False
|
||||
assert "Permission denied." in result.data["message"]
|
||||
with pytest.raises(Exception, match="prowler_get_current_user"):
|
||||
await client.call_tool(
|
||||
"prowler_delete_integration", {"integration_id": "i1"}
|
||||
)
|
||||
|
||||
|
||||
async def test_checking_a_connection_surfaces_why_it_failed(
|
||||
@@ -1026,6 +1014,32 @@ async def test_an_accepted_dispatch_with_no_task_id_is_not_safe_to_retry(
|
||||
assert "task_id" not in result.data
|
||||
|
||||
|
||||
async def test_a_dispatch_with_no_usable_credential_is_raised_not_called_unknown(
|
||||
mcp_root_server, mock_api_client, mock_router, monkeypatch
|
||||
):
|
||||
"""Authentication runs before the request, so nothing was ever queued.
|
||||
|
||||
Reported as `unknown` it reads as "work items may exist in Jira, go and
|
||||
look" -- for a call that never reached Prowler. It is not a dispatch outcome
|
||||
at all: the credential has to be fixed, and no retry of this call does that.
|
||||
"""
|
||||
monkeypatch.setattr(mock_api_client.auth_manager, "mode", "http")
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="no usable credential"):
|
||||
await client.call_tool(
|
||||
"prowler_send_findings_to_jira",
|
||||
{
|
||||
"integration_id": "i1",
|
||||
"project_key": "PROJ",
|
||||
"issue_type": "Task",
|
||||
"finding_ids": ["f1"],
|
||||
},
|
||||
)
|
||||
|
||||
assert mock_router.paths() == []
|
||||
|
||||
|
||||
async def test_a_dispatch_task_that_died_halfway_is_never_safe_to_retry(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
"""Tests for the muting tools.
|
||||
|
||||
Muting is permanent and deleting a rule does not undo it, so the one thing
|
||||
these assertions protect is that an agent is never told a deletion failed when
|
||||
it did not: the old answer keyed off the *shape* of the API's reply rather than
|
||||
off anything having gone wrong, and said nothing a caller could act on.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.jsonapi import jsonapi_document, jsonapi_error, jsonapi_resource
|
||||
|
||||
MUTE_RULE = "/api/v1/mute-rules/m1"
|
||||
|
||||
|
||||
async def test_a_deleted_rule_is_reported_deleted_whatever_the_body(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Prowler answers 204 with no body, but 200 with one is just as much a yes.
|
||||
|
||||
The old check read ``success`` out of the parsed body, which only exists for
|
||||
the empty-body case, so a deletion that worked could be reported as
|
||||
"Failed to delete mute rule".
|
||||
"""
|
||||
mock_router.add(
|
||||
"DELETE",
|
||||
MUTE_RULE,
|
||||
json=jsonapi_document(jsonapi_resource("mute-rules", "m1", {})),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool("prowler_delete_mute_rule", {"rule_id": "m1"})
|
||||
|
||||
assert "m1 deleted successfully" in result.data["message"]
|
||||
assert "stay muted" in result.data["message"]
|
||||
# A flag with one reachable value is not a fact, it is an invitation to
|
||||
# branch on a shape that does not exist.
|
||||
assert "success" not in result.data
|
||||
|
||||
|
||||
async def test_an_empty_body_deletion_is_reported_the_same_way(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The 204 path, which is what Prowler actually sends today."""
|
||||
mock_router.add("DELETE", MUTE_RULE, status=204)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool("prowler_delete_mute_rule", {"rule_id": "m1"})
|
||||
|
||||
assert "m1 deleted successfully" in result.data["message"]
|
||||
|
||||
|
||||
async def test_a_refused_deletion_is_an_error(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""A rule that was not deleted has to leave the tool as an error."""
|
||||
mock_router.add(
|
||||
"DELETE", MUTE_RULE, status=404, json=jsonapi_error(404, "Not found.")
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="Not found"):
|
||||
await client.call_tool("prowler_delete_mute_rule", {"rule_id": "m1"})
|
||||
@@ -0,0 +1,316 @@
|
||||
"""Tests for the provider tools.
|
||||
|
||||
Two behaviours drive most of the assertions here, and both are about telling a
|
||||
failure apart from an outcome that is merely not final yet:
|
||||
|
||||
* Deleting a provider removes it together with its scans, findings and
|
||||
resources, in a background task that routinely outlives the polling window.
|
||||
A deletion still running is not a failure, and reporting it as one invites a
|
||||
retry of a destructive call that is already in flight.
|
||||
* ``connect_provider`` runs a connection check, and a check that could not be
|
||||
run says nothing about the provider's credentials. Reporting it as ``failed``
|
||||
blames an AWS role for an expired Prowler credential and sends the user off to
|
||||
fix something that works.
|
||||
|
||||
Tools are driven through an in-memory MCP client so FastMCP resolves the
|
||||
pydantic ``Field`` defaults and a raised failure arrives the way a client sees
|
||||
it.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.http import MockRouter
|
||||
from tests.helpers.jsonapi import (
|
||||
jsonapi_collection,
|
||||
jsonapi_document,
|
||||
jsonapi_error,
|
||||
jsonapi_resource,
|
||||
task_document,
|
||||
)
|
||||
|
||||
PROVIDERS = "/api/v1/providers"
|
||||
PROVIDER = f"{PROVIDERS}/p1"
|
||||
CONNECTION = f"{PROVIDER}/connection"
|
||||
SECRETS = f"{PROVIDERS}/secrets"
|
||||
TASK = "/api/v1/tasks/t1"
|
||||
|
||||
PROVIDER_ATTRIBUTES = {
|
||||
"uid": "123456789012",
|
||||
"provider": "aws",
|
||||
"alias": "production",
|
||||
"connection": {"connected": True},
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_fast_polling(monkeypatch, api_client):
|
||||
"""Run the real polling loop, with a timeout a test can afford to wait out.
|
||||
|
||||
The timeout path is the one worth testing here -- it is what used to be
|
||||
reported as a failed deletion -- so the loop, its exception and the fallback
|
||||
that reads the task afterwards all stay real. Only the 60 seconds go.
|
||||
"""
|
||||
original = type(api_client).poll_task_until_complete
|
||||
|
||||
async def _fast(self, task_id, **_overridden):
|
||||
return await original(self, task_id, timeout=0.3, poll_interval=0.05)
|
||||
|
||||
monkeypatch.setattr(type(api_client), "poll_task_until_complete", _fast)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_polling_timeout(monkeypatch, api_client):
|
||||
"""Make the polling window run out on the first call, without the wait.
|
||||
|
||||
The clock is what ends the polling loop here, so a test about *what happens
|
||||
afterwards* has no reason to spend it. The read the fallback then makes is
|
||||
the real one.
|
||||
"""
|
||||
|
||||
async def _timeout(self, task_id, **_overridden):
|
||||
raise TimeoutError(f"Task {task_id} polling timed out after 60 seconds.")
|
||||
|
||||
monkeypatch.setattr(type(api_client), "poll_task_until_complete", _timeout)
|
||||
|
||||
|
||||
def stub_deletion_start(mock_router: MockRouter) -> MockRouter:
|
||||
"""Serve the DELETE as Prowler does: a task to poll, not a finished deletion."""
|
||||
return mock_router.add(
|
||||
"DELETE", PROVIDER, json=jsonapi_document(jsonapi_resource("tasks", "t1", {}))
|
||||
)
|
||||
|
||||
|
||||
# --------------------------------------------------------------- deletion
|
||||
|
||||
|
||||
async def test_a_refused_deletion_is_an_error_not_a_result(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Nothing started, so the classifier owns the message.
|
||||
|
||||
Returned as ``{"deleted": false}`` it arrives with ``isError: false`` and a
|
||||
model has no reason to treat it as anything but a completed call.
|
||||
"""
|
||||
mock_router.add(
|
||||
"DELETE", PROVIDER, status=403, json=jsonapi_error(403, "Not allowed.")
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="not allowed to do this"):
|
||||
await client.call_tool("prowler_delete_provider", {"provider_id": "p1"})
|
||||
|
||||
|
||||
async def test_a_deletion_with_no_task_back_warns_before_a_blind_retry(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Prowler accepted it, so the deletion is probably running.
|
||||
|
||||
Without the task ID there is nothing to watch it with, which makes "check
|
||||
whether it is gone" the only safe instruction.
|
||||
"""
|
||||
mock_router.add("DELETE", PROVIDER, json={"data": {}})
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="did not return the ID"):
|
||||
await client.call_tool("prowler_delete_provider", {"provider_id": "p1"})
|
||||
|
||||
assert mock_router.paths() == [f"DELETE {PROVIDER}"]
|
||||
|
||||
|
||||
async def test_a_finished_deletion_reports_it_plainly(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The success path is unchanged: the provider is gone."""
|
||||
stub_deletion_start(mock_router)
|
||||
mock_router.add("GET", TASK, json=task_document("t1", "completed"))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_delete_provider", {"provider_id": "p1"}
|
||||
)
|
||||
|
||||
assert result.data["status"] == "deleted"
|
||||
|
||||
|
||||
async def test_a_deletion_still_running_is_not_reported_as_a_failure(
|
||||
mcp_root_server, mock_api_client, mock_router, mock_fast_polling
|
||||
):
|
||||
"""Outliving the polling window is normal for a provider with many findings.
|
||||
|
||||
The task ID goes back so the deletion can be followed, and the message says
|
||||
not to send it again -- which is the whole point of not calling this failed.
|
||||
"""
|
||||
stub_deletion_start(mock_router)
|
||||
mock_router.add("GET", TASK, json=task_document("t1", "executing"))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_delete_provider", {"provider_id": "p1"}
|
||||
)
|
||||
|
||||
assert result.data["status"] == "in_progress"
|
||||
assert result.data["task_id"] == "t1"
|
||||
assert "Do not send the deletion again" in result.data["message"]
|
||||
|
||||
|
||||
async def test_a_deletion_that_finished_just_after_the_wait_is_reported_as_deleted(
|
||||
mcp_root_server, mock_api_client, mock_router, mock_polling_timeout
|
||||
):
|
||||
"""Polling gives up on the clock, not on the task.
|
||||
|
||||
A deletion that completed a moment after the last poll is a finished
|
||||
deletion, and the read the fallback makes is what says so. Reporting it as
|
||||
still running would send the caller off to watch a provider that is gone.
|
||||
"""
|
||||
stub_deletion_start(mock_router)
|
||||
mock_router.add("GET", TASK, json=task_document("t1", "completed"))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_delete_provider", {"provider_id": "p1"}
|
||||
)
|
||||
|
||||
assert result.data["status"] == "deleted"
|
||||
assert "task_id" not in result.data
|
||||
|
||||
|
||||
async def test_a_deletion_task_that_stopped_is_an_error_naming_what_is_left(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Here the provider really is still there, so this one is a failure.
|
||||
|
||||
A provider is removed together with everything attached to it, so a task
|
||||
that stopped halfway can leave part of that gone -- which is why the message
|
||||
sends the caller to look rather than asserting the state.
|
||||
"""
|
||||
stub_deletion_start(mock_router)
|
||||
mock_router.add("GET", TASK, json=task_document("t1", "failed", error="boom"))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="ended as 'failed'"):
|
||||
await client.call_tool("prowler_delete_provider", {"provider_id": "p1"})
|
||||
|
||||
|
||||
async def test_a_deletion_task_failure_does_not_relay_the_upstream_text(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The task's own error is a celery traceback; it stays in the log."""
|
||||
stub_deletion_start(mock_router)
|
||||
mock_router.add(
|
||||
"GET",
|
||||
TASK,
|
||||
json=task_document("t1", "failed", error="Traceback: secret-internal-detail"),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception) as raised:
|
||||
await client.call_tool("prowler_delete_provider", {"provider_id": "p1"})
|
||||
|
||||
assert "secret-internal-detail" not in str(raised.value)
|
||||
|
||||
|
||||
async def test_a_deletion_whose_progress_cannot_be_read_still_says_do_not_retry(
|
||||
mcp_root_server, mock_api_client, mock_router, mock_fast_polling
|
||||
):
|
||||
"""The outcome is unknown, which for a destructive call means: do not repeat.
|
||||
|
||||
Reading the task is what tells "still running" from "stopped", so when that
|
||||
read fails too the message drops the claim rather than guessing at one.
|
||||
"""
|
||||
stub_deletion_start(mock_router)
|
||||
mock_router.add("GET", TASK, status=503, json=jsonapi_error(503, "Unavailable."))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_delete_provider", {"provider_id": "p1"}
|
||||
)
|
||||
|
||||
assert result.data["status"] == "in_progress"
|
||||
assert "could not be read" in result.data["message"]
|
||||
assert "Do not send the deletion again" in result.data["message"]
|
||||
# The failure that got us here is the classifier's to phrase, so its raw
|
||||
# text stays in the log rather than riding along in the message.
|
||||
assert "API request failed" not in result.data["message"]
|
||||
|
||||
|
||||
# ------------------------------------------------------- connection check
|
||||
|
||||
|
||||
async def test_a_connection_check_that_cannot_run_is_not_reported_as_failed(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""`not_tested` says nothing about the credentials, and that is the point.
|
||||
|
||||
A 401 here is this server's own credential, not the provider's. Calling it
|
||||
`failed` tells the user their AWS role is broken when it is fine.
|
||||
"""
|
||||
# Registered in order and consumed in order: the lookup before the create
|
||||
# finds nothing, the one after it finds the provider that was just made.
|
||||
mock_router.add("GET", PROVIDERS, json=jsonapi_collection([]))
|
||||
mock_router.add(
|
||||
"GET",
|
||||
PROVIDERS,
|
||||
json=jsonapi_collection(
|
||||
[jsonapi_resource("providers", "p1", PROVIDER_ATTRIBUTES)]
|
||||
),
|
||||
)
|
||||
mock_router.add(
|
||||
"POST",
|
||||
PROVIDERS,
|
||||
json=jsonapi_document(jsonapi_resource("providers", "p1", PROVIDER_ATTRIBUTES)),
|
||||
)
|
||||
mock_router.add(
|
||||
"POST", CONNECTION, status=401, json=jsonapi_error(401, "Token expired.")
|
||||
)
|
||||
mock_router.add(
|
||||
"GET",
|
||||
PROVIDER,
|
||||
json=jsonapi_document(jsonapi_resource("providers", "p1", PROVIDER_ATTRIBUTES)),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_connect_provider",
|
||||
{"provider_uid": "123456789012", "provider_type": "aws"},
|
||||
)
|
||||
|
||||
assert result.data["connected"] == "not_tested"
|
||||
assert "never tested" in result.data["error"]
|
||||
|
||||
|
||||
async def test_a_secret_lookup_failure_does_not_pass_as_having_no_secret(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Returning None here would send the write down the create branch.
|
||||
|
||||
A provider holds at most one secret, so creating a second one is refused and
|
||||
the caller would be told its credentials were rejected when all that
|
||||
actually failed was this read.
|
||||
"""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
PROVIDERS,
|
||||
json=jsonapi_collection(
|
||||
[jsonapi_resource("providers", "p1", PROVIDER_ATTRIBUTES)]
|
||||
),
|
||||
)
|
||||
mock_router.add(
|
||||
"GET", SECRETS, status=429, json=jsonapi_error(429, "Too many requests.")
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="rate limiting"):
|
||||
await client.call_tool(
|
||||
"prowler_connect_provider",
|
||||
{
|
||||
"provider_uid": "123456789012",
|
||||
"provider_type": "aws",
|
||||
"credentials": {
|
||||
"aws_access_key_id": "AKIA",
|
||||
"aws_secret_access_key": "s",
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
assert f"POST {SECRETS}" not in mock_router.paths()
|
||||
@@ -0,0 +1,147 @@
|
||||
"""Tests for the role (RBAC) tools.
|
||||
|
||||
``prowler_set_user_role`` replaces the single role a user holds, and the API
|
||||
silently drops a role ID that does not exist in the tenant -- which would leave
|
||||
the user with no role at all. So the tool reads the role first, and what that
|
||||
read says has to be told apart carefully: only a not-found is about the role ID
|
||||
the caller passed. A permission error, a rate limit or a server error is about
|
||||
the request, and reporting either as "find a valid role ID" sends the user to
|
||||
fix an ID that was fine.
|
||||
|
||||
Tools are driven through an in-memory MCP client so FastMCP resolves the
|
||||
pydantic ``Field`` defaults and a raised failure arrives the way a client sees
|
||||
it.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.http import MockRouter
|
||||
from tests.helpers.jsonapi import (
|
||||
jsonapi_document,
|
||||
jsonapi_error,
|
||||
jsonapi_resource,
|
||||
)
|
||||
|
||||
USER = "/api/v1/users/u1"
|
||||
USER_ROLES = f"{USER}/relationships/roles"
|
||||
ROLE = "/api/v1/roles/r2"
|
||||
|
||||
ROLE_ATTRIBUTES = {"name": "admin", "manage_account": True}
|
||||
|
||||
|
||||
def stub_user_holding(mock_router: MockRouter, role_id: str) -> MockRouter:
|
||||
"""Serve ``GET /users/u1?include=roles`` with the user holding one role."""
|
||||
return mock_router.add(
|
||||
"GET",
|
||||
USER,
|
||||
json=jsonapi_document(
|
||||
jsonapi_resource("users", "u1", {"name": "Ada"}),
|
||||
included=[jsonapi_resource("roles", role_id, ROLE_ATTRIBUTES)],
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
async def test_setting_a_role_the_user_already_holds_changes_nothing(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Idempotent by design: no PATCH goes out, so nothing can be replaced."""
|
||||
stub_user_holding(mock_router, "r2")
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_set_user_role", {"user_id": "u1", "role_id": "r2"}
|
||||
)
|
||||
|
||||
assert result.data["changed"] is False
|
||||
assert mock_router.paths() == [f"GET {USER}"]
|
||||
|
||||
|
||||
async def test_a_role_that_does_not_exist_is_named_as_the_bad_argument(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""404 is the one answer that really is about the role ID.
|
||||
|
||||
The PATCH would accept the ID and drop it, leaving the user with no role, so
|
||||
the read has to stop the call -- and say which ID to replace.
|
||||
"""
|
||||
stub_user_holding(mock_router, "r1")
|
||||
mock_router.add("GET", ROLE, status=404, json=jsonapi_error(404, "Not found."))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="does not exist in this tenant") as raised:
|
||||
await client.call_tool(
|
||||
"prowler_set_user_role", {"user_id": "u1", "role_id": "r2"}
|
||||
)
|
||||
|
||||
assert "left unchanged" in str(raised.value)
|
||||
assert f"PATCH {USER_ROLES}" not in mock_router.paths()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("status", "detail", "expected"),
|
||||
[
|
||||
(403, "Not allowed.", "not allowed to do this"),
|
||||
(429, "Slow down.", "rate limiting"),
|
||||
(500, "Boom.", "failed on Prowler's side"),
|
||||
],
|
||||
ids=["forbidden", "rate-limited", "server-error"],
|
||||
)
|
||||
async def test_a_role_read_that_failed_for_another_reason_is_not_a_bad_role_id(
|
||||
mcp_root_server, mock_api_client, mock_router, status, detail, expected
|
||||
):
|
||||
"""These say nothing about the ID, so the classifier owns the message.
|
||||
|
||||
Told "use prowler_list_roles to find a valid role ID", an agent goes looking
|
||||
for a role that was never the problem -- and finds the same wall.
|
||||
"""
|
||||
stub_user_holding(mock_router, "r1")
|
||||
mock_router.add("GET", ROLE, status=status, json=jsonapi_error(status, detail))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match=expected) as raised:
|
||||
await client.call_tool(
|
||||
"prowler_set_user_role", {"user_id": "u1", "role_id": "r2"}
|
||||
)
|
||||
|
||||
assert "valid role ID" not in str(raised.value)
|
||||
assert f"PATCH {USER_ROLES}" not in mock_router.paths()
|
||||
|
||||
|
||||
async def test_a_set_role_reports_the_role_the_user_holds_afterwards(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The authoritative state is read back rather than assumed from the PATCH."""
|
||||
mock_router.add(
|
||||
"GET",
|
||||
USER,
|
||||
json=jsonapi_document(
|
||||
jsonapi_resource("users", "u1", {"name": "Ada"}),
|
||||
included=[jsonapi_resource("roles", "r1", ROLE_ATTRIBUTES)],
|
||||
),
|
||||
)
|
||||
mock_router.add(
|
||||
"GET",
|
||||
USER,
|
||||
json=jsonapi_document(
|
||||
jsonapi_resource("users", "u1", {"name": "Ada"}),
|
||||
included=[jsonapi_resource("roles", "r2", ROLE_ATTRIBUTES)],
|
||||
),
|
||||
)
|
||||
mock_router.add(
|
||||
"GET",
|
||||
ROLE,
|
||||
json=jsonapi_document(jsonapi_resource("roles", "r2", ROLE_ATTRIBUTES)),
|
||||
)
|
||||
mock_router.add("PATCH", USER_ROLES, status=204)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_set_user_role", {"user_id": "u1", "role_id": "r2"}
|
||||
)
|
||||
|
||||
assert result.data["changed"] is True
|
||||
assert [role["id"] for role in result.data["roles"]] == ["r2"]
|
||||
assert mock_router.json_body("PATCH", USER_ROLES) == {
|
||||
"data": [{"type": "roles", "id": "r2"}]
|
||||
}
|
||||
@@ -0,0 +1,201 @@
|
||||
"""Tests for the scans tools.
|
||||
|
||||
Both write tools here used to answer a failure with a result object that the
|
||||
protocol, the client and the model all read as a success, and both are calls
|
||||
whose outcome an agent acts on:
|
||||
|
||||
* ``prowler_trigger_scan`` starts work. Anything that reads as "nothing
|
||||
happened" invites a second scan of the same provider.
|
||||
* ``prowler_schedule_daily_scan`` was deciding whether the schedule existed by
|
||||
reading the state of a different thing entirely -- the first scan Prowler
|
||||
starts alongside it -- so a schedule that had just been created could be
|
||||
reported as a failure. Retrying that can only hit the 409 the API raises for a
|
||||
provider that already has one.
|
||||
|
||||
Tools are driven through an in-memory MCP client so FastMCP resolves the
|
||||
pydantic ``Field`` defaults and a raised failure arrives the way a client sees
|
||||
it.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from fastmcp import Client
|
||||
|
||||
from tests.helpers.http import MockRouter
|
||||
from tests.helpers.jsonapi import (
|
||||
jsonapi_document,
|
||||
jsonapi_error,
|
||||
jsonapi_resource,
|
||||
)
|
||||
|
||||
SCANS = "/api/v1/scans"
|
||||
SCAN = f"{SCANS}/s1"
|
||||
DAILY = "/api/v1/schedules/daily"
|
||||
|
||||
SCAN_ATTRIBUTES = {
|
||||
"name": "Nightly",
|
||||
"trigger": "manual",
|
||||
"state": "executing",
|
||||
"progress": 40,
|
||||
}
|
||||
|
||||
|
||||
def stub_scan_creation(mock_router: MockRouter, scan_id: str = "s1") -> MockRouter:
|
||||
"""Serve the creation as Prowler does: a task carrying the new scan's ID."""
|
||||
return mock_router.add(
|
||||
"POST",
|
||||
SCANS,
|
||||
json=jsonapi_document(
|
||||
jsonapi_resource("tasks", "t1", {"task_args": {"scan_id": scan_id}})
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
# ------------------------------------------------------------- trigger_scan
|
||||
|
||||
|
||||
async def test_a_refused_scan_is_an_error_not_a_failed_looking_result(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""A rejection has to leave the tool as an error.
|
||||
|
||||
Returned as a result it arrives with ``isError: false``, and a model reading
|
||||
a successful tool call has no reason to doubt that a scan is now running.
|
||||
"""
|
||||
mock_router.add(
|
||||
"POST", SCANS, status=403, json=jsonapi_error(403, "Insufficient permissions.")
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="not allowed to do this"):
|
||||
await client.call_tool("prowler_trigger_scan", {"provider_id": "p1"})
|
||||
|
||||
|
||||
async def test_a_scan_with_no_id_back_warns_before_a_blind_retry(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Prowler accepted it, so a second call could start a duplicate scan.
|
||||
|
||||
The message names the provider because that is what makes the suggested
|
||||
check actionable without another lookup.
|
||||
"""
|
||||
mock_router.add("POST", SCANS, json={"data": {"attributes": {"task_args": {}}}})
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="did not return its ID"):
|
||||
await client.call_tool("prowler_trigger_scan", {"provider_id": "p1"})
|
||||
|
||||
assert mock_router.paths() == [f"POST {SCANS}"]
|
||||
|
||||
|
||||
async def test_a_scan_whose_read_back_fails_still_hands_over_the_id(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The scan is running; only reading it back went wrong.
|
||||
|
||||
Reporting the read failure alone would read as "the scan was not created"
|
||||
and invite a duplicate, so the error carries the ID to monitor instead.
|
||||
"""
|
||||
stub_scan_creation(mock_router)
|
||||
mock_router.add("GET", SCAN, status=400, json=jsonapi_error(400, "Server error."))
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="Scan s1 was created") as raised:
|
||||
await client.call_tool("prowler_trigger_scan", {"provider_id": "p1"})
|
||||
|
||||
# Naming the scan is the whole reason this message exists, so it is written
|
||||
# here rather than assembled from the failure. Splicing the failure text in
|
||||
# would put whatever it happens to say in front of the model unclassified,
|
||||
# which is the one thing the shared classifier exists to decide.
|
||||
assert "API request failed" not in str(raised.value)
|
||||
|
||||
|
||||
async def test_a_created_scan_comes_back_with_its_details(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The success path still reports the scan, which is what gets monitored."""
|
||||
stub_scan_creation(mock_router)
|
||||
mock_router.add(
|
||||
"GET",
|
||||
SCAN,
|
||||
json=jsonapi_document(jsonapi_resource("scans", "s1", SCAN_ATTRIBUTES)),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool("prowler_trigger_scan", {"provider_id": "p1"})
|
||||
|
||||
assert result.data["scan"]["id"] == "s1"
|
||||
# No status flag: a scan that was not created is raised, so "success" could
|
||||
# only ever be the one value and says nothing a reader can act on.
|
||||
assert "status" not in result.data
|
||||
|
||||
|
||||
# ------------------------------------------------------ schedule_daily_scan
|
||||
|
||||
|
||||
@pytest.mark.parametrize("first_run_state", ["available", "scheduled", "executing"])
|
||||
async def test_a_schedule_is_reported_created_whatever_the_first_run_does(
|
||||
mcp_root_server, mock_api_client, mock_router, first_run_state
|
||||
):
|
||||
"""The task in the answer is the first scan run, not the schedule.
|
||||
|
||||
Prowler commits the recurring schedule inside the request that serves this
|
||||
call, so an answer at all means it exists. Reading that task's state as the
|
||||
outcome of the scheduling reported a schedule that had just been created as
|
||||
a failure.
|
||||
"""
|
||||
mock_router.add(
|
||||
"POST",
|
||||
DAILY,
|
||||
json=jsonapi_document(
|
||||
jsonapi_resource("tasks", "t1", {"state": first_run_state})
|
||||
),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_schedule_daily_scan", {"provider_id": "p1"}
|
||||
)
|
||||
|
||||
assert result.data["first_run_state"] == first_run_state
|
||||
assert "every 24 hours" in result.data["message"]
|
||||
assert "scheduled" not in result.data
|
||||
|
||||
|
||||
async def test_a_failed_first_run_leaves_the_schedule_standing(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""Worth saying, but it is not the schedule that failed.
|
||||
|
||||
The gap it leaves is real -- no findings until tomorrow -- so the message
|
||||
points at the manual scan that fills it rather than at the scheduling.
|
||||
"""
|
||||
mock_router.add(
|
||||
"POST",
|
||||
DAILY,
|
||||
json=jsonapi_document(jsonapi_resource("tasks", "t1", {"state": "failed"})),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
result = await client.call_tool(
|
||||
"prowler_schedule_daily_scan", {"provider_id": "p1"}
|
||||
)
|
||||
|
||||
assert result.data["first_run_state"] == "failed"
|
||||
assert "schedule is unaffected" in result.data["message"]
|
||||
assert "prowler_trigger_scan" in result.data["message"]
|
||||
|
||||
|
||||
async def test_an_existing_schedule_is_relayed_as_the_api_explains_it(
|
||||
mcp_root_server, mock_api_client, mock_router
|
||||
):
|
||||
"""The 409 already says the useful thing, so the classifier relays it."""
|
||||
mock_router.add(
|
||||
"POST",
|
||||
DAILY,
|
||||
status=409,
|
||||
json=jsonapi_error(409, "There is already a scheduled scan for this provider."),
|
||||
)
|
||||
|
||||
async with Client(mcp_root_server) as client:
|
||||
with pytest.raises(Exception, match="already a scheduled scan"):
|
||||
await client.call_tool("prowler_schedule_daily_scan", {"provider_id": "p1"})
|
||||
@@ -6,8 +6,12 @@ Reference for later branches: ``ProwlerAppAuth`` resolves its ``mode`` and
|
||||
and ``base_url=`` explicitly, as these tests do.
|
||||
"""
|
||||
|
||||
import base64
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
from prowler_mcp_server.lib.errors import CredentialError
|
||||
from prowler_mcp_server.prowler_app.utils.auth import ProwlerAppAuth
|
||||
from tests.helpers.tokens import FAKE_API_KEY, MALFORMED_API_KEY, fake_jwt
|
||||
|
||||
@@ -42,13 +46,92 @@ async def test_http_mode_accepts_a_bearer_api_key(http_request_headers):
|
||||
assert await auth.get_valid_token() == FAKE_API_KEY
|
||||
|
||||
|
||||
def _jwt_with_payload(payload: object) -> str:
|
||||
"""Mint an unsigned JWT carrying an arbitrary payload.
|
||||
|
||||
``fake_jwt`` always writes a well-formed object, so the malformed payloads
|
||||
below are built here instead.
|
||||
"""
|
||||
encoded = (
|
||||
base64.urlsafe_b64encode(json.dumps(payload).encode()).decode().rstrip("=")
|
||||
)
|
||||
return f"header.{encoded}.fake-signature-not-verified"
|
||||
|
||||
|
||||
async def test_http_mode_accepts_a_lowercase_bearer_scheme(http_request_headers):
|
||||
"""Authentication scheme names are case-insensitive (RFC 7235)."""
|
||||
http_request_headers(authorization=f"bearer {FAKE_API_KEY}")
|
||||
|
||||
auth = ProwlerAppAuth(mode="http")
|
||||
|
||||
assert await auth.get_valid_token() == FAKE_API_KEY
|
||||
|
||||
|
||||
async def test_http_mode_strips_only_the_scheme_prefix(http_request_headers):
|
||||
"""A token that repeats the scheme keeps it: only the prefix is removed."""
|
||||
token = f"{FAKE_API_KEY}_Bearer_suffix"
|
||||
http_request_headers(authorization=f"Bearer {token}")
|
||||
|
||||
auth = ProwlerAppAuth(mode="http")
|
||||
|
||||
assert await auth.get_valid_token() == token
|
||||
|
||||
|
||||
async def test_http_mode_rejects_an_authorization_header_without_a_token(
|
||||
http_request_headers,
|
||||
):
|
||||
"""A bare scheme carries no credential to authenticate with."""
|
||||
http_request_headers(authorization="Bearer ")
|
||||
|
||||
auth = ProwlerAppAuth(mode="http")
|
||||
|
||||
with pytest.raises(CredentialError, match="'Bearer <token>' form"):
|
||||
await auth.get_valid_token()
|
||||
|
||||
|
||||
async def test_http_mode_rejects_a_jwt_whose_payload_is_not_an_object(
|
||||
http_request_headers,
|
||||
):
|
||||
"""A payload that decodes to a list has no claims, so it is a bad credential.
|
||||
|
||||
Without the type check it would reach `payload.get` and fail as an
|
||||
unclassified `AttributeError`, which the client only sees masked.
|
||||
"""
|
||||
http_request_headers(authorization=f"Bearer {_jwt_with_payload(['exp'])}")
|
||||
|
||||
auth = ProwlerAppAuth(mode="http")
|
||||
|
||||
with pytest.raises(CredentialError, match="not a readable JWT"):
|
||||
await auth.get_valid_token()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("payload", "case"),
|
||||
[
|
||||
({"sub": "user"}, "missing"),
|
||||
({"exp": "1700000000"}, "string"),
|
||||
({"exp": None}, "null"),
|
||||
],
|
||||
)
|
||||
async def test_http_mode_rejects_a_jwt_without_a_numeric_expiration(
|
||||
http_request_headers, payload: dict, case: str
|
||||
):
|
||||
"""`exp` is a numeric date: comparing anything else raises a `TypeError`."""
|
||||
http_request_headers(authorization=f"Bearer {_jwt_with_payload(payload)}")
|
||||
|
||||
auth = ProwlerAppAuth(mode="http")
|
||||
|
||||
with pytest.raises(CredentialError, match="no readable 'exp' expiration claim"):
|
||||
await auth.get_valid_token()
|
||||
|
||||
|
||||
async def test_http_mode_rejects_an_expired_jwt(http_request_headers):
|
||||
"""An expired JWT is refused locally instead of being forwarded to the API."""
|
||||
http_request_headers(authorization=f"Bearer {fake_jwt(expires_in=-60)}")
|
||||
|
||||
auth = ProwlerAppAuth(mode="http")
|
||||
|
||||
with pytest.raises(ValueError, match="Token has expired"):
|
||||
with pytest.raises(CredentialError, match="The token has expired"):
|
||||
await auth.get_valid_token()
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user