feat(finding): copy finding fix + autodetected vulnerability id type + uniqueness constraint (#15145)

* feat(finding): autodetected vulnerability_id type + uniqueness constraint

Adds Vulnerability_Id.vulnerability_id_type, autodetected from the id's
leading prefix (CVE-2024-1234 -> CVE, GHSA-... -> GHSA), stored and indexed
so identifiers can be filtered/grouped by type. Populated on import (bulk)
and on save(); existing rows backfilled by migration.

Also de-duplicates (finding, vulnerability_id) rows and adds a unique
constraint on the pair. CWE is a weakness class and is intentionally NOT
part of this change; vulnerability_id_type does not participate in
hash_code, so existing hash codes and deduplication are unaffected.

Migrations: 0276 (type column + lookup index), 0277 (dedupe + backfill,
data), 0278 (unique constraint).

* fix(vuln-id): cap autodetected vulnerability_id_type at column width (20)

resolve_vulnerability_id_type returned the raw uppercased prefix with no
length bound, but vulnerability_id_type is CharField(max_length=20) while
vulnerability_id is unbounded text. A >20-char prefix (e.g. a dash-less
token/hash used as an id) raised DataError on save()/bulk_create (import
500) and was silently truncated by the 0277 backfill's bulk_update CAST.
Treat an over-length prefix as untyped (None) so save, bulk_create, and
the backfill all agree and never error or truncate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(migrations): rebase onto dev, renumber vuln-id chain 0280-0282

Resolves the migration-graph leaf collision: dev has advanced to
0279_jira_project_transition_fields, so the vulnerability-id migrations are
renumbered 0276/0277/0278 -> 0280/0281/0282 and the first is re-parented onto
the current dev leaf.

Also merges a duplicate `class Meta` on Vulnerability_Id introduced by the
rebase: dev's global-search FTS work added its own Meta (GIN indexes) adjacent
to this branch's Meta (unique constraint + dojo_vuln_id_lookup_idx). Python
keeps only the last class body, which silently dropped the constraint/index
from the model state (makemigrations wanted to remove them). Both are now in a
single Meta. `makemigrations --check dojo` reports no changes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* perf(migrations): build vuln-id index + unique constraint concurrently (F9)

0280's lookup index and 0282's (finding, vulnerability_id) unique constraint
were built with plain AddIndex / AddConstraint, taking a SHARE lock (blocks
writes) and an ACCESS EXCLUSIVE lock (blocks reads+writes) for the full build
over the whole dojo_vulnerability_id table -- a material downtime window on
large instances.

Build both CONCURRENTLY instead (atomic=False), keeping the work in-migration:
- 0280: AddIndexConcurrently for dojo_vuln_id_lookup_idx.
- 0282: CREATE UNIQUE INDEX CONCURRENTLY, then ADD CONSTRAINT ... USING INDEX
  (brief lock to adopt the pre-built index); SeparateDatabaseAndState keeps the
  UniqueConstraint in Django's model state.

Verified on a 100k-finding / 174k-id dataset with duplicate rows: 0281 dedupe
-> 0, constraint built valid as a real UNIQUE constraint, makemigrations
--check clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: 3 people

docs/content/releases/os_upgrading/3.2.md

@@ -0,0 +1,34 @@
+---
+title: 'Upgrading to DefectDojo Version 3.2.x'
+toc_hide: true
+weight: -20260701
+description: Vulnerability ids gain an autodetected type. A migration adds the type column, de-duplicates vulnerability-id rows, and adds a uniqueness constraint. Existing hash codes are unaffected.
+---
+
+## Vulnerability id type
+
+Each `Vulnerability_Id` gains an autodetected `vulnerability_id_type` — the identifier's leading
+prefix (`CVE-2024-1234` → `CVE`, `GHSA-…` → `GHSA`, `RUSTSEC-…` → `RUSTSEC`). It is derived
+structurally (no registry) and stored (indexed) so identifiers can be filtered and grouped by type
+efficiently. It is `NULL` when there is no non-numeric prefix.
+
+The type is populated automatically: on import (bulk paths) and on `save()`. Existing rows are
+backfilled by the migration below. This is a denormalized, derived attribute — it does not
+participate in `hash_code`, so **existing hash codes and deduplication are unaffected**.
+
+## Database migration
+
+Three migrations run automatically on upgrade:
+
+- `0276_vulnerability_id_type` — adds the indexed `vulnerability_id_type` column and a leading
+  index on `vulnerability_id`.
+- `0277_backfill_vulnerability_id_type` — backfills `vulnerability_id_type` for existing rows and
+  removes duplicate `(finding, vulnerability_id)` rows, keeping the earliest of each — such
+  duplicates are unintended, and consolidating them allows a uniqueness constraint to be added.
+- `0278_unique_finding_vulnerability_id` — adds a unique constraint on `(finding, vulnerability_id)`.
+
+### What you need to do
+
+The migrations are applied automatically. No manual steps are required.
+
+For more information, check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/3.2.0).

