Rule deletion in security policy sync can collide with an orphaned tombstone rule and silently block all further syncs for that policy type

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Collaborate/take over this issue

Summary

A rule deletion in UpdateSecurityPoliciesService can pick a tombstone index that a previous deletion already used. Postgres rejects the write with a unique-index violation, and because the whole policy-type sync for a security policy configuration runs in one transaction, that single collision rolls back every policy in the batch — including unrelated ones with valid pending changes. Nothing tells the user. The job just retries and fails forever.

Steps to reproduce

1. Create a security policy configuration with two rule-bearing policies of the same type (for example, scan execution): Policy A and Policy B.
2. Give Policy A two rules, at rule_index 0 and 1.
3. Remove the rule at index 1 from Policy A's YAML and sync. The service tombstones that row: rule_index becomes -1.
4. Add a rule back to Policy A, so it again has live rules at 0 and 1, alongside the tombstone at -1.
5. Remove a rule from Policy A again and sync.

What is the current bug behavior

Step 5 raises ActiveRecord::RecordNotUnique (PG::UniqueViolation) on the unique index over (security_policy_id, rule_index). The service computes the new tombstone slot from the current maximum absolute rule_index, which is 1, and writes -1 — the exact slot the earlier tombstone already holds.

Security::PersistPolicyService#execute wraps the sync for an entire policy type on a configuration in a single ApplicationRecord.transaction. The collision rolls that transaction back, so Policy B's unrelated, valid changes are discarded along with it. The policy YAML in the security policy project still looks correct. The UI shows no error. The only trace is a Sidekiq exception, and the worker retries and fails on the same collision indefinitely.

What is the expected correct behavior

A rule deletion should always land on an index that no other rule of that policy — live or tombstoned — currently occupies, regardless of how many prior deletions ran. A data problem in one policy must not block the sync of other policies sharing the same worker run.

Root cause

mark_rules_for_deletion in ee/app/services/security/security_orchestration_policies/update_security_policies_service.rb:

def mark_rules_for_deletion(policy, deleted_rules)
  new_index = policy.max_rule_index || 1
  deleted_rules.each_with_index do |rule_diff, index|
    rule_record = rule_diff.from
    rule_record.update!(rule_index: -(new_index + index))
  end
end

max_rule_index returns the maximum absolute rule_index across all rules of the policy, live and tombstoned. The first rule in a deletion batch is written to -max_rule_index — exactly on the boundary of the existing range, not past it. If an earlier tombstone already sits at that value, the update collides.

The codebase already gets this right one level up, for policies rather than rules, in the same model file (ee/app/models/security/policy.rb):

def self.next_deletion_index
  (maximum("ABS(policy_index)") || 0) + 1
end

The + 1 moves past the existing maximum instead of landing on it. PersistPolicyService#mark_policies_for_deletion and #update_rearranged_policies use this method and don't have the bug.

Impact

The blast radius is bigger than the one policy that triggers the collision. Because all policies of a given type on a configuration sync inside one transaction, one policy hitting this bug silently blocks every other policy of that type on the same configuration from having its changes take effect — with no error surfaced anywhere a user would see it. In production this has shown up as a job retrying dozens of times against the same (security_policy_id, rule_index) pair over weeks, while a completely different policy's scope changes quietly never applied.

Proposed fix

Change the index calculation in mark_rules_for_deletion to match next_deletion_index:

new_index = policy.max_rule_index + 1

This stops new collisions. It does not retroactively fix rows that are already stuck on a colliding index from before the fix; those may need a separate cleanup pass.

A fix merge request is linked below.