Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq

CHANGELOG.md

@@ -2,6 +2,23 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 12.0.3 (2019-06-27)
+
+- No changes.
+### Security (10 changes)
+
+- Persist tmp snippet uploads at users.
+- Gate MR head_pipeline behind read_pipeline ability.
+- Fix DoS vulnerability in color validation regex.
+- Expose merge requests count based on user access.
+- Fix Denial of Service for comments when rendering issues/MR comments.
+- Add missing authorizations in GraphQL.
+- Disable Rails SQL query cache when applying service templates.
+- Prevent Billion Laughs attack.
+- Correctly check permissions when creating snippet notes.
+- Prevent the detection of merge request templates by unauthorized users.
+
+
 ## 12.0.2 (2019-06-25)
 
 ### Fixed (7 changes, 1 of them is from the community)
@@ -555,6 +572,27 @@ entry.
 - Add some frozen string to spec/**/*.rb. (gfyoung)
 
 
+## 11.10.8 (2019-06-27)
+
+- No changes.
+### Security (10 changes)
+
+- Fix Denial of Service for comments when rendering issues/MR comments.
+- Gate MR head_pipeline behind read_pipeline ability.
+- Fix DoS vulnerability in color validation regex.
+- Expose merge requests count based on user access.
+- Persist tmp snippet uploads at users.
+- Add missing authorizations in GraphQL.
+- Disable Rails SQL query cache when applying service templates.
+- Prevent Billion Laughs attack.
+- Correctly check permissions when creating snippet notes.
+- Prevent the detection of merge request templates by unauthorized users.
+
+### Performance (1 change)
+
+- Add improvements to global search of issues and merge requests. !27817
+
+
 ## 11.10.6 (2019-06-04)
 
 ### Fixed (7 changes, 1 of them is from the community)

app/controllers/concerns/issuable_collections.rb

@@ -42,7 +42,7 @@ module IssuableCollections
 
     @issuables          = @issuables.page(params[:page])
     @issuables          = per_page_for_relative_position if params[:sort] == 'relative_position'
-    @issuable_meta_data = issuable_meta_data(@issuables, collection_type)
+    @issuable_meta_data = issuable_meta_data(@issuables, collection_type, current_user)
     @total_pages        = issuable_page_count
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables

app/controllers/concerns/issuable_collections_action.rb

@@ -11,7 +11,7 @@ module IssuableCollectionsAction
               .non_archived
               .page(params[:page])
 
-    @issuable_meta_data = issuable_meta_data(@issues, collection_type)
+    @issuable_meta_data = issuable_meta_data(@issues, collection_type, current_user)
 
     respond_to do |format|
       format.html
@@ -22,7 +22,7 @@ module IssuableCollectionsAction
   def merge_requests
     @merge_requests = issuables_collection.page(params[:page])
 
-    @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type)
+    @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type, current_user)
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 

app/controllers/concerns/notes_actions.rb

@@ -203,17 +203,17 @@ module NotesActions
 
       # These params are also sent by the client but we need to set these based on
       # target_type and target_id because we're checking permissions based on that
-      create_params[:noteable_type] = params[:target_type].classify
+      create_params[:noteable_type] = noteable.class.name
 
-      case params[:target_type]
-      when 'commit'
-        create_params[:commit_id] = params[:target_id]
-      when 'merge_request'
-        create_params[:noteable_id] = params[:target_id]
+      case noteable
+      when Commit
+        create_params[:commit_id] = noteable.id
+      when MergeRequest
+        create_params[:noteable_id] = noteable.id
         # Notes on MergeRequest can have an extra `commit_id` context
         create_params[:commit_id] = params.dig(:note, :commit_id)
       else
-        create_params[:noteable_id] = params[:target_id]
+        create_params[:noteable_id] = noteable.id
       end
     end
   end

app/controllers/projects/application_controller.rb

@@ -12,6 +12,11 @@ class Projects::ApplicationController < ApplicationController
 
   helper_method :repository, :can_collaborate_with_project?, :user_access
 
+  rescue_from Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError do |exception|
+    log_exception(exception)
+    render_404
+  end
+
   private
 
   def project

app/controllers/projects/templates_controller.rb

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
 class Projects::TemplatesController < Projects::ApplicationController
-  before_action :authenticate_user!, :get_template_class
+  before_action :authenticate_user!
+  before_action :authorize_can_read_issuable!
+  before_action :get_template_class
 
   def show
     template = @template_type.find(params[:key], project)
@@ -13,9 +15,20 @@ class Projects::TemplatesController < Projects::ApplicationController
 
   private
 
+  # User must have:
+  # - `read_merge_request` to see merge request templates, or
+  # - `read_issue` to see issue templates
+  #
+  # Note params[:template_type] has a route constraint to limit it to
+  # `merge_request` or `issue`
+  def authorize_can_read_issuable!
+    action = [:read_, params[:template_type]].join
+
+    authorize_action!(action)
+  end
+
   def get_template_class
     template_types = { issue: Gitlab::Template::IssueTemplate, merge_request: Gitlab::Template::MergeRequestTemplate }.with_indifferent_access
     @template_type = template_types[params[:template_type]]
-    render json: [], status: :not_found unless @template_type
   end
 end

app/controllers/projects_controller.rb

@@ -298,7 +298,7 @@ class ProjectsController < Projects::ApplicationController
       elsif @project.feature_available?(:issues, current_user)
         @issues = issuables_collection.page(params[:page])
         @collection_type = 'Issue'
-        @issuable_meta_data = issuable_meta_data(@issues, @collection_type)
+        @issuable_meta_data = issuable_meta_data(@issues, @collection_type, current_user)
       end
 
       render :show

app/controllers/snippets/notes_controller.rb

@@ -5,8 +5,8 @@ class Snippets::NotesController < ApplicationController
   include ToggleAwardEmoji
 
   skip_before_action :authenticate_user!, only: [:index]
-  before_action :snippet
-  before_action :authorize_read_snippet!, only: [:show, :index, :create]
+  before_action :authorize_read_snippet!, only: [:show, :index]
+  before_action :authorize_create_note!, only: [:create]
 
   private
 
@@ -33,4 +33,8 @@ class Snippets::NotesController < ApplicationController
   def authorize_read_snippet!
     return render_404 unless can?(current_user, :read_personal_snippet, snippet)
   end
+
+  def authorize_create_note!
+    access_denied! unless can?(current_user, :create_note, noteable)
+  end
 end

app/controllers/snippets_controller.rb

@@ -137,7 +137,7 @@ class SnippetsController < ApplicationController
 
   def move_temporary_files
     params[:files].each do |file|
-      FileMover.new(file, @snippet).execute
+      FileMover.new(file, from_model: current_user, to_model: @snippet).execute
     end
   end
 end

app/controllers/uploads_controller.rb

@@ -41,7 +41,11 @@ class UploadsController < ApplicationController
       when Note
         can?(current_user, :read_project, model.project)
       when User
-        true
+        # We validate the current user has enough (writing)
+        # access to itself when a secret is given.
+        # For instance, user avatars are readable by anyone,
+        # while temporary, user snippet uploads are not.
+        !secret? || can?(current_user, :update_user, model)
       when Appearance
         true
       else
@@ -56,9 +60,13 @@ class UploadsController < ApplicationController
   def authorize_create_access!
     return unless model
 
-    # for now we support only personal snippets comments. Only personal_snippet
-    # is allowed as a model to #create through routing.
-    authorized = can?(current_user, :create_note, model)
+    authorized =
+      case model
+      when User
+        can?(current_user, :update_user, model)
+      else
+        can?(current_user, :create_note, model)
+      end
 
     render_unauthorized unless authorized
   end
@@ -75,6 +83,10 @@ class UploadsController < ApplicationController
     User === model || Appearance === model
   end
 
+  def secret?
+    params[:secret].present?
+  end
+
   def upload_model_class
     MODEL_CLASSES[params[:model]] || raise(UnknownUploadModelError)
   end

app/graphql/types/ci/detailed_status_type.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 module Types
   module Ci
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This is presented through `PipelineType` that has its own authorization
     class DetailedStatusType < BaseObject
       graphql_name 'DetailedStatus'
 
@@ -13,5 +15,6 @@ module Types
       field :text, GraphQL::STRING_TYPE, null: false
       field :tooltip, GraphQL::STRING_TYPE, null: false, method: :status_tooltip
     end
+    # rubocop: enable Graphql/AuthorizeTypes
   end
 end

app/graphql/types/issue_state_enum.rb

@@ -1,8 +1,11 @@
 # frozen_string_literal: true
 
 module Types
+  # rubocop: disable Graphql/AuthorizeTypes
+  # This is a BaseEnum through IssuableEnum, so it does not need authorization
   class IssueStateEnum < IssuableStateEnum
     graphql_name 'IssueState'
     description 'State of a GitLab issue'
   end
+  # rubocop: enable Graphql/AuthorizeTypes
 end

app/graphql/types/label_type.rb

@@ -4,6 +4,8 @@ module Types
   class LabelType < BaseObject
     graphql_name 'Label'
 
+    authorize :read_label
+
     field :description, GraphQL::STRING_TYPE, null: true
     markdown_field :description_html, null: true
     field :title, GraphQL::STRING_TYPE, null: false

app/graphql/types/merge_request_state_enum.rb

@@ -1,10 +1,13 @@
 # frozen_string_literal: true
 
 module Types
+  # rubocop: disable Graphql/AuthorizeTypes
+  # This is a BaseEnum through IssuableEnum, so it does not need authorization
   class MergeRequestStateEnum < IssuableStateEnum
     graphql_name 'MergeRequestState'
     description 'State of a GitLab merge request'
 
     value 'merged'
   end
+  # rubocop: enable Graphql/AuthorizeTypes
 end

app/graphql/types/metadata_type.rb

@@ -4,6 +4,8 @@ module Types
   class MetadataType < ::Types::BaseObject
     graphql_name 'Metadata'
 
+    authorize :read_instance_metadata
+
     field :version, GraphQL::STRING_TYPE, null: false
     field :revision, GraphQL::STRING_TYPE, null: false
   end

app/graphql/types/namespace_type.rb

@@ -4,6 +4,8 @@ module Types
   class NamespaceType < BaseObject
     graphql_name 'Namespace'
 
+    authorize :read_namespace
+
     field :id, GraphQL::ID_TYPE, null: false
 
     field :name, GraphQL::STRING_TYPE, null: false

app/graphql/types/notes/diff_position_type.rb

@@ -2,6 +2,8 @@
 
 module Types
   module Notes
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This is presented through `NoteType` that has its own authorization
     class DiffPositionType < BaseObject
       graphql_name 'DiffPosition'
 
@@ -42,5 +44,6 @@ module Types
             description: "The total height of the image",
             resolve: -> (position, _args, _ctx) { position.height if position.on_image? }
     end
+    # rubocop: enable Graphql/AuthorizeTypes
   end
 end

app/graphql/types/project_type.rb

@@ -67,14 +67,14 @@ module Types
     field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true
     field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true
 
-    field :namespace, Types::NamespaceType, null: false
+    field :namespace, Types::NamespaceType, null: true
     field :group, Types::GroupType, null: true
 
     field :statistics, Types::ProjectStatisticsType,
           null: true,
           resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchProjectStatisticsLoader.new(obj.id).find }
 
-    field :repository, Types::RepositoryType, null: false
+    field :repository, Types::RepositoryType, null: true
 
     field :merge_requests,
           Types::MergeRequestType.connection_type,

app/graphql/types/query_type.rb

@@ -22,10 +22,7 @@ module Types
     field :metadata, Types::MetadataType,
           null: true,
           resolver: Resolvers::MetadataResolver,
-          description: 'Metadata about GitLab' do |*args|
-
-      authorize :read_instance_metadata
-    end
+          description: 'Metadata about GitLab'
 
     field :echo, GraphQL::STRING_TYPE, null: false, function: Functions::Echo.new
   end

app/graphql/types/task_completion_status.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 module Types
+  # rubocop: disable Graphql/AuthorizeTypes
+  # This is used in `IssueType` and `MergeRequestType` both of which have their
+  # own authorization
   class TaskCompletionStatus < BaseObject
     graphql_name 'TaskCompletionStatus'
     description 'Completion status of tasks'
@@ -8,4 +11,5 @@ module Types
     field :count, GraphQL::INT_TYPE, null: false
     field :completed_count, GraphQL::INT_TYPE, null: false
   end
+  # rubocop: enable Graphql/AuthorizeTypes
 end

app/graphql/types/tree/blob_type.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 module Types
   module Tree
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This is presented through `Repository` that has its own authorization
     class BlobType < BaseObject
       implements Types::Tree::EntryType
 
@@ -12,6 +14,7 @@ module Types
       field :lfs_oid, GraphQL::STRING_TYPE, null: true, resolve: -> (blob, args, ctx) do
         Gitlab::Graphql::Loaders::BatchLfsOidLoader.new(blob.repository, blob.id).find
       end
+      # rubocop: enable Graphql/AuthorizeTypes
     end
   end
 end

app/graphql/types/tree/submodule_type.rb

@@ -1,10 +1,13 @@
 # frozen_string_literal: true
 module Types
   module Tree
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This is presented through `Repository` that has its own authorization
     class SubmoduleType < BaseObject
       implements Types::Tree::EntryType
 
       graphql_name 'Submodule'
     end
+    # rubocop: enable Graphql/AuthorizeTypes
   end
 end

app/graphql/types/tree/tree_entry_type.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 module Types
   module Tree
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This is presented through `Repository` that has its own authorization
     class TreeEntryType < BaseObject
       implements Types::Tree::EntryType
 
@@ -11,5 +13,6 @@ module Types
 
       field :web_url, GraphQL::STRING_TYPE, null: true
     end
+    # rubocop: enable Graphql/AuthorizeTypes
   end
 end

app/graphql/types/tree/tree_type.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 module Types
   module Tree
+    # rubocop: disable Graphql/AuthorizeTypes
+    # This is presented through `Repository` that has its own authorization
     class TreeType < BaseObject
       graphql_name 'Tree'
 
@@ -13,6 +15,7 @@ module Types
       field :blobs, Types::Tree::BlobType.connection_type, null: false, resolve: -> (obj, args, ctx) do
         Gitlab::Graphql::Representation::TreeEntry.decorate(obj.blobs, obj.repository)
       end
+      # rubocop: enable Graphql/AuthorizeTypes
     end
   end
 end

app/helpers/issuables_helper.rb

@@ -280,7 +280,7 @@ module IssuablesHelper
       initialTaskStatus: issuable.task_status
     }
 
-    data[:hasClosingMergeRequest] = issuable.merge_requests_count != 0 if issuable.is_a?(Issue)
+    data[:hasClosingMergeRequest] = issuable.merge_requests_count(current_user) != 0 if issuable.is_a?(Issue)
 
     if parent.is_a?(Group)
       data[:groupPath] = parent.path

app/helpers/snippets_helper.rb

@@ -1,6 +1,16 @@
 # frozen_string_literal: true
 
 module SnippetsHelper
+  def snippets_upload_path(snippet, user)
+    return unless user
+
+    if snippet&.persisted?
+      upload_path('personal_snippet', id: snippet.id)
+    else
+      upload_path('user', id: user.id)
+    end
+  end
+
   def reliable_snippet_path(snippet, opts = nil)
     if snippet.project_id?
       project_snippet_path(snippet.project, snippet, opts)

app/models/concerns/issuable.rb

@@ -29,7 +29,11 @@ module Issuable
   # This object is used to gather issuable meta data for displaying
   # upvotes, downvotes, notes and closing merge requests count for issues and merge requests
   # lists avoiding n+1 queries and improving performance.
-  IssuableMeta = Struct.new(:upvotes, :downvotes, :user_notes_count, :merge_requests_count)
+  IssuableMeta = Struct.new(:upvotes, :downvotes, :user_notes_count, :mrs_count) do
+    def merge_requests_count(user = nil)
+      mrs_count
+    end
+  end
 
   included do
     cache_markdown_field :title, pipeline: :single_line

app/models/issue.rb

@@ -250,8 +250,8 @@ class Issue < ApplicationRecord
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  def merge_requests_count
-    merge_requests_closing_issues.count
+  def merge_requests_count(user = nil)
+    ::MergeRequestsClosingIssues.count_for_issue(self.id, user)
   end
 
   def labels_hook_attrs

app/models/merge_requests_closing_issues.rb

@@ -7,11 +7,38 @@ class MergeRequestsClosingIssues < ApplicationRecord
   validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true
   validates :issue_id, presence: true
 
+  scope :with_issues, ->(ids) { where(issue_id: ids) }
+  scope :with_merge_requests_enabled, -> do
+    joins(:merge_request)
+      .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id')
+      .where('project_features.merge_requests_access_level >= :access', access: ProjectFeature::ENABLED)
+  end
+
+  scope :accessible_by, ->(user) do
+    joins(:merge_request)
+      .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id')
+      .where('project_features.merge_requests_access_level >= :access OR EXISTS(:authorizations)',
+             access: ProjectFeature::ENABLED,
+             authorizations: user.authorizations_for_projects(min_access_level: Gitlab::Access::REPORTER, related_project_column: "merge_requests.target_project_id")
+            )
+  end
+
   class << self
-    def count_for_collection(ids)
-      group(:issue_id)
-        .where(issue_id: ids)
-        .pluck('issue_id', 'COUNT(*) as count')
+    def count_for_collection(ids, current_user)
+      closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', 'COUNT(*) as count')
+    end
+
+    def count_for_issue(id, current_user)
+      closing_merge_requests(id, current_user).count
+    end
+
+    private
+
+    def closing_merge_requests(ids, current_user)
+      return with_issues(ids) if current_user&.admin?
+      return with_issues(ids).with_merge_requests_enabled if current_user.blank?
+
+      with_issues(ids).accessible_by(current_user)
     end
   end
 end

app/policies/repository_policy.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class RepositoryPolicy < BasePolicy
+  delegate { @subject.project }
+end

app/uploaders/file_mover.rb

@@ -1,22 +1,29 @@
 # frozen_string_literal: true
 
 class FileMover
-  attr_reader :secret, :file_name, :model, :update_field
+  include Gitlab::Utils::StrongMemoize
 
-  def initialize(file_path, model, update_field = :description)
+  attr_reader :secret, :file_name, :from_model, :to_model, :update_field
+
+  def initialize(file_path, update_field = :description, from_model:, to_model:)
     @secret = File.split(File.dirname(file_path)).last
     @file_name = File.basename(file_path)
-    @model = model
+    @from_model = from_model
+    @to_model = to_model
     @update_field = update_field
   end
 
   def execute
+    temp_file_uploader.retrieve_from_store!(file_name)
+
     return unless valid?
 
+    uploader.retrieve_from_store!(file_name)
+
     move
 
     if update_markdown
-      uploader.record_upload
+      update_upload_model
       uploader.schedule_background_upload
     end
   end
@@ -24,52 +31,77 @@ class FileMover
   private
 
   def valid?
-    Pathname.new(temp_file_path).realpath.to_path.start_with?(
-      (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
-    )
+    if temp_file_uploader.file_storage?
+      Pathname.new(temp_file_path).realpath.to_path.start_with?(
+        (Pathname(temp_file_uploader.root) + temp_file_uploader.base_dir).to_path
+      )
+    else
+      temp_file_uploader.exists?
+    end
   end
 
   def move
-    FileUtils.mkdir_p(File.dirname(file_path))
-    FileUtils.move(temp_file_path, file_path)
+    if temp_file_uploader.file_storage?
+      FileUtils.mkdir_p(File.dirname(file_path))
+      FileUtils.move(temp_file_path, file_path)
+    else
+      uploader.copy_file(temp_file_uploader.file)
+      temp_file_uploader.upload.destroy!
+    end
   end
 
   def update_markdown
-    updated_text = model.read_attribute(update_field)
-                        .gsub(temp_file_uploader.markdown_link, uploader.markdown_link)
-    model.update_attribute(update_field, updated_text)
+    updated_text = to_model.read_attribute(update_field)
+                           .gsub(temp_file_uploader.markdown_link, uploader.markdown_link)
+    to_model.update_attribute(update_field, updated_text)
   rescue
     revert
     false
   end
 
+  def update_upload_model
+    return unless upload = temp_file_uploader.upload
+    return if upload.destroyed?
+
+    upload.update!(model: to_model)
+  end
+
   def temp_file_path
-    return @temp_file_path if @temp_file_path
-
-    temp_file_uploader.retrieve_from_store!(file_name)
-
-    @temp_file_path = temp_file_uploader.file.path
+    strong_memoize(:temp_file_path) do
+      temp_file_uploader.file.path
+    end
   end
 
   def file_path
-    return @file_path if @file_path
-
-    uploader.retrieve_from_store!(file_name)
-
-    @file_path = uploader.file.path
+    strong_memoize(:file_path) do
+      uploader.file.path
+    end
   end
 
   def uploader
-    @uploader ||= PersonalFileUploader.new(model, secret: secret)
+    @uploader ||=
+      begin
+        uploader = PersonalFileUploader.new(to_model, secret: secret)
+
+        # Enforcing a REMOTE object storage given FileUploader#retrieve_from_store! won't do it
+        # (there's no upload at the target yet).
+        if uploader.class.object_store_enabled?
+          uploader.object_store = ::ObjectStorage::Store::REMOTE
+        end
+
+        uploader
+      end
   end
 
   def temp_file_uploader
-    @temp_file_uploader ||= PersonalFileUploader.new(nil, secret: secret)
+    @temp_file_uploader ||= PersonalFileUploader.new(from_model, secret: secret)
   end
 
   def revert
-    Rails.logger.warn("Markdown not updated, file move reverted for #{model}")
+    Rails.logger.warn("Markdown not updated, file move reverted for #{to_model}")
 
-    FileUtils.move(file_path, temp_file_path)
+    if temp_file_uploader.file_storage?
+      FileUtils.move(file_path, temp_file_path)
+    end
   end
 end

app/validators/color_validator.rb

@@ -12,7 +12,7 @@
 #   end
 #
 class ColorValidator < ActiveModel::EachValidator
-  PATTERN = /\A\#[0-9A-Fa-f]{3}{1,2}+\Z/.freeze
+  PATTERN = /\A\#(?:[0-9A-Fa-f]{3}){1,2}\Z/.freeze
 
   def validate_each(record, attribute, value)
     unless value =~ PATTERN

app/views/layouts/snippets.html.haml

@@ -1,9 +1,10 @@
 - header_title _("Snippets"), snippets_path
+- snippets_upload_path = snippets_upload_path(@snippet, current_user)
 
 - content_for :page_specific_javascripts do
-  - if @snippet && current_user
+  - if snippets_upload_path
     -# haml-lint:disable InlineJavaScript
     :javascript
-      window.uploads_path = "#{upload_path('personal_snippet', id: @snippet.id)}";
+      window.uploads_path = "#{snippets_upload_path}";
 
 = render template: "layouts/application"

app/views/shared/_issuable_meta_data.html.haml

@@ -2,7 +2,7 @@
 - issue_votes        = @issuable_meta_data[issuable.id]
 - upvotes, downvotes = issue_votes.upvotes, issue_votes.downvotes
 - issuable_url       = @collection_type == "Issue" ? issue_path(issuable, anchor: 'notes') : merge_request_path(issuable, anchor: 'notes')
-- issuable_mr        = @issuable_meta_data[issuable.id].merge_requests_count
+- issuable_mr        = @issuable_meta_data[issuable.id].merge_requests_count(current_user)
 
 - if issuable_mr > 0
   %li.issuable-mr.d-none.d-sm-block.has-tooltip{ title: _('Related merge requests') }

changelogs/unreleased/osw-persist-tmp-snippet-uploads.yml

@@ -0,0 +1,5 @@
+---
+title: Persist tmp snippet uploads at users
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2858-fix-color-validation.yml

@@ -0,0 +1,5 @@
+---
+title: Fix DoS vulnerability in color validation regex
+merge_request:
+author:
+type: security

changelogs/unreleased/security-59581-related-merge-requests-count.yml

@@ -0,0 +1,5 @@
+---
+title: Expose merge requests count based on user access
+merge_request:
+author:
+type: security

changelogs/unreleased/security-DOS_issue_comments_banzai.yml

@@ -0,0 +1,5 @@
+---
+title: Fix Denial of Service for comments when rendering issues/MR comments
+merge_request:
+author:
+type: security

changelogs/unreleased/security-bvl-enforce-graphql-type-authorization.yml

@@ -0,0 +1,5 @@
+---
+title: Add missing authorizations in GraphQL
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fp-prevent-billion-laughs-attack.yml

@@ -0,0 +1,5 @@
+---
+title: Prevent Billion Laughs attack
+merge_request:
+author:
+type: security

changelogs/unreleased/security-mr-head-pipeline-leak.yml

@@ -0,0 +1,5 @@
+---
+title: Gate MR head_pipeline behind read_pipeline ability.
+merge_request:
+author:
+type: security

changelogs/unreleased/security-notes-in-private-snippets.yml

@@ -0,0 +1,5 @@
+---
+title: Correctly check permissions when creating snippet notes
+merge_request:
+author:
+type: security

changelogs/unreleased/security-prevent-detection-of-merge-request-template-name.yml

@@ -0,0 +1,5 @@
+---
+title: Prevent the detection of merge request templates by unauthorized users
+merge_request:
+author:
+type: security

config/routes/project.rb

@@ -182,7 +182,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       #
       # Templates
       #
-      get '/templates/:template_type/:key' => 'templates#show', as: :template, constraints: { key: %r{[^/]+} }
+      get '/templates/:template_type/:key' => 'templates#show',
+          as: :template,
+          defaults: { format: 'json' },
+          constraints: { key: %r{[^/]+}, template_type: %r{issue|merge_request}, format: 'json' }
 
       resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do
         member do

config/routes/uploads.rb

@@ -7,7 +7,7 @@ scope path: :uploads do
   # show uploads for models, snippets (notes) available for now
   get '-/system/:model/:id/:secret/:filename',
     to: 'uploads#show',
-    constraints: { model: /personal_snippet/, id: /\d+/, filename: %r{[^/]+} }
+    constraints: { model: /personal_snippet|user/, id: /\d+/, filename: %r{[^/]+} }
 
   # show temporary uploads
   get '-/system/temp/:secret/:filename',
@@ -28,7 +28,7 @@ scope path: :uploads do
   # create uploads for models, snippets (notes) available for now
   post ':model',
     to: 'uploads#create',
-    constraints: { model: /personal_snippet/, id: /\d+/ },
+    constraints: { model: /personal_snippet|user/, id: /\d+/ },
     as: 'upload'
 end
 

lib/api/entities.rb

@@ -498,9 +498,9 @@ module API
       expose :state, :created_at, :updated_at
 
       # Avoids an N+1 query when metadata is included
-      def issuable_metadata(subject, options, method)
+      def issuable_metadata(subject, options, method, args = nil)
         cached_subject = options.dig(:issuable_metadata, subject.id)
-        (cached_subject || subject).public_send(method) # rubocop: disable GitlabSecurity/PublicSend
+        (cached_subject || subject).public_send(method, *args) # rubocop: disable GitlabSecurity/PublicSend
       end
     end
 
@@ -564,7 +564,7 @@ module API
       end
 
       expose(:user_notes_count)     { |issue, options| issuable_metadata(issue, options, :user_notes_count) }
-      expose(:merge_requests_count) { |issue, options| issuable_metadata(issue, options, :merge_requests_count) }
+      expose(:merge_requests_count) { |issue, options| issuable_metadata(issue, options, :merge_requests_count, options[:current_user]) }
       expose(:upvotes)              { |issue, options| issuable_metadata(issue, options, :upvotes) }
       expose(:downvotes)            { |issue, options| issuable_metadata(issue, options, :downvotes) }
       expose :due_date
@@ -757,7 +757,9 @@ module API
         merge_request.metrics&.pipeline
       end
 
-      expose :head_pipeline, using: 'API::Entities::Pipeline'
+      expose :head_pipeline, using: 'API::Entities::Pipeline', if: -> (_, options) do
+        Ability.allowed?(options[:current_user], :read_pipeline, options[:project])
+      end
 
       expose :diff_refs, using: Entities::DiffRefs
 

lib/api/issues.rb

@@ -96,7 +96,7 @@ module API
           with: Entities::Issue,
           with_labels_details: declared_params[:with_labels_details],
           current_user: current_user,
-          issuable_metadata: issuable_meta_data(issues, 'Issue')
+          issuable_metadata: issuable_meta_data(issues, 'Issue', current_user)
         }
 
         present issues, options
@@ -122,7 +122,7 @@ module API
           with: Entities::Issue,
           with_labels_details: declared_params[:with_labels_details],
           current_user: current_user,
-          issuable_metadata: issuable_meta_data(issues, 'Issue')
+          issuable_metadata: issuable_meta_data(issues, 'Issue', current_user)
         }
 
         present issues, options
@@ -161,7 +161,7 @@ module API
           with_labels_details: declared_params[:with_labels_details],
           current_user: current_user,
           project: user_project,
-          issuable_metadata: issuable_meta_data(issues, 'Issue')
+          issuable_metadata: issuable_meta_data(issues, 'Issue', current_user)
         }
 
         present issues, options

lib/api/merge_requests.rb

@@ -72,7 +72,7 @@ module API
         if params[:view] == 'simple'
           options[:with] = Entities::MergeRequestSimple
         else
-          options[:issuable_metadata] = issuable_meta_data(merge_requests, 'MergeRequest')
+          options[:issuable_metadata] = issuable_meta_data(merge_requests, 'MergeRequest', current_user)
         end
 
         options

lib/api/todos.rb

@@ -65,7 +65,7 @@ module API
             next unless collection
 
             targets = collection.map(&:target)
-            options[type] = { issuable_metadata: issuable_meta_data(targets, type) }
+            options[type] = { issuable_metadata: issuable_meta_data(targets, type, current_user) }
           end
         end
       end

lib/banzai/filter/relative_link_filter.rb

@@ -102,7 +102,7 @@ module Banzai
       end
 
       def relative_file_path(uri)
-        path = Addressable::URI.unescape(uri.path)
+        path = Addressable::URI.unescape(uri.path).delete("\0")
         request_path = Addressable::URI.unescape(context[:requested_path])
         nested_path = build_relative_path(path, request_path)
         file_exists?(nested_path) ? nested_path : path

lib/gitlab/ci/config.rb

@@ -23,6 +23,11 @@ module Gitlab
 
         @root = Entry::Root.new(@config)
         @root.compose!
+
+      rescue Gitlab::Config::Loader::Yaml::DataTooLargeError => e
+        Gitlab::Sentry.track_exception(e, extra: { user: user.inspect, project: project.inspect })
+        raise Config::ConfigError, e.message
+
       rescue *rescue_errors => e
         raise Config::ConfigError, e.message
       end

lib/gitlab/config/loader/yaml.rb

@@ -4,6 +4,13 @@ module Gitlab
   module Config
     module Loader
       class Yaml
+        DataTooLargeError = Class.new(Loader::FormatError)
+
+        include Gitlab::Utils::StrongMemoize
+
+        MAX_YAML_SIZE = 1.megabyte
+        MAX_YAML_DEPTH = 100
+
         def initialize(config)
           @config = YAML.safe_load(config, [Symbol], [], true)
         rescue Psych::Exception => e
@@ -11,16 +18,35 @@ module Gitlab
         end
 
         def valid?
-          @config.is_a?(Hash)
+          hash? && !too_big?
         end
 
         def load!
-          unless valid?
-            raise Loader::FormatError, 'Invalid configuration format'
-          end
+          raise DataTooLargeError, 'The parsed YAML is too big' if too_big?
+          raise Loader::FormatError, 'Invalid configuration format' unless hash?
 
           @config.deep_symbolize_keys
         end
+
+        private
+
+        def hash?
+          @config.is_a?(Hash)
+        end
+
+        def too_big?
+          return false unless Feature.enabled?(:ci_yaml_limit_size, default_enabled: true)
+
+          !deep_size.valid?
+        end
+
+        def deep_size
+          strong_memoize(:deep_size) do
+            Gitlab::Utils::DeepSize.new(@config,
+              max_size: MAX_YAML_SIZE,
+              max_depth: MAX_YAML_DEPTH)
+          end
+        end
       end
     end
   end

lib/gitlab/graphql/authorize/authorize_field_service.rb

@@ -39,6 +39,8 @@ module Gitlab
             type = node_type_for_basic_connection(type)
           end
 
+          type = type.unwrap if type.kind.non_null?
+
           Array.wrap(type.metadata[:authorize])
         end
 

lib/gitlab/issuable_metadata.rb

@@ -2,7 +2,7 @@
 
 module Gitlab
   module IssuableMetadata
-    def issuable_meta_data(issuable_collection, collection_type)
+    def issuable_meta_data(issuable_collection, collection_type, user = nil)
       # ActiveRecord uses Object#extend for null relations.
       if !(issuable_collection.singleton_class < ActiveRecord::NullRelation) &&
           issuable_collection.respond_to?(:limit_value) &&
@@ -23,7 +23,7 @@ module Gitlab
       issuable_votes_count = ::AwardEmoji.votes_for_collection(issuable_ids, collection_type)
       issuable_merge_requests_count =
         if collection_type == 'Issue'
-          ::MergeRequestsClosingIssues.count_for_collection(issuable_ids)
+          ::MergeRequestsClosingIssues.count_for_collection(issuable_ids, user)
         else
           []
         end

lib/gitlab/utils/deep_size.rb

@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require 'objspace'
+
+module Gitlab
+  module Utils
+    class DeepSize
+      Error = Class.new(StandardError)
+      TooMuchDataError = Class.new(Error)
+
+      DEFAULT_MAX_SIZE = 1.megabyte
+      DEFAULT_MAX_DEPTH = 100
+
+      def initialize(root, max_size: DEFAULT_MAX_SIZE, max_depth: DEFAULT_MAX_DEPTH)
+        @root = root
+        @max_size = max_size
+        @max_depth = max_depth
+        @size = 0
+        @depth = 0
+
+        evaluate
+      end
+
+      def valid?
+        !too_big? && !too_deep?
+      end
+
+      private
+
+      def evaluate
+        add_object(@root)
+      rescue Error
+        # NOOP
+      end
+
+      def too_big?
+        @size > @max_size
+      end
+
+      def too_deep?
+        @depth > @max_depth
+      end
+
+      def add_object(object)
+        @size += ObjectSpace.memsize_of(object)
+        raise TooMuchDataError if @size > @max_size
+
+        add_array(object) if object.is_a?(Array)
+        add_hash(object) if object.is_a?(Hash)
+      end
+
+      def add_array(object)
+        with_nesting do
+          object.each do |n|
+            add_object(n)
+          end
+        end
+      end
+
+      def add_hash(object)
+        with_nesting do
+          object.each do |key, value|
+            add_object(key)
+            add_object(value)
+          end
+        end
+      end
+
+      def with_nesting
+        @depth += 1
+        raise TooMuchDataError if too_deep?
+
+        yield
+
+        @depth -= 1
+      end
+    end
+  end
+end

rubocop/cop/graphql/authorize_types.rb

@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+require_relative '../../spec_helpers'
+
+module RuboCop
+  module Cop
+    module Graphql
+      class AuthorizeTypes < RuboCop::Cop::Cop
+        include SpecHelpers
+
+        MSG = 'Add an `authorize :ability` call to the type: '\
+              'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#type-authorization'
+
+        TYPES_DIR = 'app/graphql/types'
+
+        # We want to exclude our own basetypes and scalars
+        WHITELISTED_TYPES = %w[BaseEnum BaseScalar BasePermissionType MutationType
+                               QueryType GraphQL::Schema BaseUnion].freeze
+
+        def_node_search :authorize?, <<~PATTERN
+          (send nil? :authorize ...)
+        PATTERN
+
+        def on_class(node)
+          return unless in_type?(node)
+          return if whitelisted?(class_constant(node))
+          return if whitelisted?(superclass_constant(node))
+
+          add_offense(node, location: :expression) unless authorize?(node)
+        end
+
+        private
+
+        def in_type?(node)
+          return if in_spec?(node)
+
+          path = node.location.expression.source_buffer.name
+
+          path.include?(TYPES_DIR)
+        end
+
+        def whitelisted?(class_node)
+          return false unless class_node&.const_name
+
+          WHITELISTED_TYPES.any? { |whitelisted| class_node.const_name.include?(whitelisted) }
+        end
+
+        def class_constant(node)
+          node.descendants.first
+        end
+
+        def superclass_constant(class_node)
+          # First one is the class name itself, second is it's superclass
+          _class_constant, *others = class_node.descendants
+
+          others.find { |node| node.const_type? && node&.const_name != 'Types' }
+        end
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -43,3 +43,4 @@ require_relative 'cop/code_reuse/serializer'
 require_relative 'cop/code_reuse/active_record'
 require_relative 'cop/group_public_or_visible_to_user'
 require_relative 'cop/inject_enterprise_edition_module'
+require_relative 'cop/graphql/authorize_types'

spec/controllers/projects/notes_controller_spec.rb

@@ -252,7 +252,7 @@ describe Projects::NotesController do
       before do
         service_params = ActionController::Parameters.new({
           note: 'some note',
-          noteable_id: merge_request.id.to_s,
+          noteable_id: merge_request.id,
           noteable_type: 'MergeRequest',
           commit_id: nil,
           merge_request_diff_head_sha: 'sha'

spec/controllers/projects/templates_controller_spec.rb

@@ -3,49 +3,101 @@
 require 'spec_helper'
 
 describe Projects::TemplatesController do
-  let(:project) { create(:project, :repository) }
+  let(:project) { create(:project, :repository, :private) }
   let(:user) { create(:user) }
-  let(:user2) { create(:user) }
-  let(:file_path_1) { '.gitlab/issue_templates/bug.md' }
+  let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' }
+  let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' }
   let(:body) { JSON.parse(response.body) }
-
-  before do
-    project.add_developer(user)
-    sign_in(user)
-  end
-
-  before do
-    project.add_user(user, Gitlab::Access::MAINTAINER)
-    project.repository.create_file(user, file_path_1, 'something valid',
-      message: 'test 3', branch_name: 'master')
-  end
+  let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') }
+  let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') }
 
   describe '#show' do
-    it 'renders template name and content as json' do
-      get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json)
+    shared_examples 'renders issue templates as json' do
+      it do
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json)
 
-      expect(response.status).to eq(200)
-      expect(body["name"]).to eq("bug")
-      expect(body["content"]).to eq("something valid")
+        expect(response.status).to eq(200)
+        expect(body['name']).to eq('issue_template')
+        expect(body['content']).to eq('issue content')
+      end
     end
 
-    it 'renders 404 when unauthorized' do
-      sign_in(user2)
-      get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json)
+    shared_examples 'renders merge request templates as json' do
+      it do
+        get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json)
 
-      expect(response.status).to eq(404)
+        expect(response.status).to eq(200)
+        expect(body['name']).to eq('merge_request_template')
+        expect(body['content']).to eq('merge request content')
+      end
     end
 
-    it 'renders 404 when template type is not found' do
-      sign_in(user)
-      get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json)
+    shared_examples 'renders 404 when requesting an issue template' do
+      it do
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json)
 
-      expect(response.status).to eq(404)
+        expect(response.status).to eq(404)
+      end
     end
 
-    it 'renders 404 without errors' do
-      sign_in(user)
-      expect { get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) }.not_to raise_error
+    shared_examples 'renders 404 when requesting a merge request template' do
+      it do
+        get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json)
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    shared_examples 'renders 404 when params are invalid' do
+      it 'does not route when the template type is invalid' do
+        expect do
+          get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template', project_id: project }, format: :json)
+        end.to raise_error(ActionController::UrlGenerationError)
+      end
+
+      it 'renders 404 when the format type is invalid' do
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :html)
+
+        expect(response.status).to eq(404)
+      end
+
+      it 'renders 404 when the key is unknown' do
+        get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'unknown_template', project_id: project }, format: :json)
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'when the user is not a member of the project' do
+      before do
+        sign_in(user)
+      end
+
+      include_examples 'renders 404 when requesting an issue template'
+      include_examples 'renders 404 when requesting a merge request template'
+      include_examples 'renders 404 when params are invalid'
+    end
+
+    context 'when user is a member of the project' do
+      before do
+        project.add_developer(user)
+        sign_in(user)
+      end
+
+      include_examples 'renders issue templates as json'
+      include_examples 'renders merge request templates as json'
+      include_examples 'renders 404 when params are invalid'
+    end
+
+    context 'when user is a guest of the project' do
+      before do
+        project.add_guest(user)
+        sign_in(user)
+      end
+
+      include_examples 'renders issue templates as json'
+      include_examples 'renders 404 when requesting a merge request template'
+      include_examples 'renders 404 when params are invalid'
     end
   end
 end

spec/controllers/snippets/notes_controller_spec.rb

@@ -119,6 +119,119 @@ describe Snippets::NotesController do
     end
   end
 
+  describe 'POST create' do
+    context 'when a snippet is public' do
+      let(:request_params) do
+        {
+          note: attributes_for(:note_on_personal_snippet, noteable: public_snippet),
+          snippet_id: public_snippet.id
+        }
+      end
+
+      before do
+        sign_in user
+      end
+
+      it 'returns status 302' do
+        post :create, params: request_params
+
+        expect(response).to have_gitlab_http_status(302)
+      end
+
+      it 'creates the note' do
+        expect { post :create, params: request_params }.to change { Note.count }.by(1)
+      end
+    end
+
+    context 'when a snippet is internal' do
+      let(:request_params) do
+        {
+          note: attributes_for(:note_on_personal_snippet, noteable: internal_snippet),
+          snippet_id: internal_snippet.id
+        }
+      end
+
+      before do
+        sign_in user
+      end
+
+      it 'returns status 302' do
+        post :create, params: request_params
+
+        expect(response).to have_gitlab_http_status(302)
+      end
+
+      it 'creates the note' do
+        expect { post :create, params: request_params }.to change { Note.count }.by(1)
+      end
+    end
+
+    context 'when a snippet is private' do
+      let(:request_params) do
+        {
+          note: attributes_for(:note_on_personal_snippet, noteable: private_snippet),
+          snippet_id: private_snippet.id
+        }
+      end
+
+      before do
+        sign_in user
+      end
+
+      context 'when user is not the author' do
+        before do
+          sign_in(user)
+        end
+
+        it 'returns status 404' do
+          post :create, params: request_params
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+
+        it 'does not create the note' do
+          expect { post :create, params: request_params }.not_to change { Note.count }
+        end
+
+        context 'when user sends a snippet_id for a public snippet' do
+          let(:request_params) do
+            {
+              note: attributes_for(:note_on_personal_snippet, noteable: private_snippet),
+              snippet_id: public_snippet.id
+            }
+          end
+
+          it 'returns status 302' do
+            post :create, params: request_params
+
+            expect(response).to have_gitlab_http_status(302)
+          end
+
+          it 'creates the note on the public snippet' do
+            expect { post :create, params: request_params }.to change { Note.count }.by(1)
+            expect(Note.last.noteable).to eq public_snippet
+          end
+        end
+      end
+
+      context 'when user is the author' do
+        before do
+          sign_in(private_snippet.author)
+        end
+
+        it 'returns status 302' do
+          post :create, params: request_params
+
+          expect(response).to have_gitlab_http_status(302)
+        end
+
+        it 'creates the note' do
+          expect { post :create, params: request_params }.to change { Note.count }.by(1)
+        end
+      end
+    end
+  end
+
   describe 'DELETE destroy' do
     let(:request_params) do
       {

spec/controllers/snippets_controller_spec.rb

@@ -209,8 +209,8 @@ describe SnippetsController do
     context 'when the snippet description contains a file' do
       include FileMoverHelpers
 
-      let(:picture_file) { '/-/system/temp/secret56/picture.jpg' }
-      let(:text_file) { '/-/system/temp/secret78/text.txt' }
+      let(:picture_file) { "/-/system/user/#{user.id}/secret56/picture.jpg" }
+      let(:text_file) { "/-/system/user/#{user.id}/secret78/text.txt" }
       let(:description) do
         "Description with picture: ![picture](/uploads#{picture_file}) and "\
         "text: [text.txt](/uploads#{text_file})"

spec/controllers/uploads_controller_spec.rb

@@ -24,121 +24,160 @@ describe UploadsController do
   let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) }
 
   describe 'POST create' do
-    let(:model)   { 'personal_snippet' }
-    let(:snippet) { create(:personal_snippet, :public) }
     let(:jpg)     { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') }
     let(:txt)     { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') }
 
-    context 'when a user does not have permissions to upload a file' do
-      it "returns 401 when the user is not logged in" do
-        post :create, params: { model: model, id: snippet.id }, format: :json
+    context 'snippet uploads' do
+      let(:model)   { 'personal_snippet' }
+      let(:snippet) { create(:personal_snippet, :public) }
+
+      context 'when a user does not have permissions to upload a file' do
+        it "returns 401 when the user is not logged in" do
+          post :create, params: { model: model, id: snippet.id }, format: :json
+
+          expect(response).to have_gitlab_http_status(401)
+        end
+
+        it "returns 404 when user can't comment on a snippet" do
+          private_snippet = create(:personal_snippet, :private)
+
+          sign_in(user)
+          post :create, params: { model: model, id: private_snippet.id }, format: :json
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'when a user is logged in' do
+        before do
+          sign_in(user)
+        end
+
+        it "returns an error without file" do
+          post :create, params: { model: model, id: snippet.id }, format: :json
+
+          expect(response).to have_gitlab_http_status(422)
+        end
+
+        it "returns an error with invalid model" do
+          expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json }
+            .to raise_error(ActionController::UrlGenerationError)
+        end
+
+        it "returns 404 status when object not found" do
+          post :create, params: { model: model, id: 9999 }, format: :json
+
+          expect(response).to have_gitlab_http_status(404)
+        end
+
+        context 'with valid image' do
+          before do
+            post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json
+          end
+
+          it 'returns a content with original filename, new link, and correct type.' do
+            expect(response.body).to match '\"alt\":\"rails_sample\"'
+            expect(response.body).to match "\"url\":\"/uploads"
+          end
+
+          it 'creates a corresponding Upload record' do
+            upload = Upload.last
+
+            aggregate_failures do
+              expect(upload).to exist
+              expect(upload.model).to eq snippet
+            end
+          end
+        end
+
+        context 'with valid non-image file' do
+          before do
+            post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json
+          end
+
+          it 'returns a content with original filename, new link, and correct type.' do
+            expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
+            expect(response.body).to match "\"url\":\"/uploads"
+          end
+
+          it 'creates a corresponding Upload record' do
+            upload = Upload.last
+
+            aggregate_failures do
+              expect(upload).to exist
+              expect(upload.model).to eq snippet
+            end
+          end
+        end
+      end
+    end
+
+    context 'user uploads' do
+      let(:model) { 'user' }
+
+      it 'returns 401 when the user has no access' do
+        post :create, params: { model: 'user', id: user.id }, format: :json
 
         expect(response).to have_gitlab_http_status(401)
       end
 
-      it "returns 404 when user can't comment on a snippet" do
-        private_snippet = create(:personal_snippet, :private)
-
-        sign_in(user)
-        post :create, params: { model: model, id: private_snippet.id }, format: :json
-
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
-
-    context 'when a user is logged in' do
-      before do
-        sign_in(user)
-      end
-
-      it "returns an error without file" do
-        post :create, params: { model: model, id: snippet.id }, format: :json
-
-        expect(response).to have_gitlab_http_status(422)
-      end
-
-      it "returns an error with invalid model" do
-        expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json }
-        .to raise_error(ActionController::UrlGenerationError)
-      end
-
-      it "returns 404 status when object not found" do
-        post :create, params: { model: model, id: 9999 }, format: :json
-
-        expect(response).to have_gitlab_http_status(404)
-      end
-
-      context 'with valid image' do
+      context 'when user is logged in' do
         before do
-          post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json
+          sign_in(user)
         end
 
-        it 'returns a content with original filename, new link, and correct type.' do
-          expect(response.body).to match '\"alt\":\"rails_sample\"'
-          expect(response.body).to match "\"url\":\"/uploads"
-        end
-
-        it 'creates a corresponding Upload record' do
-          upload = Upload.last
-
-          aggregate_failures do
-            expect(upload).to exist
-            expect(upload.model).to eq snippet
-          end
-        end
-      end
-
-      context 'with valid non-image file' do
-        before do
-          post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json
-        end
-
-        it 'returns a content with original filename, new link, and correct type.' do
-          expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
-          expect(response.body).to match "\"url\":\"/uploads"
-        end
-
-        it 'creates a corresponding Upload record' do
-          upload = Upload.last
-
-          aggregate_failures do
-            expect(upload).to exist
-            expect(upload.model).to eq snippet
-          end
-        end
-      end
-
-      context 'temporal with valid image' do
         subject do
-          post :create, params: { model: 'personal_snippet', file: jpg }, format: :json
+          post :create, params: { model: model, id: user.id, file: jpg }, format: :json
         end
 
         it 'returns a content with original filename, new link, and correct type.' do
           subject
 
           expect(response.body).to match '\"alt\":\"rails_sample\"'
-          expect(response.body).to match "\"url\":\"/uploads/-/system/temp"
+          expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/"
         end
 
-        it 'does not create an Upload record' do
-          expect { subject }.not_to change { Upload.count }
-        end
-      end
+        it 'creates a corresponding Upload record' do
+          expect { subject }.to change { Upload.count }
 
-      context 'temporal with valid non-image file' do
-        subject do
-          post :create, params: { model: 'personal_snippet', file: txt }, format: :json
+          upload = Upload.last
+
+          aggregate_failures do
+            expect(upload).to exist
+            expect(upload.model).to eq user
+          end
         end
 
-        it 'returns a content with original filename, new link, and correct type.' do
-          subject
+        context 'with valid non-image file' do
+          subject do
+            post :create, params: { model: model, id: user.id, file: txt }, format: :json
+          end
 
-          expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
-          expect(response.body).to match "\"url\":\"/uploads/-/system/temp"
+          it 'returns a content with original filename, new link, and correct type.' do
+            subject
+
+            expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
+            expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/"
+          end
+
+          it 'creates a corresponding Upload record' do
+            expect { subject }.to change { Upload.count }
+
+            upload = Upload.last
+
+            aggregate_failures do
+              expect(upload).to exist
+              expect(upload.model).to eq user
+            end
+          end
         end
 
-        it 'does not create an Upload record' do
-          expect { subject }.not_to change { Upload.count }
+        it 'returns 404 when given user is not the logged in one' do
+          another_user = create(:user)
+
+          post :create, params: { model: model, id: another_user.id, file: txt }, format: :json
+
+          expect(response).to have_gitlab_http_status(404)
         end
       end
     end

spec/features/snippets/user_creates_snippet_spec.rb

@@ -41,7 +41,7 @@ describe 'User creates snippet', :js do
       expect(page).to have_content('My Snippet')
 
       link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
-      expect(link).to match(%r{/uploads/-/system/temp/\h{32}/banana_sample\.gif\z})
+      expect(link).to match(%r{/uploads/-/system/user/#{user.id}/\h{32}/banana_sample\.gif\z})
 
       reqs = inspect_requests { visit(link) }
       expect(reqs.first.status_code).to eq(200)

spec/graphql/types/label_type_spec.rb

@@ -7,4 +7,6 @@ describe GitlabSchema.types['Label'] do
 
     is_expected.to have_graphql_fields(*expected_fields)
   end
+
+  it { is_expected.to require_graphql_authorizations(:read_label) }
 end

spec/graphql/types/metadata_type_spec.rb

@@ -2,4 +2,5 @@ require 'spec_helper'
 
 describe GitlabSchema.types['Metadata'] do
   it { expect(described_class.graphql_name).to eq('Metadata') }
+  it { is_expected.to require_graphql_authorizations(:read_instance_metadata) }
 end

spec/graphql/types/namespace_type_spec.rb

@@ -13,4 +13,6 @@ describe GitlabSchema.types['Namespace'] do
 
     is_expected.to have_graphql_fields(*expected_fields)
   end
+
+  it { is_expected.to require_graphql_authorizations(:read_namespace) }
 end

spec/graphql/types/query_type_spec.rb

@@ -34,9 +34,5 @@ describe GitlabSchema.types['Query'] do
       is_expected.to have_graphql_type(Types::MetadataType)
       is_expected.to have_graphql_resolver(Resolvers::MetadataResolver)
     end
-
-    it 'authorizes with read_instance_metadata' do
-      is_expected.to require_graphql_authorizations(:read_instance_metadata)
-    end
   end
 end

spec/lib/banzai/filter/relative_link_filter_spec.rb

@@ -83,6 +83,11 @@ describe Banzai::Filter::RelativeLinkFilter do
     expect { filter(act) }.not_to raise_error
   end
 
+  it 'does not explode with an escaped null byte' do
+    act = link("/%00")
+    expect { filter(act) }.not_to raise_error
+  end
+
   it 'does not raise an exception with a space in the path' do
     act = link("/uploads/d18213acd3732630991986120e167e3d/Landscape_8.jpg  \nBut here's some more unexpected text :smile:)")
     expect { filter(act) }.not_to raise_error

spec/lib/gitlab/ci/config_spec.rb

@@ -90,6 +90,27 @@ describe Gitlab::Ci::Config do
       end
     end
 
+    context 'when yml is too big' do
+      let(:yml) do
+        <<~YAML
+          --- &1
+          - hi
+          - *1
+        YAML
+      end
+
+      describe '.new' do
+        it 'raises error' do
+          expect(Gitlab::Sentry).to receive(:track_exception)
+
+          expect { config }.to raise_error(
+            described_class::ConfigError,
+            /The parsed YAML is too big/
+          )
+        end
+      end
+    end
+
     context 'when config logic is incorrect' do
       let(:yml) { 'before_script: "ls"' }
 

spec/lib/gitlab/config/loader/yaml_spec.rb

@@ -8,7 +8,7 @@ describe Gitlab::Config::Loader::Yaml do
 
     describe '#valid?' do
       it 'returns true' do
-        expect(loader.valid?).to be true
+        expect(loader).to be_valid
       end
     end
 
@@ -24,7 +24,7 @@ describe Gitlab::Config::Loader::Yaml do
 
     describe '#valid?' do
       it 'returns false' do
-        expect(loader.valid?).to be false
+        expect(loader).not_to be_valid
       end
     end
 
@@ -43,7 +43,10 @@ describe Gitlab::Config::Loader::Yaml do
 
     describe '#initialize' do
       it 'raises FormatError' do
-        expect { loader }.to raise_error(Gitlab::Config::Loader::FormatError, 'Unknown alias: bad_alias')
+        expect { loader }.to raise_error(
+          Gitlab::Config::Loader::FormatError,
+          'Unknown alias: bad_alias'
+        )
       end
     end
   end
@@ -53,7 +56,68 @@ describe Gitlab::Config::Loader::Yaml do
 
     describe '#valid?' do
       it 'returns false' do
-        expect(loader.valid?).to be false
+        expect(loader).not_to be_valid
+      end
+    end
+  end
+
+  # Prevent Billion Laughs attack: https://gitlab.com/gitlab-org/gitlab-ce/issues/56018
+  context 'when yaml size is too large' do
+    let(:yml) do
+      <<~YAML
+        a: &a ["lol","lol","lol","lol","lol","lol","lol","lol","lol"]
+        b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a]
+        c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b]
+        d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c]
+        e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d]
+        f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e]
+        g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f]
+        h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g]
+        i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h]
+      YAML
+    end
+
+    describe '#valid?' do
+      it 'returns false' do
+        expect(loader).not_to be_valid
+      end
+
+      it 'returns true if "ci_yaml_limit_size" feature flag is disabled' do
+        stub_feature_flags(ci_yaml_limit_size: false)
+
+        expect(loader).to be_valid
+      end
+    end
+
+    describe '#load!' do
+      it 'raises FormatError' do
+        expect { loader.load! }.to raise_error(
+          Gitlab::Config::Loader::FormatError,
+          'The parsed YAML is too big'
+        )
+      end
+    end
+  end
+
+  # Prevent Billion Laughs attack: https://gitlab.com/gitlab-org/gitlab-ce/issues/56018
+  context 'when yaml has cyclic data structure' do
+    let(:yml) do
+      <<~YAML
+        --- &1
+        - hi
+        - *1
+      YAML
+    end
+
+    describe '#valid?' do
+      it 'returns false' do
+        expect(loader.valid?).to be(false)
+      end
+    end
+
+    describe '#load!' do
+      it 'raises FormatError' do
+        expect { loader.load! }.to raise_error(Gitlab::Config::Loader::FormatError, 'The parsed YAML is too big')
       end
     end
   end

spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb

@@ -7,35 +7,39 @@ require 'spec_helper'
 describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
   def type(type_authorizations = [])
     Class.new(Types::BaseObject) do
-      graphql_name "TestType"
+      graphql_name 'TestType'
 
       authorize type_authorizations
     end
   end
 
-  def type_with_field(field_type, field_authorizations = [], resolved_value = "Resolved value")
+  def type_with_field(field_type, field_authorizations = [], resolved_value = 'Resolved value', **options)
     Class.new(Types::BaseObject) do
-      graphql_name "TestTypeWithField"
-      field :test_field, field_type, null: true, authorize: field_authorizations, resolve: -> (_, _, _) { resolved_value}
+      graphql_name 'TestTypeWithField'
+      options.reverse_merge!(null: true)
+      field :test_field, field_type,
+            authorize: field_authorizations,
+            resolve: -> (_, _, _) { resolved_value },
+            **options
     end
   end
 
   let(:current_user) { double(:current_user) }
   subject(:service) { described_class.new(field) }
 
-  describe "#authorized_resolve" do
-    let(:presented_object) { double("presented object") }
-    let(:presented_type) { double("parent type", object: presented_object) }
+  describe '#authorized_resolve' do
+    let(:presented_object) { double('presented object') }
+    let(:presented_type) { double('parent type', object: presented_object) }
     subject(:resolved) { service.authorized_resolve.call(presented_type, {}, { current_user: current_user }) }
 
-    context "scalar types" do
-      shared_examples "checking permissions on the presented object" do
-        it "checks the abilities on the object being presented and returns the value" do
+    context 'scalar types' do
+      shared_examples 'checking permissions on the presented object' do
+        it 'checks the abilities on the object being presented and returns the value' do
           expected_permissions.each do |permission|
             spy_ability_check_for(permission, presented_object, passed: true)
           end
 
-          expect(resolved).to eq("Resolved value")
+          expect(resolved).to eq('Resolved value')
         end
 
         it "returns nil if the value wasn't authorized" do
@@ -45,61 +49,71 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
         end
       end
 
-      context "when the field is a built-in scalar type" do
-        let(:field) { type_with_field(GraphQL::STRING_TYPE, :read_field).fields["testField"].to_graphql }
+      context 'when the field is a built-in scalar type' do
+        let(:field) { type_with_field(GraphQL::STRING_TYPE, :read_field).fields['testField'].to_graphql }
         let(:expected_permissions) { [:read_field] }
 
-        it_behaves_like "checking permissions on the presented object"
+        it_behaves_like 'checking permissions on the presented object'
       end
 
-      context "when the field is a list of scalar types" do
-        let(:field) { type_with_field([GraphQL::STRING_TYPE], :read_field).fields["testField"].to_graphql }
+      context 'when the field is a list of scalar types' do
+        let(:field) { type_with_field([GraphQL::STRING_TYPE], :read_field).fields['testField'].to_graphql }
         let(:expected_permissions) { [:read_field] }
 
-        it_behaves_like "checking permissions on the presented object"
+        it_behaves_like 'checking permissions on the presented object'
       end
 
-      context "when the field is sub-classed scalar type" do
-        let(:field) { type_with_field(Types::TimeType, :read_field).fields["testField"].to_graphql }
+      context 'when the field is sub-classed scalar type' do
+        let(:field) { type_with_field(Types::TimeType, :read_field).fields['testField'].to_graphql }
         let(:expected_permissions) { [:read_field] }
 
-        it_behaves_like "checking permissions on the presented object"
+        it_behaves_like 'checking permissions on the presented object'
       end
 
-      context "when the field is a list of sub-classed scalar types" do
-        let(:field) { type_with_field([Types::TimeType], :read_field).fields["testField"].to_graphql }
+      context 'when the field is a list of sub-classed scalar types' do
+        let(:field) { type_with_field([Types::TimeType], :read_field).fields['testField'].to_graphql }
         let(:expected_permissions) { [:read_field] }
 
-        it_behaves_like "checking permissions on the presented object"
+        it_behaves_like 'checking permissions on the presented object'
       end
     end
 
-    context "when the field is a specific type" do
+    context 'when the field is a specific type' do
       let(:custom_type) { type(:read_type) }
-      let(:object_in_field) { double("presented in field") }
-      let(:field) { type_with_field(custom_type, :read_field, object_in_field).fields["testField"].to_graphql }
+      let(:object_in_field) { double('presented in field') }
+      let(:field) { type_with_field(custom_type, :read_field, object_in_field).fields['testField'].to_graphql }
 
-      it "checks both field & type permissions" do
+      it 'checks both field & type permissions' do
         spy_ability_check_for(:read_field, object_in_field, passed: true)
         spy_ability_check_for(:read_type, object_in_field, passed: true)
 
         expect(resolved).to eq(object_in_field)
       end
 
-      it "returns nil if viewing was not allowed" do
+      it 'returns nil if viewing was not allowed' do
         spy_ability_check_for(:read_field, object_in_field, passed: false)
         spy_ability_check_for(:read_type, object_in_field, passed: true)
 
         expect(resolved).to be_nil
       end
 
-      context "when the field is a list" do
-        let(:object_1) { double("presented in field 1") }
-        let(:object_2) { double("presented in field 2") }
-        let(:presented_types) { [double(object: object_1), double(object: object_2)] }
-        let(:field) { type_with_field([custom_type], :read_field, presented_types).fields["testField"].to_graphql }
+      context 'when the field is not nullable' do
+        let(:field) { type_with_field(custom_type, [], object_in_field, null: false).fields['testField'].to_graphql }
 
-        it "checks all permissions" do
+        it 'returns nil when viewing is not allowed' do
+          spy_ability_check_for(:read_type, object_in_field, passed: false)
+
+          expect(resolved).to be_nil
+        end
+      end
+
+      context 'when the field is a list' do
+        let(:object_1) { double('presented in field 1') }
+        let(:object_2) { double('presented in field 2') }
+        let(:presented_types) { [double(object: object_1), double(object: object_2)] }
+        let(:field) { type_with_field([custom_type], :read_field, presented_types).fields['testField'].to_graphql }
+
+        it 'checks all permissions' do
           allow(Ability).to receive(:allowed?) { true }
 
           spy_ability_check_for(:read_field, object_1, passed: true)
@@ -110,7 +124,7 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do
           expect(resolved).to eq(presented_types)
         end
 
-        it "filters out objects that the user cannot see" do
+        it 'filters out objects that the user cannot see' do
           allow(Ability).to receive(:allowed?) { true }
 
           spy_ability_check_for(:read_type, object_1, passed: false)

spec/lib/gitlab/issuable_metadata_spec.rb

