fix(findings): normalize blank components to NULL (SC-13073)

docs/content/releases/os_upgrading/3.1.md

@@ -0,0 +1,16 @@
+---
+title: 'Upgrading to DefectDojo Version 3.1.x'
+toc_hide: true
+weight: -20260617
+description: Blank Finding components are now normalized to NULL so component-less findings group together.
+---
+
+## Blank Finding components normalized to NULL
+
+Previously a Finding could store an empty string (`""`) for `component_name` or `component_version` depending on how it was created or edited. The database treats an empty string as distinct from `NULL`, so the **All Components** view (which groups findings by component) could show two separate "None" rows — one for findings with a `NULL` component and one for findings with an empty-string component.
+
+Findings without a component now consistently store `NULL`. Blank values are normalized to `NULL` when a Finding is saved, and a data migration converts existing empty-string component values to `NULL` on upgrade.
+
+### What you need to do
+
+Nothing — the change is applied automatically by the database migration included in this release. After upgrading, component-less findings will group together under a single "None" entry.

dojo/db_migrations/0269_normalize_blank_finding_components.py

@@ -0,0 +1,75 @@
+import logging
+
+from django.db import migrations
+from django.db.models import Q
+from django.db.models.functions import Trim
+
+logger = logging.getLogger(__name__)
+
+# Process blank components in bounded chunks so a single UPDATE never locks/writes
+# "millions" of findings at once. Matches the page_size convention used by other
+# Finding data migrations (e.g. 0201_populate_finding_sla_expiration_date).
+BATCH_SIZE = 1000
+
+
+def _normalize_field_to_null(Finding, field_name, batch_size=BATCH_SIZE):
+    """
+    Set blank (empty or whitespace-only) values of `field_name` to NULL, one
+    seek-paged chunk at a time. Returns the number of rows updated.
+
+    Pages over the blank queryset by `id__gt=last_id`: once a chunk is set to NULL it
+    no longer matches the blank filter, so each iteration re-evaluates the filter and
+    returns only not-yet-processed blank rows with a higher id. Rows at/below last_id
+    were already updated in a prior page, so the loop touches only blank rows.
+    """
+    trimmed = f"{field_name}_trimmed"
+    blank = (
+        Finding.objects.annotate(**{trimmed: Trim(field_name)})
+        .filter(Q(**{field_name: ""}) | Q(**{trimmed: ""}))
+        .order_by("id")
+    )
+
+    total = 0
+    last_id = 0
+    while True:
+        page_ids = list(blank.filter(id__gt=last_id).values_list("id", flat=True)[:batch_size])
+        if not page_ids:
+            break
+        last_id = page_ids[-1]
+        total += Finding.objects.filter(id__in=page_ids).update(**{field_name: None})
+        logger.info("Normalized %d blank %s values so far...", total, field_name)
+
+    return total
+
+
+def normalize_blank_components(apps, schema_editor):
+    """
+    Convert blank (empty or whitespace-only) Finding component_name/component_version
+    values to NULL so that findings without a component group together instead of
+    appearing as a separate "None" component group (SC-13073).
+    """
+    Finding = apps.get_model("dojo", "Finding")
+
+    name_updated = _normalize_field_to_null(Finding, "component_name")
+    version_updated = _normalize_field_to_null(Finding, "component_version")
+
+    if name_updated or version_updated:
+        logger.info(
+            "Normalized blank Finding components to NULL: %d component_name, %d component_version",
+            name_updated,
+            version_updated,
+        )
+
+
+def noop_reverse(apps, schema_editor):
+    pass
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("dojo", "0268_release_authorization_to_pro"),
+    ]
+
+    operations = [
+        migrations.RunPython(normalize_blank_components, noop_reverse),
+    ]

dojo/models.py

