Merge branch 'use-dast-common-report-format-fields' into 'master'

Parse DAST reports using the common report format

Closes #33913

See merge request gitlab-org/gitlab!22413

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

@@ -5,52 +5,17 @@ module Gitlab
     module Parsers
       module Security
         class Dast < Common
-          FORMAT_VERSION = '2.0'.freeze
-
           protected
 
           def parse_report(json_data)
             report = super
 
-            format_report(report)
-          end
-
-          private
-
-          def format_report(data)
-            {
-              'vulnerabilities' => extract_vulnerabilities_from(Array.wrap(data['site'])),
-              'version' => FORMAT_VERSION
-            }
-          end
-
-          # Log messages to be added here to track usage of legacy reports,
-          # parsing failures and any other scenarios: https://gitlab.com/gitlab-org/gitlab/issues/34668
-          def extract_vulnerabilities_from(sites = [])
-            return [] if sites.empty?
-
-            vulnerabilities = []
+            return Formatters::Dast.new(report).format if Formatters::Dast.satisfies?(report)
 
-            sites.each do |site|
-              site_report = Hash(site)
-              next if site_report.blank?
-
-              # If host is blank for legacy reports
-              host = site_report['@name']
-
-              site_report['alerts'].each do |vulnerability|
-                vulnerabilities += flatten_vulnerabilities(vulnerability, host)
-              end
-            end
-
-            vulnerabilities
+            report
           end
 
-          def flatten_vulnerabilities(vulnerability, host)
-            vulnerability['instances'].map do |instance|
-              Formatters::Dast.new(vulnerability).format(instance, host)
-            end
-          end
+          private
 
           def create_location(location_data)
             ::Gitlab::Ci::Reports::Security::Locations::Dast.new(

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

@@ -6,11 +6,53 @@ module Gitlab
       module Security
         module Formatters
           class Dast
-            def initialize(vulnerability)
-              @vulnerability = vulnerability
+            FORMAT_VERSION = '2.0'.freeze
+
+            def initialize(report)
+              @report = report
+            end
+
+            def self.satisfies?(report)
+              report.key?('site') && !report.key?('vulnerabilities')
+            end
+
+            def format
+              {
+                'vulnerabilities' => extract_vulnerabilities_from(Array.wrap(@report['site'])),
+                'version' => FORMAT_VERSION
+              }
+            end
+
+            private
+
+            # Log messages to be added here to track usage of legacy reports,
+            # parsing failures and any other scenarios: https://gitlab.com/gitlab-org/gitlab/issues/34668
+            def extract_vulnerabilities_from(sites = [])
+              return [] if sites.empty?
+
+              vulnerabilities = []
+
+              sites.each do |site|
+                site_report = Hash(site)
+
+                next if site_report.blank?
+
+                # If host is blank for legacy reports
+                host = site_report['@name']
+
+                site_report['alerts'].each do |vulnerability|
+                  vulnerabilities += flatten_vulnerabilities(vulnerability, host)
+                end
+              end
+
+              vulnerabilities
+            end
+
+            def flatten_vulnerabilities(vulnerability, host)
+              vulnerability['instances'].map { |instance| format_vulnerability(vulnerability, instance, host) }
             end
 
-            def format(instance, hostname)
+            def format_vulnerability(vulnerability, instance, hostname)
               {
                 'category' => 'dast',
                 'message' => vulnerability['name'],
@@ -50,10 +92,6 @@ module Gitlab
               }
             end
 
-            private
-
-            attr_reader :vulnerability
-
             SEVERITY_MAPPING = %w{info low medium high}.freeze
             CONFIDENCE_MAPPING = %w{ignore low medium high confirmed}.freeze
 

ee/spec/factories/ci/job_artifacts.rb

@@ -42,13 +42,25 @@ FactoryBot.define do
       end
     end
 
-    trait :dast_deprecated do
+    trait :dast_deprecated_no_spider 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')
+          Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-dast-report-no-spider.json'), 'application/json')
+      end
+    end
+
+    trait :dast_deprecated_no_common_fields 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-no-common-fields.json'),
+          'application/json'
+        )
       end
     end
 

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

@@ -22,8 +22,9 @@ describe Gitlab::Ci::Parsers::Security::Dast do
           :last_occurrence_severity,
           :last_occurrence_confidence) do
       :dast                             | 24 | 15 | 1 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low'
