Skip to content

Commit a42f9c7

Browse files
valentijnscholtenMaffooch
authored andcommitted
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 b2e7f28 commit a42f9c7

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
@@ -848,21 +848,27 @@ def _get_saved_vulnerability_ids(finding) -> str:
848848

849849
return _get_saved_vulnerability_ids(self) or _get_unsaved_vulnerability_ids(self)
850850

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

867873
# Get locations/endpoints to use for hash_code computation
868874
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)