fix(vuln-id): make backfill convergent + guard the legacy-table drop

Addresses review of the entity-only cutover (rollout machinery):

- backfill.py: the reference build is now convergent — per finding_id window it
  DELETEs that window's references and re-inserts from the current legacy rows,
  inside one transaction per window. A pre-run + deploy-time catch-up where a
  finding's ids changed no longer (a) leaves stale references behind, nor (b)
  raises on the unique_finding_order constraint by re-inserting at occupied
  order slots. Still idempotent and resumable. Window bounds span both the legacy
  and reference tables so stale refs past the legacy max are still reconciled.
- Remove the silent non-PostgreSQL vendor guard from the backfill migration (0287)
  and the migrate_vulnerability_ids command. DefectDojo is PostgreSQL-only, so the
  backfill runs unconditionally and fails loudly rather than skipping and letting
  the unconditional 0288 drop destroy un-migrated data.
- 0288: add a one-query safety invariant before DeleteModel — abort the drop if
  dojo_vulnerability_id still has rows but the reference table is empty (the useful
  residue of the removed verify command).
- Fix stale "frozen until dropped in a follow-up migration" docs in manager.py /
  queries.py — 0288 in this same change drops the legacy table.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Maffooch

dojo/db_migrations/0287_backfill_vulnerability_id_entities.py

@@ -1,36 +1,33 @@
 # Data-only backfill of the Vulnerability entity + FindingVulnerabilityReference tables created
-# in 0286. atomic=False so each set-based statement (and each finding_id window in the reference
-# build) commits on its own — a crash mid-backfill resumes where it left off, and ON CONFLICT DO
-# NOTHING everywhere makes any re-run a no-op on completed work. Insert-only into the two NEW
-# tables: dojo_vulnerability_id and dojo_finding are never rewritten, locked, or deleted, so
-# concurrent readers/writers are not blocked.
+# in 0286. atomic=False so each entity insert and each finding_id window in the (convergent)
+# reference build commits on its own — a crash mid-backfill resumes where it left off, and the
+# reference build re-derives each window from the current legacy rows so any re-run converges on
+# already-current work. Only the two NEW tables are written (the reference table is also deleted
+# from, per window); dojo_vulnerability_id and dojo_finding are never rewritten, locked, or deleted.
 #
 # The actual work lives in dojo.vulnerability.backfill.run_backfill, shared with the
 # `migrate_vulnerability_ids` management command (big installs pre-run the command on the previous
-# release; this migration is then an idempotent catch-up during the deploy).
+# release; this migration is then a convergent catch-up during the deploy).
+#
+# PostgreSQL only: DefectDojo is a PostgreSQL-only application (0286's GIN / full-text indexes
+# already require it), so this runs unconditionally and the underlying SQL fails loudly on any
+# other backend rather than skipping silently and leaving 0288 to drop the un-migrated source.
 import logging
 
-from django.db import connection, migrations
+from django.db import migrations
 
 logger = logging.getLogger(__name__)
 
 
 def backfill_vulnerability_id_entities(apps, schema_editor):
-    if connection.vendor != "postgresql":
-        logger.warning(
-            "Vulnerability ID entity backfill is PostgreSQL-only and was skipped on the %s "
-            "backend. Run `manage.py migrate_vulnerability_ids` for a backend-appropriate backfill.",
-            connection.vendor,
-        )
-        return
     from dojo.vulnerability.backfill import run_backfill  # noqa: PLC0415 -- lazy import
 
     run_backfill(logger=logger)
 
 
 class Migration(migrations.Migration):
 
-    """Idempotent, resumable, insert-only backfill of the vulnerability-id entity tables."""
+    """Idempotent, resumable, convergent backfill of the vulnerability-id entity tables."""
 
     atomic = False
 

dojo/db_migrations/0288_delete_vulnerability_id.py

@@ -1,10 +1,34 @@
 # Entity-only cutover: drop the legacy Vulnerability_Id store. Vulnerability ids now live entirely in
 # the Vulnerability entity + FindingVulnerabilityReference through-table (created in 0286, backfilled
 # from dojo_vulnerability_id in 0287 — which runs BEFORE this drop). ⚠️ DESTRUCTIVE: drops
