From 98f9e5296dedfb24da694e1ebdf24465546b94c8 Mon Sep 17 00:00:00 2001
From: rossfuhrman <rfuhrman@gitlab.com>
Date: Mon, 12 Aug 2019 20:56:46 -0500
Subject: [PATCH 1/4] Move SAST reports logic for MR widget to backend

Generate SAST reports for the MR widget in the backend
---
 .../ee/projects/merge_requests_controller.rb  |  4 +
 ee/app/models/ee/ci/job_artifact.rb           |  5 ++
 ee/app/models/ee/merge_request.rb             | 12 +++
 .../ci/compare_sast_reports_service.rb        | 21 +++++
 .../projects/merge_requests/show.html.haml    |  7 +-
 ...move-sast-reports-to-mr-widget-backend.yml |  5 ++
 ee/config/routes/project.rb                   |  1 +
 .../reports/security/sast_reports_comparer.rb | 38 ++++++++
 .../merge_requests_controller_spec.rb         | 85 ++++++++++++++++++
 ee/spec/factories/ci/builds.rb                |  6 ++
 ee/spec/factories/ci/job_artifacts.rb         | 10 +++
 ee/spec/factories/ci/pipelines.rb             |  8 ++
 ee/spec/factories/merge_requests.rb           | 12 +++
 .../feature-branch/gl-sast-report.json        | 86 -------------------
 ee/spec/models/merge_request_spec.rb          | 82 +++++++++++++++++-
 ...occurrence_reports_comparer_entity_spec.rb | 45 +++++++---
 ...container_scanning_reports_service_spec.rb |  2 +-
 .../ci/compare_sast_reports_service_spec.rb   | 54 ++++++++++++
 18 files changed, 379 insertions(+), 104 deletions(-)
 create mode 100644 ee/app/services/ci/compare_sast_reports_service.rb
 create mode 100644 ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml
 create mode 100644 ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb
 create mode 100644 ee/spec/services/ci/compare_sast_reports_service_spec.rb

diff --git a/ee/app/controllers/ee/projects/merge_requests_controller.rb b/ee/app/controllers/ee/projects/merge_requests_controller.rb
index ca6ba7f7101633..7a45591e9b365a 100644
--- a/ee/app/controllers/ee/projects/merge_requests_controller.rb
+++ b/ee/app/controllers/ee/projects/merge_requests_controller.rb
@@ -50,6 +50,10 @@ def dependency_scanning_reports
         reports_response(merge_request.compare_dependency_scanning_reports)
       end
 
+      def sast_reports
+        reports_response(merge_request.compare_sast_reports)
+      end
+
       def metrics_reports
         reports_response(merge_request.compare_metrics_reports)
       end
diff --git a/ee/app/models/ee/ci/job_artifact.rb b/ee/app/models/ee/ci/job_artifact.rb
index f61cf856609194..acd9d35e17c239 100644
--- a/ee/app/models/ee/ci/job_artifact.rb
+++ b/ee/app/models/ee/ci/job_artifact.rb
@@ -16,6 +16,7 @@ module Ci::JobArtifact
       DEPENDENCY_LIST_REPORT_FILE_TYPES = %w[dependency_scanning].freeze
       METRICS_REPORT_FILE_TYPES = %w[metrics].freeze
       CONTAINER_SCANNING_REPORT_TYPES = %w[container_scanning].freeze
+      SAST_REPORT_TYPES = %w[sast].freeze
 
       scope :not_expired, -> { where('expire_at IS NULL OR expire_at > ?', Time.current) }
       scope :project_id_in, ->(ids) { joins(:project).merge(::Project.id_in(ids)) }
@@ -38,6 +39,10 @@ module Ci::JobArtifact
         with_file_types(CONTAINER_SCANNING_REPORT_TYPES)
       end
 
+      scope :sast_reports, -> do
+        with_file_types(SAST_REPORT_TYPES)
+      end
+
       scope :metrics_reports, -> do
         with_file_types(METRICS_REPORT_FILE_TYPES)
       end
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index 16ffc4635547d3..e1519f4bb46813 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -140,6 +140,18 @@ def compare_container_scanning_reports
       compare_reports(::Ci::CompareContainerScanningReportsService)
     end
 
