diff --git a/ee/app/models/security/policy_dismissal.rb b/ee/app/models/security/policy_dismissal.rb index 9a80f64b9140f258497d5e170a92c51c14c565cd..81461ae5e580f862846af626f7995d5b9f7caf7d 100644 --- a/ee/app/models/security/policy_dismissal.rb +++ b/ee/app/models/security/policy_dismissal.rb @@ -25,6 +25,7 @@ class PolicyDismissal < ApplicationRecord scope :for_security_findings_uuids, ->(security_findings_uuids) do where("security_findings_uuids && ARRAY[?]::text[]", security_findings_uuids) end + scope :pluck_security_findings_uuid, -> { pluck(Arel.sql('DISTINCT unnest(security_findings_uuids)')) } # rubocop: disable Database/AvoidUsingPluckWithoutLimit -- pluck limited to batch size in ee/lib/search/elastic/references/vulnerability.rb#preload_indexing_data scope :including_merge_request_and_user, -> { includes(:user, :merge_request) } state_machine :status, initial: :open do diff --git a/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb b/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb index 97b3b8254090db96f164c3992e8f13cbaaf93278..a2a5df77ff728d11ed38a909f6e9976d09350a07 100644 --- a/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb +++ b/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb @@ -8,7 +8,7 @@ module Vulnerability # enhanced record proxies for elasticsearch indexing. # # This class: - # 1. Coordinates multiple individual preloaders (EPSS, CVE, Reachability, TokenStatus) + # 1. Coordinates multiple individual preloaders (EPSS, CVE, Reachability, TokenStatus, PolicyViolations) # 2. Creates enhanced proxies with all preloaded data # 3. Assigns proxies back to reference objects # 4. Provides a single entry point for all vulnerability data preloading @@ -33,6 +33,7 @@ def preload_and_enhance! def preload_all_data @reachability_data = Reachability.new(records).preload @token_status_data = TokenStatus.new(records).preload + @policy_violations_data = PolicyViolations.new(records).preload end # Create enhanced proxies and assign them to references @@ -52,7 +53,8 @@ def create_enhanced_proxy(record) vulnerability_id = record.vulnerability_id enhancements = { 'reachability' => fetch_reachability_value(vulnerability_id), - 'token_status' => fetch_token_status_value(vulnerability_id) + 'token_status' => fetch_token_status_value(vulnerability_id), + 'policy_violations' => fetch_policy_violation_value(vulnerability_id) }.with_indifferent_access ::Search::Elastic::RecordProxy::Vulnerability.create_with_enhancements(record, enhancements) @@ -65,6 +67,10 @@ def fetch_reachability_value(vulnerability_id) def fetch_token_status_value(vulnerability_id) @token_status_data[vulnerability_id] || ::Security::TokenStatus::UNKNOWN end + + def fetch_policy_violation_value(vulnerability_id) + @policy_violations_data[vulnerability_id] + end end end end diff --git a/ee/lib/search/elastic/preloaders/vulnerability/policy_violations.rb b/ee/lib/search/elastic/preloaders/vulnerability/policy_violations.rb new file mode 100644 index 0000000000000000000000000000000000000000..4e7c20da3ea01e6a034e9ad73b94bb876c0e168a --- /dev/null +++ b/ee/lib/search/elastic/preloaders/vulnerability/policy_violations.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Search + module Elastic + module Preloaders + module Vulnerability + class PolicyViolations < Base + def perform_preload + security_findings_uuids = records.map(&:uuid) + return {} if security_findings_uuids.empty? + + fetch_policy_violations_data(security_findings_uuids) + end + + private + + def fetch_policy_violations_data(security_findings_uuids) + project_ids = records.map(&:project_id) + + dismissed_uuids = ::Security::PolicyDismissal + .for_projects(project_ids) + .with_status(:preserved) + .for_security_findings_uuids(security_findings_uuids) + .pluck_security_findings_uuid.to_set + + records.each_with_object({}) do |record, result| + result[record.vulnerability_id] = policy_violation(dismissed_uuids, record.uuid) + end + end + + def policy_violation(dismissed_uuids, uuid) + is_dismissed = dismissed_uuids.include?(uuid) + is_dismissed ? ::Security::PolicyViolations::DISMISSED_IN_MR : ::Security::PolicyViolations::NOT_APPLICABLE + end + end + end + end + end +end diff --git a/ee/lib/search/elastic/record_proxy/vulnerability.rb b/ee/lib/search/elastic/record_proxy/vulnerability.rb index 3d3d7370dd393822379332af4ad3efe6ac5dccf4..b67860896a9369864d26f3c8b976523f463819be 100644 --- a/ee/lib/search/elastic/record_proxy/vulnerability.rb +++ b/ee/lib/search/elastic/record_proxy/vulnerability.rb @@ -5,7 +5,7 @@ module Elastic module RecordProxy # Vulnerability-specific record proxy that provides optimized access # to elasticsearch indexing data including EPSS scores, CVE values, - # reachability and token status information. + # reachability, token status, and policy_violations information. class Vulnerability < Base # Creates a vulnerability proxy with all optimizations applied def self.create_with_enhancements(record, enhancements) @@ -13,7 +13,8 @@ def self.create_with_enhancements(record, enhancements) proxy.enhance_with_data({ reachability: enhancements[:reachability], - token_status: enhancements[:token_status] + token_status: enhancements[:token_status], + policy_violations: enhancements[:policy_violations] }) proxy diff --git a/ee/lib/search/elastic/references/vulnerability.rb b/ee/lib/search/elastic/references/vulnerability.rb index 6703054034811f556e7cf6ccf1d31baff40d950e..5fc54783d40e3a2eba1191de4f581a03af3a4cd7 100644 --- a/ee/lib/search/elastic/references/vulnerability.rb +++ b/ee/lib/search/elastic/references/vulnerability.rb @@ -7,7 +7,7 @@ class Vulnerability < Reference include Search::Elastic::Concerns::DatabaseReference include ::Gitlab::Utils::StrongMemoize - SCHEMA_VERSION = 25_37 + SCHEMA_VERSION = 25_42 DOC_TYPE = 'vulnerability' INDEX_NAME = 'vulnerabilities' @@ -81,12 +81,7 @@ def as_indexed_json fields[field] = database_record.method(:"#{field}_before_type_cast").call end - # Add other fields - fields["scanner_external_id"] = database_record.scanner&.external_id - fields["created_at"] = database_record.vulnerability.created_at - fields["updated_at"] = database_record.vulnerability.updated_at - fields["traversal_ids"] = database_record.project.namespace.elastic_namespace_ancestry - fields["epss_scores"] = fetch_record_attribute(database_record, :epss_scores) + fields.merge!(build_other_fields) if reachability_migration_finished? fields["reachability"] = fetch_record_attribute(database_record, :reachability) @@ -96,6 +91,10 @@ def as_indexed_json fields["token_status"] = fetch_record_attribute(database_record, :token_status) end + if policy_violations_migration_finished? + fields["policy_violations"] = fetch_record_attribute(database_record, :policy_violations) + end + if resolved_at_dismissed_at_migration_completed? fields["resolved_at"] = database_record.vulnerability.resolved_at fields["dismissed_at"] = database_record.vulnerability.dismissed_at @@ -119,6 +118,16 @@ def database_record private + def build_other_fields + { + scanner_external_id: database_record.scanner&.external_id, + created_at: database_record.vulnerability.created_at, + updated_at: database_record.vulnerability.updated_at, + traversal_ids: database_record.project.namespace.elastic_namespace_ancestry, + epss_scores: fetch_record_attribute(database_record, :epss_scores) + } + end + def fetch_record_attribute(record, attribute) return record.method(attribute).call if record.respond_to?(attribute) @@ -134,8 +143,10 @@ def internal_es_fields end def fetch_schema_version - if token_status_migration_finished? + if policy_violations_migration_finished? SCHEMA_VERSION + elsif token_status_migration_finished? + 25_37 elsif resolved_at_dismissed_at_migration_completed? 25_36 elsif reachability_migration_finished? @@ -157,6 +168,10 @@ def token_status_migration_finished? ) end + def policy_violations_migration_finished? + ::Elastic::DataMigrationService.migration_has_finished?(:add_policy_violations_field_to_vulnerability) + end + def resolved_at_dismissed_at_migration_completed? ::Elastic::DataMigrationService.migration_has_finished?( :add_resolved_at_dismissed_at_fields_to_vulnerability) diff --git a/ee/lib/security/policy_violations.rb b/ee/lib/security/policy_violations.rb new file mode 100644 index 0000000000000000000000000000000000000000..9f420611d0c2c9ece8ca56c9b373d142f9499084 --- /dev/null +++ b/ee/lib/security/policy_violations.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Security + module PolicyViolations + NOT_APPLICABLE = 0 + DISMISSED_IN_MR = 1 + + VIOLATIONS_TYPES = { + not_applicable: NOT_APPLICABLE, + dismissed_in_mr: DISMISSED_IN_MR + }.freeze + end +end diff --git a/ee/spec/factories/security/policy_dismissal.rb b/ee/spec/factories/security/policy_dismissal.rb index 9e5cd02039490cbf62c4d83e69e74bc982924fb4..e607660bb0944647c4501e65d22a3b7f5886b079 100644 --- a/ee/spec/factories/security/policy_dismissal.rb +++ b/ee/spec/factories/security/policy_dismissal.rb @@ -8,5 +8,9 @@ user dismissal_types { Security::PolicyDismissal::DISMISSAL_TYPES.values.sample(2) } security_findings_uuids { [SecureRandom.uuid] } + + trait :preserved do + status { 1 } + end end end diff --git a/ee/spec/lib/search/elastic/preloaders/vulnerability/enhanced_proxy_spec.rb b/ee/spec/lib/search/elastic/preloaders/vulnerability/enhanced_proxy_spec.rb index bc77efba9a34b8c8800f88feef6e86a0c48aca1d..8f02be1088606b052c7687fb1e480b87dec67559 100644 --- a/ee/spec/lib/search/elastic/preloaders/vulnerability/enhanced_proxy_spec.rb +++ b/ee/spec/lib/search/elastic/preloaders/vulnerability/enhanced_proxy_spec.rb @@ -19,6 +19,9 @@ let(:mock_reachability_preloader) { instance_double(Search::Elastic::Preloaders::Vulnerability::Reachability) } let(:mock_token_status_preloader) { instance_double(Search::Elastic::Preloaders::Vulnerability::TokenStatus) } + let(:mock_policy_violations_preloader) do + instance_double(Search::Elastic::Preloaders::Vulnerability::PolicyViolations) + end let(:reachability_data) do { @@ -35,6 +38,13 @@ } end + let(:policy_violations_data) do + { + vulnerability_reads[0].vulnerability_id => ::Security::PolicyViolations::DISMISSED_IN_MR, + vulnerability_reads[1].vulnerability_id => nil + } + end + let(:enhanced_proxy) { described_class.new(refs, vulnerability_reads) } before do @@ -55,6 +65,15 @@ allow(mock_token_status_preloader) .to receive(:preload) .and_return(token_status_data) + + allow(Search::Elastic::Preloaders::Vulnerability::PolicyViolations) + .to receive(:new) + .with(vulnerability_reads) + .and_return(mock_policy_violations_preloader) + + allow(mock_policy_violations_preloader) + .to receive(:preload) + .and_return(policy_violations_data) end describe '#initialize' do @@ -86,6 +105,10 @@ expect(Search::Elastic::Preloaders::Vulnerability::TokenStatus) .to have_received(:new).with(vulnerability_reads) expect(mock_token_status_preloader).to have_received(:preload) + + expect(Search::Elastic::Preloaders::Vulnerability::PolicyViolations) + .to have_received(:new).with(vulnerability_reads) + expect(mock_policy_violations_preloader).to have_received(:preload) end it 'creates enhanced proxies for matching refs' do @@ -129,17 +152,20 @@ it 'enhances proxies with correct reachability and token_status data' do expected_enhancements_in_use = { 'reachability' => ::Enums::Sbom::REACHABILITY_TYPES[::Enums::Sbom::IN_USE], - 'token_status' => ::Security::TokenStatus::ACTIVE + 'token_status' => ::Security::TokenStatus::ACTIVE, + 'policy_violations' => ::Security::PolicyViolations::DISMISSED_IN_MR }.with_indifferent_access expected_enhancements_not_found = { 'reachability' => ::Enums::Sbom::REACHABILITY_TYPES[::Enums::Sbom::NOT_FOUND], - 'token_status' => ::Security::TokenStatus::INACTIVE + 'token_status' => ::Security::TokenStatus::INACTIVE, + 'policy_violations' => nil }.with_indifferent_access expected_enhancements_unknown = { 'reachability' => ::Enums::Sbom::REACHABILITY_TYPES[::Enums::Sbom::UNKNOWN], - 'token_status' => ::Security::TokenStatus::UNKNOWN + 'token_status' => ::Security::TokenStatus::UNKNOWN, + 'policy_violations' => nil }.with_indifferent_access expect(::Search::Elastic::RecordProxy::Vulnerability) @@ -168,6 +194,7 @@ expect(assigned_proxy.project_id).to eq(vulnerability_reads[0].project_id) expect(assigned_proxy.reachability).to eq(reachability_data[vulnerability_reads[0].vulnerability_id]) expect(assigned_proxy.token_status).to eq(token_status_data[vulnerability_reads[0].vulnerability_id]) + expect(assigned_proxy.policy_violations).to eq(policy_violations_data[vulnerability_reads[0].vulnerability_id]) end end @@ -217,4 +244,21 @@ expect(result).to eq(::Security::TokenStatus::UNKNOWN) end end + + describe '#fetch_policy_violations_value' do + it 'returns preloaded value when available' do + enhanced_proxy.instance_variable_set(:@policy_violations_data, + { 123 => ::Security::PolicyViolations::DISMISSED_IN_MR }) + + result = enhanced_proxy.send(:fetch_policy_violation_value, 123) + expect(result).to eq(::Security::PolicyViolations::DISMISSED_IN_MR) + end + + it 'returns nil for missing data' do + enhanced_proxy.instance_variable_set(:@policy_violations_data, {}) + + result = enhanced_proxy.send(:fetch_policy_violation_value, 999) + expect(result).to be_nil + end + end end diff --git a/ee/spec/lib/search/elastic/preloaders/vulnerability/policy_violations_spec.rb b/ee/spec/lib/search/elastic/preloaders/vulnerability/policy_violations_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2528ff1a8f288be83d61c894684c0091774714bf --- /dev/null +++ b/ee/spec/lib/search/elastic/preloaders/vulnerability/policy_violations_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Search::Elastic::Preloaders::Vulnerability::PolicyViolations, feature_category: :vulnerability_management do + let_it_be(:project) { create(:project) } + let_it_be(:reads_by_policy_violation_type) do + { + dismissed_in_mr: create_read_with_policy_violations(true, project), + no_violations: create_read_with_policy_violations(false, project) + } + end + + def create_read_with_policy_violations(has_violations, project) + vulnerability = create(:vulnerability, :with_finding, project: project) + + if has_violations + create(:policy_dismissal, :preserved, project: project, + security_findings_uuids: [vulnerability.finding.uuid]) + end + + vulnerability.vulnerability_read + end + + describe '#preload' do + subject(:preloader) { described_class.new(records) } + + context 'with vulnerability records having different policy violation types' do + let(:records) { reads_by_policy_violation_type.values } + + it 'returns policy violation status for vulnerabilities' do + result = preloader.preload + + vulnerability_dismissed_in_mr = reads_by_policy_violation_type[:dismissed_in_mr].vulnerability_id + vulnerability_without_dismissal = reads_by_policy_violation_type[:no_violations].vulnerability_id + + expect(result[vulnerability_dismissed_in_mr]).to eq(Security::PolicyViolations::DISMISSED_IN_MR) + expect(result[vulnerability_without_dismissal]).to eq(Security::PolicyViolations::NOT_APPLICABLE) + end + end + + context 'with edge cases' do + context 'when records are empty' do + let(:records) { [] } + + it 'returns empty hash' do + expect(preloader.preload).to eq({}) + end + end + + context 'when database query fails' do + let(:records) { [reads_by_policy_violation_type[:dismissed_in_mr]] } + + before do + allow(::Security::PolicyDismissal).to receive(:for_projects).and_raise(StandardError, 'Database error') + allow(::Gitlab::ErrorTracking).to receive(:track_exception) + end + + it 'handles errors gracefully and tracks exceptions' do + result = preloader.preload + + expect(result).to eq({}) + expect(::Gitlab::ErrorTracking).to have_received(:track_exception) + .with(instance_of(StandardError), class: described_class.name) + end + end + end + + describe 'performance characteristics' do + let_it_be(:large_vulnerability_set) do + create_list(:vulnerability, 5, :with_read, project: project).map(&:vulnerability_read) + end + + let_it_be(:extra_vulnerability_set) do + create_list(:vulnerability, 2, :with_read, project: project).map(&:vulnerability_read) + end + + it 'performs efficient batch querying' do + preloader = described_class.new(large_vulnerability_set) + + expect { preloader.preload }.not_to exceed_query_limit(3) + end + + it 'avoids N+1 queries with larger datasets' do + # Setup some vulnerabilities with policy dismissals + large_vulnerability_set.first(3).each do |vulnerability_read| + create(:policy_dismissal, :preserved, project: project, security_findings_uuids: [vulnerability_read.uuid]) + end + + preloader = described_class.new(large_vulnerability_set) + control_count = count_queries { preloader.preload } + + # Adding more records should not significantly increase query count + larger_set = large_vulnerability_set + extra_vulnerability_set + larger_preloader = described_class.new(larger_set) + + expect { larger_preloader.preload }.not_to exceed_query_limit(control_count + 1) + end + end + end + + private + + def count_queries(&block) + ActiveRecord::QueryRecorder.new(&block).count + end +end diff --git a/ee/spec/lib/search/elastic/record_proxy/vulnerability_spec.rb b/ee/spec/lib/search/elastic/record_proxy/vulnerability_spec.rb index 2a6d450c94c23fb202a61d17c4a18ca44167e2c8..b984d6af48527a1ab518adc69efc22a19ab88f0b 100644 --- a/ee/spec/lib/search/elastic/record_proxy/vulnerability_spec.rb +++ b/ee/spec/lib/search/elastic/record_proxy/vulnerability_spec.rb @@ -22,7 +22,10 @@ end describe '.create_with_enhancements' do - let(:enhancements) { { reachability: 1, token_status: ::Security::TokenStatus::ACTIVE } } + let(:enhancements) do + { reachability: 1, token_status: ::Security::TokenStatus::ACTIVE, + policy_violations: ::Security::PolicyViolations::DISMISSED_IN_MR } + end it 'creates a new proxy instance' do proxy = described_class.create_with_enhancements(vulnerability_record, enhancements) @@ -48,6 +51,12 @@ expect(proxy.token_status).to eq(::Security::TokenStatus::ACTIVE) end + it 'enhances the proxy with policy_violations data' do + proxy = described_class.create_with_enhancements(vulnerability_record, enhancements) + + expect(proxy.policy_violations).to eq(::Security::PolicyViolations::DISMISSED_IN_MR) + end + context 'with missing reachability in enhancements' do let(:enhancements) { { token_status: ::Security::TokenStatus::INACTIVE } } @@ -80,6 +89,11 @@ .to receive(:new).and_return(instance_double( Search::Elastic::Preloaders::Vulnerability::TokenStatus, preload: {} )) + + allow(Search::Elastic::Preloaders::Vulnerability::PolicyViolations) + .to receive(:new).and_return(instance_double( + Search::Elastic::Preloaders::Vulnerability::PolicyViolations, preload: {} + )) end it 'defaults token_status to UNKNOWN when missing' do @@ -88,6 +102,13 @@ expect(proxy.token_status).to eq(::Security::TokenStatus::UNKNOWN) end + + it 'defaults policy_violations to nil when missing' do + enhanced_proxy.preload_and_enhance! + proxy = refs.first.database_record + + expect(proxy.policy_violations).to be_nil + end end end end diff --git a/ee/spec/lib/search/elastic/references/vulnerability_spec.rb b/ee/spec/lib/search/elastic/references/vulnerability_spec.rb index 3009f7f5d77a6c40f26c0fc50b493ec26e4455f1..827b99670649782ef7a4d7d1010ecde678568043 100644 --- a/ee/spec/lib/search/elastic/references/vulnerability_spec.rb +++ b/ee/spec/lib/search/elastic/references/vulnerability_spec.rb @@ -77,6 +77,7 @@ epss_scores: [], reachability: [], token_status: [], + policy_violations: [], type: described_class::DOC_TYPE, schema_version: described_class::SCHEMA_VERSION } @@ -246,6 +247,41 @@ end end + context 'with policy_violations migration mappings' do + context 'when policy_violations migration has finished' do + let(:policy_violations_data) { ::Security::PolicyViolations::DISMISSED_IN_MR } + + before do + set_elasticsearch_migration_to(:add_policy_violations_field_to_vulnerability) + + allow(object).to receive(:policy_violations).and_return(policy_violations_data) + allow(vulnerability_reference_object).to receive(:database_record).and_return(object) + end + + it 'returns schema version' do + expect(indexed_json[:schema_version]).to eq(25_42) + end + + it 'includes policy_violations in the indexed json' do + expect(indexed_json[:policy_violations]).to eq(policy_violations_data) + end + end + + context 'when migration has not completed' do + before do + set_elasticsearch_migration_to(:add_policy_violations_field_to_vulnerability, including: false) + end + + it 'returns schema version with token_status' do + expect(indexed_json[:schema_version]).to eq(25_37) + end + + it 'does not assign token_status on the indexed json', :aggregate_failures do + expect(indexed_json[:policy_violations]).to be_nil + end + end + end + context 'when all migrations have completed' do it 'returns the current schema version' do expect(indexed_json[:schema_version]).to eq(described_class::SCHEMA_VERSION) @@ -350,6 +386,20 @@ end end + context 'with policy_violations data' do + before do + create(:policy_dismissal, :preserved, project: project, security_findings_uuids: [vulnerability_read.uuid]) + end + + it 'preloads policy_violations' do + described_class.preload_indexing_data(refs) + + expect(vulnerability_ref.database_record.policy_violations).to eq(::Security::PolicyViolations::DISMISSED_IN_MR) + expect(user_vulnerability_ref.database_record.policy_violations).to eq( + ::Security::PolicyViolations::NOT_APPLICABLE) + end + end + context 'for database_record' do it 'without preloading returns an instance of model class' do expect(vulnerability_ref.database_record).to be_an_instance_of(described_class.model_klass) @@ -362,8 +412,10 @@ end end - context 'when checking for N+1 queries' do + context 'when checking for N+1 queries', :request_store do it 'does not have N+1 queries when preloading multiple references' do + stub_env('GITALY_DISABLE_REQUEST_LIMITS', 'true') + # Create initial test data project1 = create(:project, group: group) project2 = create(:project, namespace: create(:namespace)) @@ -400,6 +452,10 @@ occurrence: sbom_occurrence2, vulnerability_id: vulnerability_read2.vulnerability_id) + # Create Policy Dismissal occurrences for policy_violations data + create(:policy_dismissal, :preserved, project: project1, security_findings_uuids: [vulnerability_read1.uuid]) + create(:policy_dismissal, :preserved, project: project2, security_findings_uuids: [vulnerability_read2.uuid]) + # Create refs array for initial batch refs = [ described_class.new(vulnerability_read1.vulnerability_id, vulnerability_read1.es_parent) @@ -433,6 +489,9 @@ occurrence: sbom_occurrence3, vulnerability_id: vulnerability_read3.vulnerability_id) + # Create Policy Dismissal occurrences for policy_violations data + create(:policy_dismissal, :preserved, project: project3, security_findings_uuids: [vulnerability_read3.uuid]) + # Add new refs to the array refs += [ described_class.new(vulnerability_read2.vulnerability_id, vulnerability_read2.es_parent),