diff --git a/ee/app/services/sbom/create_occurrences_vulnerabilities_service.rb b/ee/app/services/sbom/create_occurrences_vulnerabilities_service.rb index 9ddffdc1a2accb171961c8dc842bf8d2be69cf9b..71359e2c8eee5c90b6a118ad7808b3953ea5b7b2 100644 --- a/ee/app/services/sbom/create_occurrences_vulnerabilities_service.rb +++ b/ee/app/services/sbom/create_occurrences_vulnerabilities_service.rb @@ -29,15 +29,19 @@ def execute # rubocop:disable CodeReuse/ActiveRecord -- Custom query def fetch_occurrence_data - cte_sql = "WITH cte (uuid, package_manager, component_name, version, project_id) AS (#{cte_values})" + cte_sql = "WITH cte (uuid, purl_type, component_name, version, project_id, vulnerability_id) AS (#{cte_values})" select_sql = Sbom::Occurrence .joins('INNER JOIN cte - ON cte.package_manager = sbom_occurrences.package_manager - AND cte.component_name = sbom_occurrences.component_name + ON cte.component_name = sbom_occurrences.component_name AND cte.project_id = sbom_occurrences.project_id') .joins('INNER JOIN sbom_component_versions ON sbom_component_versions.id = sbom_occurrences.component_version_id AND cte.version = sbom_component_versions.version') + .joins('INNER JOIN sbom_components + ON sbom_components.id = sbom_occurrences.component_id + AND cte.purl_type = sbom_components.purl_type') + .joins('INNER JOIN vulnerabilities ON cte.vulnerability_id = vulnerabilities.id') + .merge(Vulnerability.active.with_resolution(false)) .select('cte.uuid', 'cte.project_id', 'sbom_occurrences.id').to_sql full_query = [cte_sql, select_sql].join("\n") @@ -52,7 +56,8 @@ def cte_values def filtered_data @filtered_data ||= finding_data.filter_map do |data| - [data[:uuid], data[:purl_type], data[:package_name], data[:package_version], data[:project_id]] + data[:purl_type] = ::Enums::Sbom.purl_types[data[:purl_type]] + data.values_at(:uuid, :purl_type, :package_name, :package_version, :project_id, :vulnerability_id) end end diff --git a/ee/app/services/sbom/create_vulnerabilities_service.rb b/ee/app/services/sbom/create_vulnerabilities_service.rb index 0c612705bd453a193583f5109ae46910b338d418..a1a4a5f6d0610a285e5961c4d2774d485f30f929 100644 --- a/ee/app/services/sbom/create_vulnerabilities_service.rb +++ b/ee/app/services/sbom/create_vulnerabilities_service.rb @@ -20,6 +20,7 @@ def initialize(pipeline_id) def execute start_time = Time.current.iso8601 ingested_ids_by_report_type = Hash.new([]) + total_finding_maps = [] valid_sbom_reports.each do |sbom_report| next unless sbom_report.source.present? @@ -59,12 +60,15 @@ def execute end ingested_ids_by_report_type[sbom_report.source.source_type] += create_vulnerabilities(finding_maps) + total_finding_maps += finding_maps end end end mark_resolved_vulnerabilities(ingested_ids_by_report_type) + trigger_vulnerabilities_created_event(total_finding_maps) + track_internal_event( 'cvs_on_sbom_change', project: project, diff --git a/ee/app/services/security/vulnerability_scanning/create_vulnerability_service.rb b/ee/app/services/security/vulnerability_scanning/create_vulnerability_service.rb index 1e625770096b9ba4c2f546bd7f02457cb3103895..1d913c3452e26dfae2c8a977de41acc158c40739 100644 --- a/ee/app/services/security/vulnerability_scanning/create_vulnerability_service.rb +++ b/ee/app/services/security/vulnerability_scanning/create_vulnerability_service.rb @@ -5,14 +5,11 @@ module VulnerabilityScanning class CreateVulnerabilityService include Gitlab::Utils::StrongMemoize - FINDINGS_LIMIT = 50 - PRE_CREATE_TASKS = %i[ reject_findings_maps_exceeding_quota ].freeze POST_CREATE_TASKS = %i[ - trigger_vulnerabilities_created_event track_upsert_for_project_id schedule_updating_archived_status_if_needed schedule_updating_traversal_ids_if_needed @@ -190,41 +187,6 @@ def projects end strong_memoize_attr :projects - def trigger_vulnerabilities_created_event - return unless findings_for_vulnerabilities.present? - - event_group = findings_for_vulnerabilities - .each_slice(FINDINGS_LIMIT) - .map { |findings| vulnerabilities_created_event(findings) } - - ::Gitlab::EventStore.publish_group(event_group) - end - - def vulnerabilities_created_event(findings) - Sbom::VulnerabilitiesCreatedEvent.new(data: { findings: findings }.with_indifferent_access) - end - - def findings_for_vulnerabilities - finding_maps.filter_map do |finding_map| - next unless include_finding_map?(finding_map) - - { - uuid: finding_map.uuid, - vulnerability_id: finding_map.vulnerability_id, - project_id: finding_map.project.id, - pipeline_id: finding_map.pipeline.id, - package_name: finding_map.location.package_name, - package_version: finding_map.location.package_version, - purl_type: finding_map.purl_type - } - end - end - strong_memoize_attr :findings_for_vulnerabilities - - def include_finding_map?(finding_map) - finding_map.report_type == 'dependency_scanning' - end - def pipelines finding_maps.map(&:pipeline) end diff --git a/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb b/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb index 018c1d697c9573eba468f570c17a7d82dd1faac4..7818efa48f737f7ad89e012d81cb243e33c87ecc 100644 --- a/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb +++ b/ee/lib/gitlab/vulnerability_scanning/advisory_scanner.rb @@ -106,6 +106,8 @@ def bulk_vulnerability_ingestion(affected_package, advisory_data_object, occurre return if finding_maps.empty? create_vulnerabilities(finding_maps) + + trigger_vulnerabilities_created_event(finding_maps) end def affected_occurrence?(occurrence, affected_package, advisory_data_object) diff --git a/ee/lib/gitlab/vulnerability_scanning/advisory_utils.rb b/ee/lib/gitlab/vulnerability_scanning/advisory_utils.rb index ad8e61489c2a74cb4b868a55956ab5ecff75d02c..669085706d482d4bd446e8f41510ee1700e808c9 100644 --- a/ee/lib/gitlab/vulnerability_scanning/advisory_utils.rb +++ b/ee/lib/gitlab/vulnerability_scanning/advisory_utils.rb @@ -5,6 +5,8 @@ module VulnerabilityScanning module AdvisoryUtils include Gitlab::Utils::StrongMemoize + FINDINGS_LIMIT = 50 + def occurrence_is_affected?(purl_type:, range:, version:, distro:, source:, project_id:, xid:, source_xid:) matcher = build_matcher(purl_type: purl_type, range: range) if Enums::Sbom.container_scanning_purl_type?(purl_type) @@ -68,6 +70,41 @@ def log_error(error, project_ids_with_upsert:) Gitlab::AppJsonLogger.error(message: "Failed to create vulnerabilities on advisory ingestion", error: error, project_ids_with_upsert: project_ids_with_upsert) end + + def trigger_vulnerabilities_created_event(finding_maps) + findings_data = findings_for_vulnerabilities(finding_maps) + return unless findings_data.present? + + event_group = findings_data + .each_slice(FINDINGS_LIMIT) + .map { |findings| vulnerabilities_created_event(findings) } + + ::Gitlab::EventStore.publish_group(event_group) + end + + def vulnerabilities_created_event(findings) + Sbom::VulnerabilitiesCreatedEvent.new(data: { findings: findings }.with_indifferent_access) + end + + def findings_for_vulnerabilities(finding_maps) + finding_maps.filter_map do |finding_map| + next unless include_finding_map?(finding_map) + + { + uuid: finding_map.uuid, + vulnerability_id: finding_map.vulnerability_id, + project_id: finding_map.project.id, + pipeline_id: finding_map.pipeline.id, + package_name: finding_map.location.package_name, + package_version: finding_map.location.package_version, + purl_type: finding_map.purl_type + } + end + end + + def include_finding_map?(finding_map) + finding_map.report_type == 'dependency_scanning' + end end end end diff --git a/ee/spec/lib/gitlab/vulnerability_scanning/advisory_scanner_spec.rb b/ee/spec/lib/gitlab/vulnerability_scanning/advisory_scanner_spec.rb index 4a88c1426bf7ab14b067696d8964825fc80662f5..989602e7a30f54eaaf356e814267429b074ab88c 100644 --- a/ee/spec/lib/gitlab/vulnerability_scanning/advisory_scanner_spec.rb +++ b/ee/spec/lib/gitlab/vulnerability_scanning/advisory_scanner_spec.rb @@ -53,6 +53,16 @@ described_class.scan_projects_for(affected_package.advisory) end + shared_examples 'with trigger_vulnerabilities_created_event' do + it 'calls trigger_vulnerabilities_created_event' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:trigger_vulnerabilities_created_event) + end + + described_class.scan_projects_for(affected_package.advisory) + end + end + context 'with Dependency Scanning' do before_all do eslint = create(:sbom_component, name: 'eslint', purl_type: :npm) @@ -170,6 +180,8 @@ } ) end + + it_behaves_like 'with trigger_vulnerabilities_created_event' end context 'when component matches but version is not in the affected range' do @@ -338,6 +350,8 @@ } ) end + + it_behaves_like 'with trigger_vulnerabilities_created_event' end context 'when component matches but version is not in the affected range' do diff --git a/ee/spec/lib/gitlab/vulnerability_scanning/advisory_utils_spec.rb b/ee/spec/lib/gitlab/vulnerability_scanning/advisory_utils_spec.rb index 4a1cb780b92b48320f673b0e4c5d5b4a11f06695..3b35d96b7843e1e013731799092ba6d78041afe1 100644 --- a/ee/spec/lib/gitlab/vulnerability_scanning/advisory_utils_spec.rb +++ b/ee/spec/lib/gitlab/vulnerability_scanning/advisory_utils_spec.rb @@ -187,4 +187,65 @@ end end end + + describe '.trigger_vulnerabilities_created_event' do + subject(:trigger_vulnerabilities) do + advisory_utils_test_class.new.trigger_vulnerabilities_created_event(finding_maps) + end + + let(:project) { pipeline.project } + let(:vuln) { create(:vulnerability) } + let(:security_finding) do + create(:ci_reports_security_finding, :dependency_scanning, report_type: 'dependency_scanning') + end + + let(:finding_map) do + create(:vs_finding_map, pipeline: pipeline, vulnerability: vuln, report_finding: security_finding) + end + + let(:finding_maps) { [finding_map] } + + it 'publishes a new event with sbom and vulnerability information' do + expect { trigger_vulnerabilities }.to publish_event(Sbom::VulnerabilitiesCreatedEvent).with({ + "findings" => match_array([ + { + "uuid" => finding_map.uuid, + "vulnerability_id" => vuln.id, + "project_id" => project.id, + "pipeline_id" => pipeline.id, + "package_name" => finding_map.location.package_name, + "package_version" => finding_map.location.package_version, + "purl_type" => finding_map.purl_type + } + ]) + }) + end + + context 'with a number of findings higher than `FINDINGS_LIMIT`' do + let(:security_finding_second) do + create(:ci_reports_security_finding, :dependency_scanning, report_type: 'dependency_scanning') + end + + let(:vuln_second) { create(:vulnerability) } + + let(:finding_map_second) do + create(:vs_finding_map, pipeline: pipeline, report_finding: security_finding_second, vulnerability: vuln_second) + end + + let(:finding_maps) { [finding_map, finding_map_second] } + + before do + stub_const("#{described_class}::FINDINGS_LIMIT", 1) + end + + it 'publishes a new event with findings based on `FINDINGS_LIMIT`' do + expect(Sbom::VulnerabilitiesCreatedEvent).to receive(:new) + .with({ data: { findings: [hash_including(uuid: finding_map.uuid)] } }).and_call_original + expect(Sbom::VulnerabilitiesCreatedEvent).to receive(:new) + .with({ data: { findings: [hash_including(uuid: finding_map_second.uuid)] } }).and_call_original + + trigger_vulnerabilities + end + end + end end diff --git a/ee/spec/services/sbom/create_occurrences_vulnerabilities_service_spec.rb b/ee/spec/services/sbom/create_occurrences_vulnerabilities_service_spec.rb index 0949ebdf974d8ead3b34a4dab1d0a6b68526fc28..212b6899af2ed2002cf6766417b15513b6f08250 100644 --- a/ee/spec/services/sbom/create_occurrences_vulnerabilities_service_spec.rb +++ b/ee/spec/services/sbom/create_occurrences_vulnerabilities_service_spec.rb @@ -44,6 +44,30 @@ ) end + context 'when vulnerability is not in an active state' do + before do + vulnerability.update!(state: :dismissed) + end + + it 'creates DB entries based on vulnerability and occurrence data' do + expect { service_execute }.not_to change { Sbom::OccurrencesVulnerability.count } + + expect(Sbom::OccurrencesVulnerability.count).to be_zero + end + end + + context 'when vulnerability is resolved on the default branch' do + before do + vulnerability.update!(resolved_on_default_branch: true) + end + + it 'creates DB entries based on vulnerability and occurrence data' do + expect { service_execute }.not_to change { Sbom::OccurrencesVulnerability.count } + + expect(Sbom::OccurrencesVulnerability.count).to be_zero + end + end + context 'with no related records' do let(:findings) do [ @@ -63,6 +87,25 @@ end end + context 'with different purl_type' do + let(:findings) do + [ + { + uuid: vulnerability.finding.uuid, + project_id: project.id, + vulnerability_id: vulnerability.id, + package_name: occurrence.component_name, + package_version: occurrence.version, + purl_type: 'pypi' + } + ] + end + + it 'does not create DB entries based on vulnerability and occurrence data' do + expect { service_execute }.not_to change { Sbom::OccurrencesVulnerability.count } + end + end + context 'with multiple vulnerabilities related to a single occurrence' do let(:findings) do [ diff --git a/ee/spec/services/sbom/create_vulnerabilities_service_spec.rb b/ee/spec/services/sbom/create_vulnerabilities_service_spec.rb index dc5f67d019a3d9c0aed26c3f414fe081e5ac0b31..737d91225b79aa876612612d1e0f153dc476f06d 100644 --- a/ee/spec/services/sbom/create_vulnerabilities_service_spec.rb +++ b/ee/spec/services/sbom/create_vulnerabilities_service_spec.rb @@ -77,6 +77,14 @@ def sanitized_distro_version(source) expect(Vulnerabilities::Finding.pluck(:security_project_tracked_context_id)).to all(eq(tracked_context.id)) end + it 'calls trigger_vulnerabilities_created_event' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:trigger_vulnerabilities_created_event) + end + + result + end + context 'when FindOrCreateService returns an error' do before do allow_next_instance_of(Security::ProjectTrackedContexts::FindOrCreateService) do |instance| diff --git a/ee/spec/services/security/vulnerability_scanning/create_vulnerability_service_spec.rb b/ee/spec/services/security/vulnerability_scanning/create_vulnerability_service_spec.rb index 576f07aea6f62bd8b2b358169903580879792d7f..ffd2e1b055a00680f402b26441063d0717fdc6cc 100644 --- a/ee/spec/services/security/vulnerability_scanning/create_vulnerability_service_spec.rb +++ b/ee/spec/services/security/vulnerability_scanning/create_vulnerability_service_spec.rb @@ -89,58 +89,6 @@ it_behaves_like 'vulnerability creation' - it 'publishes a new event with sbom and vulnerability information' do - expect { service_response }.to publish_event(Sbom::VulnerabilitiesCreatedEvent).with({ - "findings" => match_array([ - { - "uuid" => security_finding.uuid, - "vulnerability_id" => kind_of(Integer), - "project_id" => project.id, - "pipeline_id" => finding_map.pipeline.id, - "package_name" => locations.package_name, - "package_version" => locations.package_version, - "purl_type" => purl_type - }, - { - "uuid" => security_finding_2.uuid, - "vulnerability_id" => kind_of(Integer), - "project_id" => project.id, - "pipeline_id" => finding_map_2.pipeline.id, - "package_name" => locations.package_name, - "package_version" => locations.package_version, - "purl_type" => purl_type - } - ]) - }) - end - - context 'with a number of findings higher than `FINDINGS_LIMIT`' do - let(:security_finding_second) do - create(:ci_reports_security_finding, location: locations, report_type: report_type) - end - - let(:finding_map_second) do - create(:vs_finding_map, pipeline: pipeline, report_finding: security_finding_second, purl_type: purl_type) - end - - let(:finding_maps) { [finding_map, finding_map_second] } - - before do - stub_const("#{described_class}::FINDINGS_LIMIT", 1) - end - - it 'publishes a new event with findings based on `FINDINGS_LIMIT`' do - expect_next_instance_of(described_class) do |service| - expect(service).to receive(:vulnerabilities_created_event) - .with([hash_including(uuid: security_finding.uuid)]).and_call_original - expect(service).to receive(:vulnerabilities_created_event) - .with([hash_including(uuid: security_finding_second.uuid)]).and_call_original - end - - service_response - end - end - context 'when report type and source is container scanning' do let(:report_type) { 'container_scanning' } let(:locations) { create(:ci_reports_security_locations_container_scanning) }