Merge branch 'false-positive' into 'master'

Make database changes to persist false_positive information

See merge request gitlab-org/gitlab!65573

db/migrate/20210707171554_create_vulnerability_flags.rb

+# frozen_string_literal: true
+
+class CreateVulnerabilityFlags < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  FALSE_POSITIVE_ENUM_VALUE = 0
+
+  disable_ddl_transaction!
+
+  def up
+    create_table_with_constraints :vulnerability_flags do |t|
+      t.timestamps_with_timezone null: false
+
+      t.references :vulnerability_occurrence, null: false, foreign_key: { on_delete: :cascade }
+
+      t.integer :flag_type, limit: 2, null: false, default: FALSE_POSITIVE_ENUM_VALUE
+
+      t.text :origin, null: false
+      t.text :description, null: false
+
+      t.text_limit :origin, 255
+      t.text_limit :description, 1024
+    end
+  end
+
+  def down
+    drop_table :vulnerability_flags
+  end
+end

db/schema_migrations/20210707171554

+5f2acbd5ed9132ad6c11cf4be34061decde2f3c602ef319331454b424e6b4344
\ No newline at end of file

db/structure.sql

@@ -19331,6 +19331,27 @@ CREATE SEQUENCE vulnerability_findings_remediations_id_seq
 
 ALTER SEQUENCE vulnerability_findings_remediations_id_seq OWNED BY vulnerability_findings_remediations.id;
 
