Commit 99112fa3 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge remote-tracking branch 'dev/13-5-stable' into 13-5-stable

parents 8ee0746f 187cae1b
......@@ -2,6 +2,21 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
## 13.5.2 (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.5.1 (2020-10-22)
### Other (1 change)
......
13.5.1
\ No newline at end of file
13.5.2
\ No newline at end of file
13.5.1
\ No newline at end of file
13.5.2
\ No newline at end of file
<script>
/* eslint-disable vue/no-v-html */
import { throttle, isEmpty } from 'lodash';
import { mapGetters, mapState, mapActions } from 'vuex';
import { GlLoadingIcon, GlIcon } from '@gitlab/ui';
import { GlLoadingIcon, GlIcon, 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 +35,9 @@ export default {
GlLoadingIcon,
SharedRunner: () => import('ee_component/jobs/components/shared_runner_limit_block.vue'),
},
directives: {
SafeHtml,
},
mixins: [delayedJobMixin],
props: {
artifactHelpUrl: {
......@@ -223,7 +225,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 -->
......
......@@ -37,6 +37,7 @@ class Packages::Package < ApplicationRecord
validate :package_already_taken, if: :npm?
validates :name, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
validates :name, format: { with: Gitlab::Regex.generic_package_name_regex }, if: :generic?
validates :name, format: { with: Gitlab::Regex.nuget_package_name_regex }, if: :nuget?
validates :version, format: { with: Gitlab::Regex.semver_regex }, if: :npm?
validates :version, format: { with: Gitlab::Regex.nuget_version_regex }, if: :nuget?
validates :version, format: { with: Gitlab::Regex.conan_recipe_component_regex }, if: :conan?
......
......@@ -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/ee/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)
......@@ -107,6 +103,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: 'gl-button 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: 'gl-button 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: 'gl-button 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: 'gl-button 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: 'gl-button btn btn-danger has-tooltip', title: _('Remove'), ref: 'tooltip', aria: { label: _('Remove') }, data: { placement: 'top', container: 'body', confirm: _('Are you sure?') } do
......
......@@ -148,8 +148,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
......
......@@ -85,7 +85,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
......
......@@ -39,7 +39,6 @@ module API
env['api.format'] = :binary # this bypasses json serialization
body state.latest_file.read
status :ok
end
end
......@@ -53,8 +52,10 @@ module API
remote_state_handler.handle_with_lock do |state|
state.update_file!(CarrierWaveStringFile.new(data), version: params[:serial])
status :ok
end
body false
status :ok
end
desc 'Delete a terraform state of a certain name'
......@@ -64,8 +65,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'
......
......@@ -31,6 +31,7 @@ module Gitlab
RACK_ENV_KEY = 'HTTP_GITLAB_WORKHORSE_MULTIPART_FIELDS'
JWT_PARAM_SUFFIX = '.gitlab-workhorse-upload'
JWT_PARAM_FIXED_KEY = 'upload'
REWRITTEN_FIELD_NAME_MAX_LENGTH = 10000.freeze
class Handler
def initialize(env, message)
......@@ -41,6 +42,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
......@@ -108,6 +111,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)
......@@ -141,6 +155,8 @@ module Gitlab
class HandlerForJWTParams < Handler
def with_open_files
@rewritten_fields.keys.each do |field|
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
......
......@@ -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 nuget_version_regex
@nuget_version_regex ||= /
\A#{_semver_major_minor_patch_regex}(\.\d*)?#{_semver_prerelease_build_regex}\z
......@@ -204,7 +208,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
# 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 'rejecting invalid keys' do |key_name:, message: nil|
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(message.presence || "invalid field: \"#{key_name}\"") }
end
end
RSpec.shared_examples 'by rejecting uploads with an invalid key' do
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]]'
end
# These keys are rejected directly by rack itself.
# The request will not be received by multipart.rb (can't use the 'handling file uploads' shared example)
it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000, message: 'Puma caught this error: exceeded available parameter key space (RangeError)'
it_behaves_like 'rejecting invalid keys', key_name: 'package[]test', message: 'Puma caught this error: expected Hash (got Array)'
it_behaves_like 'handling file uploads', 'by rejecting uploads with an invalid key'
end
end
......@@ -123,15 +123,46 @@ RSpec.describe Gitlab::Middleware::Multipart do
end
end
context 'with invalid key in parameters' do
context 'with an invalid upload key' do
include_context 'with one temporary file for multipart'
let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'wrong_key', filename: filename, remote_id: remote_id) }
RSpec.shared_examples 'rejecting the invalid key' do |key_in_header:, key_in_upload_params:, error_message:|
let(:rewritten_fields) { rewritten_fields_hash(key_in_header => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: key_in_upload_params, filename: filename, remote_id: remote_id) }
it 'raises an error' do
expect { subject }.to raise_error(RuntimeError, 'Empty JWT param: file.gitlab-workhorse-upload')
end
expect { subject }.to raise_error(RuntimeError, error_message)
end
end
it_behaves_like 'rejecting the invalid key',
key_in_header: 'file',
key_in_upload_params: 'wrong_key',
error_message: 'Empty JWT param: file.gitlab-workhorse-upload'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'user[avatar',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "user[avatar"'
it_behaves_like 'rejecting the invalid key',
key_in_header: '[user]avatar',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "[user]avatar"'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'user[]avatar',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "user[]avatar"'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'user[avatar[image[url]]]',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "user[avatar[image[url]]]"'
it_behaves_like 'rejecting the invalid key',
key_in_header: '[]',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "[]"'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'x' * 11000,
key_in_upload_params: 'user[avatar]',
error_message: "invalid field: \"#{'x' * 11000}\""
end
context 'with a modified JWT payload' do
......
......@@ -139,6 +139,58 @@ RSpec.describe Gitlab::Middleware::Multipart do
subject
end
end
context 'with invalid key in header' do
include_context 'with one temporary file for multipart'
RSpec.shared_examples 'rejecting the invalid key' do |key_in_header:, key_in_upload_params:, error_message:|
let(:rewritten_fields) { rewritten_fields_hash(key_in_header => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: key_in_upload_params, filename: filename, remote_id: remote_id) }
it 'raises an error' do
expect { subject }.to raise_error(RuntimeError, error_message)
end
end
it_behaves_like 'rejecting the invalid key',
key_in_header: 'user[avatar',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "user[avatar"'
it_behaves_like 'rejecting the invalid key',
key_in_header: '[user]avatar',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "[user]avatar"'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'user[]avatar',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "user[]avatar"'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'user[avatar[image[url]]]',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "user[avatar[image[url]]]"'
it_behaves_like 'rejecting the invalid key',
key_in_header: '[]',
key_in_upload_params: 'user[avatar]',
error_message: 'invalid field: "[]"'
it_behaves_like 'rejecting the invalid key',
key_in_header: 'x' * 11000,
key_in_upload_params: 'user[avatar]',
error_message: "invalid field: \"#{'x' * 11000}\""
end
context 'with key with unbalanced brackets in header' do
include_context 'with one temporary file for multipart'
let(:invalid_key) { 'user[avatar' }
let(:rewritten_fields) { rewritten_fields_hash( invalid_key => uploaded_filepath) }
let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'user[avatar]', filename: filename, remote_id: remote_id) }
it 'builds no UploadedFile' do
expect(app).not_to receive(:call)
expect { subject }.to raise_error(RuntimeError, "invalid field: \"#{invalid_key}\"")
end
end
end
end
end
......@@ -137,11 +137,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
......@@ -363,6 +368,21 @@ RSpec.describe Gitlab::Regex do
it { is_expected.not_to match('%2e%2e%2f1.2.3') }
end
describe '.nuget_package_name_regex' do
subject { described_class.nuget_package_name_regex }
it { is_expected.to match('My.Package') }
it { is_expected.to match('My.Package.Mvc') }
it { is_expected.to match('MyPackage') }
it { is_expected.to match('My.23.Package') }
it { is_expected.to match('My23Package') }
it { is_expected.to match('runtime.my-test64.runtime.package.Mvc') }
it { is_expected.to match('my_package') }
it { is_expected.not_to match('My/package') }
it { is_expected.not_to match('../../../my_package') }
it { is_expected.not_to match('%2e%2e%2fmy_package') }
end
describe '.pypi_version_regex' do
subject { described_class.pypi_version_regex }
......
......@@ -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
......@@ -122,6 +122,21 @@ RSpec.describe Packages::Package, type: :model do
it { is_expected.not_to allow_value('my file name').for(:name) }
it { is_expected.not_to allow_value('!!().for(:name)().for(:name)').for(:name) }
end
context 'nuget package' do
subject { build_stubbed(:nuget_package) }
it { is_expected.to allow_value('My.Package').for(:name) }
it { is_expected.to allow_value('My.Package.Mvc').for(:name) }
it { is_expected.to allow_value('MyPackage').for(:name) }
it { is_expected.to allow_value('My.23.Package').for(:name) }
it { is_expected.to allow_value('My23Package').for(:name) }
it { is_expected.to allow_value('runtime.my-test64.runtime.package.Mvc').for(:name) }
it { is_expected.to allow_value('my_package').for(:name) }
it { is_expected.not_to allow_value('My/package').for(:name) }
it { is_expected.not_to allow_value('../../../my_package').for(:name) }
it { is_expected.not_to allow_value('%2e%2e%2fmy_package').for(:name) }
end
end
describe '#version' do
......
......@@ -137,7 +137,7 @@ RSpec.describe ProjectPolicy do
it 'disallows all permissions except pipeline when the feature is disabled' do
builds_permissions = [
:create_build, :read_build, :update_build, :admin_build, :destroy_build,
:create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
:create_pipeline_schedule, :read_pipeline_schedule_variables, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
:create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
:create_cluster, :read_cluster, :update_cluster, :admin_cluster, :destroy_cluster,
:create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
......
......@@ -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
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 have_gitlab_http_status(:ok)
expect(response).to match_response_schema('pipeline_schedule')
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
......
......@@ -166,6 +166,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
send_request(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
......@@ -190,7 +200,7 @@ RSpec.describe API::Internal::Kubernetes do
context 'project does not exist' do
it 'returns 404' do
send_request(params: { id: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" })
send_request(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
before do
allow(service).to receive(:package_name).and_return('')
end
it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
end
context 'with an invalid package name' do
invalid_names = [
'',
'My/package',
'../../../my_package',
'%2e%2e%2fmy_package'
]
context 'with package file with a blank package version' do
invalid_names.each do |invalid_name|
before do
allow(service).to receive(:package_version).and_return('')
allow(service).to receive(:package_name).and_return(invalid_name)
end
it_behaves_like 'raising an', ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError
end
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
......
......@@ -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
......
......@@ -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('')
allow_next_instance_of(::Packages::Nuget::UpdatePackageFromMetadataService) do |service|
allow(service).to receive(:package_name).and_return(invalid_name)
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
context 'with package file with a blank package version' do
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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment