Merge branch '34431-add-report-type-vulnerabilities' into 'master'

Resolve "Vulnerabilities must have report_type attribute"

Closes #34431

See merge request gitlab-org/gitlab!19179

changelogs/unreleased/34431-add-report-type-vulnerabilities.yml

+---
+title: Added report_type attribute to Vulnerabilities
+merge_request: 19179
+author:
+type: changed

db/migrate/20191105094558_add_report_type_to_vulnerabilities.rb

+# frozen_string_literal: true
+
+class AddReportTypeToVulnerabilities < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :vulnerabilities, :report_type, :integer, limit: 2
+  end
+end

db/post_migrate/20191105094625_set_report_type_for_vulnerabilities.rb

+# frozen_string_literal: true
+
+class SetReportTypeForVulnerabilities < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def up
+    # set report_type based on associated vulnerability_occurrences
+    execute <<~SQL
+    UPDATE vulnerabilities
+    SET report_type = vulnerability_occurrences.report_type
+    FROM vulnerability_occurrences
+    WHERE vulnerabilities.id = vulnerability_occurrences.vulnerability_id
+    SQL
+
+    # set default report_type for orphan vulnerabilities (there should be none but...)
+    execute 'UPDATE vulnerabilities SET report_type = 0 WHERE report_type IS NULL'
+
+    change_column_null :vulnerabilities, :report_type, false
+  end
+
+  def down
+    change_column_null :vulnerabilities, :report_type, true
+
+    execute 'UPDATE vulnerabilities SET report_type = NULL'
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_10_29_191901) do
+ActiveRecord::Schema.define(version: 2019_11_05_094625) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -3915,6 +3915,7 @@ ActiveRecord::Schema.define(version: 2019_10_29_191901) do
     t.boolean "severity_overridden", default: false
     t.integer "confidence", limit: 2, null: false
     t.boolean "confidence_overridden", default: false
+    t.integer "report_type", limit: 2, null: false
     t.index ["author_id"], name: "index_vulnerabilities_on_author_id"
     t.index ["closed_by_id"], name: "index_vulnerabilities_on_closed_by_id"
     t.index ["due_date_sourcing_milestone_id"], name: "index_vulnerabilities_on_due_date_sourcing_milestone_id"

ee/app/models/vulnerabilities/occurrence.rb

@@ -123,7 +123,7 @@ module Vulnerabilities
           report_type: report_type,
           project_fingerprint: project_fingerprints
         )
-        .select('report_type, vulnerability_id, project_fingerprint, raw_metadata, '\
+        .select('vulnerability_occurrences.report_type, vulnerability_id, project_fingerprint, raw_metadata, '\
                 'vulnerabilities.id, vulnerabilities.state') # fetching only required attributes
     end
 

ee/app/models/vulnerability.rb

@@ -15,8 +15,9 @@ class Vulnerability < ApplicationRecord
   enum state: { opened: 1, closed: 2 }
   enum severity: Vulnerabilities::Occurrence::SEVERITY_LEVELS, _prefix: :severity
   enum confidence: Vulnerabilities::Occurrence::CONFIDENCE_LEVELS, _prefix: :confidence
+  enum report_type: Vulnerabilities::Occurrence::REPORT_TYPES
 
-  validates :project, :author, :title, :title_html, :severity, :confidence, presence: true
+  validates :project, :author, :title, :title_html, :severity, :confidence, :report_type, presence: true
 
   # at this stage Vulnerability is not an Issuable, has some important attributes (and their constraints) in common
   validates :title, length: { maximum: Issuable::TITLE_LENGTH_MAX }

ee/app/serializers/vulnerability_entity.rb

@@ -8,6 +8,7 @@ class VulnerabilityEntity < Grape::Entity
   expose :state
   expose :severity
   expose :confidence
+  expose :report_type
 
   expose :project, using: ::ProjectEntity
 

ee/spec/factories/vulnerabilities.rb

@@ -8,6 +8,7 @@ FactoryBot.define do
     title_html { "<h2>#{title}</h2>" }
     severity { :high }
     confidence { :medium }
+    report_type { :sast }
 
     trait :opened do
       state { :opened }
@@ -24,6 +25,7 @@ FactoryBot.define do
           :vulnerabilities_occurrence,
           2,
           vulnerability: vulnerability,
+          report_type: vulnerability.report_type,
           project: vulnerability.project)
       end
     end