+CREATE TABLE vulnerability_flags (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    vulnerability_occurrence_id bigint NOT NULL,
+    flag_type smallint DEFAULT 0 NOT NULL,
+    origin text NOT NULL,
+    description text NOT NULL,
+    CONSTRAINT check_45e743349f CHECK ((char_length(description) <= 1024)),
+    CONSTRAINT check_49c1d00032 CHECK ((char_length(origin) <= 255))
+);
+
+CREATE SEQUENCE vulnerability_flags_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE vulnerability_flags_id_seq OWNED BY vulnerability_flags.id;
+
 CREATE TABLE vulnerability_historical_statistics (
     id bigint NOT NULL,
     created_at timestamp with time zone NOT NULL,
@@ -20612,6 +20633,8 @@ ALTER TABLE ONLY vulnerability_finding_signatures ALTER COLUMN id SET DEFAULT ne
 
 ALTER TABLE ONLY vulnerability_findings_remediations ALTER COLUMN id SET DEFAULT nextval('vulnerability_findings_remediations_id_seq'::regclass);
 
+ALTER TABLE ONLY vulnerability_flags ALTER COLUMN id SET DEFAULT nextval('vulnerability_flags_id_seq'::regclass);
+
 ALTER TABLE ONLY vulnerability_historical_statistics ALTER COLUMN id SET DEFAULT nextval('vulnerability_historical_statistics_id_seq'::regclass);
 
 ALTER TABLE ONLY vulnerability_identifiers ALTER COLUMN id SET DEFAULT nextval('vulnerability_identifiers_id_seq'::regclass);
@@ -22342,6 +22365,9 @@ ALTER TABLE ONLY vulnerability_finding_signatures
 ALTER TABLE ONLY vulnerability_findings_remediations
     ADD CONSTRAINT vulnerability_findings_remediations_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY vulnerability_flags
+    ADD CONSTRAINT vulnerability_flags_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY vulnerability_historical_statistics
     ADD CONSTRAINT vulnerability_historical_statistics_pkey PRIMARY KEY (id);
 
@@ -25251,6 +25277,8 @@ CREATE INDEX index_vulnerability_findings_remediations_on_remediation_id ON vuln
 
 CREATE UNIQUE INDEX index_vulnerability_findings_remediations_on_unique_keys ON vulnerability_findings_remediations USING btree (vulnerability_occurrence_id, vulnerability_remediation_id);
 
+CREATE INDEX index_vulnerability_flags_on_vulnerability_occurrence_id ON vulnerability_flags USING btree (vulnerability_occurrence_id);
+
 CREATE INDEX index_vulnerability_historical_statistics_on_date_and_id ON vulnerability_historical_statistics USING btree (date, id);
 
 CREATE UNIQUE INDEX index_vulnerability_identifiers_on_project_id_and_fingerprint ON vulnerability_identifiers USING btree (project_id, fingerprint);
@@ -27901,6 +27929,9 @@ ALTER TABLE ONLY clusters_integration_prometheus
 ALTER TABLE ONLY vulnerability_occurrence_identifiers
     ADD CONSTRAINT fk_rails_e4ef6d027c FOREIGN KEY (occurrence_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY vulnerability_flags
+    ADD CONSTRAINT fk_rails_e59393b48b FOREIGN KEY (vulnerability_occurrence_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY serverless_domain_cluster
     ADD CONSTRAINT fk_rails_e59e868733 FOREIGN KEY (clusters_applications_knative_id) REFERENCES clusters_applications_knative(id) ON DELETE CASCADE;
 

ee/app/models/vulnerabilities/finding.rb

@@ -37,6 +37,8 @@ module Vulnerabilities
 
     has_many :signatures, class_name: 'Vulnerabilities::FindingSignature', inverse_of: :finding
 
+    has_many :vulnerability_flags, class_name: 'Vulnerabilities::Flag', inverse_of: :finding, foreign_key: 'vulnerability_occurrence_id'
+
     has_one :evidence, class_name: 'Vulnerabilities::Finding::Evidence', inverse_of: :finding, foreign_key: 'vulnerability_occurrence_id'
 
     serialize :config_options, Serializers::Json # rubocop:disable Cop/ActiveRecordSerialize
@@ -91,6 +93,7 @@ module Vulnerabilities
     end
 
     scope :scoped_project, -> { where('vulnerability_occurrences.project_id = projects.id') }
+    scope :eager_load_vulnerability_flags, -> { includes(:vulnerability_flags) }
 
     def self.for_pipelines_with_sha(pipelines)
       joins(:pipelines)
@@ -387,6 +390,10 @@ module Vulnerabilities
       pipelines&.last&.sha || project.default_branch
     end
 
+    def false_positive?
+      vulnerability_flags.false_positive.any?
+    end
+
     protected
 
     def primary_identifier_fingerprint

ee/app/models/vulnerabilities/flag.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  class Flag < ApplicationRecord
+    self.table_name = 'vulnerability_flags'
+
+    belongs_to :finding, class_name: 'Vulnerabilities::Finding', foreign_key: 'vulnerability_occurrence_id', inverse_of: :vulnerability_flags
+
+    validates :origin, length: { maximum: 255 }
+    validates :description, length: { maximum: 1024 }
+    validates :flag_type, presence: true
+
+    enum flag_type: {
+      false_positive: 0
+    }
+  end
+end

ee/spec/factories/vulnerabilities/flags.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :vulnerabilities_flag, class: 'Vulnerabilities::Flag' do
+    finding factory: :vulnerabilities_finding
+    origin { 'post analyzer X' }
+    description { 'static string to sink' }
+
+    trait :false_positive do
+      flag_type { Vulnerabilities::Flag.flag_types[:false_positive] }
+    end
+  end
+end

ee/spec/models/vulnerabilities/finding_spec.rb

@@ -26,6 +26,7 @@ RSpec.describe Vulnerabilities::Finding do
       it { is_expected.to have_many(:finding_identifiers).class_name('Vulnerabilities::FindingIdentifier').with_foreign_key('occurrence_id') }
       it { is_expected.to have_many(:finding_links).class_name('Vulnerabilities::FindingLink').with_foreign_key('vulnerability_occurrence_id') }
       it { is_expected.to have_many(:finding_remediations).class_name('Vulnerabilities::FindingRemediation').with_foreign_key('vulnerability_occurrence_id') }
+      it { is_expected.to have_many(:vulnerability_flags).class_name('Vulnerabilities::Flag').with_foreign_key('vulnerability_occurrence_id') }
       it { is_expected.to have_many(:remediations).through(:finding_remediations) }
       it { is_expected.to have_one(:evidence).class_name('Vulnerabilities::Finding::Evidence').with_foreign_key('vulnerability_occurrence_id') }
     end
@@ -456,6 +457,19 @@ RSpec.describe Vulnerabilities::Finding do
       end
     end
 
+    describe '#false_positive?' do
+      let_it_be(:finding) { create(:vulnerabilities_finding) }
+      let_it_be(:finding_with_fp) { create(:vulnerabilities_finding, vulnerability_flags: [create(:vulnerabilities_flag)]) }
+
+      it 'returns false if the finding does not have any false_positive' do
+        expect(finding.false_positive?).to eq(false)
+      end
+
+      it 'returns true if the finding has false_positives' do
+        expect(finding_with_fp.false_positive?).to eq(true)
+      end
+    end
+
     describe '#links' do
       let_it_be(:finding, reload: true) do
         create(

ee/spec/models/vulnerabilities/flag_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::Flag do
+  describe 'associations' do
+    it { is_expected.to belong_to(:finding).class_name('Vulnerabilities::Finding').with_foreign_key('vulnerability_occurrence_id') }
+  end
+
+  describe 'validations' do
+    it { is_expected.to validate_length_of(:origin).is_at_most(255) }
+    it { is_expected.to validate_length_of(:description).is_at_most(1024) }
+    it { is_expected.to validate_presence_of(:flag_type) }
+    it { is_expected.to define_enum_for(:flag_type).with_values(false_positive: 0) }
+  end
+end