Merge branch 'use_security_findings_to_count_vulnerabilities_238951' into 'master'

Use security findings to count vulnerabilities

See merge request gitlab-org/gitlab!53116

ee/app/models/security/finding.rb

@@ -40,7 +40,12 @@ module Security
     scope :with_scan, -> { includes(:scan) }
     scope :with_scanner, -> { includes(:scanner) }
     scope :deduplicated, -> { where(deduplicated: true) }
+    scope :grouped_by_scan_type, -> { joins(:scan).group('security_scans.scan_type') }
 
     delegate :scan_type, to: :scan, allow_nil: true
+
+    def self.count_by_scan_type
+      grouped_by_scan_type.count
+    end
   end
 end

ee/app/services/security/vulnerability_counting_service.rb

@@ -13,11 +13,13 @@ module Security
     end
 
     def execute
-      vulnerabilities = ::Security::PipelineVulnerabilitiesFinder.new(pipeline: @pipeline, params: { report_type: @report_types }).execute
-      vulnerabilities.findings.group_by(&:report_type).compact.transform_values(&:size).reverse_merge(no_counts)
-    end
+      counts = @pipeline.security_findings
+                        .deduplicated
+                        .by_report_types(@report_types)
+                        .count_by_scan_type
 
-    private
+      counts.transform_keys { |key| Security::Scan.scan_types.key(key) }.reverse_merge(no_counts)
+    end
 
     def no_counts
       @report_types.zip([0].cycle).to_h

ee/spec/models/security/finding_spec.rb

@@ -129,4 +129,21 @@ RSpec.describe Security::Finding do
 
     it { is_expected.to eq(expected_findings) }
   end
+
+  describe '.count_by_scan_type' do
+    let!(:sast_scan) { create(:security_scan, scan_type: :sast) }
+    let!(:dast_scan) { create(:security_scan, scan_type: :dast) }
+    let!(:finding_1) { create(:security_finding, scan: sast_scan) }
+    let!(:finding_2) { create(:security_finding, scan: sast_scan) }
+    let!(:finding_3) { create(:security_finding, scan: dast_scan) }
+
+    subject { described_class.count_by_scan_type }
+
+    it {
+      is_expected.to eq({
+        Security::Scan.scan_types['dast'] => 1,
+        Security::Scan.scan_types['sast'] => 2
+      })
+    }
+  end
 end

ee/spec/requests/api/graphql/project/pipeline/security_report_summary_spec.rb

@@ -5,14 +5,38 @@ require 'spec_helper'
 RSpec.describe 'Query.project(fullPath).pipeline(iid).securityReportSummary' do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
+
+  let_it_be(:build_sast) { create(:ci_build, :success, name: 'sast', pipeline: pipeline) }
+  let_it_be(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_sast) }
+  let_it_be(:report_sast) { create(:ci_reports_security_report, type: :sast) }
+
+  let_it_be(:build_dast) { create(:ci_build, :success, name: 'dast', pipeline: pipeline) }
+  let_it_be(:artifact_dast) { create(:ee_ci_job_artifact, :dast_large_scanned_resources_field, job: build_dast) }
+  let_it_be(:report_dast) { create(:ci_reports_security_report, type: :dast) }
+
   let_it_be(:user) { create(:user) }
 
   before_all do
-    create(:ci_build, :success, name: 'dast_job', pipeline: pipeline, project: project) do |job|
-      create(:ee_ci_job_artifact, :dast_large_scanned_resources_field, job: job, project: project)
+    sast_content = File.read(artifact_sast.file.path)
+    Gitlab::Ci::Parsers::Security::Sast.parse!(sast_content, report_sast)
+    report_sast.merge!(report_sast)
+
+    dast_content = File.read(artifact_dast.file.path)
+    Gitlab::Ci::Parsers::Security::Dast.parse!(dast_content, report_dast)
+    report_dast.merge!(report_dast)
+
+    { artifact_dast => report_dast, artifact_sast => report_sast }.each do |artifact, report|
+      scan = create(:security_scan, scan_type: artifact.job.name, build: artifact.job)
+
+      report.findings.each_with_index do |finding, index|
+        create(:security_finding,
+              severity: finding.severity,
+              confidence: finding.confidence,
+              project_fingerprint: finding.project_fingerprint,
+              deduplicated: true,
+              position: index,
+              scan: scan)
       end
-    create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) do |job|
-      create(:ee_ci_job_artifact, :sast, job: job, project: project)
     end
   end
 

ee/spec/services/security/report_summary_service_spec.rb

@@ -3,22 +3,53 @@
 require 'spec_helper'
 
 RSpec.describe Security::ReportSummaryService, '#execute' do
-  let_it_be(:project) { create(:project, :repository) }
-  let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, :success) }
 
-  before_all do
-    create(:ci_build, :success, name: 'dast_job', pipeline: pipeline, project: project) do |job|
-      create(:ee_ci_job_artifact, :dast_large_scanned_resources_field, job: job, project: project)
-    end
-    create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) do |job|
-      create(:ee_ci_job_artifact, :sast, job: job, project: project)
-    end
+  let_it_be(:build_ds) { create(:ci_build, :success, name: 'dependency_scanning', pipeline: pipeline) }
+  let_it_be(:artifact_ds) { create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds) }
+  let_it_be(:report_ds) { create(:ci_reports_security_report, type: :dependency_scanning) }
+
+  let_it_be(:build_sast) { create(:ci_build, :success, name: 'sast', pipeline: pipeline) }
+  let_it_be(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_sast) }
+  let_it_be(:report_sast) { create(:ci_reports_security_report, type: :sast) }
+
+  let_it_be(:build_dast) { create(:ci_build, :success, name: 'dast', pipeline: pipeline) }
+  let_it_be(:artifact_dast) { create(:ee_ci_job_artifact, :dast_large_scanned_resources_field, job: build_dast) }
+  let_it_be(:report_dast) { create(:ci_reports_security_report, type: :dast) }
+
+  let_it_be(:build_cs) { create(:ci_build, :success, name: 'container_scanning', pipeline: pipeline) }
+  let_it_be(:artifact_cs) { create(:ee_ci_job_artifact, :container_scanning, job: build_cs) }
+  let_it_be(:report_cs) { create(:ci_reports_security_report, type: :container_scanning) }
+
+  before(:all) do
+    ds_content = File.read(artifact_ds.file.path)
+    Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(ds_content, report_ds)
+    report_ds.merge!(report_ds)
+
+    sast_content = File.read(artifact_sast.file.path)
+    Gitlab::Ci::Parsers::Security::Sast.parse!(sast_content, report_sast)
+    report_sast.merge!(report_sast)
+
+    dast_content = File.read(artifact_dast.file.path)
+    Gitlab::Ci::Parsers::Security::Dast.parse!(dast_content, report_dast)
+    report_dast.merge!(report_dast)
+
+    cs_content = File.read(artifact_cs.file.path)
+    Gitlab::Ci::Parsers::Security::ContainerScanning.parse!(cs_content, report_cs)
+    report_cs.merge!(report_cs)
+
+    { artifact_cs => report_cs, artifact_dast => report_dast, artifact_ds => report_ds, artifact_sast => report_sast }.each do |artifact, report|
+      scan = create(:security_scan, scan_type: artifact.job.name, build: artifact.job)
 
-    create(:ci_build, :success, name: 'cs_job', pipeline: pipeline, project: project) do |job|
-      create(:ee_ci_job_artifact, :container_scanning, job: job, project: project)
+      report.findings.each_with_index do |finding, index|
+        create(:security_finding,
+              severity: finding.severity,
+              confidence: finding.confidence,
+              project_fingerprint: finding.project_fingerprint,
+              deduplicated: true,
+              position: index,
+              scan: scan)
       end
-    create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) do |job|
-      create(:ee_ci_job_artifact, :dependency_scanning, job: job, project: project)
     end
   end
 
@@ -101,7 +132,7 @@ RSpec.describe Security::ReportSummaryService, '#execute' do
 
     it 'returns the scanned_resources_csv_path' do
       expected_path = Gitlab::Routing.url_helpers.project_security_scanned_resources_path(
-        project,
+        pipeline.project,
         format: :csv,
         pipeline_id: pipeline.id
       )

ee/spec/services/security/vulnerability_counting_service_spec.rb

