From 1ebdda69d61ae26379f8fac27671103374031944 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 31 Jul 2023 14:35:12 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@16-2-stable-ee --- app/models/project.rb | 11 +++ app/models/project_setting.rb | 11 +++ .../bulk_imports/archive_extraction_service.rb | 6 +- .../bulk_imports/file_decompression_service.rb | 2 +- .../common/pipelines/lfs_objects_pipeline.rb | 2 +- .../common/pipelines/uploads_pipeline.rb | 2 +- lib/bulk_imports/file_downloads/validations.rb | 2 +- .../projects/pipelines/design_bundle_pipeline.rb | 2 +- .../pipelines/repository_bundle_pipeline.rb | 2 +- lib/gitlab/ci/decompressed_gzip_size_validator.rb | 2 +- lib/gitlab/import_export/command_line_util.rb | 33 +++++-- .../decompressed_archive_size_validator.rb | 2 +- lib/gitlab/import_export/file_importer.rb | 4 +- lib/gitlab/import_export/json/ndjson_reader.rb | 6 +- .../import_export/recursive_merge_folders.rb | 2 +- lib/gitlab/pages/virtual_host_finder.rb | 5 +- lib/gitlab/utils/file_info.rb | 35 +++++++ locale/gitlab.pot | 6 ++ .../common/pipelines/lfs_objects_pipeline_spec.rb | 15 ++- .../common/pipelines/uploads_pipeline_spec.rb | 14 ++- .../pipelines/design_bundle_pipeline_spec.rb | 19 +++- .../pipelines/repository_bundle_pipeline_spec.rb | 19 +++- .../ci/decompressed_gzip_size_validator_spec.rb | 10 ++ .../github_import/attachments_downloader_spec.rb | 22 ++++- .../gitlab/import_export/command_line_util_spec.rb | 108 ++++++++++++++------- .../decompressed_archive_size_validator_spec.rb | 17 +++- .../lib/gitlab/import_export/file_importer_spec.rb | 57 ++++++++++- .../import_export/json/ndjson_reader_spec.rb | 46 +++++---- .../import_export/recursive_merge_folders_spec.rb | 4 +- spec/lib/gitlab/pages/virtual_host_finder_spec.rb | 29 ++++-- spec/lib/gitlab/utils/file_info_spec.rb | 88 +++++++++++++++++ spec/models/project_setting_spec.rb | 26 ++++- spec/models/project_spec.rb | 58 +++++++++++ .../archive_extraction_service_spec.rb | 10 +- .../file_decompression_service_spec.rb | 67 +++++++++---- .../bulk_imports/file_download_service_spec.rb | 32 +++++- 36 files changed, 644 insertions(+), 132 deletions(-) create mode 100644 lib/gitlab/utils/file_info.rb create mode 100644 spec/lib/gitlab/utils/file_info_spec.rb diff --git a/app/models/project.rb b/app/models/project.rb index 931f4db3a54..8959eccbd1f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -579,6 +579,8 @@ class Project < ApplicationRecord validates :max_artifacts_size, numericality: { only_integer: true, greater_than: 0, allow_nil: true } validates :suggestion_commit_message, length: { maximum: MAX_SUGGESTIONS_TEMPLATE_LENGTH } + validate :path_availability, if: :path_changed? + # Scopes scope :pending_delete, -> { where(pending_delete: true) } scope :without_deleted, -> { where(pending_delete: false) } @@ -3221,6 +3223,15 @@ class Project < ApplicationRecord group.crm_enabled? end + def path_availability + base, _, host = path.partition('.') + + return unless host == Gitlab.config.pages&.dig('host') + return unless ProjectSetting.where(pages_unique_domain: base).exists? + + errors.add(:path, s_('Project|already in use')) + end + private # overridden in EE diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 7ca74d4e970..aeefa5c8dcd 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -59,6 +59,8 @@ class ProjectSetting < ApplicationRecord validate :validates_mr_default_target_self + validate :pages_unique_domain_availability, if: :pages_unique_domain_changed? + attribute :legacy_open_source_license_available, default: -> do Feature.enabled?(:legacy_open_source_license_available, type: :ops) end @@ -109,6 +111,15 @@ class ProjectSetting < ApplicationRecord pages_unique_domain_enabled || pages_unique_domain_in_database.present? end + + def pages_unique_domain_availability + host = Gitlab.config.pages&.dig('host') + + return if host.blank? + return unless Project.where(path: "#{pages_unique_domain}.#{host}").exists? + + errors.add(:pages_unique_domain, s_('ProjectSetting|already in use')) + end end ProjectSetting.prepend_mod diff --git a/app/services/bulk_imports/archive_extraction_service.rb b/app/services/bulk_imports/archive_extraction_service.rb index 4485b19035b..bce2a67218a 100644 --- a/app/services/bulk_imports/archive_extraction_service.rb +++ b/app/services/bulk_imports/archive_extraction_service.rb @@ -49,11 +49,7 @@ module BulkImports end def validate_symlink - raise(BulkImports::Error, 'Invalid file') if symlink?(filepath) - end - - def symlink?(filepath) - File.lstat(filepath).symlink? + raise(BulkImports::Error, 'Invalid file') if Gitlab::Utils::FileInfo.linked?(filepath) end def extract_archive diff --git a/app/services/bulk_imports/file_decompression_service.rb b/app/services/bulk_imports/file_decompression_service.rb index 94573f6bb13..77638f10f54 100644 --- a/app/services/bulk_imports/file_decompression_service.rb +++ b/app/services/bulk_imports/file_decompression_service.rb @@ -53,7 +53,7 @@ module BulkImports end def validate_symlink(filepath) - raise(ServiceError, 'Invalid file') if File.lstat(filepath).symlink? + raise(ServiceError, 'Invalid file') if Gitlab::Utils::FileInfo.linked?(filepath) end def decompress_file diff --git a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb index 0bf4d341aad..bd09b6add00 100644 --- a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb @@ -26,7 +26,7 @@ module BulkImports return if tar_filepath?(file_path) return if lfs_json_filepath?(file_path) return if File.directory?(file_path) - return if File.lstat(file_path).symlink? + return if Gitlab::Utils::FileInfo.linked?(file_path) size = File.size(file_path) oid = LfsObject.calculate_oid(file_path) diff --git a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb index 81ce20db9ab..ea17af36c9a 100644 --- a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb @@ -26,7 +26,7 @@ module BulkImports # Validate that the path is OK to load Gitlab::PathTraversal.check_allowed_absolute_path_and_path_traversal!(file_path, [Dir.tmpdir]) return if File.directory?(file_path) - return if File.lstat(file_path).symlink? + return if Gitlab::Utils::FileInfo.linked?(file_path) avatar_path = AVATAR_PATTERN.match(file_path) return save_avatar(file_path) if avatar_path diff --git a/lib/bulk_imports/file_downloads/validations.rb b/lib/bulk_imports/file_downloads/validations.rb index b852a50c888..e1844843408 100644 --- a/lib/bulk_imports/file_downloads/validations.rb +++ b/lib/bulk_imports/file_downloads/validations.rb @@ -32,7 +32,7 @@ module BulkImports end def validate_symlink - return unless File.lstat(filepath).symlink? + return unless Gitlab::Utils::FileInfo.linked?(filepath) File.delete(filepath) raise_error 'Invalid downloaded file' diff --git a/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb b/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb index 373cd2bd75a..235d2629b9e 100644 --- a/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/design_bundle_pipeline.rb @@ -26,7 +26,7 @@ module BulkImports return unless portable.lfs_enabled? return unless File.exist?(bundle_path) return if File.directory?(bundle_path) - return if File.lstat(bundle_path).symlink? + return if Gitlab::Utils::FileInfo.linked?(bundle_path) portable.design_repository.create_from_bundle(bundle_path) end diff --git a/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb b/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb index f19d8931f4a..4307cb2bafd 100644 --- a/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb +++ b/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline.rb @@ -26,7 +26,7 @@ module BulkImports return unless File.exist?(bundle_path) return if File.directory?(bundle_path) - return if File.lstat(bundle_path).symlink? + return if Gitlab::Utils::FileInfo.linked?(bundle_path) portable.repository.create_from_bundle(bundle_path) end diff --git a/lib/gitlab/ci/decompressed_gzip_size_validator.rb b/lib/gitlab/ci/decompressed_gzip_size_validator.rb index 9b7b5f0dd66..b386e400423 100644 --- a/lib/gitlab/ci/decompressed_gzip_size_validator.rb +++ b/lib/gitlab/ci/decompressed_gzip_size_validator.rb @@ -65,7 +65,7 @@ module Gitlab def validate_archive_path Gitlab::PathTraversal.check_path_traversal!(archive_path) - raise(ServiceError, 'Archive path is a symlink') if File.lstat(archive_path).symlink? + raise(ServiceError, 'Archive path is a symlink or hard link') if Gitlab::Utils::FileInfo.linked?(archive_path) raise(ServiceError, 'Archive path is not a file') unless File.file?(archive_path) end diff --git a/lib/gitlab/import_export/command_line_util.rb b/lib/gitlab/import_export/command_line_util.rb index d681f39f00b..e2f365fcbf8 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -5,8 +5,11 @@ module Gitlab module CommandLineUtil UNTAR_MASK = 'u+rwX,go+rX,go-w' DEFAULT_DIR_MODE = 0700 + CLEAN_DIR_IGNORE_FILE_NAMES = %w[. ..].freeze - FileOversizedError = Class.new(StandardError) + CommandLineUtilError = Class.new(StandardError) + FileOversizedError = Class.new(CommandLineUtilError) + HardLinkError = Class.new(CommandLineUtilError) def tar_czf(archive:, dir:) tar_with_options(archive: archive, dir: dir, options: 'czf') @@ -90,7 +93,7 @@ module Gitlab def untar_with_options(archive:, dir:, options:) execute_cmd(%W(tar -#{options} #{archive} -C #{dir})) execute_cmd(%W(chmod -R #{UNTAR_MASK} #{dir})) - remove_symlinks(dir) + clean_extraction_dir!(dir) end # rubocop:disable Gitlab/ModuleWithInstanceVariables @@ -122,17 +125,27 @@ module Gitlab true end - def remove_symlinks(dir) - ignore_file_names = %w[. ..] - + # Scans and cleans the directory tree. + # Symlinks are considered legal but are removed. + # Files sharing hard links are considered illegal and the directory will be removed + # and a `HardLinkError` exception will be raised. + # + # @raise [HardLinkError] if there multiple hard links to the same file detected. + # @return [Boolean] true + def clean_extraction_dir!(dir) # Using File::FNM_DOTMATCH to also delete symlinks starting with "." - Dir.glob("#{dir}/**/*", File::FNM_DOTMATCH) - .reject { |f| ignore_file_names.include?(File.basename(f)) } - .each do |filepath| - FileUtils.rm(filepath) if File.lstat(filepath).symlink? - end + Dir.glob("#{dir}/**/*", File::FNM_DOTMATCH).each do |filepath| + next if CLEAN_DIR_IGNORE_FILE_NAMES.include?(File.basename(filepath)) + + raise HardLinkError, 'File shares hard link' if Gitlab::Utils::FileInfo.shares_hard_link?(filepath) + + FileUtils.rm(filepath) if Gitlab::Utils::FileInfo.linked?(filepath) + end true + rescue HardLinkError + FileUtils.remove_dir(dir) + raise end end end diff --git a/lib/gitlab/import_export/decompressed_archive_size_validator.rb b/lib/gitlab/import_export/decompressed_archive_size_validator.rb index 104c9e6c456..2e39f3f38c2 100644 --- a/lib/gitlab/import_export/decompressed_archive_size_validator.rb +++ b/lib/gitlab/import_export/decompressed_archive_size_validator.rb @@ -87,7 +87,7 @@ module Gitlab def validate_archive_path Gitlab::PathTraversal.check_path_traversal!(@archive_path) - raise(ServiceError, 'Archive path is a symlink') if File.lstat(@archive_path).symlink? + raise(ServiceError, 'Archive path is a symlink or hard link') if Gitlab::Utils::FileInfo.linked?(@archive_path) raise(ServiceError, 'Archive path is not a file') unless File.file?(@archive_path) end diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb index d2593289c23..37c83e88ef2 100644 --- a/lib/gitlab/import_export/file_importer.rb +++ b/lib/gitlab/import_export/file_importer.rb @@ -23,7 +23,7 @@ module Gitlab mkdir_p(@shared.export_path) mkdir_p(@shared.archive_path) - remove_symlinks(@shared.export_path) + clean_extraction_dir!(@shared.export_path) copy_archive wait_for_archived_file do @@ -35,7 +35,7 @@ module Gitlab false ensure remove_import_file - remove_symlinks(@shared.export_path) + clean_extraction_dir!(@shared.export_path) end private diff --git a/lib/gitlab/import_export/json/ndjson_reader.rb b/lib/gitlab/import_export/json/ndjson_reader.rb index 3de56aacf18..93a94716f8d 100644 --- a/lib/gitlab/import_export/json/ndjson_reader.rb +++ b/lib/gitlab/import_export/json/ndjson_reader.rb @@ -21,7 +21,9 @@ module Gitlab # This reads from `tree/project.json` path = file_path("#{importable_path}.json") - raise Gitlab::ImportExport::Error, 'Invalid file' if !File.exist?(path) || File.symlink?(path) + if !File.exist?(path) || Gitlab::Utils::FileInfo.linked?(path) + raise Gitlab::ImportExport::Error, 'Invalid file' + end data = File.read(path, MAX_JSON_DOCUMENT_SIZE) json_decode(data) @@ -34,7 +36,7 @@ module Gitlab # This reads from `tree/project/merge_requests.ndjson` path = file_path(importable_path, "#{key}.ndjson") - next if !File.exist?(path) || File.symlink?(path) + next if !File.exist?(path) || Gitlab::Utils::FileInfo.linked?(path) File.foreach(path, MAX_JSON_DOCUMENT_SIZE).with_index do |line, line_num| documents << [json_decode(line), line_num] diff --git a/lib/gitlab/import_export/recursive_merge_folders.rb b/lib/gitlab/import_export/recursive_merge_folders.rb index 827385d4daf..e6eba60db93 100644 --- a/lib/gitlab/import_export/recursive_merge_folders.rb +++ b/lib/gitlab/import_export/recursive_merge_folders.rb @@ -57,7 +57,7 @@ module Gitlab source_child = File.join(source_path, child) target_child = File.join(target_path, child) - next if File.lstat(source_child).symlink? + next if Gitlab::Utils::FileInfo.linked?(source_child) if File.directory?(source_child) FileUtils.mkdir_p(target_child, mode: DEFAULT_DIR_MODE) unless File.exist?(target_child) diff --git a/lib/gitlab/pages/virtual_host_finder.rb b/lib/gitlab/pages/virtual_host_finder.rb index 5fec60188f8..d5e2159fb52 100644 --- a/lib/gitlab/pages/virtual_host_finder.rb +++ b/lib/gitlab/pages/virtual_host_finder.rb @@ -10,13 +10,12 @@ module Gitlab def execute return if host.blank? - gitlab_host = ::Settings.pages.host.downcase.prepend(".") + gitlab_host = ::Gitlab.config.pages.host.downcase.prepend(".") if host.ends_with?(gitlab_host) name = host.delete_suffix(gitlab_host) - by_namespace_domain(name) || - by_unique_domain(name) + by_unique_domain(name) || by_namespace_domain(name) else by_custom_domain(host) end diff --git a/lib/gitlab/utils/file_info.rb b/lib/gitlab/utils/file_info.rb new file mode 100644 index 00000000000..a0ec370e225 --- /dev/null +++ b/lib/gitlab/utils/file_info.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Gitlab + module Utils + module FileInfo + class << self + # Returns true if: + # - File or directory is a symlink. + # - File shares a hard link. + def linked?(file) + stat = to_file_stat(file) + + stat.symlink? || shares_hard_link?(stat) + end + + # Returns: + # - true if file shares a hard link with another file. + # - false if file is a directory, as directories cannot be hard linked. + def shares_hard_link?(file) + stat = to_file_stat(file) + + stat.file? && stat.nlink > 1 + end + + private + + def to_file_stat(filepath_or_stat) + return filepath_or_stat if filepath_or_stat.is_a?(File::Stat) + + File.lstat(filepath_or_stat) + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5a15c526b7e..0ea487138c6 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -36590,6 +36590,9 @@ msgstr "" msgid "ProjectSettings|Your project is set up. %{linkStart}View instrumentation instructions%{linkEnd}." msgstr "" +msgid "ProjectSetting|already in use" +msgstr "" + msgid "ProjectTemplates|.NET Core" msgstr "" @@ -36887,6 +36890,9 @@ msgstr "" msgid "ProjectsNew|Your project will be created at:" msgstr "" +msgid "Project|already in use" +msgstr "" + msgid "PrometheusAlerts|exceeded" msgstr "" diff --git a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb index 297ac0ca0ba..50640e1e4ba 100644 --- a/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/lfs_objects_pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline do +RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline, feature_category: :importers do let_it_be(:portable) { create(:project) } let_it_be(:oid) { 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } @@ -118,13 +118,22 @@ RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline do context 'when file path is symlink' do it 'returns' do symlink = File.join(tmpdir, 'symlink') + FileUtils.ln_s(lfs_file_path, symlink) - FileUtils.ln_s(File.join(tmpdir, lfs_file_path), symlink) - + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(symlink).and_call_original expect { pipeline.load(context, symlink) }.not_to change { portable.lfs_objects.count } end end + context 'when file path shares multiple hard links' do + it 'returns' do + FileUtils.link(lfs_file_path, File.join(tmpdir, 'hard_link')) + + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(lfs_file_path).and_call_original + expect { pipeline.load(context, lfs_file_path) }.not_to change { portable.lfs_objects.count } + end + end + context 'when path is a directory' do it 'returns' do expect { pipeline.load(context, Dir.tmpdir) }.not_to change { portable.lfs_objects.count } diff --git a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb index bc6d36452b4..09c8c7b92c2 100644 --- a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb @@ -105,6 +105,7 @@ RSpec.describe BulkImports::Common::Pipelines::UploadsPipeline, feature_category it 'returns' do path = File.join(tmpdir, 'test') FileUtils.touch(path) + expect { pipeline.load(context, path) }.not_to change { portable.uploads.count } end end @@ -118,13 +119,22 @@ RSpec.describe BulkImports::Common::Pipelines::UploadsPipeline, feature_category context 'when path is a symlink' do it 'does not upload the file' do symlink = File.join(tmpdir, 'symlink') + FileUtils.ln_s(upload_file_path, symlink) - FileUtils.ln_s(File.join(tmpdir, upload_file_path), symlink) - + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(symlink).and_call_original expect { pipeline.load(context, symlink) }.not_to change { portable.uploads.count } end end + context 'when path has multiple hard links' do + it 'does not upload the file' do + FileUtils.link(upload_file_path, File.join(tmpdir, 'hard_link')) + + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(upload_file_path).and_call_original + expect { pipeline.load(context, upload_file_path) }.not_to change { portable.uploads.count } + end + end + context 'when path traverses' do it 'does not upload the file' do path_traversal = "#{uploads_dir_path}/avatar/../../../../etc/passwd" diff --git a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb index 5b7309b09f5..87efad92131 100644 --- a/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/design_bundle_pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline do +RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline, feature_category: :importers do let_it_be(:design) { create(:design, :with_file) } let(:portable) { create(:project) } @@ -125,9 +125,9 @@ RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline do context 'when path is symlink' do it 'returns' do symlink = File.join(tmpdir, 'symlink') + FileUtils.ln_s(design_bundle_path, symlink) - FileUtils.ln_s(File.join(tmpdir, design_bundle_path), symlink) - + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(symlink).and_call_original expect(portable.design_repository).not_to receive(:create_from_bundle) pipeline.load(context, symlink) @@ -136,6 +136,19 @@ RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline do end end + context 'when path has multiple hard links' do + it 'returns' do + FileUtils.link(design_bundle_path, File.join(tmpdir, 'hard_link')) + + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(design_bundle_path).and_call_original + expect(portable.design_repository).not_to receive(:create_from_bundle) + + pipeline.load(context, design_bundle_path) + + expect(portable.design_repository.exists?).to eq(false) + end + end + context 'when path is not under tmpdir' do it 'returns' do expect { pipeline.load(context, '/home/test.txt') } diff --git a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb index 07fafc19026..5aae8c959fa 100644 --- a/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb +++ b/spec/lib/bulk_imports/projects/pipelines/repository_bundle_pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline do +RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline, feature_category: :importers do let_it_be(:source) { create(:project, :repository) } let(:portable) { create(:project) } @@ -123,9 +123,9 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline do context 'when path is symlink' do it 'returns' do symlink = File.join(tmpdir, 'symlink') + FileUtils.ln_s(bundle_path, symlink) - FileUtils.ln_s(File.join(tmpdir, bundle_path), symlink) - + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(symlink).and_call_original expect(portable.repository).not_to receive(:create_from_bundle) pipeline.load(context, symlink) @@ -134,6 +134,19 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline do end end + context 'when path has mutiple hard links' do + it 'returns' do + FileUtils.link(bundle_path, File.join(tmpdir, 'hard_link')) + + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(bundle_path).and_call_original + expect(portable.repository).not_to receive(:create_from_bundle) + + pipeline.load(context, bundle_path) + + expect(portable.repository.exists?).to eq(false) + end + end + context 'when path is not under tmpdir' do it 'returns' do expect { pipeline.load(context, '/home/test.txt') } diff --git a/spec/lib/gitlab/ci/decompressed_gzip_size_validator_spec.rb b/spec/lib/gitlab/ci/decompressed_gzip_size_validator_spec.rb index 6ca3f4d415e..dad5bd2548b 100644 --- a/spec/lib/gitlab/ci/decompressed_gzip_size_validator_spec.rb +++ b/spec/lib/gitlab/ci/decompressed_gzip_size_validator_spec.rb @@ -105,6 +105,16 @@ RSpec.describe Gitlab::Ci::DecompressedGzipSizeValidator, feature_category: :imp end end + context 'when archive path has multiple hard links' do + before do + FileUtils.link(filepath, File.join(Dir.mktmpdir, 'hard_link')) + end + + it 'returns false' do + expect(subject).not_to be_valid + end + end + context 'when archive path is not a file' do let(:filepath) { Dir.mktmpdir } let(:filesize) { File.size(filepath) } diff --git a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb index 84d6713efdb..086aa4be17e 100644 --- a/spec/lib/gitlab/github_import/attachments_downloader_spec.rb +++ b/spec/lib/gitlab/github_import/attachments_downloader_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::AttachmentsDownloader do +RSpec.describe Gitlab::GithubImport::AttachmentsDownloader, feature_category: :importers do subject(:downloader) { described_class.new(file_url) } let_it_be(:file_url) { 'https://example.com/avatar.png' } @@ -39,6 +39,26 @@ RSpec.describe Gitlab::GithubImport::AttachmentsDownloader do end end + context 'when file shares multiple hard links' do + let(:tmpdir) { Dir.mktmpdir } + let(:hard_link) { File.join(tmpdir, 'hard_link') } + + before do + existing_file = File.join(tmpdir, 'file.txt') + FileUtils.touch(existing_file) + FileUtils.link(existing_file, hard_link) + allow(downloader).to receive(:filepath).and_return(hard_link) + end + + it 'raises expected exception' do + expect(Gitlab::Utils::FileInfo).to receive(:linked?).with(hard_link).and_call_original + expect { downloader.perform }.to raise_exception( + described_class::DownloadError, + 'Invalid downloaded file' + ) + end + end + context 'when filename is malicious' do let_it_be(:file_url) { 'https://example.com/ava%2F..%2Ftar.png' } diff --git a/spec/lib/gitlab/import_export/command_line_util_spec.rb b/spec/lib/gitlab/import_export/command_line_util_spec.rb index 91cfab1688a..b2e047f5621 100644 --- a/spec/lib/gitlab/import_export/command_line_util_spec.rb +++ b/spec/lib/gitlab/import_export/command_line_util_spec.rb @@ -5,13 +5,16 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importers do include ExportFileHelper - let(:path) { "#{Dir.tmpdir}/symlink_test" } - let(:archive) { 'spec/fixtures/symlink_export.tar.gz' } let(:shared) { Gitlab::ImportExport::Shared.new(nil) } - let(:tmpdir) { Dir.mktmpdir } + # Separate where files are written during this test by their kind, to avoid them interfering with each other: + # - `source_dir` Dir to compress files from. + # - `target_dir` Dir to decompress archived files into. + # - `archive_dir` Dir to write any archive files to. + let(:source_dir) { Dir.mktmpdir } + let(:target_dir) { Dir.mktmpdir } let(:archive_dir) { Dir.mktmpdir } - subject do + subject(:mock_class) do Class.new do include Gitlab::ImportExport::CommandLineUtil @@ -25,38 +28,59 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe end before do - FileUtils.mkdir_p(path) + FileUtils.mkdir_p(source_dir) end after do - FileUtils.rm_rf(path) + FileUtils.rm_rf(source_dir) + FileUtils.rm_rf(target_dir) FileUtils.rm_rf(archive_dir) - FileUtils.remove_entry(tmpdir) end shared_examples 'deletes symlinks' do |compression, decompression| it 'deletes the symlinks', :aggregate_failures do - Dir.mkdir("#{tmpdir}/.git") - Dir.mkdir("#{tmpdir}/folder") - FileUtils.touch("#{tmpdir}/file.txt") - FileUtils.touch("#{tmpdir}/folder/file.txt") - FileUtils.touch("#{tmpdir}/.gitignore") - FileUtils.touch("#{tmpdir}/.git/config") - File.symlink('file.txt', "#{tmpdir}/.symlink") - File.symlink('file.txt', "#{tmpdir}/.git/.symlink") - File.symlink('file.txt', "#{tmpdir}/folder/.symlink") - archive = File.join(archive_dir, 'archive') - subject.public_send(compression, archive: archive, dir: tmpdir) - - subject.public_send(decompression, archive: archive, dir: archive_dir) - - expect(File.exist?("#{archive_dir}/file.txt")).to eq(true) - expect(File.exist?("#{archive_dir}/folder/file.txt")).to eq(true) - expect(File.exist?("#{archive_dir}/.gitignore")).to eq(true) - expect(File.exist?("#{archive_dir}/.git/config")).to eq(true) - expect(File.exist?("#{archive_dir}/.symlink")).to eq(false) - expect(File.exist?("#{archive_dir}/.git/.symlink")).to eq(false) - expect(File.exist?("#{archive_dir}/folder/.symlink")).to eq(false) + Dir.mkdir("#{source_dir}/.git") + Dir.mkdir("#{source_dir}/folder") + FileUtils.touch("#{source_dir}/file.txt") + FileUtils.touch("#{source_dir}/folder/file.txt") + FileUtils.touch("#{source_dir}/.gitignore") + FileUtils.touch("#{source_dir}/.git/config") + File.symlink('file.txt', "#{source_dir}/.symlink") + File.symlink('file.txt', "#{source_dir}/.git/.symlink") + File.symlink('file.txt', "#{source_dir}/folder/.symlink") + archive_file = File.join(archive_dir, 'symlink_archive.tar.gz') + subject.public_send(compression, archive: archive_file, dir: source_dir) + subject.public_send(decompression, archive: archive_file, dir: target_dir) + + expect(File).to exist("#{target_dir}/file.txt") + expect(File).to exist("#{target_dir}/folder/file.txt") + expect(File).to exist("#{target_dir}/.gitignore") + expect(File).to exist("#{target_dir}/.git/config") + expect(File).not_to exist("#{target_dir}/.symlink") + expect(File).not_to exist("#{target_dir}/.git/.symlink") + expect(File).not_to exist("#{target_dir}/folder/.symlink") + end + end + + shared_examples 'handles shared hard links' do |compression, decompression| + let(:archive_file) { File.join(archive_dir, 'hard_link_archive.tar.gz') } + + subject(:decompress) { mock_class.public_send(decompression, archive: archive_file, dir: target_dir) } + + before do + Dir.mkdir("#{source_dir}/dir") + FileUtils.touch("#{source_dir}/file.txt") + FileUtils.touch("#{source_dir}/dir/.file.txt") + FileUtils.link("#{source_dir}/file.txt", "#{source_dir}/.hard_linked_file.txt") + + mock_class.public_send(compression, archive: archive_file, dir: source_dir) + end + + it 'raises an exception and deletes the extraction dir', :aggregate_failures do + expect(FileUtils).to receive(:remove_dir).with(target_dir).and_call_original + expect(Dir).to exist(target_dir) + expect { decompress }.to raise_error(described_class::HardLinkError) + expect(Dir).not_to exist(target_dir) end end @@ -212,6 +236,8 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe end describe '#gzip' do + let(:path) { source_dir } + it 'compresses specified file' do tempfile = Tempfile.new('test', path) filename = File.basename(tempfile.path) @@ -229,14 +255,16 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe end describe '#gunzip' do + let(:path) { source_dir } + it 'decompresses specified file' do filename = 'labels.ndjson.gz' gz_filepath = "spec/fixtures/bulk_imports/gz/#{filename}" - FileUtils.copy_file(gz_filepath, File.join(tmpdir, filename)) + FileUtils.copy_file(gz_filepath, File.join(path, filename)) - subject.gunzip(dir: tmpdir, filename: filename) + subject.gunzip(dir: path, filename: filename) - expect(File.exist?(File.join(tmpdir, 'labels.ndjson'))).to eq(true) + expect(File.exist?(File.join(path, 'labels.ndjson'))).to eq(true) end context 'when exception occurs' do @@ -250,7 +278,7 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe it 'archives a folder without compression' do archive_file = File.join(archive_dir, 'archive.tar') - result = subject.tar_cf(archive: archive_file, dir: tmpdir) + result = subject.tar_cf(archive: archive_file, dir: source_dir) expect(result).to eq(true) expect(File.exist?(archive_file)).to eq(true) @@ -270,29 +298,35 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe end describe '#untar_zxf' do + let(:tar_archive_fixture) { 'spec/fixtures/symlink_export.tar.gz' } + it_behaves_like 'deletes symlinks', :tar_czf, :untar_zxf + it_behaves_like 'handles shared hard links', :tar_czf, :untar_zxf it 'has the right mask for project.json' do - subject.untar_zxf(archive: archive, dir: path) + subject.untar_zxf(archive: tar_archive_fixture, dir: target_dir) - expect(file_permissions("#{path}/project.json")).to eq(0755) # originally 777 + expect(file_permissions("#{target_dir}/project.json")).to eq(0755) # originally 777 end it 'has the right mask for uploads' do - subject.untar_zxf(archive: archive, dir: path) + subject.untar_zxf(archive: tar_archive_fixture, dir: target_dir) - expect(file_permissions("#{path}/uploads")).to eq(0755) # originally 555 + expect(file_permissions("#{target_dir}/uploads")).to eq(0755) # originally 555 end end describe '#untar_xf' do + let(:tar_archive_fixture) { 'spec/fixtures/symlink_export.tar.gz' } + it_behaves_like 'deletes symlinks', :tar_cf, :untar_xf + it_behaves_like 'handles shared hard links', :tar_cf, :untar_xf it 'extracts archive without decompression' do filename = 'archive.tar.gz' archive_file = File.join(archive_dir, 'archive.tar') - FileUtils.copy_file(archive, File.join(archive_dir, filename)) + FileUtils.copy_file(tar_archive_fixture, File.join(archive_dir, filename)) subject.gunzip(dir: archive_dir, filename: filename) result = subject.untar_xf(archive: archive_file, dir: archive_dir) diff --git a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb index a6cb74c3c9f..8c5823edc51 100644 --- a/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb +++ b/spec/lib/gitlab/import_export/decompressed_archive_size_validator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator do +RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator, feature_category: :importers do let_it_be(:filepath) { File.join(Dir.tmpdir, 'decompressed_archive_size_validator_spec.gz') } before(:all) do @@ -121,7 +121,7 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator do context 'which archive path is a symlink' do let(:filepath) { File.join(Dir.tmpdir, 'symlink') } - let(:error_message) { 'Archive path is a symlink' } + let(:error_message) { 'Archive path is a symlink or hard link' } before do FileUtils.ln_s(filepath, filepath, force: true) @@ -132,6 +132,19 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator do end end + context 'when archive path shares multiple hard links' do + let(:filesize) { 32 } + let(:error_message) { 'Archive path is a symlink or hard link' } + + before do + FileUtils.link(filepath, File.join(Dir.mktmpdir, 'hard_link')) + end + + it 'returns false' do + expect(subject).not_to be_valid + end + end + context 'when archive path is not a file' do let(:filepath) { Dir.mktmpdir } let(:filesize) { File.size(filepath) } diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb index 5a75631ec4d..aff11f7ac30 100644 --- a/spec/lib/gitlab/import_export/file_importer_spec.rb +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::ImportExport::FileImporter do +RSpec.describe Gitlab::ImportExport::FileImporter, feature_category: :importers do include ExportFileHelper let(:shared) { Gitlab::ImportExport::Shared.new(nil) } @@ -113,28 +113,73 @@ RSpec.describe Gitlab::ImportExport::FileImporter do end context 'error' do + subject(:import) { described_class.import(importable: build(:project), archive_file: '', shared: shared) } + before do allow_next_instance_of(described_class) do |instance| - allow(instance).to receive(:wait_for_archived_file).and_raise(StandardError) + allow(instance).to receive(:wait_for_archived_file).and_raise(StandardError, 'foo') end - described_class.import(importable: build(:project), archive_file: '', shared: shared) end it 'removes symlinks in root folder' do + import + expect(File.exist?(symlink_file)).to be false end it 'removes hidden symlinks in root folder' do + import + expect(File.exist?(hidden_symlink_file)).to be false end it 'removes symlinks in subfolders' do + import + expect(File.exist?(subfolder_symlink_file)).to be false end it 'does not remove a valid file' do + import + expect(File.exist?(valid_file)).to be true end + + it 'returns false and sets an error on shared' do + result = import + + expect(result).to eq(false) + expect(shared.errors.join).to eq('foo') + end + + context 'when files in the archive share hard links' do + let(:hard_link_file) { "#{shared.export_path}/hard_link_file.txt" } + + before do + FileUtils.link(valid_file, hard_link_file) + end + + it 'returns false and sets an error on shared' do + result = import + + expect(result).to eq(false) + expect(shared.errors.join).to eq('File shares hard link') + end + + it 'removes all files in export path' do + expect(Dir).to exist(shared.export_path) + expect(File).to exist(symlink_file) + expect(File).to exist(hard_link_file) + expect(File).to exist(valid_file) + + import + + expect(File).not_to exist(symlink_file) + expect(File).not_to exist(hard_link_file) + expect(File).not_to exist(valid_file) + expect(Dir).not_to exist(shared.export_path) + end + end end context 'when file exceeds acceptable decompressed size' do @@ -157,8 +202,10 @@ RSpec.describe Gitlab::ImportExport::FileImporter do allow(Gitlab::ImportExport::DecompressedArchiveSizeValidator).to receive(:max_bytes).and_return(1) end - it 'returns false' do - expect(subject.import).to eq(false) + it 'returns false and sets an error on shared' do + result = subject.import + + expect(result).to eq(false) expect(shared.errors.join).to eq('Decompressed archive size validation failed.') end end diff --git a/spec/lib/gitlab/import_export/json/ndjson_reader_spec.rb b/spec/lib/gitlab/import_export/json/ndjson_reader_spec.rb index 98afe01c08b..1f98a9efb7d 100644 --- a/spec/lib/gitlab/import_export/json/ndjson_reader_spec.rb +++ b/spec/lib/gitlab/import_export/json/ndjson_reader_spec.rb @@ -35,16 +35,22 @@ RSpec.describe Gitlab::ImportExport::Json::NdjsonReader, feature_category: :impo expect(subject).to eq(root_tree) end - context 'when project.json is symlink' do - it 'raises error an error' do - Dir.mktmpdir do |tmpdir| - FileUtils.touch(File.join(tmpdir, 'passwd')) - File.symlink(File.join(tmpdir, 'passwd'), File.join(tmpdir, 'project.json')) + context 'when project.json is symlink or hard link' do + using RSpec::Parameterized::TableSyntax - ndjson_reader = described_class.new(tmpdir) + where(:link_method) { [:link, :symlink] } - expect { ndjson_reader.consume_attributes(importable_path) } - .to raise_error(Gitlab::ImportExport::Error, 'Invalid file') + with_them do + it 'raises an error' do + Dir.mktmpdir do |tmpdir| + FileUtils.touch(File.join(tmpdir, 'passwd')) + FileUtils.send(link_method, File.join(tmpdir, 'passwd'), File.join(tmpdir, 'project.json')) + + ndjson_reader = described_class.new(tmpdir) + + expect { ndjson_reader.consume_attributes(importable_path) } + .to raise_error(Gitlab::ImportExport::Error, 'Invalid file') + end end end end @@ -97,18 +103,24 @@ RSpec.describe Gitlab::ImportExport::Json::NdjsonReader, feature_category: :impo end end - context 'when relation file is a symlink' do - it 'yields nothing to the Enumerator' do - Dir.mktmpdir do |tmpdir| - Dir.mkdir(File.join(tmpdir, 'project')) - File.write(File.join(tmpdir, 'passwd'), "{}\n{}") - File.symlink(File.join(tmpdir, 'passwd'), File.join(tmpdir, 'project', 'issues.ndjson')) + context 'when relation file is a symlink or hard link' do + using RSpec::Parameterized::TableSyntax - ndjson_reader = described_class.new(tmpdir) + where(:link_method) { [:link, :symlink] } + + with_them do + it 'yields nothing to the Enumerator' do + Dir.mktmpdir do |tmpdir| + Dir.mkdir(File.join(tmpdir, 'project')) + File.write(File.join(tmpdir, 'passwd'), "{}\n{}") + FileUtils.send(link_method, File.join(tmpdir, 'passwd'), File.join(tmpdir, 'project', 'issues.ndjson')) + + ndjson_reader = described_class.new(tmpdir) - result = ndjson_reader.consume_relation(importable_path, 'issues') + result = ndjson_reader.consume_relation(importable_path, 'issues') - expect(result.to_a).to eq([]) + expect(result.to_a).to eq([]) + end end end end diff --git a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb index cb8ac088493..2b9590da421 100644 --- a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb +++ b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb @@ -4,15 +4,17 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::RecursiveMergeFolders do describe '.merge' do - it 'merge folder and ignore symlinks' do + it 'merges folder and ignores symlinks and files that share hard links' do Dir.mktmpdir do |tmpdir| source = "#{tmpdir}/source" FileUtils.mkdir_p("#{source}/folder/folder") FileUtils.touch("#{source}/file1.txt") + FileUtils.touch("#{source}/file_that_shares_hard_links.txt") FileUtils.touch("#{source}/folder/file2.txt") FileUtils.touch("#{source}/folder/folder/file3.txt") FileUtils.ln_s("#{source}/file1.txt", "#{source}/symlink-file1.txt") FileUtils.ln_s("#{source}/folder", "#{source}/symlink-folder") + FileUtils.link("#{source}/file_that_shares_hard_links.txt", "#{source}/hard_link.txt") target = "#{tmpdir}/target" FileUtils.mkdir_p("#{target}/folder/folder") diff --git a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb index 4b584a45503..49eee772f8d 100644 --- a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb +++ b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb @@ -9,6 +9,10 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do project.update_pages_deployment!(create(:pages_deployment, project: project)) end + before do + stub_pages_setting(host: 'example.com') + end + it 'returns nil when host is empty' do expect(described_class.new(nil).execute).to be_nil expect(described_class.new('').execute).to be_nil @@ -69,7 +73,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain with no lookup_paths' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to match(/pages_domain_for_namespace_#{project.namespace.id}_/) @@ -82,7 +86,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain with no lookup_paths' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}".downcase).execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com".downcase).execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to be_nil @@ -104,7 +108,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain when there are pages deployed for the project' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to match(/pages_domain_for_namespace_#{project.namespace.id}_/) @@ -113,7 +117,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'finds domain with case-insensitive' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host.upcase}").execute + virtual_domain = described_class.new("#{project.namespace.path}.Example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to match(/pages_domain_for_namespace_#{project.namespace.id}_/) @@ -127,7 +131,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end it 'returns the virual domain when there are pages deployed for the project' do - virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute + virtual_domain = described_class.new("#{project.namespace.path}.example.com").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to be_nil @@ -143,7 +147,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do project.project_setting.update!(pages_unique_domain: 'unique-domain') end - subject(:virtual_domain) { described_class.new("unique-domain.#{Settings.pages.host.upcase}").execute } + subject(:virtual_domain) { described_class.new('unique-domain.example.com').execute } context 'when pages unique domain is enabled' do before_all do @@ -171,6 +175,19 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do expect(virtual_domain.lookup_paths.first.project_id).to eq(project.id) end + context 'when a project path conflicts with a unique domain' do + it 'prioritizes the unique domain project' do + group = create(:group, path: 'unique-domain') + other_project = build(:project, path: 'unique-domain.example.com', group: group) + other_project.save!(validate: false) + other_project.update_pages_deployment!(create(:pages_deployment, project: other_project)) + other_project.mark_pages_as_deployed + + expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) + expect(virtual_domain.lookup_paths.first.project_id).to eq(project.id) + end + end + context 'when :cache_pages_domain_api is disabled' do before do stub_feature_flags(cache_pages_domain_api: false) diff --git a/spec/lib/gitlab/utils/file_info_spec.rb b/spec/lib/gitlab/utils/file_info_spec.rb new file mode 100644 index 00000000000..480036b2fd0 --- /dev/null +++ b/spec/lib/gitlab/utils/file_info_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' + +RSpec.describe Gitlab::Utils::FileInfo, feature_category: :shared do + let(:tmpdir) { Dir.mktmpdir } + let(:file_path) { "#{tmpdir}/test.txt" } + + before do + FileUtils.touch(file_path) + end + + after do + FileUtils.rm_rf(tmpdir) + end + + describe '.linked?' do + it 'raises an error when file does not exist' do + expect { subject.linked?('foo') }.to raise_error(Errno::ENOENT) + end + + shared_examples 'identifies a linked file' do + it 'returns false when file or dir is not a link' do + expect(subject.linked?(tmpdir)).to eq(false) + expect(subject.linked?(file)).to eq(false) + end + + it 'returns true when file or dir is symlinked' do + FileUtils.symlink(tmpdir, "#{tmpdir}/symlinked_dir") + FileUtils.symlink(file_path, "#{tmpdir}/symlinked_file.txt") + + expect(subject.linked?("#{tmpdir}/symlinked_dir")).to eq(true) + expect(subject.linked?("#{tmpdir}/symlinked_file.txt")).to eq(true) + end + + it 'returns true when file has more than one hard link' do + FileUtils.link(file_path, "#{tmpdir}/hardlinked_file.txt") + + expect(subject.linked?(file)).to eq(true) + expect(subject.linked?("#{tmpdir}/hardlinked_file.txt")).to eq(true) + end + end + + context 'when file is a File::Stat' do + let(:file) { File.lstat(file_path) } + + it_behaves_like 'identifies a linked file' + end + + context 'when file is path' do + let(:file) { file_path } + + it_behaves_like 'identifies a linked file' + end + end + + describe '.shares_hard_link?' do + it 'raises an error when file does not exist' do + expect { subject.shares_hard_link?('foo') }.to raise_error(Errno::ENOENT) + end + + shared_examples 'identifies a file that shares a hard link' do + it 'returns false when file or dir does not share hard links' do + expect(subject.shares_hard_link?(tmpdir)).to eq(false) + expect(subject.shares_hard_link?(file)).to eq(false) + end + + it 'returns true when file has more than one hard link' do + FileUtils.link(file_path, "#{tmpdir}/hardlinked_file.txt") + + expect(subject.shares_hard_link?(file)).to eq(true) + expect(subject.shares_hard_link?("#{tmpdir}/hardlinked_file.txt")).to eq(true) + end + end + + context 'when file is a File::Stat' do + let(:file) { File.lstat(file_path) } + + it_behaves_like 'identifies a file that shares a hard link' + end + + context 'when file is path' do + let(:file) { file_path } + + it_behaves_like 'identifies a file that shares a hard link' + end + end +end diff --git a/spec/models/project_setting_spec.rb b/spec/models/project_setting_spec.rb index 6928cc8ba08..5d06b30a529 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -76,12 +76,36 @@ RSpec.describe ProjectSetting, type: :model, feature_category: :groups_and_proje expect(project_setting).not_to be_valid expect(project_setting.errors.full_messages).to include("Pages unique domain has already been taken") end + + it "validates if the pages_unique_domain already exist as a project path" do + stub_pages_setting(host: 'example.com') + + create(:project, path: "random-unique-domain.example.com") + project_setting = build(:project_setting, pages_unique_domain: "random-unique-domain") + + expect(project_setting).not_to be_valid + expect(project_setting.errors.full_messages_for(:pages_unique_domain)) + .to match(["Pages unique domain already in use"]) + end + + context "when updating" do + it "validates if the pages_unique_domain already exist as a project path" do + stub_pages_setting(host: 'example.com') + project_setting = create(:project_setting) + + create(:project, path: "random-unique-domain.example.com") + + expect(project_setting.update(pages_unique_domain: "random-unique-domain")).to eq(false) + expect(project_setting.errors.full_messages_for(:pages_unique_domain)) + .to match(["Pages unique domain already in use"]) + end + end end describe 'target_platforms=' do it 'stringifies and sorts' do project_setting = build(:project_setting, target_platforms: [:watchos, :ios]) - expect(project_setting.target_platforms).to eq %w(ios watchos) + expect(project_setting.target_platforms).to eq %w[ios watchos] end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 044408e86e9..538f6b363e9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -827,6 +827,37 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr expect(project).to be_valid end + context 'when validating if path already exist as pages unique domain' do + before do + stub_pages_setting(host: 'example.com') + end + + it 'rejects paths that match pages unique domain' do + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + project = build(:project, path: 'some-unique-domain.example.com') + + expect(project).not_to be_valid + expect(project.errors.full_messages_for(:path)).to match(['Path already in use']) + end + + it 'accepts path when the host does not match' do + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + project = build(:project, path: 'some-unique-domain.another-example.com') + + expect(project).to be_valid + end + + it 'accepts path when the domain does not match' do + create(:project_setting, pages_unique_domain: 'another-unique-domain') + + project = build(:project, path: 'some-unique-domain.example.com') + + expect(project).to be_valid + end + end + context 'path is unchanged' do let_it_be(:invalid_path_project) do project = create(:project, :repository, :public) @@ -4729,6 +4760,33 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) end + context 'when validating if path already exist as pages unique domain' do + before do + stub_pages_setting(host: 'example.com') + end + + it 'rejects paths that match pages unique domain' do + stub_pages_setting(host: 'example.com') + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + expect(project.update(path: 'some-unique-domain.example.com')).to eq(false) + expect(project.errors.full_messages_for(:path)).to match(['Path already in use']) + end + + it 'accepts path when the host does not match' do + create(:project_setting, pages_unique_domain: 'some-unique-domain') + + expect(project.update(path: 'some-unique-domain.another-example.com')).to eq(true) + end + + it 'accepts path when the domain does not match' do + stub_pages_setting(host: 'example.com') + create(:project_setting, pages_unique_domain: 'another-unique-domain') + + expect(project.update(path: 'some-unique-domain.example.com')).to eq(true) + end + end + it 'does not validate the visibility' do expect(project).not_to receive(:visibility_level_allowed_as_fork).and_call_original expect(project).not_to receive(:visibility_level_allowed_by_group).and_call_original diff --git a/spec/services/bulk_imports/archive_extraction_service_spec.rb b/spec/services/bulk_imports/archive_extraction_service_spec.rb index 5593218c259..f76ac722ad7 100644 --- a/spec/services/bulk_imports/archive_extraction_service_spec.rb +++ b/spec/services/bulk_imports/archive_extraction_service_spec.rb @@ -43,13 +43,21 @@ RSpec.describe BulkImports::ArchiveExtractionService, feature_category: :importe context 'when archive file is a symlink' do it 'raises an error' do - FileUtils.ln_s(File.join(tmpdir, filename), File.join(tmpdir, 'symlink')) + FileUtils.ln_s(filepath, File.join(tmpdir, 'symlink')) expect { described_class.new(tmpdir: tmpdir, filename: 'symlink').execute } .to raise_error(BulkImports::Error, 'Invalid file') end end + context 'when archive file shares multiple hard links' do + it 'raises an error' do + FileUtils.link(filepath, File.join(tmpdir, 'hard_link')) + + expect { subject.execute }.to raise_error(BulkImports::Error, 'Invalid file') + end + end + context 'when filepath is being traversed' do it 'raises an error' do expect { described_class.new(tmpdir: File.join(Dir.mktmpdir, 'test', '..'), filename: 'name').execute } diff --git a/spec/services/bulk_imports/file_decompression_service_spec.rb b/spec/services/bulk_imports/file_decompression_service_spec.rb index 9d80ab3cd8f..e6d919c3499 100644 --- a/spec/services/bulk_imports/file_decompression_service_spec.rb +++ b/spec/services/bulk_imports/file_decompression_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe BulkImports::FileDecompressionService, feature_category: :importers do + using RSpec::Parameterized::TableSyntax + let_it_be(:tmpdir) { Dir.mktmpdir } let_it_be(:ndjson_filename) { 'labels.ndjson' } let_it_be(:ndjson_filepath) { File.join(tmpdir, ndjson_filename) } @@ -70,39 +72,68 @@ RSpec.describe BulkImports::FileDecompressionService, feature_category: :importe end end - context 'when compressed file is a symlink' do - let_it_be(:symlink) { File.join(tmpdir, 'symlink.gz') } + shared_examples 'raises an error and removes the file' do |error_message:| + specify do + expect { subject.execute } + .to raise_error(BulkImports::FileDecompressionService::ServiceError, error_message) + expect(File).not_to exist(file) + end + end + + shared_context 'when compressed file' do + let_it_be(:file) { File.join(tmpdir, 'file.gz') } + + subject { described_class.new(tmpdir: tmpdir, filename: 'file.gz') } before do - FileUtils.ln_s(File.join(tmpdir, gz_filename), symlink) + FileUtils.send(link_method, File.join(tmpdir, gz_filename), file) end + end - subject { described_class.new(tmpdir: tmpdir, filename: 'symlink.gz') } + shared_context 'when decompressed file' do + let_it_be(:file) { File.join(tmpdir, 'file.txt') } - it 'raises an error and removes the file' do - expect { subject.execute } - .to raise_error(BulkImports::FileDecompressionService::ServiceError, 'File decompression error') + subject { described_class.new(tmpdir: tmpdir, filename: gz_filename) } - expect(File.exist?(symlink)).to eq(false) + before do + original_file = File.join(tmpdir, 'original_file.txt') + FileUtils.touch(original_file) + FileUtils.send(link_method, original_file, file) + + subject.instance_variable_set(:@decompressed_filepath, file) end end + context 'when compressed file is a symlink' do + let(:link_method) { :symlink } + + include_context 'when compressed file' + + include_examples 'raises an error and removes the file', error_message: 'File decompression error' + end + + context 'when compressed file shares multiple hard links' do + let(:link_method) { :link } + + include_context 'when compressed file' + + include_examples 'raises an error and removes the file', error_message: 'File decompression error' + end + context 'when decompressed file is a symlink' do - let_it_be(:symlink) { File.join(tmpdir, 'symlink') } + let(:link_method) { :symlink } - before do - FileUtils.ln_s(File.join(tmpdir, ndjson_filename), symlink) + include_context 'when decompressed file' - subject.instance_variable_set(:@decompressed_filepath, symlink) - end + include_examples 'raises an error and removes the file', error_message: 'Invalid file' + end - subject { described_class.new(tmpdir: tmpdir, filename: gz_filename) } + context 'when decompressed file shares multiple hard links' do + let(:link_method) { :link } - it 'raises an error and removes the file' do - expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid file') + include_context 'when decompressed file' - expect(File.exist?(symlink)).to eq(false) - end + include_examples 'raises an error and removes the file', error_message: 'Invalid file' end end end diff --git a/spec/services/bulk_imports/file_download_service_spec.rb b/spec/services/bulk_imports/file_download_service_spec.rb index cbeea5b0f46..c035eabf767 100644 --- a/spec/services/bulk_imports/file_download_service_spec.rb +++ b/spec/services/bulk_imports/file_download_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe BulkImports::FileDownloadService, feature_category: :importers do let_it_be(:content_type) { 'application/octet-stream' } let_it_be(:content_disposition) { nil } let_it_be(:filename) { 'file_download_service_spec' } - let_it_be(:tmpdir) { Dir.tmpdir } + let_it_be(:tmpdir) { Dir.mktmpdir } let_it_be(:filepath) { File.join(tmpdir, filename) } let_it_be(:content_length) { 1000 } @@ -247,6 +247,36 @@ RSpec.describe BulkImports::FileDownloadService, feature_category: :importers do end end + context 'when file shares multiple hard links' do + let_it_be(:hard_link) { File.join(tmpdir, 'hard_link') } + + before do + existing_file = File.join(Dir.mktmpdir, filename) + FileUtils.touch(existing_file) + FileUtils.link(existing_file, hard_link) + end + + subject do + described_class.new( + configuration: config, + relative_url: '/test', + tmpdir: tmpdir, + filename: 'hard_link', + file_size_limit: file_size_limit, + allowed_content_types: allowed_content_types + ) + end + + it 'raises an error and removes the file' do + expect { subject.execute }.to raise_error( + described_class::ServiceError, + 'Invalid downloaded file' + ) + + expect(File.exist?(hard_link)).to eq(false) + end + end + context 'when dir is not in tmpdir' do subject do described_class.new( -- cgit v1.2.3