+    def has_sast_reports?
+      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.sast_reports)
+    end
+
+    def compare_sast_reports
+      unless has_sast_reports?
+        return { status: :error, status_reason: 'This merge request does not have sast reports' }
+      end
+
+      compare_reports(::Ci::CompareSastReportsService)
+    end
+
     def compare_license_management_reports
       unless has_license_management_reports?
         return { status: :error, status_reason: 'This merge request does not have license management reports' }
diff --git a/ee/app/services/ci/compare_sast_reports_service.rb b/ee/app/services/ci/compare_sast_reports_service.rb
new file mode 100644
index 00000000000000..822e1cc02b45b8
--- /dev/null
+++ b/ee/app/services/ci/compare_sast_reports_service.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module Ci
+  class CompareSastReportsService < ::Ci::CompareReportsBaseService
+    def comparer_class
+      Gitlab::Ci::Reports::Security::SastReportsComparer
+    end
+
+    def serializer_class
+      Vulnerabilities::OccurrenceDiffSerializer
+    end
+
+    def get_report(pipeline)
+      report = pipeline&.security_reports&.get_report('sast')
+
+      raise report.error if report&.errored? # propagate error to base class's execute method
+
+      report
+    end
+  end
+end
diff --git a/ee/app/views/projects/merge_requests/show.html.haml b/ee/app/views/projects/merge_requests/show.html.haml
index e799ca40d6f6a3..c4bd4a55370716 100644
--- a/ee/app/views/projects/merge_requests/show.html.haml
+++ b/ee/app/views/projects/merge_requests/show.html.haml
@@ -16,6 +16,7 @@
     window.gl.mrWidgetData.vulnerability_feedback_help_path = '#{help_page_path("user/application_security/index")}';
     window.gl.mrWidgetData.approvals_help_path = '#{help_page_path("user/project/merge_requests/merge_request_approvals")}';
     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)}'
+    window.gl.mrWidgetData.license_management_comparison_path = '#{license_management_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:license_management)}'
+    window.gl.mrWidgetData.container_scanning_comparison_path = '#{container_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:container_scanning)}'
+    window.gl.mrWidgetData.dependency_scanning_comparison_path = '#{dependency_scanning_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:dependency_scanning)}'
+    window.gl.mrWidgetData.sast_comparison_path = '#{sast_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:sast)}'
diff --git a/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml b/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml
new file mode 100644
index 00000000000000..01665a48f04a4c
--- /dev/null
+++ b/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml
@@ -0,0 +1,5 @@
+---
+title: Present SAST report comparison via API
+merge_request: 15114
+author:
+type: changed
diff --git a/ee/config/routes/project.rb b/ee/config/routes/project.rb
index ba9c7329f43a5f..4bb2e6626945fc 100644
--- a/ee/config/routes/project.rb
+++ b/ee/config/routes/project.rb
@@ -71,6 +71,7 @@
           get :license_management_reports
           get :container_scanning_reports
           get :dependency_scanning_reports
+          get :sast_reports
         end
       end
 
diff --git a/ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb b/ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb
new file mode 100644
index 00000000000000..69e34040e6d0e1
--- /dev/null
+++ b/ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Reports
+      module Security
+        class SastReportsComparer
+          include Gitlab::Utils::StrongMemoize
+
+          attr_reader :base_report, :head_report, :report_diff
+
+          def initialize(base_report, head_report)
+            @base_report = base_report || ::Gitlab::Ci::Reports::Security::Report.new('sast', '')
+            @head_report = head_report
+          end
+
+          def added
+            strong_memoize(:added) do
+              head_report.occurrences - base_report.occurrences
+            end
+          end
+
+          def fixed
+            strong_memoize(:fixed) do
+              base_report.occurrences - head_report.occurrences
+            end
+          end
+
+          def existing
+            strong_memoize(:existing) do
+              base_report.occurrences & head_report.occurrences
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/controllers/projects/merge_requests_controller_spec.rb b/ee/spec/controllers/projects/merge_requests_controller_spec.rb
index e5976ec4cb9a92..25bacf70b73235 100644
--- a/ee/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/ee/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -563,6 +563,91 @@ def expect_rebase_worker_for(user)
     end
   end
 
