diff --git a/db/docs/batched_background_migrations/backfill_finding_id_in_vulnerabilities.yml b/db/docs/batched_background_migrations/backfill_finding_id_in_vulnerabilities.yml index aaa901d46ea92e24c1a26b82be81f8a737786eb8..f2eca56c407fee10206a26559779a4b5d0cf0d8f 100644 --- a/db/docs/batched_background_migrations/backfill_finding_id_in_vulnerabilities.yml +++ b/db/docs/batched_background_migrations/backfill_finding_id_in_vulnerabilities.yml @@ -6,3 +6,4 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/130058 milestone: '16.7' queued_migration_version: 20231129105945 finalize_after: '2024-01-15' +finalized_by: 20240214204757 diff --git a/db/docs/batched_background_migrations/drop_vulnerabilities_without_finding_id.yml b/db/docs/batched_background_migrations/drop_vulnerabilities_without_finding_id.yml index 37e7fae2ea6bc10c172a7d5636586d285b0ca4fd..b183369fa856dd4721dde1f5afc15e30a458bc7d 100644 --- a/db/docs/batched_background_migrations/drop_vulnerabilities_without_finding_id.yml +++ b/db/docs/batched_background_migrations/drop_vulnerabilities_without_finding_id.yml @@ -6,4 +6,4 @@ introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/140532 milestone: '16.8' queued_migration_version: 20231221223259 finalize_after: '2024-01-22' -finalized_by: # version of the migration that finalized this BBM +finalized_by: 20240214204757 diff --git a/db/post_migrate/20240214204757_finalize_finding_id_migrations.rb b/db/post_migrate/20240214204757_finalize_finding_id_migrations.rb new file mode 100644 index 0000000000000000000000000000000000000000..7b925bcb6da615441a1890d4c6b00a85cb733ad3 --- /dev/null +++ b/db/post_migrate/20240214204757_finalize_finding_id_migrations.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class FinalizeFindingIdMigrations < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '16.9' + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + finalize_finding_id_backfill + finalize_empty_finding_id_removal + end + + def down; end + + private + + def finalize_finding_id_backfill + ensure_batched_background_migration_is_finished( + job_class_name: 'BackfillFindingIdInVulnerabilities', + table_name: :vulnerabilities, + column_name: 'id', + job_arguments: [] + ) + end + + def finalize_empty_finding_id_removal + ensure_batched_background_migration_is_finished( + job_class_name: 'DropVulnerabilitiesWithoutFindingId', + table_name: :vulnerabilities, + column_name: 'id', + job_arguments: [] + ) + end +end diff --git a/db/post_migrate/20240214204805_make_finding_id_not_null.rb b/db/post_migrate/20240214204805_make_finding_id_not_null.rb new file mode 100644 index 0000000000000000000000000000000000000000..380e6fca5daa21deb795daa82c47e72dc2c40fce --- /dev/null +++ b/db/post_migrate/20240214204805_make_finding_id_not_null.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class MakeFindingIdNotNull < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '16.9' + + def up + add_not_null_constraint :vulnerabilities, :finding_id + end + + def down + remove_not_null_constraint :vulnerabilities, :finding_id + end +end diff --git a/db/schema_migrations/20240214204757 b/db/schema_migrations/20240214204757 new file mode 100644 index 0000000000000000000000000000000000000000..96c471ced6173f6f21252c9d80d13ee89c3e798c --- /dev/null +++ b/db/schema_migrations/20240214204757 @@ -0,0 +1 @@ +35a60bc57ed21353aa5570831aac6f991a10bb4e3ec10559bc38000488f59799 \ No newline at end of file diff --git a/db/schema_migrations/20240214204805 b/db/schema_migrations/20240214204805 new file mode 100644 index 0000000000000000000000000000000000000000..946011a1e6f95423c538e0e1f618a10e53bc8a31 --- /dev/null +++ b/db/schema_migrations/20240214204805 @@ -0,0 +1 @@ +078e43ac4de3b81bcc9cd73d37e7bd614f1ffd70df41693988a187caee6fdf6d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 47f57e1d96085c31fdf1ba0585ccfe58fd0e147f..2d12315c9f8d3fc292dca3b77aec1e9c7b35ca7c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16930,7 +16930,8 @@ CREATE TABLE vulnerabilities ( present_on_default_branch boolean DEFAULT true NOT NULL, detected_at timestamp with time zone DEFAULT now(), finding_id bigint, - cvss jsonb DEFAULT '[]'::jsonb + cvss jsonb DEFAULT '[]'::jsonb, + CONSTRAINT check_4d8a873f1f CHECK ((finding_id IS NOT NULL)) ); CREATE SEQUENCE vulnerabilities_id_seq diff --git a/ee/app/models/ee/vulnerability.rb b/ee/app/models/ee/vulnerability.rb index 591d2672a261e381808c36eb981807fb4a6195f6..f5f13300253176e62e0c0fd6a4e0390d0addfc40 100644 --- a/ee/app/models/ee/vulnerability.rb +++ b/ee/app/models/ee/vulnerability.rb @@ -42,6 +42,9 @@ module Vulnerability has_one :group, through: :project has_one :vulnerability_read, class_name: '::Vulnerabilities::Read' + belongs_to :vulnerability_finding, class_name: '::Vulnerabilities::Finding', + foreign_key: :finding_id, inverse_of: :one_vulnerability, optional: false + has_many :findings, class_name: '::Vulnerabilities::Finding', inverse_of: :vulnerability has_many :dismissed_findings, -> { dismissed }, class_name: '::Vulnerabilities::Finding', inverse_of: :vulnerability has_many :merge_request_links, class_name: '::Vulnerabilities::MergeRequestLink', inverse_of: :vulnerability diff --git a/ee/app/models/vulnerabilities/finding.rb b/ee/app/models/vulnerabilities/finding.rb index e017e94b694a812b471f583d2d3d5632c6f8f0b4..7e9c851bde19980b3413e3d4a593ed67225ab603 100644 --- a/ee/app/models/vulnerabilities/finding.rb +++ b/ee/app/models/vulnerabilities/finding.rb @@ -26,6 +26,7 @@ class Finding < ApplicationRecord belongs_to :scanner, class_name: 'Vulnerabilities::Scanner' belongs_to :primary_identifier, class_name: 'Vulnerabilities::Identifier', inverse_of: :primary_findings, foreign_key: 'primary_identifier_id' belongs_to :vulnerability, class_name: 'Vulnerability', inverse_of: :findings, foreign_key: 'vulnerability_id' + has_one :one_vulnerability, class_name: 'Vulnerability', inverse_of: :vulnerability_finding has_many :state_transitions, through: :vulnerability has_many :issue_links, through: :vulnerability has_many :merge_request_links, through: :vulnerability diff --git a/ee/app/services/vulnerabilities/create_service.rb b/ee/app/services/vulnerabilities/create_service.rb index fa58d368523e3f9ab015c59c064378196cb7ee75..743aea3caa278da347fa448e77b41fc5c60d849c 100644 --- a/ee/app/services/vulnerabilities/create_service.rb +++ b/ee/app/services/vulnerabilities/create_service.rb @@ -50,6 +50,7 @@ def execute def save_vulnerability(vulnerability, finding) from_state = finding.state + vulnerability.assign_attributes( author: @author, project: @project, diff --git a/ee/app/services/vulnerabilities/manually_create_service.rb b/ee/app/services/vulnerabilities/manually_create_service.rb index 52826ac560db2ce7e4df68a7ea96e47becf52674..587c85e6831f0581ea10f1add429c013220fdf14 100644 --- a/ee/app/services/vulnerabilities/manually_create_service.rb +++ b/ee/app/services/vulnerabilities/manually_create_service.rb @@ -34,9 +34,10 @@ def execute ) Vulnerability.transaction do - vulnerability.save! finding.save! - vulnerability.update!(finding_id: finding.id) + vulnerability.vulnerability_finding = finding + vulnerability.save! + finding.update!(vulnerability_id: vulnerability.id) Statistics::UpdateService.update_for(vulnerability) ServiceResponse.success(payload: { vulnerability: vulnerability }) diff --git a/ee/app/services/vulnerabilities/starboard_vulnerability_create_service.rb b/ee/app/services/vulnerabilities/starboard_vulnerability_create_service.rb index b7b49c0de45bf9f6ee0f050840cbb571a762a25e..232437bb371ae685024867fff4103fdcaf0b211b 100644 --- a/ee/app/services/vulnerabilities/starboard_vulnerability_create_service.rb +++ b/ee/app/services/vulnerabilities/starboard_vulnerability_create_service.rb @@ -36,9 +36,10 @@ def execute end Vulnerability.transaction do - vulnerability.save! finding.save! - vulnerability.update!(finding_id: finding.id) + vulnerability.vulnerability_finding = finding + vulnerability.save! + finding.update!(vulnerability_id: vulnerability.id) agent.update!(has_vulnerabilities: true) unless agent.has_vulnerabilities? Statistics::UpdateService.update_for(vulnerability) diff --git a/ee/spec/factories/vulnerabilities.rb b/ee/spec/factories/vulnerabilities.rb index 834864e54572f39d3461068880002c4379cf50fa..ed429d8794bf07bceb952a8bce99e60a72e3958f 100644 --- a/ee/spec/factories/vulnerabilities.rb +++ b/ee/spec/factories/vulnerabilities.rb @@ -2,6 +2,7 @@ FactoryBot.define do factory :vulnerability do + association :vulnerability_finding, factory: :vulnerabilities_finding project author title { generate(:title) } diff --git a/ee/spec/factories/vulnerabilities/identifiers.rb b/ee/spec/factories/vulnerabilities/identifiers.rb index ae15957c39d4655fd1788f32461bbe7c87614cd9..ee90ba69bccec4d5b1a1a5955a60cd4dc1b30e91 100644 --- a/ee/spec/factories/vulnerabilities/identifiers.rb +++ b/ee/spec/factories/vulnerabilities/identifiers.rb @@ -4,7 +4,7 @@ factory :vulnerabilities_identifier, class: 'Vulnerabilities::Identifier' do external_type { 'CVE' } external_id { 'CVE-2018-1234' } - fingerprint { '52d084cede3db8fafcd6b8ae382ddf1970da3b7f' } + sequence(:fingerprint) { |_| SecureRandom.uuid } name { 'CVE-2018-1234' } url { 'http://cve.mitre.org/cgi-bin/cvename.cgi?name=2018-1234' } project diff --git a/ee/spec/lib/ee/gitlab/background_migration/fix_namespace_ids_of_vulnerability_reads_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/fix_namespace_ids_of_vulnerability_reads_spec.rb index 1313150bbbf4522a14765ff43832c4a2d8a9e937..6362ae222ba963ba2ee77a34ae958df7f0193159 100644 --- a/ee/spec/lib/ee/gitlab/background_migration/fix_namespace_ids_of_vulnerability_reads_spec.rb +++ b/ee/spec/lib/ee/gitlab/background_migration/fix_namespace_ids_of_vulnerability_reads_spec.rb @@ -6,6 +6,8 @@ let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } let(:users) { table(:users) } + let(:vulnerability_findings) { table(:vulnerability_occurrences) } + let(:vulnerability_identifiers) { table(:vulnerability_identifiers) } let(:vulnerability_reads) { table(:vulnerability_reads) } let(:vulnerabilities) { table(:vulnerabilities) } let(:scanners) { table(:vulnerability_scanners) } @@ -15,9 +17,15 @@ let(:namespace_2) { namespaces.create!(name: 'Test Namespace', path: 'namespace-path-2') } let(:project) { projects.create!(name: 'Test', namespace_id: namespace_1.id, project_namespace_id: namespace_1.id) } let(:scanner) { scanners.create!(project_id: project.id, external_id: 'foo', name: 'bar') } + let(:finding) { create_finding(project, scanner) } - let!(:vulnerability_read_with_correct_namespace) { create_vulnerability_read(namespace_id: namespace_1.id) } - let!(:vulnerability_read_with_wrong_namespace) { create_vulnerability_read(namespace_id: namespace_2.id) } + let!(:vulnerability_read_with_correct_namespace) do + create_vulnerability_read(namespace_id: namespace_1.id, finding_id: finding.id) + end + + let!(:vulnerability_read_with_wrong_namespace) do + create_vulnerability_read(namespace_id: namespace_2.id, finding_id: finding.id) + end let(:migration_instance) do described_class.new( @@ -39,7 +47,7 @@ .and not_change { vulnerability_read_with_correct_namespace.reload.namespace_id } end - def create_vulnerability_read(namespace_id:) + def create_vulnerability_read(namespace_id:, finding_id:) vulnerability = vulnerabilities.create!( project_id: project.id, author_id: user.id, @@ -47,7 +55,8 @@ def create_vulnerability_read(namespace_id:) severity: 1, confidence: 1, report_type: 1, - state: 1 + state: 1, + finding_id: finding_id ) vulnerability_reads.create!( @@ -61,4 +70,35 @@ def create_vulnerability_read(namespace_id:) uuid: SecureRandom.uuid ) end + + def create_identifier(project, overrides = {}) + attrs = { + project_id: project.id, + external_id: "CVE-2018-1234", + external_type: "CVE", + name: "CVE-2018-1234", + fingerprint: SecureRandom.hex(20) + }.merge(overrides) + + vulnerability_identifiers.create!(attrs) + end + + def create_finding(project, scanner, overrides = {}) + attrs = { + project_id: project.id, + scanner_id: scanner.id, + severity: 5, # medium + confidence: 2, # unknown, + report_type: 99, # generic + primary_identifier_id: create_identifier(project).id, + project_fingerprint: SecureRandom.hex(20), + location_fingerprint: SecureRandom.hex(20), + uuid: SecureRandom.uuid, + name: "CVE-2018-1234", + raw_metadata: "{}", + metadata_version: "test:1.0" + }.merge(overrides) + + vulnerability_findings.create!(attrs) + end end diff --git a/ee/spec/lib/ee/gitlab/background_migration/migrate_shared_vulnerability_scanners_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/migrate_shared_vulnerability_scanners_spec.rb index 9563f332f0f3cf18f301acdae13654aa14e3e6ab..73ef05744569e61255ecc39b557f5468e7ed3886 100644 --- a/ee/spec/lib/ee/gitlab/background_migration/migrate_shared_vulnerability_scanners_spec.rb +++ b/ee/spec/lib/ee/gitlab/background_migration/migrate_shared_vulnerability_scanners_spec.rb @@ -70,7 +70,8 @@ def vulnerability(project, overrides = {}) report_type: 0, # sast description: "test", project_id: project.id, - author_id: overrides.fetch(:author_id) { user.id } + author_id: overrides.fetch(:author_id) { user.id }, + finding_id: overrides.fetch(:finding_id) } vulnerabilities.create!(attrs) @@ -220,11 +221,16 @@ def identifier(project, overrides = {}) let!(:user_a) { user(email: "test1@example.com", username: "test1") } let!(:user_b) { user(email: "test2@example.com", username: "test2") } - let(:vulnerability_a) { vulnerability(project, author_id: user_a.id) } - let(:vulnerability_b) { vulnerability(other_project, author_id: user_b.id) } + let!(:correct_finding) { finding(project, shared_scanner) } + let!(:incorrect_finding) { finding(other_project, shared_scanner) } - let!(:correct_finding) { finding(project, shared_scanner, vulnerability_id: vulnerability_a.id) } - let!(:incorrect_finding) { finding(other_project, shared_scanner, vulnerability_id: vulnerability_b.id) } + let(:vulnerability_a) { vulnerability(project, author_id: user_a.id, finding_id: correct_finding.id) } + let(:vulnerability_b) { vulnerability(other_project, author_id: user_b.id, finding_id: incorrect_finding.id) } + + before do + correct_finding.update!(vulnerability_id: vulnerability_a.id) + incorrect_finding.update!(vulnerability_id: vulnerability_b.id) + end it "updates vulnerability reads" do subject diff --git a/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb index 636661ee7e9662685f09dffcc580be162ca11ff3..3ec51d38af78e896ed817ace229eae4cbb5dba29 100644 --- a/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb +++ b/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb @@ -247,8 +247,10 @@ context "when a Vulnerability is dismissed" do before do - vulnerability = create_vulnerability(project, user) - finding = create_finding(project, scanner, vulnerability_id: vulnerability.id) + finding = create_finding(project, scanner) + vulnerability = create_vulnerability(project, user, finding_id: finding.id) + finding.update!(vulnerability_id: vulnerability.id) + create_feedback( project, user, @@ -309,8 +311,8 @@ # rubocop:disable RSpec/MultipleMemoizedHelpers context "when a Vulnerability is dismissed with a comment" do let(:comment) { "This is a test comment" } - let(:vulnerability) { create_vulnerability(project, user) } - let(:finding) { create_finding(project, scanner, vulnerability_id: vulnerability.id) } + let!(:finding) { create_finding(project, scanner) } + let(:vulnerability) { create_vulnerability(project, user, finding_id: finding.id) } let(:feedback) do create_feedback( project, @@ -325,6 +327,7 @@ end before do + finding.update!(vulnerability_id: vulnerability.id) create_feedback( project, user, @@ -375,8 +378,9 @@ dismissal_reason = 3 # used_in_test before do - vulnerability = create_vulnerability(project, user) - finding = create_finding(project, scanner, vulnerability_id: vulnerability.id) + finding = create_finding(project, scanner) + vulnerability = create_vulnerability(project, user, finding_id: finding.id) + finding.update!(vulnerability_id: vulnerability.id) create_feedback( project, user, @@ -498,7 +502,8 @@ def create_vulnerability(project, user, overrides = {}) report_type: 0, # sast description: "test", project_id: project.id, - author_id: overrides.fetch(:author_id) { user.id } + author_id: overrides.fetch(:author_id) { user.id }, + finding_id: overrides.fetch(:finding_id) } vulnerabilities.create!(attrs) diff --git a/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb b/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb index 1b406bfddaeaa55cd797d1f2b3373a5e4a3d7450..9bd31ab78a0bcbd85f1da353f5c6bb4046f804e1 100644 --- a/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb +++ b/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb @@ -41,8 +41,7 @@ let(:merge_request) { create_merge_request } let!(:pipeline) { create_ci_pipeline } - let(:vulnerability_id) { nil } - let(:finding) { create_finding(scanner, vulnerability_id: vulnerability_id) } + let(:finding) { create_finding(scanner) } let(:migration_attrs) do { @@ -287,8 +286,11 @@ end context "when there is a vulnerability" do - let(:vulnerability) { create_vulnerability } - let(:vulnerability_id) { vulnerability.id } + before do + finding.update!(vulnerability_id: vulnerability.id) + end + + let!(:vulnerability) { create_vulnerability(finding_id: finding.id) } let!(:feedback) do create_feedback( finding.report_type, @@ -378,8 +380,11 @@ end context "when there is a vulnerability" do - let(:vulnerability) { create_vulnerability } - let(:vulnerability_id) { vulnerability.id } + before do + finding.update!(vulnerability_id: vulnerability.id) + end + + let(:vulnerability) { create_vulnerability(finding_id: finding.id) } let!(:feedback) do create_feedback( finding.report_type, @@ -484,7 +489,8 @@ def create_vulnerability(overrides = {}) report_type: 0, # sast description: "test", project_id: project.id, - author_id: overrides.fetch(:author_id) { user.id } + author_id: overrides.fetch(:author_id) { user.id }, + finding_id: overrides.fetch(:finding_id) } vulnerabilities.create!(attrs) diff --git a/ee/spec/models/vulnerabilities/finding_spec.rb b/ee/spec/models/vulnerabilities/finding_spec.rb index d2868cabbb3bbdc7e9eb81d5a15405bc9f8137df..d0a8b4ed358e5715f2803fc402d8ceb1cff871d0 100644 --- a/ee/spec/models/vulnerabilities/finding_spec.rb +++ b/ee/spec/models/vulnerabilities/finding_spec.rb @@ -20,6 +20,7 @@ it { is_expected.to belong_to(:primary_identifier).class_name('Vulnerabilities::Identifier') } it { is_expected.to belong_to(:scanner).class_name('Vulnerabilities::Scanner') } it { is_expected.to belong_to(:vulnerability).inverse_of(:findings) } + it { is_expected.to have_one(:one_vulnerability).class_name('Vulnerability').inverse_of(:vulnerability_finding) } it { is_expected.to have_many(:finding_pipelines).class_name('Vulnerabilities::FindingPipeline').with_foreign_key('occurrence_id') } it { is_expected.to have_many(:identifiers).class_name('Vulnerabilities::Identifier') } it { is_expected.to have_many(:finding_identifiers).class_name('Vulnerabilities::FindingIdentifier').with_foreign_key('occurrence_id') } diff --git a/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities_spec.rb b/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities_spec.rb index c76496aa90e5139b9c7a799c0bb274b4a9083973..068d0746a9b7820841570ac4edf834d583ace94c 100644 --- a/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities_spec.rb +++ b/ee/spec/services/security/ingestion/tasks/ingest_vulnerabilities_spec.rb @@ -64,8 +64,8 @@ it 'backfills the finding_id column' do expect { ingest_vulnerabilities }.to change { existing_vulnerability.reload.finding_id } - .from(nil).to(existing_vulnerability.finding.id).and change { resolved_vulnerability.reload.finding_id } - .from(nil).to(resolved_vulnerability.finding.id) + .to(existing_vulnerability.finding.id).and change { resolved_vulnerability.reload.finding_id } + .to(resolved_vulnerability.finding.id) end it 'creates new vulnerabilities with present_on_default_branch set to true' do 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 2fcf7a93d47f2038f004895715106948bf36c06b..fb3c797f516b39c8e225f0e12f48e6028babb50f 100644 --- a/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb +++ b/ee/spec/services/vulnerabilities/starboard_vulnerability_resolve_service_spec.rb @@ -62,13 +62,17 @@ expect { service.execute }.not_to change { Vulnerability.resolved.count } end - it "does not resolve vulnerabilities created by other agent" do - Vulnerabilities::Finding.all.each do |finding| - finding.location['kubernetes_resource']['agent_id'] = other_agent.id.to_s - finding.save! + context 'when vulnerabilities were created by other agents' do + before do + existing_vulnerabilities.map(&:finding).each do |finding| + finding.location['kubernetes_resource']['agent_id'] = other_agent.id.to_s + finding.save! + end end - expect { service.execute }.not_to change { Vulnerability.resolved.count } + it "does not resolve vulnerabilities created by other agent" do + expect { service.execute }.not_to change { Vulnerability.resolved.count } + end end it 'does not resolve vulnerabilities in passive states' do diff --git a/spec/lib/gitlab/background_migration/backfill_cluster_agents_has_vulnerabilities_spec.rb b/spec/lib/gitlab/background_migration/backfill_cluster_agents_has_vulnerabilities_spec.rb index edb6ff59340f3351100de1bd180a19c9742e2c69..b14adbc46f4b3281132afac0862469094ca39b11 100644 --- a/spec/lib/gitlab/background_migration/backfill_cluster_agents_has_vulnerabilities_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_cluster_agents_has_vulnerabilities_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::BackfillClusterAgentsHasVulnerabilities, :migration do # rubocop:disable Layout/LineLength +RSpec.describe Gitlab::BackgroundMigration::BackfillClusterAgentsHasVulnerabilities, :migration, + feature_category: :vulnerability_management do let(:migration) do described_class.new( start_id: 1, end_id: 10, @@ -13,6 +14,8 @@ end let(:users_table) { table(:users) } + let(:vulnerability_identifiers_table) { table(:vulnerability_identifiers) } + let(:vulnerability_occurrences_table) { table(:vulnerability_occurrences) } let(:vulnerability_reads_table) { table(:vulnerability_reads) } let(:vulnerability_scanners_table) { table(:vulnerability_scanners) } let(:vulnerabilities_table) { table(:vulnerabilities) } @@ -84,6 +87,17 @@ private def add_vulnerability_read!(id, project_id:, cluster_agent_id:, report_type:) + identifier = vulnerability_identifiers_table.create!(project_id: project_id, external_type: 'uuid-v5', + external_id: 'uuid-v5', fingerprint: OpenSSL::Digest.hexdigest('SHA256', SecureRandom.uuid), + name: "Identifier for UUIDv5 #{project_id} #{cluster_agent_id}") + + finding = vulnerability_occurrences_table.create!( + project_id: project_id, scanner_id: project_id, + primary_identifier_id: identifier.id, name: 'test', severity: 4, confidence: 4, report_type: 0, + uuid: SecureRandom.uuid, project_fingerprint: '123qweasdzxc', + location_fingerprint: 'test', metadata_version: 'test', + raw_metadata: "") + vulnerabilities_table.create!( id: id, project_id: project_id, @@ -91,7 +105,8 @@ def add_vulnerability_read!(id, project_id:, cluster_agent_id:, report_type:) title: "Vulnerability #{id}", severity: 5, confidence: 5, - report_type: report_type + report_type: report_type, + finding_id: finding.id ) vulnerability_reads_table.create!( diff --git a/spec/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads_spec.rb b/spec/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads_spec.rb index 0e7a0210758dd589e1de18199b495d278531803d..bce8e1e095d586314c50e0c6a7bc63947db01728 100644 --- a/spec/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_has_remediations_of_vulnerability_reads_spec.rb @@ -16,8 +16,11 @@ let(:user) { users.create!(username: 'john_doe', email: 'johndoe@gitlab.com', projects_limit: 10) } let(:scanner) { scanners.create!(project_id: project.id, external_id: 'external_id', name: 'Test Scanner') } - let(:vulnerability_1) { create_vulnerability(title: 'vulnerability 1') } - let(:vulnerability_2) { create_vulnerability(title: 'vulnerability 2') } + let!(:vuln_finding_1) { create_finding } + let!(:vuln_finding_2) { create_finding } + + let!(:vulnerability_1) { create_vulnerability(title: 'vulnerability 1', finding_id: vuln_finding_1.id) } + let!(:vulnerability_2) { create_vulnerability(title: 'vulnerability 2', finding_id: vuln_finding_2.id) } let!(:vulnerability_read_1) { create_vulnerability_read(vulnerability_id: vulnerability_1.id) } let!(:vulnerability_read_2) { create_vulnerability_read(vulnerability_id: vulnerability_2.id) } @@ -25,6 +28,7 @@ let(:vulnerability_findings) { table(:vulnerability_occurrences) } let(:vulnerability_findings_remediations) { table(:vulnerability_findings_remediations) } let(:vulnerability_remediations) { table(:vulnerability_remediations) } + let(:vuln_remediation_1) { create_remediation } let(:vulnerability_identifiers) { table(:vulnerability_identifiers) } subject(:perform_migration) do @@ -39,16 +43,25 @@ ).perform end - it 'updates vulnerability_reads records which has remediations' do - vuln_remediation = create_remediation - vuln_finding = create_finding(vulnerability_id: vulnerability_1.id) - vulnerability_findings_remediations.create!( - vulnerability_occurrence_id: vuln_finding.id, - vulnerability_remediation_id: vuln_remediation.id - ) + before do + vuln_finding_1.update!(vulnerability_id: vulnerability_1.id) + vuln_finding_2.update!(vulnerability_id: vulnerability_2.id) + vulnerability_read_1.update!(vulnerability_id: vulnerability_1.id) + vulnerability_read_2.update!(vulnerability_id: vulnerability_2.id) + end - expect { perform_migration }.to change { vulnerability_read_1.reload.has_remediations }.from(false).to(true) - .and not_change { vulnerability_read_2.reload.has_remediations }.from(false) + context "when finding_remediation record exists" do + let!(:finding_remediation) do + vulnerability_findings_remediations.create!( + vulnerability_occurrence_id: vuln_finding_1.id, + vulnerability_remediation_id: vuln_remediation_1.id + ) + end + + it 'updates vulnerability_reads records which has remediations' do + expect { perform_migration }.to change { vulnerability_read_1.reload.has_remediations }.from(false).to(true) + .and not_change { vulnerability_read_2.reload.has_remediations }.from(false) + end end it 'does not modify has_remediations of vulnerabilities which do not have remediations' do diff --git a/spec/lib/gitlab/background_migration/backfill_owasp_top_ten_of_vulnerability_reads_spec.rb b/spec/lib/gitlab/background_migration/backfill_owasp_top_ten_of_vulnerability_reads_spec.rb index 1462848845e006a6ea6dc979004f5c065f633fe4..37c9219cca1aa654a2139ed4bffdb4d1fdd73326 100644 --- a/spec/lib/gitlab/background_migration/backfill_owasp_top_ten_of_vulnerability_reads_spec.rb +++ b/spec/lib/gitlab/background_migration/backfill_owasp_top_ten_of_vulnerability_reads_spec.rb @@ -19,29 +19,43 @@ let(:user) { users.create!(username: 'john_doe', email: 'johndoe@gitlab.com', projects_limit: 10) } let(:scanner) { scanners.create!(project_id: project.id, external_id: 'external_id', name: 'Test Scanner') } - shared_context 'with vulnerability data' do + shared_context 'with vulnerability data' do # rubocop:disable RSpec/MultipleMemoizedHelpers -- we need to satifsy foreign keys let(:external_id) { '' } let(:external_type) { '' } let(:identifier_name) { '' } - let(:vulnerability_1) { create_vulnerability(title: 'vulnerability 1') } - let(:vulnerability_2) { create_vulnerability(title: 'vulnerability 2') } - let(:vulnerability_3) { create_vulnerability(title: 'vulnerability 3') } - let(:vuln_identifier) do create_identifier(external_id: external_id, external_type: external_type, name: identifier_name) end - let(:vuln_finding) do - create_finding(vulnerability_id: vulnerability_1.id, primary_identifier_id: vuln_identifier.id) + let(:vuln_identifier_2) { create_identifier(external_id: 'A1:2021', external_type: 'owasp', name: 'A1 2021') } + let(:vuln_identifier_3) { create_identifier } + + let(:vuln_finding_1) do + create_finding(primary_identifier_id: vuln_identifier.id) + end + + let(:vuln_finding_2) do + create_finding(primary_identifier_id: vuln_identifier_2.id) end + let(:vuln_finding_3) do + create_finding(primary_identifier_id: vuln_identifier_3.id) + end + + let(:vulnerability_1) { create_vulnerability(title: 'vulnerability 1', finding_id: vuln_finding_1.id) } + let(:vulnerability_2) { create_vulnerability(title: 'vulnerability 2', finding_id: vuln_finding_2.id) } + let(:vulnerability_3) { create_vulnerability(title: 'vulnerability 3', finding_id: vuln_finding_3.id) } + let!(:vulnerability_read_1) { create_vulnerability_read(vulnerability_id: vulnerability_1.id) } let!(:vulnerability_read_2) { create_vulnerability_read(vulnerability_id: vulnerability_2.id) } let!(:vulnerability_read_3) { create_vulnerability_read(vulnerability_id: vulnerability_3.id) } before do - create_vulnerability_occurrence_identifier(occurrence_id: vuln_finding.id, identifier_id: vuln_identifier.id) + create_vulnerability_occurrence_identifier(occurrence_id: vuln_finding_1.id, identifier_id: vuln_identifier.id) + vuln_finding_1.update!(vulnerability_id: vulnerability_1.id) + vuln_finding_2.update!(vulnerability_id: vulnerability_2.id) + vuln_finding_3.update!(vulnerability_id: vulnerability_3.id) end end @@ -72,13 +86,6 @@ end it 'updates vulnerability_reads with correct mapping' do - vuln_identifier_2 = create_identifier(external_id: 'A1:2021', external_type: 'owasp', name: 'A1 2021') - vuln_identifier_3 = create_identifier - vuln_finding_2 = create_finding(vulnerability_id: vulnerability_2.id, - primary_identifier_id: vuln_identifier_2.id) - vuln_finding_3 = create_finding(vulnerability_id: vulnerability_3.id, - primary_identifier_id: vuln_identifier_3.id) - create_vulnerability_occurrence_identifier(occurrence_id: vuln_finding_2.id, identifier_id: vuln_identifier_2.id) create_vulnerability_occurrence_identifier(occurrence_id: vuln_finding_3.id, diff --git a/spec/lib/gitlab/background_migration/delete_orphaned_operational_vulnerabilities_spec.rb b/spec/lib/gitlab/background_migration/delete_orphaned_operational_vulnerabilities_spec.rb index 4a1985eeccd423e1c598f8e388ea67b4abb19a90..0860e4b5cd1b3dd220905e85fcaa6a74c06e3eb5 100644 --- a/spec/lib/gitlab/background_migration/delete_orphaned_operational_vulnerabilities_spec.rb +++ b/spec/lib/gitlab/background_migration/delete_orphaned_operational_vulnerabilities_spec.rb @@ -35,18 +35,29 @@ ) end + let!(:vulnerabilities_findings) { table(:vulnerability_occurrences) } + let!(:finding) do + create_finding!( + project_id: project.id, + scanner_id: scanner.id, + primary_identifier_id: primary_identifier.id + ) + end + let!(:vulnerabilities) { table(:vulnerabilities) } let!(:vulnerability_with_finding) do create_vulnerability!( project_id: project.id, - author_id: user.id + author_id: user.id, + finding_id: finding.id ) end let!(:vulnerability_without_finding) do create_vulnerability!( project_id: project.id, - author_id: user.id + author_id: user.id, + finding_id: finding.id ) end @@ -54,7 +65,8 @@ create_vulnerability!( project_id: project.id, author_id: user.id, - report_type: 7 + report_type: 7, + finding_id: finding.id ) end @@ -62,7 +74,8 @@ create_vulnerability!( project_id: project.id, author_id: user.id, - report_type: 99 + report_type: 99, + finding_id: finding.id ) end @@ -76,16 +89,6 @@ name: 'Identifier for UUIDv5') end - let!(:vulnerabilities_findings) { table(:vulnerability_occurrences) } - let!(:finding) do - create_finding!( - vulnerability_id: vulnerability_with_finding.id, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: primary_identifier.id - ) - end - subject(:background_migration) do described_class.new( start_id: vulnerabilities.minimum(:id), diff --git a/spec/lib/gitlab/background_migration/migrate_evidences_for_vulnerability_findings_spec.rb b/spec/lib/gitlab/background_migration/migrate_evidences_for_vulnerability_findings_spec.rb index ba2f571f5aabff24bd7f2203863479999420335c..dd150a8b8572bd34bc13556c197c60dbfd4082bf 100644 --- a/spec/lib/gitlab/background_migration/migrate_evidences_for_vulnerability_findings_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_evidences_for_vulnerability_findings_spec.rb @@ -102,18 +102,18 @@ private def create_finding!(project_id, scanner_id, raw_metadata) - vulnerability = table(:vulnerabilities).create!(project_id: project_id, author_id: user.id, title: 'test', - severity: 4, confidence: 4, report_type: 0) - identifier = table(:vulnerability_identifiers).create!(project_id: project_id, external_type: 'uuid-v5', - external_id: 'uuid-v5', fingerprint: OpenSSL::Digest::SHA256.hexdigest(vulnerability.id.to_s), + external_id: 'uuid-v5', fingerprint: OpenSSL::Digest::SHA256.hexdigest(SecureRandom.uuid), name: 'Identifier for UUIDv5 2 2') - - table(:vulnerability_occurrences).create!( - vulnerability_id: vulnerability.id, project_id: project_id, scanner_id: scanner_id, + finding = table(:vulnerability_occurrences).create!( + project_id: project_id, scanner_id: scanner_id, primary_identifier_id: identifier.id, name: 'test', severity: 4, confidence: 4, report_type: 0, uuid: SecureRandom.uuid, project_fingerprint: '123qweasdzxc', location: { "image" => "alpine:3.4" }, location_fingerprint: 'test', metadata_version: 'test', raw_metadata: raw_metadata.to_json) + vulnerability = table(:vulnerabilities).create!(project_id: project_id, author_id: user.id, title: 'test', + severity: 4, confidence: 4, report_type: 0, finding_id: finding.id) + finding.update!(vulnerability_id: vulnerability.id) + finding end end diff --git a/spec/lib/gitlab/background_migration/migrate_links_for_vulnerability_findings_spec.rb b/spec/lib/gitlab/background_migration/migrate_links_for_vulnerability_findings_spec.rb index b973f9d4350b39fe92604513eb448d42de453f34..36105e174414262394d6d8457e436021acae212b 100644 --- a/spec/lib/gitlab/background_migration/migrate_links_for_vulnerability_findings_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_links_for_vulnerability_findings_spec.rb @@ -175,18 +175,22 @@ private def create_finding!(project_id, scanner_id, raw_metadata) - vulnerability = table(:vulnerabilities).create!(project_id: project_id, author_id: user.id, title: 'test', - severity: 4, confidence: 4, report_type: 0) - identifier = table(:vulnerability_identifiers).create!(project_id: project_id, external_type: 'uuid-v5', - external_id: 'uuid-v5', fingerprint: OpenSSL::Digest::SHA256.hexdigest(vulnerability.id.to_s), + external_id: 'uuid-v5', fingerprint: OpenSSL::Digest::SHA256.hexdigest(SecureRandom.uuid), name: 'Identifier for UUIDv5 2 2') - table(:vulnerability_occurrences).create!( - vulnerability_id: vulnerability.id, project_id: project_id, scanner_id: scanner_id, + finding = table(:vulnerability_occurrences).create!( + project_id: project_id, scanner_id: scanner_id, primary_identifier_id: identifier.id, name: 'test', severity: 4, confidence: 4, report_type: 0, uuid: SecureRandom.uuid, project_fingerprint: '123qweasdzxc', location: { "image" => "alpine:3.4" }, location_fingerprint: 'test', metadata_version: 'test', raw_metadata: raw_metadata.to_json) + + vulnerability = table(:vulnerabilities).create!(project_id: project_id, author_id: user.id, title: 'test', + severity: 4, confidence: 4, report_type: 0, finding_id: finding.id) + + finding.update!(vulnerability_id: vulnerability.id) + + finding end end diff --git a/spec/lib/gitlab/background_migration/migrate_remediations_for_vulnerability_findings_spec.rb b/spec/lib/gitlab/background_migration/migrate_remediations_for_vulnerability_findings_spec.rb index b75c0e61b19ae550792a1f1fae4a13734f8b7ee9..7a641d6f8d7cfa33f25cd1827b40bfb59d28fc5f 100644 --- a/spec/lib/gitlab/background_migration/migrate_remediations_for_vulnerability_findings_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_remediations_for_vulnerability_findings_spec.rb @@ -151,19 +151,23 @@ private def create_finding!(project_id, scanner_id, raw_metadata) - vulnerability = table(:vulnerabilities).create!(project_id: project_id, author_id: user.id, title: 'test', - severity: 4, confidence: 4, report_type: 0) - identifier = table(:vulnerability_identifiers).create!(project_id: project_id, external_type: 'uuid-v5', - external_id: 'uuid-v5', fingerprint: OpenSSL::Digest::SHA256.hexdigest(vulnerability.id.to_s), + external_id: 'uuid-v5', fingerprint: OpenSSL::Digest.hexdigest('SHA256', SecureRandom.uuid), name: 'Identifier for UUIDv5 2 2') - table(:vulnerability_occurrences).create!( - vulnerability_id: vulnerability.id, project_id: project_id, scanner_id: scanner_id, + finding = table(:vulnerability_occurrences).create!( + project_id: project_id, scanner_id: scanner_id, primary_identifier_id: identifier.id, name: 'test', severity: 4, confidence: 4, report_type: 0, uuid: SecureRandom.uuid, project_fingerprint: '123qweasdzxc', location: { "image" => "alpine:3.4" }, location_fingerprint: 'test', metadata_version: 'test', raw_metadata: raw_metadata.to_json) + + vulnerability = table(:vulnerabilities).create!(project_id: project_id, author_id: user.id, title: 'test', + severity: 4, confidence: 4, report_type: 0, finding_id: finding.id) + + finding.update!(vulnerability_id: vulnerability.id) + + finding end def checksum(value) diff --git a/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb b/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb index df1ee49498778fc73b7de9d27216132dccedd71f..50fbb2500ca8cc3504ecd0516f3a4125c7edb265 100644 --- a/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb +++ b/spec/lib/gitlab/background_migration/set_correct_vulnerability_state_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::BackgroundMigration::SetCorrectVulnerabilityState do +RSpec.describe Gitlab::BackgroundMigration::SetCorrectVulnerabilityState, feature_category: :vulnerability_management do let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') } let(:users) { table(:users) } let(:user) { create_user! } @@ -13,8 +13,12 @@ packages_enabled: false) end + let(:identifiers) { table(:vulnerability_identifiers) } + let(:findings) { table(:vulnerability_occurrences) } let(:vulnerabilities) { table(:vulnerabilities) } + let!(:scanner) { create_scanner!(project_id: project.id) } + let!(:vulnerability_with_dismissed_at) do create_vulnerability!( project_id: project.id, @@ -66,9 +70,32 @@ private + def create_scanner!(project_id:) + table(:vulnerability_scanners).create!( + project_id: project_id, + external_id: "External ID", + name: "Test Scanner" + ) + end + def create_vulnerability!( project_id:, author_id:, title: 'test', severity: 7, confidence: 7, report_type: 0, state: 1, dismissed_at: nil ) + identifier = identifiers.create!( + project_id: project_id, + external_type: 'uuid-v5', + external_id: 'uuid-v5', + fingerprint: OpenSSL::Digest.hexdigest('SHA256', SecureRandom.uuid), + name: "Identifier for UUIDv5 #{project_id}" + ) + + finding = findings.create!( + project_id: project_id, scanner_id: scanner.id, + primary_identifier_id: identifier.id, name: 'test', severity: 4, confidence: 4, report_type: 0, + uuid: SecureRandom.uuid, project_fingerprint: '123qweasdzxc', + location_fingerprint: 'test', metadata_version: 'test', + raw_metadata: "") + vulnerabilities.create!( project_id: project_id, author_id: author_id, @@ -77,7 +104,8 @@ def create_vulnerability!( confidence: confidence, report_type: report_type, state: state, - dismissed_at: dismissed_at + dismissed_at: dismissed_at, + finding_id: finding.id ) end diff --git a/spec/support/helpers/migrations_helpers/vulnerabilities_helper.rb b/spec/support/helpers/migrations_helpers/vulnerabilities_helper.rb index 0a86d7abc8398118994d2b21a29951b25c0c124d..a12e85ed8e1085013e46e78bfe4382ad209bb659 100644 --- a/spec/support/helpers/migrations_helpers/vulnerabilities_helper.rb +++ b/spec/support/helpers/migrations_helpers/vulnerabilities_helper.rb @@ -4,7 +4,7 @@ module MigrationHelpers module VulnerabilitiesHelper # rubocop:disable Metrics/ParameterLists def create_finding!( - vulnerability_id:, project_id:, scanner_id:, primary_identifier_id:, + project_id:, scanner_id:, primary_identifier_id:, vulnerability_id: nil, name: "test", severity: 7, confidence: 7, report_type: 0, project_fingerprint: '123qweasdzxc', location_fingerprint: 'test', metadata_version: 'test', raw_metadata: 'test', uuid: 'b1cee17e-3d7a-11ed-b878-0242ac120002') @@ -26,14 +26,16 @@ def create_finding!( end # rubocop:enable Metrics/ParameterLists - def create_vulnerability!(project_id:, author_id:, title: 'test', severity: 7, confidence: 7, report_type: 0) + def create_vulnerability!( + project_id:, author_id:, finding_id:, title: 'test', severity: 7, confidence: 7, report_type: 0) table(:vulnerabilities).create!( project_id: project_id, author_id: author_id, title: title, severity: severity, confidence: confidence, - report_type: report_type + report_type: report_type, + finding_id: finding_id ) end end