fix(dedupe): stable content-based tiebreaker for import/reimport ordering#15222
Open
Maffooch wants to merge 2 commits into
Open
fix(dedupe): stable content-based tiebreaker for import/reimport ordering#15222Maffooch wants to merge 2 commits into
Maffooch wants to merge 2 commits into
Conversation
…ring 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>
…ce test Two .fpr fixtures contain findings that share a unique_id_from_tool but differ in content, in opposite document order. Under unique_id_from_tool dedup the surviving original must be the same finding regardless of the scanner's export order (asserted by test_fortify_same_uid_different_content_original_is_order_independent). Fails without the ordering fix (surviving finding flips with file order), passes with it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[sc-13656]
Problem
Deduplication chose the "original" among findings that collide on the dedupe key by lowest DB id — i.e. whichever the scanner listed first in the file. Scanners don't guarantee a stable finding order between scans, so the original/duplicate assignment flips back and forth between scans even though the underlying vulnerabilities never change. Reported via a customer support ticket; observed with Fortify but not tool-specific (any parser without a stable export order can trigger it). Discovered via reimport, but the same class of bug exists on the import/deduplication path.
Fix
deduplication_ordering_key()— a stable key derived from finding content (hash_code, unique_id_from_tool, file_path, line, title, thenidas final tiebreak)._is_candidate_older()batch-aware: findings that predate the current import batch keep id ordering (so an already-established original never flips), while ties within a single import/report are broken by the content key instead of parser emission order.Backward compatibility
Tests
.fprfiles whose findings share aunique_id_from_toolbut differ in content, in opposite document order; the surviving original must be identical across both orders. Fails on the pre-fix code (surviving finding flips), passes with the fix.unique_id_from_tooldedup viaoverride_settings(open source defaults Fortify to legacy, where different-content findings would not collide at all). This mirrors the realistic customer configuration.Related
Pairs with the corresponding Dojo Pro change, which mirrors this fix in Pro's dedupe engine and depends on the batch-aware
_is_candidate_olderand the exported ordering helpers here. They should merge together.🤖 Generated with Claude Code