Merge branch...

Merge branch '235146-sidekiq-storesecurityreportsworker-nomethoderror-undefined-method-update-for-nil-nilclass' into 'master'

Check for scanner when creating vulnerability finding

Closes #235146

See merge request gitlab-org/gitlab!39500

ee/app/services/security/store_report_service.rb

@@ -31,7 +31,7 @@ module Security
     end
 
     def create_all_vulnerabilities!
-      @report.findings.map { |finding| create_vulnerability_finding(finding).id }.uniq
+      @report.findings.map { |finding| create_vulnerability_finding(finding)&.id }.compact.uniq
     end
 
     def mark_as_resolved_except(vulnerability_ids)
@@ -42,6 +42,8 @@ module Security
     end
 
     def create_vulnerability_finding(finding)
+      return if finding.scanner.blank?
+
       vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner)
       vulnerability_finding = create_or_find_vulnerability_finding(finding, vulnerability_params)
 
@@ -60,8 +62,6 @@ module Security
 
     # rubocop: disable CodeReuse/ActiveRecord
     def create_or_find_vulnerability_finding(finding, create_params)
-      return if finding.scanner.blank?
-
       find_params = {
         scanner: scanners_objects[finding.scanner.key],
         primary_identifier: identifiers_objects[finding.primary_identifier.key],
@@ -81,8 +81,6 @@ module Security
     end
 
     def update_vulnerability_scanner(finding)
-      return if finding.scanner.blank?
-
       scanner = scanners_objects[finding.scanner.key]
       scanner.update!(finding.scanner.to_hash)
     end

ee/changelogs/unreleased/235146-sidekiq-storesecurityreportsworker-nomethoderror-undefined-meth.yml

+---
+title: Fix scanner check when creating vulnerability findings
+merge_request: 39500
+author:
+type: fixed

ee/spec/services/security/store_report_service_spec.rb

@@ -153,6 +153,29 @@ RSpec.describe Security::StoreReportService, '#execute' do
         expect { subject }.to change { vulnerability.reload[:resolved_on_default_branch] }.from(true).to(false)
       end
     end
+
+    context 'when the finding does not include a scanner' do
+      let(:bad_pipeline) { create(:ci_pipeline, project: project) }
+      let(:bad_build) { create(:ci_build, pipeline: bad_pipeline) }
+      let!(:bad_artifact) { create(:ee_ci_job_artifact, :sast_with_missing_scanner, job: bad_build) }
+      let(:bad_report) { bad_pipeline.security_reports.get_report(report_type.to_s, bad_artifact) }
+      let(:report_type) { :sast }
+
+      before do
+        project.add_developer(user)
+        allow(bad_pipeline).to receive(:user).and_return(user)
+      end
+
+      subject { described_class.new(bad_pipeline, bad_report).execute }
+
+      it 'does not create a new finding' do
+        expect { subject }.not_to change { Vulnerabilities::Finding.count }
+      end
+
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
   end
 
   context 'with existing data from same pipeline' do