Merge branch '12003-move-dast-reports-logic-backend' into 'master'

Move DAST reports logic for the Merge Request widget to the backend

See merge request gitlab-org/gitlab!18660

app/controllers/projects/merge_requests_controller.rb

@@ -364,7 +364,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     when :error
       render json: { status_reason: report_comparison[:status_reason] }, status: :bad_request
     else
-      render json: { status_reason: 'Unknown error' }, status: :internal_server_error
+      raise "Failed to build comparison response as comparison yielded unknown status '#{report_comparison[:status]}'"
     end
   end
 

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

@@ -18,7 +18,7 @@ module EE
 
         before_action :whitelist_query_limiting_ee_merge, only: [:merge]
         before_action :whitelist_query_limiting_ee_show, only: [:show]
-        before_action :authorize_read_pipeline!, only: [:container_scanning_reports, :dependency_scanning_reports, :sast_reports]
+        before_action :authorize_read_pipeline!, only: [:container_scanning_reports, :dependency_scanning_reports, :sast_reports, :dast_reports]
       end
 
       def approve
@@ -63,6 +63,10 @@ module EE
         reports_response(merge_request.compare_sast_reports(current_user))
       end
 
+      def dast_reports
+        reports_response(merge_request.compare_dast_reports(current_user))
+      end
+
       def metrics_reports
         reports_response(merge_request.compare_metrics_reports)
       end

ee/app/models/ee/ci/job_artifact.rb

@@ -17,6 +17,7 @@ module EE
       METRICS_REPORT_FILE_TYPES = %w[metrics].freeze
       CONTAINER_SCANNING_REPORT_TYPES = %w[container_scanning].freeze
       SAST_REPORT_TYPES = %w[sast].freeze
+      DAST_REPORT_TYPES = %w[dast].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)) }
@@ -42,6 +43,10 @@ module EE
         with_file_types(SAST_REPORT_TYPES)
       end
 
+      scope :dast_reports, -> do
+        with_file_types(DAST_REPORT_TYPES)
+      end
+
       scope :metrics_reports, -> do
         with_file_types(METRICS_REPORT_FILE_TYPES)
       end

ee/app/models/ee/merge_request.rb

@@ -127,61 +127,61 @@ module EE
     end
 
     def has_dependency_scanning_reports?
-      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dependency_list_reports)
+      !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dependency_list_reports))
     end
 
     def compare_dependency_scanning_reports(current_user)
-      unless has_dependency_scanning_reports?
-        return { status: :error, status_reason: 'This merge request does not have dependency scanning reports' }
-      end
+      return missing_report_error("dependency scanning") unless has_dependency_scanning_reports?
 
       compare_reports(::Ci::CompareDependencyScanningReportsService, current_user)
     end
 
     def has_license_management_reports?
-      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.license_management_reports)
+      !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.license_management_reports))
     end
 
     def has_container_scanning_reports?
-      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.container_scanning_reports)
+      !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.container_scanning_reports))
     end
 
     def compare_container_scanning_reports(current_user)
-      unless has_container_scanning_reports?
-        return { status: :error, status_reason: 'This merge request does not have container scanning reports' }
-      end
+      return missing_report_error("container scanning") unless has_container_scanning_reports?
 
       compare_reports(::Ci::CompareContainerScanningReportsService, current_user)
     end
 
     def has_sast_reports?
-      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.sast_reports)
+      !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.sast_reports))
     end
 
     def compare_sast_reports(current_user)
-      unless has_sast_reports?
-        return { status: :error, status_reason: 'This merge request does not have SAST reports' }
-      end
+      return missing_report_error("SAST") unless has_sast_reports?
 
       compare_reports(::Ci::CompareSastReportsService, current_user)
     end
 
