Skip to content

Commit ae8c325

Browse files
Maffoochclaude
andcommitted
fix(dedupe): stable content-based tiebreaker for import/reimport ordering
Deduplication picked the "original" among findings that collide on the dedupe key by lowest DB id, i.e. whichever the scanner emitted first. When a tool reorders content-duplicate findings between scans, the original/duplicate assignment flipped even though the underlying vulnerabilities were unchanged. Reported via a customer support ticket; observed with Fortify but not tool-specific (any parser without a stable export order can trigger it). Introduce deduplication_ordering_key() (hash_code, unique_id_from_tool, file_path, line, title, then id) and order the batch dedupe and reimport matching by it. Findings created before the current import batch keep id ordering (via a batch-minimum-id marker), so an already-established original never flips; only the previously-arbitrary intra-batch / intra-report winner becomes deterministic and independent of the scanner's export order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent af3716f commit ae8c325

3 files changed

Lines changed: 214 additions & 34 deletions

File tree

dojo/finding/deduplication.py

Lines changed: 99 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import logging
22
from collections.abc import Iterator
3-
from operator import attrgetter
43

54
import hyperlink
65
from django.conf import settings
@@ -544,12 +543,81 @@ def find_candidates_for_reimport_legacy(test, findings, service=None):
544543
return existing_by_key
545544

546545

546+
def deduplication_ordering_key(finding):
547+
"""
548+
Stable, content-derived sort key for choosing the canonical "original"
549+
among findings that collide on the deduplication key.
550+
551+
Independent of parser emission order and DB id, so the winner is
552+
reproducible across re-scans regardless of the order the scanner exports
553+
its findings in. id is the final tiebreak and is only reached when two
554+
findings are identical across every content field in the key (in which
555+
case the choice is immaterial because the findings are interchangeable).
556+
557+
All fields referenced here are part of ``Finding.DEDUPLICATION_FIELDS``, so
558+
building this key never triggers extra database queries during dedupe.
559+
"""
560+
return (
561+
finding.hash_code or "",
562+
finding.unique_id_from_tool or "",
563+
finding.file_path or "",
564+
finding.line if finding.line is not None else -1,
565+
finding.title or "",
566+
finding.id or 0,
567+
)
568+
569+
570+
def _candidate_sort_key(candidate, batch_min_id):
571+
"""
572+
Order candidates so the first "older" one encountered is the canonical original.
573+
574+
Findings that predate the current import batch (id < batch_min_id, or when no
575+
batch context is available) are ranked first and ordered by id, preserving the
576+
historical "pre-existing finding stays the original" behaviour. Findings created
577+
within the current batch are ranked after pre-existing ones and ordered by the
578+
stable content key so the winner does not depend on parser emission order.
579+
580+
``batch_min_id`` is an optional marker; when it is not an integer (absent) or the
581+
candidate has no integer id yet (unsaved/preview), the candidate is treated as
582+
pre-existing and ordered by id, so arithmetic is only ever done on real ids.
583+
"""
584+
candidate_id = candidate.id if isinstance(candidate.id, int) else None
585+
if not isinstance(batch_min_id, int) or candidate_id is None or candidate_id < batch_min_id:
586+
return (0, candidate_id or 0)
587+
return (1, deduplication_ordering_key(candidate))
588+
589+
590+
def _prepare_batch_ordering(findings):
591+
"""
592+
Stamp every finding in a dedup batch with the batch's minimum id and return the
593+
batch sorted by the stable content key.
594+
595+
The stamped ``_dedupe_batch_min_id`` lets ``_is_candidate_older`` tell findings
596+
created within this batch apart from pre-existing candidates loaded from the DB
597+
(which always have smaller ids), so pre-existing findings keep their id-based
598+
ordering while intra-batch ties are broken deterministically by content.
599+
"""
600+
batch_ids = [f.id for f in findings if f.id is not None]
601+
batch_min_id = min(batch_ids) if batch_ids else None
602+
for f in findings:
603+
f._dedupe_batch_min_id = batch_min_id
604+
return sorted(findings, key=deduplication_ordering_key)
605+
606+
547607
def _is_candidate_older(new_finding, candidate):
548608
# Unsaved findings (e.g. preview mode) have no PK — all DB candidates are older by definition
549609
if new_finding.pk is None:
550610
return True
551-
# Ensure the newer finding is marked as duplicate of the older finding
552-
is_older = candidate.id < new_finding.id
611+
# Findings created within the current import batch all have ids >= batch_min_id
612+
# (auto-increment guarantees pre-existing findings have smaller ids). For those
613+
# we break ties by the stable content key so the "original" is deterministic
614+
# regardless of the order the scanner emitted the findings. Pre-existing
615+
# candidates keep the id ordering so an already-existing finding stays original.
616+
batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
617+
if batch_min_id is not None and candidate.id is not None and candidate.id >= batch_min_id:
618+
is_older = deduplication_ordering_key(candidate) < deduplication_ordering_key(new_finding)
619+
else:
620+
is_older = candidate.id < new_finding.id
553621
if not is_older:
554622
deduplicationLogger.debug(f"candidate is newer than or equal to new finding: {new_finding.id} and candidate {candidate.id}")
555623
return is_older
@@ -558,7 +626,11 @@ def _is_candidate_older(new_finding, candidate):
558626
def get_matches_from_hash_candidates(new_finding, candidates_by_hash) -> Iterator[Finding]:
559627
if new_finding.hash_code is None:
560628
return
561-
possible_matches = candidates_by_hash.get(new_finding.hash_code, [])
629+
batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
630+
possible_matches = sorted(
631+
candidates_by_hash.get(new_finding.hash_code, []),
632+
key=lambda c: _candidate_sort_key(c, batch_min_id),
633+
)
562634
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]}")
563635

