New upstream version 15.6.8+ds1

This commit is contained in:
Pirate Praveen 2023-02-26 17:17:37 +05:30
parent aef0f23401
commit 13ea837db1
36 changed files with 846 additions and 432 deletions

View file

@ -77,6 +77,8 @@ workflow:
when: never when: never
# For stable, auto-deploy, and security branches, create a pipeline. # For stable, auto-deploy, and security branches, create a pipeline.
- if: '$CI_COMMIT_BRANCH =~ /^[\d-]+-stable(-ee)?$/' - if: '$CI_COMMIT_BRANCH =~ /^[\d-]+-stable(-ee)?$/'
variables:
NOTIFY_PIPELINE_FAILURE_CHANNEL: "releases"
- if: '$CI_COMMIT_BRANCH =~ /^\d+-\d+-auto-deploy-\d+$/' - if: '$CI_COMMIT_BRANCH =~ /^\d+-\d+-auto-deploy-\d+$/'
- if: '$CI_COMMIT_BRANCH =~ /^security\//' - if: '$CI_COMMIT_BRANCH =~ /^security\//'

View file

@ -2,6 +2,25 @@
documentation](doc/development/changelog.md) for instructions on adding your own documentation](doc/development/changelog.md) for instructions on adding your own
entry. entry.
## 15.6.8 (2023-02-10)
No changes.
## 15.6.7 (2023-01-30)
### Fixed (2 changes)
- [Clear DuplicateJobs cookies from post-deployment migration](gitlab-org/security/gitlab@9071bc623c81f4ecbccb63bcfc78d6d503421e2b)
- [Geo: Container Repository push events don't work](gitlab-org/security/gitlab@00ca7dd923444da0b19afa7d72d5e3b505889290)
### Security (5 changes)
- [Quarantine features/users/login_spec line 292 [15.6]](gitlab-org/security/gitlab@d202f35e1cac8df0bcbb5d40d42cea2312c09762) ([merge request](gitlab-org/security/gitlab!3025))
- [Add size validation for Chart.yaml during file extraction](gitlab-org/security/gitlab@59df02bf2658468f9f254c34ed009a6414d6c6b3) ([merge request](gitlab-org/security/gitlab!3020))
- [Prevent default branches from storing paths](gitlab-org/security/gitlab@b7b402a0a37bb839b601569a035a62fe79febe72) ([merge request](gitlab-org/security/gitlab!3013))
- [Validate Issuable description max length on update](gitlab-org/security/gitlab@fa68365e853a5701b217ccafea9885705d4a4133) ([merge request](gitlab-org/security/gitlab!3002))
- [Security fix dynamic child pipeline zip extraction](gitlab-org/security/gitlab@2285d716f10f33d8dbea5112de95d9d7e5cd8b00) ([merge request](gitlab-org/security/gitlab!2981))
## 15.6.6 (2023-01-12) ## 15.6.6 (2023-01-12)
No changes. No changes.

View file

@ -1 +1 @@
15.6.6 15.6.8

View file

@ -1 +1 @@
15.6.6 15.6.8

View file

@ -1 +1 @@
15.6.6 15.6.8

View file

@ -92,10 +92,9 @@ module Issuable
validates :author, presence: true validates :author, presence: true
validates :title, presence: true, length: { maximum: TITLE_LENGTH_MAX } validates :title, presence: true, length: { maximum: TITLE_LENGTH_MAX }
# we validate the description against DESCRIPTION_LENGTH_MAX only for Issuables being created # we validate the description against DESCRIPTION_LENGTH_MAX only for Issuables being created and on updates if
# to avoid breaking the existing Issuables which may have their descriptions longer # the description changes to avoid breaking the existing Issuables which may have their descriptions longer
validates :description, length: { maximum: DESCRIPTION_LENGTH_MAX }, allow_blank: true, on: :create validates :description, bytesize: { maximum: -> { DESCRIPTION_LENGTH_MAX } }, if: :validate_description_length?
validate :description_max_length_for_new_records_is_valid, on: :update
validate :validate_assignee_size_length, unless: :importing? validate :validate_assignee_size_length, unless: :importing?
before_validation :truncate_description_on_import! before_validation :truncate_description_on_import!
@ -229,10 +228,14 @@ module Issuable
private private
def description_max_length_for_new_records_is_valid def validate_description_length?
if new_record? && description.length > Issuable::DESCRIPTION_LENGTH_MAX return false unless description_changed?
errors.add(:description, :too_long, count: Issuable::DESCRIPTION_LENGTH_MAX)
end previous_description = changes['description'].first
# previous_description will be nil for new records
return true if previous_description.blank?
previous_description.bytesize <= DESCRIPTION_LENGTH_MAX
end end
def truncate_description_on_import! def truncate_description_on_import!

View file

@ -45,6 +45,15 @@ module Sanitizable
unless input.to_s == CGI.unescapeHTML(input.to_s) unless input.to_s == CGI.unescapeHTML(input.to_s)
record.errors.add(attr, 'cannot contain escaped HTML entities') record.errors.add(attr, 'cannot contain escaped HTML entities')
end end
# This method raises an exception on failure so perform this
# last if multiple errors should be returned.
Gitlab::Utils.check_path_traversal!(input.to_s)
rescue Gitlab::Utils::DoubleEncodingError
record.errors.add(attr, 'cannot contain escaped components')
rescue Gitlab::Utils::PathTraversalAttackError
record.errors.add(attr, "cannot contain a path traversal component")
end end
end end
end end

View file

@ -14,10 +14,11 @@ class NamespaceSetting < ApplicationRecord
validates :enabled_git_access_protocol, inclusion: { in: enabled_git_access_protocols.keys } validates :enabled_git_access_protocol, inclusion: { in: enabled_git_access_protocols.keys }
validate :default_branch_name_content
validate :allow_mfa_for_group validate :allow_mfa_for_group
validate :allow_resource_access_token_creation_for_group validate :allow_resource_access_token_creation_for_group
sanitizes! :default_branch_name
before_validation :normalize_default_branch_name before_validation :normalize_default_branch_name
chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval
@ -45,8 +46,6 @@ class NamespaceSetting < ApplicationRecord
NAMESPACE_SETTINGS_PARAMS NAMESPACE_SETTINGS_PARAMS
end end
sanitizes! :default_branch_name
def prevent_sharing_groups_outside_hierarchy def prevent_sharing_groups_outside_hierarchy
return super if namespace.root? return super if namespace.root?
@ -69,14 +68,6 @@ class NamespaceSetting < ApplicationRecord
self.default_branch_name = default_branch_name.presence self.default_branch_name = default_branch_name.presence
end end
def default_branch_name_content
return if default_branch_name.nil?
if default_branch_name.blank?
errors.add(:default_branch_name, "can not be an empty string")
end
end
def allow_mfa_for_group def allow_mfa_for_group
if namespace&.subgroup? && allow_mfa_for_subgroups == false if namespace&.subgroup? && allow_mfa_for_subgroups == false
errors.add(:allow_mfa_for_subgroups, _('is not allowed since the group is not top-level group.')) errors.add(:allow_mfa_for_subgroups, _('is not allowed since the group is not top-level group.'))

View file

@ -7,6 +7,10 @@ module Packages
class ExtractFileMetadataService class ExtractFileMetadataService
ExtractionError = Class.new(StandardError) ExtractionError = Class.new(StandardError)
# Charts must be smaller than 1M because of the storage limitations of Kubernetes objects.
# based on https://helm.sh/docs/chart_template_guide/accessing_files/
MAX_FILE_SIZE = 1.megabytes.freeze
def initialize(package_file) def initialize(package_file)
@package_file = package_file @package_file = package_file
end end
@ -42,6 +46,7 @@ module Packages
end end
raise ExtractionError, 'Chart.yaml not found within a directory' unless chart_yaml raise ExtractionError, 'Chart.yaml not found within a directory' unless chart_yaml
raise ExtractionError, 'Chart.yaml too big' if chart_yaml.size > MAX_FILE_SIZE
chart_yaml.read chart_yaml.read
ensure ensure

View file

@ -0,0 +1,22 @@
# frozen_string_literal: true
# This is workaround for
# https://gitlab.com/gitlab-org/gitlab/-/issues/388253. During a
# zero-downtime upgrade, duplicate jobs cookies can fail to get deleted.
# This post-deployment migration deletes all such cookies. This can
# cause some jobs that normally would have been deduplicated to twice
# instead of once.
class ClearDuplicateJobsCookies < Gitlab::Database::Migration[2.0]
disable_ddl_transaction!
restrict_gitlab_migration gitlab_schema: :gitlab_main
def up
Gitlab::Redis::Queues.with do |redis| # rubocop:disable Cop/RedisQueueUsage
redis.scan_each(match: "resque:gitlab:duplicate:*:cookie:v2").each_slice(100) do |keys|
redis.del(keys)
end
end
end
def down; end
end

View file

@ -0,0 +1 @@
f4ba0d1de73da2b7a912c06ca458898f3404235025089efc74aee9fc4caa511a

View file

@ -441,13 +441,27 @@ def default_min_key_size(name)
end end
``` ```
## Nightly Omnibus FIPS builds ## Omnibus FIPS packages
The Distribution team has created [nightly FIPS Omnibus builds](https://packages.gitlab.com/gitlab/nightly-fips-builds). These GitLab has a dedicated repository
GitLab builds are compiled to use the system OpenSSL instead of the Omnibus-embedded version of OpenSSL. ([`gitlab/gitlab-fips`](https://packages.gitlab.com/gitlab/gitlab-fips))
for builds of the Omnibus GitLab which are built with FIPS compliance.
These GitLab builds are compiled to use the system OpenSSL, instead of
the Omnibus-embedded version of OpenSSL. These packages are built for:
- RHEL 8 (and compatible)
- AmazonLinux 2
- Ubuntu
These are [consumed by the GitLab Environment Toolkit](#install-gitlab-with-fips-compliance) (GET).
See [the section on how FIPS builds are created](#how-fips-builds-are-created). See [the section on how FIPS builds are created](#how-fips-builds-are-created).
### Nightly Omnibus FIPS builds
The Distribution team has created [nightly FIPS Omnibus builds](https://packages.gitlab.com/gitlab/nightly-fips-builds),
which can be used for *testing* purposes. These should never be used for production environments.
## Runner ## Runner
See the [documentation on installing a FIPS-compliant GitLab Runner](https://docs.gitlab.com/runner/install/#fips-compliant-gitlab-runner). See the [documentation on installing a FIPS-compliant GitLab Runner](https://docs.gitlab.com/runner/install/#fips-compliant-gitlab-runner).

View file

@ -29,8 +29,8 @@ module API
end end
params do params do
requires :events, type: Array, desc: 'Event notifications' do requires :events, type: Array, desc: 'Event notifications' do
requires :action, type: String, desc: 'The action to perform, `push`, `delete`', requires :action, type: String, desc: 'The action to perform, `push`, `delete`, `pull`',
values: %w[push delete].freeze values: %w[push delete pull].freeze
optional :target, type: Hash, desc: 'The target of the action' do optional :target, type: Hash, desc: 'The target of the action' do
optional :tag, type: String, desc: 'The target tag' optional :tag, type: String, desc: 'The target tag'
optional :repository, type: String, desc: 'The target repository' optional :repository, type: String, desc: 'The target repository'

View file

@ -9,6 +9,7 @@ module Gitlab
Error = Class.new(StandardError) Error = Class.new(StandardError)
MAX_ARCHIVE_SIZE = 5.megabytes MAX_ARCHIVE_SIZE = 5.megabytes
TMP_ARTIFACT_EXTRACTION_DIR = "extracted_artifacts_job_%{id}"
def initialize(job) def initialize(job)
@job = job @job = job
@ -45,22 +46,22 @@ module Gitlab
end end
def read_zip_file!(file_path) def read_zip_file!(file_path)
job.artifacts_file.use_open_file do |file| dir_name = format(TMP_ARTIFACT_EXTRACTION_DIR, id: job.id.to_i)
zip_file = Zip::File.new(file, false, true)
entry = zip_file.find_entry(file_path)
unless entry job.artifacts_file.use_open_file(unlink_early: false) do |tmp_open_file|
Dir.mktmpdir(dir_name) do |tmp_dir|
SafeZip::Extract.new(tmp_open_file.file_path).extract(files: [file_path], to: tmp_dir)
File.read(File.join(tmp_dir, file_path))
end
end
rescue SafeZip::Extract::NoMatchingError
raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!"
end rescue SafeZip::Extract::EntrySizeError
raise Error, "Path `#{file_path}` has invalid size in the zip!"
if entry.name_is_directory? rescue Errno::EISDIR
raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!"
end end
zip_file.read(entry)
end
end
def max_archive_size_in_mb def max_archive_size_in_mb
ActiveSupport::NumberHelper.number_to_human_size(MAX_ARCHIVE_SIZE) ActiveSupport::NumberHelper.number_to_human_size(MAX_ARCHIVE_SIZE)
end end

View file

@ -4,6 +4,7 @@ module Gitlab
module Utils module Utils
extend self extend self
PathTraversalAttackError ||= Class.new(StandardError) PathTraversalAttackError ||= Class.new(StandardError)
DoubleEncodingError ||= Class.new(StandardError)
private_class_method def logger private_class_method def logger
@logger ||= Gitlab::AppLogger @logger ||= Gitlab::AppLogger
@ -55,7 +56,7 @@ module Gitlab
def decode_path(encoded_path) def decode_path(encoded_path)
decoded = CGI.unescape(encoded_path) decoded = CGI.unescape(encoded_path)
if decoded != CGI.unescape(decoded) if decoded != CGI.unescape(decoded)
raise StandardError, "path #{encoded_path} is not allowed" raise DoubleEncodingError, "path #{encoded_path} is not allowed"
end end
decoded decoded

View file

@ -25,8 +25,8 @@ module SafeZip
end end
def extract def extract
# do not extract if file is not part of target directory # do not extract if file is not part of target directory or target file
return false unless matching_target_directory return false unless matching_target_directory || matching_target_file
# do not overwrite existing file # do not overwrite existing file
raise SafeZip::Extract::AlreadyExistsError, "File already exists #{zip_entry.name}" if exist? raise SafeZip::Extract::AlreadyExistsError, "File already exists #{zip_entry.name}" if exist?
@ -44,6 +44,8 @@ module SafeZip
end end
rescue SafeZip::Extract::Error rescue SafeZip::Extract::Error
raise raise
rescue Zip::EntrySizeError => e
raise SafeZip::Extract::EntrySizeError, e.message
rescue StandardError => e rescue StandardError => e
raise SafeZip::Extract::ExtractError, e.message raise SafeZip::Extract::ExtractError, e.message
end end
@ -84,6 +86,10 @@ module SafeZip
params.matching_target_directory(path) params.matching_target_directory(path)
end end
def matching_target_file
params.matching_target_file(path)
end
def read_symlink def read_symlink
zip_archive.read(zip_entry) zip_archive.read(zip_entry)
end end

View file

@ -6,6 +6,7 @@ module SafeZip
PermissionDeniedError = Class.new(Error) PermissionDeniedError = Class.new(Error)
SymlinkSourceDoesNotExistError = Class.new(Error) SymlinkSourceDoesNotExistError = Class.new(Error)
UnsupportedEntryError = Class.new(Error) UnsupportedEntryError = Class.new(Error)
EntrySizeError = Class.new(Error)
AlreadyExistsError = Class.new(Error) AlreadyExistsError = Class.new(Error)
NoMatchingError = Class.new(Error) NoMatchingError = Class.new(Error)
ExtractError = Class.new(Error) ExtractError = Class.new(Error)

View file

@ -4,11 +4,13 @@ module SafeZip
class ExtractParams class ExtractParams
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
attr_reader :directories, :extract_path attr_reader :directories, :files, :extract_path
def initialize(directories:, to:) def initialize(to:, directories: [], files: [])
@directories = directories @directories = directories
@files = files
@extract_path = ::File.realpath(to) @extract_path = ::File.realpath(to)
validate!
end end
def matching_target_directory(path) def matching_target_directory(path)
@ -32,5 +34,23 @@ module SafeZip
end end
end end
end end
def matching_target_file(path)
target_files.include?(path)
end
private
def target_files
strong_memoize(:target_files) do
files.map do |file|
::File.join(extract_path, file)
end
end
end
def validate!
raise ArgumentError, 'Either directories or files are required' if directories.empty? && files.empty?
end
end end
end end

View file

@ -926,7 +926,8 @@ RSpec.describe 'Login', :clean_gitlab_redis_sessions do
stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config])
end end
it 'asks the user to accept the terms before setting an email' do it 'asks the user to accept the terms before setting an email',
quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/388049', type: :flaky } do
expect(authentication_metrics) expect(authentication_metrics)
.to increment(:user_authenticated_counter) .to increment(:user_authenticated_counter)

Binary file not shown.

Binary file not shown.

Binary file not shown.

View file

@ -10,9 +10,7 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do
subject { described_class.new(job).read(path) } subject { described_class.new(job).read(path) }
context 'when job has artifacts and metadata' do context 'when job has artifacts and metadata' do
let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } shared_examples 'extracting job artifact archive' do
let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) }
it 'returns the content at the path' do it 'returns the content at the path' do
is_expected.to be_present is_expected.to be_present
expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom')
@ -80,6 +78,54 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do
end end
end end
context 'when job artifact is on local storage' do
let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) }
let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) }
it_behaves_like 'extracting job artifact archive'
end
context 'when job artifact is on remote storage' do
before do
stub_artifacts_object_storage
stub_request(:get, %r{https://artifacts.+ci_build_artifacts\.zip})
.to_return(
status: 200,
body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts.zip')),
headers: {}
)
stub_request(:get, %r{https://artifacts.+ci_build_artifacts_metadata})
.to_return(
status: 200,
body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz')),
headers: {}
)
end
let!(:artifacts) { create(:ci_job_artifact, :archive, :remote_store, job: job) }
let!(:metadata) { create(:ci_job_artifact, :metadata, :remote_store, job: job) }
it_behaves_like 'extracting job artifact archive'
end
context 'when extracting job artifact raises entry size error' do
let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) }
let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) }
before do
allow_next_instance_of(SafeZip::Extract, anything) do |extractor|
allow(extractor).to receive(:extract).and_raise(SafeZip::Extract::EntrySizeError)
end
end
it 'raises an error' do
expected_error = "Path `#{path}` has invalid size in the zip!"
expect { subject }.to raise_error(described_class::Error, expected_error)
end
end
end
context 'when job does not have metadata artifacts' do context 'when job does not have metadata artifacts' do
let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) }
let(:expected_error) do let(:expected_error) do

