Add latest changes from gitlab-org/security/gitlab@13-5-stable-ee

6 files changed, 178 insertions, 70 deletions

diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index 66ed80a7d61..451526021c1 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/spec/lib/gitlab/regex_spec.rb
@@ -368,6 +368,21 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.not_to match('%2e%2e%2f1.2.3') }
   end
 
+  describe '.nuget_package_name_regex' do
+    subject { described_class.nuget_package_name_regex }
+
+    it { is_expected.to match('My.Package') }
+    it { is_expected.to match('My.Package.Mvc') }
+    it { is_expected.to match('MyPackage') }
+    it { is_expected.to match('My.23.Package') }
+    it { is_expected.to match('My23Package') }
+    it { is_expected.to match('runtime.my-test64.runtime.package.Mvc') }
+    it { is_expected.to match('my_package') }
+    it { is_expected.not_to match('My/package') }
+    it { is_expected.not_to match('../../../my_package') }
+    it { is_expected.not_to match('%2e%2e%2fmy_package') }
+  end
+
   describe '.pypi_version_regex' do
     subject { described_class.pypi_version_regex }
 
diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb
index ca408303524..41a731b87e9 100644
--- a/spec/models/packages/package_spec.rb
+++ b/spec/models/packages/package_spec.rb
@@ -122,6 +122,21 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('my file name').for(:name) }
         it { is_expected.not_to allow_value('!!().for(:name)().for(:name)').for(:name) }
       end
+
+      context 'nuget package' do
+        subject { build_stubbed(:nuget_package) }
+
+        it { is_expected.to allow_value('My.Package').for(:name) }
+        it { is_expected.to allow_value('My.Package.Mvc').for(:name) }
+        it { is_expected.to allow_value('MyPackage').for(:name) }
+        it { is_expected.to allow_value('My.23.Package').for(:name) }
+        it { is_expected.to allow_value('My23Package').for(:name) }
+        it { is_expected.to allow_value('runtime.my-test64.runtime.package.Mvc').for(:name) }
+        it { is_expected.to allow_value('my_package').for(:name) }
+        it { is_expected.not_to allow_value('My/package').for(:name) }
+        it { is_expected.not_to allow_value('../../../my_package').for(:name) }
+        it { is_expected.not_to allow_value('%2e%2e%2fmy_package').for(:name) }
+      end
     end
 
     describe '#version' do
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index d66ef81efca..b28fb9a0255 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -137,7 +137,7 @@ RSpec.describe ProjectPolicy do
       it 'disallows all permissions except pipeline when the feature is disabled' do
         builds_permissions = [
           :create_build, :read_build, :update_build, :admin_build, :destroy_build,
-          :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
+          :create_pipeline_schedule, :read_pipeline_schedule_variables, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
           :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
           :create_cluster, :read_cluster, :update_cluster, :admin_cluster, :destroy_cluster,
           :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb
index e0199b7b51c..4c8a356469d 100644
--- a/spec/requests/api/ci/pipeline_schedules_spec.rb
+++ b/spec/requests/api/ci/pipeline_schedules_spec.rb
@@ -97,46 +97,112 @@ RSpec.describe API::Ci::PipelineSchedules do
       pipeline_schedule.pipelines << build(:ci_pipeline, project: project)
     end
 
-    context 'authenticated user with valid permissions' do
-      it 'returns pipeline_schedule details' do
-        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer)
-
+    matcher :return_pipeline_schedule_sucessfully do
+      match_unless_raises do |reponse|
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to match_response_schema('pipeline_schedule')
       end
+    end
 
-      it 'responds with 404 Not Found if requesting non-existing pipeline_schedule' do
-        get api("/projects/#{project.id}/pipeline_schedules/-5", developer)
+    shared_context 'request with project permissions' do
+      context 'authenticated user with project permisions' do
+        before do
+          project.add_maintainer(user)
+        end
 
-        expect(response).to have_gitlab_http_status(:not_found)
+        it 'returns pipeline_schedule details' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+
+          expect(response).to return_pipeline_schedule_sucessfully
+          expect(json_response).to have_key('variables')
+        end
       end
     end
 
-    context 'authenticated user with invalid permissions' do
-      it 'does not return pipeline_schedules list' do
-        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+    shared_examples 'request with schedule ownership' do
+      context 'authenticated user with pipeline schedule ownership' do
+        it 'returns pipeline_schedule details' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", developer)
 
-        expect(response).to have_gitlab_http_status(:not_found)
+          expect(response).to return_pipeline_schedule_sucessfully
+          expect(json_response).to have_key('variables')
+        end
       end
     end
 
-    context 'authenticated user with insufficient permissions' do
-      before do
-        project.add_guest(user)
+    shared_examples 'request with unauthenticated user' do
+      context 'with unauthenticated user' do
+        it 'does not return pipeline_schedule' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}")
+
+          expect(response).to have_gitlab_http_status(:unauthorized)
+        end
       end
+    end
 
-      it 'does not return pipeline_schedules list' do
-        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+    shared_examples 'request with non-existing pipeline_schedule' do
+      it 'responds with 404 Not Found if requesting non-existing pipeline_schedule' do
+        get api("/projects/#{project.id}/pipeline_schedules/-5", developer)
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
 
-    context 'unauthenticated user' do
-      it 'does not return pipeline_schedules list' do
-        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}")
+    context 'with private project' do
+      it_behaves_like 'request with schedule ownership'
+      it_behaves_like 'request with project permissions'
+      it_behaves_like 'request with unauthenticated user'
+      it_behaves_like 'request with non-existing pipeline_schedule'
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
+      context 'authenticated user with no project permissions' do
+        it 'does not return pipeline_schedule' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'authenticated user with insufficient project permissions' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'does not return pipeline_schedule' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+
+    context 'with public project' do
+      let_it_be(:project) { create(:project, :repository, :public, public_builds: false) }
+
+      it_behaves_like 'request with schedule ownership'
+      it_behaves_like 'request with project permissions'
+      it_behaves_like 'request with unauthenticated user'
+      it_behaves_like 'request with non-existing pipeline_schedule'
+
+      context 'authenticated user with no project permissions' do
+        it 'returns pipeline_schedule with no variables' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+
+          expect(response).to return_pipeline_schedule_sucessfully
+          expect(json_response).not_to have_key('variables')
+        end
+      end
+
+      context 'authenticated user with insufficient project permissions' do
+        before do
+          project.add_guest(user)
+        end
+
+        it 'returns pipeline_schedule with no variables' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+
+          expect(response).to return_pipeline_schedule_sucessfully
+          expect(json_response).not_to have_key('variables')
+        end
       end
     end
   end
diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb
index b7c780c1ee2..92b493ed376 100644
--- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb
+++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb
@@ -198,24 +198,26 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
       it_behaves_like 'raising an', ::Packages::Nuget::MetadataExtractionService::ExtractionError
     end
 
-    context 'with package file with a blank package name' do
-      before do
-        allow(service).to receive(:package_name).and_return('')
-      end
+    context 'with an invalid package name' do
+      invalid_names = [
+        '',
+        'My/package',
+        '../../../my_package',
+        '%2e%2e%2fmy_package'
+      ]
 
-      it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
-    end
+      invalid_names.each do |invalid_name|
+        before do
+          allow(service).to receive(:package_name).and_return(invalid_name)
+        end
 
-    context 'with package file with a blank package version' do
-      before do
-        allow(service).to receive(:package_version).and_return('')
+        it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
       end
-
-      it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
     end
 
     context 'with an invalid package version' do
       invalid_versions = [
+        '',
         '555',
         '1.2',
         '1./2.3',
@@ -224,13 +226,11 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_
       ]
 
       invalid_versions.each do |invalid_version|
