From c953fa7e673fa15adfd00463915fccc7bb91f964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Pe=C3=B1a?= Date: Fri, 27 Mar 2026 09:05:02 +0100 Subject: [PATCH] fix(api): resolve check_title filter to check_id for consistent finding-group counts (#10486) --- api/CHANGELOG.md | 1 + api/src/backend/api/filters.py | 86 +++++++++++++++++++------ api/src/backend/api/tests/test_views.py | 82 +++++++++++++++++++++++ api/src/backend/api/v1/serializers.py | 2 + api/src/backend/api/v1/views.py | 48 ++++++++++---- api/src/backend/conftest.py | 85 ++++++++++++++++++++++++ 6 files changed, 269 insertions(+), 35 deletions(-) diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index f2103ea4a4..8141929052 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -16,6 +16,7 @@ All notable changes to the **Prowler API** are documented in this file. - Filter transient Neo4j defunct connection logs in Sentry `before_send` to suppress false-positive alerts handled by `RetryableSession` retries [(#10452)](https://github.com/prowler-cloud/prowler/pull/10452) - `MANAGE_ACCOUNT` permission no longer required for listing and creating tenants [(#10468)](https://github.com/prowler-cloud/prowler/pull/10468) - Finding groups muted filter, counters, metadata extraction and mute reaggregation [(#10477)](https://github.com/prowler-cloud/prowler/pull/10477) +- Finding groups `check_title__icontains` resolution, `name__icontains` resource filter and `resource_group` field in `/resources` response [(#10486)](https://github.com/prowler-cloud/prowler/pull/10486) ## [1.23.0] (Prowler v5.22.0) diff --git a/api/src/backend/api/filters.py b/api/src/backend/api/filters.py index ab6df84834..e736bc667f 100644 --- a/api/src/backend/api/filters.py +++ b/api/src/backend/api/filters.py @@ -44,6 +44,7 @@ from api.models import ( ProviderGroup, ProviderSecret, Resource, + ResourceFindingMapping, ResourceTag, Role, Scan, @@ -197,17 +198,13 @@ class CommonFindingFilters(FilterSet): field_name="resource_services", lookup_expr="icontains" ) - resource_uid = CharFilter(field_name="resources__uid") - resource_uid__in = CharInFilter(field_name="resources__uid", lookup_expr="in") - resource_uid__icontains = CharFilter( - field_name="resources__uid", lookup_expr="icontains" - ) + resource_uid = CharFilter(method="filter_resource_uid") + resource_uid__in = CharInFilter(method="filter_resource_uid_in") + resource_uid__icontains = CharFilter(method="filter_resource_uid_icontains") - resource_name = CharFilter(field_name="resources__name") - resource_name__in = CharInFilter(field_name="resources__name", lookup_expr="in") - resource_name__icontains = CharFilter( - field_name="resources__name", lookup_expr="icontains" - ) + resource_name = CharFilter(method="filter_resource_name") + resource_name__in = CharInFilter(method="filter_resource_name_in") + resource_name__icontains = CharFilter(method="filter_resource_name_icontains") resource_type = CharFilter(method="filter_resource_type") resource_type__in = CharInFilter(field_name="resource_types", lookup_expr="overlap") @@ -266,10 +263,49 @@ class CommonFindingFilters(FilterSet): return queryset.filter(overall_query).distinct() def filter_check_title_icontains(self, queryset, name, value): + # Resolve from the summary table (has check_title column + trigram + # GIN index) instead of scanning JSON in the findings table. + matching_check_ids = ( + FindingGroupDailySummary.objects.filter( + check_title__icontains=value, + ) + .values_list("check_id", flat=True) + .distinct() + ) + return queryset.filter(check_id__in=matching_check_ids) + + # --- Resource subquery filters --- + # Resolve resource → RFM → finding_ids first, then filter findings + # by id__in. This avoids a 3-way JOIN driven from the (huge) + # findings side and lets PostgreSQL start from the resources + # unique-constraint index instead. + + @staticmethod + def _finding_ids_for_resources(**lookup): + return ResourceFindingMapping.objects.filter( + resource__in=Resource.objects.filter(**lookup).values("id") + ).values("finding_id") + + def filter_resource_uid(self, queryset, name, value): + return queryset.filter(id__in=self._finding_ids_for_resources(uid=value)) + + def filter_resource_uid_in(self, queryset, name, value): + return queryset.filter(id__in=self._finding_ids_for_resources(uid__in=value)) + + def filter_resource_uid_icontains(self, queryset, name, value): return queryset.filter( - Q(check_metadata__CheckTitle__icontains=value) - | Q(check_metadata__checktitle__icontains=value) - | Q(check_metadata__Checktitle__icontains=value) + id__in=self._finding_ids_for_resources(uid__icontains=value) + ) + + def filter_resource_name(self, queryset, name, value): + return queryset.filter(id__in=self._finding_ids_for_resources(name=value)) + + def filter_resource_name_in(self, queryset, name, value): + return queryset.filter(id__in=self._finding_ids_for_resources(name__in=value)) + + def filter_resource_name_icontains(self, queryset, name, value): + return queryset.filter( + id__in=self._finding_ids_for_resources(name__icontains=value) ) @@ -919,7 +955,19 @@ class LatestFindingGroupFilter(CommonFindingFilters): } -class FindingGroupSummaryFilter(FilterSet): +class _CheckTitleToCheckIdMixin: + """Resolve check_title search to check_ids so all provider rows are kept.""" + + def filter_check_title_to_check_ids(self, queryset, name, value): + matching_check_ids = ( + queryset.filter(check_title__icontains=value) + .values_list("check_id", flat=True) + .distinct() + ) + return queryset.filter(check_id__in=matching_check_ids) + + +class FindingGroupSummaryFilter(_CheckTitleToCheckIdMixin, FilterSet): """ Filter for FindingGroupDailySummary queries. @@ -942,9 +990,7 @@ class FindingGroupSummaryFilter(FilterSet): check_id = CharFilter(field_name="check_id", lookup_expr="exact") check_id__in = CharInFilter(field_name="check_id", lookup_expr="in") check_id__icontains = CharFilter(field_name="check_id", lookup_expr="icontains") - check_title__icontains = CharFilter( - field_name="check_title", lookup_expr="icontains" - ) + check_title__icontains = CharFilter(method="filter_check_title_to_check_ids") # Provider filters provider_id = UUIDFilter(field_name="provider_id", lookup_expr="exact") @@ -1032,7 +1078,7 @@ class FindingGroupSummaryFilter(FilterSet): return dt -class LatestFindingGroupSummaryFilter(FilterSet): +class LatestFindingGroupSummaryFilter(_CheckTitleToCheckIdMixin, FilterSet): """ Filter for FindingGroupDailySummary /latest endpoint. @@ -1044,9 +1090,7 @@ class LatestFindingGroupSummaryFilter(FilterSet): check_id = CharFilter(field_name="check_id", lookup_expr="exact") check_id__in = CharInFilter(field_name="check_id", lookup_expr="in") check_id__icontains = CharFilter(field_name="check_id", lookup_expr="icontains") - check_title__icontains = CharFilter( - field_name="check_title", lookup_expr="icontains" - ) + check_title__icontains = CharFilter(method="filter_check_title_to_check_ids") # Provider filters provider_id = UUIDFilter(field_name="provider_id", lookup_expr="exact") diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index ec907b149b..49026a1cd6 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -15810,6 +15810,50 @@ class TestFindingGroupViewSet: assert len(data) == 1 assert data[0]["id"] == "s3_bucket_public_access" + @pytest.mark.parametrize( + "extra_filters", + [ + {}, + {"filter[muted]": "include"}, + ], + ids=["summary_path", "finding_level_path"], + ) + def test_check_title_icontains_includes_all_title_variants( + self, + authenticated_client, + finding_groups_title_variants_fixture, + extra_filters, + ): + """ + Regression: two providers report the same check_id with different + checktitle values (e.g. after a Prowler version upgrade). Filtering + by check_title__icontains with a term that matches only ONE variant + must still return the finding group with counts from BOTH providers. + + Parametrized to cover both aggregation paths: + - summary_path: default, uses _CheckTitleToCheckIdMixin on summaries + - finding_level_path: filter[muted]=include forces CommonFindingFilters + """ + params = { + "filter[inserted_at]": TODAY, + "filter[check_title.icontains]": "Ensure repository", + **extra_filters, + } + response = authenticated_client.get( + reverse("finding-group-list"), + params, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 1 + assert data[0]["id"] == "github_secret_scanning_enabled" + attrs = data[0]["attributes"] + # Both providers' findings must be counted + assert attrs["fail_count"] == 2, ( + "fail_count must include findings from both providers, " + "regardless of which title variant matches the search" + ) + def test_resources_not_found(self, authenticated_client): """Test 404 returned for nonexistent check_id.""" response = authenticated_client.get( @@ -15851,6 +15895,44 @@ class TestFindingGroupViewSet: assert resource.get("region"), "resource.region must not be empty" assert resource.get("type"), "resource.type must not be empty" + def test_resources_resource_group( + self, authenticated_client, finding_groups_fixture + ): + """Test resource_group is extracted from check_metadata.resourcegroup.""" + response = authenticated_client.get( + reverse( + "finding-group-resources", kwargs={"pk": "s3_bucket_public_access"} + ), + {"filter[inserted_at]": TODAY}, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 2 + for item in data: + resource = item["attributes"]["resource"] + assert ( + resource["resource_group"] == "storage" + ), "resource_group must be 'storage'" + + def test_resources_name_icontains( + self, authenticated_client, finding_groups_fixture + ): + """Test resource_name__icontains filters resources by name substring.""" + # s3_bucket_public_access has "My Instance 1" and "My Instance 2" + response = authenticated_client.get( + reverse( + "finding-group-resources", kwargs={"pk": "s3_bucket_public_access"} + ), + { + "filter[inserted_at]": TODAY, + "filter[resource_name.icontains]": "Instance 1", + }, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 1 + assert "Instance 1" in data[0]["attributes"]["resource"]["name"] + def test_resources_provider_info( self, authenticated_client, finding_groups_fixture ): diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py index f627fb8963..993c680557 100644 --- a/api/src/backend/api/v1/serializers.py +++ b/api/src/backend/api/v1/serializers.py @@ -4194,6 +4194,7 @@ class FindingGroupResourceSerializer(BaseSerializerV1): "service": {"type": "string"}, "region": {"type": "string"}, "type": {"type": "string"}, + "resource_group": {"type": "string"}, }, } ) @@ -4205,6 +4206,7 @@ class FindingGroupResourceSerializer(BaseSerializerV1): "service": obj.get("resource_service", ""), "region": obj.get("resource_region", ""), "type": obj.get("resource_type", ""), + "resource_group": obj.get("resource_group", ""), } @extend_schema_field( diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index 9d4d311b46..7d5beeb66a 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -3483,7 +3483,7 @@ class FindingViewSet(PaginateByPkMixin, BaseRLSViewSet): request, filtered_queryset, manager=Finding.all_objects, - select_related=["scan"], + select_related=["scan__provider"], prefetch_related=["resources"], ) @@ -3653,7 +3653,7 @@ class FindingViewSet(PaginateByPkMixin, BaseRLSViewSet): tenant_id = request.tenant_id filtered_queryset = self.filter_queryset(self.get_queryset()) - latest_scan_ids = ( + latest_scan_ids = list( Scan.all_objects.filter(tenant_id=tenant_id, state=StateChoices.COMPLETED) .order_by("provider_id", "-inserted_at") .distinct("provider_id") @@ -3667,7 +3667,7 @@ class FindingViewSet(PaginateByPkMixin, BaseRLSViewSet): request, filtered_queryset, manager=Finding.all_objects, - select_related=["scan"], + select_related=["scan__provider"], prefetch_related=["resources"], ) @@ -6878,8 +6878,15 @@ class FindingGroupViewSet(BaseRLSViewSet): "resource_type__icontains": "type__icontains", } + # Fields accepted directly by LatestResourceFilter (no translation needed) + _RESOURCE_FILTER_FIELDS = { + f"{field}__{lookup}" + for field, lookups in LatestResourceFilter.Meta.fields.items() + for lookup in lookups + } | set(LatestResourceFilter.Meta.fields.keys()) + def _split_resource_filters(self, params: QueryDict) -> tuple[QueryDict, QueryDict]: - resource_keys = set(self.RESOURCE_FILTER_MAP) + resource_keys = set(self.RESOURCE_FILTER_MAP) | self._RESOURCE_FILTER_FIELDS finding_params = QueryDict(mutable=True) resource_params = QueryDict(mutable=True) for key, values in params.lists(): @@ -6900,11 +6907,16 @@ class FindingGroupViewSet(BaseRLSViewSet): queryset = queryset.filter(tenant_id=tenant_id) filter_params = QueryDict(mutable=True) - for key, mapped_key in self.RESOURCE_FILTER_MAP.items(): - if key not in params: + for key, values in params.lists(): + # Translate resource_* prefixed keys via the map + if key in self.RESOURCE_FILTER_MAP: + mapped_key = self.RESOURCE_FILTER_MAP[key] + elif key in self._RESOURCE_FILTER_FIELDS: + mapped_key = key + else: continue + if key == "resources" or key.endswith("__in"): - values = params.getlist(key) items: list[str] = [] for value in values: if value is None: @@ -7232,11 +7244,13 @@ class FindingGroupViewSet(BaseRLSViewSet): ), first_seen_at=Min("finding__first_seen_at"), last_seen_at=Max("finding__inserted_at"), - # Max() picks an arbitrary reason when a resource has multiple - # muted findings; this is acceptable because mute rules are - # applied per-check so all findings for the same resource - # share the same muted_reason in practice. + # Max() on muted_reason / check_metadata is safe because + # all findings for the same resource+check share identical + # values (mute rules and metadata are applied per-check). muted_reason=Max("finding__muted_reason"), + resource_group=Max( + KeyTextTransform("resourcegroup", "finding__check_metadata") + ), ) .filter(resource_id__isnull=False) .order_by("resource_id") @@ -7273,6 +7287,7 @@ class FindingGroupViewSet(BaseRLSViewSet): "first_seen_at": row["first_seen_at"], "last_seen_at": row["last_seen_at"], "muted_reason": row.get("muted_reason"), + "resource_group": row.get("resource_group", ""), } ) @@ -7307,14 +7322,19 @@ class FindingGroupViewSet(BaseRLSViewSet): raise ValidationError(filterset.errors) filtered_queryset = filterset.qs # Only include summaries from each provider's most recent date - # (within the filtered range) - filtered_queryset = filtered_queryset.annotate( + # (within the filtered range). + # We use a subquery to strip the Window annotation so it does not + # leak into the GROUP BY of _aggregate_daily_summaries. + latest_per_provider = filtered_queryset.annotate( _max_provider_date=Window( expression=Max("inserted_at"), partition_by=[F("provider_id")], ), ).filter(inserted_at=F("_max_provider_date")) - return self._aggregate_daily_summaries(filtered_queryset) + clean_queryset = FindingGroupDailySummary.objects.filter( + pk__in=latest_per_provider.values("pk") + ) + return self._aggregate_daily_summaries(clean_queryset) def _sorted_paginated_response(self, request, aggregated_queryset): """Apply ordering, pagination, post-processing, and return the Response.""" diff --git a/api/src/backend/conftest.py b/api/src/backend/conftest.py index 6535706cf7..8df1b19de6 100644 --- a/api/src/backend/conftest.py +++ b/api/src/backend/conftest.py @@ -2012,6 +2012,7 @@ def finding_groups_fixture( "CheckId": "s3_bucket_public_access", "checktitle": "Ensure S3 buckets do not allow public access", "Description": "S3 buckets should be configured to restrict public access.", + "resourcegroup": "storage", }, first_seen_at="2024-01-02T00:00:00Z", muted=False, @@ -2036,6 +2037,7 @@ def finding_groups_fixture( "CheckId": "s3_bucket_public_access", "checktitle": "Ensure S3 buckets do not allow public access", "Description": "S3 buckets should be configured to restrict public access.", + "resourcegroup": "storage", }, first_seen_at="2024-01-03T00:00:00Z", muted=False, @@ -2234,6 +2236,89 @@ def finding_groups_fixture( return findings +@pytest.fixture +def finding_groups_title_variants_fixture( + tenants_fixture, providers_fixture, scans_fixture, resources_fixture +): + """ + Two providers report the same check_id with different checktitle values. + + Simulates a Prowler version upgrade where the check title changed but the + check_id stayed the same. Used to verify that check_title__icontains + resolves to check_id first, so results include all providers regardless + of which title variant matches the search term. + """ + tenant = tenants_fixture[0] + provider1, provider2, *_ = providers_fixture + scan1, scan2, *_ = scans_fixture + resource1, resource2, *_ = resources_fixture + + findings = [] + + # Provider 1 — OLD title variant + finding_old = Finding.objects.create( + tenant_id=tenant.id, + uid="fg_title_variant_old", + scan=scan1, + delta="new", + status=Status.FAIL, + status_extended="Secret scanning not enabled", + impact=Severity.high, + impact_extended="High risk", + severity=Severity.high, + raw_result={"status": Status.FAIL, "severity": Severity.high}, + tags={}, + check_id="github_secret_scanning_enabled", + check_metadata={ + "CheckId": "github_secret_scanning_enabled", + "checktitle": "Ensure repository has secret scanning enabled", + "Description": "Checks if secret scanning is enabled.", + }, + first_seen_at="2024-01-01T00:00:00Z", + muted=False, + ) + finding_old.add_resources([resource1]) + findings.append(finding_old) + + # Provider 2 — NEW title variant (same check_id, different checktitle) + finding_new = Finding.objects.create( + tenant_id=tenant.id, + uid="fg_title_variant_new", + scan=scan2, + delta="new", + status=Status.FAIL, + status_extended="Secret scanning not enabled on repo", + impact=Severity.high, + impact_extended="High risk", + severity=Severity.high, + raw_result={"status": Status.FAIL, "severity": Severity.high}, + tags={}, + check_id="github_secret_scanning_enabled", + check_metadata={ + "CheckId": "github_secret_scanning_enabled", + "checktitle": "Check if secret scanning is enabled in GitHub", + "Description": "Checks if secret scanning is enabled.", + }, + first_seen_at="2024-01-02T00:00:00Z", + muted=False, + ) + finding_new.add_resources([resource2]) + findings.append(finding_new) + + from tasks.jobs.scan import aggregate_finding_group_summaries + + aggregate_finding_group_summaries( + tenant_id=str(tenant.id), + scan_id=str(scan1.id), + ) + aggregate_finding_group_summaries( + tenant_id=str(tenant.id), + scan_id=str(scan2.id), + ) + + return findings + + def pytest_collection_modifyitems(items): """Ensure test_rbac.py is executed first.""" items.sort(key=lambda item: 0 if "test_rbac.py" in item.nodeid else 1)