View file

@ -5,12 +5,13 @@ require 'spec_helper'
RSpec.describe SafeZip::Entry do RSpec.describe SafeZip::Entry do
let(:target_path) { Dir.mktmpdir('safe-zip') } let(:target_path) { Dir.mktmpdir('safe-zip') }
let(:directories) { %w(public folder/with/subfolder) } let(:directories) { %w(public folder/with/subfolder) }
let(:params) { SafeZip::ExtractParams.new(directories: directories, to: target_path) } let(:files) { %w(public/index.html public/assets/image.png) }
let(:params) { SafeZip::ExtractParams.new(directories: directories, files: files, to: target_path) }
let(:entry) { described_class.new(zip_archive, zip_entry, params) } let(:entry) { described_class.new(zip_archive, zip_entry, params) }
let(:entry_name) { 'public/folder/index.html' } let(:entry_name) { 'public/folder/index.html' }
let(:entry_path_dir) { File.join(target_path, File.dirname(entry_name)) } let(:entry_path_dir) { File.join(target_path, File.dirname(entry_name)) }
let(:entry_path) { File.join(target_path, entry_name) } let(:entry_path) { File.join(File.realpath(target_path), entry_name) }
let(:zip_archive) { double } let(:zip_archive) { double }
let(:zip_entry) do let(:zip_entry) do
@ -28,7 +29,7 @@ RSpec.describe SafeZip::Entry do
describe '#path_dir' do describe '#path_dir' do
subject { entry.path_dir } subject { entry.path_dir }
it { is_expected.to eq(target_path + '/public/folder') } it { is_expected.to eq(File.realpath(target_path) + '/public/folder') }
end end
describe '#exist?' do describe '#exist?' do
@ -51,6 +52,9 @@ RSpec.describe SafeZip::Entry do
subject { entry.extract } subject { entry.extract }
context 'when entry does not match the filtered directories' do context 'when entry does not match the filtered directories' do
let(:directories) { %w(public folder/with/subfolder) }
let(:files) { [] }
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
where(:entry_name) do where(:entry_name) do
@ -70,7 +74,30 @@ RSpec.describe SafeZip::Entry do
end end
end end
context 'when entry does exist' do context 'when entry does not match the filtered files' do
let(:directories) { [] }
let(:files) { %w(public/index.html public/assets/image.png) }
using RSpec::Parameterized::TableSyntax
where(:entry_name) do
[
'assets/folder/index.html',
'public/../folder/index.html',
'public/../../../../../index.html',
'../../../../../public/index.html',
'/etc/passwd'
]
end
with_them do
it 'does not extract file' do
is_expected.to be_falsey
end
end
end
context 'when there is an existing extracted entry' do
before do before do
create_entry create_entry
end end

