Merge branch 'add_severity_levels_validation_and_usage_in_backend' into 'master'

Add severity_levels validation and usage

See merge request gitlab-org/gitlab!67064

ee/app/models/approval_project_rule.rb

@@ -31,6 +31,9 @@ class ApprovalProjectRule < ApplicationRecord
   validates :vulnerabilities_allowed, numericality: { only_integer: true }
   default_value_for :vulnerabilities_allowed, allows_nil: false, value: 0
 
+  validates :severity_levels, inclusion: { in: ::Enums::Vulnerability.severity_levels.keys }
+  default_value_for :severity_levels, allows_nil: false, value: ::Enums::Vulnerability.severity_levels.keys
+
   def applies_to_branch?(branch)
     return true if protected_branches.empty?
 

ee/app/services/ci/sync_reports_to_approval_rules_service.rb

@@ -23,7 +23,7 @@ module Ci
       )
       error("Failed to update approval rules")
     ensure
-      [:project_rule_vulnerabilities_allowed, :project_vulnerability_rule, :reports].each do |memoization|
+      [:project_rule_vulnerabilities_allowed, :project_rule_scanners, :project_rule_severity_levels, :project_vulnerability_report, :reports].each do |memoization|
         clear_memoization(memoization)
       end
     end
@@ -59,19 +59,19 @@ module Ci
 
     def reports
       strong_memoize(:reports) do
-        project_vulnerability_rule ? pipeline.security_reports(report_types: project_vulnerability_rule) : []
+        project_rule_scanners ? pipeline.security_reports(report_types: project_rule_scanners) : []
       end
     end
 
-    def project_vulnerability_rule
-      strong_memoize(:project_vulnerability_rule) do
-        pipeline.project.vulnerability_report_rule&.scanners
+    def project_rule_scanners
+      strong_memoize(:project_rule_scanners) do
+        project_vulnerability_report&.scanners
       end
     end
 
     def project_rule_vulnerabilities_allowed
       strong_memoize(:project_rule_vulnerabilities_allowed) do
-        pipeline.project.vulnerability_report_rule&.vulnerabilities_allowed
+        project_vulnerability_report&.vulnerabilities_allowed
       end
     end
 
@@ -86,7 +86,7 @@ module Ci
 
     def merge_requests_approved_security_reports
       pipeline.merge_requests_as_head_pipeline.reject do |merge_request|
-        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports, project_rule_vulnerabilities_allowed)
+        reports.present? && reports.violates_default_policy_against?(merge_request.base_pipeline&.security_reports, project_rule_vulnerabilities_allowed, project_rule_severity_levels)
       end
     end
 
@@ -95,5 +95,17 @@ module Ci
         .for_unmerged_merge_requests(merge_requests)
         .update_all(approvals_required: 0)
     end
+
+    def project_rule_severity_levels
+      strong_memoize(:project_rule_severity_levels) do
+        project_vulnerability_report&.severity_levels
+      end
+    end
+
+    def project_vulnerability_report
+      strong_memoize(:project_vulnerability_report) do
+        pipeline.project.vulnerability_report_rule
+      end
+    end
   end
 end

ee/spec/lib/gitlab/ci/reports/security/finding_spec.rb

@@ -179,19 +179,19 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
   end
 
   describe '#unsafe?' do
-    where(:severity, :unsafe?) do
-      'critical' | true
-      'high'     | true
-      'medium'   | false
-      'low'      | false
-      'info'     | false
-      'unknown'  | true
+    where(:severity, :levels, :unsafe?) do
+      'critical' | %w(critical high) | true
+      'high'     | %w(critical high) | true
+      'medium'   | %w(critical high) | false
+      'low'      | %w(critical high) | false
+      'info'     | %w(critical high) | false
+      'unknown'  | []                | false
     end
 
     with_them do
       let(:finding) { create(:ci_reports_security_finding, severity: severity) }
 
-      subject { finding.unsafe? }
+      subject { finding.unsafe?(levels) }
 
       it { is_expected.to be(unsafe?) }
     end

ee/spec/models/approval_project_rule_spec.rb

@@ -161,19 +161,20 @@ RSpec.describe ApprovalProjectRule do
       context "with a `Vulnerability-Check` rule" do
         using RSpec::Parameterized::TableSyntax
 
-        where(:is_valid, :scanners, :vulnerabilities_allowed) do
-          true  | []                                     | 0
-          true  | %w(dast)                               | 1
-          true  | %w(dast sast)                          | 10
-          true  | %w(dast dast)                          | 100
-          false | %w(dast unknown_scanner)               | 100
-          false | [described_class::UNSUPPORTED_SCANNER] | 100
-          false | %w(dast sast)                          | 1.1
-          false | %w(dast sast)                          | 'one'
+        where(:is_valid, :scanners, :vulnerabilities_allowed, :severity_levels) do
+          true  | []                                     | 0     | []
+          true  | %w(dast)                               | 1     | %w(critical high medium)
+          true  | %w(dast sast)                          | 10    | %w(critical high)
+          true  | %w(dast dast)                          | 100   | %w(critical)
+          false | %w(dast dast)                          | 100   | %w(unknown_severity)
+          false | %w(dast unknown_scanner)               | 100   | %w(critical)
+          false | [described_class::UNSUPPORTED_SCANNER] | 100   | %w(critical)
+          false | %w(dast sast)                          | 1.1   | %w(critical)
+          false | %w(dast sast)                          | 'one' | %w(critical)
         end
 
         with_them do
-          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed) }
+          let(:vulnerability_check_rule) { build(:approval_project_rule, :vulnerability, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels) }
 
           specify { expect(vulnerability_check_rule.valid?).to be(is_valid) }
         end

ee/spec/services/ci/sync_reports_to_approval_rules_service_spec.rb

@@ -21,9 +21,10 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
     let(:report_approver_rule) { create(:report_approver_rule, merge_request: merge_request, approvals_required: 2) }
     let(:scanners) { %w[dependency_scanning] }
     let(:vulnerabilities_allowed) { 0 }
+    let(:severity_levels) { %w[high unknown] }
 
     before do
-      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed)
+      create(:approval_project_rule, :vulnerability, project: project, approvals_required: 2, scanners: scanners, vulnerabilities_allowed: vulnerabilities_allowed, severity_levels: severity_levels)
     end
 
     context 'when there are security reports' do
@@ -68,6 +69,15 @@ RSpec.describe Ci::SyncReportsToApprovalRulesService, '#execute' do
                 .to change { report_approver_rule.reload.approvals_required }.from(2).to(0)
             end
           end
+
+          context 'without any findings related to the severity levels' do
+            let(:severity_levels) { %w[info] }
+
+            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

lib/gitlab/ci/reports/security/finding.rb

@@ -7,8 +7,6 @@ module Gitlab
         class Finding
           include ::VulnerabilityFindingHelpers
 
-          UNSAFE_SEVERITIES = %w[unknown high critical].freeze
-
           attr_reader :compare_key
           attr_reader :confidence
           attr_reader :identifiers
@@ -86,8 +84,8 @@ module Gitlab
             @location = new_location
           end
 
-          def unsafe?
-            severity.in?(UNSAFE_SEVERITIES)
+          def unsafe?(severity_levels)
+            severity.in?(severity_levels)
           end
 
           def eql?(other)

lib/gitlab/ci/reports/security/reports.rb

@@ -22,8 +22,8 @@ module Gitlab
             reports.values.flat_map(&:findings)
           end
 
-          def violates_default_policy_against?(target_reports, vulnerabilities_allowed)
-            unsafe_findings_count(target_reports) > vulnerabilities_allowed
+          def violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels)
+            unsafe_findings_count(target_reports, severity_levels) > vulnerabilities_allowed
           end
 
           private
@@ -32,8 +32,8 @@ module Gitlab
             findings - target_reports&.findings.to_a
           end
 
-          def unsafe_findings_count(target_reports)
-            findings_diff(target_reports).count(&:unsafe?)
+          def unsafe_findings_count(target_reports, severity_levels)
+            findings_diff(target_reports).count {|finding| finding.unsafe?(severity_levels)}
           end
         end
       end

spec/lib/gitlab/ci/reports/security/reports_spec.rb

@@ -54,27 +54,25 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
   end
 
   describe "#violates_default_policy_against?" do
-    let(:low_severity_sast) { build(:ci_reports_security_finding, severity: 'low', report_type: :sast) }
     let(:high_severity_dast) { build(:ci_reports_security_finding, severity: 'high', report_type: :dast) }
     let(:vulnerabilities_allowed) { 0 }
+    let(:severity_levels) { %w(critical high) }
 
-    subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed) }
+    subject { security_reports.violates_default_policy_against?(target_reports, vulnerabilities_allowed, severity_levels) }
+
+    before do
+      security_reports.get_report('sast', artifact).add_finding(high_severity_dast)
+    end
 
     context 'when the target_reports is `nil`' do
       let(:target_reports) { nil }
 
-      context "when a report has unsafe vulnerability" do
-        before do
-          security_reports.get_report('sast', artifact).add_finding(high_severity_dast)
-        end
-
+      context 'with severity levels matching the existing vulnerabilities' do
         it { is_expected.to be(true) }
       end
 
-      context "when none of the reports have an unsafe vulnerability" do
-        before do
-          security_reports.get_report('sast', artifact).add_finding(low_severity_sast)
-        end
+      context "without any severity levels matching the existing vulnerabilities" do
+        let(:severity_levels) { %w(critical) }
 
         it { is_expected.to be(false) }
       end
@@ -84,10 +82,8 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
       let(:target_reports) { described_class.new(pipeline) }
 
       context "when a report has a new unsafe vulnerability" do
-        before do
-          security_reports.get_report('sast', artifact).add_finding(high_severity_dast)
-          security_reports.get_report('dependency_scanning', artifact).add_finding(low_severity_sast)
-          target_reports.get_report('dependency_scanning', artifact).add_finding(low_severity_sast)
+        context 'with severity levels matching the existing vulnerabilities' do
+          it { is_expected.to be(true) }
         end
 
         it { is_expected.to be(true) }
@@ -97,12 +93,16 @@ RSpec.describe Gitlab::Ci::Reports::Security::Reports do
 
           it { is_expected.to be(false) }
         end
+
+        context "without any severity levels matching the existing vulnerabilities" do
+          let(:severity_levels) { %w(critical) }
+
+          it { is_expected.to be(false) }
+        end
       end
 
       context "when none of the reports have a new unsafe vulnerability" do
         before do
-          security_reports.get_report('sast', artifact).add_finding(high_severity_dast)
-          security_reports.get_report('sast', artifact).add_finding(low_severity_sast)
           target_reports.get_report('sast', artifact).add_finding(high_severity_dast)
         end