Commit 65de4f54 authored by Can Eldem's avatar Can Eldem Committed by Rémy Coutable

Use fingerprint when comparing security reports in MR widget

Added test for new comparison
No change required in fixtures
parent 9e04c966
---
title: Use fingerprint when comparing security reports in MR widget
merge_request: 19654
author:
type: fixed
...@@ -84,8 +84,8 @@ module Security ...@@ -84,8 +84,8 @@ module Security
occurrence = Vulnerabilities::Occurrence.new(occurrence_hash) occurrence = Vulnerabilities::Occurrence.new(occurrence_hash)
# assigning Vulnerabilities to Findings to enable the computed state # assigning Vulnerabilities to Findings to enable the computed state
occurrence.location_fingerprint = report_occurrence.location.fingerprint
occurrence.vulnerability = vulnerabilities[occurrence.project_fingerprint] occurrence.vulnerability = vulnerabilities[occurrence.project_fingerprint]
occurrence.project = pipeline.project occurrence.project = pipeline.project
occurrence.sha = pipeline.sha occurrence.sha = pipeline.sha
occurrence.build_scanner(report_occurrence.scanner.to_hash) occurrence.build_scanner(report_occurrence.scanner.to_hash)
......
...@@ -246,13 +246,13 @@ module Vulnerabilities ...@@ -246,13 +246,13 @@ module Vulnerabilities
def eql?(other) def eql?(other)
other.report_type == report_type && other.report_type == report_type &&
other.location == location && other.location_fingerprint == location_fingerprint &&
other.first_fingerprint == first_fingerprint other.first_fingerprint == first_fingerprint
end end
# Array.difference (-) method uses hash and eql? methods to do comparison # Array.difference (-) method uses hash and eql? methods to do comparison
def hash def hash
report_type.hash ^ location.hash ^ first_fingerprint.hash report_type.hash ^ location_fingerprint.hash ^ first_fingerprint.hash
end end
def severity_value def severity_value
......
...@@ -83,24 +83,30 @@ describe Security::PipelineVulnerabilitiesFinder do ...@@ -83,24 +83,30 @@ describe Security::PipelineVulnerabilitiesFinder do
context 'by report type' do context 'by report type' do
context 'when sast' do context 'when sast' do
let(:params) { { report_type: %w[sast] } } 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 it 'includes only sast' do
expect(subject.map(&:location_fingerprint)).to match_array(sast_report_fingerprints)
expect(subject.count).to eq sast_count expect(subject.count).to eq sast_count
end end
end end
context 'when dependency_scanning' do context 'when dependency_scanning' do
let(:params) { { report_type: %w[dependency_scanning] } } 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 it 'includes only dependency_scanning' do
expect(subject.map(&:location_fingerprint)).to match_array(ds_report_fingerprints)
expect(subject.count).to eq ds_count expect(subject.count).to eq ds_count
end end
end end
context 'when dast' do context 'when dast' do
let(:params) { { report_type: %w[dast] } } 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 it 'includes only dast' do
expect(subject.map(&:location_fingerprint)).to match_array(dast_report_fingerprints)
expect(subject.count).to eq dast_count expect(subject.count).to eq dast_count
end end
end end
...@@ -109,6 +115,8 @@ describe Security::PipelineVulnerabilitiesFinder do ...@@ -109,6 +115,8 @@ describe Security::PipelineVulnerabilitiesFinder do
let(:params) { { report_type: %w[container_scanning] } } let(:params) { { report_type: %w[container_scanning] } }
it 'includes only container_scanning' do 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 expect(subject.count).to eq cs_count
end end
end end
......
...@@ -24,6 +24,16 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do ...@@ -24,6 +24,16 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
expect(comparer.existing).to eq([head_vulnerability]) expect(comparer.existing).to eq([head_vulnerability])
end 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 it 'does not change order' do
comparer = described_class.new([base_vulnerability, vuln], [head_vulnerability, vuln, low_vuln]) comparer = described_class.new([base_vulnerability, vuln], [head_vulnerability, vuln, low_vuln])
...@@ -45,6 +55,16 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do ...@@ -45,6 +55,16 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
expect(comparer.added).to eq([vuln]) expect(comparer.added).to eq([vuln])
end 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 it 'does not change order' do
comparer = described_class.new([base_vulnerability], [head_vulnerability, vuln, low_vuln]) comparer = described_class.new([base_vulnerability], [head_vulnerability, vuln, low_vuln])
...@@ -64,6 +84,17 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do ...@@ -64,6 +84,17 @@ describe Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer do
expect(comparer.fixed).to eq([vuln]) expect(comparer.fixed).to eq([vuln])
end 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 it 'does not change order' do
comparer = described_class.new([vuln, medium_vuln, base_vulnerability], [head_vulnerability]) comparer = described_class.new([vuln, medium_vuln, base_vulnerability], [head_vulnerability])
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment