From 8cf2cb57cd7b00cae1f1b8e5c6405da79d33fdd9 Mon Sep 17 00:00:00 2001 From: Subashis Date: Thu, 27 Nov 2025 12:15:31 -0700 Subject: [PATCH] Add detection_transitions table to track no longer detected vulnerabilites --- config/gitlab_loose_foreign_keys.yml | 4 + .../vulnerability_detection_transitions.yml | 13 +++ ...ate_vulnerability_detection_transitions.rb | 30 +++++ db/schema_migrations/20251126190948 | 1 + db/structure.sql | 30 +++++ .../vulnerabilities/detection_transition.rb | 17 +++ ee/app/models/vulnerabilities/finding.rb | 2 + .../vulnerabilities/detection_transitions.rb | 17 +++ .../detection_transition_spec.rb | 104 ++++++++++++++++++ .../models/vulnerabilities/finding_spec.rb | 1 + spec/lib/gitlab/import_export/all_models.yml | 1 + 11 files changed, 220 insertions(+) create mode 100644 db/docs/vulnerability_detection_transitions.yml create mode 100644 db/migrate/20251126190948_create_vulnerability_detection_transitions.rb create mode 100644 db/schema_migrations/20251126190948 create mode 100644 ee/app/models/vulnerabilities/detection_transition.rb create mode 100644 ee/spec/factories/vulnerabilities/detection_transitions.rb create mode 100644 ee/spec/models/vulnerabilities/detection_transition_spec.rb diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 1f937c90cf6298..faa24f1e2723a3 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -952,6 +952,10 @@ vulnerability_archives: - table: projects column: project_id on_delete: async_delete +vulnerability_detection_transitions: + - table: projects + column: project_id + on_delete: async_delete vulnerability_export_parts: - table: organizations column: organization_id diff --git a/db/docs/vulnerability_detection_transitions.yml b/db/docs/vulnerability_detection_transitions.yml new file mode 100644 index 00000000000000..c96769d00f4858 --- /dev/null +++ b/db/docs/vulnerability_detection_transitions.yml @@ -0,0 +1,13 @@ +--- +table_name: vulnerability_detection_transitions +classes: +- Vulnerabilities::DetectionTransition +feature_categories: +- vulnerability_management +description: Tracks detection state transitions for vulnerability findings. +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/214443 +milestone: '18.7' +gitlab_schema: gitlab_sec +sharding_key: + project_id: projects +table_size: small diff --git a/db/migrate/20251126190948_create_vulnerability_detection_transitions.rb b/db/migrate/20251126190948_create_vulnerability_detection_transitions.rb new file mode 100644 index 00000000000000..eb9be31ab2b055 --- /dev/null +++ b/db/migrate/20251126190948_create_vulnerability_detection_transitions.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class CreateVulnerabilityDetectionTransitions < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.7' + + FINDING_INDEX_NAME = 'index_vulnerability_detection_transitions_on_finding_id' + PROJECT_INDEX_NAME = 'index_vulnerability_detection_transitions_on_project_id' + + def up + create_table :vulnerability_detection_transitions, if_not_exists: true do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/578566 + t.bigint :vulnerability_occurrence_id, null: false, index: { name: FINDING_INDEX_NAME } + t.bigint :project_id, null: false, index: { name: PROJECT_INDEX_NAME } + t.timestamps_with_timezone null: false + t.boolean :detected, null: false, default: true + end + + add_concurrent_foreign_key( + :vulnerability_detection_transitions, + :vulnerability_occurrences, + column: :vulnerability_occurrence_id, + on_delete: :cascade, + if_not_exists: true + ) + end + + def down + drop_table :vulnerability_detection_transitions + end +end diff --git a/db/schema_migrations/20251126190948 b/db/schema_migrations/20251126190948 new file mode 100644 index 00000000000000..2e9dc7512a6c23 --- /dev/null +++ b/db/schema_migrations/20251126190948 @@ -0,0 +1 @@ +c0ba9dd0ce705135d13cc8ece7875231e3ea52ea8c784b9e1b31d8e03292fc41 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 733aa27777349d..b638b5a746b120 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -29234,6 +29234,24 @@ CREATE SEQUENCE vulnerability_archives_id_seq ALTER SEQUENCE vulnerability_archives_id_seq OWNED BY vulnerability_archives.id; +CREATE TABLE vulnerability_detection_transitions ( + id bigint NOT NULL, + vulnerability_occurrence_id bigint NOT NULL, + project_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + detected boolean DEFAULT true NOT NULL +); + +CREATE SEQUENCE vulnerability_detection_transitions_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE vulnerability_detection_transitions_id_seq OWNED BY vulnerability_detection_transitions.id; + CREATE TABLE vulnerability_export_part_uploads ( id bigint NOT NULL, size bigint NOT NULL, @@ -33088,6 +33106,8 @@ ALTER TABLE ONLY vulnerability_archived_records ALTER COLUMN id SET DEFAULT next ALTER TABLE ONLY vulnerability_archives ALTER COLUMN id SET DEFAULT nextval('vulnerability_archives_id_seq'::regclass); +ALTER TABLE ONLY vulnerability_detection_transitions ALTER COLUMN id SET DEFAULT nextval('vulnerability_detection_transitions_id_seq'::regclass); + ALTER TABLE ONLY vulnerability_export_parts ALTER COLUMN id SET DEFAULT nextval('vulnerability_export_parts_id_seq'::regclass); ALTER TABLE ONLY vulnerability_exports ALTER COLUMN id SET DEFAULT nextval('vulnerability_exports_id_seq'::regclass); @@ -37017,6 +37037,9 @@ ALTER TABLE ONLY vulnerability_archived_records ALTER TABLE ONLY vulnerability_archives ADD CONSTRAINT vulnerability_archives_pkey PRIMARY KEY (id, date); +ALTER TABLE ONLY vulnerability_detection_transitions + ADD CONSTRAINT vulnerability_detection_transitions_pkey PRIMARY KEY (id); + ALTER TABLE ONLY vulnerability_export_part_uploads ADD CONSTRAINT vulnerability_export_part_uploads_pkey PRIMARY KEY (id, model_type); @@ -44665,6 +44688,10 @@ CREATE UNIQUE INDEX index_vulnerability_archived_records_on_unique_attributes ON CREATE UNIQUE INDEX index_vulnerability_archives_on_project_id_and_date ON ONLY vulnerability_archives USING btree (project_id, date); +CREATE INDEX index_vulnerability_detection_transitions_on_finding_id ON vulnerability_detection_transitions USING btree (vulnerability_occurrence_id); + +CREATE INDEX index_vulnerability_detection_transitions_on_project_id ON vulnerability_detection_transitions USING btree (project_id); + CREATE INDEX index_vulnerability_export_parts_on_organization_id ON vulnerability_export_parts USING btree (organization_id); CREATE INDEX index_vulnerability_export_parts_on_vulnerability_export_id ON vulnerability_export_parts USING btree (vulnerability_export_id); @@ -49582,6 +49609,9 @@ ALTER TABLE ONLY lists ALTER TABLE ONLY subscription_user_add_on_assignments ADD CONSTRAINT fk_0d89020c49 FOREIGN KEY (add_on_purchase_id) REFERENCES subscription_add_on_purchases(id) ON DELETE CASCADE; +ALTER TABLE ONLY vulnerability_detection_transitions + ADD CONSTRAINT fk_0df30df5da FOREIGN KEY (vulnerability_occurrence_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE; + ALTER TABLE ONLY approval_project_rules_users ADD CONSTRAINT fk_0dfcd9e339 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/vulnerabilities/detection_transition.rb b/ee/app/models/vulnerabilities/detection_transition.rb new file mode 100644 index 00000000000000..f2879c9c21627e --- /dev/null +++ b/ee/app/models/vulnerabilities/detection_transition.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Vulnerabilities + class DetectionTransition < ::SecApplicationRecord + self.table_name = 'vulnerability_detection_transitions' + + belongs_to :finding, + class_name: 'Vulnerabilities::Finding', + foreign_key: 'vulnerability_occurrence_id', + inverse_of: :detection_transitions + + belongs_to :project, optional: false + + validates :vulnerability_occurrence_id, presence: true + validates :detected, inclusion: { in: [true, false] } + end +end diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb index 7a0ee200bc9ecb..2153297c24704f 100644 --- a/ee/app/models/vulnerabilities/finding.rb +++ b/ee/app/models/vulnerabilities/finding.rb @@ -116,6 +116,8 @@ class Finding < ::SecApplicationRecord has_one :finding_evidence, class_name: 'Vulnerabilities::Finding::Evidence', inverse_of: :finding, foreign_key: 'vulnerability_occurrence_id' + has_many :detection_transitions, class_name: 'Vulnerabilities::DetectionTransition', inverse_of: :finding, foreign_key: 'vulnerability_occurrence_id' + has_many :security_findings, class_name: 'Security::Finding', primary_key: :uuid, diff --git a/ee/spec/factories/vulnerabilities/detection_transitions.rb b/ee/spec/factories/vulnerabilities/detection_transitions.rb new file mode 100644 index 00000000000000..1e2034458f6723 --- /dev/null +++ b/ee/spec/factories/vulnerabilities/detection_transitions.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :vulnerability_detection_transition, class: 'Vulnerabilities::DetectionTransition' do + project + finding { association(:vulnerabilities_finding) } + detected { true } + + trait :detected do + detected { true } + end + + trait :not_detected do + detected { false } + end + end +end diff --git a/ee/spec/models/vulnerabilities/detection_transition_spec.rb b/ee/spec/models/vulnerabilities/detection_transition_spec.rb new file mode 100644 index 00000000000000..9c4a0b456e6f80 --- /dev/null +++ b/ee/spec/models/vulnerabilities/detection_transition_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Vulnerabilities::DetectionTransition, feature_category: :vulnerability_management do + describe 'associations' do + it { is_expected.to belong_to(:finding).with_foreign_key('vulnerability_occurrence_id') } + it { is_expected.to belong_to(:finding).inverse_of(:detection_transitions) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:vulnerability_occurrence_id) } + it { is_expected.to belong_to(:project).required } + it { is_expected.to validate_inclusion_of(:detected).in_array([true, false]) } + end + + describe 'table_name' do + it 'uses vulnerability_detection_transitions table' do + expect(described_class.table_name).to eq('vulnerability_detection_transitions') + end + end + + describe 'database constraints' do + let_it_be(:finding) { create(:vulnerabilities_finding) } + + it 'enforces presence of vulnerability_occurrence_id' do + transition = described_class.new(detected: true) + expect(transition).not_to be_valid + expect(transition.errors[:vulnerability_occurrence_id]).to include("can't be blank") + end + + it 'allows detected to be true' do + transition = build(:vulnerability_detection_transition, finding: finding, detected: true) + expect(transition).to be_valid + end + + it 'allows detected to be false' do + transition = build(:vulnerability_detection_transition, finding: finding, detected: false) + expect(transition).to be_valid + end + + it 'does not allow detected to be nil' do + transition = build(:vulnerability_detection_transition, finding: finding) + transition.detected = nil + expect(transition).not_to be_valid + expect(transition.errors[:detected]).to include('is not included in the list') + end + end + + describe 'foreign key cascade' do + let_it_be(:finding) { create(:vulnerabilities_finding) } + let_it_be(:transition) { create(:vulnerability_detection_transition, finding: finding) } + + it 'deletes transition when finding is deleted' do + expect { finding.destroy! }.to change { described_class.count }.by(-1) + end + end + + describe 'timestamps' do + let(:transition) { create(:vulnerability_detection_transition) } + + it 'sets created_at on creation' do + expect(transition.created_at).to be_present + end + + it 'sets updated_at on creation' do + expect(transition.updated_at).to be_present + end + + it 'updates updated_at on update' do + original_updated_at = transition.updated_at + travel_to(1.hour.from_now) do + transition.update!(detected: false) + expect(transition.updated_at).to be > original_updated_at + end + end + end + + describe 'multiple transitions for same finding' do + let_it_be(:finding) { create(:vulnerabilities_finding) } + + let_it_be(:first_transition) do + create(:vulnerability_detection_transition, finding: finding, detected: true, created_at: 3.days.ago) + end + + let_it_be(:second_transition) do + create(:vulnerability_detection_transition, finding: finding, detected: false, created_at: 2.days.ago) + end + + let_it_be(:third_transition) do + create(:vulnerability_detection_transition, finding: finding, detected: true, created_at: 1.day.ago) + end + + it 'allows multiple transitions for the same finding' do + expect(described_class.where(vulnerability_occurrence_id: finding.id).count).to eq(3) + expect([first_transition, second_transition, third_transition]).to all(be_persisted) + end + + it 'tracks detection state changes over time' do + transitions = described_class.where(vulnerability_occurrence_id: finding.id).order(created_at: :asc) + expect(transitions.pluck(:detected)).to eq([true, false, true]) + end + end +end diff --git a/ee/spec/models/vulnerabilities/finding_spec.rb b/ee/spec/models/vulnerabilities/finding_spec.rb index cc834d51561c2a..c9413c1f11406f 100644 --- a/ee/spec/models/vulnerabilities/finding_spec.rb +++ b/ee/spec/models/vulnerabilities/finding_spec.rb @@ -33,6 +33,7 @@ it { is_expected.to have_many(:merge_request_links).through(:vulnerability) } it { is_expected.to have_many(:security_findings).class_name('Security::Finding') } it { is_expected.to have_many(:triggered_workflows).class_name('::Vulnerabilities::TriggeredWorkflow') } + it { is_expected.to have_many(:detection_transitions).class_name('::Vulnerabilities::DetectionTransition') } end describe 'validations' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 7f9e180b1c561a..cbb1352c177e3a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -1313,6 +1313,7 @@ vulnerability_finding: - cve_enrichments - triggered_workflows - flip_guard + - detection_transitions scanner: - findings - security_findings -- GitLab