Skip to content

Commit f9ca488

Browse files
fix(finding): compute cwe hash from finding_cwe_set directly, not the cached property
get_cwes() used the cwes @cached_property for the saved path; if finding.cwes was accessed before the Finding_CWE rows were written (serializer/signal/log), it cached an empty/partial set and the hash then used that stale value -> nondeterministic hash_code (flaky dedup under parallel test runs). Mirror get_vulnerability_ids: read the finding_cwe_set reverse relation directly (uncached, prefetch-honoring), falling back to the unsaved values before save. save_cwes stores the primary cwe as a row too, so finding_cwe_set carries the full set.
1 parent eaf697a commit f9ca488

2 files changed

Lines changed: 31 additions & 14 deletions

File tree

dojo/finding/models.py

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -837,21 +837,27 @@ def _get_saved_vulnerability_ids(finding) -> str:
837837

838838
return _get_saved_vulnerability_ids(self) or _get_unsaved_vulnerability_ids(self)
839839

840-
# Get CWEs (canonical CWE-<n> labels) to use for hash_code computation
840+
# Get CWEs (canonical CWE-<n> labels) to use for hash_code computation.
841+
# Mirrors get_vulnerability_ids: the saved path reads the reverse relation directly (never the
842+
# cached `cwes` property, which could be stale if accessed before the rows were written), and
843+
# falls back to the unsaved values before the finding is saved. save_cwes persists the primary
844+
# self.cwe as a Finding_CWE row too, so finding_cwe_set already carries the full set.
841845
def get_cwes(self):
842-
# The extra CWE rows (finding_cwe_set) are written *after* the finding is saved during
843-
# import, but unsaved_cwes carries them at hash time. Unlike vulnerability_ids there is a
844-
# primary scalar (self.cwe) that is always set, so self.cwes is non-empty even before the
845-
# extra rows exist — a plain "saved or unsaved" fallback would miss the extras. Prefer
846-
# unsaved_cwes whenever it is present so the pre-save and post-save hashes agree.
847-
if self.unsaved_cwes:
848-
labels = finding_cwe_labels(self.cwe, self.unsaved_cwes)
849-
elif self.id is not None:
850-
labels = self.cwes
851-
else:
852-
labels = finding_cwe_labels(self.cwe, None)
853-
# sort for a deterministic hash regardless of row/label ordering
854-
return "".join(sorted(labels))
846+
847+
def _get_unsaved_cwes(finding) -> str:
848+
# primary self.cwe + any unsaved extra CWEs, canonical CWE-<n>, deduplicated
849+
labels = finding_cwe_labels(finding.cwe, finding.unsaved_cwes)
850+
return "".join(sorted(labels))
851+
852+
def _get_saved_cwes(finding) -> str:
853+
if finding.id is not None:
854+
# Use the reverse relation (finding_cwe_set) rather than the cached `cwes` property
855+
# so prefetch is honored and a stale cached value can never corrupt the hash.
856+
labels = [row.cwe for row in finding.finding_cwe_set.all()]
857+
return "".join(sorted(labels))
858+
return ""
859+
860+
return _get_saved_cwes(self) or _get_unsaved_cwes(self)
855861

856862
# Get locations/endpoints to use for hash_code computation
857863
def get_locations(self):

unittests/test_finding_cwe.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,17 @@ def test_get_cwes_empty_when_no_cwe(self):
158158
save_cwes(self.finding)
159159
self.assertEqual(Finding.objects.get(id=2).get_cwes(), "")
160160

161+
def test_get_cwes_ignores_stale_cwes_cached_property(self):
162+
# Regression: get_cwes() must read finding_cwe_set directly, not the cwes @cached_property.
163+
# If cwes was cached (e.g. accessed before the rows were written) get_cwes must still be
164+
# correct, otherwise the hash_code is nondeterministic.
165+
self.finding.cwe = 79
166+
self.finding.unsaved_cwes = [89]
167+
save_cwes(self.finding)
168+
f = Finding.objects.get(id=2)
169+
f.__dict__["cwes"] = [] # poison the cached_property with a stale/empty value
170+
self.assertEqual(f.get_cwes(), "".join(sorted(["CWE-79", "CWE-89"])))
171+
161172
def test_compute_hash_code_uses_cwe_set(self):
162173
self.finding.cwe = 79
163174
scanner = self.finding.test.test_type.name

0 commit comments

Comments
 (0)