From b72833979ad3d7c2489921297c23352bae60dd1f Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 4 Jun 2025 00:08:23 -0400 Subject: [PATCH 01/20] Add creation logic for vulnerability read igestion Took the work of creating a new vulnerability outside of Postgres and into a task. This way is easier to debug and manage when dealing with data consistency issues. Also implemented a feature flag for the current postgres logic. Changelog: added EE: true --- .../tasks/ingest_vulnerability_reads.rb | 6 ++ .../ingest_vulnerability_reads/create.rb | 63 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb index c5a1d2be60f068..54b9b8a7e44648 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb @@ -5,6 +5,8 @@ module Ingestion module Tasks class IngestVulnerabilityReads < AbstractTask def execute + create_vulnerability_reads + update_vulnerability_reads finding_maps @@ -12,6 +14,10 @@ def execute private + def create_vulnerability_reads + IngestVulnerabilityReads::Create.new(pipeline, finding_maps).execute + end + def update_vulnerability_reads IngestVulnerabilityReads::Update.new(pipeline, finding_maps).execute end diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb new file mode 100644 index 00000000000000..cdad92f98eae03 --- /dev/null +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module Security + module Ingestion + module Tasks + class IngestVulnerabilityReads + class Create < AbstractTask + include Gitlab::Ingestion::BulkInsertableTask + + self.model = Vulnerabilities::Read + + private + + def attributes + finding_maps.filter_map do |finding_map| + vulnerability_id = finding_map.vulnerability_id + next unless vulnerability_id + + vulnerability = vulnerabilities[vulnerability_id] + next unless vulnerability + + occurrence = finding_map.report_finding + location = occurrence.location || {} + agent_id = location.dig('kubernetes_resource', 'agent_id') + + { + vulnerability_id: vulnerability_id, + project_id: vulnerability.project_id, + scanner_id: finding_map.scanner_id, + report_type: vulnerability.report_type, + severity: vulnerability.severity, + state: vulnerability.state, + resolved_on_default_branch: vulnerability.resolved_on_default_branch, + uuid: occurrence.uuid, + location_image: location['image'], + cluster_agent_id: agent_id, + casted_cluster_agent_id: agent_id&.to_i, + has_issues: issue_links.include?(vulnerability_id), + has_merge_request: merge_request_links.include?(vulnerability_id) + } + end + end + + def vulnerabilities + @vulnerabilities ||= Vulnerability.by_ids(vulnerability_ids).index_by(&:id) + end + + def issue_links + @issue_links ||= Vulnerabilities::IssueLink.by_vulnerability_ids(vulnerability_ids).to_set + end + + def merge_request_links + @merge_request_links ||= Vulnerabilities::MergeRequestLink.by_vulnerability_ids(vulnerability_ids).to_set + end + + def vulnerability_ids + @vulnerability_ids ||= finding_maps.filter_map(&:vulnerability_id) + end + end + end + end + end +end -- GitLab From 0d107f31c8b1ec7cc53583eb7ef26024462b9f54 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Thu, 5 Jun 2025 10:50:35 -0400 Subject: [PATCH 02/20] Add unique_by for handling existing records --- .../ingestion/tasks/ingest_vulnerability_reads/create.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb index cdad92f98eae03..dbe0d27373ee06 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -8,6 +8,7 @@ class Create < AbstractTask include Gitlab::Ingestion::BulkInsertableTask self.model = Vulnerabilities::Read + self.unique_by = :vulnerability_id private -- GitLab From fcebd3637a5217514c2315603f4c35adb34d13fe Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Fri, 6 Jun 2025 17:24:31 -0400 Subject: [PATCH 03/20] Add migration to check if trigger should be skipped --- ..._or_update_vulnerability_reads_function.rb | 117 ++++++++++++++++++ db/schema_migrations/20250605145847 | 1 + db/structure.sql | 4 + 3 files changed, 122 insertions(+) create mode 100644 db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb create mode 100644 db/schema_migrations/20250605145847 diff --git a/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb b/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb new file mode 100644 index 00000000000000..030ce72e208b49 --- /dev/null +++ b/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +class UpdateInsertOrUpdateVulnerabilityReadsFunction < Gitlab::Database::Migration[2.3] + milestone '18.1' + + def up + execute <<~SQL + CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads() RETURNS trigger + LANGUAGE plpgsql + AS $$ + DECLARE + severity smallint; + state smallint; + report_type smallint; + resolved_on_default_branch boolean; + present_on_default_branch boolean; + has_issues boolean; + has_merge_request boolean; + BEGIN + IF (SELECT current_setting('vulnerability_management_dont_execute_db_trigger', true)::boolean) THEN + RETURN NULL; + END IF; + + IF (NEW.vulnerability_id IS NULL AND (TG_OP = 'INSERT' OR TG_OP = 'UPDATE')) THEN + RETURN NULL; + END IF; + + IF (TG_OP = 'UPDATE' AND OLD.vulnerability_id IS NOT NULL AND NEW.vulnerability_id IS NOT NULL) THEN + RETURN NULL; + END IF; + + SELECT + vulnerabilities.severity, vulnerabilities.state, vulnerabilities.report_type, vulnerabilities.resolved_on_default_branch, vulnerabilities.present_on_default_branch + INTO + severity, state, report_type, resolved_on_default_branch, present_on_default_branch + FROM + vulnerabilities + WHERE + vulnerabilities.id = NEW.vulnerability_id; + + IF present_on_default_branch IS NOT true THEN + RETURN NULL; + END IF; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.vulnerability_id) + INTO + has_issues; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_merge_request_links WHERE vulnerability_merge_request_links.vulnerability_id = NEW.vulnerability_id) + INTO + has_merge_request; + + INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request) + VALUES (NEW.vulnerability_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', NEW.location->'kubernetes_resource'->>'agent_id', CAST(NEW.location->'kubernetes_resource'->>'agent_id' AS bigint), has_issues, has_merge_request) + ON CONFLICT(vulnerability_id) DO NOTHING; + RETURN NULL; + END + $$; + SQL + end + + def down + execute <<~SQL + CREATE OR REPLACE FUNCTION insert_or_update_vulnerability_reads() RETURNS trigger + LANGUAGE plpgsql + AS $$ + DECLARE + severity smallint; + state smallint; + report_type smallint; + resolved_on_default_branch boolean; + present_on_default_branch boolean; + has_issues boolean; + has_merge_request boolean; + BEGIN + IF (NEW.vulnerability_id IS NULL AND (TG_OP = 'INSERT' OR TG_OP = 'UPDATE')) THEN + RETURN NULL; + END IF; + + IF (TG_OP = 'UPDATE' AND OLD.vulnerability_id IS NOT NULL AND NEW.vulnerability_id IS NOT NULL) THEN + RETURN NULL; + END IF; + + SELECT + vulnerabilities.severity, vulnerabilities.state, vulnerabilities.report_type, vulnerabilities.resolved_on_default_branch, vulnerabilities.present_on_default_branch + INTO + severity, state, report_type, resolved_on_default_branch, present_on_default_branch + FROM + vulnerabilities + WHERE + vulnerabilities.id = NEW.vulnerability_id; + + IF present_on_default_branch IS NOT true THEN + RETURN NULL; + END IF; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.vulnerability_id) + INTO + has_issues; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_merge_request_links WHERE vulnerability_merge_request_links.vulnerability_id = NEW.vulnerability_id) + INTO + has_merge_request; + + INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request) + VALUES (NEW.vulnerability_id, NEW.project_id, NEW.scanner_id, report_type, severity, state, resolved_on_default_branch, NEW.uuid::uuid, NEW.location->>'image', NEW.location->'kubernetes_resource'->>'agent_id', CAST(NEW.location->'kubernetes_resource'->>'agent_id' AS bigint), has_issues, has_merge_request) + ON CONFLICT(vulnerability_id) DO NOTHING; + RETURN NULL; + END + $$; + SQL + end +end diff --git a/db/schema_migrations/20250605145847 b/db/schema_migrations/20250605145847 new file mode 100644 index 00000000000000..b0d3a04f18a295 --- /dev/null +++ b/db/schema_migrations/20250605145847 @@ -0,0 +1 @@ +f358b1de2f5057493523354b372564108a84f861855af36034caedfb9f886616 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index be5d6e2182c980..c1444577b7ec82 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -628,6 +628,10 @@ DECLARE has_issues boolean; has_merge_request boolean; BEGIN + IF (SELECT current_setting('vulnerability_management_dont_execute_db_trigger', true)::boolean) THEN + RETURN NULL; + END IF; + IF (NEW.vulnerability_id IS NULL AND (TG_OP = 'INSERT' OR TG_OP = 'UPDATE')) THEN RETURN NULL; END IF; -- GitLab From 7a712eae514374e9d7694af2b4683d89493f3b2b Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Fri, 6 Jun 2025 17:28:52 -0400 Subject: [PATCH 04/20] Turn off SQL trigger function --- .../services/security/ingestion/ingest_slice_base_service.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ee/app/services/security/ingestion/ingest_slice_base_service.rb b/ee/app/services/security/ingestion/ingest_slice_base_service.rb index bc9739cf9c4a6f..fbd3ff303fe03a 100644 --- a/ee/app/services/security/ingestion/ingest_slice_base_service.rb +++ b/ee/app/services/security/ingestion/ingest_slice_base_service.rb @@ -30,6 +30,10 @@ def execute def run_tasks_in_sec_db ::SecApplicationRecord.transaction do + ::SecApplicationRecord.connection.execute( + "select set_config('vulnerability_management_dont_execute_db_trigger',false,true);" + ) + self.class::SEC_DB_TASKS.each { |task| execute_task(task) } end end -- GitLab From 013b1f34e7eb150226b7d06b1b04ddbf7f5cdb2f Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Fri, 6 Jun 2025 17:35:43 -0400 Subject: [PATCH 05/20] Configure trigger to be turned on and off with feature flag --- .../security/ingestion/ingest_slice_base_service.rb | 7 ++++--- ...f_vulnerability_read_create_db_trigger_function.yml | 10 ++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml diff --git a/ee/app/services/security/ingestion/ingest_slice_base_service.rb b/ee/app/services/security/ingestion/ingest_slice_base_service.rb index fbd3ff303fe03a..633241ee7db45b 100644 --- a/ee/app/services/security/ingestion/ingest_slice_base_service.rb +++ b/ee/app/services/security/ingestion/ingest_slice_base_service.rb @@ -30,9 +30,10 @@ def execute def run_tasks_in_sec_db ::SecApplicationRecord.transaction do - ::SecApplicationRecord.connection.execute( - "select set_config('vulnerability_management_dont_execute_db_trigger',false,true);" - ) + feature_enabled = Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + ::SecApplicationRecord.connection.execute("SELECT set_config( + 'vulnerability.management.dont_execute_db_trigger' + ,'#{feature_enabled}', true);") self.class::SEC_DB_TASKS.each { |task| execute_task(task) } end diff --git a/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml b/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml new file mode 100644 index 00000000000000..d144e277cc5c51 --- /dev/null +++ b/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml @@ -0,0 +1,10 @@ +--- +name: turn_off_vulnerability_read_create_db_trigger_function +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193387 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193387 +rollout_issue_url: +milestone: '18.1' +group: group::security infrastructure +type: gitlab_com_derisk +default_enabled: false -- GitLab From 88d1192896dd6171f51ece50c074ae97fa6cb16e Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 11 Jun 2025 01:50:03 -0400 Subject: [PATCH 06/20] Add spec for create task --- .../ingest_vulnerability_reads/create_spec.rb | 323 ++++++++++++++++++ 1 file changed, 323 insertions(+) create mode 100644 ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb new file mode 100644 index 00000000000000..04c1ef59e7a6fb --- /dev/null +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb @@ -0,0 +1,323 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::Ingestion::Tasks::IngestVulnerabilityReads::Create, feature_category: :vulnerability_management do + let_it_be(:user) { create(:user) } + let_it_be_with_refind(:project) { create(:project) } + let_it_be_with_refind(: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_with_refind(:vulnerability) { create_vulnerability } + + let(:external_type) { 'cve' } + let(:external_id) { 'CVE-2023-XXXX' } + let(:identifier_name) { external_id } + + let(:ci_reports_security_identifier) do + create(:ci_reports_security_identifier, external_id: external_id, + external_type: external_type, name: identifier_name) + end + + let(:identifier) do + create(:vulnerabilities_identifier, external_id: external_id, external_type: external_type, + name: identifier_name, fingerprint: ci_reports_security_identifier.fingerprint) + end + + let(:ci_reports_security_finding) do + create(:ci_reports_security_finding, identifiers: [ci_reports_security_identifier], + location: location, scanner: scanner) + end + + let_it_be(:vulnerability_scanner) { create(:vulnerabilities_scanner, external_id: 'scanner', name: 'Scanner') } + let!(:vulnerability_finding) do + create( + :vulnerabilities_finding, + vulnerability: vulnerability, + project: pipeline.project, + primary_identifier: identifier, + identifiers: [identifier], + location_fingerprint: location.fingerprint, + scanner: vulnerability_scanner + ) + end + + let(:security_finding) { create(:security_finding, scanner: vulnerability_scanner) } + let!(:finding_map) do + create(:finding_map, + pipeline: pipeline, + security_finding: security_finding, + report_finding: ci_reports_security_finding) + end + + let(:create_service) { described_class.new(pipeline, [finding_map]) } + + before do + finding_map.vulnerability_id = vulnerability.id + vulnerability.vulnerability_read&.destroy! + end + + subject(:execute) { create_service.execute } + + describe 'creating vulnerability_read record' do + it 'creates a vulnerability_read record' do + expect { execute }.to change { Vulnerabilities::Read.count }.by(1) + end + + it 'associates the vulnerability_read with the vulnerability' do + execute + + expect(vulnerability.reload.vulnerability_read).to be_present + end + + it 'sets the correct attributes on the vulnerability_read' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read).to have_attributes( + vulnerability_id: vulnerability.id, + project_id: project.id, + scanner: vulnerability_scanner, + report_type: vulnerability.report_type, + severity: vulnerability.severity, + state: vulnerability.state, + traversal_ids: project.namespace.traversal_ids, + archived: project.archived, + identifier_names: [identifier_name] + ) + end + end + + describe 'owasp_top_10 assignment' do + let(:external_type) { 'owasp' } + let(:external_id) { 'A1:2017-Injection' } + + shared_examples 'sets owasp_top_10 correctly' do |expected_value| + it 'sets owasp_top_10 on vulnerability read' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.owasp_top_10).to eq(expected_value) + end + end + + shared_examples 'sets owasp_top_10 to undefined' do + it 'sets owasp_top_10 to undefined' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.owasp_top_10).to eq('undefined') + end + end + + context 'with valid owasp identifier' do + context 'for 2017 external_id without name' do + let(:external_id) { 'A1:2017' } + + it_behaves_like 'sets owasp_top_10 correctly', 'A1:2017-Injection' + end + + context 'for 2021 external_id without name' do + let(:external_id) { 'A3:2021' } + + it_behaves_like 'sets owasp_top_10 correctly', 'A3:2021-Injection' + end + + context 'for 2017 external_id with name' do + it_behaves_like 'sets owasp_top_10 correctly', 'A1:2017-Injection' + end + + context 'for 2021 external_id with name' do + let(:external_id) { 'A3:2021-Injection' } + + it_behaves_like 'sets owasp_top_10 correctly', 'A3:2021-Injection' + end + + context 'when primary identifier is not owasp identifier' do + let(:external_type) { 'cve' } + let(:external_id) { 'CVE-2023-XXXX' } + let(:owasp_external_id) { 'A1:2017-Injection' } + + let(:ci_reports_security_owasp_identifier) do + create(:ci_reports_security_identifier, external_id: owasp_external_id, external_type: 'owasp') + end + + let(:ci_reports_security_finding) do + create(:ci_reports_security_finding, identifiers: + [ci_reports_security_identifier, ci_reports_security_owasp_identifier], + location: location, scanner: scanner) + end + + let(:owasp_identifier) do + create(:vulnerabilities_identifier, external_id: owasp_external_id, external_type: 'owasp', + fingerprint: ci_reports_security_owasp_identifier.fingerprint) + end + + let!(:vulnerability_finding) do + create(:vulnerabilities_finding, vulnerability: vulnerability, project: pipeline.project, + primary_identifier: identifier, + identifiers: [identifier, owasp_identifier], + location_fingerprint: location.fingerprint, + scanner: vulnerability_scanner) + end + + it_behaves_like 'sets owasp_top_10 correctly', 'A1:2017-Injection' + end + end + + context 'with invalid owasp identifier' do + context 'for invalid priority label' do + let(:external_id) { 'A1' } + + it_behaves_like 'sets owasp_top_10 to undefined' + end + + context 'for invalid year' do + let(:external_id) { 'A1:2010' } + + it_behaves_like 'sets owasp_top_10 to undefined' + end + + context 'for invalid external_id with correct label year and incorrect name' do + let(:external_id) { 'A1:2021-Injection' } + + it_behaves_like 'sets owasp_top_10 to undefined' + end + end + + context 'with non owasp identifier' do + let(:external_type) { 'cve' } + let(:external_id) { 'CVE-2023-XXXX' } + + it_behaves_like 'sets owasp_top_10 to undefined' + end + end + + describe 'has_vulnerability_resolution assignment' do + context 'when the report type is not supported' do + let(:external_type) { 'cwe' } + let(:external_id) { '23' } + let(:ci_reports_security_finding) do + create( + :ci_reports_security_finding, + report_type: :dast, + identifiers: [ci_reports_security_identifier], + location: location, + scanner: scanner) + end + + it 'sets has_vulnerability_resolution to false' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.has_vulnerability_resolution).to be false + end + end + + context 'when the CWE is not supported' do + let(:external_type) { 'cwe' } + let(:external_id) { 'unsupported' } + + it 'sets has_vulnerability_resolution to false' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.has_vulnerability_resolution).to be false + end + end + + context 'when the report is sast and a supported CWE' do + let(:external_type) { 'cwe' } + let(:external_id) { '23' } + let(:identifier_name) { 'CWE-23' } + + it 'sets has_vulnerability_resolution to true' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.has_vulnerability_resolution).to be true + end + end + end + + describe 'identifier_names assignment' do + let(:external_type) { 'cwe' } + let(:external_id) { '1035' } + let(:identifier_name) { 'CWE-1035' } + + it 'sets identifier_names from the identifiers' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.identifier_names).to eq([identifier_name]) + end + + context 'with multiple identifiers' do + let(:external_id_b) { '937' } + let(:external_type_b) { 'cwe' } + let(:identifier_name_b) { 'CWE-937' } + + let(:ci_reports_security_identifier_b) do + create(:ci_reports_security_identifier, external_id: external_id_b, + external_type: external_type_b, name: identifier_name_b) + end + + let(:ci_reports_security_finding) do + create(:ci_reports_security_finding, + identifiers: [ci_reports_security_identifier, ci_reports_security_identifier_b], + location: location, scanner: scanner) + end + + let(:identifier_b) do + create(:vulnerabilities_identifier, external_id: external_id_b, external_type: external_type_b, + name: identifier_name_b, fingerprint: ci_reports_security_identifier_b.fingerprint) + end + + let!(:vulnerability_finding) do + create( + :vulnerabilities_finding, + vulnerability: vulnerability, + project: pipeline.project, + primary_identifier: identifier, + identifiers: [identifier, identifier_b], + location_fingerprint: location.fingerprint, + scanner: vulnerability_scanner + ) + end + + it 'sets identifier_names for all identifiers' do + execute + vulnerability_read = vulnerability.reload.vulnerability_read + + expect(vulnerability_read.identifier_names).to match_array([identifier_name, identifier_name_b]) + end + end + end + + describe 'when vulnerability_read already exists' do + let!(:existing_vulnerability_read) do + create(:vulnerability_read, vulnerability: vulnerability, project: project) + end + + it 'does not create a new vulnerability_read record' do + expect { execute }.not_to change { Vulnerabilities::Read.count } + end + + it 'does not modify the existing vulnerability_read' do + original_attributes = existing_vulnerability_read.attributes + execute + + expect(existing_vulnerability_read.reload.attributes).to eq(original_attributes) + end + end + + private + + def create_vulnerability(severity: 7, report_type: 0) + create(:vulnerability, + project: pipeline.project, + author: user, + severity: severity, + report_type: report_type) + end +end -- GitLab From e0f4589c0353791bd95f5cafa722edb6ed148abb Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 11 Jun 2025 01:50:47 -0400 Subject: [PATCH 07/20] Fix set_config parameter name error and comparison error --- ...847_update_insert_or_update_vulnerability_reads_function.rb | 2 +- db/structure.sql | 2 +- .../services/security/ingestion/ingest_slice_base_service.rb | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb b/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb index 030ce72e208b49..5f373d79c50555 100644 --- a/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb +++ b/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb @@ -17,7 +17,7 @@ def up has_issues boolean; has_merge_request boolean; BEGIN - IF (SELECT current_setting('vulnerability_management_dont_execute_db_trigger', true)::boolean) THEN + IF (SELECT current_setting('vulnerability_management.dont_execute_db_trigger', true) = 'true') THEN RETURN NULL; END IF; diff --git a/db/structure.sql b/db/structure.sql index c1444577b7ec82..9b7ae1d731843f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -628,7 +628,7 @@ DECLARE has_issues boolean; has_merge_request boolean; BEGIN - IF (SELECT current_setting('vulnerability_management_dont_execute_db_trigger', true)::boolean) THEN + IF (SELECT current_setting('vulnerability_management.dont_execute_db_trigger', true) = 'true') THEN RETURN NULL; END IF; diff --git a/ee/app/services/security/ingestion/ingest_slice_base_service.rb b/ee/app/services/security/ingestion/ingest_slice_base_service.rb index 633241ee7db45b..011e7439070bee 100644 --- a/ee/app/services/security/ingestion/ingest_slice_base_service.rb +++ b/ee/app/services/security/ingestion/ingest_slice_base_service.rb @@ -32,8 +32,7 @@ def run_tasks_in_sec_db ::SecApplicationRecord.transaction do feature_enabled = Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) ::SecApplicationRecord.connection.execute("SELECT set_config( - 'vulnerability.management.dont_execute_db_trigger' - ,'#{feature_enabled}', true);") + 'vulnerability_management.dont_execute_db_trigger', '#{feature_enabled}', true);") self.class::SEC_DB_TASKS.each { |task| execute_task(task) } end -- GitLab From faaae4b788af4c21c84d21893d88265659196f77 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 11 Jun 2025 01:55:44 -0400 Subject: [PATCH 08/20] Better handle getting location data Can directly get agent_id, image, etc. as well as from hashes and can handle setting it to nil. --- .../ingest_vulnerability_reads/create.rb | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb index dbe0d27373ee06..f7b73a41780797 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -21,8 +21,8 @@ def attributes next unless vulnerability occurrence = finding_map.report_finding - location = occurrence.location || {} - agent_id = location.dig('kubernetes_resource', 'agent_id') + location = occurrence.location + location_data = extract_location_data(location) { vulnerability_id: vulnerability_id, @@ -33,9 +33,9 @@ def attributes state: vulnerability.state, resolved_on_default_branch: vulnerability.resolved_on_default_branch, uuid: occurrence.uuid, - location_image: location['image'], - cluster_agent_id: agent_id, - casted_cluster_agent_id: agent_id&.to_i, + location_image: location_data[:image], + cluster_agent_id: location_data[:agent_id], + casted_cluster_agent_id: location_data[:agent_id]&.to_i, has_issues: issue_links.include?(vulnerability_id), has_merge_request: merge_request_links.include?(vulnerability_id) } @@ -57,6 +57,23 @@ def merge_request_links def vulnerability_ids @vulnerability_ids ||= finding_maps.filter_map(&:vulnerability_id) end + + def extract_location_data(location) + case location + when Hash + { + image: location['image'], + agent_id: location.dig('kubernetes_resource', 'agent_id'), + cluster_id: location.dig('kubernetes_resource', 'cluster_id') + } + else + { + image: location.respond_to?(:image) ? location.image : nil, + agent_id: location.respond_to?(:agent_id) ? location.agent_id : nil, + cluster_id: location.respond_to?(:cluster_id) ? location.cluster_id : nil + } + end + end end end end -- GitLab From 76249a1b2041d0a52b58c8174baf769a99b17e61 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 11 Jun 2025 21:49:13 -0400 Subject: [PATCH 09/20] Update timestamp of migration --- ...4729_update_insert_or_update_vulnerability_reads_function.rb} | 0 db/schema_migrations/20250605145847 | 1 - db/schema_migrations/20250612014729 | 1 + 3 files changed, 1 insertion(+), 1 deletion(-) rename db/migrate/{20250605145847_update_insert_or_update_vulnerability_reads_function.rb => 20250612014729_update_insert_or_update_vulnerability_reads_function.rb} (100%) delete mode 100644 db/schema_migrations/20250605145847 create mode 100644 db/schema_migrations/20250612014729 diff --git a/db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb b/db/migrate/20250612014729_update_insert_or_update_vulnerability_reads_function.rb similarity index 100% rename from db/migrate/20250605145847_update_insert_or_update_vulnerability_reads_function.rb rename to db/migrate/20250612014729_update_insert_or_update_vulnerability_reads_function.rb diff --git a/db/schema_migrations/20250605145847 b/db/schema_migrations/20250605145847 deleted file mode 100644 index b0d3a04f18a295..00000000000000 --- a/db/schema_migrations/20250605145847 +++ /dev/null @@ -1 +0,0 @@ -f358b1de2f5057493523354b372564108a84f861855af36034caedfb9f886616 \ No newline at end of file diff --git a/db/schema_migrations/20250612014729 b/db/schema_migrations/20250612014729 new file mode 100644 index 00000000000000..3f5bea8a13994f --- /dev/null +++ b/db/schema_migrations/20250612014729 @@ -0,0 +1 @@ +849eb31cf28d40b5627ff29f29a6bd4c437a56b87ea4f5437efd321a4710d9de \ No newline at end of file -- GitLab From 41bf09a161ba490a0ccb881ffbcf0968b90476e4 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 11 Jun 2025 23:13:04 -0400 Subject: [PATCH 10/20] Fix check for existing issues and MR's --- .../tasks/ingest_vulnerability_reads/create.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb index f7b73a41780797..2531f74639d80a 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -36,8 +36,8 @@ def attributes location_image: location_data[:image], cluster_agent_id: location_data[:agent_id], casted_cluster_agent_id: location_data[:agent_id]&.to_i, - has_issues: issue_links.include?(vulnerability_id), - has_merge_request: merge_request_links.include?(vulnerability_id) + has_issues: vulnerability.issue_links.exists?, + has_merge_request: vulnerability.merge_request_links.exists? } end end @@ -46,14 +46,6 @@ def vulnerabilities @vulnerabilities ||= Vulnerability.by_ids(vulnerability_ids).index_by(&:id) end - def issue_links - @issue_links ||= Vulnerabilities::IssueLink.by_vulnerability_ids(vulnerability_ids).to_set - end - - def merge_request_links - @merge_request_links ||= Vulnerabilities::MergeRequestLink.by_vulnerability_ids(vulnerability_ids).to_set - end - def vulnerability_ids @vulnerability_ids ||= finding_maps.filter_map(&:vulnerability_id) end -- GitLab From af4d9ffed4863fd5981fb66c2083dbcffc9552cd Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 11 Jun 2025 23:46:12 -0400 Subject: [PATCH 11/20] Overrides the timestamps method used by bulk_insertable_task --- .../ingestion/tasks/ingest_vulnerability_reads/create.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb index 2531f74639d80a..64db43cea2562b 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -66,6 +66,10 @@ def extract_location_data(location) } end end + + def timestamps + {} + end end end end -- GitLab From 6c6b3a3fc9ce62895f515529cb26b71471492108 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Thu, 12 Jun 2025 00:17:16 -0400 Subject: [PATCH 12/20] Add additional attributes to creation --- .../ingest_vulnerability_reads/create.rb | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb index 64db43cea2562b..ebd968d5850363 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -37,7 +37,12 @@ def attributes cluster_agent_id: location_data[:agent_id], casted_cluster_agent_id: location_data[:agent_id]&.to_i, has_issues: vulnerability.issue_links.exists?, - has_merge_request: vulnerability.merge_request_links.exists? + has_merge_request: vulnerability.merge_request_links.exists?, + traversal_ids: vulnerability.project.namespace.traversal_ids, + archived: vulnerability.project.archived, + identifier_names: occurrence.identifiers.map(&:name), + owasp_top_10: extract_owasp_top_10(occurrence), + has_vulnerability_resolution: extract_vulnerability_resolution(occurrence) } end end @@ -67,6 +72,47 @@ def extract_location_data(location) end end + def extract_owasp_top_10(occurrence) + owasp_identifier = occurrence.identifiers.find { |id| id.external_type.casecmp?('owasp') } + return ::Vulnerabilities::Read::OWASP_TOP_10_DEFAULT unless owasp_identifier + + 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'. + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/423557#note_1539490082 + def map_owasp_external_id(external_id) + default_value = ::Vulnerabilities::Read::OWASP_TOP_10_DEFAULT + + return default_value unless valid_owasp_external_id?(external_id) + + ::Enums::Vulnerability.owasp_top_10.keys.find { |key| key.include?(external_id) } || + default_value + end + + def valid_owasp_external_id?(external_id) + arr = external_id.split(':') + + priority_label = arr.first + year = arr.second ? arr.second[0..3] : nil + + return false if year.nil? || ::Enums::Vulnerability.owasp_years.exclude?(year) + + Enums::Vulnerability.owasp_categories.include?(priority_label) + end + + def extract_vulnerability_resolution(occurrence) + report_type = occurrence.report_type.to_s + cwe_identifier = occurrence.identifiers.find { |id| id.external_type == 'cwe' } + return false unless cwe_identifier + + cwe_value = cwe_identifier.name + return false unless cwe_value + + ::Vulnerabilities::Finding::AI_ALLOWED_REPORT_TYPES.include?(report_type) && + ::Vulnerabilities::Finding::HIGH_CONFIDENCE_AI_RESOLUTION_CWES.include?(cwe_value&.upcase) + end + def timestamps {} end -- GitLab From 0d3439f72503d168b1f982b6a7c47f656b646f5c Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Sat, 14 Jun 2025 19:29:32 -0400 Subject: [PATCH 13/20] Fix incorrect setting of attributes to vulnerability_read --- .../ingestion/tasks/ingest_vulnerability_reads/create.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb index ebd968d5850363..1bb49dd6adb135 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb @@ -13,7 +13,7 @@ class Create < AbstractTask private def attributes - finding_maps.filter_map do |finding_map| + @attributes ||= finding_maps.filter_map do |finding_map| vulnerability_id = finding_map.vulnerability_id next unless vulnerability_id @@ -24,7 +24,7 @@ def attributes location = occurrence.location location_data = extract_location_data(location) - { + attributes = { vulnerability_id: vulnerability_id, project_id: vulnerability.project_id, scanner_id: finding_map.scanner_id, @@ -44,6 +44,8 @@ def attributes owasp_top_10: extract_owasp_top_10(occurrence), has_vulnerability_resolution: extract_vulnerability_resolution(occurrence) } + + attributes end end -- GitLab From 7a37718e4d65249b0f93dfa9a70918386b6c2cae Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Wed, 18 Jun 2025 10:10:19 -0400 Subject: [PATCH 14/20] Fix owasp overwriting --- ee/app/models/vulnerabilities/read.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/models/vulnerabilities/read.rb b/ee/app/models/vulnerabilities/read.rb index 682ce28f9848a8..06f49372e7619f 100644 --- a/ee/app/models/vulnerabilities/read.rb +++ b/ee/app/models/vulnerabilities/read.rb @@ -306,7 +306,7 @@ def database_serialized_traversal_ids end def set_default_values - self.owasp_top_10 = 'undefined' + self.owasp_top_10 ||= 'undefined' end end end -- GitLab From b87b4f8d4e9566a591275b8f94ff4d23cc5c04c6 Mon Sep 17 00:00:00 2001 From: Adrien Narinesingh Date: Thu, 19 Jun 2025 10:03:08 -0400 Subject: [PATCH 15/20] Rename task to upsert and replace update task Renamed the task from create to the more appropraite name upsert and added in conditional statements so that the old logic is used if the FF is off. Also did some minor formatting. --- .../tasks/ingest_vulnerability_reads.rb | 12 ++++++----- .../{create.rb => upsert.rb} | 4 ++-- .../{create_spec.rb => upsert_spec.rb} | 17 ++++++++-------- .../tasks/ingest_vulnerability_reads_spec.rb | 20 ++++++++++++++----- 4 files changed, 33 insertions(+), 20 deletions(-) rename ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/{create.rb => upsert.rb} (97%) rename ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/{create_spec.rb => upsert_spec.rb} (96%) diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb index 54b9b8a7e44648..4e12b1f00240bf 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb @@ -5,17 +5,19 @@ module Ingestion module Tasks class IngestVulnerabilityReads < AbstractTask def execute - create_vulnerability_reads - - update_vulnerability_reads + if Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + upsert_vulnerability_reads + else + update_vulnerability_reads + end finding_maps end private - def create_vulnerability_reads - IngestVulnerabilityReads::Create.new(pipeline, finding_maps).execute + def upsert_vulnerability_reads + IngestVulnerabilityReads::Upsert.new(pipeline, finding_maps).execute end def update_vulnerability_reads diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb similarity index 97% rename from ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb rename to ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb index 1bb49dd6adb135..2f016f2574676a 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/create.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb @@ -4,7 +4,7 @@ module Security module Ingestion module Tasks class IngestVulnerabilityReads - class Create < AbstractTask + class Upsert < AbstractTask include Gitlab::Ingestion::BulkInsertableTask self.model = Vulnerabilities::Read @@ -50,7 +50,7 @@ def attributes end def vulnerabilities - @vulnerabilities ||= Vulnerability.by_ids(vulnerability_ids).index_by(&:id) + @vulnerabilities ||= Vulnerability.by_ids(vulnerability_ids).includes(:merge_request_links, :issue_links).index_by(&:id) end def vulnerability_ids diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb similarity index 96% rename from ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb rename to ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb index 04c1ef59e7a6fb..e9442a1749ecca 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/create_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Security::Ingestion::Tasks::IngestVulnerabilityReads::Create, feature_category: :vulnerability_management do +RSpec.describe Security::Ingestion::Tasks::IngestVulnerabilityReads::Upsert, feature_category: :vulnerability_management do let_it_be(:user) { create(:user) } let_it_be_with_refind(:project) { create(:project) } let_it_be_with_refind(:pipeline) { create(:ci_pipeline, project: project) } @@ -299,15 +299,16 @@ create(:vulnerability_read, vulnerability: vulnerability, project: project) end - it 'does not create a new vulnerability_read record' do - expect { execute }.not_to change { Vulnerabilities::Read.count } - end - - it 'does not modify the existing vulnerability_read' do - original_attributes = existing_vulnerability_read.attributes + it 'updates the existing vulnerability_read' do + expected_attributes = existing_vulnerability_read.attributes.merge({ + 'identifier_names' => [external_id], + 'scanner_id' => vulnerability_scanner.id, + 'severity' => vulnerability.severity, + 'uuid' => ci_reports_security_finding.uuid + }) execute - expect(existing_vulnerability_read.reload.attributes).to eq(original_attributes) + expect(existing_vulnerability_read.reload.attributes).to eq(expected_attributes) end end diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb index 21d268b40b22d3..b554b054a286d9 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb @@ -20,19 +20,29 @@ let_it_be(:finding_maps) { [create(:finding_map, report_finding: ci_reports_security_finding)] } let!(:update_service_double) do - instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update, - execute: true) + if Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Upsert, + execute: true) + else + instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update, + execute: true) + end end let_it_be(:ingest_service) { described_class.new(pipeline, finding_maps) } describe '#execute' do before do - allow(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update) - .to receive(:new).with(pipeline, finding_maps).and_return(update_service_double) + if Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + allow(Security::Ingestion::Tasks::IngestVulnerabilityReads::Upsert) + .to receive(:new).with(pipeline, finding_maps).and_return(update_service_double) + else + allow(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update) + .to receive(:new).with(pipeline, finding_maps).and_return(update_service_double) + end end - it 'calls the update service' do + it 'calls the upsert service' do expect(update_service_double).to receive(:execute) ingest_service.execute -- GitLab From 2ad2ceb9d9a3a3a056c75f5f1f6a559002b1a57c Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Fri, 20 Jun 2025 10:33:24 -0400 Subject: [PATCH 16/20] Update and restructure ingest spec --- .../tasks/ingest_vulnerability_reads_spec.rb | 52 +++++++++++-------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb index b554b054a286d9..33e02ce4d079b8 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads_spec.rb @@ -18,38 +18,48 @@ end let_it_be(:finding_maps) { [create(:finding_map, report_finding: ci_reports_security_finding)] } + let_it_be(:ingest_service) { described_class.new(pipeline, finding_maps) } + + describe '#execute' do + shared_examples 'executes service and returns finding maps' do + it 'calls the upsert service' do + expect(update_service_double).to receive(:execute) + + ingest_service.execute + end - let!(:update_service_double) do - if Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) - instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Upsert, - execute: true) - else - instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update, - execute: true) + it 'returns finding maps' do + expect(ingest_service.execute).to eq(finding_maps) + end end - end - let_it_be(:ingest_service) { described_class.new(pipeline, finding_maps) } + context 'with feature flag enabled' do + let!(:update_service_double) do + instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Upsert, + execute: true) + end - describe '#execute' do - before do - if Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + before do allow(Security::Ingestion::Tasks::IngestVulnerabilityReads::Upsert) .to receive(:new).with(pipeline, finding_maps).and_return(update_service_double) - else - allow(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update) - .to receive(:new).with(pipeline, finding_maps).and_return(update_service_double) end + + include_examples 'executes service and returns finding maps' end - it 'calls the upsert service' do - expect(update_service_double).to receive(:execute) + context 'with feature flag disabled' do + let!(:update_service_double) do + instance_double(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update, + execute: true) + end - ingest_service.execute - end + before do + stub_feature_flags(turn_off_vulnerability_read_create_db_trigger_function: false) + allow(Security::Ingestion::Tasks::IngestVulnerabilityReads::Update) + .to receive(:new).with(pipeline, finding_maps).and_return(update_service_double) + end - it 'returns finding maps' do - expect(ingest_service.execute).to eq(finding_maps) + include_examples 'executes service and returns finding maps' end end end -- GitLab From 49ab65a6472d7fb6d06594a6dd1e2dd482e1a4fe Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Mon, 30 Jun 2025 03:09:32 -0400 Subject: [PATCH 17/20] Simplify and clean up upsert task, add n+1 spec Add in scopes for preloadings, N+1 testing, remove & refactor unneccesary logic, modify logic for accessing attributes. --- ee/app/models/ee/vulnerability.rb | 2 + .../ingestion/ingest_slice_base_service.rb | 6 +- .../tasks/ingest_vulnerability_reads.rb | 12 ++- .../ingest_vulnerability_reads/upsert.rb | 89 +++---------------- .../ingest_vulnerability_reads/upsert_spec.rb | 32 +++++++ lib/gitlab/ci/reports/security/finding.rb | 50 +++++++++++ 6 files changed, 112 insertions(+), 79 deletions(-) diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index d70bba74d39eaf..f1cdd54136764d 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -191,6 +191,8 @@ module Vulnerability scope :last_updated_before, ->(date) { where(updated_at: ...date) } scope :with_archival_related_entities, -> { includes(:vulnerability_read, :notes, findings: [:identifiers, :scanner]) } scope :with_mrs_and_issues, -> { includes(:merge_requests, :related_issues) } + scope :with_mrs_and_issue_links, -> { includes(:merge_request_links, :issue_links, project: :namespace) } + scope :with_vulnerability_occurrences, -> { includes(:findings) } delegate :scanner_name, :scanner_external_id, :scanner_id, :metadata, :description, :description_html, :details, :uuid, to: :finding, prefix: true, allow_nil: true diff --git a/ee/app/services/security/ingestion/ingest_slice_base_service.rb b/ee/app/services/security/ingestion/ingest_slice_base_service.rb index 011e7439070bee..20eed23ef84919 100644 --- a/ee/app/services/security/ingestion/ingest_slice_base_service.rb +++ b/ee/app/services/security/ingestion/ingest_slice_base_service.rb @@ -30,7 +30,11 @@ def execute def run_tasks_in_sec_db ::SecApplicationRecord.transaction do - feature_enabled = Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + project = pipeline&.project + + feature_enabled = Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, + project || :instance) + ::SecApplicationRecord.connection.execute("SELECT set_config( 'vulnerability_management.dont_execute_db_trigger', '#{feature_enabled}', true);") diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb index 4e12b1f00240bf..91040075e5dd66 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads.rb @@ -5,7 +5,9 @@ module Ingestion module Tasks class IngestVulnerabilityReads < AbstractTask def execute - if Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, pipeline.project) + project = pipeline&.project + + if should_use_upsert?(project) upsert_vulnerability_reads else update_vulnerability_reads @@ -23,6 +25,14 @@ def upsert_vulnerability_reads def update_vulnerability_reads IngestVulnerabilityReads::Update.new(pipeline, finding_maps).execute end + + def should_use_upsert?(project) + if project + Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, project) + else + Feature.enabled?(:turn_off_vulnerability_read_create_db_trigger_function, :instance) + end + end end end end diff --git a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb index 2f016f2574676a..5c31136475cab1 100644 --- a/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb +++ b/ee/app/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert.rb @@ -14,17 +14,10 @@ class Upsert < AbstractTask def attributes @attributes ||= finding_maps.filter_map do |finding_map| + occurrence = finding_map.report_finding vulnerability_id = finding_map.vulnerability_id - next unless vulnerability_id - vulnerability = vulnerabilities[vulnerability_id] - next unless vulnerability - - occurrence = finding_map.report_finding - location = occurrence.location - location_data = extract_location_data(location) - - attributes = { + { vulnerability_id: vulnerability_id, project_id: vulnerability.project_id, scanner_id: finding_map.scanner_id, @@ -33,88 +26,30 @@ def attributes state: vulnerability.state, resolved_on_default_branch: vulnerability.resolved_on_default_branch, uuid: occurrence.uuid, - location_image: location_data[:image], - cluster_agent_id: location_data[:agent_id], - casted_cluster_agent_id: location_data[:agent_id]&.to_i, - has_issues: vulnerability.issue_links.exists?, - has_merge_request: vulnerability.merge_request_links.exists?, + location_image: vulnerability.location&.dig('image'), + cluster_agent_id: vulnerability.location&.dig('kubernetes_resource', 'agent_id'), + casted_cluster_agent_id: vulnerability.location&.dig('kubernetes_resource', 'agent_id')&.to_i, + has_issues: vulnerability.issue_links.any?, + has_merge_request: vulnerability.merge_request_links.any?, traversal_ids: vulnerability.project.namespace.traversal_ids, archived: vulnerability.project.archived, identifier_names: occurrence.identifiers.map(&:name), - owasp_top_10: extract_owasp_top_10(occurrence), - has_vulnerability_resolution: extract_vulnerability_resolution(occurrence) + owasp_top_10: occurrence.owasp_top_10, + has_vulnerability_resolution: occurrence.has_vulnerability_resolution? } - - attributes end end def vulnerabilities - @vulnerabilities ||= Vulnerability.by_ids(vulnerability_ids).includes(:merge_request_links, :issue_links).index_by(&:id) + @vulnerabilities ||= Vulnerability.by_ids(vulnerability_ids).with_mrs_and_issue_links + .with_vulnerability_occurrences + .index_by(&:id) end def vulnerability_ids @vulnerability_ids ||= finding_maps.filter_map(&:vulnerability_id) end - def extract_location_data(location) - case location - when Hash - { - image: location['image'], - agent_id: location.dig('kubernetes_resource', 'agent_id'), - cluster_id: location.dig('kubernetes_resource', 'cluster_id') - } - else - { - image: location.respond_to?(:image) ? location.image : nil, - agent_id: location.respond_to?(:agent_id) ? location.agent_id : nil, - cluster_id: location.respond_to?(:cluster_id) ? location.cluster_id : nil - } - end - end - - def extract_owasp_top_10(occurrence) - owasp_identifier = occurrence.identifiers.find { |id| id.external_type.casecmp?('owasp') } - return ::Vulnerabilities::Read::OWASP_TOP_10_DEFAULT unless owasp_identifier - - 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'. - # See: https://gitlab.com/gitlab-org/gitlab/-/issues/423557#note_1539490082 - def map_owasp_external_id(external_id) - default_value = ::Vulnerabilities::Read::OWASP_TOP_10_DEFAULT - - return default_value unless valid_owasp_external_id?(external_id) - - ::Enums::Vulnerability.owasp_top_10.keys.find { |key| key.include?(external_id) } || - default_value - end - - def valid_owasp_external_id?(external_id) - arr = external_id.split(':') - - priority_label = arr.first - year = arr.second ? arr.second[0..3] : nil - - return false if year.nil? || ::Enums::Vulnerability.owasp_years.exclude?(year) - - Enums::Vulnerability.owasp_categories.include?(priority_label) - end - - def extract_vulnerability_resolution(occurrence) - report_type = occurrence.report_type.to_s - cwe_identifier = occurrence.identifiers.find { |id| id.external_type == 'cwe' } - return false unless cwe_identifier - - cwe_value = cwe_identifier.name - return false unless cwe_value - - ::Vulnerabilities::Finding::AI_ALLOWED_REPORT_TYPES.include?(report_type) && - ::Vulnerabilities::Finding::HIGH_CONFIDENCE_AI_RESOLUTION_CWES.include?(cwe_value&.upcase) - end - def timestamps {} end diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb index e9442a1749ecca..17fdb17c7b4e55 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerability_reads/upsert_spec.rb @@ -312,6 +312,38 @@ end end + describe 'N+1 queries' do + it 'does not cause N+1 queries when processing multiple vulnerabilities' do + single_vulnerability = create(:vulnerability, project: project, author: user) + single_finding_map = create(:finding_map, pipeline: pipeline, security_finding: security_finding, + report_finding: ci_reports_security_finding) + single_finding_map.vulnerability_id = single_vulnerability.id + single_service = described_class.new(pipeline, [single_finding_map]) + + control = ActiveRecord::QueryRecorder.new { single_service.execute } + + Vulnerabilities::Read.delete_all + + additional_vulnerabilities = create_list(:vulnerability, 2, project: project, author: user) + additional_finding_maps = additional_vulnerabilities.map do |vulnerability| + unique_report_finding = create(:ci_reports_security_finding, + identifiers: [ci_reports_security_identifier], + location: location, + scanner: scanner) + + map = create(:finding_map, pipeline: pipeline, security_finding: security_finding, + report_finding: unique_report_finding) + map.vulnerability_id = vulnerability.id + map + end + + all_finding_maps = [single_finding_map] + additional_finding_maps + multi_service = described_class.new(pipeline, all_finding_maps) + + expect { multi_service.execute }.not_to exceed_query_limit(control) + end + end + private def create_vulnerability(severity: 7, report_type: 0) diff --git a/lib/gitlab/ci/reports/security/finding.rb b/lib/gitlab/ci/reports/security/finding.rb index 43918773914cab..9ffe92c9e130c1 100644 --- a/lib/gitlab/ci/reports/security/finding.rb +++ b/lib/gitlab/ci/reports/security/finding.rb @@ -6,6 +6,7 @@ module Reports module Security class Finding include ::VulnerabilityFindingHelpers + include Gitlab::Utils::StrongMemoize attr_reader :confidence attr_reader :identifiers @@ -187,6 +188,16 @@ def location_fingerprint location_fingerprints.first end + def owasp_top_10 + extract_owasp_top_10 + end + strong_memoize_attr :owasp_top_10 + + def has_vulnerability_resolution? + extract_vulnerability_resolution + end + strong_memoize_attr :has_vulnerability_resolution? + private def location_fingerprints @@ -199,6 +210,45 @@ def signature_hexes signatures.sort_by { |sig| -sig.priority }.map(&:signature_hex) end + + def extract_owasp_top_10 + owasp_identifier = identifiers.find { |id| id.external_type.casecmp?('owasp') } + return ::Vulnerabilities::Read::OWASP_TOP_10_DEFAULT unless owasp_identifier + + map_owasp_external_id(owasp_identifier.external_id) + end + + def map_owasp_external_id(external_id) + default_value = ::Vulnerabilities::Read::OWASP_TOP_10_DEFAULT + + return default_value unless valid_owasp_external_id?(external_id) + + ::Enums::Vulnerability.owasp_top_10.keys.find { |key| key.include?(external_id) } || + default_value + end + + def valid_owasp_external_id?(external_id) + arr = external_id.split(':') + + priority_label = arr.first + year = arr.second ? arr.second[0..3] : nil + + return false if year.nil? || ::Enums::Vulnerability.owasp_years.exclude?(year) + + Enums::Vulnerability.owasp_categories.include?(priority_label) + end + + def extract_vulnerability_resolution + report_type_str = report_type.to_s + cwe_identifier = identifiers.find { |id| id.external_type == 'cwe' } + return false unless cwe_identifier + + cwe_value = cwe_identifier.name + return false unless cwe_value + + ::Vulnerabilities::Finding::AI_ALLOWED_REPORT_TYPES.include?(report_type_str) && + ::Vulnerabilities::Finding::HIGH_CONFIDENCE_AI_RESOLUTION_CWES.include?(cwe_value&.upcase) + end end end end -- GitLab From c87755eb5cd3d4845d9a05d2c5601587aa03766c Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Tue, 1 Jul 2025 01:31:31 -0400 Subject: [PATCH 18/20] Added in db trigger testing for FF functionality --- ee/spec/models/vulnerabilities/read_spec.rb | 73 +++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/ee/spec/models/vulnerabilities/read_spec.rb b/ee/spec/models/vulnerabilities/read_spec.rb index 3c37285982d9ab..e222e54fc4f259 100644 --- a/ee/spec/models/vulnerabilities/read_spec.rb +++ b/ee/spec/models/vulnerabilities/read_spec.rb @@ -1116,6 +1116,79 @@ end end + describe 'skip insert_vulnerability_reads_from_vulnerability() with feature flag' do + let(:user) { create(:user) } + let(:identifier) { create(:vulnerabilities_identifier, project: project) } + let(:scanner) { create(:vulnerabilities_scanner, project: project) } + let(:vulnerability) { create(:vulnerability, project: project, present_on_default_branch: false) } + let!(:finding) { create_finding(vulnerability: vulnerability) } + + context 'when vulnerability_management.dont_execute_db_trigger is true' do + before do + ::SecApplicationRecord.connection.execute( + "SELECT set_config('vulnerability_management.dont_execute_db_trigger', 'true', true);" + ) + end + + it 'database trigger returns NULL and does not create vulnerability_reads record' do + expect do + # This will hit trigger_insert_vulnerability_reads_from_vulnerability + vulnerability.update!(present_on_default_branch: true) + end.not_to change { Vulnerabilities::Read.count } + end + end + + context 'when vulnerability_management.dont_execute_db_trigger is false or unset' do + before do + ::SecApplicationRecord.connection.execute( + "SELECT set_config('vulnerability_management.dont_execute_db_trigger', 'false', true);" + ) + end + + it 'database trigger proceeds normally and creates vulnerability_reads record' do + expect do + vulnerability.update!(present_on_default_branch: true) + end.to change { Vulnerabilities::Read.count }.by(1) + end + end + end + + describe 'skip insert_or_update_vulnerability_reads() with feature flag' do + let(:user) { create(:user) } + let(:identifier) { create(:vulnerabilities_identifier, project: project) } + let(:scanner) { create(:vulnerabilities_scanner, project: project) } + let(:vulnerability) { create(:vulnerability, project: project, present_on_default_branch: true) } + + context 'when vulnerability_management.dont_execute_db_trigger is true' do + before do + ::SecApplicationRecord.connection.execute( + "SELECT set_config('vulnerability_management.dont_execute_db_trigger', 'true', true);" + ) + end + + it 'database trigger returns NULL and does not create vulnerability_reads record' do + expect do + # This will hit trigger_insert_or_update_vulnerability_reads_from_occurrences + create_finding(vulnerability: vulnerability) + end.not_to change { Vulnerabilities::Read.count } + end + end + + context 'when vulnerability_management.dont_execute_db_trigger is false or unset' do + before do + ::SecApplicationRecord.connection.execute( + "SELECT set_config('vulnerability_management.dont_execute_db_trigger', 'false', true);" + ) + end + + it 'database trigger proceeds normally and creates vulnerability_reads record' do + expect do + create_finding(vulnerability: vulnerability) + end.to change { Vulnerabilities::Read.count }.by(1) + end + end + end + private def create_vulnerability(severity: 7, report_type: 0) -- GitLab From 1d1370097141e4c931d689784c065986bbce9f73 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Mon, 7 Jul 2025 04:29:52 -0400 Subject: [PATCH 19/20] Update feature flag .yml and fix db structure --- ..._off_vulnerability_read_create_db_trigger_function.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml b/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml index d144e277cc5c51..244db0b82cfc0d 100644 --- a/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml +++ b/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml @@ -1,10 +1,10 @@ --- name: turn_off_vulnerability_read_create_db_trigger_function -description: -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193387 +description: Bypasses the trigger functions in the structure.sql that insert/update vulnerability_reads records and enables the application code that does that job instead. +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/546223 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193387 -rollout_issue_url: -milestone: '18.1' +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/553939 +milestone: '18.2' group: group::security infrastructure type: gitlab_com_derisk default_enabled: false -- GitLab From 9dd508788d4641116cd459c72f503ba901c4af17 Mon Sep 17 00:00:00 2001 From: anarinesingh Date: Tue, 15 Jul 2025 09:27:03 -0400 Subject: [PATCH 20/20] Update migration timestamps and logic Fixed stale timestamps for migrations and some logical issues. --- ...ate_insert_vulnerability_reads_function.rb | 95 +++++++++++++++++++ ...or_update_vulnerability_reads_function.rb} | 2 +- db/schema_migrations/20250612014729 | 1 - db/schema_migrations/20250715024615 | 1 + db/schema_migrations/20250715024650 | 1 + db/structure.sql | 4 + ...bility_read_create_db_trigger_function.yml | 2 +- 7 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20250715024615_update_insert_vulnerability_reads_function.rb rename db/migrate/{20250612014729_update_insert_or_update_vulnerability_reads_function.rb => 20250715024650_update_insert_or_update_vulnerability_reads_function.rb} (99%) delete mode 100644 db/schema_migrations/20250612014729 create mode 100644 db/schema_migrations/20250715024615 create mode 100644 db/schema_migrations/20250715024650 diff --git a/db/migrate/20250715024615_update_insert_vulnerability_reads_function.rb b/db/migrate/20250715024615_update_insert_vulnerability_reads_function.rb new file mode 100644 index 00000000000000..ee9df0a8435079 --- /dev/null +++ b/db/migrate/20250715024615_update_insert_vulnerability_reads_function.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +class UpdateInsertVulnerabilityReadsFunction < Gitlab::Database::Migration[2.3] + milestone '18.3' + + def up + execute <<~SQL + CREATE OR REPLACE FUNCTION insert_vulnerability_reads_from_vulnerability() RETURNS trigger + LANGUAGE plpgsql + AS $$ + DECLARE + scanner_id bigint; + uuid uuid; + location_image text; + cluster_agent_id text; + casted_cluster_agent_id bigint; + has_issues boolean; + has_merge_request boolean; + BEGIN + IF (SELECT current_setting('vulnerability_management.dont_execute_db_trigger', true) = 'true') THEN + RETURN NULL; + END IF; + + SELECT + v_o.scanner_id, v_o.uuid, v_o.location->>'image', v_o.location->'kubernetes_resource'->>'agent_id', CAST(v_o.location->'kubernetes_resource'->>'agent_id' AS bigint) + INTO + scanner_id, uuid, location_image, cluster_agent_id, casted_cluster_agent_id + FROM + vulnerability_occurrences v_o + WHERE + v_o.vulnerability_id = NEW.id + LIMIT 1; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.id) + INTO + has_issues; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_merge_request_links WHERE vulnerability_merge_request_links.vulnerability_id = NEW.id) + INTO + has_merge_request; + + INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request) + VALUES (NEW.id, NEW.project_id, scanner_id, NEW.report_type, NEW.severity, NEW.state, NEW.resolved_on_default_branch, uuid::uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request) + ON CONFLICT(vulnerability_id) DO NOTHING; + RETURN NULL; + END + $$; + SQL + end + + def down + execute <<~SQL + CREATE OR REPLACE FUNCTION insert_vulnerability_reads_from_vulnerability() RETURNS trigger + LANGUAGE plpgsql + AS $$ + DECLARE + scanner_id bigint; + uuid uuid; + location_image text; + cluster_agent_id text; + casted_cluster_agent_id bigint; + has_issues boolean; + has_merge_request boolean; + BEGIN + SELECT + v_o.scanner_id, v_o.uuid, v_o.location->>'image', v_o.location->'kubernetes_resource'->>'agent_id', CAST(v_o.location->'kubernetes_resource'->>'agent_id' AS bigint) + INTO + scanner_id, uuid, location_image, cluster_agent_id, casted_cluster_agent_id + FROM + vulnerability_occurrences v_o + WHERE + v_o.vulnerability_id = NEW.id + LIMIT 1; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_issue_links WHERE vulnerability_issue_links.vulnerability_id = NEW.id) + INTO + has_issues; + + SELECT + EXISTS (SELECT 1 FROM vulnerability_merge_request_links WHERE vulnerability_merge_request_links.vulnerability_id = NEW.id) + INTO + has_merge_request; + + INSERT INTO vulnerability_reads (vulnerability_id, project_id, scanner_id, report_type, severity, state, resolved_on_default_branch, uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request) + VALUES (NEW.id, NEW.project_id, scanner_id, NEW.report_type, NEW.severity, NEW.state, NEW.resolved_on_default_branch, uuid::uuid, location_image, cluster_agent_id, casted_cluster_agent_id, has_issues, has_merge_request) + ON CONFLICT(vulnerability_id) DO NOTHING; + RETURN NULL; + END + $$; + SQL + end +end diff --git a/db/migrate/20250612014729_update_insert_or_update_vulnerability_reads_function.rb b/db/migrate/20250715024650_update_insert_or_update_vulnerability_reads_function.rb similarity index 99% rename from db/migrate/20250612014729_update_insert_or_update_vulnerability_reads_function.rb rename to db/migrate/20250715024650_update_insert_or_update_vulnerability_reads_function.rb index 5f373d79c50555..94bee46b8afeeb 100644 --- a/db/migrate/20250612014729_update_insert_or_update_vulnerability_reads_function.rb +++ b/db/migrate/20250715024650_update_insert_or_update_vulnerability_reads_function.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class UpdateInsertOrUpdateVulnerabilityReadsFunction < Gitlab::Database::Migration[2.3] - milestone '18.1' + milestone '18.3' def up execute <<~SQL diff --git a/db/schema_migrations/20250612014729 b/db/schema_migrations/20250612014729 deleted file mode 100644 index 3f5bea8a13994f..00000000000000 --- a/db/schema_migrations/20250612014729 +++ /dev/null @@ -1 +0,0 @@ -849eb31cf28d40b5627ff29f29a6bd4c437a56b87ea4f5437efd321a4710d9de \ No newline at end of file diff --git a/db/schema_migrations/20250715024615 b/db/schema_migrations/20250715024615 new file mode 100644 index 00000000000000..cae760dc686c9b --- /dev/null +++ b/db/schema_migrations/20250715024615 @@ -0,0 +1 @@ +171b688f2fdc186b86c798aa982bd9aea313f84e4303878a2b420e1a03d7b65a \ No newline at end of file diff --git a/db/schema_migrations/20250715024650 b/db/schema_migrations/20250715024650 new file mode 100644 index 00000000000000..5b5c64a5383919 --- /dev/null +++ b/db/schema_migrations/20250715024650 @@ -0,0 +1 @@ +88003b5383ec342db49271677ff43c5563988f8cfc8dc2c7a8db8d636573da1f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9b7ae1d731843f..de60a3f6f3b53c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -693,6 +693,10 @@ DECLARE has_issues boolean; has_merge_request boolean; BEGIN + IF (SELECT current_setting('vulnerability_management.dont_execute_db_trigger', true) = 'true') THEN + RETURN NULL; + END IF; + SELECT v_o.scanner_id, v_o.uuid, v_o.location->>'image', v_o.location->'kubernetes_resource'->>'agent_id', CAST(v_o.location->'kubernetes_resource'->>'agent_id' AS bigint) INTO diff --git a/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml b/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml index 244db0b82cfc0d..422a4870e81552 100644 --- a/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml +++ b/ee/config/feature_flags/gitlab_com_derisk/turn_off_vulnerability_read_create_db_trigger_function.yml @@ -4,7 +4,7 @@ description: Bypasses the trigger functions in the structure.sql that insert/upd feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/546223 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193387 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/553939 -milestone: '18.2' +milestone: '18.3' group: group::security infrastructure type: gitlab_com_derisk default_enabled: false -- GitLab