test(mcp): cover the integrations tools and models

Add 51 tests over the mock HTTP router for the integrations tools and models,
pinning the connection-check choreography on create/update, the provider-id
validation per integration type, and the Jira dispatch retry safety.

Changes the tests drove out:

- 'prowler_list_integrations' stops requesting the 'configuration' field it
  discards, now that the API tolerates a sparse fieldset without it.
- 'JiraDispatchResult.from_task_result' owns the 'result is not an object'
  guard instead of duplicating it at the call site.
- 'prowler_update_integration' reads the current integration through
  'DetailedIntegration' rather than poking at the raw JSON:API payload, and
  refetches once after the PATCH instead of on each branch.
- The JSON:API providers linkage is built by one '_providers_relationship'
  helper shared by create and update.
- 'MockRouter.json_body()' lets tests assert on the document actually sent,
  since the API silently ignores attributes it does not recognise.

.github/actions/setup-python-uv/action.yml

@@ -18,6 +18,10 @@ inputs:
     description: 'Install Python dependencies with uv'
     required: false
     default: 'true'
+  update-sdk-lock:
+    description: 'Rewrite uv.lock to pin the latest prowler SDK commit on master'
+    required: false
+    default: 'true'
 
 runs:
   using: 'composite'
@@ -40,7 +44,7 @@ runs:
         fi
 
     - name: Update uv.lock with latest Prowler commit
-      if: github.repository_owner == 'prowler-cloud' && github.repository != 'prowler-cloud/prowler'
+      if: inputs.update-sdk-lock == 'true' && github.repository_owner == 'prowler-cloud' && github.repository != 'prowler-cloud/prowler'
       shell: bash
       working-directory: ${{ inputs.working-directory }}
       env:
@@ -60,7 +64,7 @@ runs:
         grep "prowler-cloud/prowler" uv.lock
 
     - name: Update uv.lock SDK commit (prowler repo on push)
-      if: github.event_name == 'push' && github.ref == 'refs/heads/master' && github.repository == 'prowler-cloud/prowler'
+      if: inputs.update-sdk-lock == 'true' && github.event_name == 'push' && github.ref == 'refs/heads/master' && github.repository == 'prowler-cloud/prowler'
       shell: bash
       working-directory: ${{ inputs.working-directory }}
       env:

.github/workflows/mcp-tests.yml

@@ -85,6 +85,7 @@ jobs:
         with:
           python-version: ${{ matrix.python-version }}
           working-directory: ./mcp_server
+          update-sdk-lock: 'false'
 
       - name: Run tests with pytest
         if: steps.check-changes.outputs.any_changed == 'true'

mcp_server/changelog.d/integrations-tests.added.md

@@ -0,0 +1 @@
+Test coverage for the integrations tools and models, pinning the connection-check choreography and the Jira dispatch retry safety

mcp_server/changelog.d/list-integrations-sparse-fieldset.changed.md

@@ -0,0 +1 @@
+`prowler_list_integrations` no longer requests the `configuration` it discards, now that the API tolerates a sparse fieldset without it

mcp_server/prowler_mcp_server/prowler_app/models/integrations.py

@@ -295,14 +295,20 @@ class JiraDispatchResult(MinimalSerializerMixin, BaseModel):
 
     @classmethod
     def from_task_result(
-        cls, result: dict[str, Any], task_id: str | None = None
+        cls, result: Any, task_id: str | None = None
     ) -> "JiraDispatchResult":
         """Build the dispatch result from the completed background task result.
 
         Raises:
-            ValueError: If the task result does not carry both counters. Defaulting them to
-                zero would report a dispatch as retryable when it may have created work items
+            ValueError: If the task result is not an object, or does not carry both
+                counters. Defaulting them to zero would report a dispatch as retryable
+                when it may have created work items
         """
+        if not isinstance(result, dict):
+            raise ValueError(
+                "The completed dispatch task did not report a result object."
+            )
+
         created_count = result.get("created_count")
         failed_count = result.get("failed_count")
 

mcp_server/prowler_mcp_server/prowler_app/tools/integrations.py

@@ -17,7 +17,6 @@ from prowler_mcp_server.prowler_app.models.integrations import (
     IntegrationsListResponse,
     JiraDispatchResult,
     JiraIssueTypes,
-    SimplifiedIntegration,
 )
 from prowler_mcp_server.prowler_app.tools.base import BaseTool
 
@@ -25,7 +24,7 @@ from prowler_mcp_server.prowler_app.tools.base import BaseTool
 # detailed view returned by prowler_get_integration
 INTEGRATION_LIST_FIELDS = (
     "enabled,connected,connection_last_checked_at,integration_type,providers,"
-    "configuration,inserted_at,updated_at"
+    "inserted_at,updated_at"
 )
 
 CONNECTION_CHECK_TIMEOUT = 120
@@ -36,6 +35,17 @@ JIRA_DISPATCH_TIMEOUT = 300
 JIRA_REQUIRED_CREDENTIALS = ("domain", "user_mail", "api_token")
 
 
+def _providers_relationship(provider_ids: list[str]) -> dict[str, Any]:
+    """Build the JSON:API relationship linkage attaching an integration to providers."""
+    return {
+        "providers": {
+            "data": [
+                {"type": "providers", "id": provider_id} for provider_id in provider_ids
+            ]
+        }
+    }
+
+
 class IntegrationsTools(BaseTool):
     """Tools for integration management operations.
 
@@ -484,12 +494,22 @@ class IntegrationsTools(BaseTool):
         self.logger.info(f"Updating integration {integration_id}...")
 
         try:
-            current = await self._get_integration_raw(integration_id)
-            current_attributes = current["attributes"]
-            integration_type = current_attributes["integration_type"]
+            current = DetailedIntegration.from_api_response(
+                await self._get_integration_raw(integration_id)
+            )
+            integration_type = current.integration_type
 
             if provider_ids is not None:
-                self._validate_provider_ids(integration_type, provider_ids)
+                if integration_type == "jira":
+                    raise ValueError(
+                        "Jira integrations are tenant-wide and cannot be attached to providers."
+                    )
+                if integration_type == "aws_security_hub" and len(provider_ids) != 1:
+                    raise ValueError(
+                        "AWS Security Hub integrations must stay attached to exactly one AWS "
+                        f"provider, got {len(provider_ids)}. Pass a single provider ID, or use "
+                        "prowler_delete_integration to stop sending findings to Security Hub."
+                    )
 
             attributes: dict[str, Any] = {}
             if enabled is not None:
@@ -507,20 +527,16 @@ class IntegrationsTools(BaseTool):
                         "Update the credentials instead, or run prowler_test_integration_connection to "
                         "refresh the available projects and issue types."
                     )
-                merged = dict(current_attributes.get("configuration") or {})
+                merged = dict(current.configuration)
                 merged.update(self._as_dict(configuration, "configuration"))
                 # Server-owned, the API repopulates it from the connection check
                 merged.pop("regions", None)
                 merged.pop("enabled_regions", None)
                 attributes["configuration"] = merged
 
-            providers_changed = provider_ids is not None and sorted(
-                provider_ids
-            ) != sorted(SimplifiedIntegration._extract_provider_ids(current))
-
             if not attributes and provider_ids is None:
                 self.logger.info("No changes provided, returning the current state")
-                return DetailedIntegration.from_api_response(current).model_dump()
+                return current.model_dump()
 
             update_body: dict[str, Any] = {
                 "data": {
@@ -530,33 +546,35 @@ class IntegrationsTools(BaseTool):
                 }
             }
             if provider_ids is not None:
-                update_body["data"]["relationships"] = {
-                    "providers": {
-                        "data": [
-                            {"type": "providers", "id": provider_id}
-                            for provider_id in provider_ids
-                        ]
-                    }
-                }
+                update_body["data"]["relationships"] = _providers_relationship(
+                    provider_ids
+                )
 
             await self.api_client.patch(
                 f"/integrations/{integration_id}", json_data=update_body
             )
 
             # A different provider means different effective credentials and different
-            # discovered configuration, so the stored connection state is stale
-            if (
+            # discovered configuration, so the stored connection state is stale too
+            providers_changed = provider_ids is not None and set(provider_ids) != set(
+                current.provider_ids
+            )
+            recheck_connection = (
                 credentials is not None
                 or configuration is not None
                 or providers_changed
-            ):
-                connection_status = await self._test_connection(integration_id)
-                updated = await self._get_integration_raw(integration_id)
+            )
+            connection_status = (
+                await self._test_connection(integration_id)
+                if recheck_connection
+                else None
+            )
+
+            updated = await self._get_integration_raw(integration_id)
+            if connection_status is not None:
                 return IntegrationConnectionStatus.create(
                     updated, connection_status
                 ).model_dump()
-
-            updated = await self._get_integration_raw(integration_id)
             return DetailedIntegration.from_api_response(updated).model_dump()
         except Exception as e:
             self.logger.error(f"Integration update failed: {e}")
@@ -769,14 +787,10 @@ class IntegrationsTools(BaseTool):
             self.logger.error(f"Jira dispatch did not complete cleanly: {e}")
             return await self._jira_dispatch_fallback(task_id, str(e))
 
-        task_result = completed_task.get("data", {}).get("attributes", {}).get("result")
-
         try:
-            if not isinstance(task_result, dict):
-                raise ValueError(
-                    "The completed dispatch task did not report a result object."
-                )
-            return JiraDispatchResult.from_task_result(task_result).model_dump()
+            return JiraDispatchResult.from_task_result(
+                completed_task.get("data", {}).get("attributes", {}).get("result")
+            ).model_dump()
         except ValueError as e:
             self.logger.error(f"Jira dispatch result could not be read: {e}")
             return self._jira_dispatch_unknown(task_id, str(e))
@@ -828,22 +842,6 @@ class IntegrationsTools(BaseTool):
             )
         return normalized
 
-    def _validate_provider_ids(
-        self, integration_type: str, provider_ids: list[str]
-    ) -> None:
-        """Reject provider changes an integration type cannot survive."""
-        if integration_type == "jira":
-            raise ValueError(
-                "Jira integrations are tenant-wide and cannot be attached to providers."
-            )
-
-        if integration_type == "aws_security_hub" and len(provider_ids) != 1:
-            raise ValueError(
-                "AWS Security Hub integrations must stay attached to exactly one AWS provider, "
-                f"got {len(provider_ids)}. Pass a single provider ID, or use "
-                "prowler_delete_integration to stop sending findings to Security Hub."
-            )
-
     def _validate_credentials(
         self, integration_type: str, credentials: dict[str, Any]
     ) -> dict[str, Any]:
@@ -934,14 +932,7 @@ class IntegrationsTools(BaseTool):
             }
         }
         if provider_ids:
-            create_body["data"]["relationships"] = {
-                "providers": {
-                    "data": [
-                        {"type": "providers", "id": provider_id}
-                        for provider_id in provider_ids
-                    ]
-                }
-            }
+            create_body["data"]["relationships"] = _providers_relationship(provider_ids)
 
         api_response = await self.api_client.post(
             "/integrations", json_data=create_body

mcp_server/tests/helpers/http.py

@@ -7,6 +7,7 @@ JSON decoding. A test that asserts on a recorded request is therefore asserting
 on the bytes that would really have gone out.
 """
 
