Skip to content
Snippets Groups Projects
Commit 96f31f83 authored by Martin Čavoj's avatar Martin Čavoj :palm_tree:
Browse files

Merge branch '411656-spike-investigate-and-prepare-poc-for-approval-notification-mvc' into 'master'

Draft: Add security policy bot comment for policy violations in MRs

See merge request gitlab-org/gitlab!121732



Merged-by: default avatarMartin Čavoj <mcavoj@gitlab.com>
parents 528f4068 0f6ca51f
No related branches found
No related tags found
No related merge requests found
...@@ -141,6 +141,7 @@ class Note < ApplicationRecord ...@@ -141,6 +141,7 @@ class Note < ApplicationRecord
scope :with_discussion_ids, ->(discussion_ids) { where(discussion_id: discussion_ids) } scope :with_discussion_ids, ->(discussion_ids) { where(discussion_id: discussion_ids) }
scope :with_suggestions, -> { joins(:suggestions) } scope :with_suggestions, -> { joins(:suggestions) }
scope :inc_author, -> { includes(:author) } scope :inc_author, -> { includes(:author) }
scope :authored_by, ->(user) { where(author_id: user.id) }
scope :inc_note_diff_file, -> { includes(:note_diff_file) } scope :inc_note_diff_file, -> { includes(:note_diff_file) }
scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) } scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) }
scope :inc_relations_for_view, ->(noteable = nil) do scope :inc_relations_for_view, ->(noteable = nil) do
......
# frozen_string_literal: true
module Notes
class GenerateSecurityPolicyCommentService < BaseService
attr_reader :merge_request, :project, :violated_policy
def initialize(merge_request, violated_policy)
@merge_request = merge_request
@project = merge_request.project
@violated_policy = violated_policy
end
def execute
return unless bot_user
if bot_comment
body = violated_policy ? violations_note_body : fixed_note_body
Notes::UpdateService.new(project, bot_user, note_params(body)).execute(bot_comment)
elsif violated_policy
Notes::CreateService.new(project, bot_user, note_params(violations_note_body)).execute
end
end
private
def bot_comment
@bot_comment ||= merge_request.notes.authored_by(bot_user).first
end
def bot_user
@bot_user ||= project.security_orchestration_policy_configuration&.bot_user
end
def note_params(body)
{
note: body,
noteable: merge_request
}
end
def fixed_note_body
_('Security policy violations have been fixed.')
end
def violations_note_body
message = <<~TEXT.squish
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 rules in the MR widget and assign a reviewer to proceed.
TEXT
<<~MARKDOWN
| :warning: **#{_('Policy violation(s) detected')}**|
| ----------------------------------------- |
| #{_(message)} |
#{format(_('Learn more about [Security & Compliance policies](%{url}).'),
url: Rails.application.routes.url_helpers.help_page_url('user/application_security/policies'))}
MARKDOWN
end
end
end
...@@ -15,14 +15,16 @@ def initialize(merge_request:, pipeline:, pipeline_findings:) ...@@ -15,14 +15,16 @@ def initialize(merge_request:, pipeline:, pipeline_findings:)
end end
def execute def execute
return if scan_removed? violated_rules, unviolated_rules = merge_request.approval_rules.scan_finding.partition do |approval_rule|
unviolated_rules = merge_request.approval_rules.scan_finding.reject do |approval_rule|
approval_rule = approval_rule.source_rule if approval_rule.source_rule approval_rule = approval_rule.source_rule if approval_rule.source_rule
violates_approval_rule?(approval_rule) violates_approval_rule?(approval_rule)
end end
generate_policy_bot_comment(violated_rules.any? || scan_removed?)
return if scan_removed?
ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any? ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any?
end end
...@@ -40,6 +42,16 @@ def violates_approval_rule?(approval_rule) ...@@ -40,6 +42,16 @@ def violates_approval_rule?(approval_rule)
def scan_removed? def scan_removed?
(Array.wrap(target_pipeline&.security_scan_types) - pipeline.security_scan_types).any? (Array.wrap(target_pipeline&.security_scan_types) - pipeline.security_scan_types).any?
end end
strong_memoize_attr :scan_removed?
def generate_policy_bot_comment(violated_policy)
return if Feature.disabled?(:security_policy_approval_notification)
Notes::GenerateSecurityPolicyCommentService.new(
merge_request,
violated_policy
).execute
end
def target_pipeline_security_findings def target_pipeline_security_findings
target_pipeline&.security_findings || Security::Finding.none target_pipeline&.security_findings || Security::Finding.none
......
---
name: security_policy_approval_notification
introduced_by_url:
rollout_issue_url:
milestone: '16.1'
type: development
group: group::security policies
default_enabled: false
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
end end
end end
let(:report_approver_rule) do let!(:report_approver_rule) do
create(:report_approver_rule, :scan_finding, create(:report_approver_rule, :scan_finding,
merge_request: merge_request, merge_request: merge_request,
approvals_required: 2, approvals_required: 2,
...@@ -80,6 +80,19 @@ ...@@ -80,6 +80,19 @@
end end
end end
shared_examples_for 'triggers policy bot comment' do |violated_policy|
it 'calls Notes::CreateSecurityPolicyCommentService' do
mock = instance_double(Notes::GenerateSecurityPolicyCommentService)
expect(mock).to receive(:execute)
expect(Notes::GenerateSecurityPolicyCommentService).to receive(:new).with(
merge_request,
violated_policy
).and_return(mock)
service
end
end
context 'when security scan is removed in current pipeline' do context 'when security scan is removed in current pipeline' do
let_it_be(:pipeline) { create(:ee_ci_pipeline, project: project, ref: merge_request.source_branch) } let_it_be(:pipeline) { create(:ee_ci_pipeline, project: project, ref: merge_request.source_branch) }
...@@ -90,6 +103,8 @@ ...@@ -90,6 +103,8 @@
let(:vulnerabilities_allowed) { 100 } let(:vulnerabilities_allowed) { 100 }
it_behaves_like 'sets approvals_required to 0' it_behaves_like 'sets approvals_required to 0'
it_behaves_like 'triggers policy bot comment', false
end end
context 'when target pipeline is nil' do context 'when target pipeline is nil' do
...@@ -98,6 +113,8 @@ ...@@ -98,6 +113,8 @@
end end
it_behaves_like 'does not update approvals_required' it_behaves_like 'does not update approvals_required'
it_behaves_like 'triggers policy bot comment', true
end end
context 'when the number of findings in current pipeline exceed the allowed limit' do context 'when the number of findings in current pipeline exceed the allowed limit' do
...@@ -127,6 +144,8 @@ ...@@ -127,6 +144,8 @@
context 'when vulnerabilities count exceeds the allowed limit' do context 'when vulnerabilities count exceeds the allowed limit' do
it_behaves_like 'does not update approvals_required' it_behaves_like 'does not update approvals_required'
it_behaves_like 'triggers policy bot comment', true
end end
context 'when new findings are introduced and it exceeds the allowed limit' do context 'when new findings are introduced and it exceeds the allowed limit' do
...@@ -140,6 +159,8 @@ ...@@ -140,6 +159,8 @@
it_behaves_like 'does not update approvals_required' it_behaves_like 'does not update approvals_required'
it_behaves_like 'triggers policy bot comment', true
context 'when there are no new dismissed vulnerabilities' do context 'when there are no new dismissed vulnerabilities' do
let(:vulnerabilities_allowed) { 0 } let(:vulnerabilities_allowed) { 0 }
......
...@@ -26394,6 +26394,9 @@ msgstr "" ...@@ -26394,6 +26394,9 @@ msgstr ""
msgid "Learn more about X.509 signed commits" msgid "Learn more about X.509 signed commits"
msgstr "" msgstr ""
   
msgid "Learn more about [Security & Compliance policies](%{url})."
msgstr ""
msgid "Learn more about adding certificates to your project by following the %{docs_link_start}documentation on GitLab Pages%{docs_link_end}." msgid "Learn more about adding certificates to your project by following the %{docs_link_start}documentation on GitLab Pages%{docs_link_end}."
msgstr "" msgstr ""
   
...@@ -33880,6 +33883,9 @@ msgstr "" ...@@ -33880,6 +33883,9 @@ msgstr ""
msgid "Policy project doesn't exist" msgid "Policy project doesn't exist"
msgstr "" msgstr ""
   
msgid "Policy violation(s) detected"
msgstr ""
msgid "PolicyRuleMultiSelect|%{firstLabel} +%{numberOfAdditionalLabels} more" msgid "PolicyRuleMultiSelect|%{firstLabel} +%{numberOfAdditionalLabels} more"
msgstr "" msgstr ""
   
...@@ -40367,6 +40373,9 @@ msgstr "" ...@@ -40367,6 +40373,9 @@ msgstr ""
msgid "Security dashboard" msgid "Security dashboard"
msgstr "" msgstr ""
   
msgid "Security policy violations have been fixed."
msgstr ""
msgid "Security report is out of date. Please update your branch with the latest changes from the target branch (%{targetBranchName})" msgid "Security report is out of date. Please update your branch with the latest changes from the target branch (%{targetBranchName})"
msgstr "" msgstr ""
   
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment