Approval policy violations can stay stuck in running status after clean pipeline, permanently blocking merge requests

Summary

A merge request approval policy (scan_finding rule) can leave a Security::ScanResultPolicyViolation row permanently stuck in status: running, blocking the merge request indefinitely via detailed_merge_status: security_policy_violations — even when the policy is configured as enforcement_type: warn with fallback_behavior: { fail: open }, a combination that is documented as non-blocking.

Root cause: Security::SecurityOrchestrationPolicies::UpdateViolationsService only ever deletes/upserts the exact rows its caller explicitly hands it in a given evaluation run — it never enumerates and reconciles a merge request's actual pre-existing scan_result_policy_violations rows. Three concrete gaps let a policy source get evaluated (pass!/fail!/skip!) yet still leave its running row untouched forever:

  1. Dedup drops rows silently. When two Security::ScanResultPolicyReads (one per approval action under the same rule) map to the same approval_policy_rule, dedupe_by_approval_policy_rule keeps only one row for the upsert. The dropped row's pre-existing running state was never reconciled.
  2. Invalid violation_data skips the upsert but not a delete. When a violation record fails schema validation, it's logged and excluded from the upsert batch, but the pre-existing row for that policy is left completely untouched.
  3. Legacy sources are unreachable by the delete path. Under deprecate_scan_result_policies, deletes filter on approval_policy_rule_id. A legacy source (an approval rule linked only to a scan_result_policy_read) writes its row with a NULL approval_policy_rule_id, so the filter never matches it. A resolved_approval_policy_rule fallback in the delete path does not help: the stuck rows store NULL in that column, so there is nothing to fall back to.

Steps to reproduce

  1. Configure a group- or project-level approval_policy with a scan_finding rule, and give the rule two require_approval actions (for example, two different approver groups). This produces two ScanResultPolicyRead rows that both resolve to the same canonical ApprovalPolicyRule.
  2. Open a merge request whose pipeline produces a finding matching the rule — this creates a violation row for the rule, keyed by the higher of the two scan_result_policy_ids (whichever one happens to be selected first by the pre-existing row-creation logic).
  3. Push a new commit whose pipeline is now clean (no matching findings).
  4. Observe: the evaluation run selects the lower scan_result_policy_id as canonical (existing dedup tie-break) and either upserts or removes a violation for that ID — but the pre-existing row under the higher ID from step 2 is never touched.

Actual behavior

The stale running row from step 2 remains forever. The merge request stays on "Checking if merge request can be merged" / "All policy rules must be satisfied" indefinitely, with no further evaluation pass able to clear it, since every subsequent run reconciles by the same (lower) canonical ID.

Expected behavior

Every scan_result_policy_violations row belonging to a merge request should be reconcilable by a normal evaluation pass. A running row should never survive indefinitely once its owning rule has been re-evaluated, especially not when the policy is warn + fail: open (i.e. explicitly configured to be non-blocking).

Proposed fix

!250868 (merged) fixes gaps 1 and 3. Gap 2 is left fail-closed on purpose: the policy is still violated, the verdict just cannot be persisted.

  • Gap 3: each row is deleted by the identity it was written with, approval_policy_rule_id when the source has one and scan_result_policy_id otherwise.
  • Gap 1: running rows for sources dropped by deduplication are deleted. Resolved failed/warn rows are left alone.

Ships behind fix_scan_result_policy_violation_reconciliation (gitlab_com_derisk, default off), rollout in #630688 (closed). Existing stuck rows are cleaned up by !257017 (merged). Mixed-status deduplication is tracked in #630687.

Related

  • #535159 (closed) (closed) — same user-visible "stuck on All policy rules must be satisfied" symptom, different/unconfirmed root cause, no fix landed
  • gitlab-com/request-for-help#5212 — related symptom class (stuck running violations blocking merge), different root cause: a fork-MR creation race in SyncAnyMergeRequestApprovalRulesWorker for any_merge_request rules, not the scan_finding/UpdateViolationsService path described here
Edited by Alan (Maciej) Paruszewski