Merge branch '12005-move-ds-reports-to-mr-widget-backend' into 'master'

Move dependency scanning reports to mr widget backend

See merge request gitlab-org/gitlab-ee!15023

ee/app/controllers/ee/projects/merge_requests_controller.rb

@@ -46,6 +46,10 @@ module EE
         reports_response(merge_request.compare_container_scanning_reports)
       end
 
+      def dependency_scanning_reports
+        reports_response(merge_request.compare_dependency_scanning_reports)
+      end
+
       def metrics_reports
         reports_response(merge_request.compare_metrics_reports)
       end

ee/app/models/ee/merge_request.rb

@@ -112,6 +112,18 @@ module EE
       end
     end
 
+    def has_dependency_scanning_reports?
+      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dependency_list_reports)
+    end
+
+    def compare_dependency_scanning_reports
+      unless has_dependency_scanning_reports?
+        return { status: :error, status_reason: 'This merge request does not have dependency scanning reports' }
+      end
+
+      compare_reports(::Ci::CompareDependencyScanningReportsService)
+    end
+
     def has_license_management_reports?
       actual_head_pipeline&.has_reports?(::Ci::JobArtifact.license_management_reports)
     end

ee/app/services/ci/compare_dependency_scanning_reports_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CompareDependencyScanningReportsService < ::Ci::CompareReportsBaseService
+    def comparer_class
+      Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer
+    end
+
+    def serializer_class
+      Vulnerabilities::OccurrenceDiffSerializer
+    end
+
+    def get_report(pipeline)
+      report = pipeline&.security_reports&.get_report('dependency_scanning')
+
+      raise report.error if report&.errored? # propagate error to base class's execute method
+
+      report
+    end
+  end
+end

ee/app/views/projects/merge_requests/show.html.haml

@@ -18,3 +18,4 @@
     window.gl.mrWidgetData.visual_review_app_available = '#{@project.feature_available?(:visual_review_app)}' === 'true';
     window.gl.mrWidgetData.license_management_comparsion_path = '#{license_management_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_management)}'
     window.gl.mrWidgetData.container_scanning_comparsion_path = '#{container_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:container_scanning)}'
+    window.gl.mrWidgetData.dependency_scanning_comparsion_path = '#{dependency_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:dependency_scanning)}'

ee/changelogs/unreleased/12005-move-ds-reports-to-mr-widget-backend.yml

+---
+title: Move dependency scanning comparision logic to backend
+merge_request: 15023
+author:
+type: changed

ee/config/routes/project.rb

@@ -70,6 +70,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           get :metrics_reports
           get :license_management_reports
           get :container_scanning_reports
+          get :dependency_scanning_reports
         end
       end
 

ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -393,6 +393,91 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'GET #dependency_scanning_reports' do
+    let(:merge_request) { create(:ee_merge_request, :with_dependency_scanning_reports, source_project: project, author: create(:user)) }
+    let(:params) do
+      {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid
+      }
+    end
+
+    subject { get :dependency_scanning_reports, params: params, format: :json }
+
+    before do
+      allow_any_instance_of(::MergeRequest).to receive(:compare_reports)
+        .with(::Ci::CompareDependencyScanningReportsService).and_return(comparison_status)
+    end
+
+    context 'when comparison is being processed' do
+      let(:comparison_status) { { status: :parsing } }
+
+      it 'sends polling interval' do
+        expect(::Gitlab::PollingInterval).to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 204 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+    end
+
+    context 'when comparison is done' do
+      let(:comparison_status) { { status: :parsed, data: { added: [], fixed: [], existing: [] } } }
+
+      it 'does not send polling interval' do
+        expect(::Gitlab::PollingInterval).not_to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 200 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to eq({ "added" => [], "fixed" => [], "existing" => [] })
+      end
+    end
+
+    context 'when user created corrupted vulnerability reports' do
+      let(:comparison_status) { { status: :error, status_reason: 'Failed to parse container scanning reports' } }
+
+      it 'does not send polling interval' do
+        expect(::Gitlab::PollingInterval).not_to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 400 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response).to eq({ 'status_reason' => 'Failed to parse container scanning reports' })
+      end
+    end
+
+    context 'when something went wrong on our system' do
+      let(:comparison_status) { {} }
+
+      it 'does not send polling interval' do
+        expect(::Gitlab::PollingInterval).not_to receive(:set_header)
+
+        subject
+      end
+
+      it 'returns 500 HTTP status' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:internal_server_error)
+        expect(json_response).to eq({ 'status_reason' => 'Unknown error' })
+      end
+    end
+  end
+
   describe 'GET #container_scanning_reports' do
     let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project, author: create(:user)) }
     let(:params) do

