Merge branch '343332_fix_conflicting_location_fingerprints' into 'master'

Calculate location fingerprint by signature for findings if available

See merge request gitlab-org/gitlab!78389

ee/app/services/security/ingestion/finding_map.rb

@@ -50,7 +50,7 @@ module Security
                         project_fingerprint: project_fingerprint,
                         primary_identifier_id: identifier_ids.first,
                         location: report_finding.location_data,
-                        location_fingerprint: report_finding.location.fingerprint
+                        location_fingerprint: report_finding.location_fingerprint
                       )
       end
     end

ee/app/services/security/override_uuids_service.rb

@@ -120,7 +120,8 @@ module Security
     delegate :project, to: :pipeline, private: true
 
     def existing_scanners
-      @existing_scanners ||= project.vulnerability_scanners.index_by(&:external_id)
+      # Reloading the scanners will make sure that the collection proxy will be up-to-date.
+      @existing_scanners ||= project.vulnerability_scanners.reset.index_by(&:external_id)
     end
   end
 end

ee/app/services/security/store_report_service.rb

@@ -98,7 +98,7 @@ module Security
 
       vulnerability_finding_to_finding_map[vulnerability_finding] = finding
 
-      update_vulnerability_finding(vulnerability_finding, vulnerability_params.merge(location: entity_params[:location], location_fingerprint: finding.location.fingerprint))
+      update_vulnerability_finding(vulnerability_finding, vulnerability_params.merge(location: entity_params[:location], location_fingerprint: finding.location_fingerprint))
       reset_remediations_for(vulnerability_finding, finding)
 
       if project.licensed_feature_available?(:vulnerability_finding_signatures)
@@ -113,7 +113,7 @@ module Security
       find_params = {
         scanner: scanners_objects[finding.scanner.key],
         primary_identifier: identifiers_objects[finding.primary_identifier.key],
-        location_fingerprint: finding.location.fingerprint
+        location_fingerprint: finding.location_fingerprint
       }
 
       begin

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

@@ -472,4 +472,48 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
 
     it { is_expected.to eq([finding_2, finding_1, finding_3]) }
   end
+
+  describe '#location_fingerprint' do
+    let(:signature_1) { ::Gitlab::Ci::Reports::Security::FindingSignature.new(algorithm_type: 'location', signature_value: 'value1') }
+    let(:signature_2) { ::Gitlab::Ci::Reports::Security::FindingSignature.new(algorithm_type: 'scope_offset', signature_value: 'value2') }
+    let(:location) { build(:ci_reports_security_locations_sast) }
+    let(:finding) { build(:ci_reports_security_finding, vulnerability_finding_signatures_enabled: signatures_enabled, signatures: signatures, location: location) }
+
+    let(:fingerprint_from_location) { location.fingerprint }
+    let(:fingerprint_from_signature) { signature_2.signature_hex }
+
+    subject { finding.location_fingerprint }
+
+    context 'when the signatures feature is enabled' do
+      let(:signatures_enabled) { true }
+
+      context 'when the signatures are empty' do
+        let(:signatures) { [] }
+
+        it { is_expected.to eq(fingerprint_from_location) }
+      end
+
+      context 'when the signatures are not empty' do
+        let(:signatures) { [signature_1, signature_2] }
+
+        it { is_expected.to eq(fingerprint_from_signature) }
+      end
+    end
+
+    context 'when the signatures feature is not enabled' do
+      let(:signatures_enabled) { false }
+
+      context 'when the signatures are empty' do
+        let(:signatures) { [] }
+
+        it { is_expected.to eq(fingerprint_from_location) }
+      end
+
+      context 'when the signatures are not empty' do
+        let(:signatures) { [signature_1, signature_2] }
+
+        it { is_expected.to eq(fingerprint_from_location) }
+      end
+    end
+  end
 end

ee/spec/services/security/store_report_service_spec.rb

@@ -379,13 +379,16 @@ RSpec.describe Security::StoreReportService, '#execute', :snowplow do
       end
 
       it 'updates UUIDv4 to UUIDv5' do
+        # We run `OverrideUuidsService` if the signatures are enabled which
+        # kinda disables the logic of this test.
+        next if vulnerability_finding_signatures
+
         finding.uuid = '00000000-1111-2222-3333-444444444444'
         finding.save!
 
         # this report_finding should be used to update the finding's uuid
         report_finding = new_report.findings.find { |f| f.location.fingerprint == '0e7d0291d912f56880e39d4fbd80d99dd5d327ba' }
         allow(report_finding).to receive(:uuid).and_return(desired_uuid)
-        report_finding.signatures.pop
 
         subject
 

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

@@ -171,11 +171,21 @@ module Gitlab
             original_data['location']
           end
 
+          def location_fingerprint
+            max_priority_signature_hex || location&.fingerprint
+          end
+
           private
 
           def generate_project_fingerprint
             Digest::SHA1.hexdigest(compare_key)
           end
+
+          def max_priority_signature_hex
+            return unless @vulnerability_finding_signatures_enabled && signatures.present?
+
+            signatures.max_by(&:priority).signature_hex
+          end
         end
       end
     end