Merge branch '214095-standalone-vuln-backend' into 'master'

Standalone vulnerabilities on Dependency List page - backend

See merge request gitlab-org/gitlab!56394

ee/app/serializers/dependency_entity.rb

@@ -13,7 +13,7 @@ class DependencyEntity < Grape::Entity
   end
 
   class VulnerabilityEntity < Grape::Entity
-    expose :name, :severity
+    expose :name, :severity, :id, :url
   end
 
   class LicenseEntity < Grape::Entity

ee/lib/ee/api/entities/dependency/vulnerability.rb

@@ -5,7 +5,7 @@ module EE
     module Entities
       class Dependency < Grape::Entity
         class Vulnerability < Grape::Entity
-          expose :name, :severity
+          expose :name, :severity, :id, :url
         end
       end
     end

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

@@ -34,7 +34,8 @@ module Gitlab
                 next unless dependency
 
                 file = finding.file
-                vulnerability = finding.metadata
+                vulnerability = finding.metadata.merge(vulnerability_id: finding.vulnerability_id)
+
                 report.add_dependency(formatter.format(dependency, '', file, vulnerability))
               end
             else

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

@@ -79,7 +79,15 @@ module Gitlab
             def formatted_vulnerabilities(vulnerabilities)
               return [] if vulnerabilities.blank?
 
-              [{ name: vulnerabilities['message'], severity: vulnerabilities['severity'].downcase }]
+              vuln_params = { name: vulnerabilities['message'], severity: vulnerabilities['severity'].downcase }
+
+              if Feature.enabled?(:standalone_vuln_dependency_list, project)
+                id = vulnerabilities[:vulnerability_id]
+                standalone_vuln_params = { id: id, url: vulnerability_url(id) }
+                vuln_params.merge!(standalone_vuln_params)
+              end
+
+              [vuln_params]
             end
 
             # Dependency List report is generated by dependency_scanning job.
@@ -96,6 +104,10 @@ module Gitlab
                 }
               }
             end
+
+            def vulnerability_url(id)
+              ::Gitlab::Routing.url_helpers.project_security_vulnerability_url(project, id)
+            end
           end
         end
       end

ee/lib/gitlab/ci/reports/dependency_list/vulnerability.rb

@@ -5,11 +5,13 @@ module Gitlab
     module Reports
       module DependencyList
         class Vulnerability
-          attr_reader :name, :severity
+          attr_reader :name, :severity, :id, :url
 
           def initialize(params)
             @name = params.fetch(:name)
             @severity = params.fetch(:severity)
+            @id = params.fetch(:id, nil)
+            @url = params.fetch(:url, nil)
           end
 
           def ==(other)
@@ -23,7 +25,9 @@ module Gitlab
           def to_hash
             {
               name: self.name,
-              severity: self.severity
+              severity: self.severity,
+              id: self.id,
+              url: self.url
             }
           end
 

ee/spec/controllers/projects/dependencies_controller_spec.rb