-    def compare_license_management_reports(current_user)
-      unless has_license_management_reports?
-        return { status: :error, status_reason: 'This merge request does not have license management reports' }
+    def has_dast_reports?
+      !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.dast_reports))
+    end
+
+    def compare_dast_reports(current_user)
+      return missing_report_error("DAST") unless has_dast_reports?
+
+      compare_reports(::Ci::CompareDastReportsService, current_user)
     end
 
+    def compare_license_management_reports(current_user)
+      return missing_report_error("license management") unless has_license_management_reports?
+
       compare_reports(::Ci::CompareLicenseScanningReportsService, current_user)
     end
 
     def has_metrics_reports?
-      actual_head_pipeline&.has_reports?(::Ci::JobArtifact.metrics_reports)
+      !!(actual_head_pipeline&.has_reports?(::Ci::JobArtifact.metrics_reports))
     end
 
     def compare_metrics_reports
-      unless has_metrics_reports?
-        return { status: :error, status_reason: 'This merge request does not have metrics reports' }
-      end
+      return missing_report_error("metrics") unless has_metrics_reports?
 
       compare_reports(::Ci::CompareMetricsReportsService)
     end
@@ -194,5 +194,11 @@ module EE
         project_rule.apply_report_approver_rules_to(self)
       end
     end
+
+    private
+
+    def missing_report_error(report_type)
+      { status: :error, status_reason: "This merge request does not have #{report_type} reports" }
+    end
   end
 end

ee/app/services/ci/compare_dast_reports_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CompareDastReportsService < ::Ci::CompareReportsBaseService
+    def comparer_class
+      Gitlab::Ci::Reports::Security::VulnerabilityReportsComparer
+    end
+
+    def serializer_class
+      Vulnerabilities::OccurrenceDiffSerializer
+    end
+
+    def get_report(pipeline)
+      Security::PipelineVulnerabilitiesFinder.new(pipeline: pipeline, params: { report_type: %w[dast] }).execute
+    end
+  end
+end

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

@@ -20,3 +20,4 @@
     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)}'
+    window.gl.mrWidgetData.dast_comparison_path = '#{dast_reports_project_merge_request_path(@project, @merge_request) if @project.feature_available?(:dast)}'

ee/changelogs/unreleased/move-dast-reports-logic-backend.yml

+---
+title: Move DAST reports logic for the Merge Request widget to the backend
+merge_request: 18660
+author:
+type: changed

ee/config/routes/project.rb

@@ -74,6 +74,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
           get :container_scanning_reports
           get :dependency_scanning_reports
           get :sast_reports
+          get :dast_reports
         end
       end
 

ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -462,23 +462,6 @@ describe Projects::MergeRequestsController do
       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
-
     context 'public project with private builds' do
       let(:comparison_status) { {} }
       let(:project) { create(:project, :public, :builds_private) }
@@ -562,23 +545,6 @@ describe Projects::MergeRequestsController do
       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
-
     context 'public project with private builds' do
       let(:comparison_status) { {} }
       let(:project) { create(:project, :public, :builds_private) }
@@ -662,8 +628,57 @@ describe Projects::MergeRequestsController do
       end
     end
 
-    context 'when something went wrong on our system' do
+    context 'public project with private builds' do
       let(:comparison_status) { {} }
+      let(:project) { create(:project, :public, :builds_private) }
+
+      before do
+        sign_out user
+      end
+
+      it 'restricts unauthorized access' do
+        subject
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+  end
+
+  describe 'GET #dast_reports' do
+    let(:merge_request) { create(:ee_merge_request, :with_dast_reports, source_project: project) }
+    let(:params) do
+      {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid
+      }
+    end
+
+    subject { get :dast_reports, params: params, format: :json }
+
+    before do
+      allow_any_instance_of(::MergeRequest).to receive(:compare_reports)
+        .with(::Ci::CompareDastReportsService, project.users.first).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)
@@ -671,11 +686,28 @@ describe Projects::MergeRequestsController do
         subject
       end
 
-      it 'returns 500 HTTP status' do
+      it 'returns 200 HTTP status' do
         subject
 
