From 5a98e0ce0a1449ee4357a95ae2d06b6257b916da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zaj=C4=85c?= Date: Thu, 29 Oct 2020 18:41:28 +0100 Subject: [PATCH 1/3] Compute UUID before creating or updating Finding Additional, minor changes: * Remove debugging logs * Update specs --- ee/app/services/security/store_report_service.rb | 11 ++++------- ee/lib/gitlab/ci/reports/security/finding.rb | 2 -- .../security/store_report_service_spec.rb | 15 ++++++++++++++- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/ee/app/services/security/store_report_service.rb b/ee/app/services/security/store_report_service.rb index bc8217fb1fe794..a86d4d6c12dc1d 100644 --- a/ee/app/services/security/store_report_service.rb +++ b/ee/app/services/security/store_report_service.rb @@ -48,6 +48,7 @@ def create_vulnerability_finding(finding) end vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :scan, :links) + vulnerability_params[:uuid] = calculcate_uuid_v5(finding) vulnerability_finding = create_or_find_vulnerability_finding(finding, vulnerability_params) update_vulnerability_scanner(finding) @@ -81,8 +82,6 @@ def create_or_find_vulnerability_finding(finding, create_params) .create_with(create_params) .find_or_initialize_by(find_params) - vulnerability_finding.uuid = calculcate_uuid_v5(vulnerability_finding, find_params) - vulnerability_finding.save! vulnerability_finding rescue ActiveRecord::RecordNotUnique @@ -92,11 +91,11 @@ def create_or_find_vulnerability_finding(finding, create_params) end end - def calculcate_uuid_v5(vulnerability_finding, finding_params) + def calculcate_uuid_v5(vulnerability_finding) uuid_v5_name_components = { report_type: vulnerability_finding.report_type, - primary_identifier_fingerprint: vulnerability_finding.primary_identifier&.fingerprint || finding_params.dig(:primary_identifier, :fingerprint), - location_fingerprint: vulnerability_finding.location_fingerprint, + primary_identifier_fingerprint: vulnerability_finding.primary_fingerprint, + location_fingerprint: vulnerability_finding.location.fingerprint, project_id: project.id } @@ -106,8 +105,6 @@ def calculcate_uuid_v5(vulnerability_finding, finding_params) name = uuid_v5_name_components.values.join('-') - Gitlab::AppLogger.debug(message: "Generating UUIDv5 with name: #{name}") if Gitlab.dev_env_or_com? - Gitlab::Vulnerabilities::CalculateFindingUUID.call(name) end diff --git a/ee/lib/gitlab/ci/reports/security/finding.rb b/ee/lib/gitlab/ci/reports/security/finding.rb index d0471861688f83..b3c2a15330f72e 100644 --- a/ee/lib/gitlab/ci/reports/security/finding.rb +++ b/ee/lib/gitlab/ci/reports/security/finding.rb @@ -97,8 +97,6 @@ def keys end end - protected - def primary_fingerprint primary_identifier&.fingerprint end diff --git a/ee/spec/services/security/store_report_service_spec.rb b/ee/spec/services/security/store_report_service_spec.rb index bfc8b5cbc43118..ba4e06fdc6477f 100644 --- a/ee/spec/services/security/store_report_service_spec.rb +++ b/ee/spec/services/security/store_report_service_spec.rb @@ -2,6 +2,17 @@ require 'spec_helper' +UUID_REGEXP = Regexp.new("^([0-9a-f]{8})-([0-9a-f]{4})-([0-9a-f]{4})-" \ + "([0-9a-f]{2})([0-9a-f]{2})-([0-9a-f]{12})$").freeze + +def is_uuid_v5?(uuid_string) + raise TypeError unless uuid_string.is_a?(String) + + uuid_components = uuid_string.downcase.scan(UUID_REGEXP).first + time_hi_and_version = uuid_components[2].to_i(16) + (time_hi_and_version >> 12) == 5 +end + RSpec.describe Security::StoreReportService, '#execute' do let_it_be(:user) { create(:user) } let(:artifact) { create(:ee_ci_job_artifact, trait) } @@ -57,7 +68,9 @@ end it 'calculates UUIDv5 for all findings' do - expect(Vulnerabilities::Finding.pluck(:uuid)).to all(be_a(String)) + uuids = Vulnerabilities::Finding.pluck(:uuid) + expect(uuids).to all(be_a(String)) + expect(uuids.all? { |uuid| is_uuid_v5?(uuid) }).to be_truthy end end -- GitLab From f6f3c703725820e613c6a59a742045098fe76502 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zaj=C4=85c?= Date: Mon, 2 Nov 2020 15:47:52 +0100 Subject: [PATCH 2/3] Fix typo in method name --- ee/app/services/security/store_report_service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/services/security/store_report_service.rb b/ee/app/services/security/store_report_service.rb index a86d4d6c12dc1d..5e8b04cbed75ac 100644 --- a/ee/app/services/security/store_report_service.rb +++ b/ee/app/services/security/store_report_service.rb @@ -48,7 +48,7 @@ def create_vulnerability_finding(finding) end vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner, :scan, :links) - vulnerability_params[:uuid] = calculcate_uuid_v5(finding) + vulnerability_params[:uuid] = calculate_uuid_v5(finding) vulnerability_finding = create_or_find_vulnerability_finding(finding, vulnerability_params) update_vulnerability_scanner(finding) @@ -91,7 +91,7 @@ def create_or_find_vulnerability_finding(finding, create_params) end end - def calculcate_uuid_v5(vulnerability_finding) + def calculate_uuid_v5(vulnerability_finding) uuid_v5_name_components = { report_type: vulnerability_finding.report_type, primary_identifier_fingerprint: vulnerability_finding.primary_fingerprint, -- GitLab From e38238d7aba083a3b0b67e4d63e2d2325c72ddaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Zaj=C4=85c?= Date: Mon, 2 Nov 2020 15:55:15 +0100 Subject: [PATCH 3/3] Improve StoreReportService specs --- .../security/store_report_service_spec.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/ee/spec/services/security/store_report_service_spec.rb b/ee/spec/services/security/store_report_service_spec.rb index ba4e06fdc6477f..1b5301978c7a92 100644 --- a/ee/spec/services/security/store_report_service_spec.rb +++ b/ee/spec/services/security/store_report_service_spec.rb @@ -5,12 +5,14 @@ UUID_REGEXP = Regexp.new("^([0-9a-f]{8})-([0-9a-f]{4})-([0-9a-f]{4})-" \ "([0-9a-f]{2})([0-9a-f]{2})-([0-9a-f]{12})$").freeze -def is_uuid_v5?(uuid_string) - raise TypeError unless uuid_string.is_a?(String) +RSpec::Matchers.define :be_uuid_v5 do + match do |string| + expect(string).to be_a(String) - uuid_components = uuid_string.downcase.scan(UUID_REGEXP).first - time_hi_and_version = uuid_components[2].to_i(16) - (time_hi_and_version >> 12) == 5 + uuid_components = string.downcase.scan(UUID_REGEXP).first + time_hi_and_version = uuid_components[2].to_i(16) + (time_hi_and_version >> 12) == 5 + end end RSpec.describe Security::StoreReportService, '#execute' do @@ -68,9 +70,9 @@ def is_uuid_v5?(uuid_string) end it 'calculates UUIDv5 for all findings' do + subject uuids = Vulnerabilities::Finding.pluck(:uuid) - expect(uuids).to all(be_a(String)) - expect(uuids.all? { |uuid| is_uuid_v5?(uuid) }).to be_truthy + expect(uuids).to all(be_uuid_v5) end end -- GitLab