Commit a71d0d5a authored by Mehmet Emin INAC's avatar Mehmet Emin INAC

Persist remediations in database while saving the reports

We will start using only the remediation entries after migrating the
existing findings.
parent e8bd79d5
...@@ -17,5 +17,11 @@ module Vulnerabilities ...@@ -17,5 +17,11 @@ module Vulnerabilities
validates :summary, presence: true, length: { maximum: 200 } validates :summary, presence: true, length: { maximum: 200 }
validates :file, presence: true validates :file, presence: true
validates :checksum, presence: true validates :checksum, presence: true
scope :by_checksum, -> (checksum) { where(checksum: checksum) }
def retrieve_upload(_identifier, paths)
Upload.find_by(model: self, path: paths)
end
end end
end end
...@@ -54,6 +54,7 @@ module Security ...@@ -54,6 +54,7 @@ module Security
update_vulnerability_scanner(finding) update_vulnerability_scanner(finding)
update_vulnerability_finding(vulnerability_finding, vulnerability_params) update_vulnerability_finding(vulnerability_finding, vulnerability_params)
reset_remediations_for(vulnerability_finding, finding)
# The maximum number of identifiers is not used in validation # The maximum number of identifiers is not used in validation
# we just want to ignore the rest if a finding has more than that. # we just want to ignore the rest if a finding has more than that.
...@@ -133,6 +134,34 @@ module Security ...@@ -133,6 +134,34 @@ module Security
rescue ActiveRecord::RecordNotUnique rescue ActiveRecord::RecordNotUnique
end end
def reset_remediations_for(vulnerability_finding, finding)
existing_remediations = find_existing_remediations_for(finding)
new_remediations = build_new_remediations_for(finding, existing_remediations)
vulnerability_finding.remediations = existing_remediations + new_remediations
end
def find_existing_remediations_for(finding)
checksums = finding.remediations.map(&:checksum)
Vulnerabilities::Remediation.by_checksum(checksums)
end
def build_new_remediations_for(finding, existing_remediations)
find_missing_remediations_for(finding, existing_remediations)
.map { |remediation| build_vulnerability_remediation(remediation) }
end
def find_missing_remediations_for(finding, existing_remediations)
existing_remediation_checksums = existing_remediations.map(&:checksum)
finding.remediations.select { |remediation| !remediation.checksum.in?(existing_remediation_checksums) }
end
def build_vulnerability_remediation(remediation)
Vulnerabilities::Remediation.new(summary: remediation.summary, file: remediation.diff_file, checksum: remediation.checksum)
end
def create_vulnerability_pipeline_object(vulnerability_finding, pipeline) def create_vulnerability_pipeline_object(vulnerability_finding, pipeline)
vulnerability_finding.finding_pipelines.find_or_create_by!(pipeline: pipeline) vulnerability_finding.finding_pipelines.find_or_create_by!(pipeline: pipeline)
rescue ActiveRecord::RecordNotUnique rescue ActiveRecord::RecordNotUnique
......
# frozen_string_literal: true
FactoryBot.define do
factory :vulnerabilities_remediation, class: 'Vulnerabilities::Remediation' do
summary { 'Remediation Summary' }
file { Tempfile.new }
sequence :checksum do |i|
Digest::SHA256.hexdigest(i.to_s)
end
end
end
...@@ -8,12 +8,12 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do ...@@ -8,12 +8,12 @@ RSpec.describe Gitlab::Ci::Reports::Security::Finding do
describe '#initialize' do describe '#initialize' do
subject { described_class.new(**params) } subject { described_class.new(**params) }
let(:primary_identifier) { create(:ci_reports_security_identifier) } let_it_be(:primary_identifier) { build(:ci_reports_security_identifier) }
let(:other_identifier) { create(:ci_reports_security_identifier) } let_it_be(:other_identifier) { build(:ci_reports_security_identifier) }
let(:link) { create(:ci_reports_security_link) } let_it_be(:link) { build(:ci_reports_security_link) }
let(:scanner) { create(:ci_reports_security_scanner) } let_it_be(:scanner) { build(:ci_reports_security_scanner) }
let(:location) { create(:ci_reports_security_locations_sast) } let_it_be(:location) { build(:ci_reports_security_locations_sast) }
let(:remediation) { create(:ci_reports_security_remediation) } let_it_be(:remediation) { build(:ci_reports_security_remediation) }
let(:params) do let(:params) do
{ {
......
...@@ -3,17 +3,20 @@ ...@@ -3,17 +3,20 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Ci::Reports::Security::Remediation do RSpec.describe Gitlab::Ci::Reports::Security::Remediation do
let(:remediation) { build(:ci_reports_security_remediation) } let(:diff) { 'foo' }
let(:remediation) { build(:ci_reports_security_remediation, diff: diff) }
describe '#diff_file' do describe '#diff_file' do
subject { remediation.diff_file.read } subject { remediation.diff_file.read }
it { is_expected.to eq('foo') } it { is_expected.to eq(diff) }
end end
describe '#checksum' do describe '#checksum' do
let(:expected_checksum) { '2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae' }
subject { remediation.checksum } subject { remediation.checksum }
it { is_expected.to eq('2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae') } it { is_expected.to eq(expected_checksum) }
end end
end end
...@@ -10,4 +10,13 @@ RSpec.describe Vulnerabilities::Remediation do ...@@ -10,4 +10,13 @@ RSpec.describe Vulnerabilities::Remediation do
it { is_expected.to validate_presence_of(:file) } it { is_expected.to validate_presence_of(:file) }
it { is_expected.to validate_presence_of(:checksum) } it { is_expected.to validate_presence_of(:checksum) }
it { is_expected.to validate_length_of(:summary).is_at_most(200) } it { is_expected.to validate_length_of(:summary).is_at_most(200) }
describe '.by_checksum' do
let_it_be(:remediation_1) { create(:vulnerabilities_remediation) }
let_it_be(:remediation_2) { create(:vulnerabilities_remediation) }
subject { described_class.by_checksum(remediation_2.checksum) }
it { is_expected.to match_array([remediation_2]) }
end
end end
...@@ -37,11 +37,11 @@ RSpec.describe Security::StoreReportService, '#execute' do ...@@ -37,11 +37,11 @@ RSpec.describe Security::StoreReportService, '#execute' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:case_name, :trait, :scanners, :identifiers, :findings, :finding_identifiers, :finding_pipelines, :finding_links) do where(:case_name, :trait, :scanners, :identifiers, :findings, :finding_identifiers, :finding_pipelines, :remediations) do
'with SAST report' | :sast | 3 | 17 | 33 | 39 | 33 | 0 'with SAST report' | :sast | 3 | 17 | 33 | 39 | 33 | 0
'with exceeding identifiers' | :with_exceeding_identifiers | 1 | 20 | 1 | 20 | 1 | 0 'with exceeding identifiers' | :with_exceeding_identifiers | 1 | 20 | 1 | 20 | 1 | 0
'with Dependency Scanning report' | :dependency_scanning | 2 | 7 | 4 | 7 | 4 | 6 'with Dependency Scanning report' | :dependency_scanning_remediation | 1 | 3 | 2 | 3 | 2 | 1
'with Container Scanning report' | :container_scanning | 1 | 8 | 8 | 8 | 8 | 8 'with Container Scanning report' | :container_scanning | 1 | 8 | 8 | 8 | 8 | 0
end end
with_them do with_them do
...@@ -65,6 +65,10 @@ RSpec.describe Security::StoreReportService, '#execute' do ...@@ -65,6 +65,10 @@ RSpec.describe Security::StoreReportService, '#execute' do
expect { subject }.to change { Vulnerabilities::FindingPipeline.count }.by(finding_pipelines) expect { subject }.to change { Vulnerabilities::FindingPipeline.count }.by(finding_pipelines)
end end
it 'inserts all remediations' do
expect { subject }.to change { Vulnerabilities::Remediation.count }.by(remediations)
end
it 'inserts all vulnerabilties' do it 'inserts all vulnerabilties' do
expect { subject }.to change { Vulnerability.count }.by(findings) expect { subject }.to change { Vulnerability.count }.by(findings)
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment