Add security policy bot comment for policy violations in MRs

This change introduces an automatic comment on the MR
where security policy violations are detected. It adds guidance for
the user on what to do next.

Changelog: added
EE: true

app/models/note.rb

@@ -141,6 +141,7 @@ class Note < ApplicationRecord
   scope :with_discussion_ids, ->(discussion_ids) { where(discussion_id: discussion_ids) }
   scope :with_suggestions, -> { joins(:suggestions) }
   scope :inc_author, -> { includes(:author) }
+  scope :authored_by, ->(user) { where(author_id: user.id) }
   scope :inc_note_diff_file, -> { includes(:note_diff_file) }
   scope :with_api_entity_associations, -> { preload(:note_diff_file, :author) }
   scope :inc_relations_for_view, ->(noteable = nil) do

ee/app/services/notes/generate_security_policy_comment_service.rb

+# 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

ee/app/services/security/scan_result_policies/update_approvals_service.rb

@@ -15,14 +15,16 @@ def initialize(merge_request:, pipeline:, pipeline_findings:)
       end
 
       def execute
-        return if scan_removed?
-
-        unviolated_rules = merge_request.approval_rules.scan_finding.reject do |approval_rule|
+        violated_rules, unviolated_rules = merge_request.approval_rules.scan_finding.partition do |approval_rule|
           approval_rule = approval_rule.source_rule if approval_rule.source_rule
 
           violates_approval_rule?(approval_rule)
         end
 
+        generate_policy_bot_comment(violated_rules.any? || scan_removed?)
+
+        return if scan_removed?
+
         ApprovalMergeRequestRule.remove_required_approved(unviolated_rules) if unviolated_rules.any?
       end
 
@@ -40,6 +42,16 @@ def violates_approval_rule?(approval_rule)
       def scan_removed?
         (Array.wrap(target_pipeline&.security_scan_types) - pipeline.security_scan_types).any?
       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
         target_pipeline&.security_findings || Security::Finding.none

ee/config/feature_flags/development/security_policy_approval_notification.yml

+---
+name: security_policy_approval_notification
+introduced_by_url:
+rollout_issue_url:
+milestone: '16.1'
+type: development
+group: group::security policies
+default_enabled: false

ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb

@@ -21,7 +21,7 @@
       end
     end
 
-    let(:report_approver_rule) do
+    let!(:report_approver_rule) do
       create(:report_approver_rule, :scan_finding,
         merge_request: merge_request,
         approvals_required: 2,
@@ -80,6 +80,19 @@
       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
       let_it_be(:pipeline) { create(:ee_ci_pipeline, project: project, ref: merge_request.source_branch) }
 
@@ -90,6 +103,8 @@
       let(:vulnerabilities_allowed) { 100 }
 
       it_behaves_like 'sets approvals_required to 0'
+
+      it_behaves_like 'triggers policy bot comment', false
     end
 
     context 'when target pipeline is nil' do
@@ -98,6 +113,8 @@
       end
 
       it_behaves_like 'does not update approvals_required'
+
+      it_behaves_like 'triggers policy bot comment', true
     end
 
     context 'when the number of findings in current pipeline exceed the allowed limit' do
@@ -127,6 +144,8 @@
 
       context 'when vulnerabilities count exceeds the allowed limit' do
         it_behaves_like 'does not update approvals_required'
+
+        it_behaves_like 'triggers policy bot comment', true
       end
 
       context 'when new findings are introduced and it exceeds the allowed limit' do
@@ -140,6 +159,8 @@
 
         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
           let(:vulnerabilities_allowed) { 0 }
 

locale/gitlab.pot

@@ -26373,6 +26373,9 @@ msgstr ""
 msgid "Learn more about X.509 signed commits"
 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}."
 msgstr ""
@@ -33880,6 +33883,9 @@ msgstr ""
 msgid "Policy project doesn't exist"
 msgstr ""
+msgid "Policy violation(s) detected"
+msgstr ""
+
 msgid "PolicyRuleMultiSelect|%{firstLabel} +%{numberOfAdditionalLabels} more"
 msgstr ""
@@ -40358,6 +40364,9 @@ msgstr ""
 msgid "Security dashboard"
 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})"
 msgstr ""