finding api: fix hash_code for vulnerability_ids (#13304)

Author: valentijnscholten

dojo/api_v2/serializers.py

@@ -1757,13 +1757,15 @@ def update(self, instance, validated_data):
         if reporter_id := validated_data.get("reporter"):
             instance.reporter = reporter_id
 
+        # Persist vulnerability IDs first so model save computes hash including them (if there is no hash yet)
+        # we can't pass unsaved_vulnerabilitiy_ids to super.update()
+        if parsed_vulnerability_ids:
+            save_vulnerability_ids(instance, parsed_vulnerability_ids)
+
         instance = super().update(
             instance, validated_data,
         )
 
-        if parsed_vulnerability_ids:
-            save_vulnerability_ids(instance, parsed_vulnerability_ids)
-
         if push_to_jira:
             jira_helper.push_to_jira(instance)
 
@@ -1880,11 +1882,15 @@ def create(self, validated_data):
         if (vulnerability_ids := validated_data.pop("vulnerability_id_set", None)):
             logger.debug("VULNERABILITY_ID_SET: %s", vulnerability_ids)
             parsed_vulnerability_ids.extend(vulnerability_id["vulnerability_id"] for vulnerability_id in vulnerability_ids)
+            logger.debug("PARSED_VULNERABILITY_IDST: %s", parsed_vulnerability_ids)
             logger.debug("SETTING CVE FROM VULNERABILITY_ID_SET: %s", parsed_vulnerability_ids[0])
             validated_data["cve"] = parsed_vulnerability_ids[0]
+            # validated_data["unsaved_vulnerability_ids"] = parsed_vulnerability_ids
 
-        new_finding = super().create(
-            validated_data)
+        # super.create() doesn't accept unsaved_vulnerability_ids or dedupe_option=False, so call save directly.
+        new_finding = Finding(**validated_data)
+        new_finding.unsaved_vulnerability_ids = parsed_vulnerability_ids or []
+        new_finding.save()
 
         logger.debug(f"New finding CVE: {new_finding.cve}")
 
@@ -1897,9 +1903,6 @@ def create(self, validated_data):
             new_finding.reviewers.set(reviewers)
         if parsed_vulnerability_ids:
             save_vulnerability_ids(new_finding, parsed_vulnerability_ids)
-            # can we avoid this extra save? the cve has already been set above in validated_data. but there are no tests for this
-            # on finding update nothing is done # with vulnerability_ids?
-            # new_finding.save()
 
         if push_to_jira:
             jira_helper.push_to_jira(new_finding)

unittests/test_rest_framework.py

@@ -11,8 +11,10 @@
 # from drf_spectacular.renderers import OpenApiJsonRenderer
 from unittest.mock import ANY, MagicMock, call, patch
 
+from django.conf import settings
 from django.contrib.auth.models import Permission
 from django.test import tag as test_tag
+from django.test.utils import override_settings
 from django.urls import reverse
 from drf_spectacular.drainage import GENERATOR_STATS
 from drf_spectacular.settings import spectacular_settings
@@ -1245,6 +1247,63 @@ def test_duplicate(self):
         self.assertFalse(result_json["duplicate"])
         self.assertIsNone(result_json["duplicate_finding"])
 
+    def test_hash_code_includes_vulnerability_ids_on_create(self):
+        zap_fields = ["title", "cwe", "severity", "vulnerability_ids"]
+        current = dict(getattr(settings, "HASHCODE_FIELDS_PER_SCANNER", {}))
+        current["ZAP Scan"] = zap_fields
+
+        with override_settings(HASHCODE_FIELDS_PER_SCANNER=current):
+            orig = Finding.objects.filter(test__test_type__name="ZAP Scan").first()
+            self.assertIsNotNone(orig, "Fixture must provide a ZAP Scan finding")
+
+            cve_value = "CVE-9999-0001"
+
+            model_clone = Finding(
+                test=orig.test,
+                title=orig.title,
+                date=orig.date,
+                cwe=orig.cwe,
+                severity=orig.severity,
+                description=orig.description,
+                mitigation=orig.mitigation,
+                impact=orig.impact,
+                references=orig.references,
+                active=orig.active,
+                verified=orig.verified,
+                false_p=orig.false_p,
+                duplicate=orig.duplicate,
+                out_of_scope=orig.out_of_scope,
+                under_review=orig.under_review,
+                under_defect_review=orig.under_defect_review,
+                numerical_severity=orig.numerical_severity,
+                reporter=orig.reporter,
+                static_finding=orig.static_finding,
+                dynamic_finding=orig.dynamic_finding,
+                file_path=orig.file_path,
+                line=orig.line,
+            )
+            model_clone.unsaved_vulnerability_ids = [cve_value]
+            model_clone.save()
+            model_hash = model_clone.hash_code
+
+            payload = self.payload.copy()
+            payload.update({
+                "test": orig.test.id,
+                "title": orig.title,
+                "cwe": orig.cwe,
+                "severity": orig.severity,
+                "vulnerability_ids": [{"vulnerability_id": cve_value}],
+            })
+            payload["found_by"] = []
+
+            response = self.client.post(self.url, payload, format="json")
+            self.assertEqual(201, response.status_code, response.content[:1000])
+            new_id = response.data.get("id")
+            self.assertIsNotNone(new_id)
+            created = Finding.objects.get(id=new_id)
+
+            self.assertEqual(model_hash, created.hash_code)
+
     def test_filter_steps_to_reproduce(self):
         # Confirm initial data
         result = self.client.get(self.url + "?steps_to_reproduce=lorem")