Add association between Project and Vulnerabilities::Remediation

We have to scope remediation records to projects to prevent leaking
some confidential information through summary attribute to other
projects, just in case if user adds confidential information to that
attribute.

changelogs/unreleased/239177_scope_remediations_to_projects.yml

+---
+title: Add `project_id` column into the `vulnerability_remediations` table to scope
+  the records with projects
+merge_request: 49219
+author:
+type: added

db/migrate/20201204085522_add_project_id_into_vulnerability_remediations.rb

+# frozen_string_literal: true
+
+class AddProjectIdIntoVulnerabilityRemediations < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    connection.execute('DELETE FROM vulnerability_remediations')
+
+    add_column :vulnerability_remediations, :project_id, :bigint, null: false # rubocop:disable Rails/NotNullColumn
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :vulnerability_remediations, :project_id
+    end
+  end
+end

db/migrate/20201204090855_add_compound_index_to_vulnerability_remediations_table.rb

+# frozen_string_literal: true
+
+class AddCompoundIndexToVulnerabilityRemediationsTable < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  NEW_INDEX_NAME = 'index_vulnerability_remediations_on_project_id_and_checksum'
+  OLD_INDEX_NAME = 'index_vulnerability_remediations_on_checksum'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :vulnerability_remediations, [:project_id, :checksum], unique: true, name: NEW_INDEX_NAME
+    add_concurrent_foreign_key :vulnerability_remediations, :projects, column: :project_id
+
+    remove_concurrent_index_by_name :vulnerability_remediations, OLD_INDEX_NAME
+  end
+
+  def down
+    add_concurrent_index :vulnerability_remediations, :checksum, unique: true, name: OLD_INDEX_NAME
+
+    remove_concurrent_index_by_name :vulnerability_remediations, NEW_INDEX_NAME
+
+    with_lock_retries do
+      remove_foreign_key_if_exists :vulnerability_remediations, column: :project_id
+    end
+  end
+end

db/schema_migrations/20201204085522

+a1d8228731066fb6dfe436b4d8d034353421d1f45f3896e963f3c7f15fb09fbc
\ No newline at end of file

db/schema_migrations/20201204090855

+01712e32d95578fe701738529abfa0e051ef68ed646f7a9c7f775f8a8d108578
\ No newline at end of file

db/structure.sql

@@ -17584,6 +17584,7 @@ CREATE TABLE vulnerability_remediations (
     summary text NOT NULL,
     file text NOT NULL,
     checksum bytea NOT NULL,
+    project_id bigint NOT NULL,
     CONSTRAINT check_ac0ccabff3 CHECK ((char_length(summary) <= 200)),
     CONSTRAINT check_fe3325e3ba CHECK ((char_length(file) <= 255))
 );
@@ -22669,7 +22670,7 @@ CREATE UNIQUE INDEX index_vulnerability_occurrences_on_uuid ON vulnerability_occ
 
 CREATE INDEX index_vulnerability_occurrences_on_vulnerability_id ON vulnerability_occurrences USING btree (vulnerability_id);
 
-CREATE UNIQUE INDEX index_vulnerability_remediations_on_checksum ON vulnerability_remediations USING btree (checksum);
+CREATE UNIQUE INDEX index_vulnerability_remediations_on_project_id_and_checksum ON vulnerability_remediations USING btree (project_id, checksum);
 
 CREATE UNIQUE INDEX index_vulnerability_scanners_on_project_id_and_external_id ON vulnerability_scanners USING btree (project_id, external_id);
 
@@ -23692,6 +23693,9 @@ ALTER TABLE ONLY ci_stages
 ALTER TABLE ONLY system_note_metadata
     ADD CONSTRAINT fk_fbd87415c9 FOREIGN KEY (description_version_id) REFERENCES description_versions(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY vulnerability_remediations
+    ADD CONSTRAINT fk_fc61a535a0 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY merge_requests
     ADD CONSTRAINT fk_fd82eae0b9 FOREIGN KEY (head_pipeline_id) REFERENCES ci_pipelines(id) ON DELETE SET NULL;
 

ee/app/models/ee/project.rb

@@ -78,6 +78,7 @@ module EE
       has_many :vulnerability_identifiers, class_name: 'Vulnerabilities::Identifier'
       has_many :vulnerability_scanners, class_name: 'Vulnerabilities::Scanner'
       has_many :vulnerability_exports, class_name: 'Vulnerabilities::Export'
+      has_many :vulnerability_remediations, class_name: 'Vulnerabilities::Remediation', inverse_of: :project
 
       has_many :dast_site_profiles
       has_many :dast_site_tokens

ee/app/models/vulnerabilities/remediation.rb

@@ -9,6 +9,8 @@ module Vulnerabilities
 
     sha_attribute :checksum
 
+    belongs_to :project, optional: false
+
     has_many :finding_remediations, class_name: 'Vulnerabilities::FindingRemediation', inverse_of: :remediation, foreign_key: 'vulnerability_remediation_id'
     has_many :findings, through: :finding_remediations
 

ee/app/services/security/store_report_service.rb

@@ -144,7 +144,7 @@ module Security
     def find_existing_remediations_for(finding)
       checksums = finding.remediations.map(&:checksum)
 
-      Vulnerabilities::Remediation.by_checksum(checksums)
+      @project.vulnerability_remediations.by_checksum(checksums)
     end
 
     def build_new_remediations_for(finding, existing_remediations)
@@ -159,7 +159,7 @@ module Security
     end
 
     def build_vulnerability_remediation(remediation)
-      Vulnerabilities::Remediation.new(summary: remediation.summary, file: remediation.diff_file, checksum: remediation.checksum)
+      @project.vulnerability_remediations.new(summary: remediation.summary, file: remediation.diff_file, checksum: remediation.checksum)
     end
 
     def create_vulnerability_pipeline_object(vulnerability_finding, pipeline)

ee/spec/factories/vulnerabilities/remediations.rb

@@ -2,6 +2,7 @@
 
 FactoryBot.define do
   factory :vulnerabilities_remediation, class: 'Vulnerabilities::Remediation' do
+    project
     summary { 'Remediation Summary' }
     file { Tempfile.new }
 

ee/spec/models/project_spec.rb

@@ -45,6 +45,7 @@ RSpec.describe Project do
     it { is_expected.to have_many(:downstream_project_subscriptions) }
     it { is_expected.to have_many(:downstream_projects) }
     it { is_expected.to have_many(:vulnerability_historical_statistics).class_name('Vulnerabilities::HistoricalStatistic') }
+    it { is_expected.to have_many(:vulnerability_remediations).class_name('Vulnerabilities::Remediation') }
 
     it { is_expected.to have_one(:github_service) }
     it { is_expected.to have_many(:project_aliases) }

ee/spec/models/vulnerabilities/remediation_spec.rb

@@ -3,6 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Vulnerabilities::Remediation do
+  it { is_expected.to belong_to(:project).required }
   it { is_expected.to have_many(:finding_remediations).class_name('Vulnerabilities::FindingRemediation') }
   it { is_expected.to have_many(:findings).through(:finding_remediations) }
 

ee/spec/services/security/store_report_service_spec.rb

@@ -30,6 +30,11 @@ RSpec.describe Security::StoreReportService, '#execute' do
   subject { described_class.new(pipeline, report).execute }
 
   context 'without existing data' do
+    before(:all) do
+      checksum = 'f00bc6261fa512f0960b7fc3bfcce7fb31997cf32b96fa647bed5668b2c77fee'
+      create(:vulnerabilities_remediation, checksum: checksum)
+    end
+
     before do
       project.add_developer(user)
       allow(pipeline).to receive(:user).and_return(user)
@@ -66,7 +71,7 @@ RSpec.describe Security::StoreReportService, '#execute' do
       end
 
       it 'inserts all remediations' do
-        expect { subject }.to change { Vulnerabilities::Remediation.count }.by(remediations)
+        expect { subject }.to change { project.vulnerability_remediations.count }.by(remediations)
       end
 
       it 'inserts all vulnerabilties' do

spec/lib/gitlab/import_export/all_models.yml

@@ -552,6 +552,7 @@ project:
 - build_report_results
 - vulnerability_statistic
 - vulnerability_historical_statistics
+- vulnerability_remediations
 - product_analytics_events
 - pipeline_artifacts
 - terraform_states