Merge branch 'consider-location-fingerprint-in-mr-widget' into 'master'

Use fingerprint when comparing security reports in MR widget

See merge request gitlab-org/gitlab!19654

changelogs/unreleased/consider-location-fingerprint-in-mr-widget.yml

+---
+title: Use fingerprint when comparing security reports in MR widget
+merge_request: 19654
+author:
+type: fixed

ee/app/finders/security/pipeline_vulnerabilities_finder.rb

@@ -84,8 +84,8 @@ module Security
 
         occurrence = Vulnerabilities::Occurrence.new(occurrence_hash)
         # assigning Vulnerabilities to Findings to enable the computed state
+        occurrence.location_fingerprint = report_occurrence.location.fingerprint
         occurrence.vulnerability = vulnerabilities[occurrence.project_fingerprint]
-
         occurrence.project = pipeline.project
         occurrence.sha = pipeline.sha
         occurrence.build_scanner(report_occurrence.scanner.to_hash)

ee/app/models/vulnerabilities/occurrence.rb

@@ -246,13 +246,13 @@ module Vulnerabilities
 
     def eql?(other)
       other.report_type == report_type &&
-        other.location == location &&
+        other.location_fingerprint == location_fingerprint &&
         other.first_fingerprint == first_fingerprint
     end
 
     # Array.difference (-) method uses hash and eql? methods to do comparison
     def hash
-      report_type.hash ^ location.hash ^ first_fingerprint.hash
+      report_type.hash ^ location_fingerprint.hash ^ first_fingerprint.hash
     end
 
     def severity_value

ee/spec/finders/security/pipeline_vulnerabilities_finder_spec.rb

@@ -83,24 +83,30 @@ describe Security::PipelineVulnerabilitiesFinder do
     context 'by report type' do
       context 'when sast' do
         let(:params) { { report_type: %w[sast] } }
+        let(:sast_report_fingerprints) {pipeline.security_reports.reports['sast'].occurrences.map(&:location).map(&:fingerprint) }
 
         it 'includes only sast' do
+          expect(subject.map(&:location_fingerprint)).to match_array(sast_report_fingerprints)
           expect(subject.count).to eq sast_count
         end
       end
 
       context 'when dependency_scanning' do
         let(:params) { { report_type: %w[dependency_scanning] } }
+        let(:ds_report_fingerprints) {pipeline.security_reports.reports['dependency_scanning'].occurrences.map(&:location).map(&:fingerprint) }
 
         it 'includes only dependency_scanning' do
+          expect(subject.map(&:location_fingerprint)).to match_array(ds_report_fingerprints)
           expect(subject.count).to eq ds_count
         end
       end
 
       context 'when dast' do
         let(:params) { { report_type: %w[dast] } }
+        let(:dast_report_fingerprints) {pipeline.security_reports.reports['dast'].occurrences.map(&:location).map(&:fingerprint) }
 
         it 'includes only dast' do
+          expect(subject.map(&:location_fingerprint)).to match_array(dast_report_fingerprints)
           expect(subject.count).to eq dast_count
         end
       end
@@ -109,6 +115,8 @@ describe Security::PipelineVulnerabilitiesFinder do
         let(:params) { { report_type: %w[container_scanning] } }
 
         it 'includes only container_scanning' do
+          fingerprints = pipeline.security_reports.reports['container_scanning'].occurrences.map(&:location).map(&:fingerprint)
+          expect(subject.map(&:location_fingerprint)).to match_array(fingerprints)
           expect(subject.count).to eq cs_count
         end
       end

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

@@ -24,6 +24,16 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
         expect(comparer.existing).to eq([head_vulnerability])
       end
 
+      context "when comparing reports with different fingerprints" do
+        let!(:base_vulnerability) { build(:vulnerabilities_occurrence, report_type: :sast, identifiers: [identifier], location_fingerprint: 'A') }
+        let!(:head_vulnerability) { build(:vulnerabilities_occurrence, report_type: :sast, identifiers: [identifier], location_fingerprint: 'B') }
+
+        it "does not find any overlap" do
+          comparer = described_class.new([base_vulnerability], [head_vulnerability])
+          expect(comparer.existing).to eq([])
+        end
+      end
+
       it 'does not change order' do
         comparer = described_class.new([base_vulnerability, vuln], [head_vulnerability, vuln, low_vuln])
 
@@ -45,6 +55,16 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
         expect(comparer.added).to eq([vuln])
       end
 
+      context "when comparing reports with different fingerprints" do
+        let!(:base_vulnerability) { build(:vulnerabilities_occurrence, report_type: :sast, identifiers: [identifier], location_fingerprint: 'A') }
+        let!(:head_vulnerability) { build(:vulnerabilities_occurrence, report_type: :sast, identifiers: [identifier], location_fingerprint: 'B') }
+
+        it "does not find any overlap" do
+          comparer = described_class.new([base_vulnerability], [head_vulnerability, vuln])
+          expect(comparer.added).to eq([head_vulnerability, vuln])
+        end
+      end
+
       it 'does not change order' do
         comparer = described_class.new([base_vulnerability], [head_vulnerability, vuln, low_vuln])
 
@@ -64,6 +84,17 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
         expect(comparer.fixed).to eq([vuln])
       end
 
+      context "when comparing reports with different fingerprints" do
+        let!(:base_vulnerability) { build(:vulnerabilities_occurrence, report_type: :sast, identifiers: [identifier], location_fingerprint: 'A') }
+        let!(:head_vulnerability) { build(:vulnerabilities_occurrence, report_type: :sast, identifiers: [identifier], location_fingerprint: 'B') }
+
+        it "does not find any overlap" do
+          comparer = described_class.new([base_vulnerability, vuln], [head_vulnerability])
+
+          expect(comparer.fixed).to eq([base_vulnerability, vuln])
+        end
+      end
+
       it 'does not change order' do
         comparer = described_class.new([vuln, medium_vuln, base_vulnerability], [head_vulnerability])