dojo/db_migrations/0280_vulnerability_id_type.py

@@ -0,0 +1,35 @@
+from django.contrib.postgres.operations import AddIndexConcurrently
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    """
+    Schema only: add the autodetected vulnerability_id_type column and a leading index on
+    vulnerability_id. The data backfill (0281) and the unique constraint (0282, which requires
+    the dedupe data step first) are kept in separate migrations so data migrations are never
+    mixed with schema migrations.
+
+    The lookup index is built with CREATE INDEX CONCURRENTLY (no SHARE lock, so writes to
+    dojo_vulnerability_id keep flowing during the build), which requires a non-atomic migration.
+    Adding the nullable, all-NULL vulnerability_id_type column and its own index is a fast catalog
+    operation, so it stays a plain AddField.
+    """
+
+    atomic = False
+
+    dependencies = [
+        ("dojo", "0279_jira_project_transition_fields"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="vulnerability_id",
+            name="vulnerability_id_type",
+            field=models.CharField(blank=True, db_index=True, editable=False, max_length=20, null=True),
+        ),
+        AddIndexConcurrently(
+            model_name="vulnerability_id",
+            index=models.Index(fields=["vulnerability_id"], name="dojo_vuln_id_lookup_idx"),
+        ),
+    ]

dojo/db_migrations/0281_backfill_vulnerability_id_type.py

@@ -0,0 +1,49 @@
+from django.db import migrations
+
+from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
+
+BATCH_SIZE = 1000
+
+
+def dedupe_vulnerability_ids(apps, schema_editor):
+    """Remove duplicate (finding, vulnerability_id) rows, keeping the lowest id, so the unique
+    constraint added in the following schema migration (0278) can be created. Postgres."""
+    schema_editor.execute(
+        """
+        DELETE FROM dojo_vulnerability_id a
+        USING dojo_vulnerability_id b
+        WHERE a.finding_id = b.finding_id
+          AND a.vulnerability_id = b.vulnerability_id
+          AND a.id > b.id
+        """,
+    )
+
+
+def backfill_vulnerability_id_type(apps, schema_editor):
+    """Populate vulnerability_id_type for existing rows by autodetecting the id prefix."""
+    Vulnerability_Id = apps.get_model("dojo", "Vulnerability_Id")
+    batch = []
+    qs = Vulnerability_Id.objects.filter(vulnerability_id_type__isnull=True).only("id", "vulnerability_id")
+    for row in qs.iterator(chunk_size=BATCH_SIZE):
+        row.vulnerability_id_type = resolve_vulnerability_id_type(row.vulnerability_id)
+        batch.append(row)
+        if len(batch) >= BATCH_SIZE:
+            Vulnerability_Id.objects.bulk_update(batch, ["vulnerability_id_type"], batch_size=BATCH_SIZE)
+            batch = []
+    if batch:
+        Vulnerability_Id.objects.bulk_update(batch, ["vulnerability_id_type"], batch_size=BATCH_SIZE)
+
+
+class Migration(migrations.Migration):
+
+    """Data only (no schema changes): dedupe (finding, vulnerability_id) rows and backfill
+    vulnerability_id_type for existing rows."""
+
+    dependencies = [
+        ("dojo", "0280_vulnerability_id_type"),
+    ]
+
+    operations = [
+        migrations.RunPython(dedupe_vulnerability_ids, migrations.RunPython.noop),
+        migrations.RunPython(backfill_vulnerability_id_type, migrations.RunPython.noop),
+    ]