-# dojo_vulnerability_id and its data. On upgrading installs, ensure 0287's backfill (or
-# `manage.py migrate_vulnerability_ids` pre-run) has completed before this migration removes the source.
+# dojo_vulnerability_id and its data.
+#
+# Safety invariant (the useful residue of the removed verify_vulnerability_ids command): refuse to
+# drop the legacy table if it still holds rows the backfill never transferred. This is the last
+# moment such a check can act — once the table is gone, the source of truth is gone with it. It
+# catches the one catastrophic ordering mistake (0288 reached with 0287 skipped/failed, e.g. on a
+# non-PostgreSQL backend where the raw-SQL backfill never ran) and turns silent data loss into a
+# loud, actionable migration failure.
 
-from django.db import migrations
+from django.db import connection, migrations
+
+_ABORT_MESSAGE = (
+    "Refusing to drop dojo_vulnerability_id: it still has rows but "
+    "dojo_findingvulnerabilityreference is empty, so the 0287 backfill did not run (or did not "
+    "complete). Dropping now would permanently lose vulnerability-id data. Run "
+    "`manage.py migrate_vulnerability_ids` (PostgreSQL only) to backfill, then re-run migrate."
+)
+
+
+def assert_backfill_ran(apps, schema_editor):
+    """Abort the drop if the legacy table has data but the reference table is empty."""
+    with connection.cursor() as cursor:
+        cursor.execute("SELECT EXISTS (SELECT 1 FROM dojo_vulnerability_id)")
+        legacy_has_rows = cursor.fetchone()[0]
+        cursor.execute("SELECT EXISTS (SELECT 1 FROM dojo_findingvulnerabilityreference)")
+        references_exist = cursor.fetchone()[0]
+    if legacy_has_rows and not references_exist:
+        raise RuntimeError(_ABORT_MESSAGE)
 
 
 class Migration(migrations.Migration):
@@ -14,6 +38,9 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
+        # Guard runs before the drop; on an atomic migration a raised RuntimeError rolls the whole
+        # migration back, so the DeleteModel never executes when the invariant is violated.
+        migrations.RunPython(assert_backfill_ran, migrations.RunPython.noop),
         migrations.DeleteModel(
             name="Vulnerability_Id",
         ),

dojo/management/commands/migrate_vulnerability_ids.py

@@ -1,7 +1,6 @@
 import logging
 
 from django.core.management.base import BaseCommand
-from django.db import connection
 
 from dojo.vulnerability.backfill import DEFAULT_WINDOW_SIZE, run_backfill
 
@@ -14,10 +13,12 @@ class Command(BaseCommand):
     Backfill the Vulnerability entity + FindingVulnerabilityReference tables from the legacy
     dojo_vulnerability_id table and dojo_finding.cve.
 
-    Idempotent and resumable (insert-only + ON CONFLICT DO NOTHING), so it is safe to run
-    repeatedly: large tenants can pre-run it on the previous release, then it is an instant
-    catch-up during the deploy-time migration. Shares dojo.vulnerability.backfill.run_backfill
-    with migration 0286.
+    Idempotent and convergent: the reference build re-derives each finding_id window from the
+    current legacy rows, so it is safe to run repeatedly. Large tenants can pre-run it on the
+    previous release; migration 0287 is then a convergent catch-up during the deploy that also
+    reconciles any ids that changed in between. Shares dojo.vulnerability.backfill.run_backfill
+    with migration 0287. PostgreSQL only (DefectDojo is PostgreSQL-only); the underlying SQL fails
+    loudly on any other backend.
     """
 
     help = "Idempotently backfill the vulnerability-id entity + reference tables (PostgreSQL only)."
@@ -31,9 +32,6 @@ def add_arguments(self, parser):
         )
 
     def handle(self, *args, **options):
-        if connection.vendor != "postgresql":
-            self.stderr.write(f"Vulnerability-id backfill is PostgreSQL only; nothing to do on {connection.vendor}.")
-            return
         counts = run_backfill(logger=logger, window_size=options["window_size"])
         for step, count in counts.items():
             self.stdout.write(f"{step}: {count}")

dojo/vulnerability/backfill.py

@@ -1,20 +1,32 @@
 """
-Idempotent, set-based backfill of the Vulnerability entity + FindingVulnerabilityReference
-tables from the legacy dojo_vulnerability_id table and dojo_finding.cve.
-
-Shared by migration 0286 (RunPython forward) and the `migrate_vulnerability_ids` management
-command. PostgreSQL only (uses ``ON CONFLICT``); callers guard on vendor and print the escape
-hatch for other backends. Every statement is insert-only into the two NEW tables with
-``ON CONFLICT DO NOTHING`` — nothing in dojo_vulnerability_id / dojo_finding is ever read-locked,
-rewritten, or deleted, and any re-run (migration or command) is a no-op on completed work.
+Idempotent, convergent, set-based backfill of the Vulnerability entity +
+FindingVulnerabilityReference tables from the legacy dojo_vulnerability_id table and
+dojo_finding.cve.
+
+Shared by migration 0287 (RunPython forward) and the `migrate_vulnerability_ids` management
+command. PostgreSQL only (uses ``ON CONFLICT`` and window functions) — DefectDojo is a
+PostgreSQL-only application (the models rely on GIN / full-text indexes), so the raw SQL below
+fails loudly on any other backend rather than silently skipping.
+
+Entity inserts (steps 1-2) are insert-only with ``ON CONFLICT DO NOTHING``. The reference build
+(step 3) is **convergent**, not insert-only: each finding_id window ``DELETE``s the references for
+its findings and re-inserts them from the *current* legacy rows, inside one transaction per window.
+That is what makes a pre-run + deploy-time catch-up safe: if a finding's ids changed between the
+pre-run and the deploy, the stale references are removed and rebuilt rather than left to drift (and
+a plain re-insert would in any case violate the ``unique_finding_order`` constraint at the occupied
+order slots). Because each window commits atomically and the delete is a superset re-derive, a crash
+mid-backfill resumes cleanly and any re-run is a no-op on already-current work.
+
+The legacy source tables (dojo_vulnerability_id / dojo_finding) are never rewritten, locked, or
+deleted — only the two NEW tables are written, and only the reference table is deleted from.
 
 The reference `order` is assigned cve-first then legacy-PK-ascending, so ``order == 0`` is the
 finding's cve (primary id) by construction — matching every live write path.
 """
 
 import logging
 
-from django.db import connection
+from django.db import connection, transaction
 
 from dojo.finding.vulnerability_id import resolve_vulnerability_id_type
 
@@ -37,11 +49,38 @@
     ON CONFLICT (vulnerability_id) DO NOTHING
 """
 
-# Windowed by legacy finding_id. ROW_NUMBER orders cve-match first, then legacy PK ascending;
-# s.ord - 1 makes order 0-based so the cve row lands at order 0. The join to dojo_vulnerability
-# resolves each legacy string to its entity PK. ON CONFLICT covers re-runs and the unique
-# (finding_id, vulnerability_id) constraint (dojo 0282 guarantees legacy pair uniqueness, so no
-# dedupe layer is needed here).
+# Reference window bounds span BOTH tables: a catch-up run must reach any finding that already has
+# references (to re-derive/clean it) even if its last legacy row was removed since the pre-run and
+# it now sits past MAX(dojo_vulnerability_id.finding_id). LEAST/GREATEST ignore NULLs on Postgres,
+# so an empty reference table (first run) collapses this to the legacy bounds.
+_SELECT_REFERENCE_WINDOW_BOUNDS = """
+    SELECT
+        LEAST(
+            (SELECT MIN(finding_id) FROM dojo_vulnerability_id),
+            (SELECT MIN(finding_id) FROM dojo_findingvulnerabilityreference)
+        ),
+        GREATEST(
+            (SELECT MAX(finding_id) FROM dojo_vulnerability_id),
+            (SELECT MAX(finding_id) FROM dojo_findingvulnerabilityreference)
+        )
+"""
+
+# Convergent step 1: clear this window's references so the re-insert below rebuilds them from the
+# current legacy rows. Only the NEW reference table is touched. Paired with the insert inside one
+# transaction per window (see run_backfill), this removes any references whose finding's ids changed
+# since a pre-run — and it is what lets the insert stay conflict-free: after the delete there is no
+# occupied `order` slot for the ``unique_finding_order`` constraint to collide with.
+_DELETE_REFERENCES_WINDOW = """
+    DELETE FROM dojo_findingvulnerabilityreference
+    WHERE finding_id >= %(lo)s AND finding_id < %(hi)s
+"""
+
+# Convergent step 2. Windowed by legacy finding_id. ROW_NUMBER orders cve-match first, then legacy
+# PK ascending; s.ord - 1 makes order 0-based so the cve row lands at order 0. The join to
+# dojo_vulnerability resolves each legacy string to its entity PK. The ON CONFLICT on
+# (finding_id, vulnerability_id) is a belt-and-suspenders guard for a concurrent live writer having
+# re-inserted the same pair between this window's DELETE and INSERT; the DELETE guarantees the
+# backfill's own inserts never collide (dojo 0282 also guarantees legacy pair uniqueness).
 _INSERT_REFERENCES_WINDOW = """
     INSERT INTO dojo_findingvulnerabilityreference (finding_id, vulnerability_id, "order", created)
     SELECT s.finding_id, e.id, s.ord - 1, now()
@@ -98,9 +137,10 @@ def _stamp_missing_types(cursor, logger):
 
 def run_backfill(*, logger=None, window_size=DEFAULT_WINDOW_SIZE):
     """