View file

@ -4,8 +4,10 @@ require 'spec_helper'
RSpec.describe SafeZip::ExtractParams do RSpec.describe SafeZip::ExtractParams do
let(:target_path) { Dir.mktmpdir("safe-zip") } let(:target_path) { Dir.mktmpdir("safe-zip") }
let(:params) { described_class.new(directories: directories, to: target_path) } let(:real_target_path) { File.realpath(target_path) }
let(:params) { described_class.new(directories: directories, files: files, to: target_path) }
let(:directories) { %w(public folder/with/subfolder) } let(:directories) { %w(public folder/with/subfolder) }
let(:files) { %w(public/index.html public/assets/image.png) }
after do after do
FileUtils.remove_entry_secure(target_path) FileUtils.remove_entry_secure(target_path)
@ -14,13 +16,13 @@ RSpec.describe SafeZip::ExtractParams do
describe '#extract_path' do describe '#extract_path' do
subject { params.extract_path } subject { params.extract_path }
it { is_expected.to eq(target_path) } it { is_expected.to eq(real_target_path) }
end end
describe '#matching_target_directory' do describe '#matching_target_directory' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
subject { params.matching_target_directory(target_path + path) } subject { params.matching_target_directory(real_target_path + path) }
where(:path, :result) do where(:path, :result) do
'/public/index.html' | '/public/' '/public/index.html' | '/public/'
@ -30,7 +32,7 @@ RSpec.describe SafeZip::ExtractParams do
end end
with_them do with_them do
it { is_expected.to eq(result ? target_path + result : nil) } it { is_expected.to eq(result ? real_target_path + result : nil) }
end end
end end
@ -38,7 +40,7 @@ RSpec.describe SafeZip::ExtractParams do
subject { params.target_directories } subject { params.target_directories }
it 'starts with target_path' do it 'starts with target_path' do
is_expected.to all(start_with(target_path + '/')) is_expected.to all(start_with(real_target_path + '/'))
end end
it 'ends with / for all paths' do it 'ends with / for all paths' do
@ -53,4 +55,27 @@ RSpec.describe SafeZip::ExtractParams do
is_expected.to all(end_with('/*')) is_expected.to all(end_with('/*'))
end end
end end
describe '#matching_target_file' do
using RSpec::Parameterized::TableSyntax
subject { params.matching_target_file(real_target_path + path) }
where(:path, :result) do
'/public/index.html' | true
'/non/existing/path' | false
'/public/' | false
'/folder/with/index.html' | false
end
with_them do
it { is_expected.to eq(result) }
end
end
context 'when directories and files are empty' do
it 'is invalid' do
expect { described_class.new(to: target_path) }.to raise_error(ArgumentError, /directories or files are required/)
end
end
end end

View file

@ -5,6 +5,7 @@ require 'spec_helper'
RSpec.describe SafeZip::Extract do RSpec.describe SafeZip::Extract do
let(:target_path) { Dir.mktmpdir('safe-zip') } let(:target_path) { Dir.mktmpdir('safe-zip') }
let(:directories) { %w(public) } let(:directories) { %w(public) }
let(:files) { %w(public/index.html) }
let(:object) { described_class.new(archive) } let(:object) { described_class.new(archive) }
let(:archive) { Rails.root.join('spec', 'fixtures', 'safe_zip', archive_name) } let(:archive) { Rails.root.join('spec', 'fixtures', 'safe_zip', archive_name) }
@ -13,20 +14,36 @@ RSpec.describe SafeZip::Extract do
end end
describe '#extract' do describe '#extract' do
subject { object.extract(directories: directories, to: target_path) } subject { object.extract(directories: directories, files: files, to: target_path) }
shared_examples 'extracts archive' do shared_examples 'extracts archive' do
context 'when specifying directories' do
subject { object.extract(directories: directories, to: target_path) }
it 'does extract archive' do it 'does extract archive' do
subject subject
expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true)
expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(true)
expect(File.exist?(File.join(target_path, 'source'))).to eq(false) expect(File.exist?(File.join(target_path, 'source'))).to eq(false)
end end
end end
context 'when specifying files' do
subject { object.extract(files: files, to: target_path) }
it 'does extract archive' do
subject
expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true)
expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(false)
end
end
end
shared_examples 'fails to extract archive' do shared_examples 'fails to extract archive' do
it 'does not extract archive' do it 'does not extract archive' do
expect { subject }.to raise_error(SafeZip::Extract::Error) expect { subject }.to raise_error(SafeZip::Extract::Error, including(error_message))
end end
end end
@ -38,9 +55,18 @@ RSpec.describe SafeZip::Extract do
end end
end end
%w(invalid-symlink-does-not-exist.zip invalid-symlinks-outside.zip).each do |name| context 'when zip files are invalid' do
context "when using #{name} archive" do using RSpec::Parameterized::TableSyntax
where(:name, :message) do
'invalid-symlink-does-not-exist.zip' | 'does not exist'
'invalid-symlinks-outside.zip' | 'Symlink cannot be created'
'invalid-unexpected-large.zip' | 'larger when inflated'
end
with_them do
let(:archive_name) { name } let(:archive_name) { name }
let(:error_message) { message }
it_behaves_like 'fails to extract archive' it_behaves_like 'fails to extract archive'
end end
@ -49,6 +75,19 @@ RSpec.describe SafeZip::Extract do
context 'when no matching directories are found' do context 'when no matching directories are found' do
let(:archive_name) { 'valid-simple.zip' } let(:archive_name) { 'valid-simple.zip' }
let(:directories) { %w(non/existing) } let(:directories) { %w(non/existing) }
let(:error_message) { 'No entries extracted' }
subject { object.extract(directories: directories, to: target_path) }
it_behaves_like 'fails to extract archive'
end
context 'when no matching files are found' do
let(:archive_name) { 'valid-simple.zip' }
let(:files) { %w(non/existing) }
let(:error_message) { 'No entries extracted' }
subject { object.extract(files: files, to: target_path) }
it_behaves_like 'fails to extract archive' it_behaves_like 'fails to extract archive'
end end

View file

@ -0,0 +1,23 @@
# frozen_string_literal: true
require 'spec_helper'
require_migration!
RSpec.describe ClearDuplicateJobsCookies, :migration, feature_category: :redis do
def with_redis(&block)
Gitlab::Redis::Queues.with(&block)
end
it 'deletes duplicate jobs cookies' do
delete = ['resque:gitlab:duplicate:blabla:1:cookie:v2', 'resque:gitlab:duplicate:foobar:2:cookie:v2']
keep = ['resque:gitlab:duplicate:something', 'something:cookie:v2']
with_redis { |r| (delete + keep).each { |key| r.set(key, 'value') } }
expect(with_redis { |r| r.exists(delete + keep) }).to eq(4)
migrate!
expect(with_redis { |r| r.exists(delete) }).to eq(0)
expect(with_redis { |r| r.exists(keep) }).to eq(2)
end
end

View file

@ -65,7 +65,6 @@ RSpec.describe Issuable do
it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:author) }
it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_presence_of(:title) }
it { is_expected.to validate_length_of(:title).is_at_most(described_class::TITLE_LENGTH_MAX) } it { is_expected.to validate_length_of(:title).is_at_most(described_class::TITLE_LENGTH_MAX) }
it { is_expected.to validate_length_of(:description).is_at_most(described_class::DESCRIPTION_LENGTH_MAX).on(:create) }
it_behaves_like 'validates description length with custom validation' do it_behaves_like 'validates description length with custom validation' do
before do before do

View file

@ -75,7 +75,58 @@ RSpec.describe Sanitizable do
it 'is not valid', :aggregate_failures do it 'is not valid', :aggregate_failures do
expect(record).not_to be_valid expect(record).not_to be_valid
expect(record.errors.full_messages).to include('Name cannot contain escaped HTML entities') expect(record.errors.full_messages).to contain_exactly(
'Name cannot contain escaped HTML entities',
'Description cannot contain escaped HTML entities'
)
end
end
context 'when input contains double-escaped data' do
let_it_be(:input) do
'%2526lt%253Bscript%2526gt%253Balert%25281%2529%2526lt%253B%252Fscript%2526gt%253B'
end
it_behaves_like 'noop'
it 'is not valid', :aggregate_failures do
expect(record).not_to be_valid
expect(record.errors.full_messages).to contain_exactly(
'Name cannot contain escaped components',
'Description cannot contain escaped components'
)
end
end
context 'when input contains a path traversal attempt' do
let_it_be(:input) { 'main../../../../../../api/v4/projects/1/import_project_members/2' }
it_behaves_like 'noop'
it 'is not valid', :aggregate_failures do
expect(record).not_to be_valid
expect(record.errors.full_messages).to contain_exactly(
'Name cannot contain a path traversal component',
'Description cannot contain a path traversal component'
)
end
end
context 'when input contains both path traversal attempt and pre-escaped entities' do
let_it_be(:input) do
'main../../../../../../api/v4/projects/1/import_project_members/2&lt;script&gt;alert(1)&lt;/script&gt;'
end
it_behaves_like 'noop'
it 'is not valid', :aggregate_failures do
expect(record).not_to be_valid
expect(record.errors.full_messages).to contain_exactly(
'Name cannot contain a path traversal component',
'Name cannot contain escaped HTML entities',
'Description cannot contain a path traversal component',
'Description cannot contain escaped HTML entities'
)
end end
end end
end end