@@ -7,27 +7,61 @@ RSpec.describe Security::VulnerabilityCountingService, '#execute' do
     stub_licensed_features(sast: true, dependency_scanning: true, container_scanning: true, dast: true)
   end
 
-  let_it_be(:project) { create(:project, :repository) }
-  let_it_be(:pipeline) { create(:ci_pipeline, :success, project: project) }
+  let_it_be(:pipeline) { create(:ci_pipeline, :success) }
+
+  let_it_be(:build_ds) { create(:ci_build, :success, name: 'dependency_scanning', pipeline: pipeline) }
+  let_it_be(:artifact_ds) { create(:ee_ci_job_artifact, :dependency_scanning, job: build_ds) }
+  let_it_be(:report_ds) { create(:ci_reports_security_report, type: :dependency_scanning) }
+
+  let_it_be(:build_sast) { create(:ci_build, :success, name: 'sast', pipeline: pipeline) }
+  let_it_be(:artifact_sast) { create(:ee_ci_job_artifact, :sast, job: build_sast) }
+  let_it_be(:report_sast) { create(:ci_reports_security_report, type: :sast) }
+
+  let_it_be(:build_dast) { create(:ci_build, :success, name: 'dast', pipeline: pipeline) }
+  let_it_be(:artifact_dast) { create(:ee_ci_job_artifact, :dast, job: build_dast) }
+  let_it_be(:report_dast) { create(:ci_reports_security_report, type: :dast) }
+
+  let_it_be(:build_cs) { create(:ci_build, :success, name: 'container_scanning', pipeline: pipeline) }
+  let_it_be(:artifact_cs) { create(:ee_ci_job_artifact, :container_scanning, job: build_cs) }
+  let_it_be(:report_cs) { create(:ci_reports_security_report, type: :container_scanning) }
 
   context "The pipeline has security builds" do
-    before_all do
-      create(:ci_build, :success, name: 'dast_job', pipeline: pipeline, project: project) do |job|
-        create(:ee_ci_job_artifact, :dast, job: job, project: project)
-      end
-      create(:ci_build, :success, name: 'sast_job', pipeline: pipeline, project: project) do |job|
-        create(:ee_ci_job_artifact, :sast, job: job, project: project)
-      end
-      create(:ci_build, :success, name: 'cs_job', pipeline: pipeline, project: project) do |job|
-        create(:ee_ci_job_artifact, :container_scanning, job: job, project: project)
+    before(:all) do
+      ds_content = File.read(artifact_ds.file.path)
+      Gitlab::Ci::Parsers::Security::DependencyScanning.parse!(ds_content, report_ds)
+      report_ds.merge!(report_ds)
+
+      sast_content = File.read(artifact_sast.file.path)
+      Gitlab::Ci::Parsers::Security::Sast.parse!(sast_content, report_sast)
+      report_sast.merge!(report_sast)
+
+      dast_content = File.read(artifact_dast.file.path)
+      Gitlab::Ci::Parsers::Security::Dast.parse!(dast_content, report_dast)
+      report_dast.merge!(report_dast)
+
+      cs_content = File.read(artifact_cs.file.path)
+      Gitlab::Ci::Parsers::Security::ContainerScanning.parse!(cs_content, report_cs)
+      report_cs.merge!(report_cs)
+
+      { artifact_cs => report_cs, artifact_dast => report_dast, artifact_ds => report_ds, artifact_sast => report_sast }.each do |artifact, report|
+        scan = create(:security_scan, scan_type: artifact.job.name, build: artifact.job)
+
+        report.findings.each_with_index do |finding, index|
+          create(:security_finding,
+                 severity: finding.severity,
+                 confidence: finding.confidence,
+                 project_fingerprint: finding.project_fingerprint,
+                 deduplicated: true,
+                 position: index,
+                 scan: scan)
         end
-      create(:ci_build, :success, name: 'ds_job', pipeline: pipeline, project: project) do |job|
-        create(:ee_ci_job_artifact, :dependency_scanning, job: job, project: project)
       end
     end
 
     context 'All report types are requested' do
-      subject { described_class.new(pipeline, %w[sast dast container_scanning dependency_scanning]).execute }
+      subject do
+        described_class.new(pipeline, %w[sast dast container_scanning dependency_scanning]).execute
+      end
 
       it {
         is_expected.to match(a_hash_including("sast" => 33,