Merge branch '11024_add_scanned_resources_count_to_security_scan' into 'master'

Store the number of scanned URLs on security_scan

Closes #11024

See merge request gitlab-org/gitlab!27260

db/migrate/20200314060834_add_scanned_resources_count_to_security_scan.rb

+# frozen_string_literal: true
+class AddScannedResourcesCountToSecurityScan < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column :security_scans, :scanned_resources_count, :integer
+  end
+
+  def down
+    remove_column :security_scans, :scanned_resources_count
+  end
+end

db/structure.sql

@@ -5585,7 +5585,8 @@ CREATE TABLE public.security_scans (
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone NOT NULL,
     build_id bigint NOT NULL,
-    scan_type smallint NOT NULL
+    scan_type smallint NOT NULL,
+    scanned_resources_count integer
 );
 
 CREATE SEQUENCE public.security_scans_id_seq
@@ -12873,6 +12874,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200312163407
 20200313101649
 20200313123934
+20200314060834
 20200316111759
 20200316162648
 20200316173312

ee/app/services/security/store_scans_service.rb

@@ -11,12 +11,17 @@ module Security
 
       security_reports = @build.job_artifacts.security_reports
 
-      ActiveRecord::Base.transaction do
-        security_reports.each do |report|
-          Security::Scan.safe_find_or_create_by!(
+      scan_params = security_reports.map do |job_artifact|
+        {
           build: @build,
-            scan_type: report.file_type
-          )
+          scan_type: job_artifact.file_type,
+          scanned_resources_count: Gitlab::Ci::Parsers::Security::ScannedResources.new.scanned_resources_count(job_artifact)
+        }
+      end
+
+      ActiveRecord::Base.transaction do
+        scan_params.each do |param|
+          Security::Scan.safe_find_or_create_by!(param)
         end
       end
     end

ee/changelogs/unreleased/11024_add_scanned_resources_count_to_security_scan.yml

+---
+title: Add a count of the scanned resources to Security::Scan.
+merge_request: 27260
+author:
+type: added

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

+# frozen_string_literal: true
+module Gitlab
+  module Ci
+    module Parsers
+      module Security
+        class ScannedResources
+          def scanned_resources_count(job_artifact)
+            scanned_resources_sum = 0
+            job_artifact.each_blob do |blob|
+              report_data = parse_report_json(blob)
+              scanned_resources_sum += report_data.fetch('scan', {}).fetch('scanned_resources', []).length
+            end
+            scanned_resources_sum
+          end
+
+          private
+
+          def parse_report_json(blob)
+            JSON.parse!(blob)
+          rescue JSON::ParserError
+            {}
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/factories/ci/job_artifacts.rb

@@ -42,6 +42,11 @@ FactoryBot.define do
       end
     end
 
+    trait :dast_with_missing_file do
+      file_format { :raw }
+      file_type { :dast }
+    end
+
     trait :dast_deprecated_no_spider do
       file_format { :raw }
       file_type { :dast }
@@ -74,6 +79,26 @@ FactoryBot.define do
       end
     end
 
+    trait :dast_missing_scan_field 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-missing-scan.json'), 'application/json')
+      end
+    end
+
+    trait :dast_missing_scanned_resources_field 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-missing-scanned-resources.json'), 'application/json')
+      end
+    end
+
     trait :low_severity_dast_report do
       file_format { :raw }
       file_type { :dast }

ee/spec/fixtures/security_reports/master/gl-dast-report.json

 {
     "@generated": "Fri, 13 Apr 2018 09:22:01", 
     "@version": "2.7.0", 
+    "scan": {
+        "scanned_resources": [
+            {
+                "method": "GET",
+                "type": "url",
+                "url": "http://api-server/"
+            },
+            {
+                "method": "GET",
+                "type": "url",
+                "url": "http://api-server/v1"
+            },
+            {
+                "method": "DELETE",
+                "type": "url",
+                "url": "http://api-server/v1/tree/10"
+            },
+            {
+                "method": "GET",
+                "type": "url",
+                "url": "http://api-server/v1/tree/10"
+            },
+            {
+                "method": "GET",
+                "type": "url",
+                "url": "http://api-server/v1/trees"
+            },
+            {
+                "method": "POST",
+                "type": "url",
+                "url": "http://api-server/v1/trees"
+            }
+        ]
+    },
     "site": [
         {
             "@host": "goat", 

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Ci::Parsers::Security::ScannedResources do
+  describe '#scanned_resources_count' do
+    let(:parser) { described_class.new }
+
+    subject { parser.scanned_resources_count(artifact) }
+
+    context 'there are scanned resources' do
+      let(:artifact) { create(:ee_ci_job_artifact, :dast) }
+
+      it { is_expected.to be(6) }
+    end
+
+    context 'the scan key is missing' do
+      let(:artifact) { create(:ee_ci_job_artifact, :dast_missing_scan_field) }
+
+      it { is_expected.to be(0) }
+    end
+
+    context 'the scanned_resources key is missing' do
+      let(:artifact) { create(:ee_ci_job_artifact, :dast_missing_scanned_resources_field) }
+
+      it { is_expected.to be(0) }
+    end
+
+    context 'the json is invalid' do
+      let(:artifact) { create(:ee_ci_job_artifact, :dast_with_corrupted_data) }
+
+      it { is_expected.to be(0) }
+    end
+  end
+end

ee/spec/services/security/store_scans_service_spec.rb

@@ -22,12 +22,22 @@ describe Security::StoreScansService do
       expect(scans.sast.count).to be(1)
       expect(scans.dast.count).to be(1)
     end
+
+    it 'stores the scanned resources count on the scan' do
+      subject
+
+      sast_scan = Security::Scan.sast.find_by(build: build)
+      expect(sast_scan.scanned_resources_count).to be(0)
+
+      dast_scan = Security::Scan.dast.find_by(build: build)
+      expect(dast_scan.scanned_resources_count).to be(6)
+    end
   end
 
   context 'scan already exists' do
     before do
       create(:ee_ci_job_artifact, :dast, job: build)
-      create(:security_scan, build: build, scan_type: 'dast')
+      create(:security_scan, build: build, scan_type: 'dast', scanned_resources_count: 6)
     end
 
     it 'does not save' do
@@ -36,4 +46,16 @@ describe Security::StoreScansService do
       expect(Security::Scan.where(build: build).count).to be(1)
     end
   end
+
+  context 'artifact file does not exist' do
+    before do
+      create(:ee_ci_job_artifact, :dast_with_missing_file, job: build)
+    end
+    it 'stores 0 scanned resources on the scan' do
+      subject
+
+      scans = Security::Scan.where(build: build)
+      expect(scans.dast.first.scanned_resources_count).to be(0)
+    end
+  end
 end