diff --git a/config/feature_flags/wip/new_security_dashboard_exclude_no_longer_detected.yml b/config/feature_flags/wip/new_security_dashboard_exclude_no_longer_detected.yml new file mode 100644 index 0000000000000000000000000000000000000000..7c17f241313e938ed074d1dbe3ab00b8fbfe1f23 --- /dev/null +++ b/config/feature_flags/wip/new_security_dashboard_exclude_no_longer_detected.yml @@ -0,0 +1,10 @@ +--- +name: new_security_dashboard_exclude_no_longer_detected +description: Excludes no longer detected vulnerabilities from security dashboard using undetected_since timestamp +feature_issue_url: https://gitlab.com/groups/gitlab-org/-/work_items/19780 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215334 +rollout_issue_url: +milestone: '18.7' +group: group::security insights +type: wip +default_enabled: false diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb index dc3728087db84e9c35ebe963c16a6623c4abe799..3245a91c030d347cd4163c0e220e10753ed32bf0 100644 --- a/ee/app/models/vulnerabilities/finding.rb +++ b/ee/app/models/vulnerabilities/finding.rb @@ -199,6 +199,11 @@ class Finding < ::SecApplicationRecord ) end + scope :with_latest_detection_transition, -> do + last_detection_transition = Vulnerabilities::DetectionTransition.where(arel_table[:id].eq(Vulnerabilities::DetectionTransition.arel_table[:vulnerability_occurrence_id])).order(id: :desc).limit(1) + includes(:detection_transitions).where(detection_transitions: { id: last_detection_transition }) + end + scope :with_fix_available, ->(fix_available) do remediation = ::Vulnerabilities::FindingRemediation.arel_table solution_query = where(fix_available ? 'vulnerability_occurrences.solution IS NOT NULL' : 'vulnerability_occurrences.solution IS NULL') @@ -612,6 +617,10 @@ def ai_resolution_supported_cwe? end end + def latest_detection_transition + detection_transitions.last + end + protected def primary_identifier_fingerprint diff --git a/ee/app/services/security/ingestion/mark_as_resolved_service.rb b/ee/app/services/security/ingestion/mark_as_resolved_service.rb index eaad8a141e89414f0d6d8b9f646200791508ec01..3867f7c04d7040cf5c7e078daa75b916d1544390 100644 --- a/ee/app/services/security/ingestion/mark_as_resolved_service.rb +++ b/ee/app/services/security/ingestion/mark_as_resolved_service.rb @@ -89,6 +89,7 @@ def mark_as_no_longer_detected(vulnerabilities) ::Vulnerabilities::BulkEsOperationService.new(vulnerabilities_relation).execute do |relation| relation.update_all(resolved_on_default_branch: true) + create_detection_transitions(no_longer_detected_vulnerability_ids) end Vulnerabilities::Reads::UpsertService.new(vulnerabilities_relation, @@ -101,6 +102,13 @@ def mark_as_no_longer_detected(vulnerabilities) track_no_longer_detected_vulnerabilities(no_longer_detected_vulnerability_ids.count) end + def create_detection_transitions(vulnerability_ids) + return unless Feature.enabled?(:new_security_dashboard_exclude_no_longer_detected, project) + + findings = Vulnerabilities::Finding.by_vulnerability(vulnerability_ids) + ::Vulnerabilities::DetectionTransitions::InsertService.new(findings, detected: false).execute + end + def auto_resolve(no_longer_detected_vulnerability_ids) budget = AUTO_RESOLVE_LIMIT - auto_resolved_count return unless budget > 0 diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected.rb index 2cffab13b55434d8220f0bcc7954fef02583ea9a..aaa2d11ac5377b5f3461c1c44897e947e1405e49 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected.rb @@ -59,7 +59,9 @@ def update_vulnerability_records ) SecApplicationRecord.current_transaction.after_commit do - ::Vulnerabilities::BulkEsOperationService.new(vulnerabilities_relation).execute(&:itself) + ::Vulnerabilities::BulkEsOperationService.new(vulnerabilities_relation).execute do + create_detection_transitions(redetected_vulnerability_ids) + end end end @@ -67,6 +69,13 @@ def create_state_transitions ::Vulnerabilities::StateTransition.bulk_insert!(state_transitions) end + def create_detection_transitions(vulnerability_ids) + return unless Feature.enabled?(:new_security_dashboard_exclude_no_longer_detected, pipeline.project) + + findings = Vulnerabilities::Finding.by_vulnerability(vulnerability_ids) + ::Vulnerabilities::DetectionTransitions::InsertService.new(findings, detected: true).execute + end + def state_transitions redetected_vulnerability_ids.map do |vulnerability_id| ::Vulnerabilities::StateTransition.new( diff --git a/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb b/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb index 391ccc1d4d8a9b9ce5203c87c64ead98ad650e0b..f803410a14b91b06290ad99a9de980330d186f41 100644 --- a/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb +++ b/ee/app/services/vulnerabilities/starboard_vulnerability_resolve_service.rb @@ -25,12 +25,17 @@ def execute raise Gitlab::Access::AccessDeniedError unless authorized? undetected.each_batch(of: BATCH_SIZE) do |batch| + # rubocop:disable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit -- batch limited to 250 + batch_ids = batch.pluck(:id) + # rubocop:enable CodeReuse/ActiveRecord, Database/AvoidUsingPluckWithoutLimit + Vulnerabilities::BulkEsOperationService.new(batch).execute do |vulnerabilities| Vulnerability.transaction do vulnerabilities.update_all(resolved_on_default_branch: true, state: :resolved, updated_at: now) Vulnerabilities::Reads::UpsertService.new(vulnerabilities, { resolved_on_default_branch: true, state: :resolved }, projects: @project).execute end + create_detection_transitions(batch_ids) end end @@ -47,6 +52,13 @@ def undetected .with_cluster_agent_ids([agent.id.to_s]) end + def create_detection_transitions(vulnerability_ids) + return unless Feature.enabled?(:new_security_dashboard_exclude_no_longer_detected, project) + + findings = Vulnerabilities::Finding.by_vulnerability(vulnerability_ids) + ::Vulnerabilities::DetectionTransitions::InsertService.new(findings, detected: false).execute + end + def authorized? can?(author, :admin_vulnerability, project) end diff --git a/ee/elastic/docs/20251204112110_add_undetected_since_field_to_vulnerability.yml b/ee/elastic/docs/20251204112110_add_undetected_since_field_to_vulnerability.yml new file mode 100644 index 0000000000000000000000000000000000000000..c66230d186d500722124b206326f12d5628b156d --- /dev/null +++ b/ee/elastic/docs/20251204112110_add_undetected_since_field_to_vulnerability.yml @@ -0,0 +1,10 @@ +--- +name: AddUndetectedSinceFieldToVulnerability +version: '20251204112110' +description: Add undetected_since field to Vulnerabilities ES index +group: group::security insights +milestone: '18.8' +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215334 +obsolete: false +marked_obsolete_by_url: +marked_obsolete_in_milestone: diff --git a/ee/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability.rb b/ee/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability.rb new file mode 100644 index 0000000000000000000000000000000000000000..5e42104e1a7aa2795d6e87ceb81395978df34374 --- /dev/null +++ b/ee/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddUndetectedSinceFieldToVulnerability < Elastic::Migration + include ::Search::Elastic::MigrationUpdateMappingsHelper + + DOCUMENT_TYPE = Vulnerability + + private + + def new_mappings + { + undetected_since: { + type: 'date' + } + } + end +end diff --git a/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb b/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb index d6138fcbeffdba1b586bb5efbd9f0133fbe319b7..e7963305fa51c3da35040713ead9c2abbaa20d54 100644 --- a/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb +++ b/ee/lib/search/elastic/preloaders/vulnerability/enhanced_proxy.rb @@ -9,7 +9,7 @@ module Vulnerability # # This class: # 1. Coordinates multiple individual preloaders (EPSS, CVE, Reachability, TokenStatus, - # PolicyViolations, RiskScore, FalsePositive) + # PolicyViolations, RiskScore, FalsePositive, UndetectedSince) # 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 @@ -37,6 +37,7 @@ def preload_all_data @policy_violations_data = PolicyViolations.new(records).preload @risk_score_data = RiskScore.new(records).preload @false_positive_data = FalsePositive.new(records).preload + @undetected_since_data = UndetectedSince.new(records).preload end # Create enhanced proxies and assign them to references @@ -59,7 +60,8 @@ def create_enhanced_proxy(record) 'token_status' => fetch_token_status_value(vulnerability_id), 'policy_violations' => fetch_policy_violation_value(vulnerability_id), 'risk_score' => fetch_risk_score_value(vulnerability_id), - 'false_positive' => fetch_false_positive_value(vulnerability_id) + 'false_positive' => fetch_false_positive_value(vulnerability_id), + 'undetected_since' => fetch_undetected_since_value(vulnerability_id) }.with_indifferent_access ::Search::Elastic::RecordProxy::Vulnerability.create_with_enhancements(record, enhancements) @@ -84,6 +86,10 @@ def fetch_risk_score_value(vulnerability_id) def fetch_false_positive_value(vulnerability_id) @false_positive_data[vulnerability_id] || false end + + def fetch_undetected_since_value(vulnerability_id) + @undetected_since_data[vulnerability_id] + end end end end diff --git a/ee/lib/search/elastic/preloaders/vulnerability/undetected_since.rb b/ee/lib/search/elastic/preloaders/vulnerability/undetected_since.rb new file mode 100644 index 0000000000000000000000000000000000000000..b672c845b57fadf109b076294b91ff2c3a52a75a --- /dev/null +++ b/ee/lib/search/elastic/preloaders/vulnerability/undetected_since.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Search + module Elastic + module Preloaders + module Vulnerability + # Preloads the undetected_since timestamp for vulnerabilities. + # + # Returns a Hash like: + # { vulnerability_id => undetected_since_timestamp } + class UndetectedSince < Base + # @return [Hash{Integer => date}] vulnerability_id => undetected_since_timestamp + def perform_preload + vulnerability_ids = record_identifiers + + return {} if vulnerability_ids.empty? + + fetch_undetected_since_data(vulnerability_ids) + end + + private + + def fetch_undetected_since_data(vulnerability_ids) + ::Vulnerabilities::Finding + .by_vulnerability(vulnerability_ids) + .with_latest_detection_transition + .each_with_object({}) do |finding, result| + next unless finding.vulnerability_id + + transition = finding.latest_detection_transition + next if transition.nil? || transition.detected? + + result[finding.vulnerability_id] = transition.created_at + end + 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 5abb0be0d2c0939c4a7c318ac2195d3c2596b799..c43990017a3f0dac932ef038290d1ff51d267838 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, token status, policy_violations, risk_score, and false_positive information. + # reachability, token status, policy_violations, risk_score, false_positive and undetected_since information. class Vulnerability < Base # Creates a vulnerability proxy with all optimizations applied def self.create_with_enhancements(record, enhancements) @@ -16,7 +16,8 @@ def self.create_with_enhancements(record, enhancements) token_status: enhancements[:token_status], policy_violations: enhancements[:policy_violations], risk_score: enhancements[:risk_score], - false_positive: enhancements[:false_positive] + false_positive: enhancements[:false_positive], + undetected_since: enhancements[:undetected_since] }) proxy diff --git a/ee/lib/search/elastic/references/vulnerability.rb b/ee/lib/search/elastic/references/vulnerability.rb index cec15c198a4cd9fca4a69689bf9119d36b940726..0dbeb92943d2701882782b608459cbf184443e36 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_49 + SCHEMA_VERSION = 25_50 DOC_TYPE = 'vulnerability' INDEX_NAME = 'vulnerabilities' @@ -71,6 +71,7 @@ def serialize end # Generate the JSON representation for elasticsearch indexing + # rubocop:disable Metrics/AbcSize -- We want this to stay a single method override :as_indexed_json def as_indexed_json fields = {} @@ -110,8 +111,13 @@ def as_indexed_json fields["false_positive"] = fetch_record_attribute(database_record, :false_positive) end + if undetected_since_migration_completed? + fields["undetected_since"] = fetch_record_attribute(database_record, :undetected_since) + end + internal_es_fields.merge(fields) end + # rubocop:enable Metrics/AbcSize override :index_name def index_name @@ -153,8 +159,10 @@ def internal_es_fields end def fetch_schema_version - if security_project_tracked_context_id_migration_completed? + if undetected_since_migration_completed? SCHEMA_VERSION + elsif security_project_tracked_context_id_migration_completed? + 25_49 elsif false_positive_migration_completed? 25_48 elsif risk_score_migration_completed? @@ -205,6 +213,10 @@ def false_positive_migration_completed? ::Elastic::DataMigrationService.migration_has_finished?(:add_false_positive_field_to_vulnerability) end + def undetected_since_migration_completed? + ::Elastic::DataMigrationService.migration_has_finished?(:add_undetected_since_field_to_vulnerability) + end + # # Private class methods for implementation details # diff --git a/ee/lib/search/elastic/types/vulnerability.rb b/ee/lib/search/elastic/types/vulnerability.rb index 1301d88b56fc5c63ccdba1ba8a785df5a2228244..6db841d26860786610b36ab32a91419ae028444e 100644 --- a/ee/lib/search/elastic/types/vulnerability.rb +++ b/ee/lib/search/elastic/types/vulnerability.rb @@ -77,7 +77,8 @@ def base_mappings policy_violations: { type: 'short' }, # enum false_positive: { type: 'boolean' }, schema_version: { type: 'short' }, - security_project_tracked_context_id: { type: 'long' } + security_project_tracked_context_id: { type: 'long' }, + undetected_since: { type: 'date' } } end diff --git a/ee/spec/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability_spec.rb b/ee/spec/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..261bd5d00047337fd16435ff3f241066536a2d9d --- /dev/null +++ b/ee/spec/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' +require File.expand_path('ee/elastic/migrate/20251204112110_add_undetected_since_field_to_vulnerability.rb') + +RSpec.describe AddUndetectedSinceFieldToVulnerability, :elastic, feature_category: :vulnerability_management do + let(:version) { 20251204112110 } + + include_examples 'migration adds mapping' +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 dd122a773edefe919a9dfaa982ab46b4dd2b80ca..fcd54b01b6608e3aba26d9c7bec7a5d9456f241f 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 @@ -25,6 +25,7 @@ let(:mock_risk_score_preloader) { instance_double(Search::Elastic::Preloaders::Vulnerability::RiskScore) } let(:mock_false_positive_preloader) { instance_double(Search::Elastic::Preloaders::Vulnerability::FalsePositive) } + let(:mock_undetected_since_preloader) { instance_double(Search::Elastic::Preloaders::Vulnerability::UndetectedSince) } let(:reachability_data) do { @@ -64,6 +65,13 @@ } end + let(:undetected_since_data) do + { + vulnerability_reads[1].vulnerability_id => 2.days.ago + # first and third intentionally missing (still detected) + } + end + let(:enhanced_proxy) { described_class.new(refs, vulnerability_reads) } before do @@ -112,6 +120,15 @@ allow(mock_false_positive_preloader) .to receive(:preload) .and_return(false_positive_data) + + allow(Search::Elastic::Preloaders::Vulnerability::UndetectedSince) + .to receive(:new) + .with(vulnerability_reads) + .and_return(mock_undetected_since_preloader) + + allow(mock_undetected_since_preloader) + .to receive(:preload) + .and_return(undetected_since_data) end describe '#initialize' do @@ -133,7 +150,7 @@ end describe '#preload_and_enhance!' do - it 'preloads reachability, token status, policy_violations, risk score and false_positive data' do + it 'preloads reachability, token status, policy_violations, risk score, false_positive and undetected_since data' do enhanced_proxy.preload_and_enhance! expect(Search::Elastic::Preloaders::Vulnerability::Reachability) @@ -155,6 +172,10 @@ expect(Search::Elastic::Preloaders::Vulnerability::FalsePositive) .to have_received(:new).with(vulnerability_reads) expect(mock_false_positive_preloader).to have_received(:preload) + + expect(Search::Elastic::Preloaders::Vulnerability::UndetectedSince) + .to have_received(:new).with(vulnerability_reads) + expect(mock_undetected_since_preloader).to have_received(:preload) end it 'creates enhanced proxies for matching refs' do @@ -200,13 +221,15 @@ 'token_status, ' \ 'policy_violations, ' \ 'risk_score, ' \ - 'and false_positive' do + 'false_positive, ' \ + 'and undetected_since' do expected_enhancements_in_use = { 'reachability' => ::Enums::Sbom::REACHABILITY_TYPES[::Enums::Sbom::IN_USE], 'token_status' => ::Security::TokenStatus::ACTIVE, 'policy_violations' => ::Security::PolicyViolations::DISMISSED_IN_MR, 'risk_score' => 0.4, - 'false_positive' => true + 'false_positive' => true, + 'undetected_since' => nil }.with_indifferent_access expected_enhancements_not_found = { @@ -214,7 +237,8 @@ 'token_status' => ::Security::TokenStatus::INACTIVE, 'policy_violations' => nil, 'risk_score' => 0.6, - 'false_positive' => false + 'false_positive' => false, + 'undetected_since' => undetected_since_data[vulnerability_reads[1].vulnerability_id] }.with_indifferent_access expected_enhancements_unknown = { @@ -222,7 +246,8 @@ 'token_status' => ::Security::TokenStatus::UNKNOWN, 'policy_violations' => nil, 'risk_score' => 0.0, - 'false_positive' => false + 'false_positive' => false, + 'undetected_since' => nil }.with_indifferent_access expect(::Search::Elastic::RecordProxy::Vulnerability) diff --git a/ee/spec/lib/search/elastic/preloaders/vulnerability/undetected_since_spec.rb b/ee/spec/lib/search/elastic/preloaders/vulnerability/undetected_since_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..72f95329643dc10af4978c1f39e95b6a5be855bf --- /dev/null +++ b/ee/spec/lib/search/elastic/preloaders/vulnerability/undetected_since_spec.rb @@ -0,0 +1,212 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Search::Elastic::Preloaders::Vulnerability::UndetectedSince, + feature_category: :vulnerability_management do + let_it_be(:project) { create(:project) } + + def create_vulnerability_with_detection_transition(detected:, created_at: Time.current) + vulnerability = create(:vulnerability, :with_read, project: project) + finding = create(:vulnerabilities_finding, vulnerability: vulnerability) + + create(:vulnerability_detection_transition, + finding: finding, + detected: detected, + created_at: created_at + ) + + vulnerability.vulnerability_read + end + + describe '#preload' do + subject(:preloader) { described_class.new(records) } + + context 'with vulnerabilities having different detection states' do + let_it_be(:vuln_undetected) do + create_vulnerability_with_detection_transition(detected: false, created_at: 2.days.ago) + end + + let_it_be(:vuln_detected) do + create_vulnerability_with_detection_transition(detected: true, created_at: 1.day.ago) + end + + let(:records) { [vuln_undetected, vuln_detected] } + + it 'returns undetected_since timestamp for undetected vulnerabilities' do + result = preloader.preload + + expect(result[vuln_undetected.vulnerability_id]).to be_present + expect(result[vuln_undetected.vulnerability_id]).to be_within(5.seconds).of(2.days.ago) + end + + it 'does not include detected vulnerabilities in the result' do + result = preloader.preload + + expect(result).not_to have_key(vuln_detected.vulnerability_id) + end + end + + context 'with vulnerabilities having multiple detection transitions' do + let_it_be(:vuln_with_multiple_transitions) do + vulnerability = create(:vulnerability, :with_read, project: project) + finding = create(:vulnerabilities_finding, vulnerability: vulnerability) + + create(:vulnerability_detection_transition, + finding: finding, + detected: true, + created_at: 3.days.ago + ) + + create(:vulnerability_detection_transition, + finding: finding, + detected: false, + created_at: 1.day.ago + ) + + vulnerability.vulnerability_read + end + + let(:records) { [vuln_with_multiple_transitions] } + + it 'returns the timestamp from the latest detection transition' do + result = preloader.preload + + expect(result[vuln_with_multiple_transitions.vulnerability_id]).to be_within(1.second).of(1.day.ago) + end + end + + context 'with edge cases' do + context 'when records are empty' do + let(:records) { [] } + + it 'returns an empty hash' do + expect(preloader.preload).to eq({}) + end + end + + context 'when vulnerability has no detection transitions' do + let_it_be(:vuln_without_transition) do + vulnerability = create(:vulnerability, :with_read, project: project) + create(:vulnerabilities_finding, vulnerability: vulnerability) + vulnerability.vulnerability_read + end + + let(:records) { [vuln_without_transition] } + + it 'does not include the vulnerability in the result' do + result = preloader.preload + + expect(result).not_to have_key(vuln_without_transition.vulnerability_id) + end + end + + context 'when database query raises an error' do + let(:records) { [create_vulnerability_with_detection_transition(detected: false)] } + + before do + allow(::Vulnerabilities::Finding) + .to receive(:by_vulnerability) + .and_raise(StandardError, 'DB failure') + + allow(::Gitlab::ErrorTracking).to receive(:track_exception) + end + + it 'handles gracefully and tracks the error' do + result = begin + preloader.preload + rescue StandardError + {} + end + + expect(result).to eq({}) + expect(::Gitlab::ErrorTracking).to have_received(:track_exception) + .with(instance_of(StandardError), class: described_class.name) + end + end + end + end + + describe '#data_for' do + let_it_be(:vuln_undetected) do + create_vulnerability_with_detection_transition(detected: false, created_at: 2.days.ago) + end + + let_it_be(:vuln_detected) do + create_vulnerability_with_detection_transition(detected: true) + end + + subject(:preloader) { described_class.new([vuln_undetected, vuln_detected]) } + + before do + preloader.preload + end + + it 'returns undetected_since timestamp for undetected vulnerabilities' do + expect(preloader.data_for(vuln_undetected)).to be_within(5.seconds).of(2.days.ago) + end + + it 'returns nil for detected vulnerabilities' do + expect(preloader.data_for(vuln_detected)).to be_nil + end + + it 'returns nil for unknown vulnerability' do + unknown_read = create(:vulnerability, :with_read, project: project).vulnerability_read + expect(preloader.data_for(unknown_read)).to be_nil + end + end + + describe 'integration with PreloaderBase' do + let_it_be(:vuln_undetected) do + create_vulnerability_with_detection_transition(detected: false) + end + + subject(:preloader) { described_class.new([vuln_undetected]) } + + it 'tracks preload state correctly' do + expect(preloader.preloaded?).to be false + preloader.preload + expect(preloader.preloaded?).to be true + end + + it 'deduplicates vulnerabilities before querying' do + duplicates = [vuln_undetected, vuln_undetected] + preloader = described_class.new(duplicates) + + expect(::Vulnerabilities::Finding).to receive(:by_vulnerability).once.and_call_original + preloader.preload + end + + it 'caches preloaded data to avoid duplicate queries' do + expect(::Vulnerabilities::Finding).to receive(:by_vulnerability).once.and_call_original + + result1 = preloader.preload + result2 = preloader.preload + + expect(result1).to eq(result2) + end + end + + describe 'performance characteristics' do + let_it_be(:large_vulnerability_set) do + Array.new(5) do + create_vulnerability_with_detection_transition(detected: false, created_at: rand(1..10).days.ago) + end + end + + it 'performs batch queries efficiently' 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 + initial_set = large_vulnerability_set.take(2) + preloader = described_class.new(initial_set) + control = ActiveRecord::QueryRecorder.new { preloader.preload } + + larger_preloader = described_class.new(large_vulnerability_set) + + expect { larger_preloader.preload }.not_to exceed_query_limit(control) + 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 ad24b418ab14befac5f8f54dda05c9f520b975b4..e152f4f0db6e20402c04238174519d9989a01c71 100644 --- a/ee/spec/lib/search/elastic/references/vulnerability_spec.rb +++ b/ee/spec/lib/search/elastic/references/vulnerability_spec.rb @@ -81,6 +81,7 @@ policy_violations: [], risk_score: [], false_positive: [], + undetected_since: [], type: described_class::DOC_TYPE, schema_version: described_class::SCHEMA_VERSION, security_project_tracked_context_id: object.security_project_tracked_context_id @@ -409,6 +410,40 @@ end end + context 'with undetected_since migration mappings' do + context 'when undetected_since migration has finished' do + let(:undetected_since_data) { 2.days.ago } + + before do + set_elasticsearch_migration_to(:add_undetected_since_field_to_vulnerability) + allow(object).to receive(:undetected_since).and_return(undetected_since_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_50) + end + + it 'includes undetected_since in the indexed json' do + expect(indexed_json[:undetected_since]).to be_within(0.1.seconds).of(undetected_since_data) + end + end + + context 'when migration has not completed' do + before do + set_elasticsearch_migration_to(:add_undetected_since_field_to_vulnerability, including: false) + end + + it 'returns schema version with security_project_tracked_context_id' do + expect(indexed_json[:schema_version]).to eq(25_49) + end + + it 'does not assign undetected_since on the indexed json' do + expect(indexed_json[:undetected_since]).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) @@ -530,6 +565,20 @@ end end + context 'with undetected_since data' do + let(:transition_time) { 2.days.ago } + let!(:transition) do + create(:vulnerability_detection_transition, finding: finding, detected: false, created_at: transition_time) + end + + it 'preloads undetected_since' do + described_class.preload_indexing_data(refs) + + expect(vulnerability_ref.database_record.undetected_since).to be_within(0.1.seconds).of(transition_time) + expect(user_vulnerability_ref.database_record.undetected_since).to be_nil + end + end + context 'with token_status data' do let!(:secret_finding) do create( @@ -645,6 +694,10 @@ create(:finding_token_status, finding: finding2, project: project2, status: ::Security::TokenStatus::INACTIVE) + # Create detection transitions + create(:vulnerability_detection_transition, finding: finding1, detected: false, created_at: 2.days.ago) + create(:vulnerability_detection_transition, finding: finding2, detected: false, created_at: 1.day.ago) + # Create refs array for initial batch refs = [ described_class.new(vulnerability_read1.vulnerability_id, vulnerability_read1.es_parent) @@ -691,6 +744,9 @@ # Create finding with token_status create(:finding_token_status, finding: finding3, project: project2, status: ::Security::TokenStatus::UNKNOWN) + # Create detection transition for third finding + create(:vulnerability_detection_transition, finding: finding3, detected: false, created_at: 3.days.ago) + # Add new refs to the array refs += [ described_class.new(vulnerability_read2.vulnerability_id, vulnerability_read2.es_parent), diff --git a/ee/spec/models/vulnerabilities/finding_spec.rb b/ee/spec/models/vulnerabilities/finding_spec.rb index c9413c1f11406f96efcab2db861240e06398702c..0e91610af528e501db0198ee1604789a0152ef9d 100644 --- a/ee/spec/models/vulnerabilities/finding_spec.rb +++ b/ee/spec/models/vulnerabilities/finding_spec.rb @@ -1991,4 +1991,60 @@ def create_finding(state) end end end + + describe '.with_latest_detection_transition' do + let_it_be(:finding1) { create(:vulnerabilities_finding) } + let_it_be(:finding2) { create(:vulnerabilities_finding) } + let_it_be(:finding3) { create(:vulnerabilities_finding) } + + let_it_be(:_transition1_old) do + create(:vulnerability_detection_transition, finding: finding1, detected: true, created_at: 2.days.ago) + end + + let_it_be(:transition1_latest) do + create(:vulnerability_detection_transition, finding: finding1, detected: false, created_at: 1.day.ago) + end + + let_it_be(:transition2_latest) do + create(:vulnerability_detection_transition, finding: finding2, detected: true, created_at: 1.day.ago) + end + + subject { described_class.with_latest_detection_transition } + + it 'includes only the latest detection transition for each finding', :aggregate_failures do + findings = subject.to_a + + expect(findings).to contain_exactly(finding1, finding2) + expect(findings.find { |f| f.id == finding1.id }.detection_transitions).to contain_exactly(transition1_latest) + expect(findings.find { |f| f.id == finding2.id }.detection_transitions).to contain_exactly(transition2_latest) + end + + it 'does not include findings without detection transitions' do + expect(subject).not_to include(finding3) + end + + it 'orders by detection transition id descending' do + finding_with_transitions = subject.find_by(id: finding1.id) + + expect(finding_with_transitions.detection_transitions.first).to eq(transition1_latest) + end + end + + describe '#latest_detection_transition' do + let_it_be(:finding) { create(:vulnerabilities_finding) } + + context 'when finding has multiple detection transitions' do + let_it_be(:_old_transition) do + create(:vulnerability_detection_transition, finding: finding, detected: true, created_at: 2.days.ago) + end + + let_it_be(:latest_transition) do + create(:vulnerability_detection_transition, :not_detected, finding: finding, created_at: 1.day.ago) + end + + it 'returns the last detection transition' do + expect(finding.latest_detection_transition).to eq(latest_transition) + end + end + end end diff --git a/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb b/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb index d3e6dc725eb1f77e9674cb9009b8f1327d573959..28237e521b905c9ea5db6baf1315b67e654e7b3c 100644 --- a/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb +++ b/ee/spec/services/security/ingestion/mark_as_resolved_service_spec.rb @@ -53,6 +53,27 @@ def expect_vulnerability_not_to_be_resolved(vulnerability) expect(vulnerability.vulnerability_read.resolved_on_default_branch).to be_truthy end + it 'creates a detection transition with detected: false', :aggregate_failures do + finding_ids = vulnerability.findings.pluck(:id) + + expect { command.execute }.to change { + Vulnerabilities::DetectionTransition.where(vulnerability_occurrence_id: finding_ids).count + }.by(1) + + transition = Vulnerabilities::DetectionTransition.find_by(vulnerability_occurrence_id: finding_ids) + expect(transition.detected).to be(false) + end + + context 'when new_security_dashboard_exclude_no_longer_detected is disabled' do + before do + stub_feature_flags(new_security_dashboard_exclude_no_longer_detected: false) + end + + it 'does not create detection transitions' do + expect { command.execute }.not_to change { Vulnerabilities::DetectionTransition.count } + end + end + context 'when there is a no longer detected vulnerability' do let_it_be_with_reload(:no_longer_detected) do create(:vulnerability, :sast, diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected_spec.rb index 3ade67ee6faf394a598a7a89b90786130c14c04a..78c53c5be01139fd3ba1df312f652ce54afa3d05 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities/mark_resolved_as_detected_spec.rb @@ -73,6 +73,27 @@ expect(state_transition.vulnerability_id).to eq(resolved_vulnerability.id) end + it 'creates a detection transition with detected: true', :aggregate_failures do + finding_ids = resolved_vulnerability.findings.pluck(:id) + + expect { mark_resolved_as_detected }.to change { + Vulnerabilities::DetectionTransition.where(vulnerability_occurrence_id: finding_ids).count + }.by(1) + + transition = Vulnerabilities::DetectionTransition.find_by(vulnerability_occurrence_id: finding_ids) + expect(transition.detected).to be(true) + end + + context 'when new_security_dashboard_exclude_no_longer_detected is disabled' do + before do + stub_feature_flags(new_security_dashboard_exclude_no_longer_detected: false) + end + + it 'does not create detection transitions' do + expect { mark_resolved_as_detected }.not_to change { Vulnerabilities::DetectionTransition.count } + end + end + it 'marks the findings as transitioned_to_detected' do expect { mark_resolved_as_detected }.to change { existing_resolved_finding_map.transitioned_to_detected }.to(true) .and not_change { existing_detected_finding_map.transitioned_to_detected } diff --git a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb index 0d6dfc92a9a322f2f0fc5fd058d7ef48e53af0c2..eef9a85238d41124e0d1c729f31de655aa51e00c 100644 --- a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb @@ -55,6 +55,27 @@ )) end + it 'creates detection transitions with detected: false', :aggregate_failures do + finding_ids = undetected_vulnerabilities.flat_map { |v| v.findings.pluck(:id) } + + expect { service.execute }.to change { + Vulnerabilities::DetectionTransition.where(vulnerability_occurrence_id: finding_ids).count + }.by(undetected_vulnerabilities.count) + + transitions = Vulnerabilities::DetectionTransition.where(vulnerability_occurrence_id: finding_ids) + expect(transitions).to all(have_attributes(detected: false)) + end + + context 'when new_security_dashboard_exclude_no_longer_detected is disabled' do + before do + stub_feature_flags(new_security_dashboard_exclude_no_longer_detected: false) + end + + it 'does not create detection transitions' do + expect { service.execute }.not_to change { Vulnerabilities::DetectionTransition.count } + end + end + it 'touches the updated_at timestamp', :freeze_time do service.execute