diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/update.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/update.rb index 4fc7caabdd06b93ae9534cad0f66ff050e63684e..2a72048e3acfdd847648b831fc29ed582d422960 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/update.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/update.rb @@ -14,28 +14,36 @@ class Update < AbstractTask VALID_OWASP_PRIORITY_LABELS = %w[A1 A2 A3 A4 A5 A6 A7 A8 A9 A10].freeze VALID_OWASP_YEARS = %w[2017 2021].freeze + delegate :project, to: :pipeline, private: true + private # Note: https://gitlab.com/gitlab-org/gitlab/-/issues/430025 can iterate by # merging the hash for other columns like attributes_for_has_remediations, attributes_for_has_merge_requests # using hash.group_by(vulnerability_id), hash.merge functions. def attributes - attributes_for_owasp_top_10 - end + finding_maps.map do |finding_map| + attributes = { + vulnerability_id: finding_map.vulnerability_id, + traversal_ids: project.namespace.traversal_ids, + archived: project.archived, + owasp_top_10: nil + } - def attributes_for_owasp_top_10 - finding_maps_with_identifiers.filter_map do |finding_map| - owasp_identifier = finding_map.identifiers.find do |identifier| - identifier.external_type.casecmp?('owasp') - end + append_owasp_identifier_to(attributes, finding_map) - next unless owasp_identifier.present? + attributes + end + end - { - vulnerability_id: finding_map.vulnerability_id, - owasp_top_10: map_owasp_external_id(owasp_identifier.external_id) - } + def append_owasp_identifier_to(attributes, finding_map) + owasp_identifier = finding_map.identifiers.find do |identifier| + identifier.external_type.casecmp?('owasp') end + + return unless owasp_identifier.present? + + attributes.merge!({ owasp_top_10: map_owasp_external_id(owasp_identifier.external_id) }) end # The maximum set of observed external_id is having the label, year without the name. Eg: 'A5:2017'. @@ -46,10 +54,6 @@ def map_owasp_external_id(external_id) ::Enums::Vulnerability.owasp_top_10.keys.find { |key| key.include?(external_id) } end - def finding_maps_with_identifiers - finding_maps.select(&:identifiers) - end - def valid_external_id?(external_id) arr = external_id.split(':') diff --git a/ee/lib/gitlab/ingestion/bulk_updatable_task.rb b/ee/lib/gitlab/ingestion/bulk_updatable_task.rb index adf44db7b93829b0a6ce4b5be35bc7194bf3657a..e3bfe0a02d1a13007a5ea00753cbd25dc0ac590d 100644 --- a/ee/lib/gitlab/ingestion/bulk_updatable_task.rb +++ b/ee/lib/gitlab/ingestion/bulk_updatable_task.rb @@ -65,7 +65,9 @@ def set_values end def sql_type_for(attribute) - column_for_attribute(attribute).sql_type + col = column_for_attribute(attribute) + + col.sql_type_metadata.sql_type end def values diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/update_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/update_spec.rb index f784533e8a0002a6bded3ca435da6e7a7265704e..b258f766d0ff144615e232484a172a836d6eaf07 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/update_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/update_spec.rb @@ -4,10 +4,12 @@ RSpec.describe Security::Ingestion::Tasks::IngestVulnerabilityReads::Update, feature_category: :vulnerability_management do let_it_be(:user) { create(:user) } - let_it_be(:pipeline) { create(:ci_pipeline) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } let_it_be(:location) { create(:ci_reports_security_locations_sast, start_line: 29, end_line: 29) } let_it_be(:scanner) { build(:ci_reports_security_scanner, external_id: 'scanner', name: 'Scanner') } let_it_be(:vulnerability) { create_vulnerability } + let(:vulnerability_read) { vulnerability.vulnerability_read } let(:external_type) { 'owasp' } let(:external_id) { 'A1:2017-Injection' } @@ -44,23 +46,62 @@ finding_map.vulnerability_id = vulnerability.id end + subject(:execute) do + update_service.execute + + vulnerability_read.reload + end + + describe 'archived and traversal_ids' do + shared_examples 'updates vulnerability reads' do + it 'sets them for each vulnerability read to the values of the owning project' do + execute + + expect(vulnerability_read.archived).to eq(project.archived) + expect(vulnerability_read.traversal_ids).to eq( + project.namespace.traversal_ids + ) + end + end + + context 'when the project is not archived' do + it_behaves_like 'updates vulnerability reads' + end + + context 'when the project is archived' do + before do + project.update!(archived: true) + end + + it_behaves_like 'updates vulnerability reads' + end + + context 'when the project namespace is nested' do + before do + project.namespace.update!(traversal_ids: %w[9970 112345 09847]) + end + + it_behaves_like 'updates vulnerability reads' + end + end + describe 'owasp_top_10' do shared_examples 'updates vulnerability reads' do |external_id_name| - it 'updates vulnerability reads' do - update_service.execute + it 'updates owasp_top_10 on vulnerability reads' do + execute final_external_id = external_id final_external_id += "-#{external_id_name}" if external_id_name - expect(Vulnerabilities::Read.find_by(vulnerability_id: vulnerability.id).owasp_top_10).to eq(final_external_id) + expect(vulnerability_read.owasp_top_10).to eq(final_external_id) end end shared_examples 'does not update vulnerability reads' do - it 'does not update vulnerability reads' do - update_service.execute + it 'does not set owasp_top_10 on vulnerability reads' do + execute - expect(Vulnerabilities::Read.find_by(vulnerability_id: vulnerability.id).owasp_top_10).to be_nil + expect(vulnerability_read.owasp_top_10).to be_nil end end