dojo/db_migrations/0282_unique_finding_vulnerability_id.py

@@ -0,0 +1,43 @@
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    """
+    Schema only: add the (finding, vulnerability_id) unique constraint. Runs after the 0281
+    dedupe data step so the constraint can be created without violating existing duplicate rows.
+
+    The unique index is built with CREATE UNIQUE INDEX CONCURRENTLY (no ACCESS EXCLUSIVE
+    full-build lock, so reads and writes keep flowing during the build) and then attached as the
+    constraint with ADD CONSTRAINT ... USING INDEX, which only takes a brief lock to adopt the
+    already-built index. Both steps run inside this migration; CONCURRENTLY requires atomic=False.
+    """
+
+    atomic = False
+
+    dependencies = [
+        ("dojo", "0281_backfill_vulnerability_id_type"),
+    ]
+
+    operations = [
+        migrations.RunSQL(
+            sql="CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS unique_finding_vulnerability_id "
+                "ON dojo_vulnerability_id (finding_id, vulnerability_id);",
+            reverse_sql="DROP INDEX CONCURRENTLY IF EXISTS unique_finding_vulnerability_id;",
+        ),
+        migrations.SeparateDatabaseAndState(
+            database_operations=[
+                migrations.RunSQL(
+                    sql="ALTER TABLE dojo_vulnerability_id ADD CONSTRAINT unique_finding_vulnerability_id "
+                        "UNIQUE USING INDEX unique_finding_vulnerability_id;",
+                    reverse_sql="ALTER TABLE dojo_vulnerability_id DROP CONSTRAINT IF EXISTS unique_finding_vulnerability_id;",
+                ),
+            ],
+            state_operations=[
+                migrations.AddConstraint(
+                    model_name="vulnerability_id",
+                    constraint=models.UniqueConstraint(fields=("finding", "vulnerability_id"), name="unique_finding_vulnerability_id"),
+                ),
+            ],
+        ),
+    ]

dojo/finding/helper.py

@@ -26,6 +26,7 @@
     do_false_positive_history_batch,
     get_finding_models_for_deduplication,
 )
+from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
 from dojo.jira import services as jira_services
 from dojo.location.models import Location
 from dojo.location.status import FindingLocationStatus
