Calibrate untracked-branch finding UUIDs to default-branch record

Alan (Maciej) Paruszewskirequested to merge
5341-untracked-branch-uuid-calibration into master

What does this MR do and why?

Fixes the UUID of a finding differing between the default branch pipeline and pipelines on untracked branches.

OverrideInBatch#same_context_finding_from picks the existing Vulnerabilities::Finding that a report finding's uuid is calibrated to. Since 86af5ed9 it only accepts a finding whose project_tracked_context id equals the pipeline's tracked context id.

Before:

  • Pipelines on untracked branches, such as every merge request pipeline whose source branch is not tracked, have a nil tracked context. They match existing findings that also have a nil tracked context: the legacy default-branch findings created before tracked contexts existed and not yet backfilled.
  • If a project has duplicate findings for one location (a legacy nil-context record and a default-context record), the default branch pipeline calibrates to the default-context record and the merge request pipeline calibrates to the legacy record.
  • The same finding gets two UUIDs. The merge request security widget and scan_finding approval policies compare context_unaware_uuid || uuid. Legacy records have no context_unaware_uuid, and override_context_unaware_uuid_for copies that nil onto both sides, so the comparison falls back to uuid and shows the finding as both newly detected and fixed on every merge request. This is deterministic, not flaky.

After:

  • When the pipeline has no tracked context, same_context_finding_from prefers the existing finding in the default-branch context. If there is none, it falls back to the previous exact-context match, which covers legacy nil records.
  • Default-branch and tracked-branch pipelines are unchanged.
  • An untracked pipeline whose only match is a default-context record now takes that record's UUID instead of keeping its generated one. That is the record the default branch pipeline calibrates to, so both sides agree.

Checked read-only against production data, on a project with three such duplicate pairs. For all three pairs, the current code picks the legacy record for merge request pipelines and the default-context record on the default branch. The patched selection picks the default-context record for both.

Specs: a new context in ee/spec/services/security/override_uuids_service_spec.rb, "when duplicated existing findings exist with no tracked context and on the default branch", with two examples (default branch pipeline, untracked branch pipeline). The untracked one fails without the fix.

There is no feature flag. The change only affects untracked-branch pipelines, and only makes them calibrate to the record the default branch already uses. Default-branch and tracked-branch ingestion is untouched.

References

  • Duplicate findings: #601032
  • Earlier fix that did not help here (sorted candidates by id; each side had only one candidate in its own context): !253532 (merged)
  • Commit that introduced the context match: 86af5ed9, "Fix UUID calibration with multiple branch tracking"

Screenshots or screen recordings

N/A. Backend change only.

How to set up and validate locally

bundle exec rspec ee/spec/services/security/override_uuids_service_spec.rb
bundle exec rspec ee/spec/services/security/override_uuids_service/override_in_batch_spec.rb
bundle exec rspec ee/spec/services/security/store_scan_service_spec.rb

Results on this branch: 18, 4 and 41 examples, 0 failures. RuboCop is clean.

To see the bug, revert the change in same_context_finding_from and run the first command. The untracked-branch example in the new context fails.

Owed elsewhere

Cleaning up the duplicate records stays tracked in #601032. This MR stops the UUID split; it does not remove the duplicates.

Reviewable LOC: 7 (plus 65 in tests)

Context for LLM agents

Invariant: an untracked-branch pipeline must calibrate a finding to the same existing record the default branch pipeline does.

Rejected alternatives:

  • Treat nil-context legacy records as default-context in the default branch pipeline too. This would move default branch calibration to the older legacy record, which is often resolved, and churn vulnerability states on every affected project.
  • Stop copying a nil context_unaware_uuid, so comparison falls back to matching context_unaware_uuid. This leaves the UUID split, and merge request findings still link to the resolved legacy vulnerability.
  • Clean up the duplicates only. Still owed via 601032, but it does not stop the next project from hitting this.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports