Merge branch 'store_only_valid_security_findings' into 'master'

Do not store invalid security findings

See merge request gitlab-org/gitlab!50868

ee/app/services/security/store_findings_metadata_service.rb

@@ -23,7 +23,6 @@ module Security
 
     private
 
-    delegate :findings, to: :report, prefix: true
     delegate :project, to: :security_scan
 
     def already_stored?
@@ -34,9 +33,11 @@ module Security
       report_findings.each_with_index { |report_finding, position| store_finding!(report_finding, position) }
     end
 
-    def store_finding!(report_finding, position)
-      return if report_finding.scanner.blank?
+    def report_findings
+      report.findings.select(&:valid?)
+    end
 
+    def store_finding!(report_finding, position)
       security_scan.findings.create!(finding_data(report_finding, position))
     end
 

ee/changelogs/unreleased/store_only_valid_security_findings.yml

+---
+title: Do not store invalid security_findings into the database
+merge_request: 50868
+author:
+type: fixed

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

@@ -93,7 +93,7 @@ module Gitlab
           end
 
           def valid?
-            scanner.present? && primary_identifier.present? && location.present?
+            scanner.present? && primary_identifier.present? && location.present? && uuid.present?
           end
 
           def keys

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

@@ -287,12 +287,14 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
     let(:scanner) { build(:ci_reports_security_scanner) }
     let(:identifiers) { [build(:ci_reports_security_identifier)] }
     let(:location) { build(:ci_reports_security_locations_sast) }
+    let(:uuid) { SecureRandom.uuid }
 
     let(:finding) do
       build(:ci_reports_security_finding,
             scanner: scanner,
             identifiers: identifiers,
             location: location,
+            uuid: uuid,
             compare_key: '')
     end
 
@@ -316,6 +318,12 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
       it { is_expected.to be_falsey }
     end
 
+    context 'when the uuid is missing' do
+      let(:uuid) { nil }
+
+      it { is_expected.to be_falsey }
+    end
+
     context 'when all required attributes present' do
       it { is_expected.to be_truthy }
     end

ee/spec/services/security/store_findings_metadata_service_spec.rb

@@ -7,11 +7,12 @@ RSpec.describe Security::StoreFindingsMetadataService do
   let_it_be(:project) { security_scan.project }
   let_it_be(:security_finding_1) { build(:ci_reports_security_finding) }
   let_it_be(:security_finding_2) { build(:ci_reports_security_finding) }
+  let_it_be(:security_finding_3) { build(:ci_reports_security_finding, uuid: nil) }
   let_it_be(:security_scanner) { build(:ci_reports_security_scanner) }
   let_it_be(:report) do
     build(
       :ci_reports_security_report,
-      findings: [security_finding_1, security_finding_2],
+      findings: [security_finding_1, security_finding_2, security_finding_3],
       scanners: [security_scanner]
     )
   end