diff options
60 files changed, 234 insertions, 1050 deletions
diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 1d41cc41315..1f90a4c3ba5 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -5107,6 +5107,7 @@ RSpec/MissingFeatureCategory: - 'spec/policies/ci/bridge_policy_spec.rb' - 'spec/policies/ci/build_policy_spec.rb' - 'spec/policies/ci/pipeline_policy_spec.rb' + - 'spec/policies/ci/pipeline_schedule_policy_spec.rb' - 'spec/policies/ci/trigger_policy_spec.rb' - 'spec/policies/clusters/agent_policy_spec.rb' - 'spec/policies/clusters/agent_token_policy_spec.rb' diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index acdb68bc0e6..2786e971f4b 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -175,7 +175,7 @@ export default { originalStartLineCode, ...(discussion.line_codes || []), ]; - const fileHash = discussion.diff_file?.file_hash; + const fileHash = discussion.diff_file.file_hash; const lineCheck = (line) => discussionLineCodes.some( (discussionLineCode) => diff --git a/app/assets/javascripts/diffs/utils/diff_file.js b/app/assets/javascripts/diffs/utils/diff_file.js index 98e1c1cc849..f2a3224d332 100644 --- a/app/assets/javascripts/diffs/utils/diff_file.js +++ b/app/assets/javascripts/diffs/utils/diff_file.js @@ -77,7 +77,7 @@ export function prepareRawDiffFile({ file, allFiles, meta = false, index = -1 }) } export function collapsedType(file) { - const isManual = typeof file?.viewer?.manuallyCollapsed === 'boolean'; + const isManual = typeof file.viewer?.manuallyCollapsed === 'boolean'; return isManual ? DIFF_FILE_MANUAL_COLLAPSE : DIFF_FILE_AUTOMATIC_COLLAPSE; } @@ -85,8 +85,8 @@ export function collapsedType(file) { export function isCollapsed(file) { const type = collapsedType(file); const collapsedStates = { - [DIFF_FILE_AUTOMATIC_COLLAPSE]: file?.viewer?.automaticallyCollapsed || false, - [DIFF_FILE_MANUAL_COLLAPSE]: file?.viewer?.manuallyCollapsed, + [DIFF_FILE_AUTOMATIC_COLLAPSE]: file.viewer?.automaticallyCollapsed || false, + [DIFF_FILE_MANUAL_COLLAPSE]: file.viewer?.manuallyCollapsed, }; return collapsedStates[type]; diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index b1a2ab77fa8..db32079e6b9 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -41,7 +41,7 @@ export default { return getDiffMode(this.discussion.diff_file); }, diffViewerMode() { - return this.discussion.diff_file?.viewer.name; + return this.discussion.diff_file.viewer.name; }, fileDiffRefs() { return this.discussion.diff_file.diff_refs; @@ -96,7 +96,6 @@ export default { <template> <div :class="{ 'text-file': isTextFile }" class="diff-file file-holder"> <diff-file-header - v-if="discussion.diff_file" :discussion-path="discussion.discussion_path" :diff-file="discussion.diff_file" :can-current-user-fork="false" diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 5216bc406ef..499581653ba 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -157,7 +157,7 @@ export default { return !this.discussionResolved ? this.discussion.resolve_with_issue_path : ''; }, canShowReplyActions() { - if (this.shouldRenderDiffs && !this.discussion.diff_file?.diff_refs) { + if (this.shouldRenderDiffs && !this.discussion.diff_file.diff_refs) { return false; } diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb index cd495627d66..fb332fec3b5 100644 --- a/app/controllers/projects/pipeline_schedules_controller.rb +++ b/app/controllers/projects/pipeline_schedules_controller.rb @@ -21,6 +21,7 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController end def new + @schedule = project.pipeline_schedules.new end def create @@ -101,15 +102,6 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController variables_attributes: [:id, :variable_type, :key, :secret_value, :_destroy]) end - def new_schedule - # We need the `ref` here for `authorize_create_pipeline_schedule!` - @schedule ||= project.pipeline_schedules.new(ref: params.dig(:schedule, :ref)) - end - - def authorize_create_pipeline_schedule! - return access_denied! unless can?(current_user, :create_pipeline_schedule, new_schedule) - end - def authorize_play_pipeline_schedule! return access_denied! unless can?(current_user, :play_pipeline_schedule, schedule) end diff --git a/app/models/project.rb b/app/models/project.rb index be5f85a25e2..452a5c8973c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -579,8 +579,6 @@ 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) } @@ -3243,15 +3241,6 @@ class Project < ApplicationRecord end strong_memoize_attr :frozen_outbound_job_token_scopes? - 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 def pages_unique_domain_enabled? diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index aeefa5c8dcd..7ca74d4e970 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -59,8 +59,6 @@ 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 @@ -111,15 +109,6 @@ 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/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb index cbc60c4a30a..7b0d484f9f7 100644 --- a/app/policies/ci/pipeline_schedule_policy.rb +++ b/app/policies/ci/pipeline_schedule_policy.rb @@ -5,18 +5,7 @@ module Ci alias_method :pipeline_schedule, :subject condition(:protected_ref) do - if full_ref?(@subject.ref) - is_tag = Gitlab::Git.tag_ref?(@subject.ref) - ref_name = Gitlab::Git.ref_name(@subject.ref) - else - # NOTE: this block should not be removed - # until the full ref validation is in place - # and all old refs are updated and validated - is_tag = @subject.project.repository.tag_exists?(@subject.ref) - ref_name = @subject.ref - end - - ref_protected?(@user, @subject.project, is_tag, ref_name) + ref_protected?(@user, @subject.project, @subject.project.repository.tag_exists?(@subject.ref), @subject.ref) end condition(:owner_of_schedule) do @@ -42,15 +31,6 @@ module Ci enable :take_ownership_pipeline_schedule end - rule { protected_ref }.policy do - prevent :play_pipeline_schedule - prevent :create_pipeline_schedule - end - - private - - def full_ref?(ref) - Gitlab::Git.tag_ref?(ref) || Gitlab::Git.branch_ref?(ref) - end + rule { protected_ref }.prevent :play_pipeline_schedule end end diff --git a/app/services/bulk_imports/archive_extraction_service.rb b/app/services/bulk_imports/archive_extraction_service.rb index bce2a67218a..4485b19035b 100644 --- a/app/services/bulk_imports/archive_extraction_service.rb +++ b/app/services/bulk_imports/archive_extraction_service.rb @@ -49,7 +49,11 @@ module BulkImports end def validate_symlink - raise(BulkImports::Error, 'Invalid file') if Gitlab::Utils::FileInfo.linked?(filepath) + raise(BulkImports::Error, 'Invalid file') if symlink?(filepath) + end + + def symlink?(filepath) + File.lstat(filepath).symlink? 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 77638f10f54..94573f6bb13 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 Gitlab::Utils::FileInfo.linked?(filepath) + raise(ServiceError, 'Invalid file') if File.lstat(filepath).symlink? end def decompress_file diff --git a/app/services/discussions/capture_diff_note_positions_service.rb b/app/services/discussions/capture_diff_note_positions_service.rb index f9b31e0f2f1..3684a3f679a 100644 --- a/app/services/discussions/capture_diff_note_positions_service.rb +++ b/app/services/discussions/capture_diff_note_positions_service.rb @@ -26,7 +26,7 @@ module Discussions active_diff_discussions = merge_request.notes.new_diff_notes.discussions.select do |discussion| discussion.active?(merge_request.diff_refs) end - paths = active_diff_discussions.flat_map { |n| n.diff_file&.paths } + paths = active_diff_discussions.flat_map { |n| n.diff_file.paths } [active_diff_discussions, paths] end diff --git a/config/application.rb b/config/application.rb index 757d62c72ad..7eeb75a73cd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -172,7 +172,6 @@ module Gitlab # - Any parameter containing `password` # - Any parameter containing `secret` # - Any parameter ending with `key` - # - Any parameter named `redirect`, filtered for security concerns of exposing sensitive information # - Two-factor tokens (:otp_attempt) # - Repo/Project Import URLs (:import_url) # - Build traces (:trace) @@ -215,7 +214,6 @@ module Gitlab variables content sharedSecret - redirect ) # Enable escaping HTML in JSON. diff --git a/lib/banzai/filter/autolink_filter.rb b/lib/banzai/filter/autolink_filter.rb index bbddaa37380..336d60055e2 100644 --- a/lib/banzai/filter/autolink_filter.rb +++ b/lib/banzai/filter/autolink_filter.rb @@ -34,13 +34,8 @@ module Banzai # https://github.com/vmg/rinku/blob/v2.0.1/ext/rinku/autolink.c#L65 # # Rubular: http://rubular.com/r/nrL3r9yUiq - # Note that it's not possible to use Gitlab::UntrustedRegexp for LINK_PATTERN, - # as `(?<!` is unsupported in `re2`, see https://github.com/google/re2/wiki/Syntax LINK_PATTERN = %r{([a-z][a-z0-9\+\.-]+://[^\s>]+)(?<!\?|!|\.|,|:)}.freeze - ENTITY_UNTRUSTED = '((?:&[\w#]+;)+)\z' - ENTITY_UNTRUSTED_REGEX = Gitlab::UntrustedRegexp.new(ENTITY_UNTRUSTED, multiline: false) - # Text matching LINK_PATTERN inside these elements will not be linked IGNORE_PARENTS = %w(a code kbd pre script style).to_set @@ -90,14 +85,10 @@ module Banzai # Remove any trailing HTML entities and store them for appending # outside the link element. The entity must be marked HTML safe in # order to be output literally rather than escaped. - dropped = '' - match = ENTITY_UNTRUSTED_REGEX.replace_gsub(match) do |entities| - dropped = entities[1].html_safe - - '' - end + match.gsub!(/((?:&[\w#]+;)+)\z/, '') + dropped = (Regexp.last_match(1) || '').html_safe - # To match the behavior of Rinku, if the matched link ends with a + # To match the behaviour of Rinku, if the matched link ends with a # closing part of a matched pair of punctuation, we remove that trailing # character unless there are an equal number of closing and opening # characters in the link. diff --git a/lib/banzai/filter/plantuml_filter.rb b/lib/banzai/filter/plantuml_filter.rb index 7e6535b86fd..2e5f1b29c52 100644 --- a/lib/banzai/filter/plantuml_filter.rb +++ b/lib/banzai/filter/plantuml_filter.rb @@ -11,7 +11,7 @@ module Banzai def call return doc unless settings.plantuml_enabled? && doc.at_xpath(lang_tag) - Gitlab::Plantuml.configure + plantuml_setup doc.xpath(lang_tag).each do |node| img_tag = Nokogiri::HTML::DocumentFragment.parse( @@ -38,6 +38,15 @@ module Banzai def settings Gitlab::CurrentSettings.current_application_settings end + + def plantuml_setup + Asciidoctor::PlantUml.configure do |conf| + conf.url = settings.plantuml_url + conf.png_enable = settings.plantuml_enabled + conf.svg_enable = false + conf.txt_enable = false + end + end end end end diff --git a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb index 9a22d211ba1..68bd64dc2ff 100644 --- a/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/lfs_objects_pipeline.rb @@ -24,7 +24,7 @@ module BulkImports return if tar_filepath?(file_path) return if lfs_json_filepath?(file_path) return if File.directory?(file_path) - return if Gitlab::Utils::FileInfo.linked?(file_path) + return if File.lstat(file_path).symlink? 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 b1eeccf96f7..06132791ea6 100644 --- a/lib/bulk_imports/common/pipelines/uploads_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/uploads_pipeline.rb @@ -24,7 +24,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 Gitlab::Utils::FileInfo.linked?(file_path) + return if File.lstat(file_path).symlink? 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 e1844843408..b852a50c888 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 Gitlab::Utils::FileInfo.linked?(filepath) + return unless File.lstat(filepath).symlink? 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 235d2629b9e..373cd2bd75a 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 Gitlab::Utils::FileInfo.linked?(bundle_path) + return if File.lstat(bundle_path).symlink? 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 4307cb2bafd..f19d8931f4a 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 Gitlab::Utils::FileInfo.linked?(bundle_path) + return if File.lstat(bundle_path).symlink? portable.repository.create_from_bundle(bundle_path) end diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index 31e8dcd84b7..955cb14594f 100644 --- a/lib/gitlab/asciidoc.rb +++ b/lib/gitlab/asciidoc.rb @@ -78,11 +78,20 @@ module Gitlab context[:pipeline] = :ascii_doc context[:max_includes] = [MAX_INCLUDES, context[:max_includes]].compact.min - Gitlab::Plantuml.configure + plantuml_setup html = ::Asciidoctor.convert(input, asciidoc_opts) html = Banzai.render(html, context) html.html_safe end + + def self.plantuml_setup + Asciidoctor::PlantUml.configure do |conf| + conf.url = Gitlab::CurrentSettings.plantuml_url + conf.svg_enable = Gitlab::CurrentSettings.plantuml_enabled + conf.png_enable = Gitlab::CurrentSettings.plantuml_enabled + conf.txt_enable = false + end + end end end diff --git a/lib/gitlab/ci/decompressed_gzip_size_validator.rb b/lib/gitlab/ci/decompressed_gzip_size_validator.rb index b386e400423..9b7b5f0dd66 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 or hard link') if Gitlab::Utils::FileInfo.linked?(archive_path) + raise(ServiceError, 'Archive path is a symlink') if File.lstat(archive_path).symlink? 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 e2f365fcbf8..d681f39f00b 100644 --- a/lib/gitlab/import_export/command_line_util.rb +++ b/lib/gitlab/import_export/command_line_util.rb @@ -5,11 +5,8 @@ module Gitlab module CommandLineUtil UNTAR_MASK = 'u+rwX,go+rX,go-w' DEFAULT_DIR_MODE = 0700 - CLEAN_DIR_IGNORE_FILE_NAMES = %w[. ..].freeze - CommandLineUtilError = Class.new(StandardError) - FileOversizedError = Class.new(CommandLineUtilError) - HardLinkError = Class.new(CommandLineUtilError) + FileOversizedError = Class.new(StandardError) def tar_czf(archive:, dir:) tar_with_options(archive: archive, dir: dir, options: 'czf') @@ -93,7 +90,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})) - clean_extraction_dir!(dir) + remove_symlinks(dir) end # rubocop:disable Gitlab/ModuleWithInstanceVariables @@ -125,27 +122,17 @@ module Gitlab true end - # 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).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) + def remove_symlinks(dir) + ignore_file_names = %w[. ..] - FileUtils.rm(filepath) if Gitlab::Utils::FileInfo.linked?(filepath) - end + # 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 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 2e39f3f38c2..104c9e6c456 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 or hard link') if Gitlab::Utils::FileInfo.linked?(@archive_path) + raise(ServiceError, 'Archive path is a symlink') if File.lstat(@archive_path).symlink? 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 37c83e88ef2..d2593289c23 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) - clean_extraction_dir!(@shared.export_path) + remove_symlinks(@shared.export_path) copy_archive wait_for_archived_file do @@ -35,7 +35,7 @@ module Gitlab false ensure remove_import_file - clean_extraction_dir!(@shared.export_path) + remove_symlinks(@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 93a94716f8d..3de56aacf18 100644 --- a/lib/gitlab/import_export/json/ndjson_reader.rb +++ b/lib/gitlab/import_export/json/ndjson_reader.rb @@ -21,9 +21,7 @@ module Gitlab # This reads from `tree/project.json` path = file_path("#{importable_path}.json") - if !File.exist?(path) || Gitlab::Utils::FileInfo.linked?(path) - raise Gitlab::ImportExport::Error, 'Invalid file' - end + raise Gitlab::ImportExport::Error, 'Invalid file' if !File.exist?(path) || File.symlink?(path) data = File.read(path, MAX_JSON_DOCUMENT_SIZE) json_decode(data) @@ -36,7 +34,7 @@ module Gitlab # This reads from `tree/project/merge_requests.ndjson` path = file_path(importable_path, "#{key}.ndjson") - next if !File.exist?(path) || Gitlab::Utils::FileInfo.linked?(path) + next if !File.exist?(path) || File.symlink?(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 e6eba60db93..827385d4daf 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 Gitlab::Utils::FileInfo.linked?(source_child) + next if File.lstat(source_child).symlink? 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 d5e2159fb52..5fec60188f8 100644 --- a/lib/gitlab/pages/virtual_host_finder.rb +++ b/lib/gitlab/pages/virtual_host_finder.rb @@ -10,12 +10,13 @@ module Gitlab def execute return if host.blank? - gitlab_host = ::Gitlab.config.pages.host.downcase.prepend(".") + gitlab_host = ::Settings.pages.host.downcase.prepend(".") if host.ends_with?(gitlab_host) name = host.delete_suffix(gitlab_host) - by_unique_domain(name) || by_namespace_domain(name) + by_namespace_domain(name) || + by_unique_domain(name) else by_custom_domain(host) end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index 8afcf682d5d..e112423f167 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -131,7 +131,7 @@ module Gitlab # `NAMESPACE_FORMAT_REGEX`, with the negative lookbehind assertion removed. This means that the client-side validation # will pass for usernames ending in `.atom` and `.git`, but will be caught by the server-side validation. PATH_START_CHAR = '[a-zA-Z0-9_\.]' - PATH_REGEX_STR = PATH_START_CHAR + '[a-zA-Z0-9_\-\.]' + "{0,#{Namespace::URL_MAX_LENGTH - 1}}" + PATH_REGEX_STR = PATH_START_CHAR + '[a-zA-Z0-9_\-\.]*' NAMESPACE_FORMAT_REGEX_JS = PATH_REGEX_STR + '[a-zA-Z0-9_\-]|[a-zA-Z0-9_]' NO_SUFFIX_REGEX = /(?<!\.git|\.atom)/.freeze diff --git a/lib/gitlab/plantuml.rb b/lib/gitlab/plantuml.rb deleted file mode 100644 index 9ec544452fd..00000000000 --- a/lib/gitlab/plantuml.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require "asciidoctor_plantuml/plantuml" - -module Gitlab - module Plantuml - class << self - def configure - Asciidoctor::PlantUml.configure do |conf| - conf.url = Gitlab::CurrentSettings.plantuml_url - conf.png_enable = Gitlab::CurrentSettings.plantuml_enabled - conf.svg_enable = false - conf.txt_enable = false - - conf - end - end - end - end -end diff --git a/lib/gitlab/utils/file_info.rb b/lib/gitlab/utils/file_info.rb deleted file mode 100644 index a0ec370e225..00000000000 --- a/lib/gitlab/utils/file_info.rb +++ /dev/null @@ -1,35 +0,0 @@ -# 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 bd8e9bfff8c..4b7414f4480 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -36421,9 +36421,6 @@ msgstr "" msgid "ProjectSettings|With GitLab Pages you can host your static websites on GitLab. GitLab Pages uses a caching mechanism for efficiency. Your changes may not take effect until that cache is invalidated, which usually takes less than a minute." msgstr "" -msgid "ProjectSetting|already in use" -msgstr "" - msgid "ProjectTemplates|.NET Core" msgstr "" @@ -36721,9 +36718,6 @@ msgstr "" msgid "ProjectsNew|Your project will be created at:" msgstr "" -msgid "Project|already in use" -msgstr "" - msgid "PrometheusAlerts|exceeded" msgstr "" diff --git a/package.json b/package.json index c63983c4968..e679bed69c5 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "@gitlab/svgs": "3.53.0", "@gitlab/ui": "64.10.1", "@gitlab/visual-review-tools": "1.7.3", - "@gitlab/web-ide": "0.0.1-dev-20230713160749-patch-1", + "@gitlab/web-ide": "0.0.1-dev-20230614124516", "@mattiasbuelens/web-streams-adapter": "^0.1.0", "@popperjs/core": "^2.11.2", "@rails/actioncable": "6.1.4-7", diff --git a/spec/controllers/projects/pipeline_schedules_controller_spec.rb b/spec/controllers/projects/pipeline_schedules_controller_spec.rb index 4aa2bac5259..6d810fdcd51 100644 --- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb +++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' RSpec.describe Projects::PipelineSchedulesController, feature_category: :continuous_integration do include AccessMatchersForController - using RSpec::Parameterized::TableSyntax let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } @@ -46,43 +45,6 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end - shared_examples 'protecting ref' do - where(:branch_access_levels, :tag_access_level, :maintainer_accessible, :developer_accessible) do - [:no_one_can_push, :no_one_can_merge] | :no_one_can_create | \ - :be_denied_for | :be_denied_for - [:maintainers_can_push, :maintainers_can_merge] | :maintainers_can_create | \ - :be_allowed_for | :be_denied_for - [:developers_can_push, :developers_can_merge] | :developers_can_create | \ - :be_allowed_for | :be_allowed_for - end - - with_them do - context 'when branch is protected' do - let(:ref_prefix) { 'heads' } - let(:ref_name) { 'master' } - - before do - create(:protected_branch, *branch_access_levels, name: ref_name, project: project) - end - - it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) } - it { expect { go }.to try(developer_accessible, :developer).of(project) } - end - - context 'when tag is protected' do - let(:ref_prefix) { 'tags' } - let(:ref_name) { 'v1.0.0' } - - before do - create(:protected_tag, tag_access_level, name: ref_name, project: project) - end - - it { expect { go }.to try(maintainer_accessible, :maintainer).of(project) } - it { expect { go }.to try(developer_accessible, :developer).of(project) } - end - end - end - describe 'GET #index' do render_views @@ -196,9 +158,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end describe 'security' do - let(:schedule) { attributes_for(:ci_pipeline_schedule, ref: "refs/#{ref_prefix}/#{ref_name}") } - let(:ref_prefix) { 'heads' } - let(:ref_name) { "master" } + let(:schedule) { attributes_for(:ci_pipeline_schedule) } it 'is allowed for admin when admin mode enabled', :enable_admin_mode do expect { go }.to be_allowed_for(:admin) @@ -217,8 +177,6 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it { expect { go }.to be_denied_for(:user) } it { expect { go }.to be_denied_for(:external) } it { expect { go }.to be_denied_for(:visitor) } - - it_behaves_like 'protecting ref' end def go @@ -469,7 +427,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end describe 'POST #play', :clean_gitlab_redis_rate_limiting do - let(:ref_name) { 'master' } + let(:ref) { 'master' } before do project.add_developer(user) @@ -485,7 +443,7 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it 'does not allow pipeline to be executed' do expect(RunPipelineScheduleWorker).not_to receive(:perform_async) - go + post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } expect(response).to have_gitlab_http_status(:not_found) end @@ -495,14 +453,16 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu it 'executes a new pipeline' do expect(RunPipelineScheduleWorker).to receive(:perform_async).with(pipeline_schedule.id, user.id).and_return('job-123') - go + post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } expect(flash[:notice]).to start_with 'Successfully scheduled a pipeline to run' expect(response).to have_gitlab_http_status(:found) end it 'prevents users from scheduling the same pipeline repeatedly' do - 2.times { go } + 2.times do + post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } + end expect(flash.to_a.size).to eq(2) expect(flash[:alert]).to eq _('You cannot play this scheduled pipeline at the moment. Please wait a minute.') @@ -510,14 +470,17 @@ RSpec.describe Projects::PipelineSchedulesController, feature_category: :continu end end - describe 'security' do - let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, ref: "refs/#{ref_prefix}/#{ref_name}") } + context 'when a developer attempts to schedule a protected ref' do + it 'does not allow pipeline to be executed' do + create(:protected_branch, project: project, name: ref) + protected_schedule = create(:ci_pipeline_schedule, project: project, ref: ref) - it_behaves_like 'protecting ref' - end + expect(RunPipelineScheduleWorker).not_to receive(:perform_async) - def go - post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id } + post :play, params: { namespace_id: project.namespace.to_param, project_id: project, id: protected_schedule.id } + + expect(response).to have_gitlab_http_status(:not_found) + end end end diff --git a/spec/factories/diff_position.rb b/spec/factories/diff_position.rb index 87875e0fa4d..bd248452de8 100644 --- a/spec/factories/diff_position.rb +++ b/spec/factories/diff_position.rb @@ -54,10 +54,6 @@ FactoryBot.define do end end - factory :file_diff_position do - position_type { 'file' } - end - factory :image_diff_position do position_type { 'image' } x { 1 } diff --git a/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb b/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb deleted file mode 100644 index 26dbf5605ec..00000000000 --- a/spec/features/merge_request/user_views_comment_on_diff_file_spec.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'User views comment on a diff file', :js, feature_category: :code_review_workflow do - include MergeRequestDiffHelpers - include RepoHelpers - - let_it_be(:project) { create(:project, :repository) } - let_it_be(:merge_request) do - create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') - end - - let_it_be(:user) { create(:user) } - - before_all do - project.add_maintainer(user) - end - - before do - sign_in(user) - - visit(diffs_project_merge_request_path(project, merge_request)) - end - - context 'with invalid start_sha position' do - before do - diff_refs = Gitlab::Diff::DiffRefs.new( - base_sha: merge_request.diff_refs.base_sha, - start_sha: 'fakesha', - head_sha: merge_request.diff_refs.head_sha - ) - position = build(:file_diff_position, file: 'files/ruby/popen.rb', diff_refs: diff_refs) - create(:diff_note_on_merge_request, noteable: merge_request, project: project, position: position) - end - - it 'renders diffs' do - visit diffs_project_merge_request_path(project, merge_request) - - expect(page).to have_selector('.diff-file') - end - - it 'renders discussion on overview tab' do - visit project_merge_request_path(project, merge_request) - - expect(page).to have_selector('.note-discussion') - end - end -end diff --git a/spec/lib/banzai/filter/autolink_filter_spec.rb b/spec/lib/banzai/filter/autolink_filter_spec.rb index e033567753c..2c75377ec42 100644 --- a/spec/lib/banzai/filter/autolink_filter_spec.rb +++ b/spec/lib/banzai/filter/autolink_filter_spec.rb @@ -226,22 +226,6 @@ RSpec.describe Banzai::Filter::AutolinkFilter, feature_category: :team_planning end end - it 'protects against malicious backtracking' do - doc = "http://#{'&' * 1_000_000}x" - - expect do - Timeout.timeout(30.seconds) { filter(doc) } - end.not_to raise_error - end - - it 'does not timeout with excessively long scheme' do - doc = "#{'h' * 1_000_000}://example.com" - - expect do - Timeout.timeout(30.seconds) { filter(doc) } - end.not_to raise_error - end - # Rinku does not escape these characters in HTML attributes, but content_tag # does. We don't care about that difference for these specs, though. def unescape(html) diff --git a/spec/lib/banzai/filter/references/project_reference_filter_spec.rb b/spec/lib/banzai/filter/references/project_reference_filter_spec.rb index b6d6ff2309a..49c71d08364 100644 --- a/spec/lib/banzai/filter/references/project_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/project_reference_filter_spec.rb @@ -39,7 +39,6 @@ RSpec.describe Banzai::Filter::References::ProjectReferenceFilter, feature_categ it_behaves_like 'fails fast', 'A' * 50000 it_behaves_like 'fails fast', '/a' * 50000 - it_behaves_like 'fails fast', "mailto:#{'a-' * 499_000}@aaaaaaaa..aaaaaaaa.example.com" end it 'allows references with text after the > character' do 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 50640e1e4ba..297ac0ca0ba 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, feature_category: :importers do +RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline do let_it_be(:portable) { create(:project) } let_it_be(:oid) { 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' } @@ -118,19 +118,10 @@ RSpec.describe BulkImports::Common::Pipelines::LfsObjectsPipeline, feature_categ context 'when file path is symlink' do it 'returns' do symlink = File.join(tmpdir, 'symlink') - FileUtils.ln_s(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 + FileUtils.ln_s(File.join(tmpdir, lfs_file_path), symlink) - 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 } + expect { pipeline.load(context, symlink) }.not_to change { portable.lfs_objects.count } end end 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 09c8c7b92c2..bc6d36452b4 100644 --- a/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/uploads_pipeline_spec.rb @@ -105,7 +105,6 @@ 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 @@ -119,19 +118,10 @@ 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) - 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 + FileUtils.ln_s(File.join(tmpdir, upload_file_path), symlink) - 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 } + expect { pipeline.load(context, symlink) }.not_to change { portable.uploads.count } end end 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 87efad92131..5b7309b09f5 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, feature_category: :importers do +RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline do let_it_be(:design) { create(:design, :with_file) } let(:portable) { create(:project) } @@ -125,25 +125,12 @@ RSpec.describe BulkImports::Projects::Pipelines::DesignBundlePipeline, feature_c context 'when path is symlink' do it 'returns' do symlink = File.join(tmpdir, 'symlink') - FileUtils.ln_s(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) + FileUtils.ln_s(File.join(tmpdir, design_bundle_path), symlink) - expect(portable.design_repository.exists?).to eq(false) - 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) + pipeline.load(context, symlink) expect(portable.design_repository.exists?).to eq(false) end 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 5aae8c959fa..07fafc19026 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, feature_category: :importers do +RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline do let_it_be(:source) { create(:project, :repository) } let(:portable) { create(:project) } @@ -123,25 +123,12 @@ RSpec.describe BulkImports::Projects::Pipelines::RepositoryBundlePipeline, featu context 'when path is symlink' do it 'returns' do symlink = File.join(tmpdir, 'symlink') - FileUtils.ln_s(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) - - expect(portable.repository.exists?).to eq(false) - end - end + FileUtils.ln_s(File.join(tmpdir, bundle_path), symlink) - 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) + pipeline.load(context, symlink) expect(portable.repository.exists?).to eq(false) end 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 dad5bd2548b..6ca3f4d415e 100644 --- a/spec/lib/gitlab/ci/decompressed_gzip_size_validator_spec.rb +++ b/spec/lib/gitlab/ci/decompressed_gzip_size_validator_spec.rb @@ -105,16 +105,6 @@ 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 086aa4be17e..84d6713efdb 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, feature_category: :importers do +RSpec.describe Gitlab::GithubImport::AttachmentsDownloader do subject(:downloader) { described_class.new(file_url) } let_it_be(:file_url) { 'https://example.com/avatar.png' } @@ -39,26 +39,6 @@ RSpec.describe Gitlab::GithubImport::AttachmentsDownloader, feature_category: :i 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 b2e047f5621..91cfab1688a 100644 --- a/spec/lib/gitlab/import_export/command_line_util_spec.rb +++ b/spec/lib/gitlab/import_export/command_line_util_spec.rb @@ -5,16 +5,13 @@ 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) } - # 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(:tmpdir) { Dir.mktmpdir } let(:archive_dir) { Dir.mktmpdir } - subject(:mock_class) do + subject do Class.new do include Gitlab::ImportExport::CommandLineUtil @@ -28,59 +25,38 @@ RSpec.describe Gitlab::ImportExport::CommandLineUtil, feature_category: :importe end before do - FileUtils.mkdir_p(source_dir) + FileUtils.mkdir_p(path) end after do - FileUtils.rm_rf(source_dir) - FileUtils.rm_rf(target_dir) + FileUtils.rm_rf(path) 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("#{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) + 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) end end @@ -236,8 +212,6 @@ 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) @@ -255,16 +229,14 @@ 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(path, filename)) + FileUtils.copy_file(gz_filepath, File.join(tmpdir, filename)) - subject.gunzip(dir: path, filename: filename) + subject.gunzip(dir: tmpdir, filename: filename) - expect(File.exist?(File.join(path, 'labels.ndjson'))).to eq(true) + expect(File.exist?(File.join(tmpdir, 'labels.ndjson'))).to eq(true) end context 'when exception occurs' do @@ -278,7 +250,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: source_dir) + result = subject.tar_cf(archive: archive_file, dir: tmpdir) expect(result).to eq(true) expect(File.exist?(archive_file)).to eq(true) @@ -298,35 +270,29 @@ 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: tar_archive_fixture, dir: target_dir) + subject.untar_zxf(archive: archive, dir: path) - expect(file_permissions("#{target_dir}/project.json")).to eq(0755) # originally 777 + expect(file_permissions("#{path}/project.json")).to eq(0755) # originally 777 end it 'has the right mask for uploads' do - subject.untar_zxf(archive: tar_archive_fixture, dir: target_dir) + subject.untar_zxf(archive: archive, dir: path) - expect(file_permissions("#{target_dir}/uploads")).to eq(0755) # originally 555 + expect(file_permissions("#{path}/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(tar_archive_fixture, File.join(archive_dir, filename)) + FileUtils.copy_file(archive, 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 8c5823edc51..a6cb74c3c9f 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, feature_category: :importers do +RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator 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, feature_c context 'which archive path is a symlink' do let(:filepath) { File.join(Dir.tmpdir, 'symlink') } - let(:error_message) { 'Archive path is a symlink or hard link' } + let(:error_message) { 'Archive path is a symlink' } before do FileUtils.ln_s(filepath, filepath, force: true) @@ -132,19 +132,6 @@ RSpec.describe Gitlab::ImportExport::DecompressedArchiveSizeValidator, feature_c 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 aff11f7ac30..5a75631ec4d 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, feature_category: :importers do +RSpec.describe Gitlab::ImportExport::FileImporter do include ExportFileHelper let(:shared) { Gitlab::ImportExport::Shared.new(nil) } @@ -113,73 +113,28 @@ RSpec.describe Gitlab::ImportExport::FileImporter, feature_category: :importers 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, 'foo') + allow(instance).to receive(:wait_for_archived_file).and_raise(StandardError) 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 @@ -202,10 +157,8 @@ RSpec.describe Gitlab::ImportExport::FileImporter, feature_category: :importers allow(Gitlab::ImportExport::DecompressedArchiveSizeValidator).to receive(:max_bytes).and_return(1) end - it 'returns false and sets an error on shared' do - result = subject.import - - expect(result).to eq(false) + it 'returns false' do + expect(subject.import).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 1f98a9efb7d..98afe01c08b 100644 --- a/spec/lib/gitlab/import_export/json/ndjson_reader_spec.rb +++ b/spec/lib/gitlab/import_export/json/ndjson_reader_spec.rb @@ -35,22 +35,16 @@ RSpec.describe Gitlab::ImportExport::Json::NdjsonReader, feature_category: :impo expect(subject).to eq(root_tree) end - context 'when project.json is symlink or hard link' do - using RSpec::Parameterized::TableSyntax + 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')) - where(:link_method) { [:link, :symlink] } + ndjson_reader = described_class.new(tmpdir) - 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 + expect { ndjson_reader.consume_attributes(importable_path) } + .to raise_error(Gitlab::ImportExport::Error, 'Invalid file') end end end @@ -103,24 +97,18 @@ RSpec.describe Gitlab::ImportExport::Json::NdjsonReader, feature_category: :impo end end - context 'when relation file is a symlink or hard link' do - using RSpec::Parameterized::TableSyntax - - 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')) + 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')) - ndjson_reader = described_class.new(tmpdir) + 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([]) - end + expect(result.to_a).to eq([]) 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 2b9590da421..cb8ac088493 100644 --- a/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb +++ b/spec/lib/gitlab/import_export/recursive_merge_folders_spec.rb @@ -4,17 +4,15 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::RecursiveMergeFolders do describe '.merge' do - it 'merges folder and ignores symlinks and files that share hard links' do + it 'merge folder and ignore symlinks' 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 49eee772f8d..4b584a45503 100644 --- a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb +++ b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb @@ -9,10 +9,6 @@ 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 @@ -73,7 +69,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}.example.com").execute + virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").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}_/) @@ -86,7 +82,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}.example.com".downcase).execute + virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}".downcase).execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to be_nil @@ -108,7 +104,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}.example.com").execute + virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").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}_/) @@ -117,7 +113,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}.Example.com").execute + virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host.upcase}").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}_/) @@ -131,7 +127,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}.example.com").execute + virtual_domain = described_class.new("#{project.namespace.path}.#{Settings.pages.host}").execute expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.cache_key).to be_nil @@ -147,7 +143,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.example.com').execute } + subject(:virtual_domain) { described_class.new("unique-domain.#{Settings.pages.host.upcase}").execute } context 'when pages unique domain is enabled' do before_all do @@ -175,19 +171,6 @@ 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/plantuml_spec.rb b/spec/lib/gitlab/plantuml_spec.rb deleted file mode 100644 index c783dd66c48..00000000000 --- a/spec/lib/gitlab/plantuml_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - -RSpec.describe Gitlab::Plantuml, feature_category: :shared do - describe ".configure" do - subject { described_class.configure } - - let(:plantuml_url) { "http://plantuml.foo.bar" } - - before do - allow(Gitlab::CurrentSettings).to receive(:plantuml_url).and_return(plantuml_url) - end - - context "when PlantUML is enabled" do - before do - allow(Gitlab::CurrentSettings).to receive(:plantuml_enabled).and_return(true) - end - - it "configures the endpoint URL" do - expect(subject.url).to eq(plantuml_url) - end - - it "enables PNG support" do - expect(subject.png_enable).to be_truthy - end - - it "disables SVG support" do - expect(subject.svg_enable).to be_falsey - end - - it "disables TXT support" do - expect(subject.txt_enable).to be_falsey - end - end - - context "when PlantUML is disabled" do - before do - allow(Gitlab::CurrentSettings).to receive(:plantuml_enabled).and_return(false) - end - - it "configures the endpoint URL" do - expect(subject.url).to eq(plantuml_url) - end - - it "enables PNG support" do - expect(subject.png_enable).to be_falsey - end - - it "disables SVG support" do - expect(subject.svg_enable).to be_falsey - end - - it "disables TXT support" do - expect(subject.txt_enable).to be_falsey - end - end - end -end diff --git a/spec/lib/gitlab/utils/file_info_spec.rb b/spec/lib/gitlab/utils/file_info_spec.rb deleted file mode 100644 index 480036b2fd0..00000000000 --- a/spec/lib/gitlab/utils/file_info_spec.rb +++ /dev/null @@ -1,88 +0,0 @@ -# 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 c020609c26f..4b2760d7699 100644 --- a/spec/models/project_setting_spec.rb +++ b/spec/models/project_setting_spec.rb @@ -77,36 +77,12 @@ 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 6bf86558f1f..f44331521e9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -827,37 +827,6 @@ 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) @@ -4952,33 +4921,6 @@ 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/policies/ci/pipeline_schedule_policy_spec.rb b/spec/policies/ci/pipeline_schedule_policy_spec.rb index 8fc5c6ca296..7025eda1ba1 100644 --- a/spec/policies/ci/pipeline_schedule_policy_spec.rb +++ b/spec/policies/ci/pipeline_schedule_policy_spec.rb @@ -2,13 +2,10 @@ require 'spec_helper' -RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, feature_category: :continuous_integration do - using RSpec::Parameterized::TableSyntax - +RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache do let_it_be(:user) { create(:user) } - let_it_be_with_reload(:project) { create(:project, :repository, create_tag: tag_ref_name) } - let_it_be_with_reload(:pipeline_schedule) { create(:ci_pipeline_schedule, :nightly, project: project) } - let_it_be(:tag_ref_name) { "v1.0.0" } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:pipeline_schedule, reload: true) { create(:ci_pipeline_schedule, :nightly, project: project) } let(:policy) do described_class.new(user, pipeline_schedule) @@ -16,143 +13,51 @@ RSpec.describe Ci::PipelineSchedulePolicy, :models, :clean_gitlab_redis_cache, f describe 'rules' do describe 'rules for protected ref' do - context 'for branch' do - %w[refs/heads/master master].each do |branch_ref| - context "with #{branch_ref}" do - let_it_be(:branch_ref_name) { "master" } - let_it_be(:branch_pipeline_schedule) do - create(:ci_pipeline_schedule, :nightly, project: project, ref: branch_ref) - end - - where(:push_access_level, :merge_access_level, :project_role, :accessible) do - :no_one_can_push | :no_one_can_merge | :owner | :be_disallowed - :no_one_can_push | :no_one_can_merge | :maintainer | :be_disallowed - :no_one_can_push | :no_one_can_merge | :developer | :be_disallowed - :no_one_can_push | :no_one_can_merge | :reporter | :be_disallowed - :no_one_can_push | :no_one_can_merge | :guest | :be_disallowed - - :maintainers_can_push | :no_one_can_merge | :owner | :be_allowed - :maintainers_can_push | :no_one_can_merge | :maintainer | :be_allowed - :maintainers_can_push | :no_one_can_merge | :developer | :be_disallowed - :maintainers_can_push | :no_one_can_merge | :reporter | :be_disallowed - :maintainers_can_push | :no_one_can_merge | :guest | :be_disallowed - - :developers_can_push | :no_one_can_merge | :owner | :be_allowed - :developers_can_push | :no_one_can_merge | :maintainer | :be_allowed - :developers_can_push | :no_one_can_merge | :developer | :be_allowed - :developers_can_push | :no_one_can_merge | :reporter | :be_disallowed - :developers_can_push | :no_one_can_merge | :guest | :be_disallowed - - :no_one_can_push | :maintainers_can_merge | :owner | :be_allowed - :no_one_can_push | :maintainers_can_merge | :maintainer | :be_allowed - :no_one_can_push | :maintainers_can_merge | :developer | :be_disallowed - :no_one_can_push | :maintainers_can_merge | :reporter | :be_disallowed - :no_one_can_push | :maintainers_can_merge | :guest | :be_disallowed - - :maintainers_can_push | :maintainers_can_merge | :owner | :be_allowed - :maintainers_can_push | :maintainers_can_merge | :maintainer | :be_allowed - :maintainers_can_push | :maintainers_can_merge | :developer | :be_disallowed - :maintainers_can_push | :maintainers_can_merge | :reporter | :be_disallowed - :maintainers_can_push | :maintainers_can_merge | :guest | :be_disallowed - - :developers_can_push | :maintainers_can_merge | :owner | :be_allowed - :developers_can_push | :maintainers_can_merge | :maintainer | :be_allowed - :developers_can_push | :maintainers_can_merge | :developer | :be_allowed - :developers_can_push | :maintainers_can_merge | :reporter | :be_disallowed - :developers_can_push | :maintainers_can_merge | :guest | :be_disallowed - - :no_one_can_push | :developers_can_merge | :owner | :be_allowed - :no_one_can_push | :developers_can_merge | :maintainer | :be_allowed - :no_one_can_push | :developers_can_merge | :developer | :be_allowed - :no_one_can_push | :developers_can_merge | :reporter | :be_disallowed - :no_one_can_push | :developers_can_merge | :guest | :be_disallowed - - :maintainers_can_push | :developers_can_merge | :owner | :be_allowed - :maintainers_can_push | :developers_can_merge | :maintainer | :be_allowed - :maintainers_can_push | :developers_can_merge | :developer | :be_allowed - :maintainers_can_push | :developers_can_merge | :reporter | :be_disallowed - :maintainers_can_push | :developers_can_merge | :guest | :be_disallowed - - :developers_can_push | :developers_can_merge | :owner | :be_allowed - :developers_can_push | :developers_can_merge | :maintainer | :be_allowed - :developers_can_push | :developers_can_merge | :developer | :be_allowed - :developers_can_push | :developers_can_merge | :reporter | :be_disallowed - :developers_can_push | :developers_can_merge | :guest | :be_disallowed - end - - with_them do - before do - create(:protected_branch, push_access_level, merge_access_level, name: branch_ref_name, - project: project) - project.add_role(user, project_role) - end - - context 'for create_pipeline_schedule' do - subject(:policy) { described_class.new(user, new_branch_pipeline_schedule) } - - let(:new_branch_pipeline_schedule) { project.pipeline_schedules.new(ref: branch_ref) } - - it { expect(policy).to try(accessible, :create_pipeline_schedule) } - end - - context 'for play_pipeline_schedule' do - subject(:policy) { described_class.new(user, branch_pipeline_schedule) } - - it { expect(policy).to try(accessible, :play_pipeline_schedule) } - end - end - end + before do + project.add_developer(user) + end + + context 'when no one can push or merge to the branch' do + before do + create(:protected_branch, :no_one_can_push, name: pipeline_schedule.ref, project: project) + end + + it 'does not include ability to play pipeline schedule' do + expect(policy).to be_disallowed :play_pipeline_schedule end end - context 'for tag' do - %w[refs/tags/v1.0.0 v1.0.0].each do |tag_ref| - context "with #{tag_ref}" do - let_it_be(:tag_pipeline_schedule) do - create(:ci_pipeline_schedule, :nightly, project: project, ref: tag_ref) - end - - where(:access_level, :project_role, :accessible) do - :no_one_can_create | :owner | :be_disallowed - :no_one_can_create | :maintainer | :be_disallowed - :no_one_can_create | :developer | :be_disallowed - :no_one_can_create | :reporter | :be_disallowed - :no_one_can_create | :guest | :be_disallowed - - :maintainers_can_create | :owner | :be_allowed - :maintainers_can_create | :maintainer | :be_allowed - :maintainers_can_create | :developer | :be_disallowed - :maintainers_can_create | :reporter | :be_disallowed - :maintainers_can_create | :guest | :be_disallowed - - :developers_can_create | :owner | :be_allowed - :developers_can_create | :maintainer | :be_allowed - :developers_can_create | :developer | :be_allowed - :developers_can_create | :reporter | :be_disallowed - :developers_can_create | :guest | :be_disallowed - end - - with_them do - before do - create(:protected_tag, access_level, name: tag_ref_name, project: project) - project.add_role(user, project_role) - end - - context 'for create_pipeline_schedule' do - subject(:policy) { described_class.new(user, new_tag_pipeline_schedule) } - - let(:new_tag_pipeline_schedule) { project.pipeline_schedules.new(ref: tag_ref) } - - it { expect(policy).to try(accessible, :create_pipeline_schedule) } - end - - context 'for play_pipeline_schedule' do - subject(:policy) { described_class.new(user, tag_pipeline_schedule) } - - it { expect(policy).to try(accessible, :play_pipeline_schedule) } - end - end - end + context 'when developers can push to the branch' do + before do + create(:protected_branch, :developers_can_merge, name: pipeline_schedule.ref, project: project) + end + + it 'includes ability to update pipeline' do + expect(policy).to be_allowed :play_pipeline_schedule + end + end + + context 'when no one can create the tag' do + let(:tag) { 'v1.0.0' } + + before do + pipeline_schedule.update!(ref: tag) + + create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project) + end + + it 'does not include ability to play pipeline schedule' do + expect(policy).to be_disallowed :play_pipeline_schedule + end + end + + context 'when no one can create the tag but it is not a tag' do + before do + create(:protected_tag, :no_one_can_create, name: pipeline_schedule.ref, project: project) + end + + it 'includes ability to play pipeline schedule' do + expect(policy).to be_allowed :play_pipeline_schedule end end end diff --git a/spec/services/bulk_imports/archive_extraction_service_spec.rb b/spec/services/bulk_imports/archive_extraction_service_spec.rb index f76ac722ad7..5593218c259 100644 --- a/spec/services/bulk_imports/archive_extraction_service_spec.rb +++ b/spec/services/bulk_imports/archive_extraction_service_spec.rb @@ -43,21 +43,13 @@ RSpec.describe BulkImports::ArchiveExtractionService, feature_category: :importe context 'when archive file is a symlink' do it 'raises an error' do - FileUtils.ln_s(filepath, File.join(tmpdir, 'symlink')) + FileUtils.ln_s(File.join(tmpdir, filename), 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 e6d919c3499..9d80ab3cd8f 100644 --- a/spec/services/bulk_imports/file_decompression_service_spec.rb +++ b/spec/services/bulk_imports/file_decompression_service_spec.rb @@ -3,8 +3,6 @@ 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) } @@ -72,68 +70,39 @@ RSpec.describe BulkImports::FileDecompressionService, feature_category: :importe end end - 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') } + context 'when compressed file is a symlink' do + let_it_be(:symlink) { File.join(tmpdir, 'symlink.gz') } before do - FileUtils.send(link_method, File.join(tmpdir, gz_filename), file) + FileUtils.ln_s(File.join(tmpdir, gz_filename), symlink) end - end - - shared_context 'when decompressed file' do - let_it_be(:file) { File.join(tmpdir, 'file.txt') } - subject { described_class.new(tmpdir: tmpdir, filename: gz_filename) } + subject { described_class.new(tmpdir: tmpdir, filename: 'symlink.gz') } - before do - original_file = File.join(tmpdir, 'original_file.txt') - FileUtils.touch(original_file) - FileUtils.send(link_method, original_file, file) + it 'raises an error and removes the file' do + expect { subject.execute } + .to raise_error(BulkImports::FileDecompressionService::ServiceError, 'File decompression error') - subject.instance_variable_set(:@decompressed_filepath, file) + expect(File.exist?(symlink)).to eq(false) 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(:link_method) { :symlink } + let_it_be(:symlink) { File.join(tmpdir, 'symlink') } - include_context 'when decompressed file' + before do + FileUtils.ln_s(File.join(tmpdir, ndjson_filename), symlink) - include_examples 'raises an error and removes the file', error_message: 'Invalid file' - end + subject.instance_variable_set(:@decompressed_filepath, symlink) + end - context 'when decompressed file shares multiple hard links' do - let(:link_method) { :link } + subject { described_class.new(tmpdir: tmpdir, filename: gz_filename) } - include_context 'when decompressed file' + it 'raises an error and removes the file' do + expect { subject.execute }.to raise_error(described_class::ServiceError, 'Invalid file') - include_examples 'raises an error and removes the file', error_message: 'Invalid file' + expect(File.exist?(symlink)).to eq(false) + end 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 c035eabf767..cbeea5b0f46 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.mktmpdir } + let_it_be(:tmpdir) { Dir.tmpdir } let_it_be(:filepath) { File.join(tmpdir, filename) } let_it_be(:content_length) { 1000 } @@ -247,36 +247,6 @@ 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( diff --git a/spec/services/ci/pipeline_schedules/update_service_spec.rb b/spec/services/ci/pipeline_schedules/update_service_spec.rb index 63c0c996a1a..838f49f6dea 100644 --- a/spec/services/ci/pipeline_schedules/update_service_spec.rb +++ b/spec/services/ci/pipeline_schedules/update_service_spec.rb @@ -6,9 +6,7 @@ RSpec.describe Ci::PipelineSchedules::UpdateService, feature_category: :continuo let_it_be(:user) { create(:user) } let_it_be(:reporter) { create(:user) } let_it_be(:project) { create(:project, :public, :repository) } - let_it_be(:pipeline_schedule) do - create(:ci_pipeline_schedule, project: project, owner: user, ref: 'master') - end + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } before_all do project.add_maintainer(user) diff --git a/yarn.lock b/yarn.lock index f940bb572bf..83cb3e69579 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1146,10 +1146,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.7.3.tgz#9ea641146436da388ffbad25d7f2abe0df52c235" integrity sha512-NMV++7Ew1FSBDN1xiZaauU9tfeSfgDHcOLpn+8bGpP+O5orUPm2Eu66R5eC5gkjBPaXosNAxNWtriee+aFk4+g== -"@gitlab/web-ide@0.0.1-dev-20230713160749-patch-1": - version "0.0.1-dev-20230713160749-patch-1" - resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230713160749-patch-1.tgz#6420b55aae444533f9a4bd6269503d98a72aaa2e" - integrity sha512-Dh8XQyPwDY6fkd/A+hTHCqrD23u5qnlaxKu5myyxDEgBNGgu4SGblFU9B6NHNm8eGUZk6Cs5MuMk+NUvWRKbmA== +"@gitlab/web-ide@0.0.1-dev-20230614124516": + version "0.0.1-dev-20230614124516" + resolved "https://registry.yarnpkg.com/@gitlab/web-ide/-/web-ide-0.0.1-dev-20230614124516.tgz#87ef511112e954dc3614e28fdf635b140bab22b7" + integrity sha512-VRKiDzhRNIHsyKbln8YtQjVvsOkF+4N1OUqaldu2gYX4Nfw9g57sTwnnX44aYcHFgPVgo5a/ScRpnCiBl6Phxg== "@graphql-eslint/eslint-plugin@3.19.1": version "3.19.1" |