Merge dev.gitlab.org@master into GitLab.com@master

CHANGELOG.md

@@ -2,6 +2,24 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
+## 12.4.1
+
+### Security (12 changes)
+
+- Standardize error response when route is missing.
+- Do not display project labels that are not visible for user accessing group labels.
+- Show cross-referenced label and milestones in issues' activities only to authorized users.
+- Analyze incoming GraphQL queries and check for recursion.
+- Disallow unprivileged users from commenting on private repository commits.
+- Don't allow maintainers of a target project to delete the source branch of a merge request from a fork.
+- Require Maintainer permission on group where project is transferred to.
+- Don't leak private members in project member autocomplete suggestions.
+- Return 404 on LFS request if project doesn't exist.
+- Mask sentry auth token in Error Tracking dashboard.
+- Fixes a Open Redirect issue in `InternalRedirect`.
+- Sanitize all wiki markup formats with GitLab sanitization pipelines.
+
+
 ## 12.4.0
 ### Security (14 changes)

app/assets/javascripts/project_find_file.js

@@ -5,6 +5,7 @@ import fuzzaldrinPlus from 'fuzzaldrin-plus';
 import axios from '~/lib/utils/axios_utils';
 import flash from '~/flash';
 import { __ } from '~/locale';
+import sanitize from 'sanitize-html';
 // highlight text(awefwbwgtc -> <b>a</b>wefw<b>b</b>wgt<b>c</b> )
 const highlighter = function(element, text, matches) {
@@ -74,7 +75,7 @@ export default class ProjectFindFile {
   findFile() {
     var result, searchText;
-    searchText = this.inputElement.val();
+    searchText = sanitize(this.inputElement.val());
     result =
       searchText.length > 0 ? fuzzaldrinPlus.filter(this.filePaths, searchText) : this.filePaths;
     return this.renderList(result, searchText);

app/controllers/application_controller.rb

@@ -17,7 +17,7 @@ class ApplicationController < ActionController::Base
   include Gitlab::Tracking::ControllerConcern
   include Gitlab::Experimentation::ControllerConcern
-  before_action :authenticate_user!
+  before_action :authenticate_user!, except: [:route_not_found]
   before_action :enforce_terms!, if: :should_enforce_terms?
   before_action :validate_user_service_ticket!
   before_action :check_password_expiration
@@ -98,7 +98,9 @@ class ApplicationController < ActionController::Base
     if current_user
       not_found
     else
-      authenticate_user!
+      store_location_for(:user, request.fullpath) unless request.xhr?
+
+      redirect_to new_user_session_path, alert: I18n.t('devise.failure.unauthenticated')
     end
   end

app/controllers/concerns/internal_redirect.rb

@@ -6,7 +6,7 @@ module InternalRedirect
   def safe_redirect_path(path)
     return unless path
     # Verify that the string starts with a `/` and a known route character.
-    return unless path =~ %r{^/[-\w].*$}
+    return unless path =~ %r{\A/[-\w].*\z}
     uri = URI(path)
     # Ignore anything path of the redirect except for the path, querystring and,

app/controllers/concerns/lfs_request.rb

@@ -34,6 +34,7 @@ module LfsRequest
   end
   def lfs_check_access!
+    return render_lfs_not_found unless project
     return if download_request? && lfs_download_access?
     return if upload_request? && lfs_upload_access?

app/finders/labels_finder.rb

@@ -51,7 +51,7 @@ class LabelsFinder < UnionFinder
       end
       label_ids << Label.where(group_id: projects.group_ids)
-      label_ids << Label.where(project_id: projects.select(:id)) unless only_group_labels?
+      label_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels?
     end
     label_ids
@@ -188,4 +188,10 @@ class LabelsFinder < UnionFinder
       groups.select { |group| authorized_to_read_labels?(group) }
     end
   end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def ids_user_can_read_labels(projects)
+    Project.where(id: projects.select(:id)).ids_with_issuables_available_for(current_user)
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
 end

app/graphql/gitlab_schema.rb

@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema
   use Gitlab::Graphql::GenericTracing
   query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
-
-  query(Types::QueryType)
-
-  default_max_page_size 100
+  query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
   max_complexity DEFAULT_MAX_COMPLEXITY
   max_depth DEFAULT_MAX_DEPTH
-  mutation(Types::MutationType)
+  query Types::QueryType
+  mutation Types::MutationType
+
+  default_max_page_size 100
   class << self
     def multiplex(queries, **kwargs)

app/helpers/markup_helper.rb

@@ -133,15 +133,7 @@ module MarkupHelper
       issuable_state_filter_enabled: true
     )
-    html =
-      case wiki_page.format
-      when :markdown
-        markdown_unsafe(text, context)
-      when :asciidoc
-        asciidoc_unsafe(text)
-      else
-        wiki_page.formatted_content.html_safe
-      end
+    html = markup_unsafe(wiki_page.path, text, context)
     prepare_for_rendering(html, context)
   end

app/models/concerns/mentionable/reference_regexes.rb

@@ -13,7 +13,9 @@ module Mentionable
     def self.other_patterns
       [
         Commit.reference_pattern,
-        MergeRequest.reference_pattern
+        MergeRequest.reference_pattern,
+        Label.reference_pattern,
+        Milestone.reference_pattern
       ]
     end

app/models/discussion.rb

@@ -16,6 +16,7 @@ class Discussion
             :commit_id,
             :for_commit?,
             :for_merge_request?,
+            :noteable_ability_name,
             :to_ability_name,
             :editable?,
             :visible_for?,

app/models/member.rb

@@ -8,6 +8,7 @@ class Member < ApplicationRecord
   include Gitlab::Access
   include Presentable
   include Gitlab::Utils::StrongMemoize
+  include FromUnion
   attr_accessor :raw_invite_token

app/models/merge_request.rb

@@ -69,6 +69,14 @@ class MergeRequest < ApplicationRecord
   has_many :merge_request_assignees
   has_many :assignees, class_name: "User", through: :merge_request_assignees
+  KNOWN_MERGE_PARAMS = [
+    :auto_merge_strategy,
+    :should_remove_source_branch,
+    :force_remove_source_branch,
+    :commit_message,
+    :squash_commit_message,
+    :sha
+  ].freeze
   serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
   after_create :ensure_merge_request_diff

app/models/milestone.rb

@@ -261,6 +261,10 @@ class Milestone < ApplicationRecord
     group || project
   end
+  def to_ability_name
+    model_name.singular
+  end
+
   def group_milestone?
     group_id.present?
   end

app/models/note.rb

@@ -361,6 +361,10 @@ class Note < ApplicationRecord
   end
   def to_ability_name
+    model_name.singular
+  end
+
+  def noteable_ability_name
     for_snippet? ? noteable.class.name.underscore : noteable_type.demodulize.underscore
   end

app/models/project.rb

@@ -614,11 +614,11 @@ class Project < ApplicationRecord
       joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
     end
-    # Returns ids of projects with milestones available for given user
+    # Returns ids of projects with issuables available for given user
     #
-    # Used on queries to find milestones which user can see
-    # For example: Milestone.where(project_id: ids_with_milestone_available_for(user))
-    def ids_with_milestone_available_for(user)
+    # Used on queries to find milestones or labels which user can see
+    # For example: Milestone.where(project_id: ids_with_issuables_available_for(user))
+    def ids_with_issuables_available_for(user)
       with_issues_enabled = with_issues_available_for_user(user).select(:id)
       with_merge_requests_enabled = with_merge_requests_available_for_user(user).select(:id)
@@ -1265,6 +1265,10 @@ class Project < ApplicationRecord
     end
   end
+  def to_ability_name
+    model_name.singular
+  end
+
   # rubocop: disable CodeReuse/ServiceClass
   def execute_hooks(data, hooks_scope = :push_hooks)
     run_after_commit_or_now do

app/models/system_note_metadata.rb

@@ -10,6 +10,7 @@ class SystemNoteMetadata < ApplicationRecord
     commit cross_reference
     close duplicate
     moved merge
+    label milestone
   ].freeze
   ICON_TYPES = %w[

app/models/wiki_page.rb

@@ -121,6 +121,12 @@ class WikiPage
     @version ||= @page.version
   end
+  def path
+    return unless persisted?
+
+    @path ||= @page.path
+  end
+
   def versions(options = {})
     return [] unless persisted?

app/policies/commit_policy.rb

@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy
   delegate { @subject.project }
   rule { can?(:download_code) }.enable :read_commit
+  rule { ~can?(:read_commit) }.prevent :create_note
 end

app/policies/group_policy.rb

@@ -131,6 +131,8 @@ class GroupPolicy < BasePolicy
   rule { owner | admin }.enable :read_statistics
+  rule { maintainer & can?(:create_projects) }.enable :transfer_projects
+
   def access_level
     return GroupMember::NO_ACCESS if @user.nil?

app/policies/namespace_policy.rb

@@ -15,6 +15,8 @@ class NamespacePolicy < BasePolicy
   end
   rule { personal_project & ~can_create_personal_project }.prevent :create_projects
+
+  rule { (owner | admin) & can?(:create_projects) }.enable :transfer_projects
 end
 NamespacePolicy.prepend_if_ee('EE::NamespacePolicy')