@@ -1068,7 +1069,7 @@ def save_vulnerability_ids(finding, vulnerability_ids, *, delete_existing: bool
         Vulnerability_Id.objects.filter(finding=finding).delete()
 
     Vulnerability_Id.objects.bulk_create([
-        Vulnerability_Id(finding=finding, vulnerability_id=vid)
+        Vulnerability_Id(finding=finding, vulnerability_id=vid, vulnerability_id_type=resolve_vulnerability_id_type(vid))
         for vid in vulnerability_ids
     ])
 

dojo/finding/models.py

@@ -24,6 +24,7 @@
 from titlecase import titlecase
 
 from dojo.base_models.base import BaseModel
+from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
 
 # get_current_date/tomorrow/copy_model_util are defined early in dojo.models, before the
 # re-export that loads this module — so this resolves despite the partial circular load, and
@@ -700,6 +701,9 @@ def copy(self, test=None):
         copy.found_by.set(old_found_by)
         # Assign any tags
         copy.tags.set(old_tags)
+        # Copy the vulnerability ids (relation rows aren't copied by copy_model_util)
+        for vulnerability_id in self.vulnerability_id_set.all():
+            Vulnerability_Id.objects.create(finding=copy, vulnerability_id=vulnerability_id.vulnerability_id)
 
         return copy
 
@@ -1377,9 +1381,18 @@ def set_hash_code(self, dedupe_option):
 class Vulnerability_Id(models.Model):
     finding = models.ForeignKey("dojo.Finding", editable=False, on_delete=models.CASCADE)
     vulnerability_id = models.TextField(max_length=50, blank=False, null=False)
+    # Autodetected from the id prefix (CVE, GHSA, ...); NULL when there is no non-numeric
+    # prefix. Denormalized/indexed so type-scoped queries (e.g. GROUP BY type) stay cheap.
+    vulnerability_id_type = models.CharField(max_length=20, null=True, blank=True, editable=False, db_index=True)
 
     class Meta:
+        constraints = [
+            models.UniqueConstraint(fields=["finding", "vulnerability_id"], name="unique_finding_vulnerability_id"),
+        ]
         indexes = [
+            # Leading on vulnerability_id (the unique constraint's index leads on finding), for
+            # GROUP BY vulnerability_id / lookups by exact id.
+            models.Index(fields=["vulnerability_id"], name="dojo_vuln_id_lookup_idx"),
             # Global search (pro/search/): weighted tsvector FTS + trigram fuzzy match.
             GinIndex(
                 SearchVector("vulnerability_id", weight="A", config="english"),
@@ -1391,6 +1404,11 @@ class Meta:
     def __str__(self):
         return self.vulnerability_id
 
+    def save(self, *args, **kwargs):
+        # bulk_create paths set the type at construction; this covers save()/get_or_create.
+        self.vulnerability_id_type = resolve_vulnerability_id_type(self.vulnerability_id)
+        super().save(*args, **kwargs)
+
     def get_absolute_url(self):
         return reverse("view_finding", args=[str(self.finding.id)])
 

dojo/finding/vulnerability_id.py

@@ -0,0 +1,21 @@
+"""Pure helpers for vulnerability identifiers (no model imports, safe to import anywhere)."""
+
+
+def resolve_vulnerability_id_type(vulnerability_id: str | None) -> str | None:
+    """
+    Autodetect the type from the id's leading prefix (the part before the first ``-``).
+
+    Structural, no registry: ``CVE-2024-1 -> "CVE"``, ``GHSA-... -> "GHSA"``,
+    ``RUSTSEC-2021-0001 -> "RUSTSEC"``, ``ALINUX2-SA-... -> "ALINUX2"``. Returns the uppercased
+    prefix, or ``None`` when there is no non-numeric prefix (bare numbers / UUIDs / no dash).
+
+    The prefix is capped at the ``vulnerability_id_type`` column width (20). A longer prefix is
+    not a real scheme (e.g. a dash-less hash used as an id), so it is treated as untyped rather
+    than truncated/erroring on save, bulk_create, and the 0277 backfill.
+    """
+    if not vulnerability_id:
+        return None
+    prefix = str(vulnerability_id).strip().split("-", 1)[0].strip()
+    if not prefix or prefix.isdigit() or len(prefix) > 20:
+        return None
+    return prefix.upper()

dojo/importers/base_importer.py

@@ -12,6 +12,7 @@
 
 import dojo.finding.helper as finding_helper
 import dojo.risk_acceptance.helper as ra_helper
+from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
 from dojo.importers.options import ImporterOptions
 from dojo.jira.services import is_keep_in_sync
 from dojo.location.models import Location
@@ -893,7 +894,7 @@ def store_vulnerability_ids(
         vulnerability_ids_to_process = list(dict.fromkeys(finding.unsaved_vulnerability_ids or []))
         vulnerability_ids_to_process = [x for x in vulnerability_ids_to_process if x.strip()]
         self.pending_vulnerability_ids.extend([
-            Vulnerability_Id(finding=finding, vulnerability_id=vid)
+            Vulnerability_Id(finding=finding, vulnerability_id=vid, vulnerability_id_type=resolve_vulnerability_id_type(vid))
             for vid in vulnerability_ids_to_process
         ])
         if vulnerability_ids_to_process:

dojo/importers/default_reimporter.py

@@ -13,6 +13,7 @@
     find_candidates_for_deduplication_unique_id,
     find_candidates_for_reimport_legacy,
 )
+from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
 from dojo.importers.base_importer import BaseImporter, Parser
 from dojo.importers.base_location_manager import LocationHandler
 from dojo.importers.options import ImporterOptions
@@ -1000,7 +1001,7 @@ def reconcile_vulnerability_ids(
         # Accumulate delete + insert for batch flush
         self.pending_vuln_id_deletes.append(finding.id)
         self.pending_vulnerability_ids.extend([
-            Vulnerability_Id(finding=finding, vulnerability_id=vid)
+            Vulnerability_Id(finding=finding, vulnerability_id=vid, vulnerability_id_type=resolve_vulnerability_id_type(vid))
             for vid in vulnerability_ids_to_process
         ])
         if vulnerability_ids_to_process:

unittests/test_vulnerability_id_type.py

@@ -0,0 +1,51 @@
+"""Vulnerability_Id type autodetection, the backfill helper path, and the uniqueness constraint."""
+from django.db import IntegrityError, transaction
+from django.test import SimpleTestCase
+
+from dojo.finding.helper import save_vulnerability_ids
+from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
+from dojo.models import Finding, Vulnerability_Id
+from unittests.dojo_test_case import DojoTestCase, versioned_fixtures
+
+
+class TestVulnerabilityIdTypeResolver(SimpleTestCase):
+
+    def test_autodetect(self):
+        self.assertEqual(resolve_vulnerability_id_type("CVE-2024-1234"), "CVE")
+        self.assertEqual(resolve_vulnerability_id_type("GHSA-9v3m-xxxx"), "GHSA")
+        self.assertEqual(resolve_vulnerability_id_type("RUSTSEC-2021-0001"), "RUSTSEC")
+        self.assertEqual(resolve_vulnerability_id_type("ALINUX2-SA-2021"), "ALINUX2")
+        self.assertEqual(resolve_vulnerability_id_type("cve-2024-1"), "CVE")
+        # No non-numeric prefix -> None
+        self.assertIsNone(resolve_vulnerability_id_type("1234"))
+        self.assertIsNone(resolve_vulnerability_id_type(""))
+        self.assertIsNone(resolve_vulnerability_id_type(None))
+
+
+@versioned_fixtures
+class TestVulnerabilityIdType(DojoTestCase):
+    fixtures = ["dojo_testdata.json"]
+
+    def setUp(self):
+        self.finding = Finding.objects.get(id=2)
+        Vulnerability_Id.objects.filter(finding=self.finding).delete()
+
+    def test_save_sets_type(self):
+        row = Vulnerability_Id.objects.create(finding=self.finding, vulnerability_id="CVE-2024-1234")
+        self.assertEqual(row.vulnerability_id_type, "CVE")
+        # bare number -> no type
+        row2 = Vulnerability_Id.objects.create(finding=self.finding, vulnerability_id="12345")
+        self.assertIsNone(row2.vulnerability_id_type)
+
+    def test_bulk_save_sets_type(self):
+        # save_vulnerability_ids uses bulk_create, which sets the type at construction
+        save_vulnerability_ids(self.finding, ["CVE-2024-1", "GHSA-aaaa-bbbb"])
+        types = dict(
+            Vulnerability_Id.objects.filter(finding=self.finding).values_list("vulnerability_id", "vulnerability_id_type"),
+        )
+        self.assertEqual(types, {"CVE-2024-1": "CVE", "GHSA-aaaa-bbbb": "GHSA"})
+
+    def test_unique_constraint(self):
+        Vulnerability_Id.objects.create(finding=self.finding, vulnerability_id="CVE-2024-1234")
+        with self.assertRaises(IntegrityError), transaction.atomic():
+            Vulnerability_Id.objects.create(finding=self.finding, vulnerability_id="CVE-2024-1234")