@@ -7,11 +7,11 @@ describe Gitlab::IssuableMetadata do
   subject { Class.new { include Gitlab::IssuableMetadata }.new }
 
   it 'returns an empty Hash if an empty collection is provided' do
-    expect(subject.issuable_meta_data(Issue.none, 'Issue')).to eq({})
+    expect(subject.issuable_meta_data(Issue.none, 'Issue', user)).to eq({})
   end
 
   it 'raises an error when given a collection with no limit' do
-    expect { subject.issuable_meta_data(Issue.all, 'Issue') }.to raise_error(/must have a limit/)
+    expect { subject.issuable_meta_data(Issue.all, 'Issue', user) }.to raise_error(/must have a limit/)
   end
 
   context 'issues' do
@@ -23,7 +23,7 @@ describe Gitlab::IssuableMetadata do
     let!(:closing_issues) { create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) }
 
     it 'aggregates stats on issues' do
-      data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue')
+      data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue', user)
 
       expect(data.count).to eq(2)
       expect(data[issue.id].upvotes).to eq(1)
@@ -46,7 +46,7 @@ describe Gitlab::IssuableMetadata do
     let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
 
     it 'aggregates stats on merge requests' do
-      data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest')
+      data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest', user)
 
       expect(data.count).to eq(2)
       expect(data[merge_request.id].upvotes).to eq(1)

spec/lib/gitlab/utils/deep_size_spec.rb

@@ -0,0 +1,43 @@
+require 'spec_helper'
+
+describe Gitlab::Utils::DeepSize do
+  let(:data) do
+    {
+      a: [1, 2, 3],
+      b: {
+        c: [4, 5],
+        d: [
+          { e: [[6], [7]] }
+        ]
+      }
+    }
+  end
+
+  let(:max_size) { 1.kilobyte }
+  let(:max_depth) { 10 }
+  let(:deep_size) { described_class.new(data, max_size: max_size, max_depth: max_depth) }
+
+  describe '#evaluate' do
+    context 'when data within size and depth limits' do
+      it 'returns true' do
+        expect(deep_size).to be_valid
+      end
+    end
+
+    context 'when data not within size limit' do
+      let(:max_size) { 200.bytes }
+
+      it 'returns false' do
+        expect(deep_size).not_to be_valid
+      end
+    end
+
+    context 'when data not within depth limit' do
+      let(:max_depth) { 2 }
+
+      it 'returns false' do
+        expect(deep_size).not_to be_valid
+      end
+    end
+  end
+end

spec/requests/api/graphql/namespace/projects_spec.rb

@@ -58,9 +58,7 @@ describe 'getting projects', :nested_groups do
       it 'finds only public projects' do
         post_graphql(query, current_user: nil)
 
-        expect(graphql_data['namespace']['projects']['edges'].size).to eq(1)
-        project = graphql_data['namespace']['projects']['edges'][0]['node']
-        expect(project['id']).to eq(public_project.to_global_id.to_s)
+        expect(graphql_data['namespace']).to be_nil
       end
     end
   end

spec/requests/api/graphql/project/repository_spec.rb

@@ -34,4 +34,28 @@ describe 'getting a repository in a project' do
       expect(graphql_data['project']).to be(nil)
     end
   end
+
+  context 'when the repository is only accessible to members' do
+    let(:project) do
+      create(:project, :public, :repository, repository_access_level: ProjectFeature::PRIVATE)
+    end
+
+    it 'returns a repository for the owner' do
+      post_graphql(query, current_user: current_user)
+
+      expect(graphql_data['project']['repository']).not_to be_nil
+    end
+
+    it 'returns nil for the repository for other users' do
+      post_graphql(query, current_user: create(:user))
+
+      expect(graphql_data['project']['repository']).to be_nil
+    end
+
+    it 'returns nil for the repository for other users' do
+      post_graphql(query, current_user: nil)
+
+      expect(graphql_data['project']['repository']).to be_nil
+    end
+  end
 end

spec/requests/api/issues/get_group_issues_spec.rb

@@ -23,7 +23,11 @@ describe API::Issues do
 
   describe 'GET /groups/:id/issues' do
     let!(:group)            { create(:group) }
-    let!(:group_project)    { create(:project, :public, creator_id: user.id, namespace: group) }
+    let!(:group_project)    { create(:project, :public, :repository, creator_id: user.id, namespace: group) }
+    let!(:private_mrs_project) do
+      create(:project, :public, :repository, creator_id: user.id, namespace: group, merge_requests_access_level: ProjectFeature::PRIVATE)
+    end
+
     let!(:group_closed_issue) do
       create :closed_issue,
         author: user,
@@ -234,6 +238,30 @@ describe API::Issues do
             it_behaves_like 'group issues statistics'
           end
         end
+
+        context "when returns issue merge_requests_count for different access levels" do
+          let!(:merge_request1) do
+            create(:merge_request,
+                   :simple,
+                   author: user,
+                   source_project: private_mrs_project,
+                   target_project: private_mrs_project,
+                   description: "closes #{group_issue.to_reference(private_mrs_project)}")
+          end
+          let!(:merge_request2) do
+            create(:merge_request,
+                   :simple,
+                   author: user,
+                   source_project: group_project,
+                   target_project: group_project,
+                   description: "closes #{group_issue.to_reference}")
+          end
+
+          it_behaves_like 'accessible merge requests count' do
+            let(:api_url) { base_url }
+            let(:target_issue) { group_issue }
+          end
+        end
       end
     end
 

spec/requests/api/issues/get_project_issues_spec.rb

@@ -4,8 +4,9 @@ require 'spec_helper'
 
 describe API::Issues do
   set(:user) { create(:user) }
-  set(:project) do
-    create(:project, :public, creator_id: user.id, namespace: user.namespace)
+  set(:project) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) }
+  set(:private_mrs_project) do
+    create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE)
   end
 
   let(:user2)       { create(:user) }
@@ -60,9 +61,28 @@ describe API::Issues do
   let(:no_milestone_title) { 'None' }
   let(:any_milestone_title) { 'Any' }
 
+  let!(:merge_request1) do
+    create(:merge_request,
+           :simple,
+           author: user,
+           source_project: project,
+           target_project: project,
+           description: "closes #{issue.to_reference}")
+  end
+  let!(:merge_request2) do
+    create(:merge_request,
+           :simple,
+           author: user,
+           source_project: private_mrs_project,
+           target_project: private_mrs_project,
+           description: "closes #{issue.to_reference(private_mrs_project)}")
+  end
+
   before(:all) do
     project.add_reporter(user)
     project.add_guest(guest)
+    private_mrs_project.add_reporter(user)
+    private_mrs_project.add_guest(guest)
   end
 
   before do
@@ -257,6 +277,11 @@ describe API::Issues do
       expect_paginated_array_response(issue.id)
     end
 
+    it_behaves_like 'accessible merge requests count' do
+      let(:api_url) { "/projects/#{project.id}/issues" }
+      let(:target_issue) { issue }
+    end
+
     context 'with labeled issues' do
       let(:label_b) { create(:label, title: 'foo', project: project) }
       let(:label_c) { create(:label, title: 'bar', project: project) }
@@ -636,34 +661,26 @@ describe API::Issues do
         expect(json_response['iid']).to eq(confidential_issue.iid)
       end
     end
+
+    it_behaves_like 'accessible merge requests count' do
+      let(:api_url) { "/projects/#{project.id}/issues/#{issue.iid}" }
+      let(:target_issue) { issue }
+    end
   end
 
   describe 'GET :id/issues/:issue_iid/closed_by' do
-    let(:merge_request) do
-      create(:merge_request,
-        :simple,
-        author: user,
-        source_project: project,
-        target_project: project,
-        description: "closes #{issue.to_reference}")
-    end
-
-    before do
-      create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request)
-    end
-
     context 'when unauthenticated' do
       it 'return public project issues' do
         get api("/projects/#{project.id}/issues/#{issue.iid}/closed_by")
 
-        expect_paginated_array_response(merge_request.id)
+        expect_paginated_array_response(merge_request1.id)
       end
     end
 
     it 'returns merge requests that will close issue on merge' do
       get api("/projects/#{project.id}/issues/#{issue.iid}/closed_by", user)
 
-      expect_paginated_array_response(merge_request.id)
+      expect_paginated_array_response(merge_request1.id)
     end
 
     context 'when no merge requests will close issue' do
@@ -721,13 +738,6 @@ describe API::Issues do
     end
 
     it 'returns merge requests that mentioned a issue' do
-      create(:merge_request,
-        :simple,
-        author: user,
-        source_project: project,
-        target_project: project,
-        description: 'Some description')
-
       get_related_merge_requests(project.id, issue.iid, user)
 
       expect_paginated_array_response(related_mr.id)

spec/requests/api/issues/issues_spec.rb

@@ -4,8 +4,9 @@ require 'spec_helper'
 
 describe API::Issues do
   set(:user) { create(:user) }
-  set(:project) do
-    create(:project, :public, creator_id: user.id, namespace: user.namespace)
+  set(:project) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) }
+  set(:private_mrs_project) do
+    create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE)
   end
 
   let(:user2)       { create(:user) }
@@ -63,6 +64,8 @@ describe API::Issues do
   before(:all) do
     project.add_reporter(user)
     project.add_guest(guest)
+    private_mrs_project.add_reporter(user)
+    private_mrs_project.add_guest(guest)
   end
 
   before do
@@ -725,6 +728,30 @@ describe API::Issues do
         end
       end
     end
+
+    context "when returns issue merge_requests_count for different access levels" do
+      let!(:merge_request1) do
+        create(:merge_request,
+               :simple,
+               author: user,
+               source_project: private_mrs_project,
+               target_project: private_mrs_project,
+               description: "closes #{issue.to_reference(private_mrs_project)}")
+      end
+      let!(:merge_request2) do
+        create(:merge_request,
+               :simple,
+               author: user,
+               source_project: project,
+               target_project: project,
+               description: "closes #{issue.to_reference}")
+      end
+
+      it_behaves_like 'accessible merge requests count' do
+        let(:api_url) { "/issues" }
+        let(:target_issue) { issue }
+      end
+    end
   end
 
   describe 'DELETE /projects/:id/issues/:issue_iid' do

spec/requests/api/merge_requests_spec.rb

@@ -834,6 +834,31 @@ describe API::MergeRequests do
       end
     end
 
+    context 'head_pipeline' do
+      before do
+        merge_request.update(head_pipeline: create(:ci_pipeline))
+        merge_request.project.project_feature.update(builds_access_level: 10)
+      end
+
+      context 'when user can read the pipeline' do
+        it 'exposes pipeline information' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
+
+          expect(json_response).to include('head_pipeline')
+        end
+      end
+
+      context 'when user can not read the pipeline' do
+        let(:guest) { create(:user) }
+
+        it 'does not expose pipeline information' do
+          get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", guest)
+
+          expect(json_response).not_to include('head_pipeline')
+        end
+      end
+    end
+
     it 'returns the commits behind the target branch when include_diverged_commits_count is present' do
       allow_any_instance_of(merge_request.class).to receive(:diverged_commits_count).and_return(1)
 

