fix(dedupe): choose stable original by sorting creation order, keep dedupe comparisons id-based

The batch-stamping approach (_dedupe_batch_min_id + content-key comparisons
inside _is_candidate_older) made the "older" relation depend on which dedupe
batch evaluated it. That broke global antisymmetry: for a colliding pair
split across two post-processing batches of one large import (>
IMPORT_REIMPORT_DEDUPE_BATCH_SIZE findings), each side could consider the
other "older", and two concurrently running dedupe batches could mark the
pair as duplicates of each other. It also re-picked winners by content when
re-running dedupe over pre-existing findings (the dedupe management command),
flipping long-established originals.

Instead, sort a report's findings by the stable content key BEFORE creating
them - in DefaultImporter now, mirroring what DefaultReImporter already does -
and keep every dedupe comparison a pure lowest-id-wins comparison:

- within one import, id order now equals content order, so the surviving
  original is content-canonical and reproducible across re-scans regardless
  of scanner export order (in every execution mode, for any report size)
- pre-existing findings always have smaller ids, so an established original
  never flips - including when the dedupe command re-processes old data
- the "older" relation stays globally antisymmetric, so concurrent dedupe
  batches can never mark two findings as duplicates of each other

Reimport match candidate ordering returns to id order (already stable across
scans); the reimporter keeps the up-front preparation and global content sort
so intra-report duplicate resolution stays order-independent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: Maffooch

dojo/finding/deduplication.py

@@ -1,5 +1,6 @@
 import logging
 from collections.abc import Iterator
+from operator import attrgetter
 
 import hyperlink
 from django.conf import settings