-    Run the full idempotent backfill. PostgreSQL only. Returns a dict of per-step row counts.
+    Run the full idempotent, convergent backfill. PostgreSQL only. Returns per-step row counts.
 
-    Callers (migration 0286 / migrate_vulnerability_ids) are responsible for the vendor guard.
+    The reference build is convergent (per-window DELETE + re-insert in one transaction each), so it
+    is safe to re-run as a deploy-time catch-up after a pre-run even when ids changed in between.
     """
     logger = logger or logging.getLogger(__name__)
     counts = {}
@@ -115,17 +155,26 @@ def run_backfill(*, logger=None, window_size=DEFAULT_WINDOW_SIZE):
         counts["entities_from_cve"] = cursor.rowcount
         counts["types_stamped"] = _stamp_missing_types(cursor, logger)
 
-        logger.info("Backfill step 3/3: references (windowed by finding_id, window=%s)", window_size)
-        cursor.execute("SELECT MIN(finding_id), MAX(finding_id) FROM dojo_vulnerability_id")
+        logger.info("Backfill step 3/3: references (convergent, windowed by finding_id, window=%s)", window_size)
+        cursor.execute(_SELECT_REFERENCE_WINDOW_BOUNDS)
         lo, hi_max = cursor.fetchone()
-        references = 0
+        deleted = 0
+        inserted = 0
         if lo is not None:
             while lo <= hi_max:
                 hi = lo + window_size
-                cursor.execute(_INSERT_REFERENCES_WINDOW, {"lo": lo, "hi": hi})
-                references += cursor.rowcount
+                # One transaction per window: the DELETE + INSERT commit together, so a crash leaves
+                # the window either fully rebuilt or untouched (resumable), and the DELETE's row lock
+                # serializes the rebuild against a concurrent live delete-then-insert on the same
+                # finding. atomic=False on the migration keeps each window a separate commit.
+                with transaction.atomic():
+                    cursor.execute(_DELETE_REFERENCES_WINDOW, {"lo": lo, "hi": hi})
+                    deleted += cursor.rowcount
+                    cursor.execute(_INSERT_REFERENCES_WINDOW, {"lo": lo, "hi": hi})
+                    inserted += cursor.rowcount
                 lo = hi
-        counts["references_from_legacy"] = references
+        counts["references_deleted"] = deleted
+        counts["references_from_legacy"] = inserted
 
     logger.info("Backfill complete: %s", counts)
     return counts

dojo/vulnerability/manager.py

@@ -2,8 +2,8 @@
 Write seam for vulnerability ids: the ``Vulnerability`` entity + ``FindingVulnerabilityReference``
 rows are the single source of truth. Every writer persists them through this one module.
 
-The legacy ``Vulnerability_Id`` store is no longer written (entity-only cutover); the legacy table
-still exists but is frozen until it is dropped in a follow-up migration.
+The legacy ``Vulnerability_Id`` store is gone: this entity-only cutover stops writing it and drops
+its table in the same change (migration 0288, after the 0287 backfill).
 CWE buffering (store_cwes/pending_cwes/reconcile_cwes) is deliberately NOT owned here — it keeps
 riding the importer flush boundary exactly as before.
 """

dojo/vulnerability/queries.py

@@ -18,7 +18,8 @@ def get_auth_filter(key):
 # ---------------------------------------------------------------------------
 # Read helpers over the entity/reference store — the single source of truth.
 # (The legacy Vulnerability_Id read path + V3_FEATURE_VULNERABILITY_IDS flag were
-# removed in the entity-only cutover; the legacy table is frozen until dropped.)
+# removed in the entity-only cutover; the legacy table is dropped by migration 0288
+# in the same change, after the 0287 backfill.)
 # ---------------------------------------------------------------------------