Merge branch '238156_create_scan_findings_entity' into 'master'

Introduce `Security::Finding` model

See merge request gitlab-org/gitlab!40368

changelogs/unreleased/238156_create_scan_findings_entity.yml

+---
+title: Create `security_findings` table
+merge_request: 40368
+author:
+type: added

db/migrate/20200825084819_create_security_findings_table.rb

+# frozen_string_literal: true
+
+class CreateSecurityFindingsTable < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless table_exists?(:security_findings)
+      create_table :security_findings do |t|
+        t.references :scan, null: false
+        t.references :scanner, null: false
+        t.integer :severity, limit: 2, index: true, null: false
+        t.integer :confidence, limit: 2, index: true, null: false
+        t.text :project_fingerprint, index: true, null: false
+      end
+    end
+
+    add_text_limit :security_findings, :project_fingerprint, 40
+  end
+
+  def down
+    drop_table :security_findings
+  end
+end

db/migrate/20200828155134_add_foreign_key_on_scan_id_to_security_scans.rb

+# frozen_string_literal: true
+
+class AddForeignKeyOnScanIdToSecurityScans < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :security_findings, :security_scans, column: :scan_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :security_findings, column: :scan_id
+    end
+  end
+end

db/migrate/20200828155205_add_foreign_key_on_scanner_id_to_vulnerability_scanners.rb

+# frozen_string_literal: true
+
+class AddForeignKeyOnScannerIdToVulnerabilityScanners < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_foreign_key :security_findings, :vulnerability_scanners, column: :scanner_id, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :security_findings, column: :scanner_id
+    end
+  end
+end

db/schema_migrations/20200825084819

+d5e81848257b3391d99b198b177531a4c190ca6f19b27c9aedaa931f6eb3165a
\ No newline at end of file

db/schema_migrations/20200828155134

+b3ee994231a8da694dbcda227b37e19a2112be666648d918425b064ec19d239e
\ No newline at end of file

db/schema_migrations/20200828155205

+b575558752206149171a05231e4167e1ac3e1295f76d800edfe3d61c1b996b52
\ No newline at end of file

db/structure.sql

@@ -15387,6 +15387,25 @@ CREATE SEQUENCE public.scim_oauth_access_tokens_id_seq
 
 ALTER SEQUENCE public.scim_oauth_access_tokens_id_seq OWNED BY public.scim_oauth_access_tokens.id;
 
+CREATE TABLE public.security_findings (
+    id bigint NOT NULL,
+    scan_id bigint NOT NULL,
+    scanner_id bigint NOT NULL,
+    severity smallint NOT NULL,
+    confidence smallint NOT NULL,
+    project_fingerprint text NOT NULL,
+    CONSTRAINT check_b9508c6df8 CHECK ((char_length(project_fingerprint) <= 40))
+);
+
+CREATE SEQUENCE public.security_findings_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE public.security_findings_id_seq OWNED BY public.security_findings.id;
+
 CREATE TABLE public.security_scans (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -17386,6 +17405,8 @@ ALTER TABLE ONLY public.scim_identities ALTER COLUMN id SET DEFAULT nextval('pub
 
 ALTER TABLE ONLY public.scim_oauth_access_tokens ALTER COLUMN id SET DEFAULT nextval('public.scim_oauth_access_tokens_id_seq'::regclass);
 
+ALTER TABLE ONLY public.security_findings ALTER COLUMN id SET DEFAULT nextval('public.security_findings_id_seq'::regclass);
+
 ALTER TABLE ONLY public.security_scans ALTER COLUMN id SET DEFAULT nextval('public.security_scans_id_seq'::regclass);
 
 ALTER TABLE ONLY public.self_managed_prometheus_alert_events ALTER COLUMN id SET DEFAULT nextval('public.self_managed_prometheus_alert_events_id_seq'::regclass);
@@ -18648,6 +18669,9 @@ ALTER TABLE ONLY public.scim_identities
 ALTER TABLE ONLY public.scim_oauth_access_tokens
     ADD CONSTRAINT scim_oauth_access_tokens_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY public.security_findings
+    ADD CONSTRAINT security_findings_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY public.security_scans
     ADD CONSTRAINT security_scans_pkey PRIMARY KEY (id);
 
@@ -20820,6 +20844,16 @@ CREATE INDEX index_secure_ci_builds_on_user_id_created_at_parser_features ON pub
 
 CREATE INDEX index_security_ci_builds_on_name_and_id_parser_features ON public.ci_builds USING btree (name, id) WHERE (((name)::text = ANY (ARRAY[('container_scanning'::character varying)::text, ('dast'::character varying)::text, ('dependency_scanning'::character varying)::text, ('license_management'::character varying)::text, ('sast'::character varying)::text, ('secret_detection'::character varying)::text, ('coverage_fuzzing'::character varying)::text, ('license_scanning'::character varying)::text])) AND ((type)::text = 'Ci::Build'::text));
 
+CREATE INDEX index_security_findings_on_confidence ON public.security_findings USING btree (confidence);
+
+CREATE INDEX index_security_findings_on_project_fingerprint ON public.security_findings USING btree (project_fingerprint);
+
+CREATE INDEX index_security_findings_on_scan_id ON public.security_findings USING btree (scan_id);
+
+CREATE INDEX index_security_findings_on_scanner_id ON public.security_findings USING btree (scanner_id);
+
+CREATE INDEX index_security_findings_on_severity ON public.security_findings USING btree (severity);
+
 CREATE INDEX index_self_managed_prometheus_alert_events_on_environment_id ON public.self_managed_prometheus_alert_events USING btree (environment_id);
 
 CREATE INDEX index_sent_notifications_on_noteable_type_noteable_id ON public.sent_notifications USING btree (noteable_id) WHERE ((noteable_type)::text = 'Issue'::text);
@@ -22697,6 +22731,9 @@ ALTER TABLE ONLY public.list_user_preferences
 ALTER TABLE ONLY public.project_custom_attributes
     ADD CONSTRAINT fk_rails_719c3dccc5 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.security_findings
+    ADD CONSTRAINT fk_rails_729b763a54 FOREIGN KEY (scanner_id) REFERENCES public.vulnerability_scanners(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.dast_scanner_profiles
     ADD CONSTRAINT fk_rails_72a8ba7141 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
 
@@ -23030,6 +23067,9 @@ ALTER TABLE ONLY public.approval_project_rules_users
 ALTER TABLE ONLY public.lists
     ADD CONSTRAINT fk_rails_baed5f39b7 FOREIGN KEY (milestone_id) REFERENCES public.milestones(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.security_findings
+    ADD CONSTRAINT fk_rails_bb63863cf1 FOREIGN KEY (scan_id) REFERENCES public.security_scans(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.approval_merge_request_rules_users
     ADD CONSTRAINT fk_rails_bc8972fa55 FOREIGN KEY (user_id) REFERENCES public.users(id) ON DELETE CASCADE;
 

ee/app/models/security/finding.rb

+# frozen_string_literal: true
+
+# This model represents the vulnerability findings
+# discovered for all pipelines to use in pipeline
+# security tab.
+#
+# Unlike `Vulnerabilities::Finding` model, this one
+# only stores some important meta information to
+# calculate which report artifact to download and parse.
+module Security
+  class Finding < ApplicationRecord
+    self.table_name = 'security_findings'
+
+    belongs_to :scan, inverse_of: :findings, optional: false
+    belongs_to :scanner, class_name: 'Vulnerabilities::Scanner', inverse_of: :security_findings, optional: false
+
+    # TODO: These are duplicated between this model and Vulnerabilities::Finding,
+    # we should create a shared module to encapculate this in one place.
+    enum confidence: Vulnerabilities::Finding::CONFIDENCE_LEVELS, _prefix: :confidence
+    enum severity: Vulnerabilities::Finding::SEVERITY_LEVELS, _prefix: :severity
+
+    validates :project_fingerprint, presence: true, length: { maximum: 40 }
+  end
+end

ee/app/models/security/scan.rb

@@ -8,8 +8,11 @@ module Security
     validates :scan_type, presence: true
 
     belongs_to :build, class_name: 'Ci::Build'
+
     has_one :pipeline, class_name: 'Ci::Pipeline', through: :build
 
+    has_many :findings, inverse_of: :scan
+
     enum scan_type: {
       sast: 1,
       dependency_scanning: 2,

ee/app/models/vulnerabilities/scanner.rb

@@ -5,6 +5,7 @@ module Vulnerabilities
     self.table_name = "vulnerability_scanners"
 
     has_many :findings, class_name: 'Vulnerabilities::Finding', inverse_of: :scanner
+    has_many :security_findings, class_name: 'Security::Finding', inverse_of: :scanner
 
     belongs_to :project
 

ee/spec/models/security/finding_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::Finding do
+  describe 'associations' do
+    it { is_expected.to belong_to(:scan).required }
+    it { is_expected.to belong_to(:scanner).required }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:project_fingerprint) }
+    it { is_expected.to validate_length_of(:project_fingerprint).is_at_most(40) }
+  end
+end

ee/spec/models/security/scan_spec.rb

@@ -6,6 +6,7 @@ RSpec.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') }
+    it { is_expected.to have_many(:findings) }
   end
 
   describe 'validations' do

ee/spec/models/vulnerabilities/scanner_spec.rb

@@ -4,8 +4,9 @@ require 'spec_helper'
 
 RSpec.describe Vulnerabilities::Scanner do
   describe 'associations' do
-    it { is_expected.to have_many(:findings).class_name('Vulnerabilities::Finding') }
     it { is_expected.to belong_to(:project) }
+    it { is_expected.to have_many(:findings).class_name('Vulnerabilities::Finding') }
+    it { is_expected.to have_many(:security_findings).class_name('Security::Finding') }
   end
 
   describe 'validations' do