From f172ec9bfc56f8dce7957eea330bbdc3447b0b0a Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Mon, 20 Oct 2025 12:17:39 +0600 Subject: [PATCH 1/2] Gate scan result policy rules by license Scan result policy approval rules should only be applied if the security_orchestration_policies feature is licensed. This change adds a check to prevent synchronization of these rules when the feature is not available, ensuring correct feature gating. Specs are updated to cover this licensing scenario. --- ee/app/models/ee/merge_request.rb | 3 +++ ee/spec/models/merge_request_spec.rb | 17 +++++++++++++++++ .../ee/merge_requests/update_service_spec.rb | 4 ++++ .../sync_report_approver_approval_rules_spec.rb | 17 ++++++++++++++++- 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 73485c3de2bf3f..733e5115e000f3 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -472,6 +472,9 @@ def synchronize_approval_rules_from_target_project project_rules = target_project.approval_rules.report_approver.includes(:users, :groups) project_rules.find_each do |project_rule| + next if project_rule.from_scan_result_policy? && + !project.licensed_feature_available?(:security_orchestration_policies) + project_rule.apply_report_approver_rules_to(self) do |rule_attributes| rule_attributes[:approvals_required] = 0 if project_rule.from_scan_result_policy? end diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb index 3d7ea7c59e64df..e622be7fbb4e26 100644 --- a/ee/spec/models/merge_request_spec.rb +++ b/ee/spec/models/merge_request_spec.rb @@ -2188,12 +2188,29 @@ def create_mr(metrics_data = {}) subject { merge_request.synchronize_approval_rules_from_target_project } + before do + stub_licensed_features(security_orchestration_policies: true) + end + it 'resets security rules approvals but keeps code_coverage rule approvals' do subject expect(merge_request.approval_rules.map { |rule| [rule.report_type, rule.approvals_required] }) .to match_array([['license_scanning', 0], ['code_coverage', 2], ['scan_finding', 0], ['any_merge_request', 0]]) end + + context 'when the security orchestration policies feature not available' do + before do + stub_licensed_features(security_orchestration_policies: false) + end + + it 'does not create approval rules from scan result policies' do + subject + + expect(merge_request.approval_rules.map { |rule| [rule.report_type, rule.approvals_required] }) + .to match_array([['code_coverage', 2]]) + end + end end describe '#sync_project_approval_rules_for_policy_configuration' do diff --git a/ee/spec/services/ee/merge_requests/update_service_spec.rb b/ee/spec/services/ee/merge_requests/update_service_spec.rb index a7a92c3f8eb4b6..eed436dc90e1cb 100644 --- a/ee/spec/services/ee/merge_requests/update_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/update_service_spec.rb @@ -553,6 +553,10 @@ def update_merge_request(opts) approvals_required: 0) end + before do + stub_licensed_features(security_orchestration_policies: true) + end + context 'with target branch change' do let(:opts) { { target_branch: 'feature-2' } } diff --git a/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb b/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb index a5f15d938f8c4d..ac3e96fa728356 100644 --- a/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb +++ b/ee/spec/services/merge_requests/sync_report_approver_approval_rules_spec.rb @@ -12,7 +12,7 @@ using RSpec::Parameterized::TableSyntax before do - stub_licensed_features(report_approver_rules: true) + stub_licensed_features(report_approver_rules: true, security_orchestration_policies: true) end where(:default_name, :report_type) do @@ -115,6 +115,21 @@ end end + context 'when security_orchestration_policies feature is not available' do + let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) } + let!(:coverage_project_rule) { create(:approval_project_rule, :code_coverage, project: merge_request.target_project) } + + before do + stub_licensed_features(report_approver_rules: true, security_orchestration_policies: false) + + service.execute + end + + specify { expect(merge_request.reload.approval_rules.count).to be(1) } + specify { expect(merge_request.reload.approval_rules.coverage.count).to be(1) } + specify { expect(merge_request.reload.approval_rules.license_compliance.count).to be(0) } + end + context 'when coverage_check_approval_rule is disabled' do before do stub_licensed_features(coverage_check_approval_rule: false) -- GitLab From 9297acde6d2439a0983d6d161ba4adc1d15aff14 Mon Sep 17 00:00:00 2001 From: Imam Hossain Date: Wed, 22 Oct 2025 11:19:54 +0600 Subject: [PATCH 2/2] Move license check to the beginning --- ee/app/models/ee/merge_request.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb index 733e5115e000f3..c72f8f27778da5 100644 --- a/ee/app/models/ee/merge_request.rb +++ b/ee/app/models/ee/merge_request.rb @@ -472,8 +472,8 @@ def synchronize_approval_rules_from_target_project project_rules = target_project.approval_rules.report_approver.includes(:users, :groups) project_rules.find_each do |project_rule| - next if project_rule.from_scan_result_policy? && - !project.licensed_feature_available?(:security_orchestration_policies) + next if !project.licensed_feature_available?(:security_orchestration_policies) && + project_rule.from_scan_result_policy? project_rule.apply_report_approver_rules_to(self) do |rule_attributes| rule_attributes[:approvals_required] = 0 if project_rule.from_scan_result_policy? -- GitLab