Merge branch '238156_store_scan_findings' into 'master'

Store security findings in the database

See merge request gitlab-org/gitlab!41032

ee/app/models/security/scan.rb

@@ -21,5 +21,7 @@ module Security
       secret_detection: 5,
       coverage_fuzzing: 6
     }
+
+    delegate :project, to: :build
   end
 end

ee/app/services/security/store_findings_metadata_service.rb

+# frozen_string_literal: true
+
+module Security
+  # This service class stores the findings metadata for all pipelines.
+  class StoreFindingsMetadataService < ::BaseService
+    attr_reader :security_scan, :report
+
+    def self.execute(security_scan, report)
+      new(security_scan, report).execute
+    end
+
+    def initialize(security_scan, report)
+      @security_scan = security_scan
+      @report = report
+    end
+
+    def execute
+      return error('Findings are already stored!') if already_stored?
+
+      store_findings
+      success
+    end
+
+    private
+
+    delegate :findings, to: :report, prefix: true
+    delegate :project, to: :security_scan
+
+    def already_stored?
+      security_scan.findings.any?
+    end
+
+    def store_findings
+      report_findings.each { |report_finding| store_finding!(report_finding) }
+    end
+
+    def store_finding!(report_finding)
+      return if report_finding.scanner.blank?
+
+      security_scan.findings.create!(finding_data(report_finding))
+    end
+
+    def finding_data(report_finding)
+      {
+        severity: report_finding.severity,
+        confidence: report_finding.confidence,
+        project_fingerprint: report_finding.project_fingerprint,
+        scanner: persisted_scanner_for(report_finding.scanner)
+      }
+    end
+
+    def persisted_scanner_for(report_scanner)
+      existing_scanners[report_scanner.key] ||= create_scanner!(report_scanner)
+    end
+
+    def existing_scanners
+      @existing_scanners ||= project.vulnerability_scanners
+                                    .with_external_id(scanner_external_ids)
+                                    .group_by(&:external_id)
+                                    .transform_values(&:first)
+    end
+
+    def scanner_external_ids
+      report.scanners.values.map(&:external_id)
+    end
+
+    def create_scanner!(report_scanner)
+      project.vulnerability_scanners.create!(report_scanner.to_hash)
+    end
+  end
+end

ee/app/services/security/store_scans_service.rb

@@ -7,17 +7,30 @@ module Security
     end
 
     def execute
-      return if @build.canceled? || @build.skipped?
+      return if canceled_or_skipped?
 
-      security_reports = @build.job_artifacts.security_reports
+      security_reports.each { |_, report| store_scan_for(report) }
+    end
 
-      ActiveRecord::Base.transaction do
-        security_reports.each do |report|
-          Security::Scan.safe_find_or_create_by!(
-            build: @build,
-            scan_type: report.file_type
-          )
+    private
+
+    attr_reader :build
+
+    def canceled_or_skipped?
+      build.canceled? || build.skipped?
+    end
+
+    def security_reports
+      ::Gitlab::Ci::Reports::Security::Reports.new(self).tap do |security_reports|
+        build.collect_security_reports!(security_reports)
+      end
     end
+
+    def store_scan_for(report)
+      ActiveRecord::Base.transaction do
+        security_scan = Security::Scan.safe_find_or_create_by!(build: build, scan_type: report.type)
+
+        StoreFindingsMetadataService.execute(security_scan, report)
       end
     end
   end

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

@@ -7,7 +7,7 @@ module Gitlab
         class Reports
           attr_reader :reports, :pipeline
 
-          delegate :empty?, to: :reports
+          delegate :each, :empty?, to: :reports
 
           def initialize(pipeline)
             @reports = {}

ee/spec/factories/security/findings.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :security_finding, class: 'Security::Finding' do
+    scanner factory: :vulnerabilities_scanner
+    scan factory: :security_scan
+
+    severity { :critical }
+    confidence { :high }
+    project_fingerprint { generate(:project_fingerprint) }
+  end
+end

ee/spec/models/security/scan_spec.rb

@@ -14,5 +14,9 @@ RSpec.describe Security::Scan do
     it { is_expected.to validate_presence_of(:scan_type) }
   end
 
+  describe '#project' do
+    it { is_expected.to delegate_method(:project).to(:build) }
+  end
+
   it_behaves_like 'having unique enum values'
 end

ee/spec/services/security/store_findings_metadata_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::StoreFindingsMetadataService do
+  let_it_be(:security_scan) { create(:security_scan) }
+  let_it_be(:project) { security_scan.project }
+  let_it_be(:security_finding) { build(:ci_reports_security_finding) }
+  let_it_be(:security_scanner) { build(:ci_reports_security_scanner) }
+  let_it_be(:report) do
+    build(
+      :ci_reports_security_report,
+      findings: [security_finding],
+      scanners: [security_scanner]
+    )
+  end
+
+  describe '#execute' do
+    let(:service_object) { described_class.new(security_scan, report) }
+
+    subject(:store_findings) { service_object.execute }
+
+    context 'when the given security scan already has findings' do
+      before do
+        create(:security_finding, scan: security_scan)
+      end
+
+      it 'does not create new findings in database' do
+        expect { store_findings }.not_to change { Security::Finding.count }
+      end
+    end
+
+    context 'when the given security scan does not have any findings' do
+      before do
+        security_scan.findings.delete_all
+      end
+
+      it 'creates the security finding entries in database' do
+        expect { store_findings }.to change { security_scan.findings.count }.by(1)
+                                 .and change { security_scan.findings.last&.severity }.to(security_finding.severity.to_s)
+                                 .and change { security_scan.findings.last&.confidence }.to(security_finding.confidence.to_s)
+                                 .and change { security_scan.findings.last&.project_fingerprint }.to(security_finding.project_fingerprint)
+      end
+
+      context 'when the scanners already exist in the database' do
+        before do
+          create(:vulnerabilities_scanner, project: project, external_id: security_scanner.key)
+        end
+
+        it 'does not create new scanner entries in the database' do
+          expect { store_findings }.not_to change { Vulnerabilities::Scanner.count }
+        end
+      end
+
+      context 'when the scanner does not exist in the database' do
+        it 'creates new scanner entry in the database' do
+          expect { store_findings }.to change { project.vulnerability_scanners.count }.by(1)
+        end
+      end
+    end
+  end
+end

ee/spec/services/security/store_scans_service_spec.rb

@@ -7,6 +7,10 @@ RSpec.describe Security::StoreScansService do
 
   subject { Security::StoreScansService.new(build).execute }
 
+  before do
+    allow(Security::StoreFindingsMetadataService).to receive(:execute)
+  end
+
   context 'build has security reports' do
     before do
       create(:ee_ci_job_artifact, :dast, job: build)
@@ -22,6 +26,12 @@ RSpec.describe Security::StoreScansService do
       expect(scans.sast.count).to be(1)
       expect(scans.dast.count).to be(1)
     end
+
+    it 'calls the StoreFindingsMetadataService' do
+      subject
+
+      expect(Security::StoreFindingsMetadataService).to have_received(:execute).twice
+    end
   end
 
   context 'scan already exists' do
@@ -35,5 +45,11 @@ RSpec.describe Security::StoreScansService do
 
       expect(Security::Scan.where(build: build).count).to be(1)
     end
+
+    it 'calls the StoreFindingsMetadataService' do
+      subject
+
+      expect(Security::StoreFindingsMetadataService).to have_received(:execute).once
+    end
   end
 end