diff --git a/app/models/concerns/vulnerability_finding_signature_helpers.rb b/app/models/concerns/vulnerability_finding_signature_helpers.rb
index a225625815b14a44683f8ed6bd67b58c5ca358cf..01d48eac7cbecc0fba76a46bd1dfccba5b39f515 100644
--- a/app/models/concerns/vulnerability_finding_signature_helpers.rb
+++ b/app/models/concerns/vulnerability_finding_signature_helpers.rb
@@ -11,7 +11,8 @@ module VulnerabilityFindingSignatureHelpers
hash: 1,
location: 2,
scope_offset: 3,
- scope_offset_compressed: 4
+ scope_offset_compressed: 4,
+ rule_value: 5
}.with_indifferent_access.freeze
class_methods do
diff --git a/ee/app/services/security/ingestion/tasks/update_vulnerability_uuids.rb b/ee/app/services/security/ingestion/tasks/update_vulnerability_uuids.rb
index 10c24ebffcd9763911e56482885860473c68f729..34211a58ed71d1b7f28eaec20fb88dd9211b166c 100644
--- a/ee/app/services/security/ingestion/tasks/update_vulnerability_uuids.rb
+++ b/ee/app/services/security/ingestion/tasks/update_vulnerability_uuids.rb
@@ -3,7 +3,21 @@
module Security
module Ingestion
module Tasks
+ # This task is responsible for updating `uuid` DB column of:
+ # - vulnerability_occurrences table (via UpdateVulnerabilityUuids::VulnerabilityFindings task)
+ # - vulnerability_feedback table (via UpdateVulnerabilityUuids::VulnerabilityFeedback task)
+ # - vulnerability_reads table (via UpdateVulnerabilityUuids::VulnerabilityReads task)
+ # with the `uuid` value of the latest finding in the pipeline. This method prevents the
+ # duplication of newly identified findings, which could represent the same vulnerabilities
+ # stored in the database, sharing identical definitions and locations but differentiated by
+ # their fingerprint algorithms
+ #
+ # This task currently updates the findings of the following analyzers:
+ # - Semgrep SAST Analyzer (semgrep)
+ # - Secret Detection Analyzer (gitleaks)
class UpdateVulnerabilityUuids < AbstractTask
+ ALLOWED_SCANNERS = %w[semgrep gitleaks].freeze
+
def execute
return unless update_uuids?
@@ -46,7 +60,7 @@ def existing_uuids(finding_map_uuids)
end
def update_uuids?
- scanners.include?("semgrep")
+ scanners.intersect?(ALLOWED_SCANNERS)
end
def update_uuids
diff --git a/ee/app/services/security/override_uuids_service.rb b/ee/app/services/security/override_uuids_service.rb
index a4e3c20119ac7704eaf8004aae8d4121156267b4..36b091799526178c13b5b9f35e30f3e57cef14f9 100644
--- a/ee/app/services/security/override_uuids_service.rb
+++ b/ee/app/services/security/override_uuids_service.rb
@@ -17,10 +17,11 @@ def self.execute(security_report)
def initialize(security_report)
@security_report = security_report
@known_uuids = findings.map(&:uuid).to_set
+ @signatures_enabled = project.licensed_feature_available?(:vulnerability_finding_signatures)
end
def execute
- return unless type.to_s == 'sast' && has_signatures?
+ return unless override_uuids?
findings.each_slice(BATCH_SIZE) { |batch| OverrideInBatch.execute(project, batch, existing_scanners, known_uuids) }
@@ -101,7 +102,7 @@ def finding_signature_shas
def existing_findings_by_location
@existing_findings_by_location ||= project.vulnerability_findings
- .sast
+ .by_report_types([report_type])
.by_location_fingerprints(location_fingerprints)
.eager_load_comparison_entities
.group_by(&:location_fingerprint)
@@ -110,11 +111,15 @@ def existing_findings_by_location
def location_fingerprints
findings.map(&:location).compact.map(&:fingerprint)
end
+
+ def report_type
+ findings.first&.report_type
+ end
end
private
- attr_reader :security_report, :known_uuids
+ attr_reader :security_report, :known_uuids, :signatures_enabled
delegate :pipeline, :findings, :type, :has_signatures?, to: :security_report, private: true
delegate :project, to: :pipeline, private: true
@@ -123,5 +128,9 @@ def existing_scanners
# 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
+
+ def override_uuids?
+ signatures_enabled && has_signatures?
+ end
end
end
diff --git a/ee/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb b/ee/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
index bcb4bd74924dc784ac056c90401048cd87797335..7cd0ff814af1bdef8168cd7a8211b085d6110c5a 100644
--- a/ee/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
+++ b/ee/lib/gitlab/ci/reports/security/vulnerability_reports_comparer.rb
@@ -146,11 +146,11 @@ def override_uuids_for!(findings)
end
def override_uuids?
- signatures_enabled && sast_report?
+ signatures_enabled && has_signatures?
end
- def sast_report?
- head_report.findings.first&.report_type.to_s == 'sast'
+ def has_signatures?
+ head_report.findings.any?(&:signatures)
end
end
@@ -220,7 +220,7 @@ def finding_signature_shas
def persisted_findings_by_location
@persisted_findings_by_location ||= project.vulnerability_findings
- .sast
+ .by_report_types([report_type])
.by_location_fingerprints(location_fingerprints)
.eager_load_comparison_entities
.group_by(&:location_fingerprint)
@@ -237,6 +237,10 @@ def known_uuids
def scanners
@scanners ||= project.vulnerability_scanners.index_by(&:external_id)
end
+
+ def report_type
+ findings.first&.report_type
+ end
end
class FindingMatcher
diff --git a/ee/spec/services/security/ingestion/tasks/update_vulnerability_uuids_spec.rb b/ee/spec/services/security/ingestion/tasks/update_vulnerability_uuids_spec.rb
index 3e29b38b00b3e4ab7aba7c091c179d179fe4be26..7a461cbe9a2edb8dcf6f5d1b068bbb9c3fac2d82 100644
--- a/ee/spec/services/security/ingestion/tasks/update_vulnerability_uuids_spec.rb
+++ b/ee/spec/services/security/ingestion/tasks/update_vulnerability_uuids_spec.rb
@@ -221,6 +221,60 @@
end
end
+ context 'when updating the uuids' do
+ let_it_be(:secret_detection_scanner) do
+ build(:ci_reports_security_scanner, external_id: 'gitleaks', name: 'Gitleaks')
+ end
+
+ let_it_be(:spotbugs_scanner) do
+ build(:ci_reports_security_scanner, external_id: 'spotbugs', name: 'Spotbugs')
+ end
+
+ let(:report_finding1) do
+ create(
+ :ci_reports_security_finding,
+ scanner: semgrep_scanner
+ )
+ end
+
+ let(:report_finding2) do
+ create(
+ :ci_reports_security_finding,
+ scanner: secret_detection_scanner
+ )
+ end
+
+ let(:report_finding3) do
+ create(
+ :ci_reports_security_finding,
+ scanner: spotbugs_scanner
+ )
+ end
+
+ let(:finding_map1) { create(:finding_map, report_finding: report_finding1) }
+ let(:finding_map2) { create(:finding_map, report_finding: report_finding2) }
+ let(:finding_map3) { create(:finding_map, report_finding: report_finding3) }
+
+ let(:service_object1) { described_class.new(pipeline, [finding_map1]) }
+ let(:service_object2) { described_class.new(pipeline, [finding_map2]) }
+ let(:service_object3) { described_class.new(pipeline, [finding_map3]) }
+
+ it 'allowed for semgrep findings' do
+ expect(service_object1).to receive(:update_uuids)
+ service_object1.execute
+ end
+
+ it 'allowed for secret detection findings' do
+ expect(service_object2).to receive(:update_uuids)
+ service_object2.execute
+ end
+
+ it 'not allowed other findings' do
+ expect(service_object3).not_to receive(:update_uuids)
+ service_object3.execute
+ end
+ end
+
private
def create_vulnerability(severity: 7, confidence: 7, report_type: 0)
diff --git a/ee/spec/services/security/override_uuids_service_spec.rb b/ee/spec/services/security/override_uuids_service_spec.rb
index a05e372e93b4159e32e5616fe1d04a335b39e062..d549c54281f848a5137899bd019b06af2caff84a 100644
--- a/ee/spec/services/security/override_uuids_service_spec.rb
+++ b/ee/spec/services/security/override_uuids_service_spec.rb
@@ -71,15 +71,35 @@
subject(:override_uuids) { service_object.execute }
- it 'overrides finding uuids and prioritizes the existing findings' do
- expect { override_uuids }
- .to change { report.findings.map(&:overridden_uuid) }.from(Array.new(4) { nil }).to([an_instance_of(String), an_instance_of(String), nil, nil])
- .and change { matching_report_finding_by_signature.uuid }.from(matching_report_finding_uuid_1).to(vulnerability_finding_uuid_1)
- .and change { matching_report_finding_by_signature.overridden_uuid }.from(nil).to(matching_report_finding_uuid_1)
- .and change { matching_report_finding_by_location.uuid }.from(matching_report_finding_uuid_2).to(vulnerability_finding_uuid_2)
- .and change { matching_report_finding_by_location.overridden_uuid }.from(nil).to(matching_report_finding_uuid_2)
- .and not_change { matching_report_finding_by_location_conflict.uuid }
- .and not_change { unmatching_report_finding.uuid }
+ context 'when the `vulnerability_finding_signatures` is enabled' do
+ before do
+ stub_licensed_features(vulnerability_finding_signatures: true)
+ end
+
+ it 'overrides finding uuids and prioritizes the existing findings' do
+ expect { override_uuids }
+ .to change { report.findings.map(&:overridden_uuid) }.from(Array.new(4) { nil }).to([an_instance_of(String), an_instance_of(String), nil, nil])
+ .and change { matching_report_finding_by_signature.uuid }.from(matching_report_finding_uuid_1).to(vulnerability_finding_uuid_1)
+ .and change { matching_report_finding_by_signature.overridden_uuid }.from(nil).to(matching_report_finding_uuid_1)
+ .and change { matching_report_finding_by_location.uuid }.from(matching_report_finding_uuid_2).to(vulnerability_finding_uuid_2)
+ .and change { matching_report_finding_by_location.overridden_uuid }.from(nil).to(matching_report_finding_uuid_2)
+ .and not_change { matching_report_finding_by_location_conflict.uuid }
+ .and not_change { unmatching_report_finding.uuid }
+ end
+ end
+
+ context 'when the `vulnerability_finding_signatures` is disabled' do
+ before do
+ stub_licensed_features(vulnerability_finding_signatures: false)
+ end
+
+ it 'does not override finding uuids despite signatures being present' do
+ expect { override_uuids }
+ .to not_change { matching_report_finding_by_signature.uuid }
+ .and not_change { matching_report_finding_by_signature.overridden_uuid }
+ .and not_change { matching_report_finding_by_location.uuid }
+ .and not_change { matching_report_finding_by_location.overridden_uuid }
+ end
end
end
end
diff --git a/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb b/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb
index 842020896d9cce84f70b65d3d93fd1b131817ba7..c2299c839e7be77974808978e8d0ad6558805bf5 100644
--- a/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb
+++ b/spec/models/concerns/vulnerability_finding_signature_helpers_spec.rb
@@ -16,6 +16,7 @@ def initialize(algorithm_type)
describe '#priority' do
it 'returns numeric values of the priority string' do
+ expect(cls.new('rule_value').priority).to eq(5)
expect(cls.new('scope_offset_compressed').priority).to eq(4)
expect(cls.new('scope_offset').priority).to eq(3)
expect(cls.new('location').priority).to eq(2)
@@ -25,6 +26,7 @@ def initialize(algorithm_type)
describe '#self.priority' do
it 'returns the numeric value of the provided string' do
+ expect(cls.priority('rule_value')).to eq(5)
expect(cls.priority('scope_offset_compressed')).to eq(4)
expect(cls.priority('scope_offset')).to eq(3)
expect(cls.priority('location')).to eq(2)