diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-28 13:31:50 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-28 13:31:50 +0000 |
commit | c70fc855befaae4fb88bec9016421ad98db5b182 (patch) | |
tree | 4fe714c47298a6f8bc338a08d4513be42f399b63 | |
parent | c3e911be175c0aabfea1eb030f9e0ef23f5f3887 (diff) | |
download | gitlab-ce-c70fc855befaae4fb88bec9016421ad98db5b182.tar.gz |
Add latest changes from gitlab-org/security/gitlab@12-6-stable-ee
54 files changed, 742 insertions, 140 deletions
@@ -65,7 +65,7 @@ gem 'u2f', '~> 0.2.1' # GitLab Pages gem 'validates_hostname', '~> 1.0.6' -gem 'rubyzip', '~> 1.3.0', require: 'zip' +gem 'rubyzip', '~> 2.0.0', require: 'zip' # GitLab Pages letsencrypt support gem 'acme-client', '~> 2.0.2' @@ -142,7 +142,7 @@ gem 'gitlab-markup', '~> 1.7.0' gem 'github-markup', '~> 1.7.0', require: 'github/markup' gem 'commonmarker', '~> 0.20' gem 'RedCloth', '~> 4.3.2' -gem 'rdoc', '~> 6.0' +gem 'rdoc', '~> 6.1.2' gem 'org-ruby', '~> 0.9.12' gem 'creole', '~> 0.5.0' gem 'wikicloth', '0.8.1' diff --git a/Gemfile.lock b/Gemfile.lock index 57e428ca955..f510788f1f3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -260,7 +260,7 @@ GEM et-orbi (1.2.1) tzinfo eventmachine (1.2.7) - excon (0.62.0) + excon (0.71.1) execjs (2.6.0) expression_parser (0.9.0) extended-markdown-filter (0.6.0) @@ -763,7 +763,8 @@ GEM rack (>= 0.4) rack-attack (6.2.0) rack (>= 1.0, < 3) - rack-cors (1.0.2) + rack-cors (1.0.6) + rack (>= 1.6.0) rack-oauth2 (1.9.3) activesupport attr_required @@ -820,7 +821,7 @@ GEM ffi (>= 1.0.6) msgpack (>= 0.4.3) optimist (>= 3.0.0) - rdoc (6.0.4) + rdoc (6.1.2) re2 (1.1.1) recaptcha (4.13.1) json @@ -929,7 +930,7 @@ GEM sexp_processor (~> 4.9) rubyntlm (0.6.2) rubypants (0.2.0) - rubyzip (1.3.0) + rubyzip (2.0.0) rugged (0.28.4.1) safe_yaml (1.0.4) sanitize (4.6.6) @@ -1299,7 +1300,7 @@ DEPENDENCIES raindrops (~> 0.18) rblineprof (~> 0.3.6) rbtrace (~> 0.4) - rdoc (~> 6.0) + rdoc (~> 6.1.2) re2 (~> 1.1.1) recaptcha (~> 4.11) redis (~> 4.0) @@ -1323,7 +1324,7 @@ DEPENDENCIES ruby-prof (~> 1.0.0) ruby-progressbar ruby_parser (~> 3.8) - rubyzip (~> 1.3.0) + rubyzip (~> 2.0.0) rugged (~> 0.28) sanitize (~> 4.6) sassc-rails (~> 2.1.0) diff --git a/app/assets/javascripts/groups_select.js b/app/assets/javascripts/groups_select.js index a5e38022b8d..4daa8c60e58 100644 --- a/app/assets/javascripts/groups_select.js +++ b/app/assets/javascripts/groups_select.js @@ -1,6 +1,7 @@ import $ from 'jquery'; import axios from './lib/utils/axios_utils'; import Api from './api'; +import { escape } from 'lodash'; import { normalizeHeaders } from './lib/utils/common_utils'; import { __ } from '~/locale'; @@ -75,10 +76,12 @@ const groupsSelect = () => { } }, formatResult(object) { - return `<div class='group-result'> <div class='group-name'>${object.full_name}</div> <div class='group-path'>${object.full_path}</div> </div>`; + return `<div class='group-result'> <div class='group-name'>${escape( + object.full_name, + )}</div> <div class='group-path'>${object.full_path}</div> </div>`; }, formatSelection(object) { - return object.full_name; + return escape(object.full_name); }, dropdownCssClass: 'ajax-groups-dropdown select2-infinite', // we do not want to escape markup since we are displaying html in results diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d683faf6a20..db7b858125a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -368,8 +368,8 @@ module ProjectsHelper @project.grafana_integration&.grafana_url end - def grafana_integration_token - @project.grafana_integration&.token + def grafana_integration_masked_token + @project.grafana_integration&.masked_token end def grafana_integration_enabled? diff --git a/app/models/generic_commit_status.rb b/app/models/generic_commit_status.rb index 8a768b3a2c0..6c8bfc35334 100644 --- a/app/models/generic_commit_status.rb +++ b/app/models/generic_commit_status.rb @@ -1,11 +1,14 @@ # frozen_string_literal: true class GenericCommitStatus < CommitStatus + EXTERNAL_STAGE_IDX = 1_000_000 + before_validation :set_default_values validates :target_url, addressable_url: true, length: { maximum: 255 }, allow_nil: true + validate :name_uniqueness_across_types, unless: :importing? # GitHub compatible API alias_attribute :context, :name @@ -13,7 +16,7 @@ class GenericCommitStatus < CommitStatus def set_default_values self.context ||= 'default' self.stage ||= 'external' - self.stage_idx ||= 1000000 + self.stage_idx ||= EXTERNAL_STAGE_IDX end def tags @@ -25,4 +28,14 @@ class GenericCommitStatus < CommitStatus .new(self, current_user) .fabricate! end + + private + + def name_uniqueness_across_types + return if !pipeline || name.blank? + + if pipeline.statuses.by_name(name).where.not(type: type).exists? + errors.add(:name, :taken) + end + end end diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb index ed4c279965a..00213732fee 100644 --- a/app/models/grafana_integration.rb +++ b/app/models/grafana_integration.rb @@ -8,11 +8,13 @@ class GrafanaIntegration < ApplicationRecord algorithm: 'aes-256-gcm', key: Settings.attr_encrypted_db_key_base_32 + before_validation :check_token_changes + validates :grafana_url, length: { maximum: 1024 }, addressable_url: { enforce_sanitization: true, ascii_only: true } - validates :token, :project, presence: true + validates :encrypted_token, :project, presence: true validates :enabled, inclusion: { in: [true, false] } @@ -23,4 +25,28 @@ class GrafanaIntegration < ApplicationRecord @client ||= ::Grafana::Client.new(api_url: grafana_url.chomp('/'), token: token) end + + def masked_token + mask(encrypted_token) + end + + def masked_token_was + mask(encrypted_token_was) + end + + private + + def token + decrypt(:token, encrypted_token) + end + + def check_token_changes + return unless [encrypted_token_was, masked_token_was].include?(token) + + clear_attribute_changes [:token, :encrypted_token, :encrypted_token_iv] + end + + def mask(token) + token&.squish&.gsub(/./, '*') + end end diff --git a/app/models/note.rb b/app/models/note.rb index cfa7ba98081..63b1fb121f5 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -551,7 +551,8 @@ class Note < ApplicationRecord # if they are not equal, then there are private/confidential references as well user_visible_reference_count > 0 && user_visible_reference_count == total_reference_count else - referenced_mentionables(user).any? + refs = all_references(user) + refs.all.any? && refs.stateful_not_visible_counter == 0 end end diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 3f0aedfbfb2..569b91de73e 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -18,7 +18,7 @@ class CompareService return unless raw_compare && raw_compare.base && raw_compare.head Compare.new(raw_compare, - target_project, + start_project, base_sha: base_sha, straight: straight) end diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb index c96dcaae8d5..ea7d05551fd 100644 --- a/app/services/projects/group_links/destroy_service.rb +++ b/app/services/projects/group_links/destroy_service.rb @@ -6,6 +6,12 @@ module Projects def execute(group_link) return false unless group_link + if group_link.project.private? + TodosDestroyer::ProjectPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, project.id) + else + TodosDestroyer::ConfidentialIssueWorker.perform_in(Todo::WAIT_FOR_DELETE, nil, project.id) + end + group_link.destroy end end diff --git a/app/views/projects/settings/operations/_grafana_integration.html.haml b/app/views/projects/settings/operations/_grafana_integration.html.haml index cd5b5abd9ce..69e42a6c4fb 100644 --- a/app/views/projects/settings/operations/_grafana_integration.html.haml +++ b/app/views/projects/settings/operations/_grafana_integration.html.haml @@ -1,2 +1,2 @@ .js-grafana-integration{ data: { operations_settings_endpoint: project_settings_operations_path(@project), - grafana_integration: { url: grafana_integration_url, token: grafana_integration_token, enabled: grafana_integration_enabled?.to_s } } } + grafana_integration: { url: grafana_integration_url, token: grafana_integration_masked_token, enabled: grafana_integration_enabled?.to_s } } } diff --git a/changelogs/unreleased/jl-bump-rack-cors-1-0-6.yml b/changelogs/unreleased/jl-bump-rack-cors-1-0-6.yml new file mode 100644 index 00000000000..d54a7d885d1 --- /dev/null +++ b/changelogs/unreleased/jl-bump-rack-cors-1-0-6.yml @@ -0,0 +1,5 @@ +--- +title: Update rack-cors to 1.0.6 +merge_request: +author: +type: security diff --git a/changelogs/unreleased/jl-bump-rdoc-6-1-2.yml b/changelogs/unreleased/jl-bump-rdoc-6-1-2.yml new file mode 100644 index 00000000000..69c37e121a5 --- /dev/null +++ b/changelogs/unreleased/jl-bump-rdoc-6-1-2.yml @@ -0,0 +1,5 @@ +--- +title: Update rdoc to 6.1.2 +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-13-update-ruby-zip-pages-master.yml b/changelogs/unreleased/security-13-update-ruby-zip-pages-master.yml new file mode 100644 index 00000000000..976ce6f90b3 --- /dev/null +++ b/changelogs/unreleased/security-13-update-ruby-zip-pages-master.yml @@ -0,0 +1,5 @@ +--- +title: Bump rubyzip to 2.0.0 +merge_request: +author: Utkarsh Gupta +type: security diff --git a/changelogs/unreleased/security-35235-todos-cleanup.yml b/changelogs/unreleased/security-35235-todos-cleanup.yml new file mode 100644 index 00000000000..119220fbc73 --- /dev/null +++ b/changelogs/unreleased/security-35235-todos-cleanup.yml @@ -0,0 +1,5 @@ +--- +title: Cleanup todos for users from a removed linked group +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-dos-via-asciidoc-includes.yml b/changelogs/unreleased/security-dos-via-asciidoc-includes.yml new file mode 100644 index 00000000000..8fc3bd32316 --- /dev/null +++ b/changelogs/unreleased/security-dos-via-asciidoc-includes.yml @@ -0,0 +1,5 @@ +--- +title: Limit number of AsciiDoc includes per document +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-fix-grafana-token-leaked-in-plain-to-other-maintainers.yml b/changelogs/unreleased/security-fix-grafana-token-leaked-in-plain-to-other-maintainers.yml new file mode 100644 index 00000000000..a44005f8dac --- /dev/null +++ b/changelogs/unreleased/security-fix-grafana-token-leaked-in-plain-to-other-maintainers.yml @@ -0,0 +1,5 @@ +--- +title: Prevent gafana integration token from being displayed as a plain text to other project maintainers, by only displaying a masked version of it. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-fix-xss-on-project-templates.yml b/changelogs/unreleased/security-fix-xss-on-project-templates.yml new file mode 100644 index 00000000000..2930bbaff87 --- /dev/null +++ b/changelogs/unreleased/security-fix-xss-on-project-templates.yml @@ -0,0 +1,5 @@ +--- +title: Fix XSS vulnerability on custom project templates form +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml b/changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml new file mode 100644 index 00000000000..b540172d95c --- /dev/null +++ b/changelogs/unreleased/security-proctect-internal-builds-from-external-overrides.yml @@ -0,0 +1,5 @@ +--- +title: Protect internal CI builds from external overrides +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-reference-check.yml b/changelogs/unreleased/security-reference-check.yml new file mode 100644 index 00000000000..f33cea66eb1 --- /dev/null +++ b/changelogs/unreleased/security-reference-check.yml @@ -0,0 +1,5 @@ +--- +title: Make sure that only system notes where all references are visible to user are exposed in GraphQL API. +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml b/changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml new file mode 100644 index 00000000000..db6a4f064a4 --- /dev/null +++ b/changelogs/unreleased/security-reverse-polarity-of-branch-compare.yml @@ -0,0 +1,5 @@ +--- +title: Make cross-repository comparisons happen in the source repository +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-update-excon-cve-2019-16779.yml b/changelogs/unreleased/security-update-excon-cve-2019-16779.yml new file mode 100644 index 00000000000..e849dc92848 --- /dev/null +++ b/changelogs/unreleased/security-update-excon-cve-2019-16779.yml @@ -0,0 +1,5 @@ +--- +title: Update excon to 0.71.1 to fix CVE-2019-16779 +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-workhorse-package-bypass-12-6.yml b/changelogs/unreleased/security-workhorse-package-bypass-12-6.yml new file mode 100644 index 00000000000..bb9aa0a2bf1 --- /dev/null +++ b/changelogs/unreleased/security-workhorse-package-bypass-12-6.yml @@ -0,0 +1,5 @@ +--- +title: Add workhorse request verification to package upload endpoints +merge_request: +author: +type: security diff --git a/doc/user/asciidoc.md b/doc/user/asciidoc.md index b4d3cb58e97..da6bf287955 100644 --- a/doc/user/asciidoc.md +++ b/doc/user/asciidoc.md @@ -221,6 +221,11 @@ include::basics.adoc[] include::https://example.org/installation.adoc[] ``` +To guarantee good system performance and prevent malicious documents causing +problems, GitLab enforces a **maximum limit** on the number of include directives +processed in any one document. Currently a total of 32 documents can be +included, a number that is inclusive of transitive dependencies. + ### Blocks ```asciidoc diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index d108c811f4b..e0a6dc41b65 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -85,6 +85,8 @@ module API protected: @project.protected_for?(ref)) end + authorize! :update_pipeline, pipeline + status = GenericCommitStatus.running_or_pending.find_or_initialize_by( project: @project, pipeline: pipeline, diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 37cb6d6a639..7ae19ab514b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -256,11 +256,21 @@ module API end def require_gitlab_workhorse! + verify_workhorse_api! + unless env['HTTP_GITLAB_WORKHORSE'].present? forbidden!('Request should be executed via GitLab Workhorse') end end + def verify_workhorse_api! + Gitlab::Workhorse.verify_api_request!(request.headers) + rescue => e + Gitlab::ErrorTracking.track_exception(e) + + forbidden! + end + def require_pages_enabled! not_found! unless user_project.pages_available? end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb index 8419769085a..ffc6bdc612d 100644 --- a/lib/banzai/reference_parser/base_parser.rb +++ b/lib/banzai/reference_parser/base_parser.rb @@ -201,12 +201,14 @@ module Banzai gather_references(nodes) end - # Gathers the references for the given HTML nodes. + # Gathers the references for the given HTML nodes. Returns visible + # references and a list of nodes which are not visible to the user def gather_references(nodes) nodes = nodes_user_can_reference(current_user, nodes) - nodes = nodes_visible_to_user(current_user, nodes) + visible = nodes_visible_to_user(current_user, nodes) + not_visible = nodes - visible - referenced_by(nodes) + { visible: referenced_by(visible), not_visible: not_visible } end # Returns a Hash containing the projects for a given list of HTML nodes. diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index da65caa6c9c..8d072422e17 100644 --- a/lib/gitlab/asciidoc.rb +++ b/lib/gitlab/asciidoc.rb @@ -11,6 +11,7 @@ module Gitlab # the resulting HTML through HTML pipeline filters. module Asciidoc MAX_INCLUDE_DEPTH = 5 + MAX_INCLUDES = 32 DEFAULT_ADOC_ATTRS = { 'showtitle' => true, 'sectanchors' => true, @@ -40,6 +41,7 @@ module Gitlab extensions: extensions } context[:pipeline] = :ascii_doc + context[:max_includes] = [MAX_INCLUDES, context[:max_includes]].compact.min plantuml_setup diff --git a/lib/gitlab/asciidoc/include_processor.rb b/lib/gitlab/asciidoc/include_processor.rb index c6fbf540e9c..c3dca21b4b5 100644 --- a/lib/gitlab/asciidoc/include_processor.rb +++ b/lib/gitlab/asciidoc/include_processor.rb @@ -13,7 +13,9 @@ module Gitlab super(logger: Gitlab::AppLogger) @context = context - @repository = context[:project].try(:repository) + @repository = context[:repository] || context[:project].try(:repository) + @max_includes = context[:max_includes].to_i + @included = [] # Note: Asciidoctor calls #freeze on extensions, so we can't set new # instance variables after initialization. @@ -28,8 +30,11 @@ module Gitlab def include_allowed?(target, reader) doc = reader.document - return false if doc.attributes.fetch('max-include-depth').to_i < 1 + max_include_depth = doc.attributes.fetch('max-include-depth').to_i + + return false if max_include_depth < 1 return false if target_uri?(target) + return false if included.size >= max_includes true end @@ -62,7 +67,7 @@ module Gitlab private - attr_accessor :context, :repository, :cache + attr_reader :context, :repository, :cache, :max_includes, :included # Gets a Blob at a path for a specific revision. # This method will check that the Blob exists and contains readable text. @@ -77,6 +82,8 @@ module Gitlab raise 'Blob not found' unless blob raise 'File is not readable' unless blob.readable_text? + included << filename + blob end diff --git a/lib/gitlab/git/cross_repo_comparer.rb b/lib/gitlab/git/cross_repo_comparer.rb new file mode 100644 index 00000000000..3958373f7cb --- /dev/null +++ b/lib/gitlab/git/cross_repo_comparer.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class CrossRepoComparer + attr_reader :source_repo, :target_repo + + def initialize(source_repo, target_repo) + @source_repo = source_repo + @target_repo = target_repo + end + + def compare(source_ref, target_ref, straight:) + ensuring_ref_in_source(target_ref) do |target_commit_id| + Gitlab::Git::Compare.new( + source_repo, + target_commit_id, + source_ref, + straight: straight + ) + end + end + + private + + def ensuring_ref_in_source(ref, &blk) + return yield ref if source_repo == target_repo + + # If the commit doesn't exist in the target, there's nothing we can do + commit_id = target_repo.commit(ref)&.sha + return unless commit_id + + # The commit pointed to by ref may exist in the source even when they + # are different repositories. This is particularly true of close forks, + # but may also be the case if a temporary ref for this comparison has + # already been created in the past, and the result hasn't been GC'd yet. + return yield commit_id if source_repo.commit(commit_id) + + # Worst case: the commit is not in the source repo so we need to fetch + # it. Use a temporary ref and clean up afterwards + with_commit_in_source_tmp(commit_id, &blk) + end + + # Fetch the ref into source_repo from target_repo, using a temporary ref + # name that will be deleted once the method completes. This is a no-op if + # fetching the source branch fails + def with_commit_in_source_tmp(commit_id, &blk) + tmp_ref = "refs/tmp/#{SecureRandom.hex}" + + yield commit_id if source_repo.fetch_source_branch!(target_repo, commit_id, tmp_ref) + ensure + source_repo.delete_refs(tmp_ref) # best-effort + end + end + end +end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 4971a18e270..ea44a2f40c0 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -746,29 +746,9 @@ module Gitlab end def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) - reachable_ref = - if source_repository == self - source_branch_name - else - # If a tmp ref was created before for a separate repo comparison (forks), - # we're able to short-circuit the tmp ref re-creation: - # 1. Take the SHA from the source repo - # 2. Read that in the current "target" repo - # 3. If that SHA is still known (readable), it means GC hasn't - # cleaned it up yet, so we can use it instead re-writing the tmp ref. - source_commit_id = source_repository.commit(source_branch_name)&.sha - commit(source_commit_id)&.sha if source_commit_id - end - - return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref - - tmp_ref = "refs/tmp/#{SecureRandom.hex}" - - return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref) - - compare(target_branch_name, tmp_ref, straight: straight) - ensure - delete_refs(tmp_ref) if tmp_ref + CrossRepoComparer + .new(source_repository, self) + .compare(source_branch_name, target_branch_name, straight: straight) end def write_ref(ref_path, ref, old_ref: nil) @@ -1045,13 +1025,6 @@ module Gitlab private - def compare(base_ref, head_ref, straight:) - Gitlab::Git::Compare.new(self, - base_ref, - head_ref, - straight: straight) - end - def empty_diff_stats Gitlab::Git::DiffStatsCollection.new([]) end diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index f095ac9ffd1..519eb49658a 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -6,11 +6,16 @@ module Gitlab REFERABLES = %i(user issue label milestone mentioned_user mentioned_group mentioned_project merge_request snippet commit commit_range directly_addressed_user epic).freeze attr_accessor :project, :current_user, :author + # This counter is increased by a number of references filtered out by + # banzai reference exctractor. Note that this counter is stateful and + # not idempotent and is increased whenever you call `references`. + attr_reader :stateful_not_visible_counter def initialize(project, current_user = nil) @project = project @current_user = current_user @references = {} + @stateful_not_visible_counter = 0 super() end @@ -20,11 +25,15 @@ module Gitlab end def references(type) - super(type, project, current_user) + refs = super(type, project, current_user) + @stateful_not_visible_counter += refs[:not_visible].count + + refs[:visible] end def reset_memoized_values @references = {} + @stateful_not_visible_counter = 0 super() end diff --git a/package.json b/package.json index a4b6f55365e..4b11cbfe6e1 100644 --- a/package.json +++ b/package.json @@ -93,6 +93,7 @@ "jszip": "^3.1.3", "jszip-utils": "^0.0.2", "katex": "^0.10.0", + "lodash": "^4.17.15", "marked": "^0.3.12", "mermaid": "^8.4.2", "monaco-editor": "^0.15.6", diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 46228d0d1c2..1afdccac34d 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -935,14 +935,14 @@ describe ProjectsHelper do helper.instance_variable_set(:@project, project) end - subject { helper.grafana_integration_token } + subject { helper.grafana_integration_masked_token } it { is_expected.to eq(nil) } context 'grafana integration exists' do let!(:grafana_integration) { create(:grafana_integration, project: project) } - it { is_expected.to eq(grafana_integration.token) } + it { is_expected.to eq(grafana_integration.masked_token) } end end diff --git a/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb b/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb index 30b99f3eda7..8346ba93f88 100644 --- a/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/mentioned_group_parser_spec.rb @@ -19,7 +19,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do it 'returns empty array' do link['data-group'] = project.group.id.to_s - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end @@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do end it 'returns groups' do - expect(subject.gather_references([link])).to eq([group]) + expect_gathered_references(subject.gather_references([link]), [group], 0) end end @@ -38,7 +38,7 @@ describe Banzai::ReferenceParser::MentionedGroupParser do it 'returns an empty Array' do link['data-group'] = 'test-non-existing' - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end end diff --git a/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb b/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb index 154f7c4dc36..b99c02351d0 100644 --- a/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/mentioned_project_parser_spec.rb @@ -19,7 +19,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do it 'returns empty Array' do link['data-project'] = project.id.to_s - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end @@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do end it 'returns an Array of referenced projects' do - expect(subject.gather_references([link])).to eq([project]) + expect_gathered_references(subject.gather_references([link]), [project], 0) end end @@ -38,7 +38,7 @@ describe Banzai::ReferenceParser::MentionedProjectParser do it 'returns an empty Array' do link['data-project'] = 'inexisting-project-id' - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end end diff --git a/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb b/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb index 1be279375bd..b10e5d19828 100644 --- a/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/mentioned_user_parser_spec.rb @@ -22,7 +22,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do end it 'returns empty list of users' do - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 0) end end end @@ -35,7 +35,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do end it 'returns empty list of users' do - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 0) end end end @@ -44,7 +44,7 @@ describe Banzai::ReferenceParser::MentionedUserParser do it 'returns an Array of users' do link['data-user'] = user.id.to_s - expect(subject.referenced_by([link])).to eq([user]) + expect_gathered_references(subject.gather_references([link]), [user], 0) end end end diff --git a/spec/lib/banzai/reference_parser/project_parser_spec.rb b/spec/lib/banzai/reference_parser/project_parser_spec.rb index 356dde1e9c2..e87fa3e8767 100644 --- a/spec/lib/banzai/reference_parser/project_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/project_parser_spec.rb @@ -17,7 +17,7 @@ describe Banzai::ReferenceParser::ProjectParser do it 'returns an Array of projects' do link['data-project'] = project.id.to_s - expect(subject.gather_references([link])).to eq([project]) + expect_gathered_references(subject.gather_references([link]), [project], 0) end end @@ -25,7 +25,7 @@ describe Banzai::ReferenceParser::ProjectParser do it 'returns an empty Array' do link['data-project'] = '' - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end end @@ -35,7 +35,7 @@ describe Banzai::ReferenceParser::ProjectParser do link['data-project'] = private_project.id.to_s - expect(subject.gather_references([link])).to eq([]) + expect_gathered_references(subject.gather_references([link]), [], 1) end it 'returns an Array when authorized' do @@ -43,7 +43,7 @@ describe Banzai::ReferenceParser::ProjectParser do link['data-project'] = private_project.id.to_s - expect(subject.gather_references([link])).to eq([private_project]) + expect_gathered_references(subject.gather_references([link]), [private_project], 0) end end end diff --git a/spec/lib/gitlab/asciidoc/include_processor_spec.rb b/spec/lib/gitlab/asciidoc/include_processor_spec.rb new file mode 100644 index 00000000000..5fec4d9e208 --- /dev/null +++ b/spec/lib/gitlab/asciidoc/include_processor_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'nokogiri' + +describe Gitlab::Asciidoc::IncludeProcessor do + let_it_be(:project) { create(:project, :repository) } + + let(:processor_context) do + { + project: project, + max_includes: max_includes, + ref: ref + } + end + let(:ref) { project.repository.root_ref } + let(:max_includes) { 10 } + + let(:reader) { Asciidoctor::PreprocessorReader.new(document, lines, 'file.adoc') } + let(:document) { Asciidoctor::Document.new(lines) } + + subject(:processor) { described_class.new(processor_context) } + + let(:a_blob) { double(:Blob, readable_text?: true, data: a_data) } + let(:a_data) { StringIO.new('include::b.adoc[]') } + + let(:lines) { [':max-include-depth: 1000'] + Array.new(10, 'include::a.adoc[]') } + + before do + allow(project.repository).to receive(:blob_at).with(ref, 'a.adoc').and_return(a_blob) + end + + describe '#include_allowed?' do + it 'allows the first include' do + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_truthy + end + + it 'disallows the Nth + 1 include' do + max_includes.times { processor.send(:read_blob, ref, 'a.adoc') } + + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index 38ec04ebe81..e962951e6d4 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/spec/lib/gitlab/asciidoc_spec.rb @@ -425,6 +425,24 @@ module Gitlab create_file(current_file, "= AsciiDoc\n") end + def many_includes(target) + Array.new(10, "include::#{target}[]").join("\n") + end + + context 'cyclic imports' do + before do + create_file('doc/api/a.adoc', many_includes('b.adoc')) + create_file('doc/api/b.adoc', many_includes('a.adoc')) + end + + let(:include_path) { 'a.adoc' } + let(:requested_path) { 'doc/api/README.md' } + + it 'completes successfully' do + is_expected.to include('<p>Include this:</p>') + end + end + context 'with path to non-existing file' do let(:include_path) { 'not-exists.adoc' } diff --git a/spec/lib/gitlab/git/cross_repo_comparer_spec.rb b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb new file mode 100644 index 00000000000..8b37b6d1667 --- /dev/null +++ b/spec/lib/gitlab/git/cross_repo_comparer_spec.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::CrossRepoComparer do + let(:source_project) { create(:project, :repository) } + let(:target_project) { create(:project, :repository) } + + let(:source_repo) { source_project.repository.raw_repository } + let(:target_repo) { target_project.repository.raw_repository } + + let(:source_branch) { 'feature' } + let(:target_branch) { 'master' } + let(:straight) { false } + + let(:source_commit) { source_repo.commit(source_branch) } + let(:target_commit) { source_repo.commit(target_branch) } + + subject(:result) { described_class.new(source_repo, target_repo).compare(source_branch, target_branch, straight: straight) } + + describe '#compare' do + context 'within a single repository' do + let(:target_project) { source_project } + + context 'a non-straight comparison' do + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + + expect_compare(result, from: source_commit, to: target_commit) + expect(result.straight).to eq(false) + end + end + + context 'a straight comparison' do + let(:straight) { true } + + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + + expect_compare(result, from: source_commit, to: target_commit) + expect(result.straight).to eq(true) + end + end + end + + context 'across two repositories' do + context 'target ref exists in source repo' do + it 'compares without fetching from another repo' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + expect_compare(result, from: source_commit, to: target_commit) + end + end + + context 'target ref does not exist in source repo' do + it 'compares in the source repo by fetching from the target to a temporary ref' do + new_commit_id = create_commit(target_project.owner, target_repo, target_branch) + new_commit = target_repo.commit(new_commit_id) + + # This is how the temporary ref is generated + expect(SecureRandom).to receive(:hex).at_least(:once).and_return('foo') + + expect(source_repo) + .to receive(:fetch_source_branch!) + .with(target_repo, new_commit_id, 'refs/tmp/foo') + .and_call_original + + expect(source_repo).to receive(:delete_refs).with('refs/tmp/foo').and_call_original + + expect_compare(result, from: source_commit, to: new_commit) + end + end + + context 'source ref does not exist in source repo' do + let(:source_branch) { 'does-not-exist' } + + it 'returns an empty comparison' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + expect(result).to be_a(::Gitlab::Git::Compare) + expect(result.commits.size).to eq(0) + end + end + + context 'target ref does not exist in target repo' do + let(:target_branch) { 'does-not-exist' } + + it 'returns nil' do + expect(source_repo).not_to receive(:fetch_source_branch!) + expect(source_repo).not_to receive(:delete_refs) + + is_expected.to be_nil + end + end + end + end + + def expect_compare(of, from:, to:) + expect(of).to be_a(::Gitlab::Git::Compare) + expect(from).to be_a(::Gitlab::Git::Commit) + expect(to).to be_a(::Gitlab::Git::Commit) + + expect(of.commits).not_to be_empty + expect(of.head).to eq(from) + expect(of.base).to eq(to) + end + + def create_commit(user, repo, branch) + action = { action: :create, file_path: '/FILE', content: 'content' } + + result = repo.multi_action(user, branch_name: branch, message: 'Commit', actions: [action]) + + result.newrev + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 6854d514dcc..07fef203691 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1962,66 +1962,15 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#compare_source_branch' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } - - context 'within same repository' do - it 'does not create a temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', repository, 'feature', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - - it 'returns empty commits when source ref does not exist' do - compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false) + it 'delegates to Gitlab::Git::CrossRepoComparer' do + expect_next_instance_of(::Gitlab::Git::CrossRepoComparer) do |instance| + expect(instance.source_repo).to eq(:source_repository) + expect(instance.target_repo).to eq(repository) - expect(compare.commits).to be_empty + expect(instance).to receive(:compare).with('feature', 'master', straight: :straight) end - end - context 'with different repositories' do - context 'when ref is known by source repo, but not by target' do - before do - mutable_repository.write_ref('another-branch', 'feature') - end - - it 'creates temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - end - - context 'when ref is known by source and target repos' do - before do - mutable_repository.write_ref('another-branch', 'feature') - repository.write_ref('another-branch', 'feature') - end - - it 'does not create a temp ref' do - expect(repository).not_to receive(:fetch_source_branch!) - expect(repository).not_to receive(:delete_refs) - - compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) - expect(compare).to be_a(Gitlab::Git::Compare) - expect(compare.commits.count).to be > 0 - end - end - - context 'when ref is unknown by source repo' do - it 'returns nil when source ref does not exist' do - expect(repository).to receive(:fetch_source_branch!).and_call_original - expect(repository).to receive(:delete_refs).and_call_original - - compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false) - expect(compare).to be_nil - end - end + repository.compare_source_branch('master', :source_repository, 'feature', straight: :straight) end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 6bc9b6365d1..0faaaa50621 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::ReferenceExtractor do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } before do project.add_developer(project.creator) @@ -293,4 +293,34 @@ describe Gitlab::ReferenceExtractor do end end end + + describe '#references' do + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue, project: project) } + let(:text) { "Ref. #{issue.to_reference}" } + + subject { described_class.new(project, user) } + + before do + subject.analyze(text) + end + + context 'when references are visible' do + before do + project.add_developer(user) + end + + it 'returns visible references of given type' do + expect(subject.references(:issue)).to eq([issue]) + end + + it 'does not increase stateful_not_visible_counter' do + expect { subject.references(:issue) }.not_to change { subject.stateful_not_visible_counter } + end + end + + it 'increases stateful_not_visible_counter' do + expect { subject.references(:issue) }.to change { subject.stateful_not_visible_counter }.by(1) + end + end end diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index c851810ffb3..c8ed898122b 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -19,6 +19,74 @@ describe GenericCommitStatus do it { is_expected.not_to allow_value('javascript:alert(1)').for(:target_url) } end + describe '#name_uniqueness_across_types' do + let(:attributes) { {} } + let(:commit_status) { described_class.new(attributes) } + let(:status_name) { 'test-job' } + + subject(:errors) { commit_status.errors[:name] } + + shared_examples 'it does not have uniqueness errors' do + it 'does not return errors' do + commit_status.valid? + + is_expected.to be_empty + end + end + + context 'without attributes' do + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with only a pipeline' do + let(:attributes) { { pipeline: pipeline } } + + context 'without name' do + it_behaves_like 'it does not have uniqueness errors' + end + end + + context 'with only a name' do + let(:attributes) { { name: status_name } } + + context 'without pipeline' do + it_behaves_like 'it does not have uniqueness errors' + end + end + + context 'with pipeline and name' do + let(:attributes) do + { + pipeline: pipeline, + name: status_name + } + end + + context 'without other statuses' do + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with generic statuses' do + before do + create(:generic_commit_status, pipeline: pipeline, name: status_name) + end + + it_behaves_like 'it does not have uniqueness errors' + end + + context 'with ci_build statuses' do + before do + create(:ci_build, pipeline: pipeline, name: status_name) + end + + it 'returns name error' do + expect(commit_status).to be_invalid + is_expected.to include('has already been taken') + end + end + end + end + describe '#context' do subject { generic_commit_status.context } @@ -79,6 +147,12 @@ describe GenericCommitStatus do it { is_expected.not_to be_nil } end + + describe '#stage_idx' do + subject { generic_commit_status.stage_idx } + + it { is_expected.not_to be_nil } + end end describe '#present' do diff --git a/spec/models/grafana_integration_spec.rb b/spec/models/grafana_integration_spec.rb index 615865e17b9..662e8b1dd61 100644 --- a/spec/models/grafana_integration_spec.rb +++ b/spec/models/grafana_integration_spec.rb @@ -9,7 +9,7 @@ describe GrafanaIntegration do describe 'validations' do it { is_expected.to validate_presence_of(:project) } - it { is_expected.to validate_presence_of(:token) } + it { is_expected.to validate_presence_of(:encrypted_token) } it 'disallows invalid urls for grafana_url' do unsafe_url = %{https://replaceme.com/'><script>alert(document.cookie)</script>} @@ -66,4 +66,24 @@ describe GrafanaIntegration do end end end + + describe 'attribute encryption' do + subject(:grafana_integration) { create(:grafana_integration, token: 'super-secret') } + + context 'token' do + it 'encrypts original value into encrypted_token attribute' do + expect(grafana_integration.encrypted_token).not_to be_nil + end + + it 'locks access to raw value in private method', :aggregate_failures do + expect { grafana_integration.token }.to raise_error(NoMethodError, /private method .token. called/) + expect(grafana_integration.send(:token)).to eql('super-secret') + end + + it 'prevents overriding token value with its encrypted or masked version', :aggregate_failures do + expect { grafana_integration.update(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) } + expect { grafana_integration.update(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) } + end + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index a6d9ecaa7c5..2fa3f426da4 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -350,12 +350,12 @@ describe Note do end describe "cross_reference_not_visible_for?" do - let(:private_user) { create(:user) } - let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } } - let(:private_issue) { create(:issue, project: private_project) } + let_it_be(:private_user) { create(:user) } + let_it_be(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } } + let_it_be(:private_issue) { create(:issue, project: private_project) } - let(:ext_proj) { create(:project, :public) } - let(:ext_issue) { create(:issue, project: ext_proj) } + let_it_be(:ext_proj) { create(:project, :public) } + let_it_be(:ext_issue) { create(:issue, project: ext_proj) } shared_examples "checks references" do it "returns true" do @@ -393,10 +393,24 @@ describe Note do it_behaves_like "checks references" end - context "when there are two references in note" do + context "when there is a reference to a label" do + let_it_be(:private_label) { create(:label, project: private_project) } let(:note) do create :note, noteable: ext_issue, project: ext_proj, + note: "added label #{private_label.to_reference(ext_proj)}", + system: true + end + let!(:system_note_metadata) { create(:system_note_metadata, note: note, action: :label) } + + it_behaves_like "checks references" + end + + context "when there are two references in note" do + let_it_be(:ext_issue2) { create(:issue, project: ext_proj) } + let(:note) do + create :note, + noteable: ext_issue2, project: ext_proj, note: "mentioned in issue #{private_issue.to_reference(ext_proj)} and " \ "public issue #{ext_issue.to_reference(ext_proj)}", system: true diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index 639b8e96343..24ed836996e 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -164,6 +164,7 @@ describe API::CommitStatuses do expect(response).to have_gitlab_http_status(201) expect(job.status).to eq('pending') + expect(job.stage_idx).to eq(GenericCommitStatus::EXTERNAL_STAGE_IDX) end end @@ -331,6 +332,29 @@ describe API::CommitStatuses do end end + context 'when updating a protected ref' do + before do + create(:protected_branch, project: project, name: 'master') + post api(post_url, user), params: { state: 'running', ref: 'master' } + end + + context 'with user as developer' do + let(:user) { developer } + + it 'does not create commit status' do + expect(response).to have_gitlab_http_status(403) + end + end + + context 'with user as maintainer' do + let(:user) { create_user(:maintainer) } + + it 'creates commit status' do + expect(response).to have_gitlab_http_status(201) + end + end + end + context 'when commit SHA is invalid' do let(:sha) { 'invalid_sha' } @@ -372,6 +396,22 @@ describe API::CommitStatuses do .to include 'is blocked: Only allowed schemes are http, https' end end + + context 'when trying to update a status of a different type' do + let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha, ref: 'ref') } + let!(:ci_build) { create(:ci_build, pipeline: pipeline, name: 'test-job') } + let(:params) { { state: 'pending', name: 'test-job' } } + + before do + post api(post_url, developer), params: params + end + + it 'responds with bad request status and validation errors' do + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']['name']) + .to include 'has already been taken' + end + end end context 'reporter user' do diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index cc6cadb190a..8f82bd405d3 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -1509,7 +1509,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do authorize_artifacts - expect(response).to have_gitlab_http_status(500) + expect(response).to have_gitlab_http_status(:forbidden) end context 'authorization token is invalid' do @@ -1639,6 +1639,18 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + context 'Is missing GitLab Workhorse token headers' do + let(:jwt_token) { JWT.encode({ 'iss' => 'invalid-header' }, Gitlab::Workhorse.secret, 'HS256') } + + it 'fails to post artifacts without GitLab-Workhorse' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).once + + upload_artifacts(file_upload, headers_with_token) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + context 'when setting an expire date' do let(:default_artifacts_expire_in) {} let(:post_data) do diff --git a/spec/services/projects/group_links/destroy_service_spec.rb b/spec/services/projects/group_links/destroy_service_spec.rb index d78ab78c3d8..0fd1fcfe1a5 100644 --- a/spec/services/projects/group_links/destroy_service_spec.rb +++ b/spec/services/projects/group_links/destroy_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Projects::GroupLinks::DestroyService, '#execute' do - let(:group_link) { create :project_group_link } - let(:project) { group_link.project } + let(:project) { create(:project, :private) } + let!(:group_link) { create(:project_group_link, project: project) } let(:user) { create :user } let(:subject) { described_class.new(project, user) } @@ -15,4 +15,39 @@ describe Projects::GroupLinks::DestroyService, '#execute' do it 'returns false if group_link is blank' do expect { subject.execute(nil) }.not_to change { project.project_group_links.count } end + + describe 'todos cleanup' do + context 'when project is private' do + it 'triggers todos cleanup' do + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) + expect(project.private?).to be true + + subject.execute(group_link) + end + end + + context 'when project is public or internal' do + shared_examples_for 'removes confidential todos' do + it 'does not trigger todos cleanup' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id) + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, nil, project.id) + expect(project.private?).to be false + + subject.execute(group_link) + end + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it_behaves_like 'removes confidential todos' + end + + context 'when project is internal' do + let(:project) { create(:project, :public) } + + it_behaves_like 'removes confidential todos' + end + end + end end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 81d59a98b9b..e258c379bf8 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -210,7 +210,7 @@ describe Projects::Operations::UpdateService do integration = project.reload.grafana_integration expect(integration.grafana_url).to eq(expected_attrs[:grafana_url]) - expect(integration.token).to eq(expected_attrs[:token]) + expect(integration.send(:token)).to eq(expected_attrs[:token]) end end @@ -226,7 +226,7 @@ describe Projects::Operations::UpdateService do integration = project.reload.grafana_integration expect(integration.grafana_url).to eq(expected_attrs[:grafana_url]) - expect(integration.token).to eq(expected_attrs[:token]) + expect(integration.send(:token)).to eq(expected_attrs[:token]) end context 'with all grafana attributes blank in params' do diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index fe92b53cd91..95970590347 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -5,7 +5,7 @@ require "spec_helper" describe Projects::UpdatePagesService do set(:project) { create(:project, :repository) } set(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) } - set(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } + let(:build) { create(:ci_build, pipeline: pipeline, ref: 'HEAD') } let(:invalid_file) { fixture_file_upload('spec/fixtures/dk.png') } let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } @@ -242,6 +242,32 @@ describe Projects::UpdatePagesService do end end + context 'when file size is spoofed' do + let(:metadata) { spy('metadata') } + + include_context 'pages zip with spoofed size' + + before do + file = fixture_file_upload(fake_zip_path, 'pages.zip') + metafile = fixture_file_upload('spec/fixtures/pages.zip.meta') + + create(:ci_job_artifact, :archive, file: file, job: build) + create(:ci_job_artifact, :metadata, file: metafile, job: build) + + allow(build).to receive(:artifacts_metadata_entry) + .and_return(metadata) + allow(metadata).to receive(:total_size).and_return(100) + end + + it 'raises an error' do + expect do + subject.execute + end.to raise_error(Projects::UpdatePagesService::FailedToExtractError, + 'Entry public/index.html should be 1B but is larger when inflated') + expect(deploy_status).to be_script_failure + end + end + def deploy_status GenericCommitStatus.find_by(name: 'pages:deploy') end diff --git a/spec/support/helpers/reference_parser_helpers.rb b/spec/support/helpers/reference_parser_helpers.rb index f96a01d15b5..9084265b587 100644 --- a/spec/support/helpers/reference_parser_helpers.rb +++ b/spec/support/helpers/reference_parser_helpers.rb @@ -5,6 +5,11 @@ module ReferenceParserHelpers Nokogiri::HTML.fragment('<a></a>').children[0] end + def expect_gathered_references(result, visible, not_visible_count) + expect(result[:visible]).to eq(visible) + expect(result[:not_visible].count).to eq(not_visible_count) + end + shared_examples 'no project N+1 queries' do it 'avoids N+1 queries in #nodes_visible_to_user', :request_store do context = Banzai::RenderContext.new(project, user) diff --git a/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb b/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb new file mode 100644 index 00000000000..4cec5ab3b74 --- /dev/null +++ b/spec/support/shared_contexts/pages_zip_with_spoofed_size_shared_context.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# the idea of creating zip archive with spoofed size is borrowed from +# https://github.com/rubyzip/rubyzip/pull/403/files#diff-118213fb4baa6404a40f89e1147661ebR88 +RSpec.shared_context 'pages zip with spoofed size' do + let(:real_zip_path) { Tempfile.new(['real', '.zip']).path } + let(:fake_zip_path) { Tempfile.new(['fake', '.zip']).path } + + before do + full_file_name = 'public/index.html' + true_size = 500_000 + fake_size = 1 + + ::Zip::File.open(real_zip_path, ::Zip::File::CREATE) do |zf| + zf.get_output_stream(full_file_name) do |os| + os.write 'a' * true_size + end + end + + compressed_size = nil + ::Zip::File.open(real_zip_path) do |zf| + a_entry = zf.find_entry(full_file_name) + compressed_size = a_entry.compressed_size + end + + true_size_bytes = [compressed_size, true_size, full_file_name.size].pack('LLS') + fake_size_bytes = [compressed_size, fake_size, full_file_name.size].pack('LLS') + + data = File.binread(real_zip_path) + data.gsub! true_size_bytes, fake_size_bytes + + File.open(fake_zip_path, 'wb') do |file| + file.write data + end + end + + after do + File.delete(real_zip_path) if File.exist?(real_zip_path) + File.delete(fake_zip_path) if File.exist?(fake_zip_path) + end +end diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore index 259148fa18f..259148fa18f 100755..100644 --- a/vendor/gitignore/C++.gitignore +++ b/vendor/gitignore/C++.gitignore diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore index a1c2a238a96..a1c2a238a96 100755..100644 --- a/vendor/gitignore/Java.gitignore +++ b/vendor/gitignore/Java.gitignore |