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.
-
Create a project and three users, for example
alice,bob, andcarol. -
Add
aliceandbobas Developers to the project. Keepcarola non-member for now. (Policy validation requires each action'sapprovals_requiredto be covered by eligible approvers at save time, so the lane approvers must already be members.) -
Create a merge request approval policy (Secure > Policies > New policy > Merge request approval policy) with an
any_merge_requestrule on protected branches and tworequire_approvalactions:- Action 1: approvers
aliceandcarol, 1 approval required - Action 2: approver
bob, 1 approval required
- Action 1: approvers
-
Merge the auto-created MR in the security policy project and let the policy sync.
-
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 -
Add
carolas a Developer. This eventually runsAuthorizedProjectUpdate::ProjectRecalculatePerUserWorker, which publishesProjectAuthorizations::AuthorizationsAddedEventand triggersSecurity::ScanResultPolicies::AddApproversToRulesWorker. The authorization refresh is batched — allow 1-2 minutes. -
Re-run the console query from step 5:
- Before this fix:
carolis added to both rules ([[0, 0, ["alice", "carol"]], [0, 1, ["bob", "carol"]]]). - With this fix:
carolis added only to action 1's rule ([[0, 0, ["alice", "carol"]], [0, 1, ["bob"]]]).
- Before this fix:
-
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
AuthorizationsAddedEventwhile theirproject_authorizationsrow 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
ApprovalProjectRulesUserrecords 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