Commit b24538af authored by Jonathan Schafer's avatar Jonathan Schafer

Add check for identifiers when creating finding

Added tests and test file
parent 8393f464
...@@ -42,7 +42,7 @@ module Security ...@@ -42,7 +42,7 @@ module Security
end end
def create_vulnerability_finding(finding) def create_vulnerability_finding(finding)
return if finding.scanner.blank? return if finding.scanner.blank? || finding.primary_identifier.blank?
vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner) vulnerability_params = finding.to_hash.except(:compare_key, :identifiers, :location, :scanner)
vulnerability_finding = create_or_find_vulnerability_finding(finding, vulnerability_params) vulnerability_finding = create_or_find_vulnerability_finding(finding, vulnerability_params)
......
---
title: Add identifier check when creating vulnerability findings
merge_request: 39650
author:
type: fixed
...@@ -179,6 +179,16 @@ FactoryBot.define do ...@@ -179,6 +179,16 @@ FactoryBot.define do
end end
end end
trait :sast_with_missing_identifiers do
file_type { :sast }
file_format { :raw }
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
Rails.root.join('ee/spec/fixtures/security_reports/master/gl-sast-missing-identifiers.json'), 'application/json')
end
end
trait :license_management do trait :license_management do
to_create { |instance| instance.save!(validate: false) } to_create { |instance| instance.save!(validate: false) }
......
...@@ -203,6 +203,29 @@ RSpec.describe Security::StoreReportService, '#execute' do ...@@ -203,6 +203,29 @@ RSpec.describe Security::StoreReportService, '#execute' do
expect { subject }.not_to raise_error expect { subject }.not_to raise_error
end end
end end
context 'when the finding does not include a primary identifier' do
let(:bad_pipeline) { create(:ci_pipeline, project: project) }
let(:bad_build) { create(:ci_build, pipeline: bad_pipeline) }
let!(:bad_artifact) { create(:ee_ci_job_artifact, :sast_with_missing_identifiers, job: bad_build) }
let(:bad_report) { bad_pipeline.security_reports.get_report(report_type.to_s, bad_artifact) }
let(:report_type) { :sast }
before do
project.add_developer(user)
allow(bad_pipeline).to receive(:user).and_return(user)
end
subject { described_class.new(bad_pipeline, bad_report).execute }
it 'does not create a new finding' do
expect { subject }.not_to change { Vulnerabilities::Finding.count }
end
it 'does not raise an error' do
expect { subject }.not_to raise_error
end
end
end end
context 'with existing data from same pipeline' do context 'with existing data from same pipeline' do
......
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