Re-evaluate pre-existing state approval policies on vulnerability triage

Problem

Issue #560563 (closed) (the target-branch / pre-existing case): when a vulnerability is triaged (dismissed, resolved, confirmed, reverted) or its severity is changed, the merge request approval policy is not re-evaluated if the policy rule targets only pre-existing vulnerability states (detected, confirmed, dismissed, resolved). The merge request stays blocked until a new pipeline runs or a commit is pushed, even though every relevant vulnerability has been triaged.

Reproduced on a local GDK across the dismiss-location × policy-target matrix. Pure pre-existing-state policies stay blocked after triage in both the Vulnerability Report and the pipeline Security tab; a single push clears them. Newly-detected and mixed-state policies already clear on triage and are unchanged.

Root cause

scan_finding approval rules have two evaluators:

• UpdateApprovalsService — evaluates rules with newly-detected states. #execute filters to include_newly_detected? rules (filter_newly_detected_rules) and early-returns for pure pre-existing-state rules. This is the only evaluator reached by the dismissal-triggered SyncFindingsToApprovalRulesWorker path.
• SyncPreexistingStatesApprovalRulesService — evaluates pre-existing-state rules. It is enqueued only by MR-lifecycle events (push/refresh via MergeRequest#schedule_policy_synchronization, opened-MR policy sync), never by any vulnerability triage path.

So triage never re-evaluates pre-existing-state rules. That is also why pushing a commit ("empty commit" workaround) clears the block: the push triggers the pre-existing evaluator.

Pre-existing-state rules are scoped to the project's vulnerabilities, not to a single pipeline or merge request, so the affected MRs cannot be resolved from the triaged record alone.

Change

• Add Security::ScanResultPolicies::SyncProjectPreexistingStatesApprovalRulesWorker: a project-scoped, deduplicated (until_executing, including_scheduled) worker that iterates open merge requests in the project and, for those carrying a scan_finding approval rule, enqueues the existing per-MR SyncPreexistingStatesApprovalRulesWorker (which filters out newly-detected-only rules and is idempotent). Only scan_finding rules depend on vulnerability state, so license_scanning is intentionally excluded. Guarded by a cheap project.approval_policies.exists? check so projects without policies do no work.
• Add the Security::ScanResultPolicies::SyncOnVulnerabilityStateChange concern that schedules the project worker (1-minute delay, so a burst of triage collapses into one sweep), gated by the reevaluate_preexisting_states_on_vulnerability_triage flag.
• Include the concern in the triage services: Vulnerabilities::DismissService (guarded on @changed so a no-op/rolled-back dismissal does not enqueue the sweep), Vulnerabilities::BulkDismissService, Vulnerabilities::BaseStateTransitionService (Resolve / Confirm / RevertToDetected), and Security::Findings::DismissService (pipeline Security tab).
• Extend to severity changes: Vulnerabilities::BulkSeverityOverrideService and Security::Findings::SeverityOverrideService (the latter only when the severity actually changes).

Database

The sweep iterates project.merge_requests.opened.with_approval_rules_by_report_types([:scan_finding]) in batches (each_batch). The new MergeRequest.with_approval_rules_by_report_types scope uses a correlated EXISTS so the planner probes approval_merge_request_rules per merge request rather than materialising an instance-wide set of rule merge_request_ids.

Raw SQL:

SELECT "merge_requests".* FROM "merge_requests"
WHERE "merge_requests"."target_project_id" = $1
  AND "merge_requests"."state_id" = 1
  AND EXISTS (
    SELECT "approval_merge_request_rules".*
    FROM "approval_merge_request_rules"
    WHERE "approval_merge_request_rules"."report_type" = 4
      AND "approval_merge_request_rules"."merge_request_id" = "merge_requests"."id")
ORDER BY "merge_requests"."id" ASC
LIMIT 1000;

Query plan (local GDK; shape is the relevant part):

Limit
  ->  Nested Loop Semi Join
        Join Filter: (merge_requests.id = approval_merge_request_rules.merge_request_id)
        ->  Index Scan using idx_mrs_on_target_id_and_created_at_and_state_id on merge_requests
              Index Cond: ((target_project_id = $1) AND (state_id = 1))
        ->  Index Only Scan using scan_finding_approval_mr_rule_index_mr_id_and_created_at
              on approval_merge_request_rules

The outer relation is index-backed on (target_project_id, state_id); the correlated EXISTS resolves through an Index Only Scan on scan_finding_approval_mr_rule_index_mr_id_and_created_at (approval_merge_request_rules (merge_request_id, created_at)). No materialisation, no sequential scan, regardless of how many open merge requests the project has. A policy attaches only a small number of rules per MR.

/cc database reviewer for the scope addition (ee/app/models/ee/merge_request.rb).

Feature flag

reevaluate_preexisting_states_on_vulnerability_triage (gitlab_com_derisk, default disabled, actor: project). Behind the flag because the project-wide sweep fans out to open MRs; the per-MR worker is deduplicated and the sweep is deduplicated per project to bound the cost.

Testing

• Worker spec: enqueues the per-MR worker only for open MRs in the project with scan_finding rules; skips MRs without rules, closed MRs, and other projects; no-op when the project is missing or has no approval policy; idempotent.
• Concern spec: schedules the worker with the delay; no-op when the project is nil or the flag is disabled.
• Scope spec asserts the correlated EXISTS shape.
• Shared example syncs pre-existing state approval policy rules on triage covering both flag states, reused across the dismiss, resolve, bulk-dismiss, finding-dismiss, bulk-severity-override, and finding-severity-override service specs.

Related

• Related to #560563 (closed)
• Supersedes the closed !240608 (closed), !240937 (closed), and !241286 (closed), which addressed adjacent hypotheses but routed through UpdateApprovalsService and so did not re-evaluate pre-existing-state rules.

🤖 Generated with Claude Code