+  describe 'GET #sast_reports' do
+    let(:merge_request) { create(:ee_merge_request, :with_sast_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 :sast_reports, params: params, format: :json }
+
+    before do
+      allow_any_instance_of(::MergeRequest).to receive(:compare_reports)
+        .with(::Ci::CompareSastReportsService).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 sast 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 sast 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 #license_management_reports' do
     let(:merge_request) { create(:ee_merge_request, :with_license_management_reports, source_project: project, author: create(:user)) }
     let(:params) do
diff --git a/ee/spec/factories/ci/builds.rb b/ee/spec/factories/ci/builds.rb
index 3b9eec924fdbdd..1d7f1010ed4284 100644
--- a/ee/spec/factories/ci/builds.rb
+++ b/ee/spec/factories/ci/builds.rb
@@ -48,6 +48,12 @@
       end
     end
 
+    trait :sast_feature_branch do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :sast_feature_branch, job: build)
+      end
+    end
+
     trait :container_scanning_feature_branch do
       after(:build) do |build|
         build.job_artifacts << create(:ee_ci_job_artifact, :container_scanning_feature_branch, job: build)
diff --git a/ee/spec/factories/ci/job_artifacts.rb b/ee/spec/factories/ci/job_artifacts.rb
index 36f17eb3937a66..dfeb93fb9e8ceb 100644
--- a/ee/spec/factories/ci/job_artifacts.rb
+++ b/ee/spec/factories/ci/job_artifacts.rb
@@ -12,6 +12,16 @@
       end
     end
 
+    trait :sast_feature_branch do
+      file_format :raw
+      file_type :sast
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/security-reports/feature-branch/gl-sast-report.json'), 'application/json')
+      end
+    end
+
     trait :sast_deprecated do
       file_type :sast
       file_format :raw
diff --git a/ee/spec/factories/ci/pipelines.rb b/ee/spec/factories/ci/pipelines.rb
index 3b4bb9a3900157..a159bbcbe1c6c4 100644
--- a/ee/spec/factories/ci/pipelines.rb
+++ b/ee/spec/factories/ci/pipelines.rb
@@ -49,6 +49,14 @@
       end
     end
 
+    trait :with_sast_feature_branch do
+      status :success
+
+      after(:build) do |pipeline, evaluator|
+        pipeline.builds << build(:ee_ci_build, :sast_feature_branch, pipeline: pipeline, project: pipeline.project)
+      end
+    end
+
     trait :with_license_management_feature_branch do
       status :success
 
diff --git a/ee/spec/factories/merge_requests.rb b/ee/spec/factories/merge_requests.rb
index 588e5643d92fd2..c00cfc88e1ea8e 100644
--- a/ee/spec/factories/merge_requests.rb
+++ b/ee/spec/factories/merge_requests.rb
@@ -85,6 +85,18 @@
       end
     end
 
