Commit 59d21d06 authored by Tetiana Chupryna's avatar Tetiana Chupryna Committed by Rémy Coutable

Merge vulnerabilities from database

Issue https://gitlab.com/gitlab-org/gitlab/-/issues/321086
We get vulnerabilities from database. Before that
we have to parse report on each request
parent e899efc5
...@@ -89,7 +89,7 @@ module EE ...@@ -89,7 +89,7 @@ module EE
def collect_dependency_list_reports!(dependency_list_report) def collect_dependency_list_reports!(dependency_list_report)
if project.feature_available?(:dependency_scanning) if project.feature_available?(:dependency_scanning)
dependency_list = ::Gitlab::Ci::Parsers::Security::DependencyList.new(project, sha) dependency_list = ::Gitlab::Ci::Parsers::Security::DependencyList.new(project, sha, pipeline)
each_report(::Ci::JobArtifact::DEPENDENCY_LIST_REPORT_FILE_TYPES) do |_, blob| each_report(::Ci::JobArtifact::DEPENDENCY_LIST_REPORT_FILE_TYPES) do |_, blob|
dependency_list.parse!(blob, dependency_list_report) dependency_list.parse!(blob, dependency_list_report)
...@@ -101,7 +101,7 @@ module EE ...@@ -101,7 +101,7 @@ module EE
def collect_licenses_for_dependency_list!(dependency_list_report) def collect_licenses_for_dependency_list!(dependency_list_report)
if project.feature_available?(:dependency_scanning) if project.feature_available?(:dependency_scanning)
dependency_list = ::Gitlab::Ci::Parsers::Security::DependencyList.new(project, sha) dependency_list = ::Gitlab::Ci::Parsers::Security::DependencyList.new(project, sha, pipeline)
each_report(::Ci::JobArtifact::LICENSE_SCANNING_REPORT_FILE_TYPES) do |_, blob| each_report(::Ci::JobArtifact::LICENSE_SCANNING_REPORT_FILE_TYPES) do |_, blob|
dependency_list.parse_licenses!(blob, dependency_list_report) dependency_list.parse_licenses!(blob, dependency_list_report)
......
---
name: standalone_vuln_dependency_list
introduced_by_url: https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/55641
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324031
milestone: '13.10'
type: development
group: group::composition analysis
default_enabled: false
...@@ -5,8 +5,10 @@ module Gitlab ...@@ -5,8 +5,10 @@ module Gitlab
module Parsers module Parsers
module Security module Security
class DependencyList class DependencyList
def initialize(project, sha) def initialize(project, sha, pipeline)
@project = project
@formatter = Formatters::DependencyList.new(project, sha) @formatter = Formatters::DependencyList.new(project, sha)
@pipeline = pipeline
end end
def parse!(json_data, report) def parse!(json_data, report)
...@@ -24,10 +26,23 @@ module Gitlab ...@@ -24,10 +26,23 @@ module Gitlab
end end
def parse_vulnerabilities(report_data, report) def parse_vulnerabilities(report_data, report)
report_data.fetch('vulnerabilities', []).each do |vulnerability| if Feature.enabled?(:standalone_vuln_dependency_list, project)
dependency = vulnerability.dig("location", "dependency") vuln_findings = pipeline.vulnerability_findings.dependency_scanning
file = vulnerability.dig("location", "file") vuln_findings.each do |finding|
report.add_dependency(formatter.format(dependency, '', file, vulnerability)) dependency = finding.location.dig("dependency")
next unless dependency
file = finding.file
vulnerability = finding.metadata
report.add_dependency(formatter.format(dependency, '', file, vulnerability))
end
else
report_data.fetch('vulnerabilities', []).each do |vulnerability|
dependency = vulnerability.dig("location", "dependency")
file = vulnerability.dig("location", "file")
report.add_dependency(formatter.format(dependency, '', file, vulnerability))
end
end end
end end
...@@ -40,7 +55,7 @@ module Gitlab ...@@ -40,7 +55,7 @@ module Gitlab
private private
attr_reader :formatter attr_reader :formatter, :pipeline, :project
end end
end end
end end
......
...@@ -54,7 +54,7 @@ RSpec.describe Projects::DependenciesController do ...@@ -54,7 +54,7 @@ RSpec.describe Projects::DependenciesController do
end end
context 'with existing report' do context 'with existing report' do
let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) } let_it_be(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
before do before do
get :index, params: params, format: :json get :index, params: params, format: :json
...@@ -84,6 +84,11 @@ RSpec.describe Projects::DependenciesController do ...@@ -84,6 +84,11 @@ RSpec.describe Projects::DependenciesController do
end end
context 'with params' do context 'with params' do
let_it_be(:finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, :with_pipeline) }
let_it_be(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline) }
let_it_be(:other_finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, package: 'debug', file: 'yarn/yarn.lock', version: '1.0.5', raw_severity: 'Unknown') }
let_it_be(:other_pipeline) { create(:vulnerabilities_finding_pipeline, finding: other_finding, pipeline: pipeline) }
context 'with sorting params' do context 'with sorting params' do
let(:user) { developer } let(:user) { developer }
...@@ -138,7 +143,7 @@ RSpec.describe Projects::DependenciesController do ...@@ -138,7 +143,7 @@ RSpec.describe Projects::DependenciesController do
let(:user) { developer } let(:user) { developer }
it 'return vulnerable dependencies' do it 'return vulnerable dependencies' do
expect(json_response['dependencies'].length).to eq(3) expect(json_response['dependencies'].length).to eq(2)
end end
end end
end end
...@@ -192,17 +197,20 @@ RSpec.describe Projects::DependenciesController do ...@@ -192,17 +197,20 @@ RSpec.describe Projects::DependenciesController do
context 'when report doesn\'t have dependency list field' do context 'when report doesn\'t have dependency list field' do
let(:user) { developer } let(:user) { developer }
let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
let_it_be(:pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, :with_pipeline) }
let_it_be(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline) }
before do before do
get :index, params: params, format: :json get :index, params: params, format: :json
end end
it 'returns dependencies with vulnerabilities' do it 'returns dependencies with vulnerabilities' do
expect(json_response['dependencies'].count).to eq(4) expect(json_response['dependencies'].count).to eq(1)
django = json_response['dependencies'].find { |d| d['name'] == 'Django' } nokogiri = json_response['dependencies'].first
expect(django).not_to be_nil expect(nokogiri).not_to be_nil
expect(django['vulnerabilities']).to eq([{ "name" => "Possible XSS in traceback section of technical 500 debug page", "severity" => "unknown" }]) expect(nokogiri['vulnerabilities']).to eq([{ "name" => "Vulnerabilities in libxml2 in nokogiri", "severity" => "high" }])
expect(json_response['report']['status']).to eq('ok') expect(json_response['report']['status']).to eq('ok')
end end
end end
......
...@@ -450,6 +450,63 @@ FactoryBot.define do ...@@ -450,6 +450,63 @@ FactoryBot.define do
end end
end end
trait :with_dependency_scanning_metadata do
transient do
raw_severity { "High" }
id { "Gemnasium-06565b64-486d-4326-b906-890d9915804d" }
file { "rails/Gemfile.lock" }
package { "nokogiri" }
version { "1.8.0" }
end
after(:build) do |finding, evaluator|
finding.report_type = "dependency_scanning"
finding.name = "Vulnerabilities in libxml2"
finding.metadata_version = "2.1"
finding.raw_metadata = {
"category": "dependency_scanning",
"name": "Vulnerabilities in libxml2",
"message": "Vulnerabilities in libxml2 in nokogiri",
"description": " The version of libxml2 packaged with Nokogiri contains several vulnerabilities.",
"cve": "rails/Gemfile.lock:nokogiri:gemnasium:06565b64-486d-4326-b906-890d9915804d",
"severity": evaluator.raw_severity,
"solution": "Upgrade to latest version.",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {
"file": evaluator.file,
"dependency": {
"package": {
"name": evaluator.package
},
"version": evaluator.version
}
},
"identifiers": [
{
"type": "gemnasium",
"name": evaluator.id,
"value": "06565b64-486d-4326-b906-890d9915804d",
"url": "https://deps.sec.gitlab.com/packages/gem/nokogiri/versions/1.8.0/advisories"
},
{
"type": "usn",
"name": "USN-3424-1",
"value": "USN-3424-1",
"url": "https://usn.ubuntu.com/3424-1/"
}
],
"links": [
{
"url": "https://github.com/sparklemotion/nokogiri/issues/1673"
}
]
}.to_json
end
end
trait :identifier do trait :identifier do
after(:build) do |finding| after(:build) do |finding|
identifier = build( identifier = build(
......
...@@ -3,11 +3,13 @@ ...@@ -3,11 +3,13 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
let(:parser) { described_class.new(project, sha) } let(:parser) { described_class.new(project, sha, pipeline) }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:sha) { '4242424242424242' } let(:sha) { '4242424242424242' }
let(:report) { Gitlab::Ci::Reports::DependencyList::Report.new } let(:report) { Gitlab::Ci::Reports::DependencyList::Report.new }
let_it_be(:pipeline) { create :ee_ci_pipeline, :with_dependency_list_report }
describe '#parse!' do describe '#parse!' do
before do before do
artifact.each_blob do |blob| artifact.each_blob do |blob|
...@@ -16,7 +18,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do ...@@ -16,7 +18,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
end end
context 'with dependency_list artifact' do context 'with dependency_list artifact' do
let(:artifact) { create(:ee_ci_job_artifact, :dependency_list) } let(:artifact) { pipeline.job_artifacts.last }
before do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
it 'parses all files' do it 'parses all files' do
blob_path = "/#{project.full_path}/-/blob/#{sha}/yarn/yarn.lock" blob_path = "/#{project.full_path}/-/blob/#{sha}/yarn/yarn.lock"
...@@ -39,6 +47,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do ...@@ -39,6 +47,18 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
expect(report.dependencies[13][:location][:top_level]).to be_truthy expect(report.dependencies[13][:location][:top_level]).to be_truthy
expect(report.dependencies[13][:location][:ancestors]).to be_nil expect(report.dependencies[13][:location][:ancestors]).to be_nil
end end
end
context "with vulnerabilities from report" do
let(:artifact) { pipeline.job_artifacts.last }
before do
stub_feature_flags(standalone_vuln_dependency_list: false)
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
it 'merge vulnerabilities data' do it 'merge vulnerabilities data' do
vuln_nokogiri = report.dependencies[1][:vulnerabilities] vuln_nokogiri = report.dependencies[1][:vulnerabilities]
...@@ -52,13 +72,67 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do ...@@ -52,13 +72,67 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
expect(vuln_debug[0][:name]).to eq('Regular Expression Denial of Service in debug') expect(vuln_debug[0][:name]).to eq('Regular Expression Denial of Service in debug')
expect(vuln_async.size).to eq(0) expect(vuln_async.size).to eq(0)
end end
context 'with dependency scanning artifact without dependency_list' do
let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning) }
before do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
it 'list of dependencies with vulnerabilities' do
expect(report.dependencies.size).to eq(4)
end
end
end end
context 'with dependency scanning artifact without dependency_list' do context 'with vulnerabilities in the database' do
let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning) } let_it_be(:vulnerability) { create(:vulnerability, report_type: :dependency_scanning) }
let_it_be(:finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, vulnerability: vulnerability) }
let_it_be(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline) }
let(:artifact) { pipeline.job_artifacts.last }
it 'does not causes N+1 query' do
control_count = ActiveRecord::QueryRecorder.new do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
vuln2 = create(:vulnerability, report_type: :dependency_scanning)
finding2 = create(:vulnerabilities_finding, :with_dependency_scanning_metadata, package: 'mini_portile2', vulnerability: vuln2)
create(:vulnerabilities_finding_pipeline, finding: finding2, pipeline: pipeline)
expect do
ActiveRecord::QueryRecorder.new do
artifact.each_blob do |blob|
parser.parse!(blob, report)
end
end
end.not_to exceed_query_limit(control_count)
end
it 'merges vulnerability data' do
vuln_nokogiri = report.dependencies[1][:vulnerabilities]
expect(report.dependencies.size).to eq(21)
expect(vuln_nokogiri.size).to eq(1)
expect(vuln_nokogiri[0][:name]).to eq('Vulnerabilities in libxml2 in nokogiri')
end
context 'with newfound dependency' do
let_it_be(:other_finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, vulnerability: vulnerability, package: 'giri') }
let_it_be(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: other_finding, pipeline: pipeline) }
it 'adds new dependency and vulnerability to the report' do
giri = report.dependencies.detect { |dep| dep[:name] == 'giri' }
it 'list of dependencies with vulnerabilities' do expect(report.dependencies.size).to eq(22)
expect(report.dependencies.size).to eq(4) expect(giri[:vulnerabilities].size).to eq(1)
end
end end
end end
end end
......
...@@ -330,6 +330,7 @@ RSpec.describe Ci::Build do ...@@ -330,6 +330,7 @@ RSpec.describe Ci::Build do
before do before do
stub_licensed_features(dependency_scanning: true) stub_licensed_features(dependency_scanning: true)
stub_feature_flags(standalone_vuln_dependency_list: false)
end end
subject { job.collect_dependency_list_reports!(dependency_list_report) } subject { job.collect_dependency_list_reports!(dependency_list_report) }
......
...@@ -288,7 +288,7 @@ RSpec.describe Ci::Pipeline do ...@@ -288,7 +288,7 @@ RSpec.describe Ci::Pipeline do
it 'returns a dependency list report with collected data' do it 'returns a dependency list report with collected data' do
mini_portile2 = subject.dependencies.find { |x| x[:name] == 'mini_portile2' } mini_portile2 = subject.dependencies.find { |x| x[:name] == 'mini_portile2' }
expect(subject.dependencies.count).to eq(24) expect(subject.dependencies.count).to eq(21)
expect(mini_portile2[:name]).not_to be_empty expect(mini_portile2[:name]).not_to be_empty
expect(mini_portile2[:licenses]).not_to be_empty expect(mini_portile2[:licenses]).not_to be_empty
end end
......
...@@ -19,7 +19,10 @@ RSpec.describe API::Dependencies do ...@@ -19,7 +19,10 @@ RSpec.describe API::Dependencies do
context 'with an authorized user with proper permissions' do context 'with an authorized user with proper permissions' do
before do before do
create(:ee_ci_pipeline, :with_dependency_list_report, project: project) pipeline = create(:ee_ci_pipeline, :with_dependency_list_report, project: project)
finding = create(:vulnerabilities_finding, :with_dependency_scanning_metadata)
create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline)
project.add_developer(user) project.add_developer(user)
request request
end end
...@@ -32,10 +35,10 @@ RSpec.describe API::Dependencies do ...@@ -32,10 +35,10 @@ RSpec.describe API::Dependencies do
end end
it 'returns vulnerabilities info' do it 'returns vulnerabilities info' do
vulnerability = json_response.select { |dep| dep['name'] == 'debug' }[0]['vulnerabilities'][0] vulnerability = json_response.select { |dep| dep['name'] == 'nokogiri' }[0]['vulnerabilities'][0]
expect(vulnerability['name']).to eq('Regular Expression Denial of Service in debug') expect(vulnerability['name']).to eq('Vulnerabilities in libxml2 in nokogiri')
expect(vulnerability['severity']).to eq('unknown') expect(vulnerability['severity']).to eq('high')
end end
context 'with nil package_manager' do context 'with nil package_manager' do
......
...@@ -4,7 +4,11 @@ require 'spec_helper' ...@@ -4,7 +4,11 @@ require 'spec_helper'
RSpec.describe Security::DependencyListService do RSpec.describe Security::DependencyListService do
describe '#execute' do describe '#execute' do
let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report) } let_it_be(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report) }
let_it_be(:nokogiri_finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, :with_pipeline) }
let_it_be(:nokogiri_pipeline) { create(:vulnerabilities_finding_pipeline, finding: nokogiri_finding, pipeline: pipeline) }
let_it_be(:other_finding) { create(:vulnerabilities_finding, :with_dependency_scanning_metadata, package: 'saml2-js', file: 'yarn/yarn.lock', version: '1.5.0', raw_severity: 'Unknown') }
let_it_be(:other_pipeline) { create(:vulnerabilities_finding_pipeline, finding: other_finding, pipeline: pipeline) }
subject { described_class.new(pipeline: pipeline, params: params).execute } subject { described_class.new(pipeline: pipeline, params: params).execute }
...@@ -40,7 +44,7 @@ RSpec.describe Security::DependencyListService do ...@@ -40,7 +44,7 @@ RSpec.describe Security::DependencyListService do
let(:params) { { filter: 'vulnerable' } } let(:params) { { filter: 'vulnerable' } }
it 'returns filtered items' do it 'returns filtered items' do
expect(subject.size).to eq(3) expect(subject.size).to eq(2)
expect(subject.last[:vulnerabilities]).not_to be_empty expect(subject.last[:vulnerabilities]).not_to be_empty
end end
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