@@ -84,9 +84,9 @@ 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) { create(:vulnerabilities_finding, :detected, :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_finding) { create(:vulnerabilities_finding, :detected, :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
@@ -145,6 +145,16 @@ RSpec.describe Projects::DependenciesController do
                 it 'return vulnerable dependencies' do
                   expect(json_response['dependencies'].length).to eq(2)
                 end
+
+                it 'returns vulnerability params' do
+                  dependency = json_response['dependencies'].select { |dep| dep['name'] == 'nokogiri' }.first
+                  vulnerability = dependency['vulnerabilities'].first
+                  path = "/security/vulnerabilities/#{finding.vulnerability_id}"
+
+                  expect(vulnerability['name']).to eq('Vulnerabilities in libxml2 in nokogiri')
+                  expect(vulnerability['id']).to eq(finding.vulnerability_id)
+                  expect(vulnerability['url']).to end_with(path)
+                end
               end
             end
 
@@ -199,7 +209,7 @@ RSpec.describe Projects::DependenciesController do
           let(:user) { developer }
 
           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) { create(:vulnerabilities_finding, :detected, :with_dependency_scanning_metadata, :with_pipeline) }
           let_it_be(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline) }
 
           before do
@@ -210,7 +220,7 @@ RSpec.describe Projects::DependenciesController do
             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(nokogiri['vulnerabilities'].first).to include({ "id" => finding.vulnerability_id, "name" => "Vulnerabilities in libxml2 in nokogiri", "severity" => "high" })
             expect(json_response['report']['status']).to eq('ok')
           end
         end

ee/spec/factories/dependencies.rb

@@ -22,12 +22,16 @@ FactoryBot.define do
     trait :with_vulnerabilities do
       vulnerabilities do
         [{
-           name:     'DDoS',
-           severity: 'high'
+           name: 'DDoS',
+           severity: 'high',
+           id: 42,
+           url: 'http://gitlab.org/some-group/some-project/-/security/vulnerabilities/42'
          },
          {
            name:     'XSS vulnerability',
-           severity: 'low'
+           severity: 'low',
+           id: 1729,
+           url: 'http://gitlab.org/some-group/some-project/-/security/vulnerabilities/1729'
          }]
       end
     end

ee/spec/factories/vulnerabilities/findings.rb

@@ -473,6 +473,7 @@ FactoryBot.define do
       after(:build) do |finding, evaluator|
         finding.report_type = "dependency_scanning"
         finding.name = "Vulnerabilities in libxml2"
+        finding.message = "Vulnerabilities in libxml2 in nokogiri"
         finding.metadata_version = "2.1"
         finding.raw_metadata = {
           "category": "dependency_scanning",

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

@@ -68,14 +68,40 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Formatters::DependencyList do
     end
 
     context 'with vulnerable dependency' do
-      let(:data) { formatter.format(dependency, package_manager, file_path, parsed_report['vulnerabilities'].first) }
       let(:dependency) { parsed_report['dependency_files'][0]['dependencies'][1] }
+      let(:data) { formatter.format(dependency, package_manager, file_path, vulnerability_data) }
 
-      it 'merge vulnerabilities data' do
-        vulnerabilities = data[:vulnerabilities]
+      context 'with feature `standalone vulnerabilities` enabled' do
+        let_it_be(:standalone_vulnerability) { create(:vulnerability, report_type: :dependency_scanning) }
 
-        expect(vulnerabilities.first[:name]).to eq('Vulnerabilities in libxml2 in nokogiri')
-        expect(vulnerabilities.first[:severity]).to eq('high')
+        let(:vulnerability_data) do
+          create(:vulnerabilities_finding, :with_dependency_scanning_metadata, vulnerability: standalone_vulnerability)
+        end
+
+        it 'merge vulnerabilities data' do
+          vulnerability = data[:vulnerabilities].first
+          path = "/security/vulnerabilities/#{standalone_vulnerability.id}"
+
+          expect(vulnerability[:id]).to eq(standalone_vulnerability.id)
+          expect(vulnerability[:url]).to end_with(path)
+          expect(vulnerability[:name]).to eq('Vulnerabilities in libxml2 in nokogiri')
+          expect(vulnerability[:severity]).to eq('high')
+        end
+      end
+
+      context 'with disabled feature' do
+        let(:vulnerability_data) { parsed_report['vulnerabilities'].first }
+
+        before do
+          stub_feature_flags(standalone_vuln_dependency_list: false)
+        end
+
+        it 'merge vulnerabilities data' do
+          vulnerability = data[:vulnerabilities].first
+
+          expect(vulnerability[:name]).to eq('Vulnerabilities in libxml2 in nokogiri')
+          expect(vulnerability[:severity]).to eq('high')
+        end
       end
     end
   end

ee/spec/lib/gitlab/ci/reports/dependency_list/dependency_spec.rb

@@ -16,17 +16,14 @@ RSpec.describe Gitlab::Ci::Reports::DependencyList::Dependency do
         top_level: true
       },
       licenses: [],
-      vulnerabilities: [{
-                          name: 'DDoS',
-                          severity: 'high'
-                        },
-                        {
-                          name: 'XSS vulnerability',
-                          severity: 'low'
-                        }]
+      vulnerabilities: [ddos_vuln, xss_vuln]
     }
   end
 
+  let(:ddos_vuln) { { name: 'DDoS', severity: 'high', id: 12, url: 'some_url_12' } }
+  let(:xss_vuln) { { name: 'XSS vulnerability', severity: 'low', id: 4, url: 'some_url_4' } }
+  let(:problem_vuln) { { name: 'problem', severity: 'high', id: 3, url: 'some_url_3' } }
+
   context 'initialize' do
     it 'sets all required properties' do
       dep = described_class.new(dependency_nokogiri)
@@ -37,35 +34,30 @@ RSpec.describe Gitlab::Ci::Reports::DependencyList::Dependency do
                                   location: { blob_path: '/some_project/path/package_file.lock', path: 'package_file.lock', top_level: true, ancestors: nil },
                                   version: '1.8.0',
                                   licenses: [],
-                                  vulnerabilities: [{ name: 'DDoS', severity: 'high' }, { name: 'XSS vulnerability', severity: 'low' }] })
+                                  vulnerabilities: [ddos_vuln, xss_vuln] })
     end
 
     it 'keeps vulnerabilities that are not duplicates' do
-      dependency_nokogiri[:vulnerabilities] << { name: 'problem', severity: 'high' }
+      dependency_nokogiri[:vulnerabilities] << problem_vuln
       dep = described_class.new(dependency_nokogiri)
 
-      expect(dep.vulnerabilities.to_a.map(&:to_hash)).to eq([{ name: 'DDoS', severity: 'high' },
-                                                             { name: 'XSS vulnerability', severity: 'low' },
-                                                             { name: 'problem', severity: 'high' }])
+      expect(dep.vulnerabilities.to_a.map(&:to_hash)).to eq([ddos_vuln, xss_vuln, problem_vuln])
     end
 
     it 'removes vulnerability duplicates' do
-      dependency_nokogiri[:vulnerabilities] << { name: 'DDoS', severity: 'high' }
+      dependency_nokogiri[:vulnerabilities] << ddos_vuln
       dep = described_class.new(dependency_nokogiri)
 
-      expect(dep.vulnerabilities.to_a.map(&:to_hash)).to eq([{ name: 'DDoS', severity: 'high' },
-                                                             { name: 'XSS vulnerability', severity: 'low' }])
+      expect(dep.vulnerabilities.to_a.map(&:to_hash)).to eq([ddos_vuln, xss_vuln])
     end
   end
 
   context 'update dependency' do
     specify do
-      dependency_nokogiri[:vulnerabilities] << { name: 'DDoS', severity: 'high' } << { name: 'problem', severity: 'high' }
+      dependency_nokogiri[:vulnerabilities] << ddos_vuln << problem_vuln
       dep = described_class.new(dependency_nokogiri)
 
-      expect(dep.vulnerabilities.to_a.map(&:to_hash)).to eq([{ name: 'DDoS', severity: 'high' },
-                                                             { name: 'XSS vulnerability', severity: 'low' },
-                                                             { name: 'problem', severity: 'high' }])
+      expect(dep.vulnerabilities.to_a.map(&:to_hash)).to eq([ddos_vuln, xss_vuln, problem_vuln])
     end
   end
 end

ee/spec/lib/gitlab/ci/reports/dependency_list/report_spec.rb

@@ -134,7 +134,9 @@ RSpec.describe Gitlab::Ci::Reports::DependencyList::Report do
     end
 
     it 'does not duplicate same vulnerability for dependency' do
-      vulnerabilities = [{ name: 'problem', severity: 'high' }, { name: 'problem2', severity: 'medium' }]
+      vulnerabilities = [{ name: 'problem', severity: 'high', id: 2, url: 'some_url_2' },
+                         { name: 'problem2', severity: 'medium', id: 4, url: 'some_url_4' }]
+
       dependency[:vulnerabilities] = [vulnerabilities.first]
       with_extra_vuln_from_another_report = dependency.dup.merge(vulnerabilities: vulnerabilities)
 
@@ -145,12 +147,12 @@ RSpec.describe Gitlab::Ci::Reports::DependencyList::Report do
 
     it 'stores a dependency' do
       dependency[:packager] = 'Ruby (Bundler)'
-      dependency[:vulnerabilities] = [{ name: 'abc', severity: 'high' }]
+      dependency[:vulnerabilities] = [{ name: 'abc', severity: 'high', id: 5, url: 'some_url_5' }]
 
       report.add_dependency(dependency)
       expect(report.dependencies.size).to eq(1)
       expect(report.dependencies.first[:packager]).to eq('Ruby (Bundler)')
-      expect(report.dependencies.first[:vulnerabilities]).to eq([{ name: 'abc', severity: 'high' }])
+      expect(report.dependencies.first[:vulnerabilities]).to eq([{ name: 'abc', severity: 'high', id: 5, url: 'some_url_5' }])
     end
   end
 

ee/spec/requests/api/dependencies_spec.rb

@@ -18,11 +18,11 @@ RSpec.describe API::Dependencies do
     it_behaves_like 'a gitlab tracking event', described_class.name, 'view_dependencies'
 
     context 'with an authorized user with proper permissions' do
-      before do
-        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)
+      let_it_be(:finding) { create(:vulnerabilities_finding, :detected, :with_dependency_scanning_metadata) }
+      let_it_be(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
+      let_it_be(:finding_pipeline) { create(:vulnerabilities_finding_pipeline, finding: finding, pipeline: pipeline) }
 
+      before do
         project.add_developer(user)
         request
       end
@@ -36,9 +36,12 @@ RSpec.describe API::Dependencies do
 
       it 'returns vulnerabilities info' do
         vulnerability = json_response.select { |dep| dep['name'] == 'nokogiri' }[0]['vulnerabilities'][0]
+        path = "/security/vulnerabilities/#{finding.vulnerability_id}"
 
         expect(vulnerability['name']).to eq('Vulnerabilities in libxml2 in nokogiri')
         expect(vulnerability['severity']).to eq('high')
+        expect(vulnerability['id']).to eq(finding.vulnerability_id)
+        expect(vulnerability['url']).to end_with(path)
       end
 
       context 'with nil package_manager' do

ee/spec/services/security/dependency_list_service_spec.rb

@@ -5,9 +5,9 @@ require 'spec_helper'
 RSpec.describe Security::DependencyListService do
   describe '#execute' do
     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_finding) { create(:vulnerabilities_finding, :detected, :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_finding) { create(:vulnerabilities_finding, :detected, :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 }