View file

@ -18,7 +18,7 @@ RSpec.describe NamespaceSetting, type: :model do
describe "#default_branch_name_content" do describe "#default_branch_name_content" do
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:namespace_settings) { group.namespace_settings } subject(:namespace_settings) { group.namespace_settings }
shared_examples "doesn't return an error" do shared_examples "doesn't return an error" do
it "doesn't return an error" do it "doesn't return an error" do
@ -28,6 +28,10 @@ RSpec.describe NamespaceSetting, type: :model do
end end
context "when not set" do context "when not set" do
before do
namespace_settings.default_branch_name = nil
end
it_behaves_like "doesn't return an error" it_behaves_like "doesn't return an error"
end end

View file

@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe API::ContainerRegistryEvent do RSpec.describe API::ContainerRegistryEvent do
let(:secret_token) { 'secret_token' } let(:secret_token) { 'secret_token' }
let(:events) { [{ action: 'push' }] } let(:events) { [{ action: 'push' }, { action: 'pull' }] }
let(:registry_headers) { { 'Content-Type' => ::API::ContainerRegistryEvent::DOCKER_DISTRIBUTION_EVENTS_V1_JSON } } let(:registry_headers) { { 'Content-Type' => ::API::ContainerRegistryEvent::DOCKER_DISTRIBUTION_EVENTS_V1_JSON } }
describe 'POST /container_registry_event/events' do describe 'POST /container_registry_event/events' do
@ -19,14 +19,15 @@ RSpec.describe API::ContainerRegistryEvent do
end end
it 'returns 200 status and events are passed to event handler' do it 'returns 200 status and events are passed to event handler' do
event = spy(:event) allow_next_instance_of(::ContainerRegistry::Event) do |event|
allow(::ContainerRegistry::Event).to receive(:new).and_return(event) if event.supported?
expect(event).to receive(:supported?).and_return(true) expect(event).to receive(:handle!).once
expect(event).to receive(:track!).once
end
end
post_events post_events
expect(event).to have_received(:handle!).once
expect(event).to have_received(:track!).once
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end

View file

@ -54,4 +54,17 @@ RSpec.describe Packages::Helm::ExtractFileMetadataService do
it { expect { subject }.to raise_error(described_class::ExtractionError, 'Error while parsing Chart.yaml: (<unknown>): did not find expected node content while parsing a flow node at line 2 column 1') } it { expect { subject }.to raise_error(described_class::ExtractionError, 'Error while parsing Chart.yaml: (<unknown>): did not find expected node content while parsing a flow node at line 2 column 1') }
end end
context 'with a corrupted Chart.yaml of incorrect size' do
let(:helm_fixture_path) { expand_fixture_path('packages/helm/corrupted_chart.tgz') }
let(:expected_error_message) { 'Chart.yaml too big' }
before do
allow(Zlib::GzipReader).to receive(:new).and_return(Zlib::GzipReader.new(File.open(helm_fixture_path)))
end
it 'raises an error with the expected message' do
expect { subject }.to raise_error(::Packages::Helm::ExtractFileMetadataService::ExtractionError, expected_error_message)
end
end
end end

View file

