Prevent reimport from reactivating duplicate findings as active/verified (#14935)

* Prevent reimport from reactivating duplicate findings as active/verified

Fixes #14910. process_matched_mitigated_finding reactivated a matched
mitigated finding without checking whether it is a duplicate, producing
an invalid active/verified duplicate state that the finding edit form
rejects. Keep duplicates inactive/unverified on reactivation (un-mitigate
only), matching the set_duplicate invariant.

* Initialise reimporter accumulators in duplicate reactivation tests

process_matched_mitigated_finding appends to self.reactivated_items,
which is normally created in process_findings(). The tests drive the
method directly, so set the accumulator lists explicitly.

Author: valentijnscholten

dojo/importers/default_reimporter.py

@@ -801,9 +801,16 @@ def process_matched_mitigated_finding(
         existing_finding.mitigated = None
         existing_finding.is_mitigated = False
         existing_finding.mitigated_by = None
-        existing_finding.active = True
-        if self.verified is not None:
-            existing_finding.verified = self.verified
+        # A duplicate finding must stay inactive/unverified (see set_duplicate and the
+        # "Duplicate findings cannot be verified or active" form validation). Un-mitigate it
+        # but do not reactivate it, otherwise we create an invalid active/verified duplicate state.
+        if existing_finding.duplicate:
+            existing_finding.active = False
+            existing_finding.verified = False
+        else:
+            existing_finding.active = True
+            if self.verified is not None:
+                existing_finding.verified = self.verified
 
         component_name = getattr(unsaved_finding, "component_name", None)
         component_version = getattr(unsaved_finding, "component_version", None)
@@ -819,7 +826,11 @@ def process_matched_mitigated_finding(
         # don't dedupe before endpoints/locations are added, postprocessing will be done on next save (in calling method)
         existing_finding.save_no_options()
 
-        note = Notes(entry=f"Re-activated by {self.scan_type} re-upload.", author=self.user)
+        if existing_finding.duplicate:
+            note_entry = f"Un-mitigated by {self.scan_type} re-upload but kept inactive because the finding is a duplicate."
+        else:
+            note_entry = f"Re-activated by {self.scan_type} re-upload."
+        note = Notes(entry=note_entry, author=self.user)
         note.save()
         self.location_handler.record_reactivations_for_finding(existing_finding)
         existing_finding.notes.add(note)

unittests/test_importers_importer.py

@@ -932,4 +932,115 @@ def test_change_vulnerability_ids_on_reimport(self):
         # Verify only new Vulnerability_Id objects exist
         vuln_ids = list(Vulnerability_Id.objects.filter(finding=finding).values_list("vulnerability_id", flat=True))
         self.assertEqual(set(new_vulnerability_ids), set(vuln_ids))
-        finding.delete()
+
+
+class ReimportDuplicateReactivationTest(DojoTestCase):
+
+    """
+    Regression test for https://github.com/DefectDojo/django-DefectDojo/issues/14910
+
+    Reimport reactivation of a mitigated finding must not produce an invalid
+    active/verified duplicate finding state.
+    """
+
+    def setUp(self):
+        self.user, _ = User.objects.get_or_create(username="admin", is_superuser=True)
+        Development_Environment.objects.get_or_create(name="Development")
+        self.product_type, _ = Product_Type.objects.get_or_create(name="dup_reactivation_pt")
+        self.product, _ = Product.objects.get_or_create(
+            name="DupReactivationProduct",
+            description="test product",
+            prod_type=self.product_type,
+        )
+        self.engagement = Engagement.objects.create(
+            name="Dup Reactivation Engagement",
+            product=self.product,
+            target_start=timezone.now(),
+            target_end=timezone.now(),
+        )
+        self.test = self.create_test(engagement=self.engagement, scan_type=NPM_AUDIT_SCAN_TYPE, title="dup reactivation test")
+
+    def _make_finding(self, title, **kwargs):
+        return Finding.objects.create(
+            title=title,
+            test=self.test,
+            severity="High",
+            reporter=self.user,
+            **kwargs,
+        )
+
+    def test_reactivation_keeps_duplicate_inactive_and_unverified(self):
+        # Original active finding
+        original = self._make_finding("original finding", active=True, verified=True)
+        # Mitigated finding that is marked as a duplicate of the original
+        existing_duplicate = self._make_finding(
+            "duplicate finding",
+            active=False,
+            verified=False,
+            duplicate=True,
+            duplicate_finding=original,
+            is_mitigated=True,
+            mitigated=timezone.now(),
+            mitigated_by=self.user,
+        )
+        # The reimported (unsaved) finding that re-matches the duplicate, and is active/not mitigated
+        unsaved_finding = self._make_finding("duplicate finding incoming", active=True, verified=True)
+
+        reimporter = DefaultReImporter(
+            test=self.test,
+            user=self.user,
+            scan_type=NPM_AUDIT_SCAN_TYPE,
+            active=True,
+            verified=True,
+            do_not_reactivate=False,
+        )
+        # These accumulators are normally initialised inside process_findings(); set them
+        # here because the test drives process_matched_mitigated_finding() directly.
+        reimporter.new_items = []
+        reimporter.reactivated_items = []
+        reimporter.unchanged_items = []
+
+        result_finding, _ = reimporter.process_matched_mitigated_finding(unsaved_finding, existing_duplicate)
+
+        result_finding.refresh_from_db()
+        # The mitigation is cleared (the finding reappeared in the scan)...
+        self.assertFalse(result_finding.is_mitigated)
+        self.assertIsNone(result_finding.mitigated)
+        # ...but a duplicate must never become active or verified (issue #14910)
+        self.assertTrue(result_finding.duplicate)
+        self.assertFalse(result_finding.active)
+        self.assertFalse(result_finding.verified)
+
+    def test_reactivation_of_non_duplicate_still_activates(self):
+        # A regular mitigated finding (not a duplicate) must still reactivate as before
+        existing = self._make_finding(
+            "regular finding",
+            active=False,
+            verified=False,
+            is_mitigated=True,
+            mitigated=timezone.now(),
+            mitigated_by=self.user,
+        )
+        unsaved_finding = self._make_finding("regular finding incoming", active=True, verified=True)
+
+        reimporter = DefaultReImporter(
+            test=self.test,
+            user=self.user,
+            scan_type=NPM_AUDIT_SCAN_TYPE,
+            active=True,
+            verified=True,
+            do_not_reactivate=False,
+        )
+        # These accumulators are normally initialised inside process_findings(); set them
+        # here because the test drives process_matched_mitigated_finding() directly.
+        reimporter.new_items = []
+        reimporter.reactivated_items = []
+        reimporter.unchanged_items = []
+
+        result_finding, _ = reimporter.process_matched_mitigated_finding(unsaved_finding, existing)
+
+        result_finding.refresh_from_db()
+        self.assertFalse(result_finding.is_mitigated)
+        self.assertIsNone(result_finding.mitigated)
+        self.assertTrue(result_finding.active)
+        self.assertTrue(result_finding.verified)