Fix approvers leaking across rules from multi-action approval policies

What does this MR do and why?

Fixes #607408 (closed)

Security::ScanResultPolicies::AddApproversToRulesService backfills approvers onto policy-generated approval rules when project authorizations change (for example, a new member joins). It matched rules by orchestration_policy_idx alone, but a policy with multiple require_approval actions generates one rule per action, and all of them share that index. A user who was an approver in only one action was therefore added to every rule of the policy, letting a single approval satisfy approval lanes that were configured to be independent (undermining separation of duties).

This MR matches and groups by the [orchestration_policy_idx, approval_policy_action_idx] pair, mirroring how Security::ScanResultPolicies::ApprovalRules::BaseService keys existing rules. Rule creation during policy sync was already correct; only the backfill path was affected.

Screenshots or demo

N/A — backend-only fix, no UI changes.

How to set up and validate locally

Prerequisites: GDK with an Ultimate license.

  1. Create a project and three users, for example alice, bob, and carol.

  2. Add alice and bob as Developers to the project. Keep carol a non-member for now. (Policy validation requires each action's approvals_required to be covered by eligible approvers at save time, so the lane approvers must already be members.)

  3. Create a merge request approval policy (Secure > Policies > New policy > Merge request approval policy) with an any_merge_request rule on protected branches and two require_approval actions:

    • Action 1: approvers alice and carol, 1 approval required
    • Action 2: approver bob, 1 approval required
  4. Merge the auto-created MR in the security policy project and let the policy sync.

  5. Confirm the two generated rules are correctly scoped (rails console):

    project = Project.find_by_full_path('<group>/<project>')
    project.approval_rules.map do |rule|
      [rule.orchestration_policy_idx, rule.approval_policy_action_idx, rule.users.map(&:username)]
    end
    #=> [[0, 0, ["alice"]], [0, 1, ["bob"]]]   # carol is absent: not yet a member
  6. Add carol as a Developer. This eventually runs AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker, which publishes ProjectAuthorizations::AuthorizationsAddedEvent and triggers Security::ScanResultPolicies::AddApproversToRulesWorker. The authorization refresh is batched — allow 1-2 minutes.

  7. Re-run the console query from step 5:

    • Before this fix: carol is added to both rules ([[0, 0, ["alice", "carol"]], [0, 1, ["bob", "carol"]]]).
    • With this fix: carol is added only to action 1's rule ([[0, 0, ["alice", "carol"]], [0, 1, ["bob"]]]).
  8. Optional UI check: open a merge request targeting a protected branch. The approvals widget shows the two rules separately (<policy name> - Action 1/2); eligible approvers per rule follow the same scoping.

Notes for repeat runs:

  • Removing and quickly re-adding the same user does not fire a new AuthorizationsAddedEvent while their project_authorizations row still exists (the removal is also asynchronous). After removing the member, wait until the row is gone before re-adding.
  • Reset between runs by deleting the user's ApprovalProjectRulesUser records and removing the membership.

Alternatively, run the regression specs (they fail without the fix):

bundle exec rspec ee/spec/services/security/scan_result_policies/add_approvers_to_rules_service_spec.rb
Edited by Dominic Bauer

Merge request reports

Loading
Loading