Consider branch rules on merge request approval rule and policy violation registration
What does this MR do and why?
Security approval policies can be configured with branch rules to target specific branches (e.g., branches: ['feature/*']). Currently, when a merge request is created, approval rules and violations are registered for all policies, regardless of whether the policy's branch rules match the MR's target branch. Filtering only occurs later during evaluation in ApprovalState#wrapped_rules.
This causes several issues:
- Policy Bot comments may show false positive violations for policies that do not apply to the target branch. ( fixed in #553183 (closed) )
- Unnecessary violation records are created in the database.
- UI inconsistencies, where non-applicable policies appear as unviolated (e.g., in the Approval Widget).
- False positive audit events which are generated based on policy violations
This MR introduces pre-filtering based on branch rules before registering policy violations and creating merge request rules.
When the security_policy_approval_rule_branch_filtering feature flag is enabled:
- For scan_result_policies,
ApprovalProjectRulechecksapplies_to_branch?before creating the merge request approval rule and violation. - If a policy does not apply to the target branch, no approval rule or violation is created.
- If an existing rule or violation becomes applicable or inapplicable (e.g., if the target branch changes), it is created or deleted accordingly.
Side effects
This is a backend change and the merge request approval rules as well as the security policies feature are expected to work as before. However, there is one side affect of this change.
Approval overrides are applied based on persisted violations. Previously, overrides were applied regardless of the target branch because violations were registered even when policies did not apply.
This MR corrects that behavior. Approval overrides are now enforced only when the policy applies to the merge request’s target branch according to its branch rules.
See the discussion here for more on this: #553189 (comment 2903710412)
Spec changes
The ApprovalProjectRule#apply_report_approver_rules_to method now checks whether the approval rule applies to the merge request's target branch before creating or updating the MR approval rule.
If the branch is not protected or the rule doesn't apply to that branch, the method deletes any existing rules/violations and returns nil instead of a rule object.
This change impacts all services that eventually call apply_report_approver_rules_to:
-
UpdateApprovalsService→PolicyRuleEvaluationService#save→MergeRequest#reset_required_approvals→apply_report_approver_rules_to -
GroupProjectTransferWorker→LinkProjectService→SyncMergeRequestsService→apply_report_approver_rules_to -
MergeRequests::ReopenService→resync_policies→MergeRequest#reset_required_approvals→apply_report_approver_rules_to -
MergeRequests::UpdateService→schedule_policy_synchronization→MergeRequest#sync_project_approval_rules_for_*→apply_report_approver_rules_to -
SyncReportApproverApprovalRules→MergeRequest#synchronize_approval_rules_from_target_project→apply_report_approver_rules_to -
SyncMergeRequestsService→sync_merge_request→MergeRequest#sync_project_approval_rules_for_approval_policy_rules→apply_report_approver_rules_to
All these services share a common downstream dependency on apply_report_approver_rules_to. To ensure tests pass, each affected spec was updated to:
- Create a
protected_branchmatching the merge request's target branch - Add the
:for_all_protected_branchestrait toapproval_project_rulefactories
Screenshots or screen recordings
| Before | After |
|---|---|
|
|
Database Queries
New queries:
Check if any existing violations:
SELECT 1 AS one FROM "scan_result_policy_violations" INNER JOIN "scan_result_policies" ON "scan_result_policy_violations"."scan_result_policy_id" = "scan_result_policies"."id" WHERE "scan_result_policies"."id" = 23 AND "scan_result_policy_violations"."merge_request_id" = 140 LIMIT 1
explain: https://console.postgres.ai/gitlab/gitlab-production-main/sessions/49749/commands/148126
Deleting Violations:
DELETE FROM "scan_result_policy_violations"
WHERE "scan_result_policy_violations"."id" IN (
SELECT "scan_result_policy_violations"."id"
FROM "scan_result_policy_violations"
INNER JOIN "scan_result_policies"
ON "scan_result_policy_violations"."scan_result_policy_id" = "scan_result_policies"."id"
WHERE "scan_result_policies"."id" = 23
AND "scan_result_policy_violations"."merge_request_id" = 140
);
Explain: https://console.postgres.ai/gitlab/gitlab-production-main/sessions/49749/commands/148125 (It returns 0 rows, as we can't know the scan_result_policies.id since it is not exposed through APIs)
Delete Approval Rules
DELETE FROM "approval_merge_request_rules"
WHERE ("approval_merge_request_rules"."id")
IN (
SELECT
"approval_merge_request_rules"."id"
FROM
"approval_merge_request_rules"
INNER JOIN "approval_merge_request_rule_sources" ON "approval_merge_request_rule_sources"."approval_merge_request_rule_id" = "approval_merge_request_rules"."id"
WHERE
"approval_merge_request_rules"."merge_request_id" = 16248692
AND "approval_merge_request_rules"."rule_type" = 3
AND "approval_merge_request_rule_sources"."approval_project_rule_id" = 47696360)
Query plan: https://console.postgres.aiundefined/shared/322fbb4b-07b8-459e-a879-ce6536d7dea6
References
Spike issue #553189 (closed)
How to set up and validate locally
- Enable the feature flag by running:
bin/rails runner "Feature.enable(:security_policy_approval_rule_branch_filtering)"
- Create a project with security policies
- Secret detection - protected branch
- SAST scan - feature branch
Policy YAML
.gitlab/security-policies/policy.yml
---
approval_policy:
- name: SAST scan - feature branch
description: ''
enabled: true
rules:
- type: scan_finding
scanners:
- sast
vulnerabilities_allowed: 0
severity_levels: []
vulnerability_states: []
branches:
- feature/*
actions:
- type: require_approval
approvals_required: 1
role_approvers:
- developer
- maintainer
- owner
- type: send_bot_message
enabled: true
approval_settings:
block_branch_modification: false
prevent_pushing_and_force_pushing: false
prevent_approval_by_author: true
prevent_approval_by_commit_author: false
remove_approvals_with_new_commit: false
require_password_to_approve: false
fallback_behavior:
fail: closed
- name: Secret Detection - Protected Branch
description: ''
enabled: true
enforcement_type: enforce
rules:
- type: scan_finding
scanners:
- type: secret_detection
vulnerabilities_allowed: 0
severity_levels:
- critical
- high
vulnerability_states:
- new_needs_triage
vulnerabilities_allowed: 0
severity_levels: []
vulnerability_states: []
branch_type: protected
actions:
- type: require_approval
approvals_required: 1
group_approvers_ids:
- 24
- type: send_bot_message
enabled: true
approval_settings:
block_branch_modification: false
prevent_pushing_and_force_pushing: false
prevent_approval_by_author: false
prevent_approval_by_commit_author: false
remove_approvals_with_new_commit: false
require_password_to_approve: false
fallback_behavior:
fail: closed
.gitlab-ci.yml
image: busybox:latest
include:
- template: 'Jobs/Secret-Detection.gitlab-ci.yml'
- template: 'Jobs/SAST.gitlab-ci.yml'
stages:
- test
test_job:
stage: test
script:
- echo "Running tests..."
variables:
AST_ENABLE_MR_PIPELINES: "true"
-
Navigate to
project -> settings -> Protected Branchesand add a protected branchfeature/* -
Add the following 2 files in a branch and create MR targeting
mainbranch
env.sample
AWS_TOKEN=AKIAZYONPI3G4JNCCWGQ
test.rb
job = params[:job]
eval(job)
Observation: only the Secret detection - protected branch is violated. Approval rules and policy bot comment is recorded accordingly.
- Create a feature branch (
feature/test-branch) in the repository and Change the MR target branch to it
Observation: Both (Secret detection - protected branch and SAST scan - feature branch ) policy is violated. Approval rules and policy bot comment is updated accordingly
- To test, approval rule overwrites are working correctly
- Navigate to
project -\> settings -\> Merge requests -\> Merge request approvals: Uncheck all boxes - The MR is not allowed to be approved by merge request author when
Secret detection - protected branchis violated since the policy hasprevent_approval_by_author: trueoverwrite. - Change the target branch to
main, now the MR is allowed to be approved by merge request author.
- Code coverage rules are working as expected.
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.
Related to #583612

