diff --git a/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb b/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb
index 48d6405992cc49906d3063da90cbb8cd4039ce58..09c7bd536165154b04f1010c27620d85e4087947 100644
--- a/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb
+++ b/ee/app/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service.rb
@@ -24,13 +24,15 @@ def execute
delegate :project, to: :merge_request, private: true
def sync_required_approvals
- rules_with_preexisting_states = merge_request.approval_rules.scan_finding.reject do |rule|
+ all_scan_finding_rules = merge_request.approval_rules.scan_finding
+ rules_with_preexisting_states = all_scan_finding_rules.reject do |rule|
include_newly_detected?(rule)
end
return unless rules_with_preexisting_states.any?
update_scan_finding_rules_with_preexisting_states(rules_with_preexisting_states)
+ generate_policy_bot_comment(merge_request, all_scan_finding_rules, :scan_finding)
end
def update_scan_finding_rules_with_preexisting_states(approval_rules)
@@ -47,8 +49,6 @@ def update_required_approvals(violated_rules, unviolated_rules)
log_violated_rules(violated_rules)
violations.add(violated_rules.map(&:scan_result_policy_id), unviolated_rules.map(&:scan_result_policy_id))
violations.execute
-
- generate_policy_bot_comment(merge_request, violated_rules, :scan_finding)
end
def preexisting_findings_count_violated?(approval_rule)
diff --git a/ee/app/services/security/scan_result_policies/update_approvals_service.rb b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
index 05cc922e834e94360d36fb951261ca5b559c0115..7639b724d661ac3fbc059906d2d98b16eff3466a 100644
--- a/ee/app/services/security/scan_result_policies/update_approvals_service.rb
+++ b/ee/app/services/security/scan_result_policies/update_approvals_service.rb
@@ -19,11 +19,13 @@ def execute
return if pipeline.incomplete?
return unless pipeline.can_store_security_reports?
- approval_rules = merge_request.approval_rules.scan_finding
+ all_scan_finding_rules = merge_request.approval_rules.scan_finding
- if security_policies_sync_preexisting_state_enabled?
- approval_rules = approval_rules.select { |rule| include_newly_detected?(rule) }
- end
+ approval_rules = if security_policies_sync_preexisting_state_enabled?
+ all_scan_finding_rules.select { |rule| include_newly_detected?(rule) }
+ else
+ all_scan_finding_rules
+ end
return if approval_rules.empty?
@@ -37,7 +39,7 @@ def execute
update_required_approvals(violated_rules, unviolated_rules)
violations.add(violated_rules.pluck(:scan_result_policy_id), unviolated_rules.pluck(:scan_result_policy_id)) # rubocop:disable CodeReuse/ActiveRecord
violations.execute
- generate_policy_bot_comment(merge_request, violated_rules, :scan_finding)
+ generate_policy_bot_comment(merge_request, all_scan_finding_rules, :scan_finding)
end
private
diff --git a/ee/app/services/security/sync_license_scanning_rules_service.rb b/ee/app/services/security/sync_license_scanning_rules_service.rb
index 3d29484cbe20fdae2c758d0edc27d2b21153f180..c025afa6dd4b27e54933a6cf9429901b32950dc6 100644
--- a/ee/app/services/security/sync_license_scanning_rules_service.rb
+++ b/ee/app/services/security/sync_license_scanning_rules_service.rb
@@ -53,7 +53,7 @@ def remove_required_license_finding_approval(merge_request)
log_violated_rules(merge_request, violated_rules)
violations.add(violated_rules.pluck(:scan_result_policy_id), unviolated_rules.pluck(:scan_result_policy_id)) # rubocop:disable CodeReuse/ActiveRecord
violations.execute
- generate_policy_bot_comment(merge_request, violated_rules, :license_scanning)
+ generate_policy_bot_comment(merge_request, license_approval_rules, :license_scanning)
end
def update_required_approvals(merge_request, violated_rules, unviolated_rules)
diff --git a/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb b/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb
index da9de77589755384250a313a95d7c208c18de2a2..300bd79245232579621f88e3dc1df09982958575 100644
--- a/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb
+++ b/ee/spec/services/security/scan_result_policies/sync_any_merge_request_rules_service_spec.rb
@@ -70,7 +70,7 @@
end
it_behaves_like 'sets approvals_required to 0'
- it_behaves_like 'triggers policy bot comment', :any_merge_request, false
+ it_behaves_like 'triggers policy bot comment', :any_merge_request, false, requires_approval: false
it_behaves_like 'merge request without scan result violations'
it 'does not create a log' do
@@ -87,7 +87,7 @@
end
it_behaves_like 'sets approvals_required to 0'
- it_behaves_like 'triggers policy bot comment', :any_merge_request, false
+ it_behaves_like 'triggers policy bot comment', :any_merge_request, false, requires_approval: false
it_behaves_like 'merge request without scan result violations'
end
end
@@ -201,7 +201,7 @@
end
end
- it_behaves_like 'triggers policy bot comment', :any_merge_request, false
+ it_behaves_like 'triggers policy bot comment', :any_merge_request, false, requires_approval: false
it_behaves_like 'merge request without scan result violations' do
let(:scan_result_policy_read) { scan_result_policy_read_with_commits }
end
diff --git a/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb b/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb
index 9189a83a8de07f8125bc9acf7faa81642ff3d428..636e073a5d76b1e4c6a85eb8571ac3d27a172896 100644
--- a/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb
+++ b/ee/spec/services/security/scan_result_policies/sync_preexisting_states_approval_rules_service_spec.rb
@@ -71,7 +71,7 @@
it_behaves_like 'does not log violations'
end
- context 'when rules does not contain pre-existing states' do
+ context 'when rules do not contain pre-existing states' do
let!(:approver_rule) do
create(:report_approver_rule, :scan_finding, merge_request: merge_request,
approval_project_rule: approval_project_rule, approvals_required: approvals_required,
@@ -103,7 +103,7 @@
end
it_behaves_like 'does not update approval rules'
- it_behaves_like 'triggers policy bot comment', :scan_finding, false, requires_approval: true
+ it_behaves_like 'triggers policy bot comment', :scan_finding, false
it_behaves_like 'merge request without scan result violations', previous_violation: false
it 'logs update' do
@@ -128,6 +128,28 @@
it_behaves_like 'sets approvals_required to 0'
it_behaves_like 'triggers policy bot comment', :scan_finding, false
it_behaves_like 'does not log violations'
+
+ context 'when there are other scan_finding violations' do
+ let_it_be(:scan_result_policy_read_other_scan_finding) { create(:scan_result_policy_read, project: project) }
+ let_it_be(:approval_project_rule_other) do
+ create(:approval_project_rule, :scan_finding, project: project, approvals_required: 1,
+ scan_result_policy_read: scan_result_policy_read_other_scan_finding)
+ end
+
+ let_it_be(:approver_rule_other) do
+ create(:report_approver_rule, :scan_finding,
+ merge_request: merge_request, vulnerability_states: ['new_needs_triage'],
+ approval_project_rule: approval_project_rule_other, approvals_required: 1,
+ scan_result_policy_read: scan_result_policy_read_other_scan_finding)
+ end
+
+ let_it_be(:other_violation) do
+ create(:scan_result_policy_violation, scan_result_policy_read: scan_result_policy_read_other_scan_finding,
+ merge_request: merge_request)
+ end
+
+ it_behaves_like 'triggers policy bot comment', :scan_finding, true
+ end
end
end
end
diff --git a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
index 375dc5738cc783d2bcea145c426740555f674578..b1fd897fed5c1c496273a288d352b9760def3a21 100644
--- a/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
+++ b/ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb
@@ -168,6 +168,28 @@
it_behaves_like 'sets approvals_required to 0'
it_behaves_like 'triggers policy bot comment', :scan_finding, false
+
+ context 'when there are other scan_finding violations' do
+ let_it_be(:scan_result_policy_read_other_scan_finding) { create(:scan_result_policy_read, project: project) }
+ let_it_be(:approval_project_rule_other) do
+ create(:approval_project_rule, :scan_finding, project: project, approvals_required: 1,
+ scan_result_policy_read: scan_result_policy_read_other_scan_finding)
+ end
+
+ let_it_be(:approver_rule_other) do
+ create(:report_approver_rule, :scan_finding,
+ merge_request: merge_request, vulnerability_states: ['detected'],
+ approval_project_rule: approval_project_rule_other, approvals_required: 1,
+ scan_result_policy_read: scan_result_policy_read_other_scan_finding)
+ end
+
+ let_it_be(:other_violation) do
+ create(:scan_result_policy_violation, scan_result_policy_read: scan_result_policy_read_other_scan_finding,
+ merge_request: merge_request)
+ end
+
+ it_behaves_like 'triggers policy bot comment', :scan_finding, true
+ end
end
context 'when there are no required approvals' do
diff --git a/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb b/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb
index e4595a70b245377d2520eb74e140bc51df327d53..b4808aea00aa67db886c96510bd9a7f43e8f1c22 100644
--- a/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb
+++ b/ee/spec/services/security/unenforceable_policy_rules_notification_service_spec.rb
@@ -66,7 +66,7 @@
end
shared_examples_for 'unenforceable report' do |report_type|
- it_behaves_like 'triggers policy bot comment', report_type, false
+ it_behaves_like 'triggers policy bot comment', report_type, false, requires_approval: false
context 'with violated approval rules' do
let(:approvals_required) { 1 }
@@ -112,7 +112,7 @@
end
end
- it_behaves_like 'triggers policy bot comment', report_type, false
+ it_behaves_like 'triggers policy bot comment', report_type, false, requires_approval: false
end
end
end
diff --git a/ee/spec/support/shared_examples/services/security/policy_violation_comment_shared_examples.rb b/ee/spec/support/shared_examples/services/security/policy_violation_comment_shared_examples.rb
index c48a706085e98d3848836ec802ba7a60668b974f..c8b9ddfa982b3c3982ecf5f669593751a109d5f8 100644
--- a/ee/spec/support/shared_examples/services/security/policy_violation_comment_shared_examples.rb
+++ b/ee/spec/support/shared_examples/services/security/policy_violation_comment_shared_examples.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
RSpec.shared_examples_for 'triggers policy bot comment' do |report_type, expected_violation,
- requires_approval: expected_violation|
+ requires_approval: true|
it 'enqueues Security::GeneratePolicyViolationCommentWorker' do
expect(Security::GeneratePolicyViolationCommentWorker).to receive(:perform_async).with(
merge_request.id,