@ -19,6 +19,8 @@ RSpec.shared_examples 'thread comments for commit and snippet' do |resource_name
find('.js-comment-button').click find('.js-comment-button').click
wait_for_all_requests
expect(page).to have_content(comment) expect(page).to have_content(comment)
new_comment = all(comments_selector).last new_comment = all(comments_selector).last

View file

@ -10,42 +10,113 @@ RSpec.shared_examples 'matches_cross_reference_regex? fails fast' do
end end
RSpec.shared_examples 'validates description length with custom validation' do RSpec.shared_examples 'validates description length with custom validation' do
let(:issuable) { build(:issue, description: 'x' * (::Issuable::DESCRIPTION_LENGTH_MAX + 1)) } let(:invalid_description) { 'x' * (::Issuable::DESCRIPTION_LENGTH_MAX + 1) }
let(:context) { :update } let(:valid_description) { 'short description' }
let(:issuable) { build(:issue, description: description) }
subject { issuable.validate(context) } let(:error_message) do
format(
context 'when Issuable is a new record' do _('is too long (%{size}). The maximum size is %{max_size}.'),
it 'validates the maximum description length' do size: ActiveSupport::NumberHelper.number_to_human_size(invalid_description.bytesize),
subject max_size: ActiveSupport::NumberHelper.number_to_human_size(::Issuable::DESCRIPTION_LENGTH_MAX)
expect(issuable.errors[:description]).to eq(["is too long (maximum is #{::Issuable::DESCRIPTION_LENGTH_MAX} characters)"]) )
end end
context 'on create' do subject(:validate) { issuable.validate(context) }
context 'when Issuable is a new record' do
let(:context) { :create } let(:context) { :create }
it 'does not validate the maximum description length' do context 'when description exceeds the maximum size' do
allow(issuable).to receive(:description_max_length_for_new_records_is_valid).and_call_original let(:description) { invalid_description }
subject it 'adds a description too long error' do
validate
expect(issuable).not_to have_received(:description_max_length_for_new_records_is_valid) expect(issuable.errors[:description]).to contain_exactly(error_message)
end
end
context 'when description is within the allowed limits' do
let(:description) { valid_description }
it 'does not add a validation error' do
validate
expect(issuable.errors).not_to have_key(:description)
end end
end end
end end
context 'when Issuable is an existing record' do context 'when Issuable is an existing record' do
let(:context) { :update }
before do before do
allow(issuable).to receive(:expire_etag_cache) # to skip the expire_etag_cache callback allow(issuable).to receive(:expire_etag_cache) # to skip the expire_etag_cache callback
issuable.description = existing_description
issuable.save!(validate: false) issuable.save!(validate: false)
issuable.description = description
end end
it 'does not validate the maximum description length' do context 'when record already had a valid description' do
subject let(:existing_description) { 'small difference so it triggers description_changed?' }
context 'when new description exceeds the maximum size' do
let(:description) { invalid_description }
it 'adds a description too long error' do
validate
expect(issuable.errors[:description]).to contain_exactly(error_message)
end
end
context 'when new description is within the allowed limits' do
let(:description) { valid_description }
it 'does not add a validation error' do
validate
expect(issuable.errors).not_to have_key(:description) expect(issuable.errors).not_to have_key(:description)
end end
end end
end
context 'when record existed with an invalid description' do
let(:existing_description) { "#{invalid_description} small difference so it triggers description_changed?" }
context 'when description is not changed' do
let(:description) { existing_description }
it 'does not add a validation error' do
validate
expect(issuable.errors).not_to have_key(:description)
end
end
context 'when new description exceeds the maximum size' do
let(:description) { invalid_description }
it 'allows updating descriptions that already existed above the limit' do
validate
expect(issuable.errors).not_to have_key(:description)
end
end
context 'when new description is within the allowed limits' do
let(:description) { valid_description }
it 'does not add a validation error' do
validate
expect(issuable.errors).not_to have_key(:description)
end
end
end
end
end end
RSpec.shared_examples 'truncates the description to its allowed maximum length on import' do RSpec.shared_examples 'truncates the description to its allowed maximum length on import' do

View file

@ -32,8 +32,25 @@ RSpec.shared_examples 'sanitizable' do |factory, fields|
subject { build(factory, attributes) } subject { build(factory, attributes) }
it 'is not valid', :aggregate_failures do it 'is not valid', :aggregate_failures do
error = 'cannot contain escaped HTML entities'
expect(subject).not_to be_valid expect(subject).not_to be_valid
expect(subject.errors.details[field].flat_map(&:values)).to include('cannot contain escaped HTML entities') expect(subject.errors.details[field].flat_map(&:values)).to include(error)
end
end
context 'when it contains a path component' do
let_it_be(:input) do
'main../../../../../../api/v4/projects/1/import_project_members/2'
end
subject { build(factory, attributes) }
it 'is not valid', :aggregate_failures do
error = 'cannot contain a path traversal component'
expect(subject).not_to be_valid
expect(subject.errors.details[field].flat_map(&:values)).to include(error)
end end
end end
end end