Merge branch '345454-add-validation-for-corpus' into 'master'

Add extra validation in corpus model

See merge request gitlab-org/gitlab!74370

db/migrate/20211112073413_change_package_index_on_corpus.rb

+# frozen_string_literal: true
+
+class ChangePackageIndexOnCorpus < Gitlab::Database::Migration[1.0]
+  INDEX_NAME = 'index_coverage_fuzzing_corpuses_on_package_id'
+
+  disable_ddl_transaction!
+
+  # Changing this index is safe.
+  # The table does not have any data in it as it's behind a feature flag.
+  def up
+    remove_concurrent_index :coverage_fuzzing_corpuses, :package_id, name: INDEX_NAME
+    add_concurrent_index :coverage_fuzzing_corpuses, :package_id, unique: true, name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index :coverage_fuzzing_corpuses, :package_id, name: INDEX_NAME
+    add_concurrent_index :coverage_fuzzing_corpuses, :package_id, name: INDEX_NAME
+  end
+end

db/schema_migrations/20211112073413

+8960c0a2b7e621e466fde3bde6a252119008579c058046a16d57a6f6bff42008
\ No newline at end of file

db/structure.sql

@@ -25870,7 +25870,7 @@ CREATE INDEX index_container_repository_on_name_trigram ON container_repositorie
 
 CREATE UNIQUE INDEX index_content_blocked_states_on_container_id_commit_sha_path ON content_blocked_states USING btree (container_identifier, commit_sha, path);
 
-CREATE INDEX index_coverage_fuzzing_corpuses_on_package_id ON coverage_fuzzing_corpuses USING btree (package_id);
+CREATE UNIQUE INDEX index_coverage_fuzzing_corpuses_on_package_id ON coverage_fuzzing_corpuses USING btree (package_id);
 
 CREATE INDEX index_coverage_fuzzing_corpuses_on_project_id ON coverage_fuzzing_corpuses USING btree (project_id);
 

ee/app/models/app_sec/fuzzing/coverage/corpus.rb

@@ -6,11 +6,17 @@ module AppSec
       class Corpus < ApplicationRecord
         self.table_name = 'coverage_fuzzing_corpuses'
 
+        ACCEPTED_FORMATS = %w(.zip).freeze
+
         belongs_to :package, class_name: 'Packages::Package'
         belongs_to :user, optional: true
         belongs_to :project
 
         validate :project_same_as_package_project
+        validate :package_with_package_file
+        validate :validate_file_format
+
+        validates :package_id, uniqueness: true
 
         scope :by_project_id, -> (project_id) do
           joins(:package).where(package: { project_id: project_id })
@@ -27,6 +33,25 @@ module AppSec
             errors.add(:package_id, 'should belong to the associated project')
           end
         end
+
+        def package_with_package_file
+          unless first_package_file
+            errors.add(:package_id, 'should have an associated package file')
+          end
+        end
+
+        def validate_file_format
+          return unless first_package_file
+
+          unless ACCEPTED_FORMATS.include? File.extname(first_package_file.file_name)
+            errors.add(:package_id, 'format is not supported')
+          end
+        end
+
+        # Currently we are only supporting one package_file per package for a corpus model.
+        def first_package_file
+          @package_file ||= package.package_files.first
+        end
       end
     end
   end

ee/spec/factories/app_sec/fuzzing/coverage/corpuses.rb

@@ -4,6 +4,6 @@ FactoryBot.define do
   factory :corpus, class: 'AppSec::Fuzzing::Coverage::Corpus' do
     user
     project
-    package { association :package, project: project }
+    package { association(:generic_package, :with_zip_file, project: project) }
   end
 end

ee/spec/models/app_sec/fuzzing/coverage/corpus_spec.rb

@@ -15,14 +15,39 @@ RSpec.describe AppSec::Fuzzing::Coverage::Corpus, type: :model do
 
   describe 'validate' do
     describe 'project_same_as_package_project' do
-      let(:package_2) { create(:package) }
+      let(:package) { create(:generic_package, :with_zip_file) }
 
-      subject(:corpus) { build(:corpus, package: package_2) }
+      subject(:corpus) { build(:corpus, package: package) }
 
       it 'raises the error on adding the package of a different project' do
         expect { corpus.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Package should belong to the associated project')
       end
     end
+
+    describe 'package_with_package_file' do
+      let(:package) { create(:package) }
+
+      subject(:corpus) { build(:corpus, package: package, project: package.project) }
+
+      it 'raises the error on adding the package file with different format' do
+        expect { corpus.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Package should have an associated package file')
+      end
+    end
+
+    describe 'validate_file_format' do
+      let(:package_file) { create(:package_file) }
+      let(:package) { package_file.package }
+
+      subject(:corpus) { build(:corpus, package: package, project: package.project) }
+
+      it 'raises the error on adding the package file with different format' do
+        expect { corpus.save! }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Package format is not supported')
+      end
+    end
+  end
+
+  describe 'validates' do
+    it { is_expected.to validate_uniqueness_of(:package_id) }
   end
 
   describe 'scopes' do

ee/spec/services/app_sec/fuzzing/coverage/corpuses/create_service_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe AppSec::Fuzzing::Coverage::Corpuses::CreateService do
   let_it_be(:project) { create(:project) }
   let_it_be(:developer) { create(:user, developer_projects: [project] ) }
-  let_it_be(:package) { create(:package, project: project, creator: developer) }
+  let_it_be(:package) { create(:generic_package, :with_zip_file, project: project, creator: developer) }
 
   let_it_be(:default_params) do
     {

spec/factories/packages/package_files.rb

@@ -323,6 +323,14 @@ FactoryBot.define do
       size { 1149.bytes }
     end
 
+    trait(:generic_zip) do
+      package
+      file_fixture { 'spec/fixtures/packages/generic/myfile.zip' }
+      file_name { "#{package.name}.zip" }
+      file_sha256 { '3559e770bd493b326e8ec5e6242f7206d3fbf94fa47c16f82d34a037daa113e5' }
+      size { 3989.bytes }
+    end
+
     trait(:object_storage) do
       file_store { Packages::PackageFileUploader::Store::REMOTE }
     end

spec/factories/packages/packages.rb

@@ -247,6 +247,12 @@ FactoryBot.define do
       sequence(:name) { |n| "generic-package-#{n}" }
       version { '1.0.0' }
       package_type { :generic }
+
+      trait(:with_zip_file) do
+        after :create do |package|
+          create :package_file, :generic_zip, package: package
+        end
+      end
     end
   end
 end