-        expect(response).to have_gitlab_http_status(:internal_server_error)
-        expect(json_response).to eq({ 'status_reason' => 'Unknown error' })
+        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 DAST 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 DAST reports' })
       end
     end
 
@@ -683,11 +715,17 @@ describe Projects::MergeRequestsController do
       let(:comparison_status) { {} }
       let(:project) { create(:project, :public, :builds_private) }
 
-      before do
+      it 'restricts access to signed out users' do
         sign_out user
+
+        subject
+
+        expect(response).to have_gitlab_http_status(404)
       end
 
-      it 'restricts unauthorized access' do
+      it 'restricts access to other users' do
+        sign_in create(:user)
+
         subject
 
         expect(response).to have_gitlab_http_status(404)
@@ -761,23 +799,6 @@ describe Projects::MergeRequestsController do
         expect(json_response).to eq({ 'status_reason' => 'Failed to parse license 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 #metrics_reports' do
@@ -847,22 +868,5 @@ describe Projects::MergeRequestsController do
         expect(json_response).to eq({ 'status_reason' => 'Failed to parse test 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
 end

ee/spec/factories/ci/builds.rb

@@ -54,6 +54,12 @@ FactoryBot.define do
       end
     end
 
+    trait :dast_feature_branch do
+      after(:build) do |build|
+        build.job_artifacts << create(:ee_ci_job_artifact, :dast_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)

ee/spec/factories/ci/job_artifacts.rb

@@ -6,9 +6,69 @@ FactoryBot.define do
       file_type { :sast }
       file_format { :raw }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-sast-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-sast-report.json'), 'application/json')
+      end
+    end
+
+    trait :dast do
+      file_format { :raw }
+      file_type { :dast }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report.json'), 'application/json')
+      end
+    end
+
+    trait :dast_feature_branch do
+      file_format { :raw }
+      file_type { :dast }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/feature-branch/gl-dast-report.json'), 'application/json')
+      end
+    end
+
+    trait :dast_with_corrupted_data do
+      file_format { :raw }
+      file_type { :dast }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
+      end
+    end
+
+    trait :dast_deprecated do
+      file_format { :raw }
+      file_type { :dast }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-dast-report.json'), 'application/json')
+      end
+    end
+
+    trait :dast_multiple_sites do
+      file_format { :raw }
+      file_type { :dast }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report-multiple-sites.json'), 'application/json')
+      end
+    end
+
+    trait :low_severity_dast_report do
+      file_format { :raw }
+      file_type { :dast }
+
+      after(:build) do |artifact, _|
+        artifact.file = fixture_file_upload(
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report-low-severity.json'), 'application/json')
       end
     end
 
@@ -26,9 +86,9 @@ FactoryBot.define do
       file_type { :sast }
       file_format { :raw }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-sast-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-sast-report.json'), 'application/json')
       end
     end
 
@@ -36,7 +96,7 @@ FactoryBot.define do
       file_type { :sast }
       file_format { :raw }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
           Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
       end
@@ -46,7 +106,7 @@ FactoryBot.define do
       file_type { :license_management }
       file_format { :raw }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
           Rails.root.join('ee/spec/fixtures/security_reports/master/gl-license-management-report.json'), 'application/json')
       end
@@ -56,7 +116,7 @@ FactoryBot.define do
       file_type { :license_management }
       file_format { :raw }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
           Rails.root.join('ee/spec/fixtures/security_reports/feature-branch/gl-license-management-report.json'), 'application/json')
       end
@@ -66,7 +126,7 @@ FactoryBot.define do
       file_type { :license_management }
       file_format { :raw }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
           Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
       end
@@ -98,7 +158,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dependency-scanning-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dependency-scanning-report.json'), 'application/json')
       end
     end
 
@@ -108,7 +168,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/remediations/gl-dependency-scanning-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/remediations/gl-dependency-scanning-report.json'), 'application/json')
       end
     end
 
@@ -118,7 +178,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-dependency-scanning-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-dependency-scanning-report.json'), 'application/json')
       end
     end
 
@@ -136,7 +196,7 @@ FactoryBot.define do
       file_format { :raw }
       file_type { :dependency_scanning }
 
-      after(:build) do |artifact, evaluator|
+      after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
           Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
       end
@@ -148,7 +208,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-container-scanning-report.json'), 'application/json')
       end
     end
 
@@ -166,49 +226,9 @@ FactoryBot.define do
       file_format { :raw }
       file_type { :container_scanning }
 
-      after(:build) do |artifact, evaluator|
-        artifact.file = fixture_file_upload(
-          Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
-      end
-    end
-
-    trait :dast do
-      file_format { :raw }
-      file_type { :dast }
-
-      after(:build) do |artifact, _|
-        artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report.json'), 'text/plain')
-      end
-    end
-
-    trait :dast_deprecated do
-      file_format { :raw }
-      file_type { :dast }
-
-      after(:build) do |artifact, _|
-        artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-dast-report.json'), 'text/plain')
-      end
-    end
-
-    trait :dast_multiple_sites do
-      file_format { :raw }
-      file_type { :dast }
-
-      after(:build) do |artifact, _|
-        artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report-multiple-sites.json'), 'text/plain')
-      end
-    end
-
-    trait :low_severity_dast_report do
-      file_format { :raw }
-      file_type { :dast }
-
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report-low-severity.json'), 'text/plain')
+          Rails.root.join('spec/fixtures/trace/sample_trace'), 'application/json')
       end
     end
 
@@ -238,7 +258,7 @@ FactoryBot.define do
 
       after(:build) do |artifact, _|
         artifact.file = fixture_file_upload(
-          Rails.root.join('ee/spec/fixtures/security_reports/dependency_list/gl-dependency-scanning-report.json'), 'text/plain')
+          Rails.root.join('ee/spec/fixtures/security_reports/dependency_list/gl-dependency-scanning-report.json'), 'application/json')
       end
     end
   end

ee/spec/factories/ci/pipelines.rb

@@ -7,7 +7,7 @@ FactoryBot.define do
       config_source { :webide_source }
     end
 
-    %i[license_management dependency_list dependency_scanning sast container_scanning].each do |report_type|
+    %i[license_management dependency_list dependency_scanning sast dast container_scanning].each do |report_type|
       trait "with_#{report_type}_report".to_sym do
         status { :success }
 
@@ -57,6 +57,14 @@ FactoryBot.define do
       end
     end
 
+    trait :with_dast_feature_branch do
+      status { :success }
+
+      after(:build) do |pipeline, evaluator|
+        pipeline.builds << build(:ee_ci_build, :dast_feature_branch, pipeline: pipeline, project: pipeline.project)
+      end
+    end
+
     trait :with_license_management_feature_branch do
       status { :success }
 

ee/spec/factories/merge_requests.rb

@@ -119,6 +119,18 @@ FactoryBot.define do
       end
     end
 
+    trait :with_dast_reports do
+      after(:build) do |merge_request|
+        merge_request.head_pipeline = build(
+          :ee_ci_pipeline,
+          :success,
+          :with_dast_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-dast-report.json

+{
+  "site": [
+    {
+      "@port": "8080",
+      "@host": "goat",
+      "@name": "http://goat:8080",
+      "alerts": [
+        {
+          "count": "1",
+          "riskdesc": "Low (Medium)",
+          "name": "Cookie No HttpOnly Flag",
+          "reference": "<p>http://www.owasp.org/index.php/HttpOnly</p>",
+          "sourceid": "3",
+          "confidence": "2",
+          "alert": "Cookie No HttpOnly Flag",
+          "instances": [
+            {
+              "evidence": "Set-Cookie: JSESSIONID",
+              "uri": "http://goat:8080/WebGoat/login?logout",
+              "param": "JSESSIONID",
+              "method": "GET"
+            }
+          ],
+          "pluginid": "10010",
+          "riskcode": "1",
+          "wascid": "13",
+          "solution": "<p>Ensure that the HttpOnly flag is set for all cookies.</p>",
+          "cweid": "120",
+          "desc": "<p>A cookie has been set without the HttpOnly flag, which means that the cookie can be accessed by JavaScript. If a malicious script can be run on this page then the cookie will be accessible and can be transmitted to another site. If this is a session cookie then session hijacking may be possible.</p>"
+        },
+        {
+          "count": "4",
+          "riskdesc": "Informational (Medium)",
+          "name": "Information Disclosure - Suspicious Comments",
+          "reference": "<p></p>",
+          "otherinfo": "<p><!--<button type=\"button\" id=\"admin-button\" class=\"btn btn-default right_nav_button\" title=\"Administrator\">--></p><p><!--<button type=\"button\" id=\"user-management\" class=\"btn btn-default right_nav_button\"--></p><p><!--title=\"User management\">--></p><p></p>",
+          "sourceid": "3",
+          "confidence": "2",
+          "alert": "Information Disclosure - Suspicious Comments",
+          "instances": [
+            {
+              "uri": "http://goat:8080/WebGoat/start_new.mvc",
+              "method": "GET"
+            }
+          ],
+          "pluginid": "10027",
+          "riskcode": "0",
+          "wascid": "13",
+          "solution": "<p>Remove all comments that return information that may help an attacker and fix any underlying problems they refer to.</p>",
+          "cweid": "201",
+          "desc": "<p>The response appears to contain suspicious comments which may help an attacker.</p>"
+        }
+      ],
+      "@ssl": "false"
+    }
+  ],
+  "spider": {
+    "progress": "100",
+    "state": "FINISHED",
+    "result": {
+      "urlsIoError": [],
+      "urlsOutOfScope": [
+        "http://getbootstrap.com/",
+        "http://daneden.me/animate",
+        "http://fontawesome.io/",
+        "https://github.com/twbs/bootstrap/blob/master/LICENSE",
+        "https://github.com/nickpettit/glide",
+        "http://fontawesome.io/license"
+      ],
+      "urlsInScope": [
+        {
+          "url": "http://goat:8080",
+          "statusReason": "",
+          "reasonNotProcessed": "Not Text",
+          "processed": "false",
+          "method": "GET",
+          "statusCode": "404"
+        },
+        {
+          "url": "http://goat:8080/robots.txt",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "404"
+        },
+        {
+          "url": "http://goat:8080/sitemap.xml",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "404"
+        },
+        {
+          "url": "http://goat:8080/WebGoat",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/attack",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/",
+          "statusReason": "",
+          "reasonNotProcessed": "Not Text",
+          "processed": "false",
+          "method": "GET",
+          "statusCode": "404"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/start.mvc",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/welcome.mvc",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/logout",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/css/main.css",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/images/favicon.ico",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "404"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/plugins/bootstrap/css/bootstrap.min.css",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/css/font-awesome.min.css",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/css/coderay.css",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/css/animate.css",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/js/modernizr-2.6.2.min.js",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/css/lessons.css",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/js/html5shiv.js",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/js/respond.min.js",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/js/libs/require.min.js",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/login?logout",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/login",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/login",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "POST",
+          "statusCode": "302"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/login?error",
+          "statusReason": "",
+          "reasonNotProcessed": "Max Depth",
+          "processed": "false",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/registration",
+          "statusReason": "",
+          "reasonNotProcessed": "",
+          "processed": "true",
+          "method": "GET",
+          "statusCode": "200"
+        },
+        {
+          "url": "http://goat:8080/WebGoat/register.mvc",
+          "statusReason": "",
+          "reasonNotProcessed": "Max Depth",
+          "processed": "false",
+          "method": "POST",
+          "statusCode": "200"
+        }
+      ]
+    }
+  },
+  "@generated": "Fri, 13 Apr 2018 09:22:01",
+  "@version": "2.7.0"
+}

ee/spec/models/merge_request_spec.rb

@@ -248,6 +248,35 @@ describe MergeRequest do
     end
   end
 
+  describe '#has_dast_reports?' do
+    subject { merge_request.has_dast_reports? }
+
+    let(:project) { create(:project, :repository) }
+
+    before do
+      stub_licensed_features(dast: true)
+    end
+
+    context 'when head pipeline has dast reports' do
+      let(:merge_request) { create(:ee_merge_request, :with_dast_reports, source_project: project) }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when pipeline ran for an older commit than the branch head' do
+      let(:pipeline) { create(:ci_empty_pipeline, sha: 'notlatestsha') }
+      let(:merge_request) { create(:ee_merge_request, source_project: project, head_pipeline: pipeline) }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when head pipeline does not have dast 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? }
 

ee/spec/services/ci/compare_dast_reports_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Ci::CompareDastReportsService do
+  let(:current_user) { project.users.take }
+  let(:service) { described_class.new(project, current_user) }
+  let(:project) { create(:project, :repository) }
+
+  before do
+    stub_licensed_features(container_scanning: true, dast: true)
+  end
+
+  describe '#execute' do
+    subject { service.execute(base_pipeline, head_pipeline) }
+
+    context 'when head pipeline has DAST reports containing some vulnerabilities' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_dast_report, project: project) }
+
+      it 'reports the new vulnerabilities, while not changing the counts of existing and fixed vulnerabilities' do
+        expect(subject[:status]).to eq(:parsed)
+        expect(subject[:data]['added'].count).to eq(20)
+        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 DAST reports containing vulnerabilities' do
+      let!(:base_pipeline) { create(:ee_ci_pipeline, :with_dast_report, project: project) }
+      let!(:head_pipeline) { create(:ee_ci_pipeline, :with_dast_feature_branch, project: project) }
+
+      it 'reports status as parsed' do
+        expect(subject[:status]).to eq(:parsed)
+      end
+
+      it 'populates fields based on current_user' do
+        payload = subject[:data]['fixed'].first
+
+        expect(payload).not_to be_empty
+        expect(payload['create_vulnerability_feedback_issue_path']).not_to be_empty
+        expect(payload['create_vulnerability_feedback_merge_request_path']).not_to be_empty
+        expect(payload['create_vulnerability_feedback_dismissal_path']).not_to be_empty
+        expect(payload['create_vulnerability_feedback_issue_path']).not_to be_empty
+        expect(service.current_user).to eq(current_user)
+      end
+
+      it 'reports new vulnerability' do
+        expect(subject[:data]['added'].count).to eq(1)
+        expect(subject[:data]['added'].last['identifiers']).to include(a_hash_including('name' => 'CWE-201'))
+      end
+
+      it 'reports existing DAST vulnerabilities' do
+        expect(subject[:data]['existing'].count).to eq(1)
+        expect(subject[:data]['existing'].last['identifiers']).to include(a_hash_including('name' => 'CWE-120'))
+      end
+
+      it 'reports fixed DAST vulnerabilities' do
+        expect(subject[:data]['fixed'].count).to eq(19)
+        expect(subject[:data]['fixed']).to include(
+          a_hash_including(
+            {
+              'identifiers' => a_collection_including(
+                a_hash_including(
+                  "name" => "CWE-352"
+                )
+              )
+            })
+        )
+      end
+    end
+  end
+end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -889,23 +889,6 @@ describe Projects::MergeRequestsController do
         end
       end
 
-      context 'when something went wrong on our system' do
-        let(:report) { {} }
-
-        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
-
       context 'when feature flag :ci_expose_arbitrary_artifacts_in_mr is disabled' do
         let(:job_options) do
           {
@@ -1063,23 +1046,6 @@ describe Projects::MergeRequestsController do
         expect(json_response).to eq({ 'status_reason' => 'Failed to parse test 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 'POST remove_wip' do