-      :dast_multiple_sites  | 25 | 15 | 1 | 'https://goat:8080' | 'GET' | '/WebGoat/registration' | 'high' | 'medium'
-      :dast_deprecated      | 2 | 3 | 1 | 'http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io' | 'GET' | '/' | 'low' | 'medium'
+      :dast_multiple_sites              | 25 | 15 | 1 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low'
+      :dast_deprecated_no_spider        | 2  | 3  | 1 | 'http://bikebilly-spring-auto-devops-review-feature-br-3y2gpb.35.192.176.43.xip.io' | 'GET' | '/' | 'low' | 'medium'
+      :dast_deprecated_no_common_fields | 24 | 15 | 1 | 'http://goat:8080' | 'GET' | '/WebGoat/plugins/bootstrap/css/bootstrap.min.css' | 'info' | 'low'
     end
 
     with_them do

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

@@ -3,56 +3,58 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Parsers::Security::Formatters::Dast do
-  let(:formatter) { described_class.new(file_vulnerability) }
-  let(:file_vulnerability) { parsed_report['site'].first['alerts'][0] }
+  let(:formatter) { described_class.new(parsed_report) }
 
   let(:parsed_report) do
     JSON.parse!(
       File.read(
-        Rails.root.join('ee/spec/fixtures/security_reports/master/gl-dast-report.json')
+        Rails.root.join('ee/spec/fixtures/security_reports/deprecated/gl-dast-report-no-common-fields.json')
       )
     )
   end
 
   describe '#format_vulnerability' do
-    let(:instance) { file_vulnerability['instances'][0] }
     let(:hostname) { 'http://goat:8080' }
+    let(:file_vulnerability) { parsed_report['site'].first['alerts'][0] }
     let(:sanitized_desc) { file_vulnerability['desc'].gsub('<p>', '').gsub('</p>', '') }
     let(:sanitized_solution) { file_vulnerability['solution'].gsub('<p>', '').gsub('</p>', '') }
     let(:version) { parsed_report['@version'] }
 
     it 'format ZAProxy vulnerability into common format' do
-      data = formatter.format(instance, hostname)
+      data = formatter.format
+
+      expect(data['vulnerabilities'].size).to eq(24)
+      vulnerability = data['vulnerabilities'][0]
 
-      expect(data['category']).to eq('dast')
-      expect(data['message']).to eq('Anti CSRF Tokens Scanner')
-      expect(data['description']).to eq(sanitized_desc)
-      expect(data['cve']).to eq('20012')
-      expect(data['severity']).to eq('high')
-      expect(data['confidence']).to eq('medium')
-      expect(data['solution']).to eq(sanitized_solution)
-      expect(data['scanner']).to eq({ 'id' => 'zaproxy', 'name' => 'ZAProxy' })
-      expect(data['links']).to eq([{ 'url' => 'http://projects.webappsec.org/Cross-Site-Request-Forgery' },
+      expect(vulnerability['category']).to eq('dast')
+      expect(vulnerability['message']).to eq('Anti CSRF Tokens Scanner')
+      expect(vulnerability['description']).to eq(sanitized_desc)
+      expect(vulnerability['cve']).to eq('20012')
+      expect(vulnerability['severity']).to eq('high')
+      expect(vulnerability['confidence']).to eq('medium')
+      expect(vulnerability['solution']).to eq(sanitized_solution)
+      expect(vulnerability['scanner']).to eq({ 'id' => 'zaproxy', 'name' => 'ZAProxy' })
+      expect(vulnerability['links']).to eq([{ 'url' => 'http://projects.webappsec.org/Cross-Site-Request-Forgery' },
                                             { 'url' => 'http://cwe.mitre.org/data/definitions/352.html' }])
-      expect(data['identifiers'][0]).to eq({
+      expect(vulnerability['identifiers'][0]).to eq({
                                                       'type' => 'ZAProxy_PluginId',
                                                       'name' => 'Anti CSRF Tokens Scanner',
                                                       'value' => '20012',
                                                       'url' => "https://github.com/zaproxy/zaproxy/blob/w2019-01-14/docs/scanners.md"
                                                     })
-      expect(data['identifiers'][1]).to eq({
+      expect(vulnerability['identifiers'][1]).to eq({
                                                       'type' => 'CWE',
                                                       'name' => "CWE-352",
                                                       'value' => '352',
                                                       'url' => "https://cwe.mitre.org/data/definitions/352.html"
                                                     })
-      expect(data['identifiers'][2]).to eq({
+      expect(vulnerability['identifiers'][2]).to eq({
                                                       'type' => 'WASC',
                                                       'name' => "WASC-9",
                                                       'value' => '9',
                                                       'url' => "http://projects.webappsec.org/w/page/13246974/Threat%20Classification%20Reference%20Grid"
                                                     })
-      expect(data['location']).to eq({
+      expect(vulnerability['location']).to eq({
                                                 'param' => '',
                                                 'method' => 'GET',
                                                 'hostname' => hostname,