564636
for candidate in possible_matches:
@@ -575,7 +647,11 @@ def get_matches_from_unique_id_candidates(new_finding, candidates_by_uid) -> Ite
575647
if new_finding.unique_id_from_tool is None:
576648
return
577649

578-
possible_matches = candidates_by_uid.get(new_finding.unique_id_from_tool, [])
650+
batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
651+
possible_matches = sorted(
652+
candidates_by_uid.get(new_finding.unique_id_from_tool, []),
653+
key=lambda c: _candidate_sort_key(c, batch_min_id),
654+
)
579655
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]}")
580656
for candidate in possible_matches:
581657
if not _is_candidate_older(new_finding, candidate):
@@ -595,9 +671,10 @@ def get_matches_from_uid_or_hash_candidates(new_finding, candidates_by_uid, cand
595671
combined_by_id = {c.id: c for c in uid_list}
596672
for c in hash_list:
597673
combined_by_id.setdefault(c.id, c)
598-
deduplicationLogger.debug("Finding %s: UID_OR_HASH: combined candidate ids (sorted)=%s", new_finding.id, sorted(combined_by_id.keys()))
599-
for candidate_id in sorted(combined_by_id.keys()):
600-
candidate = combined_by_id[candidate_id]
674+
batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
675+
ordered_candidates = sorted(combined_by_id.values(), key=lambda c: _candidate_sort_key(c, batch_min_id))
676+
deduplicationLogger.debug("Finding %s: UID_OR_HASH: combined candidate ids (ordered)=%s", new_finding.id, [c.id for c in ordered_candidates])
677+
for candidate in ordered_candidates:
601678
if not _is_candidate_older(new_finding, candidate):
602679
continue
603680
if is_deduplication_on_engagement_mismatch(new_finding, candidate):
@@ -624,6 +701,14 @@ def get_matches_from_legacy_candidates(new_finding, candidates_by_title, candida
624701
if getattr(new_finding, "cwe", 0):
625702
candidates.extend(candidates_by_cwe.get(new_finding.cwe, []))
626703

704+
# De-duplicate (a candidate can match on both title and CWE) and walk in
705+
# canonical order so the first "older" candidate is the stable original.
706+
batch_min_id = getattr(new_finding, "_dedupe_batch_min_id", None)
707+
candidates = sorted(
708+
{c.id: c for c in candidates}.values(),
709+
key=lambda c: _candidate_sort_key(c, batch_min_id),
710+
)
711+
627712
for candidate in candidates:
628713
if not _is_candidate_older(new_finding, candidate):
629714
continue
@@ -813,9 +898,9 @@ def match_batch_of_findings(findings):
813898
enabled = System_Settings.objects.get().enable_deduplication
814899
if not enabled:
815900
return []
816-
# Only sort by id for saved findings; unsaved findings have no id
901+
# Only stamp/sort for saved findings; unsaved findings have no id or batch context
817902
if findings[0].pk is not None:
818-
findings = sorted(findings, key=attrgetter("id"))
903+
findings = _prepare_batch_ordering(findings)
819904
test = findings[0].test
820905
dedup_alg = test.deduplication_algorithm
821906
if dedup_alg == settings.DEDUPE_ALGO_HASH_CODE:
@@ -934,8 +1019,10 @@ def dedupe_batch_of_findings(findings, *args, **kwargs):
9341019
enabled = System_Settings.objects.get().enable_deduplication
9351020

9361021
if enabled:
937-
# sort findings by id to ensure deduplication is deterministic/reproducible
938-
findings = sorted(findings, key=attrgetter("id"))
1022+
# Stamp each finding with the batch's minimum id and sort by the stable
1023+
# content key so deduplication is deterministic and independent of the
1024+
# order the scanner emitted its findings (see _is_candidate_older).
1025+
findings = _prepare_batch_ordering(findings)
9391026

9401027
test = findings[0].test
9411028
dedup_alg = test.deduplication_algorithm

dojo/importers/default_reimporter.py

Lines changed: 32 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import dojo.finding.helper as finding_helper
99
from dojo.celery_dispatch import dojo_dispatch_task
1010
from dojo.finding.deduplication import (
11+
deduplication_ordering_key,
1112
find_candidates_for_deduplication_hash,
1213
find_candidates_for_deduplication_uid_or_hash,
1314
find_candidates_for_deduplication_unique_id,
@@ -306,7 +307,12 @@ def _process_findings_internal(
306307
logger.debug("STEP 1: looping over findings from the reimported report and trying to match them to existing findings")
307308
deduplicationLogger.debug(f"Algorithm used for matching new findings to existing findings: {self.deduplication_algorithm}")
308309

309-
# Pre-sanitize and filter by minimum severity to avoid loop control pitfalls
310+
# Pre-sanitize and filter by minimum severity to avoid loop control pitfalls.
311+
# Findings are also fully prepared here (test/service/hash_code) so they can be
312+
# sorted by a stable content key below — this makes the "which duplicate becomes
313+
# the surviving finding" decision deterministic regardless of the order the
314+
# scanner emitted its findings (intra-report duplicates are matched to whichever
315+
# is created first; see add_new_finding_to_candidates).
310316
cleaned_findings = []
311317
for raw_finding in parsed_findings or []:
312318
sanitized = self.sanitize_severity(raw_finding)
@@ -318,8 +324,28 @@ def _process_findings_internal(
318324
self.minimum_severity,
319325
)
320326
continue
327+
# Some parsers provide "mitigated" field but do not set timezone (because they are probably not available in the report)
328+
# Finding.mitigated is DateTimeField and it requires timezone
329+
if sanitized.mitigated and not sanitized.mitigated.tzinfo:
330+
sanitized.mitigated = sanitized.mitigated.replace(tzinfo=self.now.tzinfo)
331+
# Override the test if needed
332+
if not hasattr(sanitized, "test"):
333+
sanitized.test = self.test
334+
# Set the service supplied at import time
335+
if self.service is not None:
336+
sanitized.service = self.service
337+
self.location_handler.clean_unsaved(sanitized)
338+
# Calculate the hash code to be used to identify duplicates
339+
sanitized.hash_code = self.calculate_unsaved_finding_hash_code(sanitized)
340+
deduplicationLogger.debug(f"unsaved finding's hash_code: {sanitized.hash_code}")
321341
cleaned_findings.append(sanitized)
322342

343+
# Sort by the stable content key so intra-report duplicate resolution is
344+
# independent of the scanner's export order. Unsaved findings have no id, so
345+
# the id tiebreak is inert here and byte-identical findings keep their relative
346+
# (immaterial) order via Python's stable sort.
347+
cleaned_findings.sort(key=deduplication_ordering_key)
348+
323349
# Each entry carries the finding's own push_to_jira flag: grouped findings are pushed to
324350
# JIRA as a group, so their individual push is suppressed while ungrouped findings in the
325351
# same batch must still be pushed. The batch is partitioned by flag at dispatch time
@@ -346,22 +372,7 @@ def _process_findings_internal(
346372

347373
logger.debug(f"Processing reimport batch {batch_start}-{batch_end} of {len(cleaned_findings)} findings")
348374

349-
# Prepare findings in batch: set test, service, calculate hash codes
350-
for unsaved_finding in unsaved_findings_batch:
351-
# Some parsers provide "mitigated" field but do not set timezone (because they are probably not available in the report)
352-
# Finding.mitigated is DateTimeField and it requires timezone
353-
if unsaved_finding.mitigated and not unsaved_finding.mitigated.tzinfo:
354-
unsaved_finding.mitigated = unsaved_finding.mitigated.replace(tzinfo=self.now.tzinfo)
355-
# Override the test if needed
356-
if not hasattr(unsaved_finding, "test"):
357-
unsaved_finding.test = self.test
358-
# Set the service supplied at import time
359-
if self.service is not None:
360-
unsaved_finding.service = self.service
361-
self.location_handler.clean_unsaved(unsaved_finding)
362-
# Calculate the hash code to be used to identify duplicates
363-
unsaved_finding.hash_code = self.calculate_unsaved_finding_hash_code(unsaved_finding)
364-
deduplicationLogger.debug(f"unsaved finding's hash_code: {unsaved_finding.hash_code}")
375+
# Findings are already prepared (test/service/hash_code) and globally sorted above.
365376

366377
# Fetch all candidates for this batch at once (batch candidate finding)
367378
candidates_by_hash, candidates_by_uid, candidates_by_key = self.get_reimport_match_candidates_for_batch(
@@ -652,13 +663,13 @@ def match_finding_to_candidate_reimport(
652663
if candidates_by_hash is None or unsaved_finding.hash_code is None:
653664
return []
654665
matches = candidates_by_hash.get(unsaved_finding.hash_code, [])
655-
return sorted(matches, key=lambda f: f.id)
666+
return sorted(matches, key=deduplication_ordering_key)
656667

657668
if self.deduplication_algorithm == "unique_id_from_tool":
658669
if candidates_by_uid is None or unsaved_finding.unique_id_from_tool is None:
659670
return []
660671
matches = candidates_by_uid.get(unsaved_finding.unique_id_from_tool, [])
661-
return sorted(matches, key=lambda f: f.id)
672+
return sorted(matches, key=deduplication_ordering_key)
662673

663674
if self.deduplication_algorithm == "unique_id_from_tool_or_hash_code":
664675
if candidates_by_hash is None and candidates_by_uid is None:
@@ -681,14 +692,14 @@ def match_finding_to_candidate_reimport(
681692
matches_by_id[match.id] = match
682693

683694
matches = list(matches_by_id.values())
684-
return sorted(matches, key=lambda f: f.id)
695+
return sorted(matches, key=deduplication_ordering_key)
685696

686697
if self.deduplication_algorithm == "legacy":
687698
if candidates_by_key is None or not unsaved_finding.title:
688699
return []
689700
key = (unsaved_finding.title.lower(), unsaved_finding.severity)
690701
matches = candidates_by_key.get(key, [])
691-
return sorted(matches, key=lambda f: f.id)
702+
return sorted(matches, key=deduplication_ordering_key)
692703

693704
logger.error(f'Internal error: unexpected deduplication_algorithm: "{self.deduplication_algorithm}"')
694705
return []

0 commit comments

Comments
 (0)