Merge branch 'introduce-security-scan-model' into 'master'

Introduce security scan model

See merge request gitlab-org/gitlab!23669

app/workers/concerns/worker_attributes.rb

@@ -22,7 +22,8 @@ module WorkerAttributes
 
     # EE-specific
     epics: 2,
-    incident_management: 2
+    incident_management: 2,
+    security_scans: 2
   }.stringify_keys.freeze
 
   class_methods do

changelogs/unreleased/introduce-security-scan-model.yml

+---
+title: Store security scans run in CI jobs
+merge_request: 23669
+author:
+type: other

config/sidekiq_queues.yml

@@ -220,6 +220,8 @@
   - 1
 - - repository_update_remote_mirror
   - 1
+- - security_scans
+  - 2
 - - self_monitoring_project_create
   - 2
 - - self_monitoring_project_delete

db/migrate/20200203232433_create_security_scan.rb

+# frozen_string_literal: true
+
+class CreateSecurityScan < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    create_table :security_scans, id: :bigserial do |t|
+      t.timestamps_with_timezone null: false
+
+      t.references :build,
+                   null: false,
+                   index: false,
+                   foreign_key: { to_table: :ci_builds, on_delete: :cascade },
+                   type: :bigint
+
+      t.integer :scan_type,
+                null: false,
+                index: { name: "idx_security_scans_on_scan_type" },
+                limit: 2
+
+      t.index [:build_id, :scan_type], name: "idx_security_scans_on_build_and_scan_type", unique: true
+    end
+  end
+end

db/schema.rb

@@ -3737,6 +3737,15 @@ ActiveRecord::Schema.define(version: 2020_02_04_131054) do
     t.index ["group_id", "token_encrypted"], name: "index_scim_oauth_access_tokens_on_group_id_and_token_encrypted", unique: true
   end
 
+  create_table "security_scans", force: :cascade do |t|
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "updated_at", null: false
+    t.bigint "build_id", null: false
+    t.integer "scan_type", limit: 2, null: false
+    t.index ["build_id", "scan_type"], name: "idx_security_scans_on_build_and_scan_type", unique: true
+    t.index ["scan_type"], name: "idx_security_scans_on_scan_type"
+  end
+
   create_table "self_managed_prometheus_alert_events", force: :cascade do |t|
     t.bigint "project_id", null: false
     t.bigint "environment_id"
@@ -4863,6 +4872,7 @@ ActiveRecord::Schema.define(version: 2020_02_04_131054) do
   add_foreign_key "reviews", "users", column: "author_id", on_delete: :nullify
   add_foreign_key "saml_providers", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "scim_oauth_access_tokens", "namespaces", column: "group_id", on_delete: :cascade
+  add_foreign_key "security_scans", "ci_builds", column: "build_id", on_delete: :cascade
   add_foreign_key "self_managed_prometheus_alert_events", "environments", on_delete: :cascade
   add_foreign_key "self_managed_prometheus_alert_events", "projects", on_delete: :cascade
   add_foreign_key "sentry_issues", "issues", on_delete: :cascade

ee/app/models/ee/ci/build.rb

@@ -21,6 +21,8 @@ module EE
         include UsageStatistics
         include FromUnion
 
+        has_many :security_scans, class_name: 'Security::Scan'
+
         after_save :stick_build_if_status_changed
         delegate :service_specification, to: :runner_session, allow_nil: true
 

ee/app/models/ee/ci/pipeline.rb

@@ -24,6 +24,7 @@ module EE
         has_many :auto_canceled_jobs, class_name: 'CommitStatus', foreign_key: 'auto_canceled_by_id'
 
         has_many :downstream_bridges, class_name: '::Ci::Bridge', foreign_key: :upstream_pipeline_id
+        has_many :security_scans, class_name: 'Security::Scan', through: :builds
 
         has_one :source_bridge, through: :source_pipeline, source: :source_bridge
 

ee/app/models/security/scan.rb

+# frozen_string_literal: true
+
+module Security
+  class Scan < ApplicationRecord
+    self.table_name = 'security_scans'
+
+    validates :build_id, presence: true
+    validates :scan_type, presence: true
+
+    belongs_to :build, class_name: 'Ci::Build'
+    has_one :pipeline, class_name: 'Ci::Pipeline', through: :build
+
+    enum scan_type: {
+      sast: 1,
+      dependency_scanning: 2,
+      container_scanning: 3,
+      dast: 4
+    }
+  end
+end

ee/app/services/security/store_scans_service.rb

+# frozen_string_literal: true
+
+module Security
+  class StoreScansService
+    def initialize(build)
+      @build = build
+    end
+
+    def execute
+      return if @build.canceled? || @build.skipped?
+
+      security_reports = @build.job_artifacts.security_reports
+
+      ActiveRecord::Base.transaction do
+        security_reports.each do |report|
+          Security::Scan.safe_find_or_create_by!(
+            build: @build,
+            scan_type: report.file_type
+          )
+        end
+      end
+    end
+  end
+end

ee/app/workers/all_queues.yml

@@ -357,6 +357,12 @@
   :latency_sensitive: true
   :resource_boundary: :cpu
   :weight: 3
