Resolve finding UUID scheme skew in ingestion join (#612930)

Gregory Havengarequested to merge
ghavenga/612930-default-context-uuid-v1 into master

What does this MR do

Interim safeguard for #612930, where a VAC project's first default branch pipeline drops all of a scan's findings.

This is a read-time join tolerance fix. It changes no stored data and does not change the UUID versioning rule. The durable fix is tracked in #613547.

The bug

On a VAC-enabled project whose default branch does not yet have a tracked context, an ordering asymmetry between the storage and ingestion phases produces a UUID scheme mismatch:

  1. The storage phase runs first and resolves the tracked context with a find-only lookup (ProjectTrackedContext.for_pipeline(pipeline).tracked.first). With no default context yet, that returns nil, so the security findings are stored with context-unaware (v1) UUIDs.
  2. The ingestion phase then find-or-creates the default branch tracked context mid-pipeline. A newly created default context is correctly stamped uuid_version = 2 (context-aware), so the re-parsed report findings are computed with context-aware (v2) UUIDs.
  3. FindingMapCollection keyed the report findings map by the context-aware uuid only, so the lookup missed for every finding, report_finding came back nil, FindingMap raised an ActiveSupport::DelegationError (a NoMethodError subclass), and the whole slice was dropped and recorded as a generic IngestionError. The SAST findings silently vanished from the report (secret_detection was unaffected). This matches the customer report (Zendesk 742858).

The root cause is the store-vs-ingest asymmetry: the storage phase reads the context (find-only, gets nil) while the ingestion phase creates it (v2), so the two phases disagree on the UUID scheme for the same findings. This is not a versioning flaw. A newly created default context being v2 is the intended end state.

Runtime confirmed against master in a local GDK.

What this changes

report_findings_map is now keyed by both the context-aware uuid and the context_unaware_uuid that the report finding already carries. The join then resolves regardless of which scheme each side used:

  • first-pipeline v1-stored findings now resolve (were dropped)
  • later v2-stored findings still resolve
  • a finding that matches under neither scheme still returns nil, so genuine mismatches are not hidden

The map is built per security scan, so every report finding in it belongs to a single pipeline on a single ref, i.e. a single context. There is no cross-context mixing, so the dual-key lookup cannot resolve a finding to another context's report finding. It changes no stored data and does not alter the UUID versioning rule, so it is safe for data already written.

Why interim, and what the durable fix is

This stops the drop today without touching stored data or the versioning rule. It does not close the underlying ordering hole (storage resolving no context while ingestion creates one), which is the durable fix tracked in #613547.

The agreed durable direction is a permissive legacy fallback: make the ingestion phase find-only like the storage phase, so a project without a tracked default context resolves to nil in both phases and both compute v1. The default branch stays on legacy v1 / NULL-context ingestion until it is legitimately tracked (via the ProjectCreatedEvent handler or explicit tracking), rather than having a v2 context conjured mid-ingestion. That fix is gated behind a feature flag and shipped separately.

Separately, @rwells is exploring collapsing the dual-UUID design back toward a single UUID (#613600); that would eventually make this interim join and the versioning rule itself obsolete.

What this does not fix

  • Cross-context dismissal carry-over (the !249336 (merged) symptom): a dismissal made on an MR branch not carrying over to the default-branch vulnerability after merge. That is a separate auto-state-transition context-scoping problem, not the ingestion join.
  • Findings already dropped on affected projects. Subsequent default-branch scans re-detect and ingest them once this is in place, so no repair migration is needed.

Test coverage

Adds a regression spec to finding_map_collection_spec.rb that reproduces the v1-vs-v2 skew (stored finding keyed v1, report finding carrying a v2 uuid but a matching context_unaware_uuid) and asserts the finding still maps to its report finding.

ee/spec/services/security/ingestion/finding_map_collection_spec.rb: 5 examples, 0 failures. RuboCop clean.

Related

Edited by Gregory Havenga

Merge request reports