Commit a139d7db authored by Stan Hu's avatar Stan Hu

Gracefully handle bad dependency scanner input

We saw that in
https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6051 there
was one CI artifact that had a `dependency_files` input but a `null`
`dependencies`. We now do stronger type checking to ensure the field is
an array.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/346971

Changelog: fixed
EE: true
parent 60010802
...@@ -19,7 +19,11 @@ module Gitlab ...@@ -19,7 +19,11 @@ module Gitlab
def parse_dependency_names(report_data, report) def parse_dependency_names(report_data, report)
report_data.fetch('dependency_files', []).each do |file| report_data.fetch('dependency_files', []).each do |file|
file['dependencies'].each do |dependency| dependencies = file['dependencies']
return unless dependencies.is_a?(Array)
dependencies.each do |dependency|
report.add_dependency(formatter.format(dependency, file['package_manager'], file['path'])) report.add_dependency(formatter.format(dependency, file['package_manager'], file['path']))
end end
end end
......
...@@ -11,12 +11,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do ...@@ -11,12 +11,6 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
let_it_be(:pipeline) { create :ee_ci_pipeline, :with_dependency_list_report } let_it_be(:pipeline) { create :ee_ci_pipeline, :with_dependency_list_report }
describe '#parse!' do describe '#parse!' do
before do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
context 'with dependency_list artifact' do context 'with dependency_list artifact' do
let(:artifact) { pipeline.job_artifacts.last } let(:artifact) { pipeline.job_artifacts.last }
...@@ -56,6 +50,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do ...@@ -56,6 +50,12 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
let(:artifact) { pipeline.job_artifacts.last } let(:artifact) { pipeline.job_artifacts.last }
before do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
it 'does not causes N+1 query' do it 'does not causes N+1 query' do
control_count = ActiveRecord::QueryRecorder.new do control_count = ActiveRecord::QueryRecorder.new do
artifact.each_blob do |blob| artifact.each_blob do |blob|
...@@ -96,6 +96,45 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do ...@@ -96,6 +96,45 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
end end
end end
end end
context 'with null dependencies' do
let(:json_data) do
<<~JSON
{
"version": "3.0.0",
"vulnerabilities": [],
"remediations": [],
"dependency_files": [
{
"path": "package-lock.json",
"package_manager": "npm",
"dependencies": null
}
],
"scan": {
"scanner": {
"id": "",
"name": "",
"vendor": {
"name": ""
},
"version": ""
},
"type": "",
"start_time": "2021-12-10T15:32:33",
"end_time": "2021-12-10T15:32:54",
"status": "success"
}
}
JSON
end
it 'ignores null dependencies' do
parser.parse!(json_data, report)
expect(report.dependencies.size).to eq(0)
end
end
end end
describe '#parse_licenses!' do describe '#parse_licenses!' 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