Fix stuck running scan_result_policy_violations after clean pipeline

Alan (Maciej) Paruszewskirequested to merge
fix/mrap-violation-reconciliation into master

What does this MR do and why?

A scan_result_policy_violations row can get stuck in running status forever. The merge request then shows "All policy rules must be satisfied" with a spinner and detailed_merge_status: security_policy_violations. No pipeline rerun, policy removal/re-add, or the mergeRequestResyncSecurityPolicies GraphQL mutation clears it.

The root cause is in Security::SecurityOrchestrationPolicies::UpdateViolationsService: it only deletes and upserts the exact rows its caller hands it. A row the caller cannot name is never reconciled. Two cases produce such a row:

  1. With deprecate_scan_result_policies enabled, deletes were filtered by approval_policy_rule_id only. A legacy policy source — an approval rule linked only to a scan_result_policy_read, with no approval_policy_rule — has no id there. The row ApprovalProjectRule#create_violation creates for it carries a NULL approval_policy_rule_id, so the filter can never match it. delete_violations_by_written_identity now deletes each row by the identity it was written with: approval_policy_rule_id when the source has one, scan_result_policy_id when it does not. This part is not scoped to running — it keeps the existing "this rule now passes, clear whatever status the row had" behavior. scan_result_policy_id is unique per merge request, so it cannot reach another source's row.

  2. When two scan_result_policy_reads (one per approval action) map to the same approval_policy_rule, dedupe_by_approval_policy_rule keeps one row for the upsert and drops the other. The dropped row's own pre-existing violation was never reconciled. delete_orphaned_violations now cleans those up, scoped to running rows, so an already-resolved failed/warn row is never erased as a side effect.

Deliberately not fixed: a violation excluded because its violation_data fails schema validation is left alone. The policy is still violated, it just cannot be persisted, so its running row is the correct fail-closed state. Issue 617804 lists three gaps; this fixes two of them and leaves this one on purpose.

Ships behind fix_scan_result_policy_violation_reconciliation (gitlab_com_derisk, default off, milestone 19.5).

Rollout issue: #630688 (closed)

On GitLab.com there are currently 12 running rows with a NULL approval_policy_rule_id, older than 3 days, sitting on 10 open merge requests across 8 projects. Checked two of those merge requests in a read-only production console: the current code's delete matches 0 rows, the fixed code's delete matches exactly the stuck row. Once the flag is on, these rows clear on the next evaluation of their merge request (a new pipeline or a policy resync); merge requests that never get another evaluation are cleaned up by the post-deployment migration in !257017 (merged).

Reviewable LOC: 71 (plus 105 in tests)

Context for LLM agents
  • The invalid-violation_data case is left fail-closed on purpose: the row still represents a real, unresolved violation, and clearing it without a persisted verdict would let an unenforced policy pass silently.
  • The written-identity delete is safe because scan_result_policy_id is unique per merge request — deleting by it can only ever match the row it was written for, never another source's row.
  • An earlier revision of this MR tried a fallback lookup by approval_policy_rule_id instead. That could not work: the stuck rows store NULL in that column, so there was nothing to fall back to.
  • A duplicate pair of sources for one rule with differing statuses is out of scope here; tracked in #630687.

References

Related issue: #617804

Screenshots or screen recordings

Backend-only, no UI change.

How to set up and validate locally

  1. Enable the feature flag in the rails console: 
    Feature.enable(:fix_scan_result_policy_violation_reconciliation)
  2. Set up a merge request with an approval policy approval rule whose approval_policy_rule_id is NULL (only scan_result_policy_id set), and a running violation row for it.
  3. Run an evaluation pass and confirm the row is deleted.
  4. Disable the flag and repeat the same setup: confirm the row survives instead.

Database

The new queries are batched DELETEs on scan_result_policy_violations, each scoped to one merge request. At most a few rows per merge request, so each_batch runs one batch. No migration.

Plans from Joe on gitlab-production-main (EXPLAIN (ANALYZE, BUFFERS) on a clone). merge_request_id is this MR's id; scan_result_policy_id and approval_policy_rule_id are placeholder values, so the plans return 0 rows. The filter columns and index choice match production calls.

1. Legacy source delete (by scan_result_policy_id)

Existence probe for the audit event, each_batch bounds, then the delete.

