Merge branch '343037_fix_override_uuid_service' into 'master'

Fix `Security::OverrideUuidsService` matching logic

See merge request gitlab-org/gitlab!72723

ee/app/models/vulnerabilities/finding_signature.rb

@@ -13,7 +13,7 @@ module Vulnerabilities
 
     scope :by_project, -> (project) { joins(:finding).where(vulnerability_occurrences: { project_id: project.id }) }
     scope :by_signature_sha, -> (shas) { where(signature_sha: shas) }
-    scope :eager_load_finding, -> { includes(:finding) }
+    scope :eager_load_comparison_entities, -> { includes(finding: [:scanner, :primary_identifier]) }
 
     def signature_hex
       signature_sha.unpack1("H*")

ee/app/services/security/override_uuids_service.rb

@@ -34,13 +34,20 @@ module Security
     def existing_finding_by_signature(finding)
       shas = finding.signatures.sort_by(&:priority).map(&:signature_sha)
 
-      existing_signatures.values_at(*shas).first&.finding
+      existing_signatures.values_at(*shas).compact.map(&:finding).find do |existing_finding|
+        existing_finding.primary_identifier&.fingerprint == finding.primary_identifier_fingerprint &&
+          existing_finding.scanner == existing_scanners[finding.scanner.external_id]
+      end
+    end
+
+    def existing_scanners
+      @existing_scanners ||= pipeline.project.vulnerability_scanners.index_by(&:external_id)
     end
 
     def existing_signatures
       @existing_signatures ||= ::Vulnerabilities::FindingSignature.by_signature_sha(finding_signature_shas)
         .by_project(pipeline.project)
-        .eager_load_finding
+        .eager_load_comparison_entities
         .index_by(&:signature_sha)
     end
 

ee/spec/services/security/override_uuids_service_spec.rb

@@ -5,12 +5,20 @@ require 'spec_helper'
 RSpec.describe Security::OverrideUuidsService do
   describe '#execute' do
     let(:vulnerability_finding_uuid) { SecureRandom.uuid }
-    let(:report_finding_uuid) { SecureRandom.uuid }
+    let(:matching_report_finding_uuid) { SecureRandom.uuid }
+
     let(:pipeline) { create(:ci_pipeline) }
-    let(:vulnerability_finding) { create(:vulnerabilities_finding, project: pipeline.project, uuid: vulnerability_finding_uuid) }
+    let(:vulnerability_scanner) { create(:vulnerabilities_scanner, external_id: 'gitlab-sast', project: pipeline.project) }
+    let(:vulnerability_identifier) { create(:vulnerabilities_identifier, fingerprint: 'e2bd6788a715674769f48fadffd0bd3ea16656f5') }
+    let(:vulnerability_finding) { create(:vulnerabilities_finding, project: pipeline.project, uuid: vulnerability_finding_uuid, primary_identifier: vulnerability_identifier, scanner: vulnerability_scanner) }
+
     let(:signature) { ::Gitlab::Ci::Reports::Security::FindingSignature.new(algorithm_type: 'location', signature_value: 'value') }
-    let(:report_finding) { create(:ci_reports_security_finding, uuid: report_finding_uuid, vulnerability_finding_signatures_enabled: true, signatures: [signature]) }
-    let(:report) { create(:ci_reports_security_report, findings: [report_finding], pipeline: pipeline) }
+    let(:report_scanner) { create(:ci_reports_security_scanner, external_id: 'gitlab-sast') }
+    let(:matching_report_identifier) { create(:ci_reports_security_identifier, external_id: vulnerability_identifier.external_id, external_type: vulnerability_identifier.external_type) }
+    let(:matching_report_finding) { create(:ci_reports_security_finding, uuid: matching_report_finding_uuid, vulnerability_finding_signatures_enabled: true, signatures: [signature], identifiers: [matching_report_identifier], scanner: report_scanner) }
+    let(:unmatching_report_finding) { create(:ci_reports_security_finding, vulnerability_finding_signatures_enabled: true, signatures: [signature], scanner: report_scanner) }
+    let(:report) { create(:ci_reports_security_report, findings: [matching_report_finding, unmatching_report_finding], pipeline: pipeline) }
+
     let(:service_object) { described_class.new(report) }
 
     before do
@@ -20,8 +28,9 @@ RSpec.describe Security::OverrideUuidsService do
     subject(:override_uuids) { service_object.execute }
 
     it 'overrides finding uuids' do
-      expect { override_uuids }.to change { report_finding.uuid }.from(report_finding_uuid).to(vulnerability_finding_uuid)
-                               .and change { report_finding.overridden_uuid }.from(nil).to(report_finding_uuid)
+      expect { override_uuids }.to change { matching_report_finding.uuid }.from(matching_report_finding_uuid).to(vulnerability_finding_uuid)
+                               .and change { matching_report_finding.overridden_uuid }.from(nil).to(matching_report_finding_uuid)
+                               .and not_change { unmatching_report_finding.uuid }
     end
   end
 end