From 01760414a669de6e305d27ef6bd1ad9bacec17fc Mon Sep 17 00:00:00 2001
From: Abraham Raji <avronr@tuta.io>
Date: Thu, 5 Nov 2020 12:06:23 +0530
Subject: [PATCH] New upstream version 13.3.9

---
 CHANGELOG.md                                  |  15 +++
 GITALY_SERVER_VERSION                         |   2 +-
 VERSION                                       |   2 +-
 .../javascripts/jobs/components/job_app.vue   |   7 +-
 app/models/packages/package.rb                |   1 +
 app/policies/ci/pipeline_schedule_policy.rb   |   1 +
 app/serializers/build_details_entity.rb       |   2 +-
 .../update_package_from_metadata_service.rb   |   2 +
 app/services/projects/transfer_service.rb     |  12 +-
 .../terraform/remote_state_handler.rb         |   2 +
 app/uploaders/gitlab_uploader.rb              |  26 +++++
 app/uploaders/job_artifact_uploader.rb        |   5 +-
 .../packages/package_file_uploader.rb         |   2 +
 app/views/admin/runners/_runner.html.haml     |   4 +-
 config/routes/admin.rb                        |   4 +-
 lib/api/ci/pipeline_schedules.rb              |   2 +-
 .../entities/ci/pipeline_schedule_details.rb  |   4 +-
 lib/api/internal/kubernetes.rb                |   2 +-
 lib/api/terraform/state.rb                    |   7 +-
 lib/gitlab/middleware/multipart.rb            |  14 +++
 lib/gitlab/regex.rb                           |   6 +-
 lib/gitlab/utils.rb                           |  30 +++++
 .../multipart_invalid_uploads_spec.rb         |  49 ++++++++
 spec/lib/gitlab/middleware/multipart_spec.rb  |   8 +-
 spec/lib/gitlab/regex_spec.rb                 |  28 ++++-
 spec/lib/gitlab/utils_spec.rb                 |  31 +++++
 spec/models/packages/package_spec.rb          |  15 +++
 spec/policies/project_policy_spec.rb          |   2 +-
 .../api/ci/pipeline_schedules_spec.rb         | 108 ++++++++++++++----
 spec/requests/api/internal/kubernetes_spec.rb |  12 +-
 spec/requests/api/terraform/state_spec.rb     |   7 +-
 ...date_package_from_metadata_service_spec.rb |  36 +++---
 .../terraform/remote_state_handler_spec.rb    |  18 +--
 .../features/file_uploads_shared_context.rb   |   7 ++
 .../features/file_uploads_shared_examples.rb  |  15 +++
 .../gitlab_uploader_shared_examples.rb        |  12 ++
 spec/uploaders/import_export_uploader_spec.rb |  11 +-
 .../packages/nuget/extraction_worker_spec.rb  |  82 +++++++------
 38 files changed, 479 insertions(+), 114 deletions(-)
 create mode 100644 spec/features/file_uploads/multipart_invalid_uploads_spec.rb
 create mode 100644 spec/support/shared_contexts/features/file_uploads_shared_context.rb
 create mode 100644 spec/support/shared_examples/features/file_uploads_shared_examples.rb

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fe68016cbb..af3c353b18 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,21 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 13.3.9 (2020-11-02)
+
+### Security (9 changes)
+
+- Add CSRF protection to runner pause and resume. !1021
+- Do not expose Terraform state record in API.
+- Path traversal to RCE via LFS upload.
+- Update container_repository_name_regex to prevent catastrophic backtracking.
+- Validate nuget package names.
+- Prevent private repo from being accessed via internal Kubernetes API.
+- Validate each upload param key in multipart.rb.
+- Fix XSS vulnerability for job build dependencies.
+- Fix unauthorized user is able to access schedule pipeline variables and values.
+
+
 ## 13.3.8 (2020-10-21)
 
 ### Fixed (2 changes)
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 176df6ffec..4feaa4d3ce 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-13.3.8
\ No newline at end of file
+13.3.9
\ No newline at end of file
diff --git a/VERSION b/VERSION
index 176df6ffec..4feaa4d3ce 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-13.3.8
\ No newline at end of file
+13.3.9
\ No newline at end of file
diff --git a/app/assets/javascripts/jobs/components/job_app.vue b/app/assets/javascripts/jobs/components/job_app.vue
index e760706c97..368b9fa45d 100644
--- a/app/assets/javascripts/jobs/components/job_app.vue
+++ b/app/assets/javascripts/jobs/components/job_app.vue
@@ -1,7 +1,7 @@
 <script>
 import { throttle, isEmpty } from 'lodash';
 import { mapGetters, mapState, mapActions } from 'vuex';
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlSafeHtmlDirective as SafeHtml } from '@gitlab/ui';
 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
 import { isScrolledToBottom } from '~/lib/utils/scroll_utils';
 import { polyfillSticky } from '~/lib/utils/sticky';
@@ -36,6 +36,9 @@ export default {
     GlLoadingIcon,
     SharedRunner: () => import('ee_component/jobs/components/shared_runner_limit_block.vue'),
   },
+  directives: {
+    SafeHtml,
+  },
   mixins: [delayedJobMixin],
   props: {
     runnerSettingsUrl: {
@@ -218,7 +221,7 @@ export default {
           </div>
 
           <callout v-if="shouldRenderHeaderCallout">
-            <div v-html="job.callout_message"></div>
+            <div v-safe-html="job.callout_message"></div>
           </callout>
         </header>
         <!-- EO Header Section -->
diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb
index d6633456de..e455d541b3 100644
--- a/app/models/packages/package.rb
+++ b/app/models/packages/package.rb
@@ -35,6 +35,7 @@ class Packages::Package < ApplicationRecord
   validate :package_already_taken, if: :npm?
   validates :version, format: { with: Gitlab::Regex.semver_regex }, if: -> { npm? || nuget? }
   validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
+  validates :name, format: { with: Gitlab::Regex.nuget_package_name_regex }, if: :nuget?
   validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
   validates :version, format: { with: Gitlab::Regex.maven_version_regex }, if: -> { version? && maven? }
 
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index cf3f784f85..2ef5ffd6a5 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -17,6 +17,7 @@ module Ci
     rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do
       enable :update_pipeline_schedule
       enable :admin_pipeline_schedule
+      enable :read_pipeline_schedule_variables
     end
 
     rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do
diff --git a/app/serializers/build_details_entity.rb b/app/serializers/build_details_entity.rb
index 523f1a0f8c..625aa8a017 100644
--- a/app/serializers/build_details_entity.rb
+++ b/app/serializers/build_details_entity.rb
@@ -136,7 +136,7 @@ class BuildDetailsEntity < JobEntity
     docs_url = "https://docs.gitlab.com/ce/ci/yaml/README.html#dependencies"
 
     [
-      failure_message.html_safe,
+      failure_message,
       help_message(docs_url).html_safe
     ].join("<br />")
   end
diff --git a/app/services/packages/nuget/update_package_from_metadata_service.rb b/app/services/packages/nuget/update_package_from_metadata_service.rb
index f72b138698..0109ee23c4 100644
--- a/app/services/packages/nuget/update_package_from_metadata_service.rb
+++ b/app/services/packages/nuget/update_package_from_metadata_service.rb
@@ -32,6 +32,8 @@ module Packages
             )
           end
         end
+      rescue ActiveRecord::RecordInvalid => e
+        raise InvalidMetadataError.new(e.message)
       end
 
       private
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 0fb70feec8..595a47a53e 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -79,11 +79,7 @@ module Projects
         # Directories on disk
         move_project_folders(project)
 
-        # Move missing group labels to project
-        Labels::TransferService.new(current_user, @old_group, project).execute
-
-        # Move missing group milestones
-        Milestones::TransferService.new(current_user, @old_group, project).execute
+        transfer_missing_group_resources(@old_group)
 
         # Move uploads
         move_project_uploads(project)
@@ -104,6 +100,12 @@ module Projects
       refresh_permissions
     end
 
+    def transfer_missing_group_resources(group)
+      Labels::TransferService.new(current_user, group, project).execute
+
+      Milestones::TransferService.new(current_user, group, project).execute
+    end
+
     def allowed_transfer?(current_user, project)
       @new_namespace &&
         can?(current_user, :change_namespace, project) &&
diff --git a/app/services/terraform/remote_state_handler.rb b/app/services/terraform/remote_state_handler.rb
index d2c44d4a26..7e79cb9e00 100644
--- a/app/services/terraform/remote_state_handler.rb
+++ b/app/services/terraform/remote_state_handler.rb
@@ -23,6 +23,8 @@ module Terraform
 
         state.save! unless state.destroyed?
       end
+
+      nil
     end
 
     def lock!
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 654bb15378..411d8b2614 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -5,6 +5,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
 
   class_attribute :options
 
+  PROTECTED_METHODS = %i(filename cache_dir work_dir store_dir).freeze
+
+  ObjectNotReadyError = Class.new(StandardError)
+
   class << self
     # DSL setter
     def storage_options(options)
@@ -33,6 +37,8 @@ class GitlabUploader < CarrierWave::Uploader::Base
 
   delegate :base_dir, :file_storage?, to: :class
 
+  before :cache, :protect_from_path_traversal!
+
   def initialize(model, mounted_as = nil, **uploader_context)
     super(model, mounted_as)
   end
@@ -121,6 +127,9 @@ class GitlabUploader < CarrierWave::Uploader::Base
   # For example, `FileUploader` builds the storage path based on the associated
   # project model's `path_with_namespace` value, which can change when the
   # project or its containing namespace is moved or renamed.
+  #
+  # When implementing this method, raise `ObjectNotReadyError` if the model
+  # does not yet exist, as it will be tested in `#protect_from_path_traversal!`
   def dynamic_segment
     raise(NotImplementedError)
   end
@@ -138,4 +147,21 @@ class GitlabUploader < CarrierWave::Uploader::Base
   def pathname
     @pathname ||= Pathname.new(path)
   end
+
+  # Protect against path traversal attacks
+  # This takes a list of methods to test for path traversal, e.g. ../../
+  # and checks each of them. This uses `.send` so that any potential errors
+  # don't block the entire set from being tested.
+  #
+  # @param [CarrierWave::SanitizedFile]
+  # @return [Nil]
+  # @raise [Gitlab::Utils::PathTraversalAttackError]
+  def protect_from_path_traversal!(file)
+    PROTECTED_METHODS.each do |method|
+      Gitlab::Utils.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend
+
+    rescue ObjectNotReadyError
+      # Do nothing. This test was attempted before the file was ready for that method
+    end
+  end
 end
diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb
index 47976c909e..83dc103060 100644
--- a/app/uploaders/job_artifact_uploader.rb
+++ b/app/uploaders/job_artifact_uploader.rb
@@ -4,7 +4,6 @@ class JobArtifactUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern
 
-  ObjectNotReadyError = Class.new(StandardError)
   UnknownFileLocationError = Class.new(StandardError)
 
   storage_options Gitlab.config.artifacts
@@ -24,7 +23,9 @@ class JobArtifactUploader < GitlabUploader
   private
 
   def dynamic_segment
-    raise ObjectNotReadyError, 'JobArtifact is not ready' unless model.id
+    # This now tests model.created_at because it can for some reason be nil in the test suite,
+    # and it's not clear if this is intentional or not
+    raise ObjectNotReadyError, 'JobArtifact is not ready' unless model.id && model.created_at
 
     if model.hashed_path?
       hashed_path
diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb
index 28545b9fcd..61fbe2b4c4 100644
--- a/app/uploaders/packages/package_file_uploader.rb
+++ b/app/uploaders/packages/package_file_uploader.rb
@@ -20,6 +20,8 @@ class Packages::PackageFileUploader < GitlabUploader
   private
 
   def dynamic_segment
+    raise ObjectNotReadyError, "Package model not ready" unless model.id
+
     Gitlab::HashedPath.new('packages', model.package.id, 'files', model.id, root_hash: model.package.project_id)
   end
 end
diff --git a/app/views/admin/runners/_runner.html.haml b/app/views/admin/runners/_runner.html.haml
index 0bbe73d6f7..3c963bf85e 100644
--- a/app/views/admin/runners/_runner.html.haml
+++ b/app/views/admin/runners/_runner.html.haml
@@ -69,10 +69,10 @@
           = sprite_icon('pencil')
       .btn-group
         - if runner.active?
-          = link_to [:pause, :admin, runner], method: :get, class: 'btn btn-default btn-svg has-tooltip', title: _('Pause'), ref: 'tooltip', aria: { label: _('Pause') }, data: { placement: 'top', container: 'body', confirm: _('Are you sure?') } do
+          = link_to [:pause, :admin, runner], method: :post, class: 'btn btn-default btn-svg has-tooltip', title: _('Pause'), ref: 'tooltip', aria: { label: _('Pause') }, data: { placement: 'top', container: 'body', confirm: _('Are you sure?') } do
             = sprite_icon('pause')
         - else
-          = link_to [:resume, :admin, runner], method: :get, class: 'btn btn-default btn-svg has-tooltip gl-px-3', title: _('Resume'), ref: 'tooltip', aria: { label: _('Resume') }, data: { placement: 'top', container: 'body'} do
+          = link_to [:resume, :admin, runner], method: :post, class: 'btn btn-default btn-svg has-tooltip gl-px-3', title: _('Resume'), ref: 'tooltip', aria: { label: _('Resume') }, data: { placement: 'top', container: 'body'} do
             = sprite_icon('play')
       .btn-group
         = link_to [:admin, runner], method: :delete, class: 'btn btn-danger has-tooltip', title: _('Remove'), ref: 'tooltip', aria: { label: _('Remove') }, data: { placement: 'top', container: 'body', confirm: _('Are you sure?') } do
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index f3b7fb5ed4..f39b5fb278 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -139,8 +139,8 @@ namespace :admin do
 
   resources :runners, only: [:index, :show, :update, :destroy] do
     member do
-      get :resume
-      get :pause
+      post :resume
+      post :pause
     end
 
     collection do
diff --git a/lib/api/ci/pipeline_schedules.rb b/lib/api/ci/pipeline_schedules.rb
index 1afdb0ad34..ff32f4597c 100644
--- a/lib/api/ci/pipeline_schedules.rb
+++ b/lib/api/ci/pipeline_schedules.rb
@@ -36,7 +36,7 @@ module API
           requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id'
         end
         get ':id/pipeline_schedules/:pipeline_schedule_id' do
-          present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails
+          present pipeline_schedule, with: Entities::Ci::PipelineScheduleDetails, user: current_user
         end
 
         desc 'Create a new pipeline schedule' do
diff --git a/lib/api/entities/ci/pipeline_schedule_details.rb b/lib/api/entities/ci/pipeline_schedule_details.rb
index b233728b95..d661a8940e 100644
--- a/lib/api/entities/ci/pipeline_schedule_details.rb
+++ b/lib/api/entities/ci/pipeline_schedule_details.rb
@@ -5,7 +5,9 @@ module API
     module Ci
       class PipelineScheduleDetails < PipelineSchedule
         expose :last_pipeline, using: ::API::Entities::Ci::PipelineBasic
-        expose :variables, using: ::API::Entities::Ci::Variable
+        expose :variables,
+          using: ::API::Entities::Ci::Variable,
+          if: ->(schedule, options) { Ability.allowed?(options[:user], :read_pipeline_schedule_variables, schedule) }
       end
     end
   end
diff --git a/lib/api/internal/kubernetes.rb b/lib/api/internal/kubernetes.rb
index 7f64fd7efe..f7ce2d3da8 100644
--- a/lib/api/internal/kubernetes.rb
+++ b/lib/api/internal/kubernetes.rb
@@ -77,7 +77,7 @@ module API
 
             # TODO sort out authorization for real
             # https://gitlab.com/gitlab-org/gitlab/-/issues/220912
-            if !project || !project.public?
+            unless Ability.allowed?(nil, :download_code, project)
               not_found!
             end
 
diff --git a/lib/api/terraform/state.rb b/lib/api/terraform/state.rb
index f6e966defc..c9852ddb03 100644
--- a/lib/api/terraform/state.rb
+++ b/lib/api/terraform/state.rb
@@ -56,6 +56,9 @@ module API
               state.save!
               status :ok
             end
+
+            body false
+            status :ok
           end
 
           desc 'Delete a terraform state of a certain name'
@@ -65,8 +68,10 @@ module API
 
             remote_state_handler.handle_with_lock do |state|
               state.destroy!
-              status :ok
             end
+
+            body false
+            status :ok
           end
 
           desc 'Lock a terraform state of a certain name'
diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb
index c0b671abd4..e8893c08a1 100644
--- a/lib/gitlab/middleware/multipart.rb
+++ b/lib/gitlab/middleware/multipart.rb
@@ -29,6 +29,7 @@ module Gitlab
   module Middleware
     class Multipart
       RACK_ENV_KEY = 'HTTP_GITLAB_WORKHORSE_MULTIPART_FIELDS'
+      REWRITTEN_FIELD_NAME_MAX_LENGTH = 10000.freeze
 
       class Handler
         def initialize(env, message)
@@ -39,6 +40,8 @@ module Gitlab
 
         def with_open_files
           @rewritten_fields.each do |field, tmp_path|
+            raise "invalid field: #{field.inspect}" unless valid_field_name?(field)
+
             parsed_field = Rack::Utils.parse_nested_query(field)
             raise "unexpected field: #{field.inspect}" unless parsed_field.count == 1
 
@@ -105,6 +108,17 @@ module Gitlab
 
         private
 
+        def valid_field_name?(name)
+          # length validation
+          return false if name.size >= REWRITTEN_FIELD_NAME_MAX_LENGTH
+
+          # brackets validation
+          return false if name.include?('[]') || name.start_with?('[', ']')
+          return false unless ::Gitlab::Utils.valid_brackets?(name, allow_nested: false)
+
+          true
+        end
+
         def package_allowed_paths
           packages_config = ::Gitlab.config.packages
           return [] unless allow_packages_storage_path?(packages_config)
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 2d625737e0..07bcb64810 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -46,6 +46,10 @@ module Gitlab
         maven_app_name_regex
       end
 
+      def nuget_package_name_regex
+        @nuget_package_name_regex ||= %r{\A[-+\.\_a-zA-Z0-9]+\z}.freeze
+      end
+
       def unbounded_semver_regex
         # See the official regex: https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
 
@@ -116,7 +120,7 @@ module Gitlab
     # See https://github.com/docker/distribution/blob/master/reference/regexp.go.
     #
     def container_repository_name_regex
-      @container_repository_regex ||= %r{\A[a-z0-9]+((?:[._/]|__|[-]{0,10})[a-z0-9]+)*\Z}
+      @container_repository_regex ||= %r{\A[a-z0-9]+(([._/]|__|-*)[a-z0-9])*\z}
     end
 
     ##
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index e2d93e7cd2..3df54e74b4 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -10,6 +10,8 @@ module Gitlab
     # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580
     # It also checks for ALT_SEPARATOR aka '\' (forward slash)
     def check_path_traversal!(path)
+      return unless path.is_a?(String)
+
       path = decode_path(path)
       path_regex = /(\A(\.{1,2})\z|\A\.\.[\/\\]|[\/\\]\.\.\z|[\/\\]\.\.[\/\\]|\n)/
 
@@ -208,5 +210,33 @@ module Gitlab
     def stable_sort_by(list)
       list.sort_by.with_index { |x, idx| [yield(x), idx] }
     end
+
+    # Check for valid brackets (`[` and `]`) in a string using this aspects:
+    # * open brackets count == closed brackets count
+    # * (optionally) reject nested brackets via `allow_nested: false`
+    # * open / close brackets coherence, eg. ][[] -> invalid
+    def valid_brackets?(string = '', allow_nested: true)
+      # remove everything except brackets
+      brackets = string.remove(/[^\[\]]/)
+
+      return true if brackets.empty?
+      # balanced counts check
+      return false if brackets.size.odd?
+
+      unless allow_nested
+        # nested brackets check
+        return false if brackets.include?('[[') || brackets.include?(']]')
+      end
+
+      # open / close brackets coherence check
+      untrimmed = brackets
+      loop do
+        trimmed = untrimmed.gsub('[]', '')
+        return true if trimmed.empty?
+        return false if trimmed == untrimmed
+
+        untrimmed = trimmed
+      end
+    end
   end
 end
diff --git a/spec/features/file_uploads/multipart_invalid_uploads_spec.rb b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb
new file mode 100644
index 0000000000..c7490f88db
--- /dev/null
+++ b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Invalid uploads that must be rejected', :api, :js do
+  include_context 'file upload requests helpers'
+
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user, :admin) }
+  let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
+
+  context 'invalid upload key', :capybara_ignore_server_errors do
+    let(:api_path) { "/projects/#{project.id}/packages/nuget/" }
+    let(:url) { capybara_url(api(api_path)) }
+    let(:file) { fixture_file_upload('spec/fixtures/dk.png') }
+
+    subject do
+      HTTParty.put(
+        url,
+        basic_auth: { user: user.username, password: personal_access_token.token },
+        body: body
+      )
+    end
+
+    RSpec.shared_examples 'by rejecting uploads with an invalid key' do
+      RSpec.shared_examples 'rejecting invalid keys' do |key_name:|
+        context "with invalid key #{key_name}" do
+          let(:body) { { key_name => file, 'package[test][name]' => 'test' } }
+
+          it { expect { subject }.not_to change { Packages::Package.nuget.count } }
+
+          it { expect(subject.code).to eq(500) }
+
+          it { expect(subject.body).to include("invalid field: \"#{key_name}\"") }
+        end
+      end
+
+      it_behaves_like 'rejecting invalid keys', key_name: 'package[test'
+      it_behaves_like 'rejecting invalid keys', key_name: 'package[]test'
+      it_behaves_like 'rejecting invalid keys', key_name: '[]'
+      it_behaves_like 'rejecting invalid keys', key_name: '[package]test'
+      it_behaves_like 'rejecting invalid keys', key_name: 'package][test]]'
+      it_behaves_like 'rejecting invalid keys', key_name: 'package[test[nested]]'
+      it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000
+    end
+
+    it_behaves_like 'handling file uploads', 'by rejecting uploads with an invalid key'
+  end
+end
diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb
index 3b64fe335e..68fe851326 100644
--- a/spec/lib/gitlab/middleware/multipart_spec.rb
+++ b/spec/lib/gitlab/middleware/multipart_spec.rb
@@ -122,7 +122,13 @@ RSpec.describe Gitlab::Middleware::Multipart do
       'file]',
       ';file]',
       'file]]',
-      'file;;'
+      'file;;',
+      'user[avatar',
+      '[user]avatar',
+      'user[]avatar',
+      'user[avatar[image[url]]]',
+      '[]',
+      'x' * 11000
     ]
 
     invalid_field_names.each do |invalid_field_name|
diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index afa930b795..6eb38a09f0 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/spec/lib/gitlab/regex_spec.rb
@@ -92,11 +92,16 @@ RSpec.describe Gitlab::Regex do
     it { is_expected.to match('my/awesome/image-1') }
     it { is_expected.to match('my/awesome/image.test') }
     it { is_expected.to match('my/awesome/image--test') }
-    # docker distribution allows for infinite `-`
-    # https://github.com/docker/distribution/blob/master/reference/regexp.go#L13
-    # but we have a range of 0,10 to add a reasonable limit.
-    it { is_expected.not_to match('my/image-----------test') }
+    it { is_expected.to match('my/image__test') }
+    # this example tests for catastrophic backtracking
+    it { is_expected.to match('user1/project/a_bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb------------x') }
+    it { is_expected.not_to match('user1/project/a_bbbbb-------------') }
     it { is_expected.not_to match('my/image-.test') }
+    it { is_expected.not_to match('my/image___test') }
+    it { is_expected.not_to match('my/image_.test') }
+    it { is_expected.not_to match('my/image_-test') }
+    it { is_expected.not_to match('my/image..test') }
+    it { is_expected.not_to match('my/image\ntest') }
     it { is_expected.not_to match('.my/image') }
     it { is_expected.not_to match('my/image.') }
   end
