From e4b2950436a6b3e46018e1933ff1a9ddd408d2c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Pe=C3=B1a?= Date: Thu, 9 Apr 2026 18:07:43 +0200 Subject: [PATCH] refactor(api): split finding-groups status from muted state (#10630) --- api/CHANGELOG.md | 3 + api/src/backend/api/filters.py | 5 +- .../0088_finding_group_status_muted_fields.py | 95 ++++++ ...089_backfill_finding_group_status_muted.py | 31 ++ api/src/backend/api/models.py | 34 +- api/src/backend/api/tests/test_views.py | 316 +++++++++++++++++- api/src/backend/api/v1/serializers.py | 19 ++ api/src/backend/api/v1/views.py | 223 +++++++++--- api/src/backend/tasks/jobs/scan.py | 86 ++++- api/src/backend/tasks/tasks.py | 49 ++- api/src/backend/tasks/tests/test_tasks.py | 85 ++++- 11 files changed, 853 insertions(+), 93 deletions(-) create mode 100644 api/src/backend/api/migrations/0088_finding_group_status_muted_fields.py create mode 100644 api/src/backend/api/migrations/0089_backfill_finding_group_status_muted.py diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 33c573924a..0fbb745d3c 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -11,15 +11,18 @@ All notable changes to the **Prowler API** are documented in this file. - `VALKEY_SCHEME`, `VALKEY_USERNAME`, and `VALKEY_PASSWORD` environment variables to configure Celery broker TLS/auth connection details for Valkey/ElastiCache [(#10420)](https://github.com/prowler-cloud/prowler/pull/10420) - `Vercel` provider support [(#10190)](https://github.com/prowler-cloud/prowler/pull/10190) - Finding groups list and latest endpoints support `sort=delta`, ordering by `new_count` then `changed_count` so groups with the most new findings rank highest [(#10606)](https://github.com/prowler-cloud/prowler/pull/10606) +- Finding group resources endpoints (`/finding-groups/{check_id}/resources` and `/finding-groups/latest/{check_id}/resources`) now expose `finding_id` per row, pointing to the most recent matching Finding for each resource. UUIDv7 ordering guarantees `Max(finding__id)` resolves to the latest snapshot [(#10630)](https://github.com/prowler-cloud/prowler/pull/10630) - Handle CIS and CISA SCuBA compliance framework from google workspace [(#10629)](https://github.com/prowler-cloud/prowler/pull/10629) ### 🔄 Changed +- Finding groups list/latest/resources now expose `status` ∈ `{FAIL, PASS, MANUAL}` and `muted: bool` as orthogonal fields. The aggregated `status` reflects the underlying check outcome regardless of mute state, and `muted=true` signals that every finding in the group/resource is muted. New `manual_count` is exposed alongside `pass_count`/`fail_count`, plus `pass_muted_count`/`fail_muted_count`/`manual_muted_count` siblings so clients can isolate the muted half of each status. The `new_*`/`changed_*` deltas are now broken down by status and mute state via 12 new counters (`new_fail_count`, `new_fail_muted_count`, `new_pass_count`, `new_pass_muted_count`, `new_manual_count`, `new_manual_muted_count` and the matching `changed_*` set). New `filter[muted]=true|false` and `sort=status` (FAIL > PASS > MANUAL) / `sort=muted` are supported. `filter[status]=MUTED` is no longer accepted [(#10630)](https://github.com/prowler-cloud/prowler/pull/10630) - Attack Paths: Periodic cleanup of stale scans with dead-worker detection via Celery inspect, marking orphaned `EXECUTING` scans as `FAILED` and recovering `graph_data_ready` [(#10387)](https://github.com/prowler-cloud/prowler/pull/10387) - Attack Paths: Replace `_provider_id` property with `_Provider_{uuid}` label for provider isolation, add regex-based label injection for custom queries [(#10402)](https://github.com/prowler-cloud/prowler/pull/10402) ### 🐞 Fixed +- `reaggregate_all_finding_group_summaries_task` now refreshes finding group daily summaries for every `(provider, day)` combination instead of only the latest scan per provider, matching the unbounded scope of `mute_historical_findings_task`. Mute rule operations no longer leave older daily summaries drifting from the underlying muted findings [(#10630)](https://github.com/prowler-cloud/prowler/pull/10630) - Finding groups list/latest now apply computed status/severity filters and finding-level prefilters (delta, region, service, category, resource group, scan, resource type), plus `check_title` support for sort/filter consistency [(#10428)](https://github.com/prowler-cloud/prowler/pull/10428) - Populate compliance data inside `check_metadata` for findings, which was always returned as `null` [(#10449)](https://github.com/prowler-cloud/prowler/pull/10449) - 403 error for admin users listing tenants due to roles query not using the admin database connection [(#10460)](https://github.com/prowler-cloud/prowler/pull/10460) diff --git a/api/src/backend/api/filters.py b/api/src/backend/api/filters.py index a496a0bf67..fa31289964 100644 --- a/api/src/backend/api/filters.py +++ b/api/src/backend/api/filters.py @@ -1115,13 +1115,14 @@ class FindingGroupAggregatedComputedFilter(FilterSet): STATUS_CHOICES = ( ("FAIL", "Fail"), ("PASS", "Pass"), - ("MUTED", "Muted"), + ("MANUAL", "Manual"), ) status = ChoiceFilter(method="filter_status", choices=STATUS_CHOICES) status__in = CharInFilter(method="filter_status_in", lookup_expr="in") severity = ChoiceFilter(method="filter_severity", choices=SeverityChoices) severity__in = CharInFilter(method="filter_severity_in", lookup_expr="in") + muted = BooleanFilter(field_name="muted") include_muted = BooleanFilter(method="filter_include_muted") def filter_status(self, queryset, name, value): @@ -1198,7 +1199,7 @@ class FindingGroupAggregatedComputedFilter(FilterSet): if value is True: return queryset # include_muted=false: exclude fully-muted groups - return queryset.exclude(fail_count=0, pass_count=0, muted_count__gt=0) + return queryset.exclude(muted=True) class ProviderSecretFilter(FilterSet): diff --git a/api/src/backend/api/migrations/0088_finding_group_status_muted_fields.py b/api/src/backend/api/migrations/0088_finding_group_status_muted_fields.py new file mode 100644 index 0000000000..ff3b981435 --- /dev/null +++ b/api/src/backend/api/migrations/0088_finding_group_status_muted_fields.py @@ -0,0 +1,95 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("api", "0087_vercel_provider"), + ] + + operations = [ + migrations.AddField( + model_name="findinggroupdailysummary", + name="manual_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="pass_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="fail_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="manual_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="muted", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="new_fail_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="new_fail_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="new_pass_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="new_pass_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="new_manual_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="new_manual_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="changed_fail_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="changed_fail_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="changed_pass_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="changed_pass_muted_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="changed_manual_count", + field=models.IntegerField(default=0), + ), + migrations.AddField( + model_name="findinggroupdailysummary", + name="changed_manual_muted_count", + field=models.IntegerField(default=0), + ), + ] diff --git a/api/src/backend/api/migrations/0089_backfill_finding_group_status_muted.py b/api/src/backend/api/migrations/0089_backfill_finding_group_status_muted.py new file mode 100644 index 0000000000..501fcf3cb4 --- /dev/null +++ b/api/src/backend/api/migrations/0089_backfill_finding_group_status_muted.py @@ -0,0 +1,31 @@ +from django.db import migrations +from tasks.tasks import backfill_finding_group_summaries_task + +from api.db_router import MainRouter +from api.rls import Tenant + + +def trigger_backfill_task(apps, schema_editor): + """ + Re-dispatch the finding-group backfill task for every tenant so the new + `manual_count` and `muted` columns added in 0088 get populated from the + last 10 days of completed scans. + + The aggregator (`aggregate_finding_group_summaries`) recomputes every + column on each call, so it back-populates the new fields without touching + the existing ones beyond a normal upsert. + """ + tenant_ids = Tenant.objects.using(MainRouter.admin_db).values_list("id", flat=True) + + for tenant_id in tenant_ids: + backfill_finding_group_summaries_task.delay(tenant_id=str(tenant_id), days=10) + + +class Migration(migrations.Migration): + dependencies = [ + ("api", "0088_finding_group_status_muted_fields"), + ] + + operations = [ + migrations.RunPython(trigger_backfill_task, migrations.RunPython.noop), + ] diff --git a/api/src/backend/api/models.py b/api/src/backend/api/models.py index 5e0880be08..7f3131fe7b 100644 --- a/api/src/backend/api/models.py +++ b/api/src/backend/api/models.py @@ -1748,15 +1748,45 @@ class FindingGroupDailySummary(RowLevelSecurityProtectedModel): # Severity stored as integer for MAX aggregation (5=critical, 4=high, etc.) severity_order = models.SmallIntegerField(default=1) - # Finding counts + # Finding counts (inclusive of muted findings; use the `muted` flag to + # tell whether the group has any actionable findings). pass_count = models.IntegerField(default=0) fail_count = models.IntegerField(default=0) + manual_count = models.IntegerField(default=0) muted_count = models.IntegerField(default=0) - # Delta counts + # Status counts restricted to muted findings, so clients can isolate the + # muted half of each status (e.g. `pass_count - pass_muted_count` gives the + # actionable PASS findings). + pass_muted_count = models.IntegerField(default=0) + fail_muted_count = models.IntegerField(default=0) + manual_muted_count = models.IntegerField(default=0) + + # Whether every finding for this (provider, check, day) is muted. + muted = models.BooleanField(default=False) + + # Delta counts (non-muted, kept for convenience and as a "total" view). new_count = models.IntegerField(default=0) changed_count = models.IntegerField(default=0) + # Delta breakdown by (status, muted) so clients can answer questions like + # "how many new failing findings appeared in this scan?" without scanning + # the underlying findings table. Mirrors the existing pass/fail/manual + # naming, with `_muted_count` siblings tracking the muted half of each + # bucket explicitly. + new_fail_count = models.IntegerField(default=0) + new_fail_muted_count = models.IntegerField(default=0) + new_pass_count = models.IntegerField(default=0) + new_pass_muted_count = models.IntegerField(default=0) + new_manual_count = models.IntegerField(default=0) + new_manual_muted_count = models.IntegerField(default=0) + changed_fail_count = models.IntegerField(default=0) + changed_fail_muted_count = models.IntegerField(default=0) + changed_pass_count = models.IntegerField(default=0) + changed_pass_muted_count = models.IntegerField(default=0) + changed_manual_count = models.IntegerField(default=0) + changed_manual_muted_count = models.IntegerField(default=0) + # Resource counts resources_fail = models.IntegerField(default=0) resources_total = models.IntegerField(default=0) diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index 439a355193..7457f20f4d 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -15445,10 +15445,16 @@ class TestFindingGroupViewSet: # iam_password_policy has only PASS findings assert data[0]["attributes"]["status"] == "PASS" - def test_finding_groups_status_muted_all( + def test_finding_groups_fully_muted_group_reflects_underlying_status( self, authenticated_client, finding_groups_fixture ): - """Test that MUTED status returned when all findings are muted.""" + """A fully-muted group still surfaces its underlying status (no MUTED). + + rds_encryption has 2 muted FAIL findings, so the group must report + status=FAIL (the orthogonal `muted` boolean signals it isn't actionable). + The status×muted breakdown lets clients answer 'how many failing + findings are muted in this group'. + """ response = authenticated_client.get( reverse("finding-group-list"), {"filter[inserted_at]": TODAY, "filter[check_id]": "rds_encryption"}, @@ -15456,8 +15462,21 @@ class TestFindingGroupViewSet: assert response.status_code == status.HTTP_200_OK data = response.json()["data"] assert len(data) == 1 - # rds_encryption has all muted findings - assert data[0]["attributes"]["status"] == "MUTED" + attrs = data[0]["attributes"] + assert attrs["status"] == "FAIL" + assert attrs["muted"] is True + assert attrs["fail_count"] == 2 + assert attrs["fail_muted_count"] == 2 + assert attrs["pass_muted_count"] == 0 + assert attrs["manual_muted_count"] == 0 + assert attrs["muted_count"] == 2 + # Sanity: the per-status muted counts must add up to muted_count. + assert ( + attrs["pass_muted_count"] + + attrs["fail_muted_count"] + + attrs["manual_muted_count"] + == attrs["muted_count"] + ) def test_finding_groups_status_filter( self, authenticated_client, finding_groups_fixture @@ -15949,7 +15968,7 @@ class TestFindingGroupViewSet: "extra_filters", [ {}, - {"filter[muted]": "include"}, + {"filter[delta]": "new"}, ], ids=["summary_path", "finding_level_path"], ) @@ -15967,7 +15986,8 @@ class TestFindingGroupViewSet: Parametrized to cover both aggregation paths: - summary_path: default, uses _CheckTitleToCheckIdMixin on summaries - - finding_level_path: filter[muted]=include forces CommonFindingFilters + - finding_level_path: filter[delta]=new forces _aggregate_findings via + CommonFindingFilters (delta is finding-level, not summary-level) """ params = { "filter[inserted_at]": TODAY, @@ -16885,3 +16905,287 @@ class TestFindingGroupViewSet: data = response.json()["data"] # Should still return data, not filtered by the old date assert len(data) == 5 + + def test_finding_groups_status_choices_no_muted( + self, authenticated_client, finding_groups_fixture + ): + """Every returned group must have status ∈ {FAIL, PASS, MANUAL}.""" + response = authenticated_client.get( + reverse("finding-group-list"), + {"filter[inserted_at]": TODAY}, + ) + assert response.status_code == status.HTTP_200_OK + statuses = {item["attributes"]["status"] for item in response.json()["data"]} + assert statuses, "fixture should produce at least one group" + assert statuses <= {"FAIL", "PASS", "MANUAL"} + assert "MUTED" not in statuses + + def test_finding_groups_serializer_exposes_muted_and_manual_count( + self, authenticated_client, finding_groups_fixture + ): + """The /finding-groups payload must expose `muted`, `manual_count` and + the per-status muted siblings (`pass_muted_count`/`fail_muted_count`/ + `manual_muted_count`).""" + response = authenticated_client.get( + reverse("finding-group-list"), + {"filter[inserted_at]": TODAY, "filter[check_id]": "iam_password_policy"}, + ) + assert response.status_code == status.HTTP_200_OK + attrs = response.json()["data"][0]["attributes"] + assert "muted" in attrs and isinstance(attrs["muted"], bool) + assert "manual_count" in attrs and isinstance(attrs["manual_count"], int) + assert attrs["muted"] is False # iam_password_policy has only non-muted PASS + assert attrs["manual_count"] == 0 + assert attrs["pass_muted_count"] == 0 + assert attrs["fail_muted_count"] == 0 + assert attrs["manual_muted_count"] == 0 + + @pytest.mark.parametrize( + "endpoint_name", ["finding-group-list", "finding-group-latest"] + ) + def test_finding_groups_filter_status_muted_is_rejected( + self, authenticated_client, finding_groups_fixture, endpoint_name + ): + """`filter[status]=MUTED` is no longer a valid status value.""" + params = {"filter[status]": "MUTED"} + if endpoint_name == "finding-group-list": + params["filter[inserted_at]"] = TODAY + + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @pytest.mark.parametrize( + "endpoint_name", ["finding-group-list", "finding-group-latest"] + ) + def test_finding_groups_filter_muted_true( + self, authenticated_client, finding_groups_fixture, endpoint_name + ): + """`filter[muted]=true` returns only fully-muted groups.""" + params = {"filter[muted]": "true"} + if endpoint_name == "finding-group-list": + params["filter[inserted_at]"] = TODAY + + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + check_ids = {item["id"] for item in data} + # Only rds_encryption is fully muted in the fixture + assert check_ids == {"rds_encryption"} + assert all(item["attributes"]["muted"] is True for item in data) + + @pytest.mark.parametrize( + "endpoint_name", ["finding-group-list", "finding-group-latest"] + ) + def test_finding_groups_filter_muted_false( + self, authenticated_client, finding_groups_fixture, endpoint_name + ): + """`filter[muted]=false` returns only groups with actionable findings.""" + params = {"filter[muted]": "false"} + if endpoint_name == "finding-group-list": + params["filter[inserted_at]"] = TODAY + + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + check_ids = {item["id"] for item in data} + assert "rds_encryption" not in check_ids + assert check_ids == { + "s3_bucket_public_access", + "ec2_instance_public_ip", + "iam_password_policy", + "cloudtrail_enabled", + } + assert all(item["attributes"]["muted"] is False for item in data) + + @pytest.mark.parametrize( + "endpoint_name", ["finding-group-list", "finding-group-latest"] + ) + def test_finding_groups_sort_by_status( + self, authenticated_client, finding_groups_fixture, endpoint_name + ): + """sort=status orders by aggregated status (FAIL > PASS > MANUAL).""" + priority = {"FAIL": 3, "PASS": 2, "MANUAL": 1} + params = {"sort": "-status"} + if endpoint_name == "finding-group-list": + params["filter[inserted_at]"] = TODAY + + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data, "fixture should produce groups" + + desc_keys = [priority[item["attributes"]["status"]] for item in data] + assert desc_keys == sorted(desc_keys, reverse=True) + + params["sort"] = "status" + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_200_OK + asc_keys = [ + priority[item["attributes"]["status"]] for item in response.json()["data"] + ] + assert asc_keys == sorted(asc_keys) + + @pytest.mark.parametrize( + "endpoint_name", ["finding-group-list", "finding-group-latest"] + ) + def test_finding_groups_sort_by_muted( + self, authenticated_client, finding_groups_fixture, endpoint_name + ): + """sort=muted orders by the boolean muted attribute.""" + # Need include_muted=true so the fully-muted group is part of the result + params = {"sort": "-muted", "filter[include_muted]": "true"} + if endpoint_name == "finding-group-list": + params["filter[inserted_at]"] = TODAY + + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data, "fixture should produce groups" + + muted_values = [item["attributes"]["muted"] for item in data] + # Descending boolean: True (1) before False (0) + assert muted_values == sorted(muted_values, reverse=True) + + @pytest.mark.parametrize( + "endpoint_name", ["finding-group-list", "finding-group-latest"] + ) + def test_finding_groups_delta_status_breakdown( + self, authenticated_client, finding_groups_fixture, endpoint_name + ): + """`new_*` and `changed_*` counters split by status and mute state. + + s3_bucket_public_access has 1 new FAIL and 1 changed FAIL (both + non-muted) so the breakdown must reflect exactly that and the totals + must equal the sum of the buckets. + """ + params = {"filter[check_id]": "s3_bucket_public_access"} + if endpoint_name == "finding-group-list": + params["filter[inserted_at]"] = TODAY + + response = authenticated_client.get(reverse(endpoint_name), params) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 1 + attrs = data[0]["attributes"] + + assert attrs["new_fail_count"] == 1 + assert attrs["new_fail_muted_count"] == 0 + assert attrs["new_pass_count"] == 0 + assert attrs["new_pass_muted_count"] == 0 + assert attrs["new_manual_count"] == 0 + assert attrs["new_manual_muted_count"] == 0 + assert attrs["changed_fail_count"] == 1 + assert attrs["changed_fail_muted_count"] == 0 + assert attrs["changed_pass_count"] == 0 + assert attrs["changed_pass_muted_count"] == 0 + assert attrs["changed_manual_count"] == 0 + assert attrs["changed_manual_muted_count"] == 0 + + new_total = ( + attrs["new_fail_count"] + + attrs["new_fail_muted_count"] + + attrs["new_pass_count"] + + attrs["new_pass_muted_count"] + + attrs["new_manual_count"] + + attrs["new_manual_muted_count"] + ) + changed_total = ( + attrs["changed_fail_count"] + + attrs["changed_fail_muted_count"] + + attrs["changed_pass_count"] + + attrs["changed_pass_muted_count"] + + attrs["changed_manual_count"] + + attrs["changed_manual_muted_count"] + ) + # The non-muted variants of the breakdown must sum to the legacy + # totals (new_count/changed_count are stored as non-muted). + assert ( + attrs["new_fail_count"] + + attrs["new_pass_count"] + + attrs["new_manual_count"] + == attrs["new_count"] + ) + assert ( + attrs["changed_fail_count"] + + attrs["changed_pass_count"] + + attrs["changed_manual_count"] + == attrs["changed_count"] + ) + # And the *full* breakdown (including the muted halves) is exposed + # so clients can also count muted-only deltas without losing data. + assert new_total >= attrs["new_count"] + assert changed_total >= attrs["changed_count"] + + def test_finding_groups_resources_serializer_exposes_muted( + self, authenticated_client, finding_groups_fixture + ): + """The /finding-groups//resources payload must expose `muted`.""" + response = authenticated_client.get( + reverse( + "finding-group-resources", + kwargs={"pk": "rds_encryption"}, + ), + {"filter[inserted_at]": TODAY}, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data, "rds_encryption should expose its resources" + for item in data: + attrs = item["attributes"] + assert "muted" in attrs and isinstance(attrs["muted"], bool) + # rds_encryption has all muted findings + assert attrs["muted"] is True + # Status reflects the underlying check outcome (FAIL), not MUTED + assert attrs["status"] == "FAIL" + + def test_finding_groups_resources_exposes_finding_id( + self, authenticated_client, finding_groups_fixture + ): + """The /resources payload exposes the most recent matching finding_id. + + rds_encryption has 2 findings, one per resource. Each resource row must + report the UUID of its corresponding Finding (UUIDv7 ordering means + Max(finding__id) resolves to the latest snapshot in time). + """ + response = authenticated_client.get( + reverse( + "finding-group-resources", + kwargs={"pk": "rds_encryption"}, + ), + {"filter[inserted_at]": TODAY}, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data, "rds_encryption should expose its resources" + + rds_finding_ids = { + str(f.id) for f in finding_groups_fixture if f.check_id == "rds_encryption" + } + assert rds_finding_ids, "fixture sanity" + + for item in data: + attrs = item["attributes"] + assert "finding_id" in attrs + assert attrs["finding_id"] in rds_finding_ids + + def test_finding_groups_latest_resources_exposes_finding_id( + self, authenticated_client, finding_groups_fixture + ): + """The /latest/.../resources payload also exposes finding_id.""" + response = authenticated_client.get( + reverse( + "finding-group-latest_resources", + kwargs={"check_id": "rds_encryption"}, + ), + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert data, "rds_encryption should expose its resources via /latest" + + rds_finding_ids = { + str(f.id) for f in finding_groups_fixture if f.check_id == "rds_encryption" + } + for item in data: + attrs = item["attributes"] + assert "finding_id" in attrs + assert attrs["finding_id"] in rds_finding_ids diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py index 6af4d01000..52ec47f4f5 100644 --- a/api/src/backend/api/v1/serializers.py +++ b/api/src/backend/api/v1/serializers.py @@ -4185,6 +4185,7 @@ class FindingGroupSerializer(BaseSerializerV1): check_description = serializers.CharField(required=False, allow_null=True) severity = serializers.CharField() status = serializers.CharField() + muted = serializers.BooleanField() impacted_providers = serializers.ListField( child=serializers.CharField(), required=False ) @@ -4192,9 +4193,25 @@ class FindingGroupSerializer(BaseSerializerV1): resources_total = serializers.IntegerField() pass_count = serializers.IntegerField() fail_count = serializers.IntegerField() + manual_count = serializers.IntegerField() + pass_muted_count = serializers.IntegerField() + fail_muted_count = serializers.IntegerField() + manual_muted_count = serializers.IntegerField() muted_count = serializers.IntegerField() new_count = serializers.IntegerField() changed_count = serializers.IntegerField() + new_fail_count = serializers.IntegerField() + new_fail_muted_count = serializers.IntegerField() + new_pass_count = serializers.IntegerField() + new_pass_muted_count = serializers.IntegerField() + new_manual_count = serializers.IntegerField() + new_manual_muted_count = serializers.IntegerField() + changed_fail_count = serializers.IntegerField() + changed_fail_muted_count = serializers.IntegerField() + changed_pass_count = serializers.IntegerField() + changed_pass_muted_count = serializers.IntegerField() + changed_manual_count = serializers.IntegerField() + changed_manual_muted_count = serializers.IntegerField() first_seen_at = serializers.DateTimeField(required=False, allow_null=True) last_seen_at = serializers.DateTimeField(required=False, allow_null=True) failing_since = serializers.DateTimeField(required=False, allow_null=True) @@ -4214,8 +4231,10 @@ class FindingGroupResourceSerializer(BaseSerializerV1): id = serializers.UUIDField(source="resource_id") resource = serializers.SerializerMethodField() provider = serializers.SerializerMethodField() + finding_id = serializers.UUIDField() status = serializers.CharField() severity = serializers.CharField() + muted = serializers.BooleanField() delta = serializers.CharField(required=False, allow_null=True) first_seen_at = serializers.DateTimeField(required=False, allow_null=True) last_seen_at = serializers.DateTimeField(required=False, allow_null=True) diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index 2392b818ac..23cfe17f2d 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -26,10 +26,11 @@ from config.settings.social_login import ( ) from dj_rest_auth.registration.views import SocialLoginView from django.conf import settings as django_settings -from django.contrib.postgres.aggregates import ArrayAgg, StringAgg +from django.contrib.postgres.aggregates import ArrayAgg, BoolAnd, StringAgg from django.contrib.postgres.search import SearchQuery from django.db import transaction from django.db.models import ( + BooleanField, Case, CharField, Count, @@ -7076,9 +7077,29 @@ class FindingGroupViewSet(BaseRLSViewSet): severity_order=Max("severity_order"), pass_count=Sum("pass_count"), fail_count=Sum("fail_count"), + manual_count=Sum("manual_count"), + pass_muted_count=Sum("pass_muted_count"), + fail_muted_count=Sum("fail_muted_count"), + manual_muted_count=Sum("manual_muted_count"), muted_count=Sum("muted_count"), + # The group is muted only if every contributing daily summary is + # itself fully muted. BoolAnd returns False as soon as one row has + # at least one actionable finding. + muted=BoolAnd("muted"), new_count=Sum("new_count"), changed_count=Sum("changed_count"), + new_fail_count=Sum("new_fail_count"), + new_fail_muted_count=Sum("new_fail_muted_count"), + new_pass_count=Sum("new_pass_count"), + new_pass_muted_count=Sum("new_pass_muted_count"), + new_manual_count=Sum("new_manual_count"), + new_manual_muted_count=Sum("new_manual_muted_count"), + changed_fail_count=Sum("changed_fail_count"), + changed_fail_muted_count=Sum("changed_fail_muted_count"), + changed_pass_count=Sum("changed_pass_count"), + changed_pass_muted_count=Sum("changed_pass_muted_count"), + changed_manual_count=Sum("changed_manual_count"), + changed_manual_muted_count=Sum("changed_manual_muted_count"), resources_total=Sum("resources_total"), resources_fail=Sum("resources_fail"), impacted_providers_str=StringAgg( @@ -7104,39 +7125,95 @@ class FindingGroupViewSet(BaseRLSViewSet): output_field=IntegerField(), ) - return queryset.values("check_id").annotate( - severity_order=Max(severity_case), - pass_count=Count("id", filter=Q(status="PASS", muted=False)), - fail_count=Count("id", filter=Q(status="FAIL", muted=False)), - muted_count=Count("id", filter=Q(muted=True)), - new_count=Count("id", filter=Q(delta="new", muted=False)), - changed_count=Count("id", filter=Q(delta="changed", muted=False)), - resources_total=Count("resources__id", distinct=True), - resources_fail=Count( - "resources__id", - distinct=True, - filter=Q(status="FAIL", muted=False), - ), - impacted_providers_str=StringAgg( - Cast("scan__provider__provider", CharField()), - delimiter=",", - distinct=True, - default="", - ), - agg_first_seen_at=Min("first_seen_at"), - agg_last_seen_at=Max("inserted_at"), - agg_failing_since=Min( - "first_seen_at", filter=Q(status="FAIL", muted=False) - ), - check_title=Coalesce( - Max(KeyTextTransform("checktitle", "check_metadata")), - Max(KeyTextTransform("CheckTitle", "check_metadata")), - Max(KeyTextTransform("Checktitle", "check_metadata")), - ), - check_description=Coalesce( - Max(KeyTextTransform("description", "check_metadata")), - Max(KeyTextTransform("Description", "check_metadata")), - ), + # `pass_count`, `fail_count` and `manual_count` count *every* finding + # for the check (muted or not) so the aggregated `status` reflects the + # underlying check outcome regardless of mute state. Whether the group + # is actionable is signalled by the orthogonal `muted` flag below. + return ( + queryset.values("check_id") + .annotate( + severity_order=Max(severity_case), + pass_count=Count("id", filter=Q(status="PASS")), + fail_count=Count("id", filter=Q(status="FAIL")), + manual_count=Count("id", filter=Q(status="MANUAL")), + pass_muted_count=Count("id", filter=Q(status="PASS", muted=True)), + fail_muted_count=Count("id", filter=Q(status="FAIL", muted=True)), + manual_muted_count=Count("id", filter=Q(status="MANUAL", muted=True)), + muted_count=Count("id", filter=Q(muted=True)), + nonmuted_count=Count("id", filter=Q(muted=False)), + new_count=Count("id", filter=Q(delta="new", muted=False)), + changed_count=Count("id", filter=Q(delta="changed", muted=False)), + new_fail_count=Count( + "id", filter=Q(delta="new", status="FAIL", muted=False) + ), + new_fail_muted_count=Count( + "id", filter=Q(delta="new", status="FAIL", muted=True) + ), + new_pass_count=Count( + "id", filter=Q(delta="new", status="PASS", muted=False) + ), + new_pass_muted_count=Count( + "id", filter=Q(delta="new", status="PASS", muted=True) + ), + new_manual_count=Count( + "id", filter=Q(delta="new", status="MANUAL", muted=False) + ), + new_manual_muted_count=Count( + "id", filter=Q(delta="new", status="MANUAL", muted=True) + ), + changed_fail_count=Count( + "id", filter=Q(delta="changed", status="FAIL", muted=False) + ), + changed_fail_muted_count=Count( + "id", filter=Q(delta="changed", status="FAIL", muted=True) + ), + changed_pass_count=Count( + "id", filter=Q(delta="changed", status="PASS", muted=False) + ), + changed_pass_muted_count=Count( + "id", filter=Q(delta="changed", status="PASS", muted=True) + ), + changed_manual_count=Count( + "id", filter=Q(delta="changed", status="MANUAL", muted=False) + ), + changed_manual_muted_count=Count( + "id", filter=Q(delta="changed", status="MANUAL", muted=True) + ), + resources_total=Count("resources__id", distinct=True), + resources_fail=Count( + "resources__id", + distinct=True, + filter=Q(status="FAIL", muted=False), + ), + impacted_providers_str=StringAgg( + Cast("scan__provider__provider", CharField()), + delimiter=",", + distinct=True, + default="", + ), + agg_first_seen_at=Min("first_seen_at"), + agg_last_seen_at=Max("inserted_at"), + agg_failing_since=Min( + "first_seen_at", filter=Q(status="FAIL", muted=False) + ), + check_title=Coalesce( + Max(KeyTextTransform("checktitle", "check_metadata")), + Max(KeyTextTransform("CheckTitle", "check_metadata")), + Max(KeyTextTransform("Checktitle", "check_metadata")), + ), + check_description=Coalesce( + Max(KeyTextTransform("description", "check_metadata")), + Max(KeyTextTransform("Description", "check_metadata")), + ), + ) + .annotate( + # Group is muted only if it has zero non-muted findings. + muted=Case( + When(nonmuted_count=0, then=Value(True)), + default=Value(False), + output_field=BooleanField(), + ), + ) ) def _split_computed_aggregate_filters( @@ -7148,6 +7225,7 @@ class FindingGroupViewSet(BaseRLSViewSet): "status__in", "severity", "severity__in", + "muted", "include_muted", } finding_params = QueryDict(mutable=True) @@ -7179,7 +7257,8 @@ class FindingGroupViewSet(BaseRLSViewSet): Post-process aggregation results to add computed fields. - Converts severity integer back to string - - Computes aggregated status (FAIL > PASS > MUTED) + - Computes aggregated status (FAIL > PASS > MANUAL); the orthogonal + ``muted`` boolean is already on the row from the SQL aggregation - Converts provider string to list """ results = [] @@ -7197,13 +7276,19 @@ class FindingGroupViewSet(BaseRLSViewSet): if "agg_failing_since" in row: row["failing_since"] = row.pop("agg_failing_since") - # Compute aggregated status + # Drop the helper count we use to derive `muted` in the + # finding-level aggregation path. + row.pop("nonmuted_count", None) + + # Compute aggregated status. Counts are inclusive of muted findings, + # so the underlying check outcome surfaces even when the group is + # fully muted. if row.get("fail_count", 0) > 0: row["status"] = "FAIL" elif row.get("pass_count", 0) > 0: row["status"] = "PASS" else: - row["status"] = "MUTED" + row["status"] = "MANUAL" # Convert provider string to list providers_str = row.pop("impacted_providers_str", "") or "" @@ -7219,9 +7304,12 @@ class FindingGroupViewSet(BaseRLSViewSet): "check_id": "check_id", "check_title": "check_title", "severity": "severity_order", + "status": "status_order", + "muted": "muted", "delta": "delta_order", "fail_count": "fail_count", "pass_count": "pass_count", + "manual_count": "manual_count", "muted_count": "muted_count", "new_count": "new_count", "changed_count": "changed_count", @@ -7276,7 +7364,7 @@ class FindingGroupViewSet(BaseRLSViewSet): return ordering def _apply_aggregated_computed_filters(self, queryset, computed_params: QueryDict): - """Apply computed filters (status/severity) on aggregated finding-group rows.""" + """Apply computed filters (status/severity/muted) on aggregated finding-group rows.""" if not computed_params: return queryset @@ -7285,14 +7373,16 @@ class FindingGroupViewSet(BaseRLSViewSet): aggregated_status=Case( When(fail_count__gt=0, then=Value("FAIL")), When(pass_count__gt=0, then=Value("PASS")), - default=Value("MUTED"), + default=Value("MANUAL"), output_field=CharField(), ) ) - # Exclude fully-muted groups by default unless include_muted is set - if "include_muted" not in computed_params: - queryset = queryset.exclude(fail_count=0, pass_count=0, muted_count__gt=0) + # Exclude fully-muted groups by default unless the caller has opted in + # via either `include_muted` or an explicit `muted` filter (the latter + # gives the caller direct control over the column). + if "include_muted" not in computed_params and "muted" not in computed_params: + queryset = queryset.exclude(muted=True) filterset = FindingGroupAggregatedComputedFilter( computed_params, queryset=queryset @@ -7347,18 +7437,14 @@ class FindingGroupViewSet(BaseRLSViewSet): provider_type=Max("resource__provider__provider"), provider_uid=Max("resource__provider__uid"), provider_alias=Max("resource__provider__alias"), + # status_order considers ALL findings (muted or not) so it + # surfaces FAIL/PASS/MANUAL based on the underlying check + # outcome. Whether the resource is actionable is signalled by + # the orthogonal `muted` flag below. status_order=Max( Case( - When( - finding__status="FAIL", - finding__muted=False, - then=Value(3), - ), - When( - finding__status="PASS", - finding__muted=False, - then=Value(2), - ), + When(finding__status="FAIL", then=Value(3)), + When(finding__status="PASS", then=Value(2)), default=Value(1), output_field=IntegerField(), ) @@ -7390,6 +7476,8 @@ class FindingGroupViewSet(BaseRLSViewSet): ), first_seen_at=Min("finding__first_seen_at"), last_seen_at=Max("finding__inserted_at"), + # True only if every finding for this resource+check is muted. + muted=BoolAnd("finding__muted"), # 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). @@ -7397,6 +7485,12 @@ class FindingGroupViewSet(BaseRLSViewSet): resource_group=Max( KeyTextTransform("resourcegroup", "finding__check_metadata") ), + # Most recent matching Finding for this (resource, check): + # Finding.id is a UUIDv7 (time-ordered in its high 48 bits). + # Cast to text first because PostgreSQL has no built-in + # `max(uuid)` aggregate; on the canonical lowercase form a + # lexicographic Max() still resolves to the latest snapshot. + finding_id=Max(Cast("finding__id", output_field=CharField())), ) .filter(resource_id__isnull=False) ) @@ -7405,8 +7499,8 @@ class FindingGroupViewSet(BaseRLSViewSet): _RESOURCE_SORT_ANNOTATIONS = { "status_order": lambda: Max( Case( - When(finding__status="FAIL", finding__muted=False, then=Value(3)), - When(finding__status="PASS", finding__muted=False, then=Value(2)), + When(finding__status="FAIL", then=Value(3)), + When(finding__status="PASS", then=Value(2)), default=Value(1), output_field=IntegerField(), ) @@ -7480,7 +7574,7 @@ class FindingGroupViewSet(BaseRLSViewSet): elif status_order == 2: status = "PASS" else: - status = "MUTED" + status = "MANUAL" delta_order = row.get("delta_order", 0) if delta_order == 2: @@ -7508,8 +7602,12 @@ class FindingGroupViewSet(BaseRLSViewSet): "delta": delta, "first_seen_at": row["first_seen_at"], "last_seen_at": row["last_seen_at"], + "muted": bool(row.get("muted", False)), "muted_reason": row.get("muted_reason"), "resource_group": row.get("resource_group", ""), + "finding_id": ( + str(row["finding_id"]) if row.get("finding_id") else None + ), } ) @@ -7570,6 +7668,21 @@ class FindingGroupViewSet(BaseRLSViewSet): sort_param, self._FINDING_GROUP_SORT_MAP ) if ordering: + # status_order is annotated on demand so groups can be sorted by + # their aggregated status (FAIL > PASS > MANUAL), mirroring the + # priority used in _post_process_aggregation. Counts are + # inclusive of muted findings, so the underlying check outcome + # surfaces even for fully muted groups. + if any(field.lstrip("-") == "status_order" for field in ordering): + aggregated_queryset = aggregated_queryset.annotate( + status_order=Case( + When(fail_count__gt=0, then=Value(3)), + When(pass_count__gt=0, then=Value(2)), + default=Value(1), + output_field=IntegerField(), + ) + ) + # delta_order is a virtual sort field: expand it to a # lexicographic ordering by (new_count, changed_count) so groups # with more new findings rank higher, with changed_count as the diff --git a/api/src/backend/tasks/jobs/scan.py b/api/src/backend/tasks/jobs/scan.py index 364b12d146..2c73c97f2f 100644 --- a/api/src/backend/tasks/jobs/scan.py +++ b/api/src/backend/tasks/jobs/scan.py @@ -1803,7 +1803,12 @@ def aggregate_finding_group_summaries(tenant_id: str, scan_id: str): output_field=IntegerField(), ) - # Aggregate findings by check_id for this scan + # Aggregate findings by check_id for this scan. + # `pass_count`, `fail_count` and `manual_count` count *every* finding + # in this group, regardless of mute state, so the aggregated `status` + # always reflects the underlying check outcome (FAIL > PASS > MANUAL) + # even when the group is fully muted. The orthogonal `muted` flag is + # what tells whether the group has any actionable (non-muted) findings. aggregated = ( Finding.objects.filter( tenant_id=tenant_id, @@ -1812,11 +1817,52 @@ def aggregate_finding_group_summaries(tenant_id: str, scan_id: str): .values("check_id") .annotate( severity_order=Max(severity_case), - pass_count=Count("id", filter=Q(status="PASS", muted=False)), - fail_count=Count("id", filter=Q(status="FAIL", muted=False)), + pass_count=Count("id", filter=Q(status="PASS")), + fail_count=Count("id", filter=Q(status="FAIL")), + manual_count=Count("id", filter=Q(status="MANUAL")), + pass_muted_count=Count("id", filter=Q(status="PASS", muted=True)), + fail_muted_count=Count("id", filter=Q(status="FAIL", muted=True)), + manual_muted_count=Count("id", filter=Q(status="MANUAL", muted=True)), muted_count=Count("id", filter=Q(muted=True)), + nonmuted_count=Count("id", filter=Q(muted=False)), new_count=Count("id", filter=Q(delta="new", muted=False)), changed_count=Count("id", filter=Q(delta="changed", muted=False)), + new_fail_count=Count( + "id", filter=Q(delta="new", status="FAIL", muted=False) + ), + new_fail_muted_count=Count( + "id", filter=Q(delta="new", status="FAIL", muted=True) + ), + new_pass_count=Count( + "id", filter=Q(delta="new", status="PASS", muted=False) + ), + new_pass_muted_count=Count( + "id", filter=Q(delta="new", status="PASS", muted=True) + ), + new_manual_count=Count( + "id", filter=Q(delta="new", status="MANUAL", muted=False) + ), + new_manual_muted_count=Count( + "id", filter=Q(delta="new", status="MANUAL", muted=True) + ), + changed_fail_count=Count( + "id", filter=Q(delta="changed", status="FAIL", muted=False) + ), + changed_fail_muted_count=Count( + "id", filter=Q(delta="changed", status="FAIL", muted=True) + ), + changed_pass_count=Count( + "id", filter=Q(delta="changed", status="PASS", muted=False) + ), + changed_pass_muted_count=Count( + "id", filter=Q(delta="changed", status="PASS", muted=True) + ), + changed_manual_count=Count( + "id", filter=Q(delta="changed", status="MANUAL", muted=False) + ), + changed_manual_muted_count=Count( + "id", filter=Q(delta="changed", status="MANUAL", muted=True) + ), resources_total=Count("resources__id", distinct=True), resources_fail=Count( "resources__id", @@ -1895,9 +1941,26 @@ def aggregate_finding_group_summaries(tenant_id: str, scan_id: str): severity_order=row["severity_order"] or 1, pass_count=row["pass_count"], fail_count=row["fail_count"], + manual_count=row["manual_count"], + pass_muted_count=row["pass_muted_count"], + fail_muted_count=row["fail_muted_count"], + manual_muted_count=row["manual_muted_count"], muted_count=row["muted_count"], + muted=row["nonmuted_count"] == 0, new_count=row["new_count"], changed_count=row["changed_count"], + new_fail_count=row["new_fail_count"], + new_fail_muted_count=row["new_fail_muted_count"], + new_pass_count=row["new_pass_count"], + new_pass_muted_count=row["new_pass_muted_count"], + new_manual_count=row["new_manual_count"], + new_manual_muted_count=row["new_manual_muted_count"], + changed_fail_count=row["changed_fail_count"], + changed_fail_muted_count=row["changed_fail_muted_count"], + changed_pass_count=row["changed_pass_count"], + changed_pass_muted_count=row["changed_pass_muted_count"], + changed_manual_count=row["changed_manual_count"], + changed_manual_muted_count=row["changed_manual_muted_count"], resources_total=row["resources_total"], resources_fail=row["resources_fail"], first_seen_at=row["agg_first_seen_at"], @@ -1917,9 +1980,26 @@ def aggregate_finding_group_summaries(tenant_id: str, scan_id: str): "severity_order", "pass_count", "fail_count", + "manual_count", + "pass_muted_count", + "fail_muted_count", + "manual_muted_count", "muted_count", + "muted", "new_count", "changed_count", + "new_fail_count", + "new_fail_muted_count", + "new_pass_count", + "new_pass_muted_count", + "new_manual_count", + "new_manual_muted_count", + "changed_fail_count", + "changed_fail_muted_count", + "changed_pass_count", + "changed_pass_muted_count", + "changed_manual_count", + "changed_manual_muted_count", "resources_total", "resources_fail", "first_seen_at", diff --git a/api/src/backend/tasks/tasks.py b/api/src/backend/tasks/tasks.py index fbd0440af8..bbf4d89772 100644 --- a/api/src/backend/tasks/tasks.py +++ b/api/src/backend/tasks/tasks.py @@ -771,26 +771,49 @@ def aggregate_finding_group_summaries_task(tenant_id: str, scan_id: str): ) @set_tenant(keep_tenant=True) def reaggregate_all_finding_group_summaries_task(tenant_id: str): - """Reaggregate finding group summaries for all providers' latest completed scans.""" - latest_scan_ids = list( - Scan.objects.filter(tenant_id=tenant_id, state=StateChoices.COMPLETED) - .order_by("provider_id", "-completed_at", "-inserted_at") - .distinct("provider_id") - .values_list("id", flat=True) + """Reaggregate finding group summaries for every (provider, day) combination. + + Mirrors the unbounded scope of `mute_historical_findings_task`: that task + rewrites every Finding row whose UID matches a mute rule, with no time + limit. To keep the daily summaries consistent with that update, this task + re-runs the aggregator on the latest completed scan of every (provider, + day) pair that exists in the database. Tasks are dispatched in parallel + via a Celery group so the wallclock scales with the worker pool, not with + the number of pairs. + """ + completed_scans = list( + Scan.objects.filter( + tenant_id=tenant_id, + state=StateChoices.COMPLETED, + completed_at__isnull=False, + ) + .order_by("-completed_at") + .values("id", "completed_at", "provider_id") ) - if latest_scan_ids: + + # Keep the latest scan per (provider, day) pair so the daily summary row + # the aggregator writes is the most recent snapshot of that day for that + # provider. Iterating from most recent to oldest means the first scan we + # see for a given key wins. + latest_scans: dict[tuple, str] = {} + for scan in completed_scans: + key = (scan["provider_id"], scan["completed_at"].date()) + if key not in latest_scans: + latest_scans[key] = str(scan["id"]) + + scan_ids = list(latest_scans.values()) + if scan_ids: logger.info( - "Reaggregating finding group summaries for %d scans: %s", - len(latest_scan_ids), - latest_scan_ids, + "Reaggregating finding group summaries for %d scans (provider x day)", + len(scan_ids), ) group( aggregate_finding_group_summaries_task.si( - tenant_id=tenant_id, scan_id=str(scan_id) + tenant_id=tenant_id, scan_id=scan_id ) - for scan_id in latest_scan_ids + for scan_id in scan_ids ).apply_async() - return {"scans_reaggregated": len(latest_scan_ids)} + return {"scans_reaggregated": len(scan_ids)} @shared_task(base=RLSTask, name="lighthouse-connection-check") diff --git a/api/src/backend/tasks/tests/test_tasks.py b/api/src/backend/tasks/tests/test_tasks.py index 8469b7db09..4a4108607e 100644 --- a/api/src/backend/tasks/tests/test_tasks.py +++ b/api/src/backend/tasks/tests/test_tasks.py @@ -1,6 +1,6 @@ import uuid from contextlib import contextmanager -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from unittest.mock import MagicMock, patch import openai @@ -2362,35 +2362,96 @@ class TestReaggregateAllFindingGroupSummaries: @patch("tasks.tasks.group") @patch("tasks.tasks.aggregate_finding_group_summaries_task") @patch("tasks.tasks.Scan.objects.filter") - def test_dispatches_subtasks_for_each_provider( + def test_dispatches_subtasks_for_each_provider_per_day( self, mock_scan_filter, mock_agg_task, mock_group ): - scan_id_1 = uuid.uuid4() - scan_id_2 = uuid.uuid4() + provider_id_1 = uuid.uuid4() + provider_id_2 = uuid.uuid4() + scan_id_today_p1 = uuid.uuid4() + scan_id_yesterday_p1 = uuid.uuid4() + scan_id_today_p2 = uuid.uuid4() + today = datetime.now(tz=timezone.utc) + yesterday = today - timedelta(days=1) + mock_group_result = MagicMock() mock_group.side_effect = lambda gen: (list(gen), mock_group_result)[1] - mock_scan_filter.return_value.order_by.return_value.distinct.return_value.values_list.return_value = [ - scan_id_1, - scan_id_2, + mock_scan_filter.return_value.order_by.return_value.values.return_value = [ + { + "id": scan_id_today_p1, + "completed_at": today, + "provider_id": provider_id_1, + }, + { + "id": scan_id_today_p2, + "completed_at": today, + "provider_id": provider_id_2, + }, + { + "id": scan_id_yesterday_p1, + "completed_at": yesterday, + "provider_id": provider_id_1, + }, ] result = reaggregate_all_finding_group_summaries_task(tenant_id=self.tenant_id) - assert result == {"scans_reaggregated": 2} - assert mock_agg_task.si.call_count == 2 + assert result == {"scans_reaggregated": 3} + assert mock_agg_task.si.call_count == 3 mock_agg_task.si.assert_any_call( - tenant_id=self.tenant_id, scan_id=str(scan_id_1) + tenant_id=self.tenant_id, scan_id=str(scan_id_today_p1) ) mock_agg_task.si.assert_any_call( - tenant_id=self.tenant_id, scan_id=str(scan_id_2) + tenant_id=self.tenant_id, scan_id=str(scan_id_today_p2) + ) + mock_agg_task.si.assert_any_call( + tenant_id=self.tenant_id, scan_id=str(scan_id_yesterday_p1) + ) + mock_group_result.apply_async.assert_called_once() + + @patch("tasks.tasks.group") + @patch("tasks.tasks.aggregate_finding_group_summaries_task") + @patch("tasks.tasks.Scan.objects.filter") + def test_dedupes_scans_to_latest_per_provider_per_day( + self, mock_scan_filter, mock_agg_task, mock_group + ): + """When several scans run on the same day for the same provider, only + the latest one is dispatched (matching the daily summary unique key).""" + provider_id = uuid.uuid4() + latest_scan_today = uuid.uuid4() + earlier_scan_today = uuid.uuid4() + today_late = datetime.now(tz=timezone.utc) + today_early = today_late - timedelta(hours=4) + + mock_group_result = MagicMock() + mock_group.side_effect = lambda gen: (list(gen), mock_group_result)[1] + + # Returned ordered by `-completed_at`, so the most recent comes first. + mock_scan_filter.return_value.order_by.return_value.values.return_value = [ + { + "id": latest_scan_today, + "completed_at": today_late, + "provider_id": provider_id, + }, + { + "id": earlier_scan_today, + "completed_at": today_early, + "provider_id": provider_id, + }, + ] + + result = reaggregate_all_finding_group_summaries_task(tenant_id=self.tenant_id) + + assert result == {"scans_reaggregated": 1} + mock_agg_task.si.assert_called_once_with( + tenant_id=self.tenant_id, scan_id=str(latest_scan_today) ) mock_group_result.apply_async.assert_called_once() @patch("tasks.tasks.group") @patch("tasks.tasks.Scan.objects.filter") def test_no_completed_scans_skips_dispatch(self, mock_scan_filter, mock_group): - mock_scan_filter.return_value.order_by.return_value.distinct.return_value.values_list.return_value = [] + mock_scan_filter.return_value.order_by.return_value.values.return_value = [] result = reaggregate_all_finding_group_summaries_task(tenant_id=self.tenant_id)