diff --git a/docs/content/releases/os_upgrading/3.1.md b/docs/content/releases/os_upgrading/3.1.md new file mode 100644 index 00000000000..5dfbff992db --- /dev/null +++ b/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. diff --git a/dojo/db_migrations/0269_normalize_blank_finding_components.py b/dojo/db_migrations/0269_normalize_blank_finding_components.py new file mode 100644 index 00000000000..4591d8e5d6d --- /dev/null +++ b/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), + ] diff --git a/dojo/models.py b/dojo/models.py index a41f5640889..7f798e23ae4 100644 --- a/dojo/models.py +++ b/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() diff --git a/unittests/test_edit_finding_endpoints.py b/unittests/test_edit_finding_endpoints.py index 33037b178b5..96f6462a4a8 100644 --- a/unittests/test_edit_finding_endpoints.py +++ b/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") diff --git a/unittests/test_finding_model.py b/unittests/test_finding_model.py index 78ee40693f2..136a5e0e5f8 100644 --- a/unittests/test_finding_model.py +++ b/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"]