## Context The vulnerability ingestion pipeline (`ee/app/services/security/ingestion/ingest_reports_service.rb` and the call paths into and out of it) uses a hand-crafted UUID as its deduplication key. The context-aware / context-unaware UUID logic layered on top of it is hard to reason about. This issue captures an analysis of what the UUID actually does today, what it would take to stop relying on it, which database indexes would be needed, and what the risks and sequencing look like. Analysis performed against `master` at `20307f30dba3b2` (2026-08-13). ## 1. What the UUID actually is From `app/services/security/vulnerability_uuid.rb`: ``` uuid = UUIDv5("#{report_type}-#{primary_identifier_fingerprint}-#{location_fingerprint}-#{project_id}[-#{tracked_context_id}]") ``` Two things are worth being precise about, because they change the analysis: - **`location_fingerprint` is usually not a location fingerprint.** `Gitlab::Ci::Reports::Security::Finding#location_fingerprint` (`lib/gitlab/ci/reports/security/finding.rb:188`) returns `signature_hexes.first` when tracking signatures are present and licensed, falling back to the location hash. The `vulnerability_occurrences.location_fingerprint` column stores that same value, so the column and the hash input agree. - **The scanner is deliberately not in the key.** Two different SAST analyzers reporting the same identifier at the same location collapse to one finding. That is a feature, and any replacement key must preserve it. ## 2. The five jobs the UUID is doing | # | Job | Where | |---|---|---| | A | In-report dedup within a pipeline | `StoreScanService#register_finding_keys`; `Finding#keys` | | B | **Upsert conflict target** for `vulnerability_occurrences` | `Tasks::IngestFindings` (`unique_by = :uuid`) | | C | **Cross-table join key** (surrogate FK) | 6 tables + Elasticsearch + ClickHouse | | D | **Cross-branch comparison** | MR widget, scan-result policies | | E | **Identity migration ("takeover")** | `OverrideUuidsService`, `Tasks::UpdateVulnerabilityUuids` | Job A is mostly already independent — `finding.keys` is a cross-product of `FindingKey(location_fingerprint, identifier_fingerprint)` structs and the uuid is just appended. Removing the uuid from that set costs almost nothing. Jobs B–E are the real work, and **C is much wider than the ingestion service**: - `security_findings.uuid` -> `vulnerability_occurrences.uuid` (`ee/app/models/security/finding.rb:38-42`), driving `by_state`, `recently_detected`, `undismissed_by_vulnerability`, the pipeline security tab and the MR widget - `vulnerability_reads.uuid` (unique index, mirrored by two PL/pgSQL triggers) - `vulnerability_feedback.finding_uuid` (legacy dismissals, still read by `Vulnerabilities::Finding.dismissed/undismissed` and `PipelineVulnerabilitiesFinder`) - `security_finding_enrichments.finding_uuid` (unique on `(finding_uuid, cve)`) - `security_policy_dismissals.security_findings_uuids text[]` + `license_occurrence_uuids text[]` (GIN indexes, persisted UUID arrays) - Elasticsearch: `ee/lib/search/elastic/types/vulnerability.rb:44` indexes `uuid` as `binary`; the `vulnerabilities/read` reference serialises it too - ClickHouse: `siphon_security_findings` replicates `uuid`, `overridden_uuid`, `context_unaware_uuid` - GraphQL: 4 field exposures + **8 mutations that take `uuid:` as the identifier for a pipeline finding** (`dismiss`, `revert_to_detected`, `severity_override`, `create_issue`, `create_merge_request`, `create_vulnerability`, `create_external_issue_link`, `create_jira_issue_form_url`) plus `Resolvers::SecurityReport::FindingResolver` ## 3. The context-aware / context-unaware confusion is not caused by the UUID The v1/v2 split exists because VAC (the `vulnerabilities_across_contexts` flag) wants the *same* project's findings tracked separately per ref, while pre-existing default-branch rows must keep their old identity. `ProjectTrackedContext::CONTEXT_UNAWARE_UUID_VERSION` is a per-context discriminator for exactly that. The knock-on effect is what actually hurts: because a v2 UUID differs per branch, cross-branch comparison broke, so a **second** hash (`context_unaware_uuid` / `vulnerability_occurrences.new_uuid`) had to be added purely so the MR widget and scan-result policies could compare head vs base (`policy_comparison_uuid`, `SecurityFindingsReportsComparer:121-132`, `UpdateApprovalsService:452`). That is the genuine design smell: **you can project a tuple onto a subset of its columns; you cannot project a hash.** So the hash had to be computed twice. Deleting the UUID does not delete the underlying requirement. It re-expresses it as `security_project_tracked_context_id IS NULL` vs `= n`, and keeps every bit of the branching logic. So the goal should be "stop hashing the context into the identity", not "stop using UUIDs". ## 4. Three options **Option A — full natural key.** Replace `uuid` with a composite unique constraint on `(project_id, security_project_tracked_context_id, report_type, primary_identifier_id, location_fingerprint)`. Maximum cleanup, maximum cost. Not recommended as a starting point (sizing in section 6). **Option B — narrow the hash, widen the constraint (recommended first move).** Always generate the v1 (context-unaware) UUID; move the context into the uniqueness constraint instead: `UNIQUE (uuid, security_project_tracked_context_id)`. This deletes the hardest-to-read parts: - `VulnerabilityUUID.generate_v2` and the `context_aware_uuids_enabled?` branch (`ProjectTrackedContext:180-185`) - `ProjectTrackedContext.uuid_version` column and the two constants - `context_unaware_uuid` / `new_uuid` / `policy_comparison_uuid` / `distinct_context_unaware_uuids` / `COALESCE(context_unaware_uuid, uuid)` everywhere - The `context_unaware_uuids` bookkeeping in `OverrideUuidsService` - The ClickHouse `context_unaware_uuid` column and it keeps the cross-branch join working *by construction*, because a feature-branch security finding and a default-branch occurrence get the same uuid again. **Option C — demote the UUID to an opaque stable ID.** Keep the column, stop deriving it (random v7), add the natural key as the dedup mechanism. This is the only option that kills the "takeover" services outright, and it composes well with the `vulnerability_redirects` work. But it breaks the derived-join property: `security_findings` would need its correct occurrence UUID stamped at store time via a natural-key lookup — essentially generalising `OverrideUuidsService` to every finding. **Recommendation: B now, C later if still wanted.** B is a single index change plus deletions; C is a multi-milestone identity migration. ## 5. Code changes for Option B | File | Change | |---|---| | `app/services/security/vulnerability_uuid.rb` | Collapse to one method; drop `tracked_context:` kwarg | | `ee/app/models/security/project_tracked_context.rb` | Delete `context_aware_uuids_enabled?`, both constants; deprecate `uuid_version` | | `ee/app/services/security/ingestion/tasks/ingest_findings.rb` | `self.unique_by = %i[uuid security_project_tracked_context_id]` | | `ee/app/services/security/ingestion/finding_map.rb` + `ee/app/services/security/vulnerability_scanning/finding_map.rb` | Drop `new_uuid:` from `to_hash` | | `ee/app/services/security/override_uuids_service.rb` | Drop `context_unaware_uuids` set and `override_context_unaware_uuid_for`; keep signature matching | | `ee/app/models/security/finding.rb`, `ee/app/models/vulnerabilities/finding.rb` | Delete `policy_comparison_uuid`, `distinct_context_unaware_uuids`, the `context_unaware_uuid` alias | | `ee/lib/gitlab/ci/reports/security/security_findings_reports_comparer.rb`, `ee/app/services/security/scan_result_policies/update_approvals_service.rb`, `ee/lib/security/scan_result_policies/grouped_findings_evaluator.rb` | Back to plain `uuid` | | `lib/gitlab/ci/reports/security/finding.rb` | Delete `generate_context_unaware_uuid` and the accessor | Not deleted by B, and worth knowing: `Tasks::UpdateVulnerabilityUuids` and its three subtasks stay. Those exist because the identity is a hash of a *mutable* input (the scanner's identifier), not because of the context. Only Option C removes them. ## 6. Indexes — and why this is the hard constraint Current state: ```sql -- vulnerability_occurrences (table_size: over_limit, i.e. >100 GB incl. indexes; 14 indexes already) CREATE UNIQUE INDEX index_vuln_findings_on_uuid_including_vuln_id_1 ON vulnerability_occurrences USING btree (uuid) INCLUDE (vulnerability_id); -- vulnerability_reads (over_limit) CREATE UNIQUE INDEX index_vulnerability_reads_on_uuid ON vulnerability_reads USING btree (uuid); ``` Payload sizing per entry (before page overhead): | Key | Bytes | Ratio | |---|---|---| | Today: `uuid` INCLUDE `vulnerability_id` | 16 + 8 = 24 | 1.0x | | **Option B**: `(uuid, sptc_id)` INCLUDE `vulnerability_id` | 16 + 8 + 8 = 32 | ~1.3x | | Option A: full natural key | 8+8+2+8+(20-32) + 8 = 54-66 | ~2.5x | On an `over_limit` table that difference is the whole argument. Option A's index would likely add tens of GB and needs `Migration/PreventIndexCreation` to be overridden. Things that must be handled either way: 1. **NULL contexts.** `security_project_tracked_context_id` is nullable on `vulnerability_occurrences`. A plain composite unique index treats NULLs as distinct, which would silently disable dedup for untracked rows. Use `nulls_not_distinct: true`, or backfill NOT NULL first. The tmp indexes `tmp_idx_vuln_occurrences_on_project_id_sec_prj_trck_cnxt_id_id` and `tmp_idx_vuln_reads_on_project_id_sec_prj_trck_cnxt_id_id` (both 19.2) suggest that backfill is already being staged — coordinate rather than duplicate it. 2. **Duplicate pre-check is mandatory.** A unique index build fails outright on any duplicate. Rows that are uuid-distinct but key-identical demonstrably exist — that is precisely the failure mode `OverrideUuidsService` and `UpdateVulnerabilityUuids` produce when takeover does not fire (and what the `vulnerability_redirects` work is remediating). Needs a detect-and-merge BBM across two releases before the constraint goes on. 3. **Partitioning collision.** `partition_id` was added to both tables (`20251112135606`, `20251112135750`) with composite FKs being prepared (`fk_rails_c8661a61eb_p`). Once these are partitioned, **every unique index must include the partition key** — so the target is really `(partition_id, uuid, sptc_id)`. Doing an identity migration and a partitioning migration on the same >100 GB table concurrently is a scheduling conflict; sequence them. 4. **Index budget.** 14 indexes on `vulnerability_occurrences`; the guideline caps at 15. We would be at 15 during the swap window. Plan the old-index drop in the same milestone. One piece of good news for Options A/C: the natural-key lookup index mostly exists already — `index_vulnerability_occurrences_for_override_uuids_logic (project_id, report_type, location_fingerprint)`. It is missing only `primary_identifier_id` and the context. ## 7. Other impacts - **Two PL/pgSQL trigger functions** embed `uuid`: `insert_or_update_vulnerability_reads_from_occurrences` and `insert_vulnerability_reads_from_vulnerability` (`db/structure.sql:1066`, `db/structure.sql:1121`). Any change to how `vulnerability_reads.uuid` is derived means rewriting both, gated behind the existing `turn_off_vulnerability_read_create_db_trigger_function` flag. - **`vulnerability_identifiers` has `UNIQUE (project_id, fingerprint)`**, so `primary_identifier_id` and `primary_identifier_fingerprint` are 1:1 per project. The substitution in a natural key is safe. Good news for Option A. - **The org is already moving this direction.** There is a whole family of `backfill_occurrence_id_to_*` BBMs (issue links, MR links, external issue links, reads, severity overrides, state transitions, representation info) replacing indirect links with real `vulnerability_occurrence_id` FKs, and `vulnerability_reads.vulnerability_occurrence_id` already has a unique index. Anything done here should land inside that programme, not beside it. - **Import/Export** recomputes the UUID on import (`ee/lib/ee/gitlab/import_export/project/relation_factory.rb:99`). It passes `tracked_context: nil`, so it already assumes v1 — Option B makes that correct rather than accidental. - **GraphQL is a breaking change under Option C only.** Option B keeps `uuid` values stable for default-branch data, so no deprecation cycle. Options A/C need one, across 4 fields and 8 mutations, plus the frontend components that pass uuids through (`bulk_change_status.vue`, `vulnerability_finding_modal.vue`, `pipeline_vulnerability_report.vue`, and the graphql fragments). - **Reindex costs:** Elasticsearch `vulnerability` + `vulnerabilities/read` documents, and the ClickHouse siphon tables, would need re-sync if UUID *values* change (A/C), not if only the constraint changes (B). ## 8. Risks, ranked 1. **Silent dedup regression -> duplicate vulnerabilities at scale.** If the new key is even slightly narrower or wider than the hash, every ingesting project starts creating or merging rows. This is the same class of bug as the `OverrideUuidsService` incident, and it is discovered weeks later. Mitigate with a shadow-compare period: compute both keys, log divergence, ship nothing until divergence is ~0. 2. **Unique index build fails on production data** because the duplicate cleanup was incomplete. Non-recoverable mid-deploy; needs `prepare_async_index` plus a verified-zero-duplicates gate. 3. **NULL-context dedup hole** (see section 6.1) — quiet, and only visible as duplicate rows. 4. **Partitioning conflict** forcing a redo of the index work. 5. **Deadlock regression.** `IngestFindings#finding_maps` sorts by uuid specifically to avoid deadlocks on concurrent ingestion (see https://gitlab.com/gitlab-org/gitlab/-/issues/603320). Changing the conflict target means changing the sort key to match, or we reintroduce that bug. 6. **Externally-held UUIDs** in `security_policy_dismissals` arrays and customer automation — Option C only. ## 9. Suggested sequencing **Milestone 1 — measure.** Add a shadow computation of `(uuid, sptc_id)` alongside today's key in ingestion; log any case where the two disagree about identity. Separately, run a read-only BBM counting rows that are uuid-distinct but natural-key-identical. Both numbers are needed before committing. **Milestone 2 — clean.** Detect-and-merge BBM for the duplicates found above, reusing the `vulnerability_redirects` mechanism so old UUIDs still resolve. **Milestone 3 — backfill.** `security_project_tracked_context_id` NOT NULL on `vulnerability_occurrences` and `vulnerability_reads` (coordinate with whoever added the 19.2 tmp indexes). **Milestone 4 — swap the constraint.** `prepare_async_index` for `UNIQUE (uuid, security_project_tracked_context_id) INCLUDE (vulnerability_id)`, verify, then the synchronous follow-up and the `unique_by` change behind a flag. Drop the old index the following milestone. **Milestone 5 — delete.** Remove `generate_v2`, `uuid_version`, `context_unaware_uuid`/`new_uuid`, `policy_comparison_uuid`, and the ClickHouse column. **Then reassess.** With the second UUID gone, check whether `UpdateVulnerabilityUuids` is still painful enough to justify Option C. ## Open question Does the VAC rollout intend `security_project_tracked_context_id` to become NOT NULL on both `vulnerability_occurrences` and `vulnerability_reads`? If not, Option B needs `nulls_not_distinct` and the sizing above changes slightly. This should be resolved before Milestone 1.