-        it "raises an error for version #{invalid_version}" do
+        before do
           allow(service).to receive(:package_version).and_return(invalid_version)
-
-          expect { subject }.to raise_error(ActiveRecord::RecordInvalid, 'Validation failed: Version is invalid')
-          expect(package_file.file_name).not_to include(invalid_version)
-          expect(package_file.file.file.path).not_to include(invalid_version)
         end
+
+        it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
       end
     end
   end
diff --git a/spec/workers/packages/nuget/extraction_worker_spec.rb b/spec/workers/packages/nuget/extraction_worker_spec.rb
index 35b5f1baed5..4703afc9413 100644
--- a/spec/workers/packages/nuget/extraction_worker_spec.rb
+++ b/spec/workers/packages/nuget/extraction_worker_spec.rb
@@ -13,6 +13,18 @@ RSpec.describe Packages::Nuget::ExtractionWorker, type: :worker do
 
     subject { described_class.new.perform(package_file_id) }
 
+    shared_examples 'handling the metadata error' do |exception_class: ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError|
+      it 'removes the package and the package file' do
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
+          instance_of(exception_class),
+          project_id: package.project_id
+        )
+        expect { subject }
+          .to change { Packages::Package.count }.by(-1)
+          .and change { Packages::PackageFile.count }.by(-1)
+      end
+    end
+
     context 'with valid package file' do
       it 'updates package and package file' do
         expect { subject }
@@ -48,46 +60,46 @@ RSpec.describe Packages::Nuget::ExtractionWorker, type: :worker do
         allow_any_instance_of(Zip::File).to receive(:glob).and_return([])
       end
 
-      it 'removes the package and the package file' do
-        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
-          instance_of(::Packages::Nuget::MetadataExtractionService::ExtractionError),
-          project_id: package.project_id
-        )
-        expect { subject }
-          .to change { Packages::Package.count }.by(-1)
-          .and change { Packages::PackageFile.count }.by(-1)
-      end
+      it_behaves_like 'handling the metadata error', exception_class: ::Packages::Nuget::MetadataExtractionService::ExtractionError
     end
 
-    context 'with package file with a blank package name' do
-      before do
-        allow_any_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService).to receive(:package_name).and_return('')
-      end
+    context 'with package with an invalid package name' do
+      invalid_names = [
+        '',
+        'My/package',
+        '../../../my_package',
+        '%2e%2e%2fmy_package'
+      ]
 
-      it 'removes the package and the package file' do
-        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
-          instance_of(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError),
-          project_id: package.project_id
-        )
-        expect { subject }
-          .to change { Packages::Package.count }.by(-1)
-          .and change { Packages::PackageFile.count }.by(-1)
+      invalid_names.each do |invalid_name|
+        before do
+          allow_next_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService) do |service|
+            allow(service).to receive(:package_name).and_return(invalid_name)
+          end
+        end
+
+        it_behaves_like 'handling the metadata error'
       end
     end
 
-    context 'with package file with a blank package version' do
-      before do
-        allow_any_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService).to receive(:package_version).and_return('')
-      end
+    context 'with package with an invalid package version' do
+      invalid_versions = [
+        '',
+        '555',
+        '1.2',
+        '1./2.3',
+        '../../../../../1.2.3',
+        '%2e%2e%2f1.2.3'
+      ]
 
-      it 'removes the package and the package file' do
-        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
-          instance_of(::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError),
-          project_id: package.project_id
-        )
-        expect { subject }
-          .to change { Packages::Package.count }.by(-1)
-          .and change { Packages::PackageFile.count }.by(-1)
+      invalid_versions.each do |invalid_version|
+        before do
+          allow_next_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService) do |service|
+            allow(service).to receive(:package_version).and_return(invalid_version)
+          end
+        end
+
+        it_behaves_like 'handling the metadata error'
       end
     end
   end