+    trait :with_sast_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ee_ci_pipeline,
+          :success,
+          :with_sast_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(
diff --git a/ee/spec/fixtures/security_reports/feature-branch/gl-sast-report.json b/ee/spec/fixtures/security_reports/feature-branch/gl-sast-report.json
index 4bef3d22f705c6..27c8661013a4d2 100644
--- a/ee/spec/fixtures/security_reports/feature-branch/gl-sast-report.json
+++ b/ee/spec/fixtures/security_reports/feature-branch/gl-sast-report.json
@@ -856,92 +856,6 @@
       "line": 4,
       "url": "https://cwe.mitre.org/data/definitions/119.html",
       "tool": "flawfinder"
-    },
-    {
-      "category": "sast",
-      "message": "Check when opening files - can an attacker redirect it (via symlinks), force the opening of special file type (e.g., device files), move things around to create a race condition, control its ancestors, or change its contents? (CWE-362)",
-      "cve": "c/subdir/utils.c:bab681140fcc8fc3085b6bba74081b44ea145c1c98b5e70cf19ace2417d30770:CWE-362",
-      "confidence": "Low",
-      "scanner": {
-        "id": "flawfinder",
-        "name": "Flawfinder"
-      },
-      "location": {
-        "file": "c/subdir/utils.c",
-        "start_line": 8
-      },
-      "identifiers": [
-        {
-          "type": "cwe",
-          "name": "CWE-362",
-          "value": "362",
-          "url": "https://cwe.mitre.org/data/definitions/362.html"
-        }
-      ],
-      "file": "c/subdir/utils.c",
-      "line": 8,
-      "url": "https://cwe.mitre.org/data/definitions/362.html",
-      "tool": "flawfinder"
-    },
-    {
-      "category": "sast",
-      "message": "Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120)",
-      "cve": "cplusplus/src/hello.cpp:c8c6dd0afdae6814194cf0930b719f757ab7b379cf8f261e7f4f9f2f323a818a:CWE-119!/CWE-120",
-      "confidence": "Low",
-      "solution": "Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length",
-      "scanner": {
-        "id": "flawfinder",
-        "name": "Flawfinder"
-      },
-      "location": {
-        "file": "cplusplus/src/hello.cpp",
-        "start_line": 6
-      },
-      "identifiers": [
-        {
-          "type": "cwe",
-          "name": "CWE-119",
-          "value": "119",
-          "url": "https://cwe.mitre.org/data/definitions/119.html"
-        },
-        {
-          "type": "cwe",
-          "name": "CWE-120",
-          "value": "120",
-          "url": "https://cwe.mitre.org/data/definitions/120.html"
-        }
-      ],
-      "file": "cplusplus/src/hello.cpp",
-      "line": 6,
-      "url": "https://cwe.mitre.org/data/definitions/119.html",
-      "tool": "flawfinder"
-    },
-    {
-      "category": "sast",
-      "message": "Does not check for buffer overflows when copying to destination [MS-banned] (CWE-120)",
-      "cve": "cplusplus/src/hello.cpp:331c04062c4fe0c7c486f66f59e82ad146ab33cdd76ae757ca41f392d568cbd0:CWE-120",
-      "confidence": "Low",
-      "solution": "Consider using snprintf, strcpy_s, or strlcpy (warning: strncpy easily misused)",
-      "scanner": {
-        "id": "flawfinder",
-        "name": "Flawfinder"
-      },
-      "location": {
-        "file": "cplusplus/src/hello.cpp",
-        "start_line": 7
-      },
-      "identifiers": [
-        {
-          "type": "cwe",
-          "name": "CWE-120",
-          "value": "120",
-          "url": "https://cwe.mitre.org/data/definitions/120.html"
-        }
-      ],
-      "file": "cplusplus/src/hello.cpp",
-      "line": 7,
-      "url": "https://cwe.mitre.org/data/definitions/120.html",
-      "tool": "flawfinder"
     }
   ]
 }
diff --git a/ee/spec/models/merge_request_spec.rb b/ee/spec/models/merge_request_spec.rb
index 5fd4edaeb75a85..61ebe026bb6d93 100644
--- a/ee/spec/models/merge_request_spec.rb
+++ b/ee/spec/models/merge_request_spec.rb
@@ -168,7 +168,7 @@
       stub_licensed_features(container_scanning: true)
     end
 
-    context 'when head pipeline has container scannning reports' do
+    context 'when head pipeline has container scanning reports' do
       let(:merge_request) { create(:ee_merge_request, :with_container_scanning_reports, source_project: project) }
 
       it { is_expected.to be_truthy }
@@ -181,6 +181,27 @@
     end
   end
 