@@ -302,6 +307,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 '.semver_regex' do
     subject { described_class.semver_regex }
 
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 1eaceec1d8..36257a0605 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Utils do
+  using RSpec::Parameterized::TableSyntax
+
   delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which,
            :ensure_array_from_string, :to_exclusive_sentence, :bytes_to_megabytes,
            :append_path, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!, :decode_path, :ms_to_round_sec, to: :described_class
@@ -50,6 +52,10 @@ RSpec.describe Gitlab::Utils do
       expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb')
       expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb')
     end
+
+    it 'does nothing for a non-string' do
+      expect(check_path_traversal!(nil)).to be_nil
+    end
   end
 
   describe '.allowlisted?' do
@@ -448,4 +454,29 @@ RSpec.describe Gitlab::Utils do
       end
     end
   end
+
+  describe '.valid_brackets?' do
+    where(:input, :allow_nested, :valid) do
+      'no brackets'              | true  | true
+      'no brackets'              | false | true
+      'user[avatar]'             | true  | true
+      'user[avatar]'             | false | true
+      'user[avatar][friends]'    | true  | true
+      'user[avatar][friends]'    | false | true
+      'user[avatar[image[url]]]' | true  | true
+      'user[avatar[image[url]]]' | false | false
+      'user[avatar[]friends]'    | true  | true
+      'user[avatar[]friends]'    | false | false
+      'user[avatar]]'            | true  | false
+      'user[avatar]]'            | false | false
+      'user][avatar]]'           | true  | false
+      'user][avatar]]'           | false | false
+      'user[avatar'              | true  | false
+      'user[avatar'              | false | false
+    end
+
+    with_them do
+      it { expect(described_class.valid_brackets?(input, allow_nested: allow_nested)).to eq(valid) }
+    end
+  end
 end
diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb
index 4170bf595f..303e6cf695 100644
--- a/spec/models/packages/package_spec.rb
+++ b/spec/models/packages/package_spec.rb
@@ -107,6 +107,21 @@ RSpec.describe Packages::Package, type: :model do
         it { is_expected.not_to allow_value('.foobar').for(:name) }
         it { is_expected.not_to allow_value('%foo%bar').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 9879fc5346..4ab9e8197a 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -223,7 +223,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 e0199b7b51..4c8a356469 100644
--- a/spec/requests/api/ci/pipeline_schedules_spec.rb
+++ b/spec/requests/api/ci/pipeline_schedules_spec.rb
@@ -97,14 +97,50 @@ 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
 
+    shared_context 'request with project permissions' do
+      context 'authenticated user with project permisions' do
+        before do
+          project.add_maintainer(user)
+        end
+
+        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
+
+    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 return_pipeline_schedule_sucessfully
+          expect(json_response).to have_key('variables')
+        end
+      end
+    end
+
+    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
+
+    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)
 
@@ -112,31 +148,61 @@ RSpec.describe API::Ci::PipelineSchedules do
       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)
+    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(:not_found)
+      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 'authenticated user with insufficient permissions' do
-      before do
-        project.add_guest(user)
+    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
 
-      it 'does not return pipeline_schedules list' do
-        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+      context 'authenticated user with insufficient project permissions' do
+        before do
+          project.add_guest(user)
+        end
 
-        expect(response).to have_gitlab_http_status(:not_found)
-      end
-    end
+        it 'returns pipeline_schedule with no variables' do
+          get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
 
-    context 'unauthenticated user' do
-      it 'does not return pipeline_schedules list' do
-        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}")
-
-        expect(response).to have_gitlab_http_status(:unauthorized)
+          expect(response).to return_pipeline_schedule_sucessfully
+          expect(json_response).not_to have_key('variables')
+        end
       end
     end
   end
diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb
index 555ca441fe..485c6b8794 100644
--- a/spec/requests/api/internal/kubernetes_spec.rb
+++ b/spec/requests/api/internal/kubernetes_spec.rb
@@ -120,6 +120,16 @@ RSpec.describe API::Internal::Kubernetes do
             )
           )
         end
+
+        context 'repository is for project members only' do
+          let(:project) { create(:project, :public, :repository_private) }
+
+          it 'returns 404' do
+            get api('/internal/kubernetes/project_info'), params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
       end
 
       context 'project is private' do
@@ -144,7 +154,7 @@ RSpec.describe API::Internal::Kubernetes do
 
       context 'project does not exist' do
         it 'returns 404' do
-          get api('/internal/kubernetes/project_info'), params: { id: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }
+          get api('/internal/kubernetes/project_info'), params: { id: non_existing_record_id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb
index c47a12456c..59dba57144 100644
--- a/spec/requests/api/terraform/state_spec.rb
+++ b/spec/requests/api/terraform/state_spec.rb
@@ -125,6 +125,7 @@ RSpec.describe API::Terraform::State do
           expect { request }.to change { Terraform::State.count }.by(0)
 
           expect(response).to have_gitlab_http_status(:ok)
+          expect(Gitlab::Json.parse(response.body)).to be_empty
         end
 
         context 'on Unicorn', :unicorn do
@@ -132,6 +133,7 @@ RSpec.describe API::Terraform::State do
             expect { request }.to change { Terraform::State.count }.by(0)
 
             expect(response).to have_gitlab_http_status(:ok)
+            expect(Gitlab::Json.parse(response.body)).to be_empty
           end
         end
       end
@@ -167,6 +169,7 @@ RSpec.describe API::Terraform::State do
           expect { request }.to change { Terraform::State.count }.by(1)
 
           expect(response).to have_gitlab_http_status(:ok)
+          expect(Gitlab::Json.parse(response.body)).to be_empty
         end
 
         context 'on Unicorn', :unicorn do
@@ -174,6 +177,7 @@ RSpec.describe API::Terraform::State do
             expect { request }.to change { Terraform::State.count }.by(1)
 
             expect(response).to have_gitlab_http_status(:ok)
+            expect(Gitlab::Json.parse(response.body)).to be_empty
           end
         end
       end
@@ -206,10 +210,11 @@ RSpec.describe API::Terraform::State do
     context 'with maintainer permissions' do
       let(:current_user) { maintainer }
 
-      it 'deletes the state' do
+      it 'deletes the state and returns empty body' do
         expect { request }.to change { Terraform::State.count }.by(-1)
 
         expect(response).to have_gitlab_http_status(:ok)
+        expect(Gitlab::Json.parse(response.body)).to be_empty
       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 b7c780c1ee..92b493ed37 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('')
+    context 'with an invalid package name' do
+      invalid_names = [
+        '',
+        'My/package',
+        '../../../my_package',
+        '%2e%2e%2fmy_package'
+      ]
+
+      invalid_names.each do |invalid_name|
+        before do
+          allow(service).to receive(:package_name).and_return(invalid_name)
+        end
+
+        it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
       end
-
-      it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
-    end
-
-    context 'with package file with a blank package version' do
-      before do
-        allow(service).to receive(:package_version).and_return('')
-      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/services/terraform/remote_state_handler_spec.rb b/spec/services/terraform/remote_state_handler_spec.rb
index c47367feb1..ca392849d4 100644
--- a/spec/services/terraform/remote_state_handler_spec.rb
+++ b/spec/services/terraform/remote_state_handler_spec.rb
@@ -42,17 +42,17 @@ RSpec.describe Terraform::RemoteStateHandler do
 
     describe '#handle_with_lock' do
       it 'allows to modify a state using database locking' do
-        state = subject.handle_with_lock do |state|
+        record = nil
+        subject.handle_with_lock do |state|
+          record = state
           state.name = 'updated-name'
         end
 
-        expect(state.name).to eq 'updated-name'
+        expect(record.reload.name).to eq 'updated-name'
       end
 
-      it 'returns the state object itself' do
-        state = subject.handle_with_lock
-
-        expect(state.name).to eq 'my-state'
+      it 'returns nil' do
+        expect(subject.handle_with_lock).to be_nil
       end
     end
 
@@ -70,11 +70,13 @@ RSpec.describe Terraform::RemoteStateHandler do
       it 'handles a locked state using exclusive read lock' do
         handler.lock!
 
-        state = handler.handle_with_lock do |state|
+        record = nil
+        handler.handle_with_lock do |state|
+          record = state
           state.name = 'new-name'
         end
 
-        expect(state.name).to eq 'new-name'
+        expect(record.reload.name).to eq 'new-name'
       end
 
       it 'raises exception if lock has not been acquired before' do
diff --git a/spec/support/shared_contexts/features/file_uploads_shared_context.rb b/spec/support/shared_contexts/features/file_uploads_shared_context.rb
new file mode 100644
index 0000000000..972d25e81d
--- /dev/null
+++ b/spec/support/shared_contexts/features/file_uploads_shared_context.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+RSpec.shared_context 'file upload requests helpers' do
+  def capybara_url(path)
+    "http://#{Capybara.current_session.server.host}:#{Capybara.current_session.server.port}#{path}"
+  end
+end
diff --git a/spec/support/shared_examples/features/file_uploads_shared_examples.rb b/spec/support/shared_examples/features/file_uploads_shared_examples.rb
new file mode 100644
index 0000000000..17cc1fdfd2
--- /dev/null
+++ b/spec/support/shared_examples/features/file_uploads_shared_examples.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'handling file uploads' do |shared_examples_name|
+  context 'with object storage disabled' do
+    before do
+      stub_feature_flags(upload_middleware_jwt_params_handler: false)
+
+      expect_next_instance_of(Gitlab::Middleware::Multipart::Handler) do |handler|
+        expect(handler).to receive(:with_open_files).and_call_original
+      end
+    end
+
+    it_behaves_like shared_examples_name
+  end
+end
diff --git a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
index 12bcbb8b81..7126d3ace9 100644
--- a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
+++ b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb
@@ -14,6 +14,7 @@ end
 
 RSpec.shared_examples "builds correct paths" do |**patterns|
   let(:patterns) { patterns }
+  let(:fixture) { File.join('spec', 'fixtures', 'rails_sample.jpg') }
 
   before do
     allow(subject).to receive(:filename).and_return('<filename>')
@@ -55,4 +56,15 @@ RSpec.shared_examples "builds correct paths" do |**patterns|
       let(:target) { subject.class }
     end
   end
+
+  describe "path traversal exploits" do
+    before do
+      allow(subject).to receive(:filename).and_return("3bc58d54542d6a5efffa9a87554faac0254f73f675b337899ea869f6d38b7371/122../../../../../../../../.ssh/authorized_keys")
+    end
+
+    it "throws an exception" do
+      expect { subject.cache!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+      expect { subject.store!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError)
+    end
+  end
 end
diff --git a/spec/uploaders/import_export_uploader_spec.rb b/spec/uploaders/import_export_uploader_spec.rb
index b1fdcf067c..cb7a89193e 100644
--- a/spec/uploaders/import_export_uploader_spec.rb
+++ b/spec/uploaders/import_export_uploader_spec.rb
@@ -24,9 +24,14 @@ RSpec.describe ImportExportUploader do
 
     include_context 'with storage', described_class::Store::REMOTE
 
-    it_behaves_like 'builds correct paths',
-                    store_dir: %r[import_export_upload/import_file/],
-                    upload_path: %r[import_export_upload/import_file/]
+    patterns = {
+      store_dir: %r[import_export_upload/import_file/],
+      upload_path: %r[import_export_upload/import_file/]
+    }
+
+    it_behaves_like 'builds correct paths', patterns do
+      let(:fixture) { File.join('spec', 'fixtures', 'group_export.tar.gz') }
+    end
 
     describe '#move_to_store' do
       it 'returns false' do
diff --git a/spec/workers/packages/nuget/extraction_worker_spec.rb b/spec/workers/packages/nuget/extraction_worker_spec.rb
index 35b5f1baed..4703afc941 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)
+      it_behaves_like 'handling the metadata error', exception_class: ::Packages::Nuget::MetadataExtractionService::ExtractionError
+    end
+
+    context 'with package with an invalid package name' do
+      invalid_names = [
+        '',
+        'My/package',
+        '../../../my_package',
+        '%2e%2e%2fmy_package'
+      ]
+
+      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 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 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)
-      end
-    end
+      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
 
-    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
-
-      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)
+        it_behaves_like 'handling the metadata error'
       end
     end
   end