+import json
 from collections.abc import Callable
 from typing import Any
 
@@ -103,6 +104,15 @@ class MockRouter:
         """Return the decoded query parameters of the last request for a route."""
         return dict(self.request_for(method, path).url.params)
 
+    def json_body(self, method: str, path: str) -> Any:
+        """Return the decoded JSON body of the last request for a route.
+
+        Write tools build a JSON:API document by hand, and the API silently
+        ignores an attribute it does not recognise, so the body is the only place
+        a misspelled key shows up.
+        """
+        return json.loads(self.request_for(method, path).content)
+
     def paths(self) -> list[str]:
         """Return every request made so far, as ``"METHOD /path"`` strings."""
         return [f"{request.method} {request.url.path}" for request in self.requests]

mcp_server/tests/prowler_app/models/test_integrations.py

@@ -0,0 +1,280 @@
+"""Tests for the integration models.
+
+Two things here are not ordinary serialization and carry the weight of the
+module: the Security Hub ``regions`` map, which is rewritten into the far smaller
+``enabled_regions`` list before an agent ever sees it, and the Jira dispatch
+result, whose ``safe_to_retry`` flag is the only thing standing between a
+half-finished dispatch and a project full of duplicated work items.
+"""
+
+import pytest
+
+from prowler_mcp_server.prowler_app.models.integrations import (
+    DetailedIntegration,
+    IntegrationConnectionStatus,
+    IntegrationsListResponse,
+    JiraDispatchResult,
+    JiraIssueTypes,
+    SimplifiedIntegration,
+)
+from tests.helpers.jsonapi import (
+    jsonapi_collection,
+    jsonapi_relationship_many,
+    jsonapi_resource,
+)
+
+S3_ATTRIBUTES = {
+    "integration_type": "amazon_s3",
+    "enabled": True,
+    "connected": True,
+    "connection_last_checked_at": "2025-01-15T10:00:00Z",
+    "inserted_at": "2025-01-10T09:00:00Z",
+    "updated_at": "2025-01-15T10:00:00Z",
+    "configuration": {"bucket_name": "my-reports", "output_directory": "prowler"},
+}
+
+SECURITY_HUB_ATTRIBUTES = {
+    "integration_type": "aws_security_hub",
+    "enabled": True,
+    "connected": True,
+    "configuration": {
+        "send_only_fails": True,
+        "archive_previous_findings": False,
+        "regions": {"us-east-1": True, "eu-west-1": False, "eu-west-3": True},
+    },
+}
+
+JIRA_ATTRIBUTES = {
+    "integration_type": "jira",
+    "enabled": True,
+    "connected": None,
+    "configuration": {"domain": "acme", "projects": {}, "issue_types": {}},
+}
+
+
+def test_simplified_integration_lifts_the_attached_provider_ids():
+    """`provider_ids` comes from the relationship linkage, not the attributes.
+
+    It is what tells an agent whether an integration covers the account it is
+    looking at, so reading it out of the wrong place silently scopes every
+    integration to the whole tenant.
+    """
+    integration = SimplifiedIntegration.from_api_response(
+        jsonapi_resource(
+            "integrations",
+            "i1",
+            S3_ATTRIBUTES,
+            relationships={
+                "providers": jsonapi_relationship_many("providers", "p1", "p2")
+            },
+        )
+    )
+
+    assert integration.provider_ids == ["p1", "p2"]
+    assert integration.integration_type == "amazon_s3"
+
+
+def test_a_never_checked_integration_still_reports_its_connected_field():
+    """`connected: null` means "never checked", which is not "not connected".
+
+    Every other empty value is dropped to save tokens, so without the override
+    this field would vanish exactly when its absence is most misleading.
+    """
+    integration = SimplifiedIntegration.from_api_response(
+        jsonapi_resource("integrations", "i1", {**JIRA_ATTRIBUTES, "connected": None})
+    )
+
+    dumped = integration.model_dump()
+
+    assert dumped["connected"] is None
+    # Contrast: an untouched empty field is dropped
+    assert "connection_last_checked_at" not in dumped
+
+
+def test_the_list_view_drops_a_configuration_the_api_still_sends():
+    """The sparse fieldset asks the API to leave `configuration` out.
+
+    The model must drop it anyway rather than pass it through: the fieldset is a
+    request, not a guarantee, and a Jira configuration listing every project of
+    the site is exactly what the separate detailed view exists to hold back.
+    """
+    integration = SimplifiedIntegration.from_api_response(
+        jsonapi_resource("integrations", "i1", JIRA_ATTRIBUTES)
+    )
+
+    assert "configuration" not in integration.model_dump()
+
+
+def test_security_hub_regions_are_collapsed_into_the_enabled_ones():
+    """The API returns every region of the partition with a boolean.
+
+    Only the enabled ones carry information, so the map is rewritten as a sorted
+    list. Passing the raw map through would spend tokens listing dozens of
+    regions to say "no".
+    """
+    integration = DetailedIntegration.from_api_response(
+        jsonapi_resource("integrations", "i1", SECURITY_HUB_ATTRIBUTES)
+    )
+
+    assert integration.configuration["enabled_regions"] == ["eu-west-3", "us-east-1"]
+    assert "regions" not in integration.configuration
+
+
+def test_an_unexpected_regions_shape_is_preserved_rather_than_dropped():
+    """A shape the rewrite does not understand is kept verbatim.
+
+    Silently dropping it would hide a real API change behind an integration that
+    merely looks like it has no regions enabled.
+    """
+    attributes = {
+        **SECURITY_HUB_ATTRIBUTES,
+        "configuration": {"regions": ["us-east-1"]},
+    }
+
+    integration = DetailedIntegration.from_api_response(
+        jsonapi_resource("integrations", "i1", attributes)
+    )
+
+    assert integration.configuration["regions"] == ["us-east-1"]
+    assert "enabled_regions" not in integration.configuration
+
+
+def test_a_non_security_hub_configuration_is_passed_through_untouched():
+    """Only Security Hub has a configuration worth rewriting."""
+    integration = DetailedIntegration.from_api_response(
+        jsonapi_resource("integrations", "i1", S3_ATTRIBUTES)
+    )
+
+    assert integration.configuration == S3_ATTRIBUTES["configuration"]
+
+
+def test_the_list_response_reports_the_pagination_of_the_whole_query():
+    """Counts come from `meta.pagination`, not from the length of this page."""
+    response = IntegrationsListResponse.from_api_response(
+        jsonapi_collection(
+            [jsonapi_resource("integrations", "i1", S3_ATTRIBUTES)],
+            page=2,
+            pages=3,
+            count=7,
+        )
+    )
+
+    assert [integration.id for integration in response.integrations] == ["i1"]
+    assert (response.total_num_integrations, response.total_num_pages) == (7, 3)
+    assert response.current_page == 2
+
+
+@pytest.mark.parametrize(
+    ("connected", "expected"),
+    [(True, "connected"), (False, "failed"), (None, "not_tested")],
+)
+def test_the_connection_check_maps_its_tri_state_onto_a_readable_outcome(
+    connected, expected
+):
+    """`null` is "the check did not run", which is not the same as a failure.
+
+    Collapsing it onto `failed` would send an agent chasing credentials that were
+    never actually tested.
+    """
+    status = IntegrationConnectionStatus.create(
+        jsonapi_resource("integrations", "i1", S3_ATTRIBUTES),
+        {"connected": connected},
+    )
+
+    assert status.connected == expected
+
+
+def test_an_unreadable_connection_result_raises_instead_of_guessing():
+    """Anything other than a boolean or null is an API change, not a failure."""
+    with pytest.raises(ValueError, match="unexpected connection check result"):
+        IntegrationConnectionStatus.create(
+            jsonapi_resource("integrations", "i1", S3_ATTRIBUTES),
+            {"connected": "yes"},
+        )
+
+
+def test_the_connection_error_is_only_reported_when_there_is_one():
+    """A successful check must not carry an empty `error` key."""
+    status = IntegrationConnectionStatus.create(
+        jsonapi_resource("integrations", "i1", S3_ATTRIBUTES), {"connected": True}
+    )
+
+    assert "error" not in status.model_dump()
+
+
+def test_jira_issue_types_are_read_from_a_wrapped_or_a_bare_payload():
+    """This endpoint returns a non-model resource, so both shapes must work."""
+    wrapped = JiraIssueTypes.from_api_response(
+        jsonapi_resource(
+            "jira-issue-types", "i1", {"project_key": "PROJ", "issue_types": ["Task"]}
+        )
+    )
+    bare = JiraIssueTypes.from_api_response(
+        {"project_key": "PROJ", "issue_types": ["Task"]}
+    )
+
+    assert (
+        wrapped.model_dump()
+        == bare.model_dump()
+        == {
+            "project_key": "PROJ",
+            "issue_types": ["Task"],
+        }
+    )
+
+
+def test_an_unreadable_issue_types_payload_raises():
+    """Returning an empty list would read as "this project has no issue types"."""
+    with pytest.raises(ValueError, match="unexpected Jira issue types payload"):
+        JiraIssueTypes.from_api_response({"project_key": "PROJ"})
+
+
+def test_a_dispatch_that_created_nothing_is_the_only_one_safe_to_retry():
+    """Work items are created one by one and Prowler cannot delete them.
+
+    So a retry is only safe when the run provably created none. Anything else
+    duplicates work items in a project a human then has to clean up.
+    """
+    empty = JiraDispatchResult.from_task_result({"created_count": 0, "failed_count": 3})
+    partial = JiraDispatchResult.from_task_result(
+        {"created_count": 1, "failed_count": 2}
+    )
+
+    assert empty.safe_to_retry is True
+    assert partial.safe_to_retry is False
+
+
+def test_a_zero_count_survives_serialization():
+    """Zero created work items is an outcome; an unknown count is not.
+
+    The minimal serializer drops empty values, so without the override a fully
+    failed dispatch would report no counts at all.
+    """
+    dumped = JiraDispatchResult.from_task_result(
+        {"created_count": 0, "failed_count": 3}
+    ).model_dump()
+
+    assert dumped["created_count"] == 0
+    assert dumped["failed_count"] == 3
+    assert dumped["status"] == "completed"
+
+
+@pytest.mark.parametrize(
+    "result",
+    [
+        {"failed_count": 2},
+        {"created_count": 1},
+        {"created_count": "1", "failed_count": 0},
+        None,
+        "done",
+    ],
+    ids=["no-created", "no-failed", "not-an-int", "null", "not-an-object"],
+)
+def test_a_dispatch_result_without_usable_counters_raises(result):
+    """Defaulting the counters to zero would report the run as safe to retry.
+
+    That is the one wrong answer here: it invites a second dispatch on top of
+    work items that may already exist.
+    """
+    with pytest.raises(ValueError, match="dispatch task did not report"):
+        JiraDispatchResult.from_task_result(result)