+  describe '#has_sast_reports?' do
+    subject { merge_request.has_sast_reports? }
+    let(:project) { create(:project, :repository) }
+
+    before do
+      stub_licensed_features(sast: true)
+    end
+
+    context 'when head pipeline has sast reports' do
+      let(:merge_request) { create(:ee_merge_request, :with_sast_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when head pipeline does not have sast reports' do
+      let(:merge_request) { create(:ee_merge_request, source_project: project) }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+
   describe '#has_metrics_reports?' do
     subject { merge_request.has_metrics_reports? }
     let(:project) { create(:project, :repository) }
@@ -261,6 +282,65 @@
     end
   end
 
+  describe '#compare_sast_reports' do
+    subject { merge_request.compare_sast_reports }
+
+    let(:project) { create(:project, :repository) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
+
+    let!(:base_pipeline) do
+      create(:ee_ci_pipeline,
+             :with_sast_report,
+             project: project,
+             ref: merge_request.target_branch,
+             sha: merge_request.diff_base_sha)
+    end
+
+    before do
+      merge_request.update!(head_pipeline_id: head_pipeline.id)
+    end
+
+    context 'when head pipeline has sast reports' do
+      let!(:head_pipeline) do
+        create(:ee_ci_pipeline,
+               :with_sast_report,
+               project: project,
+               ref: merge_request.source_branch,
+               sha: merge_request.diff_head_sha)
+      end
+
+      context 'when reactive cache worker is parsing asynchronously' do
+        it 'returns status' do
+          expect(subject[:status]).to eq(:parsing)
+        end
+      end
+
+      context 'when reactive cache worker is inline' do
+        before do
+          synchronous_reactive_cache(merge_request)
+        end
+
+        it 'returns status and data' do
+          expect_any_instance_of(Ci::CompareSastReportsService)
+              .to receive(:execute).with(base_pipeline, head_pipeline).and_call_original
+
+          subject
+        end
+
+        context 'when cached results is not latest' do
+          before do
+            allow_any_instance_of(Ci::CompareSastReportsService)
+                .to receive(:latest?).and_return(false)
+          end
+
+          it 'raises and InvalidateReactiveCache error' do
+            expect { subject }.to raise_error(ReactiveCaching::InvalidateReactiveCache)
+          end
+        end
+      end
+    end
+  end
+
   describe '#compare_license_management_reports' do
     subject { merge_request.compare_license_management_reports }
 
diff --git a/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb b/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb
index 708f4a345d1239..76a27c28964bc6 100644
--- a/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb
+++ b/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb
@@ -3,22 +3,41 @@
 require 'spec_helper'
 
 describe Vulnerabilities::OccurrenceReportsComparerEntity do
-  let!(:base_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report) }
-  let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_feature_branch) }
-  let(:base_report) { base_pipeline.security_reports.get_report('container_scanning')}
-  let(:head_report) { head_pipeline.security_reports.get_report('container_scanning')}
-  let(:comparer) { Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(base_report, head_report) }
-  let(:entity) { described_class.new(comparer) }
-
-  before do
-    stub_licensed_features(container_scanning: true)
+  describe 'container scanning report comparison' do
+    let!(:base_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_report) }
+    let!(:head_pipeline) { create(:ee_ci_pipeline, :with_container_scanning_feature_branch) }
+    let(:base_report) { base_pipeline.security_reports.get_report('container_scanning')}
+    let(:head_report) { head_pipeline.security_reports.get_report('container_scanning')}
+    let(:comparer) { Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(base_report, head_report) }
+    let(:entity) { described_class.new(comparer) }
+
+    before do
+      stub_licensed_features(container_scanning: true)
+    end
+
+    describe '#as_json' do
+      subject { entity.as_json }
+
+      it 'contains the added existing and fixed vulnerabilities for container scanning' do
+        expect(subject.keys).to match_array([:added, :existing, :fixed])
+      end
+    end
   end
 
-  describe '#as_json' do
-    subject { entity.as_json }
+  describe 'sast report comparison' do
+    let!(:base_pipeline) { create(:ee_ci_pipeline, :with_sast_report) }
+    let!(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch) }
+    let(:base_report) { base_pipeline.security_reports.get_report('sast')}
+    let(:head_report) { head_pipeline.security_reports.get_report('sast')}
+    let(:comparer) { Gitlab::Ci::Reports::Security::SastReportsComparer.new(base_report, head_report) }
+    let(:entity) { described_class.new(comparer) }
+
+    describe '#as_json' do
+      subject { entity.as_json }
 
-    it 'contains the added existing and fixed vulnerabilities for container scanning' do
-      expect(subject.keys).to match_array([:added, :existing, :fixed])
+      it 'contains the added existing and fixed vulnerabilities for sast' do
+        expect(subject.keys).to match_array([:added, :existing, :fixed])
+      end
     end
   end
 end
diff --git a/ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb b/ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb
index f2ffaaa62addc7..7669a62389eb8f 100644
--- a/ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb
+++ b/ee/spec/services/ci/compare_container_scanning_reports_service_spec.rb
@@ -36,7 +36,7 @@
         expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'CVE-2017-15650'))
       end
 
