Fix MR approval policy not unblocking after vulnerabilities are dismissed

Alan (Maciej) Paruszewskirequested to merge
duo-edit-20260615-085127 into master

Problem

Issue #560563 (closed): MR approval policies keep blocking an MR even after the relevant vulnerabilities are dismissed. Developers are forced to re-run the pipeline for the policy to clear. There are two distinct causes behind the reports, addressed together here.

Root cause

Cause A — UUID-cascade mismatch (re-run does NOT unblock; tracked-context / VAC projects). Policy evaluation now compares findings using the context-unaware UUID (COALESCE(context_unaware_uuid, uuid) via Security::Finding#distinct_context_unaware_uuids), but the downstream consumers VulnerabilitiesCountService and UpdateApprovalsService#uuids_matching_policy_states still looked reads up by vulnerability_reads.uuid, which stores the context-aware UUID. When a tracked context makes the two UUIDs diverge, the dismissed/pre-existing read can no longer be matched, the count comes back as 0, and the violation data is wrong, leaving the MR blocked. This affects rules with previously-existing / mixed vulnerability states.

Cause B — stale trigger pipeline (re-run DOES unblock). DismissService triggers re-evaluation against the dismissed finding's scan pipeline. When that pipeline's security report artifacts have expired/been erased (common on older MRs), UpdateApprovalsService#execute silently early-returned with "No security reports found for the pipeline", leaving the rule untouched. A manual pipeline re-run regenerates artifacts, which is why the re-run "fixes" it.

Changes

  • Cause A: Add Vulnerabilities::Read.by_uuid_or_context_unaware_uuid scope and matching_context_unaware_uuids helper that resolve reads through the finding cascade (new_uuid / context_unaware_uuid). Route VulnerabilitiesCountService and uuids_matching_policy_states through them so counts and violation data stay correct when the UUID variants diverge.
  • Cause B: UpdateApprovalsService now resolves the evaluation pipeline: when the supplied pipeline cannot store security reports but the merge request's current head pipeline can, it evaluates against the head pipeline instead of aborting, keeping dismissal-triggered re-evaluation in sync with what a manual re-run would compare.

Testing

  • Vulnerabilities::Read scope + helper specs for the context-aware vs context-unaware lookup.
  • VulnerabilitiesCountService regression spec for divergent UUIDs (previously returned count: 0).
  • UpdateApprovalsService spec for the head-pipeline fallback, plus the divergent-context new_dismissed coverage.
  • End-to-end dismiss_service_reevaluation_integration_spec.rb: dismissing a finding re-evaluates the rule without a pipeline re-run.

Database review

This MR adds one new scope, Vulnerabilities::Read.by_uuid_or_context_unaware_uuid, in ee/app/models/vulnerabilities/read.rb. No migrations, no bulk writes, read-only.

The scope matches reads either by their own context-aware uuid, or via the related finding's context-unaware new_uuid:

scope :by_uuid_or_context_unaware_uuid, ->(uuids) do
  where(uuid: uuids).or(
    where(
      vulnerability_occurrence_id: Vulnerabilities::Finding.where(new_uuid: uuids).select(:id)
    )
  )
end

Generated SQL

SELECT "vulnerability_reads".*
FROM "vulnerability_reads"
WHERE (
  "vulnerability_reads"."uuid" IN ($1, $2, ...)
  OR "vulnerability_reads"."vulnerability_occurrence_id" IN (
    SELECT "vulnerability_occurrences"."id"
    FROM "vulnerability_occurrences"
    WHERE "vulnerability_occurrences"."new_uuid" IN ($1, $2, ...)
  )
);

In practice the scope is always chained after a project/pipeline scope (e.g. by_projects(...).by_uuid_or_context_unaware_uuid(uuids)), so the outer table is constrained by project_id before this predicate is applied, and uuids is bounded by the finite set of findings in a single pipeline comparison.

Indexes used

  • vulnerability_reads.uuid is backed by the unique index index_vulnerability_reads_on_uuid (uuid is unique per read), serving the first branch and the vulnerability_occurrence_id lookup.
  • The subquery filters vulnerability_occurrences.new_uuid, backed by index_vulnerability_occurrences_on_uuid_and_new_uuid (or the dedicated new_uuid index), so the inner SELECT id is index-only / index scan.

Execution plan

Note: this is the expected plan shape; a maintainer with production-like data should confirm via the Database Lab / @gitlab-bot plan on a representative dataset. Requesting database review.

Bitmap Heap Scan on vulnerability_reads
  Recheck Cond: ((uuid = ANY ('{...}')) OR (vulnerability_occurrence_id = ANY (...)))
  ->  BitmapOr
        ->  Bitmap Index Scan on index_vulnerability_reads_on_uuid
              Index Cond: (uuid = ANY ('{...}'))
        ->  Bitmap Index Scan on index_vulnerability_reads_on_vulnerability_occurrence_id
              Index Cond: (vulnerability_occurrence_id = ANY (
                    SubPlan: Index Scan on index_vulnerability_occurrences_on_new_uuid
                      Index Cond: (new_uuid = ANY ('{...}'))))

Both branches resolve through indexes; no sequential scans on vulnerability_reads or vulnerability_occurrences. The uuids array is bounded per evaluation, so the = ANY lists stay small.

Related Issues

Edited by Alan (Maciej) Paruszewski

Merge request reports