ee/spec/fixtures/api/schemas/vulnerability.json

 {
   "type": "object",
-  "required": ["title", "state", "confidence", "severity", "project", "author_id"],
+  "required": ["title", "state", "confidence", "severity", "report_type", "project", "author_id"],
   "properties": {
     "title": {
       "type": "string"
@@ -24,6 +24,15 @@
         "confirmed"
       ]
     },
+    "report_type": {
+      "type": "string",
+      "enum": [
+        "sast",
+        "dependency_scanning",
+        "container_scanning",
+        "dast"
+      ]
+    },
     "project": {
       "required": ["id", "name", "full_path", "full_name"],
       "id": {

ee/spec/migrations/set_report_type_for_vulnerabilities_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20191105094625_set_report_type_for_vulnerabilities.rb')
+
+describe SetReportTypeForVulnerabilities, :migration do
+  let(:confidence_levels) do
+    { undefined: 0, ignore: 1, unknown: 2, experimental: 3, low: 4, medium: 5, high: 6, confirmed: 7 }
+  end
+  let(:severity_levels) { { undefined: 0, info: 1, unknown: 2, low: 4, medium: 5, high: 6, critical: 7 } }
+  let(:report_types) { { sast: 0, dependency_scanning: 1, container_scanning: 2, dast: 3 } }
+
+  let(:users) { table(:users) }
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:vulnerabilities) { table(:vulnerabilities) }
+  let(:scanners) { table(:vulnerability_scanners) }
+  let(:identifiers) { table(:vulnerability_identifiers) }
+  let(:findings) { table(:vulnerability_occurrences) }
+
+  def fingerprint
+    hash = String.new('', capacity: 40)
+    40.times { hash << rand(16).to_s(16) }
+    hash
+  end
+
+  def bin_fingerprint
+    [fingerprint].pack('H*')
+  end
+
+  before do
+    author = users.create!(id: 1, projects_limit: 10)
+
+    namespace = namespaces.create!(id: 1, name: 'namespace_1', path: 'namespace_1', owner_id: author.id)
+
+    project = projects.create!(id: 1, creator_id: author.id, namespace_id: namespace.id)
+
+    vulnerabilities_common_attrs = { project_id: project.id, author_id: author.id, severity: severity_levels[:high],
+                                     confidence: confidence_levels[:medium], report_type: nil }
+
+    vulnerability_1 =
+      vulnerabilities.create!(id: 1, title: 'finding_1', title_html: 'finding_1', **vulnerabilities_common_attrs)
+    vulnerability_2 =
+      vulnerabilities.create!(id: 2, title: 'finding_2', title_html: 'finding_2', **vulnerabilities_common_attrs)
+    vulnerabilities.create!(id: 3, title: 'orphan', title_html: 'orphan', **vulnerabilities_common_attrs)
+
+    identifiers_common_attrs = { project_id: project.id, external_type: 'SECURITY_ID' }
+
+    identifier_1 =
+      identifiers.create!(id: 1, fingerprint: '1111111111111111111111111111111111111111', external_id: 'SECURITY_1',
+                          name: 'SECURITY_IDENTIFIER 1', **identifiers_common_attrs)
+    identifier_2 =
+      identifiers.create!(id: 2, fingerprint: '2222222222222222222222222222222222222222', external_id: 'SECURITY_2',
+                          name: 'SECURITY_IDENTIFIER 2', **identifiers_common_attrs)
+    identifier_3 =
+      identifiers.create!(id: 3, fingerprint: '3333333333333333333333333333333333333333', external_id: 'SECURITY_3',
+                          name: 'SECURITY_IDENTIFIER 3', **identifiers_common_attrs)
+    identifier_4 =
+      identifiers.create!(id: 4, fingerprint: '4444444444444444444444444444444444444444', external_id: 'SECURITY_4',
+                          name: 'SECURITY_IDENTIFIER 4', **identifiers_common_attrs)
+
+    scanner = scanners.create!(id: 1, project_id: project.id, name: 'scanner', external_id: 'SCANNER_ID')
+
+    findings_common_attrs =
+      { project_id: project.id, scanner_id: scanner.id, severity: severity_levels[:high],
+        confidence: confidence_levels[:medium], metadata_version: 'sast:1.0', raw_metadata: '{}' }
+
+    findings.create!(
+      id: 1, report_type: report_types[:sast], name: 'finding_1', primary_identifier_id: identifier_1.id,
+      uuid: fingerprint[0..35], vulnerability_id: vulnerability_1.id, project_fingerprint: bin_fingerprint,
+      location_fingerprint: bin_fingerprint, **findings_common_attrs)
+    findings.create!(
+      id: 2, report_type: report_types[:dependency_scanning], name: 'finding_1_extra',
+      primary_identifier_id: identifier_2.id, uuid: fingerprint[0..35], vulnerability_id: vulnerability_1.id,
+      project_fingerprint: bin_fingerprint, location_fingerprint: bin_fingerprint, **findings_common_attrs)
+    findings.create!(
+      id: 3, report_type: report_types[:container_scanning], name: 'finding_2', primary_identifier_id: identifier_3.id,
+      uuid: fingerprint[0..35], vulnerability_id: vulnerability_2.id, project_fingerprint: bin_fingerprint,
+      location_fingerprint: bin_fingerprint, **findings_common_attrs)
+    findings.create!(
+      id: 4, report_type: report_types[:dast], name: 'finding_orphan', primary_identifier_id: identifier_4.id,
+      uuid: fingerprint[0..35], project_fingerprint: bin_fingerprint, location_fingerprint: bin_fingerprint,
+      **findings_common_attrs)
+  end
+
+  describe '#up' do
+    it 'updates vulnerabilities.report_type from their first linked findings' do
+      expect(vulnerabilities.all).to all have_attributes(report_type: nil)
+
+      migrate!
+
+      expect(vulnerabilities.find(1).report_type).to eq(findings.find(1).report_type)
+      expect(vulnerabilities.find(2).report_type).to eq(findings.find(3).report_type)
+    end
+
+    it 'sets the default report_type for orphan vulnerabilities' do
+      expect(vulnerabilities.all).to all have_attributes(report_type: nil)
+
+      migrate!
+
+      expect(vulnerabilities.find(3).report_type).to eq report_types[:sast]
+    end
+  end
+
+  describe '#down' do
+    it 'rolls back the vulnerabilities.report_type to NULL values' do
+      migrate!
+
+      expect(vulnerabilities.find(1).report_type).to eq(findings.find(1).report_type)
+      expect(vulnerabilities.find(2).report_type).to eq(findings.find(3).report_type)
+      expect(vulnerabilities.find(3).report_type).to eq report_types[:sast]
+
+      schema_migrate_down!
+
+      expect(vulnerabilities.all).to all have_attributes(report_type: nil)
+    end
+  end
+end

ee/spec/models/vulnerability_spec.rb

@@ -3,6 +3,9 @@
 require 'spec_helper'
 
 describe Vulnerability do
+  let(:state_values) do
+    { opened: 1, closed: 2 }
+  end
   let(:severity_values) do
     { undefined: 0,
       info: 1,
@@ -22,10 +25,17 @@ describe Vulnerability do
       high: 6,
       confirmed: 7 }
   end
+  let(:report_types) do
+    { sast: 0,
+      dependency_scanning: 1,
+      container_scanning: 2,
+      dast: 3 }
+  end
 
-  it { is_expected.to define_enum_for(:state) }
+  it { is_expected.to define_enum_for(:state).with_values(state_values) }
   it { is_expected.to define_enum_for(:severity).with_values(severity_values).with_prefix(:severity) }
   it { is_expected.to define_enum_for(:confidence).with_values(confidence_values).with_prefix(:confidence) }
+  it { is_expected.to define_enum_for(:report_type).with_values(report_types) }
 
   describe 'associations' do
     subject { build(:vulnerability) }
@@ -49,6 +59,7 @@ describe Vulnerability do
     it { is_expected.to validate_presence_of(:title_html) }
     it { is_expected.to validate_presence_of(:severity) }
     it { is_expected.to validate_presence_of(:confidence) }
+    it { is_expected.to validate_presence_of(:report_type) }
 
     it { is_expected.to validate_length_of(:title).is_at_most(::Issuable::TITLE_LENGTH_MAX) }
     it { is_expected.to validate_length_of(:title_html).is_at_most(::Issuable::TITLE_HTML_LENGTH_MAX) }