ee/spec/factories/ci/builds.rb

@@ -60,6 +60,24 @@ FactoryBot.define do
       end
     end
 
+    trait :dependency_scanning_feature_branch do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :dependency_scanning_feature_branch, job: build)
+      end
+    end
+
+    trait :dependency_scanning_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :dependency_scanning_report, job: build)
+      end
+    end
+
+    trait :corrupted_dependency_scanning_report do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :corrupted_dependency_scanning_report, job: build)
+      end
+    end
+
     trait :license_management_feature_branch do
       after(:build) do |build|
         build.job_artifacts << create(:ee_ci_job_artifact, :license_management_feature_branch, job: build)

ee/spec/factories/ci/job_artifacts.rb

@@ -112,6 +112,26 @@ FactoryBot.define do
       end
     end
 
+    trait :dependency_scanning_feature_branch do
+      file_format :raw
+      file_type :dependency_scanning
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security-reports/feature-branch/gl-dependency-scanning-report.json'), 'application/json')
+      end
+    end
+
+    trait :corrupted_dependency_scanning_report do
+      file_format :raw
+      file_type :dependency_scanning
+
+      after(:build) do |artifact, evaluator|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
+      end
+    end
+
     trait :container_scanning do
       file_format :raw
       file_type :container_scanning

ee/spec/factories/ci/pipelines.rb

@@ -33,6 +33,22 @@ FactoryBot.define do
       end
     end
 
+    trait :with_dependency_scanning_feature_branch do
+      status :success
+
+      after(:build) do |pipeline, evaluator|
+        pipeline.builds << build(:ee_ci_build, :dependency_scanning_feature_branch, pipeline: pipeline, project: pipeline.project)
+      end
+    end
+
+    trait :with_corrupted_dependency_scanning_report do
+      status :success
+
+      after(:build) do |pipeline, evaluator|
+        pipeline.builds << build(:ee_ci_build, :corrupted_dependency_scanning_report, pipeline: pipeline, project: pipeline.project)
+      end
+    end
+
     trait :with_license_management_feature_branch do
       status :success
 

ee/spec/factories/merge_requests.rb

@@ -73,6 +73,18 @@ FactoryBot.define do
       end
     end
 
