Merge branch '321086-standalone-vuln-db' into 'master'

Get vulnerabilities from db [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55641

ee/app/models/ee/ci/build.rb

@@ -89,7 +89,7 @@ module EE
 
       def collect_dependency_list_reports!(dependency_list_report)
         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|
             dependency_list.parse!(blob, dependency_list_report)
@@ -101,7 +101,7 @@ module EE
 
       def collect_licenses_for_dependency_list!(dependency_list_report)
         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|
             dependency_list.parse_licenses!(blob, dependency_list_report)

ee/config/feature_flags/development/standalone_vuln_dependency_list.yml

+---
+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

ee/lib/gitlab/ci/parsers/security/dependency_list.rb

@@ -5,8 +5,10 @@ module Gitlab
     module Parsers
       module Security
         class DependencyList
-          def initialize(project, sha)
+          def initialize(project, sha, pipeline)
+            @project = project
             @formatter = Formatters::DependencyList.new(project, sha)
+            @pipeline = pipeline
           end
 
           def parse!(json_data, report)
@@ -24,10 +26,23 @@ module Gitlab
           end
 
           def parse_vulnerabilities(report_data, report)
-            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))
+            if Feature.enabled?(:standalone_vuln_dependency_list, project)
+              vuln_findings = pipeline.vulnerability_findings.dependency_scanning
+              vuln_findings.each do |finding|
+                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
 
@@ -40,7 +55,7 @@ module Gitlab
 
           private
 
-          attr_reader :formatter
+          attr_reader :formatter, :pipeline, :project
         end
       end
     end

ee/spec/controllers/projects/dependencies_controller_spec.rb

@@ -54,7 +54,7 @@ RSpec.describe Projects::DependenciesController do
         end
 
         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
             get :index, params: params, format: :json
@@ -84,6 +84,11 @@ RSpec.describe Projects::DependenciesController do
           end
 
           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
               let(:user) { developer }
 
@@ -138,7 +143,7 @@ RSpec.describe Projects::DependenciesController do
                 let(:user) { developer }
 
                 it 'return vulnerable dependencies' do
-                  expect(json_response['dependencies'].length).to eq(3)
+                  expect(json_response['dependencies'].length).to eq(2)
                 end
               end
             end
@@ -192,17 +197,20 @@ RSpec.describe Projects::DependenciesController do
 
         context 'when report doesn\'t have dependency list field' do
           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
             get :index, params: params, format: :json
           end
 
           it 'returns dependencies with vulnerabilities' do
-            expect(json_response['dependencies'].count).to eq(4)
-            django = json_response['dependencies'].find { |d| d['name'] == 'Django' }
-            expect(django).not_to be_nil
-            expect(django['vulnerabilities']).to eq([{ "name" => "Possible XSS in traceback section of technical 500 debug page", "severity" => "unknown" }])
+            expect(json_response['dependencies'].count).to eq(1)
+            nokogiri = json_response['dependencies'].first
+            expect(nokogiri).not_to be_nil
+            expect(nokogiri['vulnerabilities']).to eq([{ "name" => "Vulnerabilities in libxml2 in nokogiri", "severity" => "high" }])
             expect(json_response['report']['status']).to eq('ok')
           end
         end

ee/spec/factories/vulnerabilities/findings.rb

@@ -450,6 +450,63 @@ FactoryBot.define do
       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
       after(:build) do |finding|
         identifier = build(

ee/spec/lib/gitlab/ci/parsers/security/dependency_list_spec.rb

@@ -3,11 +3,13 @@
 require 'spec_helper'
 
 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(:sha) { '4242424242424242' }
   let(:report) { Gitlab::Ci::Reports::DependencyList::Report.new }
 
+  let_it_be(:pipeline) { create :ee_ci_pipeline, :with_dependency_list_report }
+
   describe '#parse!' do
     before do
       artifact.each_blob do |blob|
@@ -16,7 +18,13 @@ RSpec.describe Gitlab::Ci::Parsers::Security::DependencyList do
     end
 
     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
         blob_path = "/#{project.full_path}/-/blob/#{sha}/yarn/yarn.lock"
@@ -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][:ancestors]).to be_nil
       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
         vuln_nokogiri = report.dependencies[1][:vulnerabilities]
@@ -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_async.size).to eq(0)
       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
 
-    context 'with dependency scanning artifact without dependency_list' do
-      let(:artifact) { create(:ee_ci_job_artifact, :dependency_scanning) }
+    context 'with vulnerabilities in the database' do
+      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(4)
+          expect(report.dependencies.size).to eq(22)
+          expect(giri[:vulnerabilities].size).to eq(1)
+        end
       end
     end
   end

ee/spec/models/ci/build_spec.rb

@@ -330,6 +330,7 @@ RSpec.describe Ci::Build do
 
       before do
         stub_licensed_features(dependency_scanning: true)
+        stub_feature_flags(standalone_vuln_dependency_list: false)
       end
 
       subject { job.collect_dependency_list_reports!(dependency_list_report) }

ee/spec/models/ci/pipeline_spec.rb

@@ -288,7 +288,7 @@ RSpec.describe Ci::Pipeline do
       it 'returns a dependency list report with collected data' do
         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[:licenses]).not_to be_empty
       end

ee/spec/requests/api/dependencies_spec.rb

@@ -19,7 +19,10 @@ RSpec.describe API::Dependencies do
 
     context 'with an authorized user with proper permissions' 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)
         request
       end
@@ -32,10 +35,10 @@ RSpec.describe API::Dependencies do
       end
 
       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['severity']).to eq('unknown')
+        expect(vulnerability['name']).to eq('Vulnerabilities in libxml2 in nokogiri')
+        expect(vulnerability['severity']).to eq('high')
       end
 
       context 'with nil package_manager' do

ee/spec/services/security/dependency_list_service_spec.rb

@@ -4,7 +4,11 @@ require 'spec_helper'
 
 RSpec.describe Security::DependencyListService 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 }
 
@@ -40,7 +44,7 @@ RSpec.describe Security::DependencyListService do
         let(:params) { { filter: 'vulnerable' } }
 
         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
         end
       end