spec/routing/project_routing_spec.rb

@@ -693,4 +693,24 @@ describe 'project routing' do
 
     it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/settings/repository", "/gitlab/gitlabhq/-/settings/repository"
   end
+
+  describe Projects::TemplatesController, 'routing' do
+    describe '#show' do
+      def show_with_template_type(template_type)
+        "/gitlab/gitlabhq/templates/#{template_type}/template_name"
+      end
+
+      it 'routes when :template_type is `merge_request`' do
+        expect(get(show_with_template_type('merge_request'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'merge_request', key: 'template_name', format: 'json')
+      end
+
+      it 'routes when :template_type is `issue`' do
+        expect(get(show_with_template_type('issue'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'issue', key: 'template_name', format: 'json')
+      end
+
+      it 'routes to application#route_not_found when :template_type is unknown' do
+        expect(get(show_with_template_type('invalid'))).to route_to('application#route_not_found', unmatched_route: 'gitlab/gitlabhq/templates/invalid/template_name')
+      end
+    end
+  end
 end

spec/routing/uploads_routing_spec.rb

@@ -12,10 +12,19 @@ describe 'Uploads', 'routing' do
     )
   end
 
-  it 'does not allow creating uploads for other models' do
-    UploadsController::MODEL_CLASSES.keys.compact.each do |model|
-      next if model == 'personal_snippet'
+  it 'allows creating uploads for users' do
+    expect(post('/uploads/user?id=1')).to route_to(
+      controller: 'uploads',
+      action: 'create',
+      model: 'user',
+      id: '1'
+    )
+  end
 
+  it 'does not allow creating uploads for other models' do
+    unroutable_models = UploadsController::MODEL_CLASSES.keys.compact - %w(personal_snippet user)
+
+    unroutable_models.each do |model|
       expect(post("/uploads/#{model}?id=1")).not_to be_routable
     end
   end

spec/rubocop/cop/graphql/authorize_types_spec.rb

@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../../rubocop/cop/graphql/authorize_types'
+
+describe RuboCop::Cop::Graphql::AuthorizeTypes do
+  include RuboCop::RSpec::ExpectOffense
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'when in a type folder' do
+    before do
+      allow(cop).to receive(:in_type?).and_return(true)
+    end
+
+    it 'adds an offense when there is no authorize call' do
+      inspect_source(<<~TYPE)
+        module Types
+          class AType < BaseObject
+            field :a_thing
+            field :another_thing
+          end
+        end
+      TYPE
+
+      expect(cop.offenses.size).to eq 1
+    end
+
+    it 'does not add an offense for classes that have an authorize call' do
+      expect_no_offenses(<<~TYPE.strip)
+        module Types
+          class AType < BaseObject
+            graphql_name 'ATypeName'
+
+            authorize :an_ability, :second_ability
+
+            field :a_thing
+          end
+        end
+      TYPE
+    end
+
+    it 'does not add an offense for classes that only have an authorize call' do
+      expect_no_offenses(<<~TYPE.strip)
+        module Types
+          class AType < SuperClassWithFields
+            authorize :an_ability
+          end
+        end
+      TYPE
+    end
+
+    it 'does not add an offense for base types' do
+      expect_no_offenses(<<~TYPE)
+        module Types
+          class AType < BaseEnum
+            field :a_thing
+          end
+        end
+      TYPE
+    end
+  end
+end

spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb

@@ -0,0 +1,37 @@
+def get_issue
+  json_response.is_a?(Array) ? json_response.detect {|issue| issue['id'] == target_issue.id} : json_response
+end
+
+shared_examples 'accessible merge requests count' do
+  it 'returns anonymous accessible merge requests count' do
+    get api(api_url), params: { scope: 'all' }
+
+    issue = get_issue
+    expect(issue).not_to be_nil
+    expect(issue['merge_requests_count']).to eq(1)
+  end
+
+  it 'returns guest accessible merge requests count' do
+    get api(api_url, guest), params: { scope: 'all' }
+
+    issue = get_issue
+    expect(issue).not_to be_nil
+    expect(issue['merge_requests_count']).to eq(1)
+  end
+
+  it 'returns reporter accessible merge requests count' do
+    get api(api_url, user), params: { scope: 'all' }
+
+    issue = get_issue
+    expect(issue).not_to be_nil
+    expect(issue['merge_requests_count']).to eq(2)
+  end
+
+  it 'returns admin accessible merge requests count' do
+    get api(api_url, admin), params: { scope: 'all' }
+
+    issue = get_issue
+    expect(issue).not_to be_nil
+    expect(issue['merge_requests_count']).to eq(2)
+  end
+end

spec/uploaders/file_mover_spec.rb

@@ -3,79 +3,140 @@ require 'spec_helper'
 describe FileMover do
   include FileMoverHelpers
 
+  let(:user) { create(:user) }
   let(:filename) { 'banana_sample.gif' }
-  let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) }
+  let(:secret) { 'secret55' }
+  let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", secret, filename) }
 
   let(:temp_description) do
     "test ![banana_sample](/#{temp_file_path}) "\
     "same ![banana_sample](/#{temp_file_path}) "
   end
-  let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) }
+  let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, secret, filename) }
   let(:snippet) { create(:personal_snippet, description: temp_description) }
 
-  subject { described_class.new(temp_file_path, snippet).execute }
+  let(:tmp_uploader) do
+    PersonalFileUploader.new(user, secret: secret)
+  end
+
+  let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') }
+  subject { described_class.new(temp_file_path, from_model: user, to_model: snippet).execute }
 
   describe '#execute' do
+    let(:tmp_upload) { tmp_uploader.upload }
+
     before do
-      expect(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path)))
-      expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
-      allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
-      allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
-
-      stub_file_mover(temp_file_path)
+      tmp_uploader.store!(file)
     end
 
-    context 'when move and field update successful' do
-      it 'updates the description correctly' do
-        subject
-
-        expect(snippet.reload.description)
-          .to eq(
-            "test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
-            "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "
-          )
-      end
-
-      it 'creates a new update record' do
-        expect { subject }.to change { Upload.count }.by(1)
-      end
-
-      it 'schedules a background migration' do
-        expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once
-
-        subject
-      end
-    end
-
-    context 'when update_markdown fails' do
+    context 'local storage' do
       before do
-        expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path))
+        allow(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path)))
+        allow(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path))
+        allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true)
+        allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10)
+
+        stub_file_mover(temp_file_path)
       end
 
-      subject { described_class.new(file_path, snippet, :non_existing_field).execute }
+      context 'when move and field update successful' do
+        it 'updates the description correctly' do
+          subject
 
-      it 'does not update the description' do
-        subject
+          expect(snippet.reload.description)
+            .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ")
+        end
 
-        expect(snippet.reload.description)
-          .to eq(
-            "test ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) "\
-            "same ![banana_sample](/uploads/-/system/temp/secret55/banana_sample.gif) "
-          )
+        it 'updates existing upload record' do
+          expect { subject }
+            .to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') }
+            .from([user.id, 'User']).to([snippet.id, 'Snippet'])
+        end
+
+        it 'schedules a background migration' do
+          expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once
+
+          subject
+        end
       end
 
-      it 'does not create a new update record' do
-        expect { subject }.not_to change { Upload.count }
+      context 'when update_markdown fails' do
+        before do
+          expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path))
+        end
+
+        subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute }
+
+        it 'does not update the description' do
+          subject
+
+          expect(snippet.reload.description)
+            .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ")
+        end
+
+        it 'does not change the upload record' do
+          expect { subject }
+            .not_to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') }
+        end
+      end
+    end
+
+    context 'when tmp uploader is not local storage' do
+      before do
+        allow(PersonalFileUploader).to receive(:object_store_enabled?) { true }
+        tmp_uploader.object_store = ObjectStorage::Store::REMOTE
+        allow_any_instance_of(PersonalFileUploader).to receive(:file_storage?) { false }
+      end
+
+      after do
+        FileUtils.rm_f(File.join('personal_snippet', snippet.id.to_s, secret, filename))
+      end
+
+      context 'when move and field update successful' do
+        it 'updates the description correctly' do
+          subject
+
+          expect(snippet.reload.description)
+            .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ")
+        end
+
+        it 'creates new target upload record an delete the old upload' do
+          expect { subject }
+            .to change { Upload.last.attributes.values_at('model_id', 'model_type') }
+            .from([user.id, 'User']).to([snippet.id, 'Snippet'])
+
+          expect(Upload.count).to eq(1)
+        end
+      end
+
+      context 'when update_markdown fails' do
+        subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute }
+
+        it 'does not update the description' do
+          subject
+
+          expect(snippet.reload.description)
+            .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\
+                   "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ")
+        end
+
+        it 'does not change the upload record' do
+          expect { subject }
+            .to change { Upload.last.attributes.values_at('model_id', 'model_type') }.from([user.id, 'User'])
+        end
       end
     end
   end
 
   context 'security' do
     context 'when relative path is involved' do
-      let(:temp_file_path) { File.join('uploads/-/system/temp', '..', 'another_subdir_of_temp') }
+      let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", '..', 'another_subdir_of_temp') }
 
       it 'does not trigger move if path is outside designated directory' do
-        stub_file_mover('uploads/-/system/another_subdir_of_temp')
+        stub_file_mover("uploads/-/system/user/#{user.id}/another_subdir_of_temp")
         expect(FileUtils).not_to receive(:move)
 
         subject

spec/validators/color_validator_spec.rb

@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ColorValidator do
+  using RSpec::Parameterized::TableSyntax
+
+  subject do
+    Class.new do
+      include ActiveModel::Model
+      include ActiveModel::Validations
+      attr_accessor :color
+      validates :color, color: true
+    end.new
+  end
+
+  where(:color, :is_valid) do
+    '#000abc'    | true
+    '#aaa'       | true
+    '#BBB'       | true
+    '#cCc'       | true
+    '#ffff'      | false
+    '#000111222' | false
+    'invalid'    | false
+    '000'        | false
+  end
+
+  with_them do
+    it 'only accepts valid colors' do
+      subject.color = color
+
+      expect(subject.valid?).to eq(is_valid)
+    end
+  end
+
+  it 'fails fast for long invalid string' do
+    subject.color = '#' + ('0' * 50_000) + 'xxx'
+
+    expect do
+      Timeout.timeout(5.seconds) { subject.valid? }
+    end.not_to raise_error
+  end
+end