@@ -2748,6 +2748,13 @@ def save(self, dedupe_option=True, rules_option=True, product_grading_option=Tru
             user = get_current_user()
         # Title Casing
         self.title = titlecase(self.title[:511])
+        # Normalize blank component fields to NULL so that findings without a component
+        # group together. An empty string is treated as a distinct value from NULL by the
+        # database, which would otherwise produce a separate "None" component group (SC-13073).
+        if self.component_name is not None and not self.component_name.strip():
+            self.component_name = None
+        if self.component_version is not None and not self.component_version.strip():
+            self.component_version = None
         # Set the date of the finding if nothing is supplied
         if self.date is None:
             self.date = timezone.now()

unittests/test_edit_finding_endpoints.py

@@ -240,3 +240,80 @@ def test_post_keeps_all_selected_endpoints(self):
         self.finding.refresh_from_db()
         self.assertIn(self.endpoint, self.finding.endpoints.all())
         self.assertIn(endpoint2, self.finding.endpoints.all())
+
+
+@override_settings(V3_FEATURE_LOCATIONS=False)
+class TestEditFindingComponentView(TestCase):
+
+    """
+    Regression tests for SC-13073: editing a finding to remove its component must
+    store NULL (not an empty string) so the finding groups with other component-less
+    findings instead of landing in a separate "None" group.
+    """
+
+    def _minimal_post_data(self, **overrides):
+        data = {
+            "title": self.finding.title,
+            "date": "2024-01-01",
+            "severity": "High",
+            "description": "Test description",
+            "active": "on",
+            "verified": "",
+            "false_p": "",
+            "duplicate": "",
+            "out_of_scope": "",
+            "endpoints": [],
+            "endpoints_to_add": "",
+            "vulnerability_ids": "",
+            "references": "",
+            "mitigation": "",
+            "impact": "",
+            "steps_to_reproduce": "",
+            "severity_justification": "",
+        }
+        data.update(overrides)
+        return data
+
+    def setUp(self):
+        self.user = User.objects.create_user(
+            username="tester", password="pass",  # noqa: S106
+            is_staff=True, is_superuser=True,
+        )
+        self.client.force_login(self.user)
+        product_type = Product_Type.objects.create(name="PT")
+        self.product = Product.objects.create(name="P", prod_type=product_type, description="Test product")
+        engagement = Engagement.objects.create(
+            name="E", product=self.product, target_start=now(), target_end=now(),
+        )
+        test_type = Test_Type.objects.create(name="TT")
+        self.test_obj = Test.objects.create(
+            engagement=engagement, test_type=test_type,
+            target_start=now(), target_end=now(),
+        )
+        self.finding = Finding.objects.create(
+            title="Component Test Finding",
+            severity="High",
+            test=self.test_obj,
+            reporter=self.user,
+            component_name="django",
+            component_version="4.2",
+        )
+        self.url = reverse("edit_finding", args=[self.finding.id])
+
+    def test_clearing_component_stores_null(self):
+        """Submitting blank component fields stores NULL rather than an empty string."""
+        response = self.client.post(self.url, self._minimal_post_data(component_name="", component_version=""))
+
+        self.assertIn(response.status_code, [200, 302])
+        self.finding.refresh_from_db()
+        self.assertIsNone(self.finding.component_name)
+        self.assertIsNone(self.finding.component_version)
+
+    def test_setting_component_persists(self):
+        """A non-empty component value is still saved as-is."""
+        response = self.client.post(self.url, self._minimal_post_data(component_name="requests", component_version="2.0"))
+
+        self.assertIn(response.status_code, [200, 302])
+        self.finding.refresh_from_db()
+        self.assertEqual(self.finding.component_name, "requests")
+        self.assertEqual(self.finding.component_version, "2.0")

unittests/test_finding_model.py

@@ -501,6 +501,57 @@ def test_close_finding_with_naive_date(self):
         self.assertFalse(is_naive(status.mitigated_time))
 
 
+class TestFindingComponentNormalization(DojoTestCase):
+
+    """
+    SC-13073: empty component_name/component_version must be stored as NULL, not an
+    empty string. Otherwise the "All Components" view (which groups by component_name)
+    shows two separate "None" rows: one for NULL findings and one for "" findings.
+    """
+
+    def setUp(self):
+        self.user = User.objects.create_user(username="comp-tester", password="pass")  # noqa: S106
+        product_type = Product_Type.objects.create(name="PT comp")
+        product = Product.objects.create(name="P comp", description="d", prod_type=product_type)
+        engagement = Engagement.objects.create(
+            name="E comp", product=product, target_start=now(), target_end=now(),
+        )
+        test_type = Test_Type.objects.create(name="TT comp")
+        self.test = Test.objects.create(
+            engagement=engagement, test_type=test_type,
+            target_start=now(), target_end=now(),
+        )
+
+    def _save_finding(self, **kwargs):
+        finding = Finding.objects.create(title="Comp", severity="High", test=self.test, reporter=self.user, **kwargs)
+        finding.refresh_from_db()
+        return finding
+
+    def test_empty_component_name_normalized_to_none(self):
+        finding = self._save_finding(component_name="", component_version="")
+        self.assertIsNone(finding.component_name)
+        self.assertIsNone(finding.component_version)
+
+    def test_whitespace_component_name_normalized_to_none(self):
+        finding = self._save_finding(component_name="   ", component_version="\t")
+        self.assertIsNone(finding.component_name)
+        self.assertIsNone(finding.component_version)
+
+    def test_real_component_value_preserved(self):
+        finding = self._save_finding(component_name="django", component_version="4.2")
+        self.assertEqual(finding.component_name, "django")
+        self.assertEqual(finding.component_version, "4.2")
+
+    def test_empty_and_null_findings_group_together(self):
+        self._save_finding(component_name="", component_version="")
+        self._save_finding(component_name=None, component_version=None)
+        # Both should now share a single NULL component_name grouping
+        distinct_names = set(
+            Finding.objects.filter(test=self.test).values_list("component_name", flat=True),
+        )
+        self.assertEqual(distinct_names, {None})
+
+
 @versioned_fixtures
 class TestFindingSLAExpiration(DojoTestCase):
     fixtures = ["dojo_testdata.json"]