+- :name: security_scans:store_security_scans
+  :feature_category: :static_application_security_testing
+  :has_external_dependencies: 
+  :latency_sensitive: 
+  :resource_boundary: :unknown
+  :weight: 2
 - :name: adjourned_project_deletion
   :feature_category: :authentication_and_authorization
   :has_external_dependencies: 

ee/app/workers/ee/build_finished_worker.rb

@@ -8,6 +8,8 @@ module EE
       # and `Namespace#namespace_statistics` will return stale data.
       CiMinutesUsageNotifyService.new(build.project.reset).execute
 
+      StoreSecurityScansWorker.perform_async(build.id)
+
       super
     end
   end

ee/app/workers/store_security_scans_worker.rb

+# frozen_string_literal: true
+
+class StoreSecurityScansWorker
+  include ApplicationWorker
+
+  queue_namespace :security_scans
+  feature_category :static_application_security_testing
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def perform(build_id)
+    ::Ci::Build.find_by(id: build_id).try do |build|
+      break if build.job_artifacts.security_reports.empty?
+
+      Security::StoreScansService.new(build).execute
+    end
+  end
+end

ee/spec/factories/security_scans.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :security_scan, class: 'Security::Scan' do
+    scan_type { 'dast' }
+    build factory: :ci_build
+  end
+end

ee/spec/models/ci/build_spec.rb

@@ -34,6 +34,10 @@ describe Ci::Build do
     end
   end
 
+  describe 'associations' do
+    it { is_expected.to have_many(:security_scans) }
+  end
+
   describe '#shared_runners_minutes_limit_enabled?' do
     subject { job.shared_runners_minutes_limit_enabled? }
 

ee/spec/models/ci/pipeline_spec.rb

@@ -12,6 +12,7 @@ describe Ci::Pipeline do
     create(:ci_empty_pipeline, status: :created, project: project)
   end
 
+  it { is_expected.to have_many(:security_scans).through(:builds).class_name('Security::Scan') }
   it { is_expected.to have_many(:downstream_bridges) }
   it { is_expected.to have_many(:job_artifacts).through(:builds) }
   it { is_expected.to have_many(:vulnerability_findings).through(:vulnerabilities_occurrence_pipelines).class_name('Vulnerabilities::Occurrence') }

ee/spec/models/security/scan_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Security::Scan do
+  describe 'associations' do
+    it { is_expected.to belong_to(:build) }
+    it { is_expected.to have_one(:pipeline).through(:build).class_name('Ci::Pipeline') }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:build_id) }
+    it { is_expected.to validate_presence_of(:scan_type) }
+  end
+
+  it_behaves_like 'having unique enum values'
+end

ee/spec/services/security/store_scans_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Security::StoreScansService do
+  let(:build) { create(:ci_build) }
+
+  subject { Security::StoreScansService.new(build).execute }
+
+  context 'build has security reports' do
+    before do
+      create(:ee_ci_job_artifact, :dast, job: build)
+      create(:ee_ci_job_artifact, :sast, job: build)
+      create(:ee_ci_job_artifact, :codequality, job: build)
+    end
+
+    it 'saves security scans' do
+      subject
+
+      scans = Security::Scan.where(build: build)
+      expect(scans.count).to be(2)
+      expect(scans.sast.count).to be(1)
+      expect(scans.dast.count).to be(1)
+    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')
+    end
+
+    it 'does not save' do
+      subject
+
+      expect(Security::Scan.where(build: build).count).to be(1)
+    end
+  end
+end

ee/spec/workers/build_finished_worker_spec.rb

@@ -42,5 +42,11 @@ describe BuildFinishedWorker do
 
       subject
     end
+
+    it 'stores security scans' do
+      expect(StoreSecurityScansWorker).to receive(:perform_async)
+
+      subject
+    end
   end
 end

ee/spec/workers/store_security_scans_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe StoreSecurityScansWorker do
+  describe '#perform' do
+    context 'build has security reports' do
+      let(:build) { create(:ci_build, :dast) }
+
+      before do
+        create(:ee_ci_job_artifact, :dast, job: build)
+      end
+
+      it 'stores security scans' do
+        expect(Security::StoreScansService).to receive(:new).with(build).and_call_original
+
+        StoreSecurityScansWorker.new.perform(build.id)
+      end
+    end
+
+    context 'build does not have security reports' do
+      let(:build) { create(:ci_build) }
+
+      it 'does not store security scans' do
+        expect(Security::StoreScansService).not_to receive(:new)
+
+        StoreSecurityScansWorker.new.perform(build.id)
+      end
+    end
+
+    context 'build does not exist' do
+      it 'does not store security scans' do
+        expect(Security::StoreScansService).not_to receive(:new)
+
+        StoreSecurityScansWorker.new.perform(666)
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -215,6 +215,7 @@ ci_pipelines:
 - vulnerabilities_occurrence_pipelines
 - vulnerability_findings
 - pipeline_config
+- security_scans
 pipeline_variables:
 - pipeline
 stages:

spec/services/ci/retry_build_service_spec.rb

@@ -46,7 +46,7 @@ describe Ci::RetryBuildService do
        sourced_pipelines artifacts_file_store artifacts_metadata_store
        metadata runner_session trace_chunks upstream_pipeline_id
        artifacts_file artifacts_metadata artifacts_size commands
-       resource resource_group_id processed].freeze
+       resource resource_group_id processed security_scans].freeze
 
   shared_examples 'build duplication' do
     let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }