New upstream version 13.3.9
This commit is contained in:
parent
c7883f9ba0
commit
01760414a6
38 changed files with 479 additions and 114 deletions
15
CHANGELOG.md
15
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)
|
||||
|
|
|
@ -1 +1 @@
|
|||
13.3.8
|
||||
13.3.9
|
2
VERSION
2
VERSION
|
@ -1 +1 @@
|
|||
13.3.8
|
||||
13.3.9
|
|
@ -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 -->
|
||||
|
|
|
@ -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? }
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -32,6 +32,8 @@ module Packages
|
|||
)
|
||||
end
|
||||
end
|
||||
rescue ActiveRecord::RecordInvalid => e
|
||||
raise InvalidMetadataError.new(e.message)
|
||||
end
|
||||
|
||||
private
|
||||
|
|
|
@ -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) &&
|
||||
|
|
|
@ -23,6 +23,8 @@ module Terraform
|
|||
|
||||
state.save! unless state.destroyed?
|
||||
end
|
||||
|
||||
nil
|
||||
end
|
||||
|
||||
def lock!
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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'
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
|
||||
##
|
||||
|
|
|
@ -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
|
||||
|
|
49
spec/features/file_uploads/multipart_invalid_uploads_spec.rb
Normal file
49
spec/features/file_uploads/multipart_invalid_uploads_spec.rb
Normal file
|
@ -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
|
|
@ -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|
|
||||
|
|
|
@ -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 }
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
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'
|
||||
|
||||
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 permissions' do
|
||||
context 'authenticated user with insufficient project permissions' do
|
||||
before do
|
||||
project.add_guest(user)
|
||||
end
|
||||
|
||||
it 'does not return pipeline_schedules list' 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
|
||||
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 public project' do
|
||||
let_it_be(:project) { create(:project, :repository, :public, public_builds: false) }
|
||||
|
||||
expect(response).to have_gitlab_http_status(:unauthorized)
|
||||
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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
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('')
|
||||
allow(service).to receive(:package_name).and_return(invalid_name)
|
||||
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)
|
||||
end
|
||||
|
||||
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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -24,9 +24,14 @@ RSpec.describe ImportExportUploader do
|
|||
|
||||
include_context 'with storage', described_class::Store::REMOTE
|
||||
|
||||
it_behaves_like 'builds correct paths',
|
||||
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
|
||||
|
|
|
@ -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
|
||||
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_any_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService).to receive(:package_name).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)
|
||||
allow_next_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService) do |service|
|
||||
allow(service).to receive(:package_name).and_return(invalid_name)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with package file with a blank package version' do
|
||||
it_behaves_like 'handling the metadata error'
|
||||
end
|
||||
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'
|
||||
]
|
||||
|
||||
invalid_versions.each do |invalid_version|
|
||||
before do
|
||||
allow_any_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService).to receive(:package_version).and_return('')
|
||||
allow_next_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService) do |service|
|
||||
allow(service).to receive(:package_version).and_return(invalid_version)
|
||||
end
|
||||
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
|
||||
|
|
Loading…
Reference in a new issue