diff --git a/ee/app/services/security/scan_result_policies/generate_policy_violation_comment_service.rb b/ee/app/services/security/scan_result_policies/generate_policy_violation_comment_service.rb
index 1ffaf420f05ed58b7110d90917ca89db8acefb37..cefce601079a6963c2ba4439edc3113a211b7d42 100644
--- a/ee/app/services/security/scan_result_policies/generate_policy_violation_comment_service.rb
+++ b/ee/app/services/security/scan_result_policies/generate_policy_violation_comment_service.rb
@@ -44,7 +44,7 @@ def exclusive_lock_key
end
def comment
- @comment ||= PolicyViolationComment.new(existing_comment).tap do |violation_comment|
+ @comment ||= PolicyViolationComment.new(existing_comment, project).tap do |violation_comment|
violation_comment.remove_report_type(report_type)
violation_comment.add_report_type(report_type, requires_approval) if violated_policy
end
diff --git a/ee/lib/security/scan_result_policies/policy_violation_comment.rb b/ee/lib/security/scan_result_policies/policy_violation_comment.rb
index fecdf43ea9e0e347b1b988b75485aa80e431c693..c6a6be4987ac3c787fa1bdec857dc50ea36557b1 100644
--- a/ee/lib/security/scan_result_policies/policy_violation_comment.rb
+++ b/ee/lib/security/scan_result_policies/policy_violation_comment.rb
@@ -3,6 +3,8 @@
module Security
module ScanResultPolicies
class PolicyViolationComment
+ include Rails.application.routes.url_helpers
+
MESSAGE_HEADER = '<!-- policy_violation_comment -->'
VIOLATED_REPORTS_HEADER_PATTERN = /<!-- violated_reports: ([a-z_,]+)/
OPTIONAL_APPROVALS_HEADER_PATTERN = /<!-- optional_approvals: ([a-z_,]+)/
@@ -11,11 +13,17 @@ class PolicyViolationComment
scan_finding: 'scan_finding',
any_merge_request: 'any_merge_request'
}.freeze
- MESSAGE_REQUIRES_APPROVAL = <<~TEXT.squish
+ MESSAGE_REQUIRES_APPROVAL = <<~MARKDOWN
Security and compliance scanners enforced by your organization have completed and identified that approvals
are required due to one or more policy violations.
Review the policy's rules in the MR widget and assign reviewers to proceed.
- TEXT
+
+ Several factors can lead to a violation or required approval in your merge request:
+
+ - If merge request approval policies enforced on your project include a scanner in the conditions, the scanner must be properly configured to run in both the source and target branch, the required jobs must complete, and a job artifact containing the scan results must be produced (even if empty).
+ - If any violation of a merge request approval policy's rules are found, approval is required.
+- Approvals are assumed required until all pipelines associated with the merge base commit in the target branch, and all pipelines associated with the latest commit in the source branch, are complete and it's confirmed that no policy violations have occurred.
+ MARKDOWN
MESSAGE_REQUIRES_NO_APPROVAL = <<~TEXT.squish
Security and compliance scanners enforced by your organization have completed and identified one or more
@@ -23,12 +31,13 @@ class PolicyViolationComment
Consider including optional reviewers based on the policy rules in the MR widget.
TEXT
- attr_reader :reports, :optional_approval_reports, :existing_comment
+ attr_reader :reports, :optional_approval_reports, :existing_comment, :project
- def initialize(existing_comment)
+ def initialize(existing_comment, project)
@existing_comment = existing_comment
@reports = Set.new
@optional_approval_reports = Set.new
+ @project = project
return unless existing_comment
@@ -71,20 +80,33 @@ def fixed_note_body
'Security policy violations have been resolved.'
end
+ def links_approvals_required
+ <<~MARKDOWN
+
+ [View policies enforced on your project](#{project_security_policies_url(project)}).<br>
+ [View further troubleshooting guidance](#{help_page_url('user/application_security/policies/index', anchor: 'troubleshooting-common-issues-configuring-security-policies')}).
+ MARKDOWN
+ end
+
def body_message
return fixed_note_body if reports.empty?
- message = reports == optional_approval_reports ? MESSAGE_REQUIRES_NO_APPROVAL : MESSAGE_REQUIRES_APPROVAL
+ message, links = if reports == optional_approval_reports
+ [MESSAGE_REQUIRES_NO_APPROVAL, '']
+ else
+ [MESSAGE_REQUIRES_APPROVAL, links_approvals_required]
+ end
+
optional_approvals_sorted_list = optional_approval_reports.sort.join(',')
<<~MARKDOWN
<!-- violated_reports: #{reports.sort.join(',')} -->
#{"<!-- optional_approvals: #{optional_approvals_sorted_list} -->" if optional_approval_reports.any?}
- | :warning: **Policy violation(s) detected**|
- | ----------------------------------------- |
- | #{message} |
+ :warning: **Policy violation(s) detected**
+
+ #{message}#{links}
#{format('Learn more about [Security and Compliance policies](%{url}).',
- url: Rails.application.routes.url_helpers.help_page_url('user/application_security/policies/index'))}
+ url: help_page_url('user/application_security/policies/index'))}
MARKDOWN
end
end
diff --git a/ee/spec/lib/security/scan_result_policies/policy_violation_comment_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_violation_comment_spec.rb
index bed2a1745b3e01fc6b8f07e6eb6c835f0237d1f8..a2bb194e4a34b062ae462200af67c9df42c76c64 100644
--- a/ee/spec/lib/security/scan_result_policies/policy_violation_comment_spec.rb
+++ b/ee/spec/lib/security/scan_result_policies/policy_violation_comment_spec.rb
@@ -5,7 +5,8 @@
RSpec.describe Security::ScanResultPolicies::PolicyViolationComment, feature_category: :security_policy_management do
using RSpec::Parameterized::TableSyntax
- let(:comment) { described_class.new(existing_comment) }
+ let_it_be(:project) { create(:project) }
+ let(:comment) { described_class.new(existing_comment, project) }
def build_comment(reports: [], optional_approvals: [])
build(:note,
@@ -104,9 +105,9 @@ def build_comment(reports: [], optional_approvals: [])
describe '#body' do
subject { comment.body }
- let_it_be(:violations_resolved) { 'Security policy violations have been resolved.' }
- let_it_be(:violations_detected) { 'Policy violation(s) detected' }
- let_it_be(:optional_approvals_detected) { 'Consider including optional reviewers' }
+ let_it_be(:violations_resolved) { ['Security policy violations have been resolved.'] }
+ let_it_be(:violations_detected) { ['Policy violation(s) detected', 'View policies enforced on your project'] }
+ let_it_be(:optional_approvals_detected) { ['Consider including optional reviewers'] }
context 'when there is no existing comment and no reports' do
let(:existing_comment) { nil }
@@ -133,7 +134,7 @@ def build_comment(reports: [], optional_approvals: [])
end
it { is_expected.to start_with(described_class::MESSAGE_HEADER) }
- it { is_expected.to include(expected_body) }
+ it { is_expected.to include(*expected_body) }
end
end
end