Merge branch 'sync_vuln_rules_based_on_selected_scanners' into 'master'

Sync vulnerability rules

See merge request gitlab-org/gitlab!66115

ee/app/models/approval_project_rule.rb

@@ -22,6 +22,9 @@ class ApprovalProjectRule < ApplicationRecord
   validates :name, uniqueness: { scope: [:project_id, :rule_type] }
   validates :rule_type, uniqueness: { scope: :project_id, message: proc { _('any-approver for the project already exists') } }, if: :any_approver?
 
+  validates :scanners, inclusion: { in: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES }
+  default_value_for :scanners, allows_nil: false, value: ::Ci::JobArtifact::SECURITY_REPORT_FILE_TYPES
+
   def applies_to_branch?(branch)
     return true if protected_branches.empty?
 

ee/app/models/concerns/approval_rule_like.rb

@@ -29,6 +29,7 @@ module ApprovalRuleLike
     scope :with_users, -> { preload(:users, :group_users) }
     scope :regular_or_any_approver, -> { where(rule_type: [:regular, :any_approver]) }
     scope :for_groups, -> (groups) { joins(:groups).where(approval_project_rules_groups: { group_id: groups }) }
+    scope :vulnerability_reports, -> { where(rule_type: :report_approver, name: ApprovalRuleLike::DEFAULT_NAME_FOR_VULNERABILITY_REPORT) }
   end
 
   def audit_add

ee/app/models/ee/ci/job_artifact.rb

@@ -9,13 +9,14 @@ module EE
     include ::Gitlab::Utils::StrongMemoize
     extend ActiveSupport::Concern
 
+    SECURITY_REPORT_FILE_TYPES = %w[sast secret_detection dependency_scanning container_scanning cluster_image_scanning dast coverage_fuzzing api_fuzzing].freeze
+
     prepended do
       # After destroy callbacks are often skipped because of FastDestroyAll.
       # All destroy callbacks should be implemented in `Ci::JobArtifacts::DestroyBatchService`
       # See https://gitlab.com/gitlab-org/gitlab/-/issues/297472
       after_destroy :log_geo_deleted_event
 
-      SECURITY_REPORT_FILE_TYPES = %w[sast secret_detection dependency_scanning container_scanning cluster_image_scanning dast coverage_fuzzing api_fuzzing].freeze
       LICENSE_SCANNING_REPORT_FILE_TYPES = %w[license_scanning].freeze
       DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze
       METRICS_REPORT_FILE_TYPES = %w[metrics].freeze

ee/app/services/ci/sync_reports_to_approval_rules_service.rb

@@ -2,6 +2,8 @@
 
 module Ci
   class SyncReportsToApprovalRulesService < ::BaseService
+    include Gitlab::Utils::StrongMemoize
+
     def initialize(pipeline)
       @pipeline = pipeline
     end
@@ -52,7 +54,15 @@ module Ci
     end
 
     def reports
-      @reports ||= pipeline.security_reports
+      strong_memoize(:reports) do
+        project_vulnerability_rules ? pipeline.security_reports(report_types: project_vulnerability_rules) : []
+      end
+    end
+
+    def project_vulnerability_rules
+      strong_memoize(:project_vulnerability_rules) do
+        pipeline.project.approval_rules.vulnerability_reports.first&.scanners
+      end
     end
 
     def merge_requests_approved_coverage
@@ -66,7 +76,7 @@ module Ci
 
     def merge_requests_approved_security_reports
       pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
-        reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports)
+        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports)
       end
     end
 

ee/spec/models/approval_project_rule_spec.rb

@@ -47,6 +47,26 @@ RSpec.describe ApprovalProjectRule do
     end
   end
 
+  describe '.vulnerability_reports scope' do
+    subject { described_class.vulnerability_reports }
+
+    context 'with vulnerability reports' do
+      before do
+        create(:approval_project_rule, :vulnerability_report)
+      end
+
+      it { is_expected.to be_present }
+    end
+
+    context 'without vulnerability reports' do
+      before do
+        create(:approval_project_rule)
+      end
+
+      it { is_expected.to be_empty }
+    end
+  end
+
   describe '#regular?' do
     let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) }
 
@@ -139,9 +159,22 @@ RSpec.describe ApprovalProjectRule do
       end
 
       context "with a `Vulnerability-Check` rule" do
-        subject { vulnerability_check_rule }
+        using RSpec::Parameterized::TableSyntax
+
+        where(:is_valid, :scanners) do
+          true  | []
+          true  | %w(dast)
+          true  | %w(dast sast)
+          true  | %w(dast dast)
+          false | %w(dast unknown_scanner)
+          false | %w(unknown_scanner)
+        end
+
+        with_them do
+          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners) }
 
-        specify { expect(subject).to be_valid }
+          specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
+        end
       end
     end
   end

ee/spec/services/ci/sync_reports_to_approval_rules_service_spec.rb

@@ -19,6 +19,11 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
 
   context 'with security rules' do
     let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
+    let(:scanners) { %w[dependency_scanning] }
+
+    before do
+      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners)
+    end
 
     context 'when there are security reports' do
       context 'when pipeline passes' do
@@ -44,6 +49,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
                 .not_to change { report_approver_rule.reload.approvals_required }
             end
           end
+
+          context 'without any scanners related to the security reports' do
+            let(:scanners) { %w[sast] }
+
+            it 'lowers approvals_required count to zero' do
+              expect { subject }
+                .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
+            end
+          end
         end
 
         context 'when only low-severity vulnerabilities are present' do