SELECT 1 AS one FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."scan_result_policy_id" = 1 AND "scan_result_policy_violations"."violation_data" IS NOT NULL LIMIT 1;
Plan · 2.934 ms · Joe command 503132 
 Limit  (cost=0.43..3.45 rows=1 width=4) (actual time=2.114..2.115 rows=0 loops=1)
   Buffers: shared hit=3 read=3
   I/O Timings: read=2.076 write=0.000
   ->  Index Scan using index_scan_result_policy_violations_on_policy_and_merge_request on public.scan_result_policy_violations  (cost=0.43..3.45 rows=1 width=4) (actual time=2.112..2.112 rows=0 loops=1)
         Index Cond: ((scan_result_policy_violations.scan_result_policy_id = 1) AND (scan_result_policy_violations.merge_request_id = 521180568))
         Filter: (scan_result_policy_violations.violation_data IS NOT NULL)
         Buffers: shared hit=3 read=3
         I/O Timings: read=2.076 write=0.000
SELECT "scan_result_policy_violations"."id" FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."scan_result_policy_id" = 1 ORDER BY "scan_result_policy_violations"."id" ASC LIMIT 1;
Plan · 0.851 ms · Joe command 503134 
 Limit  (cost=3.46..3.46 rows=1 width=8) (actual time=0.030..0.031 rows=0 loops=1)
   Buffers: shared hit=9
   I/O Timings: read=0.000 write=0.000
   ->  Sort  (cost=3.46..3.46 rows=1 width=8) (actual time=0.029..0.029 rows=0 loops=1)
         Sort Key: scan_result_policy_violations.id
         Sort Method: quicksort  Memory: 25kB
         Buffers: shared hit=9
         I/O Timings: read=0.000 write=0.000
         ->  Index Scan using index_scan_result_policy_violations_on_policy_and_merge_request on public.scan_result_policy_violations  (cost=0.43..3.45 rows=1 width=8) (actual time=0.014..0.014 rows=0 loops=1)
               Index Cond: ((scan_result_policy_violations.scan_result_policy_id = 1) AND (scan_result_policy_violations.merge_request_id = 521180568))
               Buffers: shared hit=6
               I/O Timings: read=0.000 write=0.000
SELECT "scan_result_policy_violations"."id" FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."scan_result_policy_id" = 1 AND "scan_result_policy_violations"."id" >= 1 ORDER BY "scan_result_policy_violations"."id" ASC LIMIT 1 OFFSET 1000;
Plan · 0.787 ms · Joe command 503136 
 Limit  (cost=3.46..3.47 rows=1 width=8) (actual time=0.041..0.042 rows=0 loops=1)
   Buffers: shared hit=9
   I/O Timings: read=0.000 write=0.000
   ->  Sort  (cost=3.46..3.46 rows=1 width=8) (actual time=0.040..0.040 rows=0 loops=1)
         Sort Key: scan_result_policy_violations.id
         Sort Method: quicksort  Memory: 25kB
         Buffers: shared hit=9
         I/O Timings: read=0.000 write=0.000
         ->  Index Scan using index_scan_result_policy_violations_on_policy_and_merge_request on public.scan_result_policy_violations  (cost=0.43..3.45 rows=1 width=8) (actual time=0.019..0.020 rows=0 loops=1)
               Index Cond: ((scan_result_policy_violations.scan_result_policy_id = 1) AND (scan_result_policy_violations.merge_request_id = 521180568))
               Filter: (scan_result_policy_violations.id >= 1)
               Buffers: shared hit=6
               I/O Timings: read=0.000 write=0.000
DELETE FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."scan_result_policy_id" = 1 AND "scan_result_policy_violations"."id" >= 1;
Plan · 0.834 ms · Joe command 503138 
 Delete on public.scan_result_policy_violations  (cost=0.43..3.45 rows=0 width=0) (actual time=0.019..0.020 rows=0 loops=1)
   Buffers: shared hit=6
   I/O Timings: read=0.000 write=0.000
   ->  Index Scan using index_scan_result_policy_violations_on_policy_and_merge_request on public.scan_result_policy_violations  (cost=0.43..3.45 rows=1 width=6) (actual time=0.018..0.019 rows=0 loops=1)
         Index Cond: ((scan_result_policy_violations.scan_result_policy_id = 1) AND (scan_result_policy_violations.merge_request_id = 521180568))
         Filter: (scan_result_policy_violations.id >= 1)
         Buffers: shared hit=6
         I/O Timings: read=0.000 write=0.000

2. Orphaned running rows dropped by deduplication

each_batch start, then the delete.

SELECT "scan_result_policy_violations"."id" FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."status" = 0 AND "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."scan_result_policy_id" = 1 ORDER BY "scan_result_policy_violations"."id" ASC LIMIT 1;
Plan · 0.770 ms · Joe command 503140 
 Limit  (cost=3.46..3.46 rows=1 width=8) (actual time=0.033..0.034 rows=0 loops=1)
   Buffers: shared hit=9
   I/O Timings: read=0.000 write=0.000
   ->  Sort  (cost=3.46..3.46 rows=1 width=8) (actual time=0.032..0.033 rows=0 loops=1)
         Sort Key: scan_result_policy_violations.id
         Sort Method: quicksort  Memory: 25kB
         Buffers: shared hit=9
         I/O Timings: read=0.000 write=0.000
         ->  Index Scan using index_scan_result_policy_violations_on_policy_and_merge_request on public.scan_result_policy_violations  (cost=0.43..3.45 rows=1 width=8) (actual time=0.016..0.016 rows=0 loops=1)
               Index Cond: ((scan_result_policy_violations.scan_result_policy_id = 1) AND (scan_result_policy_violations.merge_request_id = 521180568))
               Filter: (scan_result_policy_violations.status = 0)
               Buffers: shared hit=6
               I/O Timings: read=0.000 write=0.000
DELETE FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."status" = 0 AND "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."scan_result_policy_id" = 1 AND "scan_result_policy_violations"."id" >= 1;
Plan · 0.883 ms · Joe command 503142 
 Delete on public.scan_result_policy_violations  (cost=0.43..3.45 rows=0 width=0) (actual time=0.020..0.020 rows=0 loops=1)
   Buffers: shared hit=6
   I/O Timings: read=0.000 write=0.000
   ->  Index Scan using index_scan_result_policy_violations_on_policy_and_merge_request on public.scan_result_policy_violations  (cost=0.43..3.45 rows=1 width=6) (actual time=0.019..0.019 rows=0 loops=1)
         Index Cond: ((scan_result_policy_violations.scan_result_policy_id = 1) AND (scan_result_policy_violations.merge_request_id = 521180568))
         Filter: ((scan_result_policy_violations.id >= 1) AND (scan_result_policy_violations.status = 0))
         Buffers: shared hit=6
         I/O Timings: read=0.000 write=0.000

3. Delete by approval_policy_rule_id

Existing query shape, now run separately from 1.

DELETE FROM "scan_result_policy_violations" WHERE "scan_result_policy_violations"."merge_request_id" = 521180568 AND "scan_result_policy_violations"."approval_policy_rule_id" = 1 AND "scan_result_policy_violations"."id" >= 1;
Plan · 5.207 ms · Joe command 503144 
 Delete on public.scan_result_policy_violations  (cost=4.20..5.71 rows=0 width=0) (actual time=4.337..4.339 rows=0 loops=1)
   Buffers: shared hit=3 read=3
   I/O Timings: read=4.289 write=0.000
   ->  Bitmap Heap Scan on public.scan_result_policy_violations  (cost=4.20..5.71 rows=1 width=6) (actual time=4.336..4.337 rows=0 loops=1)
         Recheck Cond: ((scan_result_policy_violations.merge_request_id = 521180568) AND (scan_result_policy_violations.approval_policy_rule_id = 1))
         Filter: (scan_result_policy_violations.id >= 1)
         Buffers: shared hit=3 read=3
         I/O Timings: read=4.289 write=0.000
         ->  BitmapAnd  (cost=4.20..4.20 rows=1 width=0) (actual time=4.327..4.328 rows=0 loops=1)
               Buffers: shared hit=3 read=3
               I/O Timings: read=4.289 write=0.000
               ->  Bitmap Index Scan on index_scan_result_policy_violations_on_merge_request_id  (cost=0.00..1.95 rows=3 width=0) (actual time=4.326..4.326 rows=0 loops=1)
                     Index Cond: (scan_result_policy_violations.merge_request_id = 521180568)
                     Buffers: shared hit=3 read=3
                     I/O Timings: read=4.289 write=0.000
               ->  Bitmap Index Scan on index_scan_result_policy_violations_on_approval_policy_rule_id  (cost=0.00..1.99 rows=9 width=0) (never executed)
                     Index Cond: (scan_result_policy_violations.approval_policy_rule_id = 1)
                     I/O Timings: read=0.000 write=0.000

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.

Ran locally: ee/spec/services/security/security_orchestration_policies/update_violations_service_spec.rb, policy_rule_evaluation_service_spec.rb, update_approvals_service_spec.rb, sync_preexisting_states_approval_rules_service_spec.rb, unblock_pending_merge_request_violations_worker_spec.rb, unenforceable_policy_rules_notification_service_spec.rb — 683 examples, 0 failures. RuboCop clean on the changed files. update_violations_service_spec.rb re-run after the latest change: 60 examples, 0 failures.

Edited by Alan (Maciej) Paruszewski

Merge request reports