From e8ee0b3792d6ad57d918c0e8116ed5016960fc74 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Wed, 17 Jun 2026 11:29:12 -0600 Subject: [PATCH 1/2] fix(findings): normalize blank components to NULL (SC-13073) Findings could store an empty string for component_name/component_version depending on the write path (the edit form already stores NULL, but the API and importers do not). Because the database treats "" as distinct from NULL, the All Components view grouped these into a separate "None" row. Finding.save() now normalizes blank (empty or whitespace-only) component values to NULL, and a data migration converts existing empty-string values so component-less findings group together. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/content/releases/os_upgrading/3.1.md | 16 ++++ ...0269_normalize_blank_finding_components.py | 47 +++++++++++ dojo/models.py | 7 ++ unittests/test_edit_finding_endpoints.py | 77 +++++++++++++++++++ unittests/test_finding_model.py | 51 ++++++++++++ 5 files changed, 198 insertions(+) create mode 100644 docs/content/releases/os_upgrading/3.1.md create mode 100644 dojo/db_migrations/0269_normalize_blank_finding_components.py 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..712fac300fc --- /dev/null +++ b/dojo/db_migrations/0269_normalize_blank_finding_components.py @@ -0,0 +1,47 @@ +import logging + +from django.db import migrations +from django.db.models import Q +from django.db.models.functions import Trim + +logger = logging.getLogger(__name__) + + +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") + + blank_name = Finding.objects.annotate( + component_name_trimmed=Trim("component_name"), + ).filter(Q(component_name="") | Q(component_name_trimmed="")) + name_updated = blank_name.update(component_name=None) + + blank_version = Finding.objects.annotate( + component_version_trimmed=Trim("component_version"), + ).filter(Q(component_version="") | Q(component_version_trimmed="")) + version_updated = blank_version.update(component_version=None) + + 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"] From cac9a833f6b2261cdf4f0c6fdf675f64ee95fa02 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 18 Jun 2026 10:19:17 -0600 Subject: [PATCH 2/2] fix(findings): chunk blank-component normalization migration Process blank component_name/component_version rows in bounded seek-paged chunks instead of two unbounded UPDATE statements, so the migration never locks/writes "millions" of findings at once. Mirrors the page_size idiom in 0201_populate_finding_sla_expiration_date. Addresses review feedback on PR #15038. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...0269_normalize_blank_finding_components.py | 46 +++++++++++++++---- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/dojo/db_migrations/0269_normalize_blank_finding_components.py b/dojo/db_migrations/0269_normalize_blank_finding_components.py index 712fac300fc..4591d8e5d6d 100644 --- a/dojo/db_migrations/0269_normalize_blank_finding_components.py +++ b/dojo/db_migrations/0269_normalize_blank_finding_components.py @@ -6,6 +6,41 @@ 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): """ @@ -15,15 +50,8 @@ def normalize_blank_components(apps, schema_editor): """ Finding = apps.get_model("dojo", "Finding") - blank_name = Finding.objects.annotate( - component_name_trimmed=Trim("component_name"), - ).filter(Q(component_name="") | Q(component_name_trimmed="")) - name_updated = blank_name.update(component_name=None) - - blank_version = Finding.objects.annotate( - component_version_trimmed=Trim("component_version"), - ).filter(Q(component_version="") | Q(component_version_trimmed="")) - version_updated = blank_version.update(component_version=None) + 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(