@@ -545,14 +546,20 @@ def find_candidates_for_reimport_legacy(test, findings, service=None):
 
 def deduplication_ordering_key(finding):
     """
-    Stable, content-derived sort key for choosing the canonical "original"
-    among findings that collide on the deduplication key.
+    Stable, content-derived sort key used by the importers to decide the order
+    findings from one report are created (and therefore get their ids) in.
 
-    Independent of parser emission order and DB id, so the winner is
-    reproducible across re-scans regardless of the order the scanner exports
-    its findings in. id is the final tiebreak and is only reached when two
-    findings are identical across every content field in the key (in which
-    case the choice is immaterial because the findings are interchangeable).
+    Deduplication itself always picks the lowest-id finding as the canonical
+    "original". Because the importers sort a report's findings by this key
+    BEFORE saving them, "lowest id" within one import equals "canonical by
+    content", so the winner among findings that collide on the deduplication
+    key is reproducible across re-scans regardless of the order the scanner
+    exports its findings in. Findings from earlier imports always have smaller
+    ids, so an already-established original never flips.
+
+    id is the final tiebreak and is only reached when two findings are
+    identical across every content field in the key (in which case the choice
+    is immaterial because the findings are interchangeable).
 
     All fields referenced here are part of ``Finding.DEDUPLICATION_FIELDS``, so
     building this key never triggers extra database queries during dedupe.
@@ -567,57 +574,18 @@ def deduplication_ordering_key(finding):
     )
 
 
-def _candidate_sort_key(candidate, batch_min_id):
-    """
-    Order candidates so the first "older" one encountered is the canonical original.
-
-    Findings that predate the current import batch (id < batch_min_id, or when no
-    batch context is available) are ranked first and ordered by id, preserving the
-    historical "pre-existing finding stays the original" behaviour. Findings created
-    within the current batch are ranked after pre-existing ones and ordered by the
-    stable content key so the winner does not depend on parser emission order.
-
-    ``batch_min_id`` is an optional marker; when it is not an integer (absent) or the
-    candidate has no integer id yet (unsaved/preview), the candidate is treated as
-    pre-existing and ordered by id, so arithmetic is only ever done on real ids.
-    """
-    candidate_id = candidate.id if isinstance(candidate.id, int) else None
-    if not isinstance(batch_min_id, int) or candidate_id is None or candidate_id < batch_min_id:
-        return (0, candidate_id or 0)
-    return (1, deduplication_ordering_key(candidate))
-
-
-def _prepare_batch_ordering(findings):
-    """
-    Stamp every finding in a dedup batch with the batch's minimum id and return the
-    batch sorted by the stable content key.
-
-    The stamped ``_dedupe_batch_min_id`` lets ``_is_candidate_older`` tell findings
-    created within this batch apart from pre-existing candidates loaded from the DB
-    (which always have smaller ids), so pre-existing findings keep their id-based
-    ordering while intra-batch ties are broken deterministically by content.
-    """
-    batch_ids = [f.id for f in findings if f.id is not None]
-    batch_min_id = min(batch_ids) if batch_ids else None
-    for f in findings:
-        f._dedupe_batch_min_id = batch_min_id
-    return sorted(findings, key=deduplication_ordering_key)
-
-
 def _is_candidate_older(new_finding, candidate):
     # Unsaved findings (e.g. preview mode) have no PK — all DB candidates are older by definition
     if new_finding.pk is None:
         return True
-    # Findings created within the current import batch all have ids >= batch_min_id
-    # (auto-increment guarantees pre-existing findings have smaller ids). For those
-    # we break ties by the stable content key so the "original" is deterministic
-    # regardless of the order the scanner emitted the findings. Pre-existing
-    # candidates keep the id ordering so an already-existing finding stays original.
-    batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
-    if batch_min_id is not None and candidate.id is not None and candidate.id >= batch_min_id:
-        is_older = deduplication_ordering_key(candidate) < deduplication_ordering_key(new_finding)
-    else:
-        is_older = candidate.id < new_finding.id
+    # Ensure the newer finding is marked as duplicate of the older finding.
+    # This comparison must stay a pure id comparison: it is evaluated
+    # independently from concurrent dedupe batches (and from the `dedupe`
+    # management command over pre-existing findings), so it has to be globally
+    # antisymmetric — for any pair, exactly one side may see the other as
+    # "older". Content-stable winner selection is achieved by the importers
+    # creating a report's findings in deduplication_ordering_key order instead.
+    is_older = candidate.id < new_finding.id
     if not is_older:
         deduplicationLogger.debug(f"candidate is newer than or equal to new finding: {new_finding.id} and candidate {candidate.id}")
     return is_older
@@ -626,11 +594,7 @@ def _is_candidate_older(new_finding, candidate):
 def get_matches_from_hash_candidates(new_finding, candidates_by_hash) -> Iterator[Finding]:
     if new_finding.hash_code is None:
         return
-    batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
-    possible_matches = sorted(
-        candidates_by_hash.get(new_finding.hash_code, []),
-        key=lambda c: _candidate_sort_key(c, batch_min_id),
-    )
+    possible_matches = candidates_by_hash.get(new_finding.hash_code, [])
     deduplicationLogger.debug(f"Finding {new_finding.id}: Found {len(possible_matches)} findings with same hash_code, ids={[(c.id, c.hash_code) for c in possible_matches]}")
 
     for candidate in possible_matches:
@@ -647,11 +611,7 @@ def get_matches_from_unique_id_candidates(new_finding, candidates_by_uid) -> Ite
     if new_finding.unique_id_from_tool is None:
         return
 
-    batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
-    possible_matches = sorted(
-        candidates_by_uid.get(new_finding.unique_id_from_tool, []),
-        key=lambda c: _candidate_sort_key(c, batch_min_id),
-    )
+    possible_matches = candidates_by_uid.get(new_finding.unique_id_from_tool, [])
     deduplicationLogger.debug(f"Finding {new_finding.id}: Found {len(possible_matches)} findings with same unique_id_from_tool, ids={[(c.id, c.unique_id_from_tool) for c in possible_matches]}")
     for candidate in possible_matches:
         if not _is_candidate_older(new_finding, candidate):
@@ -671,10 +631,9 @@ def get_matches_from_uid_or_hash_candidates(new_finding, candidates_by_uid, cand
     combined_by_id = {c.id: c for c in uid_list}
     for c in hash_list:
         combined_by_id.setdefault(c.id, c)
-    batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
-    ordered_candidates = sorted(combined_by_id.values(), key=lambda c: _candidate_sort_key(c, batch_min_id))
-    deduplicationLogger.debug("Finding %s: UID_OR_HASH: combined candidate ids (ordered)=%s", new_finding.id, [c.id for c in ordered_candidates])
-    for candidate in ordered_candidates:
+    deduplicationLogger.debug("Finding %s: UID_OR_HASH: combined candidate ids (sorted)=%s", new_finding.id, sorted(combined_by_id.keys()))
+    for candidate_id in sorted(combined_by_id.keys()):
+        candidate = combined_by_id[candidate_id]
         if not _is_candidate_older(new_finding, candidate):
             continue
         if is_deduplication_on_engagement_mismatch(new_finding, candidate):
@@ -701,14 +660,6 @@ def get_matches_from_legacy_candidates(new_finding, candidates_by_title, candida
     if getattr(new_finding, "cwe", 0):
         candidates.extend(candidates_by_cwe.get(new_finding.cwe, []))
 
-    # De-duplicate (a candidate can match on both title and CWE) and walk in
-    # canonical order so the first "older" candidate is the stable original.
-    batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
-    candidates = sorted(
-        {c.id: c for c in candidates}.values(),
-        key=lambda c: _candidate_sort_key(c, batch_min_id),
-    )
-
     for candidate in candidates:
         if not _is_candidate_older(new_finding, candidate):
             continue
@@ -898,9 +849,9 @@ def match_batch_of_findings(findings):
     enabled = System_Settings.objects.get().enable_deduplication
     if not enabled:
         return []
-    # Only stamp/sort for saved findings; unsaved findings have no id or batch context
+    # Only sort by id for saved findings; unsaved findings have no id
     if findings[0].pk is not None:
-        findings = _prepare_batch_ordering(findings)
+        findings = sorted(findings, key=attrgetter("id"))
     test = findings[0].test
     dedup_alg = test.deduplication_algorithm
     if dedup_alg == settings.DEDUPE_ALGO_HASH_CODE:
@@ -1019,10 +970,8 @@ def dedupe_batch_of_findings(findings, *args, **kwargs):
     enabled = System_Settings.objects.get().enable_deduplication
 
     if enabled:
-        # Stamp each finding with the batch's minimum id and sort by the stable
-        # content key so deduplication is deterministic and independent of the
-        # order the scanner emitted its findings (see _is_candidate_older).
-        findings = _prepare_batch_ordering(findings)
+        # sort findings by id to ensure deduplication is deterministic/reproducible
+        findings = sorted(findings, key=attrgetter("id"))
 
         test = findings[0].test
         dedup_alg = test.deduplication_algorithm

dojo/importers/default_importer.py

@@ -7,6 +7,7 @@
 
 from dojo.celery_dispatch import dojo_dispatch_task
 from dojo.finding import helper as finding_helper
+from dojo.finding.deduplication import deduplication_ordering_key
 from dojo.importers.base_importer import BaseImporter, Parser
 from dojo.importers.base_location_manager import LocationHandler
 from dojo.importers.options import ImporterOptions
@@ -203,54 +204,66 @@ def _process_findings_internal(
         logger.debug("starting import of %i parsed findings.", len(parsed_findings) if parsed_findings else 0)
         group_names_to_findings_dict = {}
 
-        # Pre-sanitize and filter by minimum severity
+        # Pre-sanitize and filter by minimum severity. Findings are also fully
+        # prepared here (fields/overrides/hash_code) so they can be sorted by a
+        # stable content key below.
         cleaned_findings = []
         for raw_finding in parsed_findings or []:
             sanitized = self.sanitize_severity(raw_finding)
             if Finding.SEVERITIES[sanitized.severity] > Finding.SEVERITIES[self.minimum_severity]:
                 logger.debug("skipping finding due to minimum severity filter (finding=%s severity=%s min=%s)", sanitized.title, sanitized.severity, self.minimum_severity)
                 continue
-            cleaned_findings.append(sanitized)
-
-        for idx, unsaved_finding in enumerate(cleaned_findings):
-            is_final_finding = idx == len(cleaned_findings) - 1
-
             # Some parsers provide "mitigated" field but do not set timezone (because they are probably not available in the report)
             # Finding.mitigated is DateTimeField and it requires timezone
-            if unsaved_finding.mitigated and not unsaved_finding.mitigated.tzinfo:
-                unsaved_finding.mitigated = unsaved_finding.mitigated.replace(tzinfo=self.now.tzinfo)
+            if sanitized.mitigated and not sanitized.mitigated.tzinfo:
+                sanitized.mitigated = sanitized.mitigated.replace(tzinfo=self.now.tzinfo)
             # Set some explicit fields on the finding
-            unsaved_finding.test = self.test
-            unsaved_finding.reporter = self.user
-            unsaved_finding.last_reviewed_by = self.user
-            unsaved_finding.last_reviewed = self.now
-            logger.debug("process_parsed_finding: unique_id_from_tool: %s, hash_code: %s, active from report: %s, verified from report: %s", unsaved_finding.unique_id_from_tool, unsaved_finding.hash_code, unsaved_finding.active, unsaved_finding.verified)
-            # indicates an override. Otherwise, do not change the value of unsaved_finding.active
+            sanitized.test = self.test
+            sanitized.reporter = self.user
+            sanitized.last_reviewed_by = self.user
+            sanitized.last_reviewed = self.now
+            logger.debug("process_parsed_finding: unique_id_from_tool: %s, hash_code: %s, active from report: %s, verified from report: %s", sanitized.unique_id_from_tool, sanitized.hash_code, sanitized.active, sanitized.verified)
+            # indicates an override. Otherwise, do not change the value of sanitized.active
             if self.active is not None:
-                unsaved_finding.active = self.active
+                sanitized.active = self.active
             # indicates an override. Otherwise, do not change the value of verified
             if self.verified is not None:
-                unsaved_finding.verified = self.verified
+                sanitized.verified = self.verified
             # scan_date was provided, override value from parser
             if self.scan_date_override:
-                unsaved_finding.date = self.scan_date.date()
+                sanitized.date = self.scan_date.date()
             if self.service is not None:
-                unsaved_finding.service = self.service
+                sanitized.service = self.service
 
             # Parsers shouldn't use the tags field, and use unsaved_tags instead.
             # Merge any tags set by parser into unsaved_tags
-            tags_from_parser = unsaved_finding.tags if isinstance(unsaved_finding.tags, list) else []
-            unsaved_tags_from_parser = unsaved_finding.unsaved_tags if isinstance(unsaved_finding.unsaved_tags, list) else []
+            tags_from_parser = sanitized.tags if isinstance(sanitized.tags, list) else []
+            unsaved_tags_from_parser = sanitized.unsaved_tags if isinstance(sanitized.unsaved_tags, list) else []
             merged_tags = unsaved_tags_from_parser + tags_from_parser
             if merged_tags:
-                unsaved_finding.unsaved_tags = merged_tags
-            unsaved_finding.tags = None
-            finding = self.process_cve(unsaved_finding)
+                sanitized.unsaved_tags = merged_tags
+            sanitized.tags = None
+            prepared = self.process_cve(sanitized)
             # Calculate hash_code before saving based on unsaved_endpoints/unsaved_locations and unsaved_vulnerability_ids
-            finding.set_hash_code(True)
+            prepared.set_hash_code(True)
+            cleaned_findings.append(prepared)
+
+        # Create findings in stable content-key order so their ids follow content, not
+        # the scanner's export order. Deduplication picks the lowest id among findings
+        # that collide on the dedupe key, so this makes the surviving "original" for
+        # intra-report collisions reproducible across re-scans regardless of the order
+        # the scanner emitted its findings. Unsaved findings have no id, so the id
+        # tiebreak is inert here and byte-identical findings keep their relative
+        # (immaterial) order via Python's stable sort.
+        cleaned_findings.sort(key=deduplication_ordering_key)
+
+        for idx, finding in enumerate(cleaned_findings):
+            is_final_finding = idx == len(cleaned_findings) - 1
 
-            # postprocessing will be done after processing related fields like locations, vulnerability ids, etc.
-            unsaved_finding.save_no_options()
+            # Findings are already prepared (fields/overrides/hash_code) and globally
+            # sorted above. postprocessing will be done after processing related
+            # fields like locations, vulnerability ids, etc.
+            finding.save_no_options()
 
             # Determine how the finding should be grouped
             finding_will_be_grouped = self.process_finding_groups(
@@ -269,8 +282,8 @@ def _process_findings_internal(
                 findings_with_parser_tags.append((finding, [cleaned_tags]))
             # Process any files
             self.process_files(finding)
-            # Process vulnerability IDs
-            finding = self.store_vulnerability_ids(finding)
+            # Process vulnerability IDs (mutates the finding in place)
+            self.store_vulnerability_ids(finding)
             # Categorize this finding as a new one
             new_findings.append(finding)
             # all data is already saved on the finding, we only need to trigger post processing in batches

dojo/importers/default_reimporter.py

@@ -663,13 +663,13 @@ def match_finding_to_candidate_reimport(
             if candidates_by_hash is None or unsaved_finding.hash_code is None:
                 return []
             matches = candidates_by_hash.get(unsaved_finding.hash_code, [])
-            return sorted(matches, key=deduplication_ordering_key)
+            return sorted(matches, key=lambda f: f.id)
 
         if self.deduplication_algorithm == "unique_id_from_tool":
             if candidates_by_uid is None or unsaved_finding.unique_id_from_tool is None:
                 return []
             matches = candidates_by_uid.get(unsaved_finding.unique_id_from_tool, [])
-            return sorted(matches, key=deduplication_ordering_key)
+            return sorted(matches, key=lambda f: f.id)
 
         if self.deduplication_algorithm == "unique_id_from_tool_or_hash_code":
             if candidates_by_hash is None and candidates_by_uid is None:
@@ -692,14 +692,14 @@ def match_finding_to_candidate_reimport(
                     matches_by_id[match.id] = match
 
             matches = list(matches_by_id.values())
-            return sorted(matches, key=deduplication_ordering_key)
+            return sorted(matches, key=lambda f: f.id)
 
         if self.deduplication_algorithm == "legacy":
             if candidates_by_key is None or not unsaved_finding.title:
                 return []
             key = (unsaved_finding.title.lower(), unsaved_finding.severity)
             matches = candidates_by_key.get(key, [])
-            return sorted(matches, key=deduplication_ordering_key)
+            return sorted(matches, key=lambda f: f.id)
 
         logger.error(f'Internal error: unexpected deduplication_algorithm: "{self.deduplication_algorithm}"')
         return []