VAC: guarantee default tracked context exists before ingestion and reconcile findings stored mid-skew (durable fix for #612930)
Background
#612930 found that a VAC project's first default-branch pipeline drops all SAST findings, because the stored findings and the re-parsed findings end up on different UUID schemes, so the ingestion join misses and a nil deref throws NoMethodError.
!250066 (closed) ships an interim safeguard: the ingestion join is keyed by both the context-aware and context-unaware UUID, so it resolves regardless of scheme. That stops the drop without changing any stored data. It does not fix the underlying ordering hole.
This issue tracks the durable fix.
The actual invariant (corrected)
Earlier framing of this issue had the invariant backwards (it assumed default context -> v1). That is wrong. Thanks to @rwells for the correction. The correct model is:
- Context-unaware UUIDs are always v1.
- Context-aware UUIDs are v2.
- A newly created project's default context is correctly v2. It has a tracked context, so its findings can safely include
tracked_context_idin the hash. This was always the intended end state. - Pre-VAC default contexts are deliberately left on v1 so we don't have to rewrite large volumes of historical UUIDs. The
uuid_versioncolumn is the switch that lets each project use the right scheme.
So v2 on a default context is not a bug, and there is nothing to reconcile in terms of downgrading v2 default contexts to v1. context_aware_uuids_enabled? returning true on uuid_version == 2 regardless of is_default? is by design.
The real bug
It is an ordering / synchronisation problem, not a versioning flaw.
The default context is meant to be pre-created via the ProjectCreatedEvent handler (CreateOrUpdateDefaultTrackedContextWorker). When that pre-creation didn't happen (the Macy's case: either the event failed, or the project pre-dates the trigger), the project reaches its first default-branch pipeline with no default context:
- Findings get stored before any default context exists (guard short-circuits v2 generation because no context is present).
- Ingestion then find-or-creates the v2 default context mid-pipeline and re-parses the artifact.
- Stored and re-parsed UUIDs no longer line up, the join misses on every finding,
NoMethodError, all SAST findings dropped.
@lorenzvanherwaarden independently hit the same anomaly from the dismissal side on !249336 (merged) (a dismissal made on an MR branch not carrying over to the default-branch vulnerability after merge), and there is a related path in find_or_create_from_security_findings that can still produce a skew today.
Scope of the durable fix
- Guarantee the default context exists before ingestion. Since we can't assume the
ProjectCreatedEventhandler is 100% reliable (failures, old projects), add a backstop so ingestion on the default branch ensures a default context is present before findings are stored/re-parsed. This is @rwells' primary fix and closes the hole that exposes the skew. - Reconcile the findings stored mid-skew. Identify findings that were stored under the wrong scheme during the window (no-context-then-context) and repair them so stored data is internally consistent. This is a data cleanup of affected findings, not a version change on default contexts. Measure the affected population first (VAC adoption is small, beta), it may be near-trivial.
- Close the related
find_or_create_from_security_findingspath @lorenzvanherwaarden found, so we stop producing new skew today. - Regression coverage: first default-branch pipeline on a VAC project with no pre-existing default context ingests findings correctly; dismissal made pre-merge carries across to the default-branch vulnerability after merge.
Longer term
Separately from this fix, @rwells and I agree the dual-UUID design (context-aware + context-unaware) is a recurring source of headaches and worth collapsing back toward a single UUID. Ryan is prototyping approaches. Tracking that separately rather than folding it in here.
Open questions
- How many projects currently lack a default context but have default-branch findings? (Sizes the reconciliation.)
Related
- Interim fix: !250066 (closed)
- Parent bug: #612930
- Dismissal-side symptom +
find_or_create_from_security_findingspath: !249336 (merged) - Epic &3430