From bf7ef976fe5067e887e6a6df14949906467e4f31 Mon Sep 17 00:00:00 2001 From: Brian Williams Date: Tue, 16 Dec 2025 13:14:00 -0600 Subject: [PATCH] Set finding id when ingesting sbom_occurrences_vulnerability records This will support the vulnerabilities on multiple branches project, as a vulnerability can now have multiple vulnerability_occurrences. --- .../services/sbom/ingestion/occurrence_map.rb | 4 +-- .../ingestion/tasks/ingest_occurrences.rb | 8 ++--- .../ingest_occurrences_vulnerabilities.rb | 17 +++++++-- .../sbom/ingestion/vulnerability_data.rb | 36 ++++++++++--------- .../sbom/ingestion/occurrence_map_spec.rb | 2 +- .../tasks/ingest_occurrences_spec.rb | 14 ++++---- ...ingest_occurrences_vulnerabilities_spec.rb | 34 +++++++++++++----- .../sbom/ingestion/vulnerability_data_spec.rb | 4 +-- 8 files changed, 75 insertions(+), 44 deletions(-) diff --git a/ee/app/services/sbom/ingestion/occurrence_map.rb b/ee/app/services/sbom/ingestion/occurrence_map.rb index 25f8b5796fcf33..5a39bd10135ace 100644 --- a/ee/app/services/sbom/ingestion/occurrence_map.rb +++ b/ee/app/services/sbom/ingestion/occurrence_map.rb @@ -7,12 +7,12 @@ class OccurrenceMap attr_reader :report_component, :report_source attr_accessor :component_id, :component_version_id, :source_id, :occurrence_id, :source_package_id, :uuid, - :vulnerability_ids + :vulnerability_info def initialize(report_component, report_source) @report_component = report_component @report_source = report_source - @vulnerability_ids = [] + @vulnerability_info = VulnerabilityData::VulnerabilityInfo.new end def to_h diff --git a/ee/app/services/sbom/ingestion/tasks/ingest_occurrences.rb b/ee/app/services/sbom/ingestion/tasks/ingest_occurrences.rb index a40f21a83c0546..48451550cea7d1 100644 --- a/ee/app/services/sbom/ingestion/tasks/ingest_occurrences.rb +++ b/ee/app/services/sbom/ingestion/tasks/ingest_occurrences.rb @@ -31,7 +31,7 @@ def attributes occurrence_maps.filter_map do |occurrence_map| uuid = occurrence_map.uuid - vulnerability_info = vulnerability_data.for(occurrence_map) + occurrence_map.vulnerability_info = vulnerability_data.for(occurrence_map) new_attributes = { project_id: project.id, @@ -46,16 +46,14 @@ def attributes input_file_path: occurrence_map.input_file_path, licenses: licenses.fetch(occurrence_map.report_component), component_name: occurrence_map.name, - highest_severity: vulnerability_info.highest_severity, - vulnerability_count: vulnerability_info.count, + highest_severity: occurrence_map.vulnerability_info.highest_severity, + vulnerability_count: occurrence_map.vulnerability_info.count, traversal_ids: project.namespace.traversal_ids, archived: project.archived, ancestors: occurrence_map.ancestors, reachability: occurrence_map.reachability } - occurrence_map.vulnerability_ids = vulnerability_info.vulnerability_ids - if attributes_changed?(new_attributes) # Remove updated items from the list so that we don't have to iterate over them # twice when setting the ids in `after_ingest`. diff --git a/ee/app/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities.rb b/ee/app/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities.rb index bedafd69b03976..0c2a2d052a0372 100644 --- a/ee/app/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities.rb +++ b/ee/app/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities.rb @@ -25,7 +25,8 @@ def existing_links existing_records.map do |link| { sbom_occurrence_id: link.sbom_occurrence_id, - vulnerability_id: link.vulnerability_id + vulnerability_id: link.vulnerability_id, + vulnerability_occurrence_id: link.vulnerability_occurrence_id } end end @@ -33,10 +34,13 @@ def existing_links def ingested_links insertable_maps.flat_map do |occurrence_map| - occurrence_map.vulnerability_ids.map do |vulnerability_id| + occurrence_map.vulnerability_info.finding_ids.map do |finding_id| + finding = findings[finding_id] + { sbom_occurrence_id: occurrence_map.occurrence_id, - vulnerability_id: vulnerability_id + vulnerability_id: finding.vulnerability_id, + vulnerability_occurrence_id: finding.id } end end @@ -57,6 +61,13 @@ def all_links ingested_links + no_longer_present_links end + def findings + finding_ids = insertable_maps.flat_map { |occurrence_map| occurrence_map.vulnerability_info.finding_ids }.uniq + + Vulnerabilities::Finding.id_in(finding_ids).select(:id, :vulnerability_id).index_by(&:id) + end + strong_memoize_attr :findings + def after_ingest delete_old_links sync_elasticsearch diff --git a/ee/app/services/sbom/ingestion/vulnerability_data.rb b/ee/app/services/sbom/ingestion/vulnerability_data.rb index ec8977851ea72e..b5d3baa25043c2 100644 --- a/ee/app/services/sbom/ingestion/vulnerability_data.rb +++ b/ee/app/services/sbom/ingestion/vulnerability_data.rb @@ -16,10 +16,12 @@ module Ingestion # vulnerability_data = VulnerabilityData.new(occurrence_maps, project) # vulnerability_info = vulnerability_data.for(occurrence_map) # - # vulnerability_info.count # => 2 defaults to 0 - # vulnerability_info.highest_severity # => 3 defaults to nil - # vulnerability_info.vulnerability_ids # => [1, 2] defaults to [] + # vulnerability_info.count # => 2 defaults to 0 + # vulnerability_info.highest_severity # => 3 defaults to nil + # vulnerability_info.finding_ids # => [1, 2] defaults to [] class VulnerabilityData + include Gitlab::Utils::StrongMemoize + CONTAINER_IMAGE_PREFIX = "container-image:" def initialize(occurrence_maps, project) @@ -45,10 +47,6 @@ def key_for(occurrence_map) end def vulnerabilities_info - @vulnerabilities_info ||= fetch_vulnerabilities_info - end - - def fetch_vulnerabilities_info # rubocop:disable CodeReuse/ActiveRecord -- highly customized query occurrence_maps_values = occurrence_maps.map { |om| key_for(om) } as_values = Arel::Nodes::ValuesList.new(occurrence_maps_values).to_sql @@ -61,7 +59,7 @@ def fetch_vulnerabilities_info occurrence_maps.name, occurrence_maps.version, occurrence_maps.path, - array_to_json(array_agg(vulnerability_occurrences.vulnerability_id)) as vulnerability_ids, + array_agg(vulnerability_occurrences.id) as finding_ids, MAX(vulnerability_occurrences.severity) as highest_severity, COUNT(vulnerability_occurrences.id) as vulnerability_count SQL @@ -93,21 +91,27 @@ def fetch_vulnerabilities_info value = { highest_severity: row['highest_severity'], vulnerability_count: row['vulnerability_count'], - vulnerability_ids: ::Gitlab::Json.parse(row['vulnerability_ids']).filter_map do |id| - id_i = id.to_i - id_i if id_i > 0 - end + finding_ids: parse_array(row['finding_ids']) } result[key] = value end # rubocop:enable CodeReuse/ActiveRecord end + strong_memoize_attr :vulnerabilities_info + + # Converts a postgres array '{1,2,3}' into a ruby array [1,2,3] + def parse_array(data) + data.delete('{}').split(',').filter_map do |id| + id_i = id.to_i + id_i if id_i > 0 + end + end class VulnerabilityInfo - def initialize(data) = @data = data || {} - def count = @data[:vulnerability_count] || 0 - def highest_severity = @data[:highest_severity] - def vulnerability_ids = @data[:vulnerability_ids] || [] + def initialize(data = {}) = @data = data || {} + def count = @data[:vulnerability_count] || 0 + def highest_severity = @data[:highest_severity] + def finding_ids = @data[:finding_ids] || [] end end end diff --git a/ee/spec/services/sbom/ingestion/occurrence_map_spec.rb b/ee/spec/services/sbom/ingestion/occurrence_map_spec.rb index 52a9ab1dac5d8b..706aae6e2e07ed 100644 --- a/ee/spec/services/sbom/ingestion/occurrence_map_spec.rb +++ b/ee/spec/services/sbom/ingestion/occurrence_map_spec.rb @@ -258,6 +258,6 @@ end context 'without vulnerability data' do - it { expect(occurrence_map.vulnerability_ids).to eq [] } + it { expect(occurrence_map.vulnerability_info.finding_ids).to eq [] } end end diff --git a/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_spec.rb b/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_spec.rb index 98675296c901c3..6577409350bc71 100644 --- a/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_spec.rb +++ b/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_spec.rb @@ -106,13 +106,15 @@ context "for each occurrence_map" do before do - allow(occurrence_map).to receive(:vulnerability_ids=).once + allow(occurrence_map).to receive(:vulnerability_info=).once end it 'passes an array of vulnerability ids into the occurrence_map' do task - expect(occurrence_map).to have_received(:vulnerability_ids=).with([finding.vulnerability_id]) + expected_object = an_object_having_attributes(finding_ids: [finding.id]) + + expect(occurrence_map).to have_received(:vulnerability_info=).with(expected_object) end end end @@ -150,16 +152,16 @@ end context "for each occurrence_map" do - let(:expected_value) { feature_flag_stub ? [finding.vulnerability_id] : nil } - before do - allow(occurrence_map).to receive(:vulnerability_ids=).once + allow(occurrence_map).to receive(:vulnerability_info=).once end it 'passes an array of vulnerability ids into the occurrence_map' do task - expect(occurrence_map).to have_received(:vulnerability_ids=).with([finding.vulnerability_id]) + expected_object = an_object_having_attributes(finding_ids: [finding.id]) + + expect(occurrence_map).to have_received(:vulnerability_info=).with(expected_object) end end end diff --git a/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities_spec.rb b/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities_spec.rb index c46c784752296a..81e06991e49556 100644 --- a/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities_spec.rb +++ b/ee/spec/services/sbom/ingestion/tasks/ingest_occurrences_vulnerabilities_spec.rb @@ -47,8 +47,16 @@ end before do - occurrence_map_1.vulnerability_ids = [finding_1.vulnerability_id] - occurrence_map_2.vulnerability_ids = [finding_2.vulnerability_id] + occurrence_map_1.vulnerability_info = create_vulnerability_info(finding_1) + occurrence_map_2.vulnerability_info = create_vulnerability_info(finding_2) + end + + def create_vulnerability_info(finding) + Sbom::Ingestion::VulnerabilityData::VulnerabilityInfo.new( + count: 1, + highest_severity: finding.severity, + finding_ids: [finding.id] + ) end it_behaves_like 'bulk insertable task' @@ -65,10 +73,16 @@ ingest_occurrences_vulnerabilities expect(Sbom::OccurrencesVulnerability.all).to match_array([ - an_object_having_attributes('sbom_occurrence_id' => occurrence_map_2.occurrence_id, - 'vulnerability_id' => finding_2.vulnerability_id), - an_object_having_attributes('sbom_occurrence_id' => occurrence_map_1.occurrence_id, - 'vulnerability_id' => finding_1.vulnerability_id) + an_object_having_attributes( + 'sbom_occurrence_id' => occurrence_map_2.occurrence_id, + 'vulnerability_id' => finding_2.vulnerability_id, + 'vulnerability_occurrence_id' => finding_2.id + ), + an_object_having_attributes( + 'sbom_occurrence_id' => occurrence_map_1.occurrence_id, + 'vulnerability_id' => finding_1.vulnerability_id, + 'vulnerability_occurrence_id' => finding_1.id + ) ]) end end @@ -77,7 +91,9 @@ let!(:existing_record) do create(:sbom_occurrences_vulnerability, sbom_occurrence_id: occurrence_map_1.occurrence_id, - vulnerability_id: finding_1.vulnerability_id) + vulnerability_id: finding_1.vulnerability_id, + vulnerability_occurrence_id: finding_1.id + ) end let(:expected_vulnerability_ids) { [finding_1.vulnerability_id, finding_2.vulnerability_id] } @@ -91,7 +107,7 @@ context 'when the vulnerability_id was not ingested' do before do - occurrence_map_1.vulnerability_ids = [] + occurrence_map_1.vulnerability_info = Sbom::Ingestion::VulnerabilityData::VulnerabilityInfo.new end it 'deletes the record' do @@ -127,7 +143,7 @@ version: occurrence_map_1.version, pipeline: pipeline ) - occurrence_map_1.vulnerability_ids << finding.vulnerability_id + occurrence_map_1.vulnerability_info.finding_ids << finding.id end it 'creates all related occurrences_vulnerabilities' do diff --git a/ee/spec/services/sbom/ingestion/vulnerability_data_spec.rb b/ee/spec/services/sbom/ingestion/vulnerability_data_spec.rb index 1ec9141b74ebda..8c0b6fbc0b40fd 100644 --- a/ee/spec/services/sbom/ingestion/vulnerability_data_spec.rb +++ b/ee/spec/services/sbom/ingestion/vulnerability_data_spec.rb @@ -86,7 +86,7 @@ it 'returns default values' do expect(vulnerability_info.count).to eq(0) expect(vulnerability_info.highest_severity).to be_nil - expect(vulnerability_info.vulnerability_ids).to eq([]) + expect(vulnerability_info.finding_ids).to eq([]) end end @@ -94,7 +94,7 @@ it 'returns the vulnerability information' do expect(vulnerability_info.count).to eq(1) expect(vulnerability_info.highest_severity).to eq(expected_severity) - expect(vulnerability_info.vulnerability_ids).to eq([finding.vulnerability_id]) + expect(vulnerability_info.finding_ids).to eq([finding.id]) end end -- GitLab