fix(dedupe): order locations/endpoints inside hash_code, and catch set-order leaks in parsers

[valentijnscholten]

Summary

#15481 and #15483 each fixed one parser that joined an unordered set into a finding. This PR attacks the same class from the other side: it orders the ingredient inside hash_code itself, fixes the one place where that ordering was missing, and adds a check that finds the parser-side variant automatically instead of one bug report at a time.

Ordering the locations does not replace those two PRs — their instability travelled through description, which the hash has to consume verbatim. But it is why several parsers that build vulnerability ids or locations from a set never needed a fix at all (dojo/tools/dependency_track/parser.py is one), and there was a real gap.

hash_code: the locations ingredient is now ordered on both code paths

The locations/endpoints ingredient is computed from two different branches of Finding.get_locations(): from the parser's output before the finding is saved — this is the value stored on import (default_importer.py) — and from the saved rows on every later recomputation (manage.py dedupe, false positive history, reimport). The saved branch sorted, the legacy unsaved branch did not, even though its comment claimed it did:

# deduplicate (usually done upon saving finding) and sort endpoints
return "".join(dict.fromkeys(endpoint_str_list))   # <- no sorted()

So for scan types that hash endpoints (Qualys Scan, ffuf, Dirsearch, httpx, Nettacker, Legitify, …) the scanner's emission order became part of the finding's identity, and the stored hash_code disagreed with whatever a recomputation produced. Both branches sort now.

The importer also hashed locations before cleaning them, while reimport cleans first. Cleaning rewrites the canonical string (leading / off the path, leading ? off the query, port coerced to int), so the two could hash the same report differently. The importer now cleans first, like reimport.

Reach: DD_V3_FEATURE_LOCATIONS=False only — the locations path already sorted on both sides.

manage.py dedupe crashed for saved findings with locations

The saved-locations branch called get_location_value() on a Location row, which has no such method (it is defined on AbstractLocation/URL; the row stores the same string in location_value). Recomputing the hash_code of any already-saved finding with URL locations therefore raised AttributeError, with the default DD_V3_FEATURE_LOCATIONS=True. It now reads the stored location_value, which is written from get_location_value() at creation, so the recomputed value matches what import computes. This was only reachable from a recomputation, which is why nothing caught it — import and reimport both hash unsaved findings.

Parsers where a set's order still reached the finding

Sorted at the point the set is consumed:

• blackduck — file paths joined into file_path, and the order the findings are produced in
• blackduck_binary_analysis — the order the findings are produced in
• sarif — the finding's tags
• legitify — the URLs listed under references
• dependency_track — the vulnerability ids: the first id becomes the finding's primary id (its cve), so PYTHONHASHSEED decided which identifier a finding was filed under. The report's own vulnId is now always first, aliases follow sorted.

None of these values is a hash_code field for its scan type, so no identity changes there; they just stop reshuffling on every import. dependency_track may now show a different (and stable) CVE for findings that carry aliases — the vulnId the title has always used.

A guard instead of one PR per parser

unittests/test_parsers.py gains an AST check over dojo/tools: it reports where the iteration order of a set decides what a parser emits — join() over a set, list()/tuple() of a set, iterating or comprehending over one, including through a local variable and through set algebra. It catches the exact shapes from both earlier PRs, including govulncheck's (assign the set to a variable first, join it later), which a regex over the source does not. A deliberate exception is annotated with # set-order-ok: <why> — there is one, in blackduck, where the loop only sums lengths. The checker has its own test pinning what it must catch and what it must not (lists, dicts, sorted(...), membership tests).

Each of the three hash_code fixes has a dedicated regression test in unittests/test_hash_code_location_ordering.py, which runs against both the locations and the legacy endpoints ingredient; every one of them was confirmed to fail with its fix reverted.

Release notes

docs/content/releases/os_upgrading/3.2.md calls out the identity change: multi-endpoint findings on installs running DD_V3_FEATURE_LOCATIONS=False. A single endpoint has one possible ordering, so those hashes do not move, and installs on the default True are unaffected. For the findings this does change there is no stable prior identity to preserve — the stored value was whatever order the report arrived in, and it never matched a recomputation.

Targeting dev rather than bugfix: these are bug fixes, but one of them changes hash_code, so it ships with a minor release.

[dryrunsecurity]

This pull request contains a critical finding where the sensitive file 'dojo/finding/models.py' was modified by an author not included in the allowed authors list.

🔴 Configured Sensitive Codepath Modified by Non-Allowed Author in dojo/finding/models.py (drs_399a8781)

Vulnerability	Configured Sensitive Codepath Modified by Non-Allowed Author
Description	File 'dojo/finding/models.py' matches configured sensitive codepath pattern 'dojo/finding/*.py' and was modified by '' (commit 026fb9b) who is not in the allowed authors list.

We've notified @mtesauro.

---

Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.