+    trait :with_dependency_scanning_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ee_ci_pipeline,
+          :success,
+          :with_dependency_scanning_report,
+          project: merge_request.source_project,
+          ref: merge_request.source_branch,
+          sha: merge_request.diff_head_sha)
+      end
+    end
+
     trait :with_metrics_reports do
       after(:build) do |merge_request|
         merge_request.head_pipeline = build(

ee/spec/fixtures/security-reports/feature-branch/gl-dependency-scanning-report.json

+{
+    "version": "1.3",
+    "vulnerabilities": [
+      {
+        "category": "dependency_scanning",
+        "name": "io.netty/netty - CVE-2014-3488",
+        "message": "DoS by CPU exhaustion when using malicious SSL packets",
+        "cve": "app/pom.xml:io.netty/netty@3.9.1.Final:CVE-2014-3488",
+        "severity": "Unknown",
+        "solution": "Upgrade to the latest version",
+        "scanner": {
+          "id": "gemnasium",
+          "name": "Gemnasium"
+        },
+        "location": {
+          "file": "app/pom.xml",
+          "dependency": {
+            "package": {
+              "name": "io.netty/netty"
+            },
+            "version": "3.9.1.Final"
+          }
+        },
+        "identifiers": [
+          {
+            "type": "gemnasium",
+            "name": "Gemnasium-d1bf36d9-9f07-46cd-9cfc-8675338ada8f",
+            "value": "d1bf36d9-9f07-46cd-9cfc-8675338ada8f",
+            "url": "https://deps.sec.gitlab.com/packages/maven/io.netty/netty/versions/3.9.1.Final/advisories"
+          },
+          {
+            "type": "cve",
+            "name": "CVE-2014-3488",
+            "value": "CVE-2014-3488",
+            "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2014-3488"
+          }
+        ],
+        "links": [
+          {
+            "url": "https://bugzilla.redhat.com/CVE-2014-3488"
+          },
+          {
+            "url": "http://netty.io/news/2014/06/11/3.html"
+          },
+          {
+            "url": "https://github.com/netty/netty/issues/2562"
+          }
+        ],
+        "priority": "Unknown",
+        "file": "app/pom.xml",
+        "url": "https://bugzilla.redhat.com/CVE-2014-3488",
+        "tool": "gemnasium"
+      },
+      {
+        "category": "dependency_scanning",
+        "name": "Django - CVE-2017-12794",
+        "message": "Possible XSS in traceback section of technical 500 debug page",
+        "cve": "app/requirements.txt:Django@1.11.3:CVE-2017-12794",
+        "severity": "Unknown",
+        "solution": "Upgrade to latest version or apply patch.",
+        "scanner": {
+          "id": "gemnasium",
+          "name": "Gemnasium"
+        },
+        "location": {
+          "file": "app/requirements.txt",
+          "dependency": {
+            "package": {
+              "name": "Django"
+            },
+            "version": "1.11.3"
+          }
+        },
+        "identifiers": [
+          {
+            "type": "gemnasium",
+            "name": "Gemnasium-6162a015-8635-4a15-8d7c-dc9321db366f",
+            "value": "6162a015-8635-4a15-8d7c-dc9321db366f",
+            "url": "https://deps.sec.gitlab.com/packages/pypi/Django/versions/1.11.3/advisories"
+          },
+          {
+            "type": "cve",
+            "name": "CVE-2017-12794",
+            "value": "CVE-2017-12794",
+            "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-12794"
+          }
+        ],
+        "links": [
+          {
+            "url": "https://www.djangoproject.com/weblog/2017/sep/05/security-releases/"
+          }
+        ],
+        "priority": "Unknown",
+        "file": "app/requirements.txt",
+        "url": "https://www.djangoproject.com/weblog/2017/sep/05/security-releases/",
+        "tool": "gemnasium"
+      },
+      {
+        "category": "dependency_scanning",
+        "message": "Directory traversal vulnerability in rubyzip",
+        "cve": "Gemfile.lock:rubyzip:cve:CVE-2017-5946",
+        "severity": "High",
+        "solution": "upgrade to \u003e= 1.2.1",
+        "scanner": {
+          "id": "bundler_audit",
+          "name": "bundler-audit"
+        },
+        "location": {
+          "file": "Gemfile.lock",
+          "dependency": {
+            "package": {
+              "name": "rubyzip"
+            },
+            "version": "1.2.0"
+          }
+        },
+        "identifiers": [
+          {
+            "type": "cve",
+            "name": "CVE-2017-5946",
+            "value": "CVE-2017-5946",
+            "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-5946"
+          }
+        ],
+        "links": [
+          {
+            "url": "https://github.com/rubyzip/rubyzip/issues/315"
+          }
+        ]
+      },
+      {
+        "category": "dependency_scanning",
+        "name": "ffi - CVE-2018-1000201",
+        "message": "ruby-ffi DDL loading issue on Windows OS",
+        "cve": "ffi:1.9.18:CVE-2018-1000201",
+        "severity": "High",
+        "solution": "upgrade to \u003e= 1.9.24",
+        "scanner": {
+          "id": "bundler_audit",
+          "name": "bundler-audit"
+        },
+        "location": {
+          "file": "sast-sample-rails/Gemfile.lock",
+          "dependency": {
+            "package": {
+              "name": "ffi"
+            },
+            "version": "1.9.18"
+          }
+        },
+        "identifiers": [
+          {
+            "type": "cve",
+            "name": "CVE-2018-1000201",
+            "value": "CVE-2018-1000201",
+            "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000201"
+          }
+        ],
+        "links": [
+          {
+            "url": "https://github.com/ffi/ffi/releases/tag/1.9.24"
+          }
+        ],
+        "priority": "High",
+        "file": "sast-sample-rails/Gemfile.lock",
+        "url": "https://github.com/ffi/ffi/releases/tag/1.9.24",
+        "tool": "bundler_audit"
+      }
+    ]
+  }
\ No newline at end of file

ee/spec/models/merge_request_spec.rb

@@ -139,6 +139,27 @@ describe MergeRequest do
     end
   end
 
+  describe '#has_dependency_scanning_reports?' do
+    subject { merge_request.has_dependency_scanning_reports? }
+    let(:project) { create(:project, :repository) }
+
+    before do
+      stub_licensed_features(container_scanning: true)
+    end
+
+    context 'when head pipeline has dependency scannning reports' do
+      let(:merge_request) { create(:ee_merge_request, :with_dependency_scanning_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when head pipeline does not have dependency scanning reports' do
+      let(:merge_request) { create(:ee_merge_request, source_project: project) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#has_container_scanning_reports?' do
     subject { merge_request.has_container_scanning_reports? }
     let(:project) { create(:project, :repository) }

ee/spec/services/ci/compare_dependency_scanning_reports_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CompareDependencyScanningReportsService do
+  let(:service) { described_class.new(project) }
+  let(:project) { create(:project, :repository) }
+
+  before do
+    stub_licensed_features(dependency_scanning: true)
+  end
+
+  describe '#execute' do
+    subject { service.execute(base_pipeline, head_pipeline) }
+
+    context 'when head pipeline has dependency scanning reports' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
+
+      it 'reports new vulnerabilities' do
+        expect(subject[:status]).to eq(:parsed)
+        expect(subject[:data]['added'].count).to eq(4)
+        expect(subject[:data]['existing'].count).to eq(0)
+        expect(subject[:data]['fixed'].count).to eq(0)
+      end
+    end
+
+    context 'when base and head pipelines have dependency scanning reports' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_feature_branch, project: project) }
+
+      it 'reports status as parsed' do
+        expect(subject[:status]).to eq(:parsed)
+      end
+
+      it 'reports fixed vulnerability' do
+        expect(subject[:data]['added'].count).to eq(1)
+        expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'Gemfile.lock:rubyzip:cve:CVE-2017-5946'))
+      end
+
+      it 'reports existing dependency vulenerabilities' do
+        expect(subject[:data]['existing'].count).to eq(3)
+      end
+
+      it 'reports fixed dependency scanning vulnerabilities' do
+        expect(subject[:data]['fixed'].count).to eq(1)
+        compare_keys = subject[:data]['fixed'].map { |t| t['compare_key'] }
+        expected_keys = %w(rails/Gemfile.lock:nokogiri@1.8.0:USN-3424-1)
+        expect(compare_keys - expected_keys).to eq([])
+      end
+    end
+
+    context 'when head pipeline has corrupted dependency scanning vulnerability reports' do
+      let!(:base_pipeline) { nil }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_corrupted_dependency_scanning_report, project: project) }
+
+      it 'returns status and error message' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:status_reason]).to include('JSON parsing failed')
+      end
+    end
+  end
+end