-      it 'reports existing container vulenerabilities' do
+      it 'reports existing container vulnerabilities' do
         expect(subject[:data]['existing'].count).to eq(0)
       end
 
diff --git a/ee/spec/services/ci/compare_sast_reports_service_spec.rb b/ee/spec/services/ci/compare_sast_reports_service_spec.rb
new file mode 100644
index 00000000000000..0d9228c5ccbeeb
--- /dev/null
+++ b/ee/spec/services/ci/compare_sast_reports_service_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CompareSastReportsService do
+  let(:service) { described_class.new(project) }
+  let(:project) { create(:project, :repository) }
+
+  before do
+    stub_licensed_features(container_scanning: true)
+    stub_licensed_features(sast: true)
+  end
+
+  describe '#execute' do
+    subject { service.execute(base_pipeline, head_pipeline) }
+
+    context 'when head pipeline has sast reports' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_report, project: project) }
+
+      it 'reports new vulnerabilities' do
+        expect(subject[:status]).to eq(:parsed)
+        expect(subject[:data]['added'].count).to eq(33)
+        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 sast reports' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_sast_report, project: project) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch, project: project) }
+
+      it 'reports status as parsed' do
+        expect(subject[:status]).to eq(:parsed)
+      end
+
+      it 'reports new vulnerability' do
+        expect(subject[:data]['added'].count).to eq(1)
+        expect(subject[:data]['added']).to include(a_hash_including('compare_key' => 'c/subdir/utils.c:b466873101951fe96e1332f6728eb7010acbbd5dfc3b65d7d53571d091a06d9e:CWE-119!/CWE-120'))
+      end
+
+      it 'reports existing sast vulnerabilities' do
+        expect(subject[:data]['existing'].count).to eq(29)
+      end
+
+      it 'reports fixed sast vulnerabilities' do
+        expect(subject[:data]['fixed'].count).to eq(4)
+        compare_keys = subject[:data]['fixed'].map { |t| t['compare_key'] }
+        expected_keys = ['c/subdir/utils.c:b466873101951fe96e1332f6728eb7010acbbd5dfc3b65d7d53571d091a06d9e:CWE-119!/CWE-120', 'c/subdir/utils.c:bab681140fcc8fc3085b6bba74081b44ea145c1c98b5e70cf19ace2417d30770:CWE-362', 'cplusplus/src/hello.cpp:c8c6dd0afdae6814194cf0930b719f757ab7b379cf8f261e7f4f9f2f323a818a:CWE-119!/CWE-120', 'cplusplus/src/hello.cpp:331c04062c4fe0c7c486f66f59e82ad146ab33cdd76ae757ca41f392d568cbd0:CWE-120']
+        expect(compare_keys - expected_keys).to eq([])
+      end
+    end
+  end
+end
-- 
GitLab


From af69dcf47836ea789ca4d08308340d1ae7655ea1 Mon Sep 17 00:00:00 2001
From: rossfuhrman <rfuhrman@gitlab.com>
Date: Tue, 13 Aug 2019 15:43:31 -0500
Subject: [PATCH 2/4] Start using new VulnerabilityReportsComparer class

---
 ee/app/services/ci/compare_sast_reports_service.rb              | 2 +-
 .../vulnerabilities/occurrence_reports_comparer_entity_spec.rb  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ee/app/services/ci/compare_sast_reports_service.rb b/ee/app/services/ci/compare_sast_reports_service.rb
index 822e1cc02b45b8..4e90f0f41417a8 100644
--- a/ee/app/services/ci/compare_sast_reports_service.rb
+++ b/ee/app/services/ci/compare_sast_reports_service.rb
@@ -3,7 +3,7 @@
 module Ci
   class CompareSastReportsService < ::Ci::CompareReportsBaseService
     def comparer_class
-      Gitlab::Ci::Reports::Security::SastReportsComparer
+      Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer
     end
 
     def serializer_class
diff --git a/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb b/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb
index 76a27c28964bc6..b1fe51cd670949 100644
--- a/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb
+++ b/ee/spec/serializers/vulnerabilities/occurrence_reports_comparer_entity_spec.rb
@@ -29,7 +29,7 @@
     let!(:head_pipeline) { create(:ee_ci_pipeline, :with_sast_feature_branch) }
     let(:base_report) { base_pipeline.security_reports.get_report('sast')}
     let(:head_report) { head_pipeline.security_reports.get_report('sast')}
-    let(:comparer) { Gitlab::Ci::Reports::Security::SastReportsComparer.new(base_report, head_report) }
+    let(:comparer) { Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer.new(base_report, head_report) }
     let(:entity) { described_class.new(comparer) }
 
     describe '#as_json' do
-- 
GitLab


From 5d60508a4996f380dc142b3a1bbedebdca16951e Mon Sep 17 00:00:00 2001
From: rossfuhrman <rfuhrman@gitlab.com>
Date: Thu, 15 Aug 2019 12:40:52 -0500
Subject: [PATCH 3/4] Address backend reviewer feedback

Removed unused file
Made changelog message clearer
Updated sast to SAST in user-visible message
---
 ee/app/models/ee/merge_request.rb             |  2 +-
 ...move-sast-reports-to-mr-widget-backend.yml |  2 +-
 .../reports/security/sast_reports_comparer.rb | 38 -------------------
 3 files changed, 2 insertions(+), 40 deletions(-)
 delete mode 100644 ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb

diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index e1519f4bb46813..bbf40a863b2ea1 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -146,7 +146,7 @@ def has_sast_reports?
 
     def compare_sast_reports
       unless has_sast_reports?
-        return { status: :error, status_reason: 'This merge request does not have sast reports' }
+        return { status: :error, status_reason: 'This merge request does not have SAST reports' }
       end
 
       compare_reports(::Ci::CompareSastReportsService)
diff --git a/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml b/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml
index 01665a48f04a4c..0baadf459eba1c 100644
--- a/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml
+++ b/ee/changelogs/unreleased/rf-move-sast-reports-to-mr-widget-backend.yml
@@ -1,5 +1,5 @@
 ---
-title: Present SAST report comparison via API
+title: Present SAST report comparison logic to backend
 merge_request: 15114
 author:
 type: changed
diff --git a/ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb b/ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb
deleted file mode 100644
index 69e34040e6d0e1..00000000000000
--- a/ee/lib/gitlab/ci/reports/security/sast_reports_comparer.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
-  module Ci
-    module Reports
-      module Security
-        class SastReportsComparer
-          include Gitlab::Utils::StrongMemoize
-
-          attr_reader :base_report, :head_report, :report_diff
-
-          def initialize(base_report, head_report)
-            @base_report = base_report || ::Gitlab::Ci::Reports::Security::Report.new('sast', '')
-            @head_report = head_report
-          end
-
-          def added
-            strong_memoize(:added) do
-              head_report.occurrences - base_report.occurrences
-            end
-          end
-
-          def fixed
-            strong_memoize(:fixed) do
-              base_report.occurrences - head_report.occurrences
-            end
-          end
-
-          def existing
-            strong_memoize(:existing) do
-              base_report.occurrences & head_report.occurrences
-            end
-          end
-        end
-      end
-    end
-  end
-end
-- 
GitLab


From 4f0a3f1f527a16b6e72376e729f93b83fb49190c Mon Sep 17 00:00:00 2001
From: rossfuhrman <rfuhrman@gitlab.com>
Date: Mon, 19 Aug 2019 15:40:52 -0500
Subject: [PATCH 4/4] Fix incorrect path due to incorrect merge

---
 ee/spec/factories/ci/job_artifacts.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/spec/factories/ci/job_artifacts.rb b/ee/spec/factories/ci/job_artifacts.rb
index dfeb93fb9e8ceb..5eda63dd04c5f7 100644
--- a/ee/spec/factories/ci/job_artifacts.rb
+++ b/ee/spec/factories/ci/job_artifacts.rb
@@ -18,7 +18,7 @@
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('spec/fixtures/security-reports/feature-branch/gl-sast-report.json'), 'application/json')
+          Rails.root.join('ee/spec/fixtures/security_reports/feature-branch/gl-sast-report.json'), 'application/json')
       end
     end
 
-- 
GitLab