Merge branch 'ce-to-ee-2018-11-29' into 'master'

CE upstream - 2018-11-29 00:21 UTC

Closes gitlab-ce#38317, #8608, #2727, #2732, and #2742

See merge request gitlab-org/gitlab-ee!8634

.gitlab-ci.yml

@@ -1140,6 +1140,7 @@ review-deploy:
     GITLAB_ADMIN_PASSWORD: "${REVIEW_APPS_ROOT_PASSWORD}"
     GITHUB_ACCESS_TOKEN: "${REVIEW_APPS_QA_GITHUB_ACCESS_TOKEN}"
     EE_LICENSE: "${REVIEW_APPS_EE_LICENSE}"
+    QA_DEBUG: "true"
   artifacts:
     paths:
       - ./qa/gitlab-qa-run-*
@@ -1155,6 +1156,7 @@ review-deploy:
 
 review-qa-smoke:
   <<: *review-qa-base
+  retry: 2
   script:
     - gitlab-qa Test::Instance::Smoke "${QA_IMAGE}" "${CI_ENVIRONMENT_URL}"
 

CHANGELOG.md

@@ -4,8 +4,9 @@ entry.
 
 ## 11.5.1 (2018-11-26)
 
-### Security (16 changes)
+### Security (17 changes)
 
+- Escape user fullname while rendering autocomplete template to prevent XSS.
 - Fix CRLF vulnerability in Project hooks.
 - Fix possible XSS attack in Markdown urls with spaces.
 - Redact sensitive information on gitlab-workhorse log.
@@ -598,12 +599,13 @@ entry.
 
 ## 11.3.11 (2018-11-26)
 
-### Security (32 changes)
+### Security (33 changes)
 
 - Filter user sensitive data from discussions JSON. !2537
 - Escape entity title while autocomplete template rendering to prevent XSS. !2557
-- Resolve reflected XSS in Ouath authorize window.
+- Restrict Personal Access Tokens to API scope on web requests.
 - Fix XSS in merge request source branch name.
+- Escape user fullname while rendering autocomplete template to prevent XSS.
 - Fix CRLF vulnerability in Project hooks.
 - Fix possible XSS attack in Markdown urls with spaces.
 - Redact sensitive information on gitlab-workhorse log.
@@ -615,8 +617,8 @@ entry.
 - Markdown API no longer displays confidential title references unless authorized.
 - Provide email notification when a user changes their email address.
 - Properly filter private references from system notes.
-- Restrict Personal Access Tokens to API scope on web requests.
 - Redact personal tokens in unsubscribe links.
+- Resolve reflected XSS in Ouath authorize window.
 - Fix SSRF in project integrations.
 - Fix stored XSS in merge requests from imported repository.
 - Fixed ability to comment on locked/confidential issues.

GITALY_SERVER_VERSION

-1.1.0
+1.2.0

GITLAB_PAGES_VERSION

-1.3.0
+1.3.1

GITLAB_SHELL_VERSION

-8.4.1
+8.4.3

GITLAB_WORKHORSE_VERSION

-7.1.1
+7.2.1

Gemfile

@@ -7,6 +7,11 @@ gem_versions = {}
 gem_versions['activerecord_sane_schema_dumper'] = rails5? ? '1.0'    : '0.2'
 gem_versions['rails']                           = rails5? ? '5.0.7'  : '4.2.10'
 gem_versions['rails-i18n']                      = rails5? ? '~> 5.1' : '~> 4.0.9'
+
+# The 2.0.6 version of rack requires monkeypatch to be present in
+# `config.ru`. This can be removed once a new update for Rack
+# is available that contains https://github.com/rack/rack/pull/1201.
+gem_versions['rack']                            = rails5? ? '2.0.6' : '1.6.11'
 # --- The end of special code for migrating to Rails 5.0 ---
 
 source 'https://rubygems.org'
@@ -164,6 +169,8 @@ gem 'icalendar'
 gem 'diffy', '~> 3.1.0'
 
 # Application server
+gem 'rack', gem_versions['rack']
+
 group :unicorn do
   gem 'unicorn', '~> 5.1.0'
   gem 'unicorn-worker-killer', '~> 0.4.4'
@@ -440,7 +447,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 1.1.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 1.2.0', require: 'gitaly'
 gem 'grpc', '~> 1.15.0'
 
 gem 'google-protobuf', '~> 3.6'

Gemfile.lock

@@ -297,7 +297,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (1.1.0)
+    gitaly-proto (1.2.0)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab-default_value_for (3.1.1)
@@ -1040,7 +1040,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 1.1.0)
+  gitaly-proto (~> 1.2.0)
   github-markup (~> 1.7.0)
   gitlab-default_value_for (~> 3.1.1)
   gitlab-license (~> 1.0)
@@ -1125,6 +1125,7 @@ DEPENDENCIES
   pry-rails (~> 0.3.4)
   puma (~> 3.12)
   puma_worker_killer
+  rack (= 2.0.6)
   rack-attack (~> 4.4.1)
   rack-cors (~> 1.0.0)
   rack-oauth2 (~> 1.2.1)

Gemfile.rails4.lock

@@ -296,7 +296,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (1.1.0)
+    gitaly-proto (1.2.0)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab-license (1.0.0)
@@ -1032,7 +1032,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 1.1.0)
+  gitaly-proto (~> 1.2.0)
   github-markup (~> 1.7.0)
   gitlab-license (~> 1.0)
   gitlab-markup (~> 1.6.5)
@@ -1116,6 +1116,7 @@ DEPENDENCIES
   pry-rails (~> 0.3.4)
   puma (~> 3.12)
   puma_worker_killer
+  rack (= 1.6.11)
   rack-attack (~> 4.4.1)
   rack-cors (~> 1.0.0)
   rack-oauth2 (~> 1.2.1)

app/assets/javascripts/behaviors/markdown/render_mermaid.js

@@ -26,6 +26,9 @@ export default function renderMermaid($els) {
         },
         // mermaidAPI options
         theme: 'neutral',
+        flowchart: {
+          htmlLabels: false,
+        },
       });
 
       $els.each((i, el) => {

app/assets/javascripts/pages/projects/wikis/wikis.js

 import bp from '../../../breakpoints';
-import { slugify } from '../../../lib/utils/text_utility';
 import { parseQueryStringIntoObject } from '../../../lib/utils/common_utils';
 import { mergeUrlParams, redirectTo } from '../../../lib/utils/url_utility';
 
@@ -26,7 +25,8 @@ export default class Wikis {
     if (!this.newWikiForm) return;
 
     const slugInput = this.newWikiForm.querySelector('#new_wiki_path');
-    const slug = slugify(slugInput.value);
+
+    const slug = slugInput.value;
 
     if (slug.length > 0) {
       const wikisPath = slugInput.getAttribute('data-wikis-path');

app/controllers/admin/impersonations_controller.rb

@@ -5,23 +5,12 @@ class Admin::ImpersonationsController < Admin::ApplicationController
   before_action :authenticate_impersonator!
 
   def destroy
-    original_user = current_user
-
-    warden.set_user(impersonator, scope: :user)
-
-    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{original_user.username}")
-
-    session[:impersonator_id] = nil
-
+    original_user = stop_impersonation
     redirect_to admin_user_path(original_user), status: :found
   end
 
   private
 
-  def impersonator
-    @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id]
-  end
-
   def authenticate_impersonator!
     render_404 unless impersonator && impersonator.admin? && !impersonator.blocked?
   end

app/controllers/admin/users_controller.rb

@@ -2,6 +2,7 @@
 
 class Admin::UsersController < Admin::ApplicationController
   before_action :user, except: [:index, :new, :create]
+  before_action :check_impersonation_availability, only: :impersonate
 
   def index
     @users = User.order_name_asc.filter(params[:filter])
@@ -227,6 +228,10 @@ class Admin::UsersController < Admin::ApplicationController
 
     result[:status] == :success
   end
+
+  def check_impersonation_availability
+    access_denied! unless Gitlab.config.gitlab.impersonation_enabled
+  end
 end
 
 Admin::UsersController.prepend(EE::Admin::UsersController)

app/controllers/application_controller.rb

@@ -28,6 +28,7 @@ class ApplicationController < ActionController::Base
   before_action :configure_permitted_parameters, if: :devise_controller?
   before_action :require_email, unless: :devise_controller?
   before_action :set_usage_stats_consent_flag
+  before_action :check_impersonation_availability
 
   around_action :set_locale
 
@@ -470,4 +471,28 @@ class ApplicationController < ActionController::Base
       .new(settings, current_user, application_setting_params)
       .execute
   end
+
+  def check_impersonation_availability
+    return unless session[:impersonator_id]
+
+    unless Gitlab.config.gitlab.impersonation_enabled
+      stop_impersonation
+      access_denied! _('Impersonation has been disabled')
+    end
+  end
+
+  def stop_impersonation
+    impersonated_user = current_user
+
+    Gitlab::AppLogger.info("User #{impersonator.username} has stopped impersonating #{impersonated_user.username}")
+
+    warden.set_user(impersonator, scope: :user)
+    session[:impersonator_id] = nil
+
+    impersonated_user
+  end
+
+  def impersonator
+    @impersonator ||= User.find(session[:impersonator_id]) if session[:impersonator_id]
+  end
 end

app/controllers/dashboard/todos_controller.rb

@@ -4,6 +4,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
   include ActionView::Helpers::NumberHelper
 
   before_action :authorize_read_project!, only: :index
+  before_action :authorize_read_group!, only: :index
   before_action :find_todos, only: [:index, :destroy_all]
 
   def index
@@ -60,6 +61,15 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     end
   end
 
+  def authorize_read_group!
+    group_id = params[:group_id]
+
+    if group_id.present?
+      group = Group.find(group_id)
+      render_404 unless can?(current_user, :read_group, group)
+    end
+  end
+
   def find_todos
     @todos ||= TodosFinder.new(current_user, todo_params).execute
   end

app/controllers/oauth/applications_controller.rb

@@ -9,7 +9,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController
   before_action :verify_user_oauth_applications_enabled, except: :index
   before_action :authenticate_user!
   before_action :add_gon_variables
-  before_action :load_scopes, only: [:index, :create, :edit]
+  before_action :load_scopes, only: [:index, :create, :edit, :update]
 
   helper_method :can?
 

app/controllers/projects/milestones_controller.rb

@@ -11,7 +11,10 @@ class Projects::MilestonesController < Projects::ApplicationController
   before_action :authorize_read_milestone!
 
   # Allow admin milestone
-  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels, :promote]
+  before_action :authorize_admin_milestone!, except: [:index, :show, :merge_requests, :participants, :labels]
+
+  # Allow to promote milestone
+  before_action :authorize_promote_milestone!, only: :promote
 
   respond_to :html
 
@@ -78,7 +81,7 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   def promote
     promoted_milestone = Milestones::PromoteService.new(project, current_user).execute(milestone)
-    flash[:notice] = flash_notice_for(promoted_milestone, project.group)
+    flash[:notice] = flash_notice_for(promoted_milestone, project_group)
 
     respond_to do |format|
       format.html do
@@ -109,6 +112,12 @@ class Projects::MilestonesController < Projects::ApplicationController
 
   protected
 
+  def project_group
+    strong_memoize(:project_group) do
+      project.group
+    end
+  end
+
   def milestones
     strong_memoize(:milestones) do
       MilestonesFinder.new(search_params).execute
@@ -125,13 +134,17 @@ class Projects::MilestonesController < Projects::ApplicationController
     return render_404 unless can?(current_user, :admin_milestone, @project)
   end
 
+  def authorize_promote_milestone!
+    return render_404 unless can?(current_user, :admin_milestone, project_group)
+  end
+
   def milestone_params
     params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
   end
 
   def search_params
-    if request.format.json? && @project.group && can?(current_user, :read_group, @project.group)
-      groups = @project.group.self_and_ancestors_ids
+    if request.format.json? && project_group && can?(current_user, :read_group, project_group)
+      groups = project_group.self_and_ancestors_ids
     end
 
     params.permit(:state).merge(project_ids: @project.id, group_ids: groups)

app/helpers/milestones_helper.rb

@@ -2,6 +2,7 @@
 
 module MilestonesHelper
   include EntityDateHelper
+  include Gitlab::Utils::StrongMemoize
 
   def milestones_filter_path(opts = {})
     if @project
@@ -243,6 +244,18 @@ module MilestonesHelper
       dashboard_milestone_path(milestone.safe_title, title: milestone.title)
     end
   end
+
+  def can_admin_project_milestones?
+    strong_memoize(:can_admin_project_milestones) do
+      can?(current_user, :admin_milestone, @project)
+    end
+  end
+
+  def can_admin_group_milestones?
+    strong_memoize(:can_admin_group_milestones) do
+      can?(current_user, :admin_milestone, @project.group)
+    end
+  end
 end
 
 MilestonesHelper.prepend(EE::MilestonesHelper)

app/helpers/users_helper.rb

@@ -72,6 +72,10 @@ module UsersHelper
     end
   end
 
+  def impersonation_enabled?
+    Gitlab.config.gitlab.impersonation_enabled
+  end
+
   private
 
   def get_profile_tabs

app/models/concerns/cache_markdown_field.rb

@@ -15,7 +15,7 @@ module CacheMarkdownField
   # Increment this number every time the renderer changes its output
   CACHE_REDCARPET_VERSION         = 3
   CACHE_COMMONMARK_VERSION_START  = 10
-  CACHE_COMMONMARK_VERSION        = 11
+  CACHE_COMMONMARK_VERSION        = 12
 
   # changes to these attributes cause the cache to be invalidates
   INVALIDATED_BY = %w[author project].freeze

app/models/project_services/prometheus_service.rb

@@ -71,7 +71,7 @@ class PrometheusService < MonitoringService
   end
 
   def prometheus_client
-    RestClient::Resource.new(api_url) if api_url && manual_configuration? && active?
+    RestClient::Resource.new(api_url, max_redirects: 0) if api_url && manual_configuration? && active?
   end
 
   def prometheus_available?

app/models/remote_mirror.rb

@@ -15,8 +15,6 @@ class RemoteMirror < ActiveRecord::Base
                  insecure_mode: true,
                  algorithm: 'aes-256-cbc'
 
-  default_value_for :only_protected_branches, true
-
   belongs_to :project, inverse_of: :remote_mirrors
 
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true, enforce_user: true }

app/models/wiki_page.rb

@@ -85,6 +85,12 @@ class WikiPage
 
   alias_method :to_param, :slug
 
+  def human_title
+    return 'Home' if title == 'home'
+
+    title
+  end
+
   # The formatted title of this page.
   def title
     if @attributes[:title]

app/services/access_token_validation_service.rb

@@ -6,6 +6,7 @@ class AccessTokenValidationService
   EXPIRED = :expired
   REVOKED = :revoked
   INSUFFICIENT_SCOPE = :insufficient_scope
+  IMPERSONATION_DISABLED = :impersonation_disabled
 
   attr_reader :token, :request
 
@@ -24,6 +25,11 @@ class AccessTokenValidationService
     elsif !self.include_any_scope?(scopes)
       return INSUFFICIENT_SCOPE
 
+    elsif token.respond_to?(:impersonation) &&
+        token.impersonation &&
+        !Gitlab.config.gitlab.impersonation_enabled
+      return IMPERSONATION_DISABLED
+
     else
       return VALID
     end

app/services/ci/archive_trace_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class ArchiveTraceService
+    def execute(job)
+      job.trace.archive!
+    rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
+      # It's already archived, thus we can safely ignore this exception.
+    rescue => e
+      # Tracks this error with application logs, Sentry, and Prometheus.
+      # If `archive!` keeps failing for over a week, that could incur data loss.
+      # (See more https://docs.gitlab.com/ee/administration/job_traces.html#new-live-trace-architecture)
+      # In order to avoid interrupting the system, we do not raise an exception here.
+      archive_error(e, job)
+    end
+
+    private
+
+    def failed_archive_counter
+      @failed_archive_counter ||=
+        Gitlab::Metrics.counter(:job_trace_archive_failed_total,
+                                "Counter of failed attempts of trace archiving")
+    end
+
+    def archive_error(error, job)
+      failed_archive_counter.increment
+      Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}"
+
+      Gitlab::Sentry
+        .track_exception(error,
+                         issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
+                         extra: { job_id: job.id })
+    end
+  end
+end

app/services/files/multi_service.rb

@@ -8,6 +8,7 @@ module Files
       transformer = Lfs::FileTransformer.new(project, @branch_name)
 
       actions = actions_after_lfs_transformation(transformer, params[:actions])
+      actions = transform_move_actions(actions)
 
       commit_actions!(actions)
     end
@@ -26,6 +27,16 @@ module Files
       end
     end
 
+    # When moving a file, `content: nil` means "use the contents of the previous
+    # file", while `content: ''` means "move the file and set it to empty"
+    def transform_move_actions(actions)
+      actions.map do |action|
+        action[:infer_content] = true if action[:content].nil?
+
+        action
+      end
+    end
+
     def commit_actions!(actions)
       repository.multi_action(
         current_user,

app/views/admin/users/_head.html.haml

@@ -10,7 +10,7 @@
     %span.cred (Auditor)
 
   .float-right
-    - if @user != current_user && @user.can?(:log_in)
+    - if impersonation_enabled? && @user != current_user && @user.can?(:log_in)
       = link_to 'Impersonate', impersonate_admin_user_path(@user), method: :post, class: "btn btn-nr btn-grouped btn-info"
     = link_to edit_admin_user_path(@user), class: "btn btn-nr btn-grouped" do
       %i.fa.fa-pencil-square-o

app/views/devise/mailer/email_changed.html.haml

+= email_default_heading("Hello, #{@resource.name}!")
+
+- if @resource.try(:unconfirmed_email?)
+  %p
+    We're contacting you to notify you that your email is being changed to #{@resource.reload.unconfirmed_email}.
+- else
+  %p
+    We're contacting you to notify you that your email has been changed to #{@resource.email}.
+
+%p
+  If you did not initiate this change, please contact your administrator
+  immediately.

app/views/devise/mailer/email_changed.text.erb

+Hello, <%= @resource.name %>!
+
+<% if @resource.try(:unconfirmed_email?) %>
+We're contacting you to notify you that your email is being changed to <%= @resource.reload.unconfirmed_email %>.
+<% else %>
+We're contacting you to notify you that your email has been changed to <%= @resource.email %>.
+<% end %>
+
+If you did not initiate this change, please contact your administrator
+immediately.

app/views/groups/labels/edit.html.haml

-- page_title 'Edit', @label.name, 'Labels'
+- add_to_breadcrumbs _("Labels"), group_labels_path(@group)
+- breadcrumb_title _("Edit")
+- page_title "Edit", @label.name, _("Labels")
 
 %h3.page-title
   Edit Label

app/views/groups/labels/new.html.haml

-- breadcrumb_title "Labels"
-- page_title 'New Label'
+- add_to_breadcrumbs _("Labels"), group_labels_path(@group)
+- breadcrumb_title _("New")
+- page_title _("New Label")
 
 %h3.page-title
   New Label

app/views/groups/milestones/edit.html.haml

-- page_title "Milestones"
+- breadcrumb_title _("Edit")
+- page_title _("Milestones")
+
 - render "header_title"
 
 %h3.page-title
   Edit Milestone
+%hr
 
 = render "form"

app/views/groups/milestones/new.html.haml

-- breadcrumb_title "Milestones"
-- page_title "Milestones"
+- @no_container = true
+- add_to_breadcrumbs _("Milestones"), group_milestones_path(@group)
+- breadcrumb_title _("New")
+- page_title _("Milestones"), @milestone.name, _("Milestones")
 
-%h3.page-title
+%div{ class: container_class }
+  %h3.page-title
     New Milestone
 
-= render "form"
+  %hr
+
+  = render "form"

app/views/projects/commits/_commit.html.haml

@@ -8,23 +8,11 @@
 - ref           = local_assigns.fetch(:ref) { merge_request&.source_branch }
 
 - link = commit_path(project, commit, merge_request: merge_request)
-- cache_key = [project.full_path,
-               ref,
-               commit.id,
-               Gitlab::CurrentSettings.current_application_settings,
-               @path.presence,
-               current_controller?(:commits),
-               merge_request&.iid,
-               view_details,
-               commit.status(ref),
-               I18n.locale].compact
 
 -# EE-only
 - show_project_name = local_assigns.fetch(:show_project_name, false)
-- cache_key << show_project_name
 
-= cache(cache_key, expires_in: 1.day) do
-  %li.commit.flex-row.js-toggle-container{ id: "commit-#{commit.short_id}" }
+%li.commit.flex-row.js-toggle-container{ id: "commit-#{commit.short_id}" }
 
   .avatar-cell.d-none.d-sm-block
     = author_avatar(commit, size: 36, has_tooltip: false)

app/views/projects/labels/edit.html.haml

 - @no_container = true
+- add_to_breadcrumbs "Labels", project_labels_path(@project)
+- breadcrumb_title "Edit"
 - page_title "Edit", @label.name, "Labels"
 
 %div{ class: container_class }

app/views/projects/labels/new.html.haml

 - @no_container = true
-- breadcrumb_title "Labels"
+- add_to_breadcrumbs "Labels", project_labels_path(@project)
+- breadcrumb_title "New"
 - page_title "New Label"
 
 %div{ class: container_class }

app/views/projects/milestones/edit.html.haml

 - @no_container = true
+- breadcrumb_title "Edit"
+- add_to_breadcrumbs "Milestones", project_milestones_path(@project)
 - page_title "Edit", @milestone.title, "Milestones"
 
+
 %div{ class: container_class }
 
   %h3.page-title

app/views/projects/milestones/new.html.haml

 - @no_container = true
-- breadcrumb_title "Milestones"
+- add_to_breadcrumbs "Milestones", project_milestones_path(@project)
+- breadcrumb_title "New"
 - page_title "New Milestone"
 
 %div{ class: container_class }

app/views/projects/wikis/_sidebar_wiki_page.html.haml

 %li{ class: active_when(params[:id] == wiki_page.slug) }
   = link_to project_wiki_path(@project, wiki_page) do
-    = wiki_page.title.capitalize
+    = wiki_page.human_title

app/views/projects/wikis/edit.html.haml

 - @content_class = "limit-container-width" unless fluid_layout
-- page_title _("Edit"), @page.title.capitalize, _("Wiki")
+- add_to_breadcrumbs _("Wiki"), project_wiki_path(@project, @page)
+- breadcrumb_title @page.persisted? ? _("Edit") : _("New")
+- page_title @page.persisted? ? _("Edit") : _("New"), @page.human_title, _("Wiki")
 
 = wiki_page_errors(@error)
 
@@ -10,9 +12,9 @@
   .nav-text
     %h2.wiki-page-title
       - if @page.persisted?
-        = link_to @page.title.capitalize, project_wiki_path(@project, @page)
+        = link_to @page.human_title, project_wiki_path(@project, @page)
       - else
-        = @page.title.capitalize
+        = @page.human_title
       %span.light
         ·
         - if @page.persisted?
@@ -28,7 +30,7 @@
       = link_to project_wiki_history_path(@project, @page), class: "btn" do
         = s_("Wiki|Page history")
       - if can?(current_user, :admin_wiki, @project)
-        #delete-wiki-modal-wrapper{ data: { delete_wiki_url: project_wiki_path(@project, @page), page_title: @page.title.capitalize } }
+        #delete-wiki-modal-wrapper{ data: { delete_wiki_url: project_wiki_path(@project, @page), page_title: @page.human_title } }
 
 = render 'form', uploads_path: wiki_attachment_upload_url
 

app/views/projects/wikis/history.html.haml

-- page_title _("History"), @page.title.capitalize, _("Wiki")
+- page_title _("History"), @page.human_title, _("Wiki")
 
 .wiki-page-header.has-sidebar-toggle
   %button.btn.btn-default.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" }
@@ -6,7 +6,7 @@
 
   .nav-text
     %h2.wiki-page-title
-      = link_to @page.title.capitalize, project_wiki_path(@project, @page)
+      = link_to @page.human_title, project_wiki_path(@project, @page)
       %span.light
         ·
         = _("History")

app/views/projects/wikis/show.html.haml

 - @content_class = "limit-container-width" unless fluid_layout
-- breadcrumb_title @page.title.capitalize
+- breadcrumb_title @page.human_title
 - wiki_breadcrumb_dropdown_links(@page.slug)
-- page_title @page.title.capitalize, _("Wiki")
+- page_title @page.human_title, _("Wiki")
 - add_to_breadcrumbs _("Wiki"), get_project_wiki_path(@project)
 
 .wiki-page-header.has-sidebar-toggle
@@ -9,7 +9,7 @@
     = icon('angle-double-left')
 
   .nav-text
-    %h2.wiki-page-title= @page.title.capitalize
+    %h2.wiki-page-title= @page.human_title
     %span.wiki-last-edit-by
       - if @page.last_version
         = (_("Last edited by %{name}") % { name: "<strong>#{@page.last_version.author_name}</strong>" }).html_safe

app/views/shared/milestones/_milestone.html.haml

@@ -35,8 +35,8 @@
     .col-sm-2
       .milestone-actions.d-flex.justify-content-sm-start.justify-content-md-end
         - if @project
-          - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
-            - if @project.group
+          - if can_admin_project_milestones? and milestone.active?
+            - if can_admin_group_milestones?
               %button.js-promote-project-milestone-button.btn.btn-blank.btn-sm.btn-grouped.has-tooltip{ title: _('Promote to Group Milestone'),
                 disabled: true,
                 type: 'button',

app/views/snippets/_actions.html.haml

@@ -3,31 +3,31 @@
 .d-none.d-sm-block
   - if can?(current_user, :update_personal_snippet, @snippet)
     = link_to edit_snippet_path(@snippet), class: "btn btn-grouped" do
-      Edit
+      = _("Edit")
   - if can?(current_user, :admin_personal_snippet, @snippet)
-    = link_to snippet_path(@snippet), method: :delete, data: { confirm: "Are you sure?" }, class: "btn btn-grouped btn-inverted btn-remove", title: 'Delete Snippet' do
-      Delete
-  = link_to new_snippet_path, class: "btn btn-grouped btn-inverted btn-success", title: "New snippet" do
-    New snippet
+    = link_to snippet_path(@snippet), method: :delete, data: { confirm: _("Are you sure?") }, class: "btn btn-grouped btn-inverted btn-remove", title: _('Delete Snippet') do
+      = _("Delete")
+  = link_to new_snippet_path, class: "btn btn-grouped btn-inverted btn-create", title: _("New snippet") do
+    = _("New snippet")
   - if @snippet.submittable_as_spam_by?(current_user)
-    = link_to 'Submit as spam', mark_as_spam_snippet_path(@snippet), method: :post, class: 'btn btn-grouped btn-spam', title: 'Submit as spam'
+    = link_to _('Submit as spam'), mark_as_spam_snippet_path(@snippet), method: :post, class: 'btn btn-grouped btn-spam', title: _('Submit as spam')
 .d-block.d-sm-none.dropdown
   %button.btn.btn-default.btn-block.append-bottom-0.prepend-top-5{ data: { toggle: "dropdown" } }
-    Options
+    = _("Options")
     = icon('caret-down')
   .dropdown-menu.dropdown-menu-full-width
     %ul
       %li
-        = link_to new_snippet_path, title: "New snippet" do
-          New snippet
+        = link_to new_snippet_path, title: _("New snippet") do
+          = _("New snippet")
       - if can?(current_user, :admin_personal_snippet, @snippet)
         %li
-          = link_to snippet_path(@snippet), method: :delete, data: { confirm: "Are you sure?" }, title: 'Delete Snippet' do
-            Delete
+          = link_to snippet_path(@snippet), method: :delete, data: { confirm: _("Are you sure?") }, title: _('Delete Snippet') do
+            = _("Delete")
       - if can?(current_user, :update_personal_snippet, @snippet)
         %li
           = link_to edit_snippet_path(@snippet) do
-            Edit
+            = _("Edit")
       - if @snippet.submittable_as_spam_by?(current_user)
         %li
-          = link_to 'Submit as spam', mark_as_spam_snippet_path(@snippet), method: :post
+          = link_to _('Submit as spam'), mark_as_spam_snippet_path(@snippet), method: :post

app/views/snippets/_snippets.html.haml

@@ -5,6 +5,6 @@
     = render partial: 'shared/snippets/snippet', collection: @snippets, locals: { link_project: link_project }
     - if @snippets.empty?
       %li
-        .nothing-here-block Nothing here.
+        .nothing-here-block= _("Nothing here.")
 
   = paginate @snippets, theme: 'gitlab'

app/views/snippets/_snippets_scope_menu.html.haml

@@ -4,7 +4,7 @@
 .nav-links.snippet-scope-menu.mobile-separator.nav.nav-tabs
   %li{ class: active_when(params[:scope].nil?) }
     = link_to subject_snippets_path(subject) do
-      All
+      = _("All")
       %span.badge.badge-pill
         - if include_private
           = subject.snippets.count
@@ -14,18 +14,18 @@
   - if include_private
     %li{ class: active_when(params[:scope] == "are_private") }
       = link_to subject_snippets_path(subject, scope: 'are_private') do
-        Private
+        = _("Private")
         %span.badge.badge-pill
           = subject.snippets.are_private.count
 
   %li{ class: active_when(params[:scope] == "are_internal") }
     = link_to subject_snippets_path(subject, scope: 'are_internal') do
-      Internal
+      = _("Internal")
       %span.badge.badge-pill
         = subject.snippets.are_internal.count
 
   %li{ class: active_when(params[:scope] == "are_public") }
     = link_to subject_snippets_path(subject, scope: 'are_public') do
-      Public
+      = _("Public")
       %span.badge.badge-pill
         = subject.snippets.are_public.count

app/views/snippets/edit.html.haml

-- page_title "Edit", "#{@snippet.title} (#{@snippet.to_reference})", "Snippets"
+- page_title _("Edit"), "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets")
+
 %h3.page-title
-  Edit Snippet
+  = _("Edit Snippet")
 %hr
 = render 'shared/snippets/form', url: snippet_path(@snippet)

app/views/snippets/index.html.haml

-- page_title "By #{@user.name}", "Snippets"
+- page_title _("By %{user_name}") % { user_name: @user.name }, _("Snippets")
 
 %ol.breadcrumb
   %li.breadcrumb-item
     = link_to snippets_path do
-      Snippets
+      = _("Snippets")
   %li.breadcrumb-item
     = @user.name
   .float-right.d-none.d-sm-block
     = link_to user_path(@user) do
-      #{@user.name} profile page
+      = _("%{user_name} profile page") % { user_name: @user.name }
 
 = render 'snippets'

app/views/snippets/new.html.haml

 - @hide_top_links = true
 - @hide_breadcrumbs = true
-- page_title "New Snippet"
+- page_title _("New Snippet")
 
 .page-title-holder
   %h1.page-title= _('New Snippet')

app/views/snippets/notes/_actions.html.haml

 - if current_user
   - if note.emoji_awardable?
     .note-actions-item
-      = link_to '#', title: 'Add reaction', class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip", data: { position: 'right' } do
+      = link_to '#', title: _('Add reaction'), class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip", data: { position: 'right' } do
         = icon('spinner spin')
         %span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
         %span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
@@ -9,7 +9,7 @@
 
   - if note_editable
     .note-actions-item
-      = button_tag title: 'Edit comment', class: 'note-action-button js-note-edit has-tooltip btn btn-transparent', data: { container: 'body' } do
+      = button_tag title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip btn btn-transparent', data: { container: 'body' } do
         %span.link-highlight
           = custom_icon('icon_pencil')
 

app/views/snippets/show.html.haml

 - @hide_top_links = true
 - @content_class = "limit-container-width limited-inner-width-container" unless fluid_layout
-- add_to_breadcrumbs "Snippets", dashboard_snippets_path
+- add_to_breadcrumbs _("Snippets"), dashboard_snippets_path
 - breadcrumb_title @snippet.to_reference
-- page_title "#{@snippet.title} (#{@snippet.to_reference})", "Snippets"
+- page_title "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets")
 
 = render 'shared/snippets/header'
 

app/workers/archive_trace_worker.rb

@@ -7,7 +7,7 @@ class ArchiveTraceWorker
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(job_id)
     Ci::Build.without_archived_trace.find_by(id: job_id).try do |job|
-      job.trace.archive!
+      Ci::ArchiveTraceService.new.execute(job)
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/workers/ci/archive_traces_cron_worker.rb

@@ -11,21 +11,9 @@ module Ci
       # This could happen when ArchiveTraceWorker sidekiq jobs were lost by receiving SIGKILL
       # More details in https://gitlab.com/gitlab-org/gitlab-ce/issues/36791
       Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
-        begin
-          build.trace.archive!
-        rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
-        rescue => e
-          failed_archive_counter.increment
-          Rails.logger.error "Failed to archive stale live trace. id: #{build.id} message: #{e.message}"
-        end
+        Ci::ArchiveTraceService.new.execute(build)
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord
-
-    private
-
-    def failed_archive_counter
-      @failed_archive_counter ||= Gitlab::Metrics.counter(:job_trace_archive_failed_total, "Counter of failed attempts of traces archiving")
-    end
   end
 end

changelogs/unreleased/40385-prohibit_impersonation.yml

+---
+title: Add config to prohibit impersonation
+merge_request: 23338
+author:
+type: added

changelogs/unreleased/51083-fix-move-api.yml

+---
+title: 'Commits API: Preserve file content in move operations if unspecified'
+merge_request: 23387
+author:
+type: fixed

changelogs/unreleased/gt-externalize-app-views-snippets.yml

+---
+title: Externalize strings from `/app/views/snippets`
+merge_request: 23351
+author: Tao Wang
+type: other

changelogs/unreleased/include-new-link-in-breadcrumb.yml

+---
+title: Include new link in breadcrumb for issues, merge requests, milestones, and labels
+merge_request: 18515
+author: George Tsiolis
+type: changed

changelogs/unreleased/security-182-update-workhorse.yml

+---
+title: Redact sensitive information on gitlab-workhorse log
+merge_request:
+author:
+type: security

changelogs/unreleased/security-2736-prometheus-ssrf.yml

+---
+title: Do not follow redirects in Prometheus service when making http requests to the configured api url
+merge_request:
+author:
+type: security

changelogs/unreleased/security-bvl-exposure-in-commits-list.yml

+---
+title: Don't expose confidential information in commit message list
+merge_request:
+author:
+type: security

changelogs/unreleased/security-email-change-notification.yml

+---
+title: Provide email notification when a user changes their email address
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fix-uri-xss-applications.yml

+---
+title: Resolve reflected XSS in Ouath authorize window
+merge_request:
+author:
+type: security

changelogs/unreleased/security-fix-webhook-ssrf-ipv6.yml

+---
+title: Fix SSRF in project integrations
+merge_request: 
+author:
+type: security

changelogs/unreleased/security-fj-crlf-injection.yml

+---
+title: Fix CRLF vulnerability in Project hooks
+merge_request:
+author:
+type: security

changelogs/unreleased/security-issue_51301.yml

+---
+title: Fix milestone promotion authorization check
+merge_request:
+author:
+type: security

changelogs/unreleased/security-mermaid-xss.yml

+---
+title: Configure mermaid to not render HTML content in diagrams
+merge_request:
+author:
+type: security

changelogs/unreleased/security-pages-toctou-race.yml

+---
+title: Fix a possible symlink time of check to time of use race condition in GitLab
+  Pages
+merge_request:
+author:
+type: security

changelogs/unreleased/security-private-group.yml

+---
+title: Removed ability to see private group names when the group id is entered in
+  the url.
+merge_request:
+author:
+type: security

changelogs/unreleased/security-stored-xss-for-environments.yml

+---
+title: Fix stored XSS for Environments
+merge_request:
+author:
+type: security

changelogs/unreleased/security-xss-in-markdown-following-unrecognized-html-element.yml

+---
+title: Fix possible XSS attack in Markdown urls with spaces
+merge_request: 2599
+author:
+type: security

changelogs/unreleased/sh-fix-issue-38317.yml

+---
+title: Remove needless auto-capitalization on Wiki page titles
+merge_request: 23288
+author:
+type: fixed

changelogs/unreleased/unicorn-monkey-patch.yml

+---
+title: Add monkey patch to unicorn to fix eof? problem
+merge_request: 23385
+author:
+type: fixed

config.ru

@@ -13,6 +13,10 @@ if defined?(Unicorn)
     # Max memory size (RSS) per worker
     use Unicorn::WorkerKiller::Oom, min, max
   end
+
+  # Monkey patch for fixing Rack 2.0.6 bug:
+  # https://gitlab.com/gitlab-org/gitlab-ee/issues/8539
+  Unicorn::StreamInput.send(:public, :eof?) # rubocop:disable GitlabSecurity/PublicSend
 end
 
 require ::File.expand_path('../config/environment',  __FILE__)

config/application.rb

@@ -117,6 +117,9 @@ module Gitlab
     # - Webhook URLs (:hook)
     # - Sentry DSN (:sentry_dsn)
     # - File content from Web Editor (:content)
+    #
+    # NOTE: It is **IMPORTANT** to also update gitlab-workhorse's filter when adding parameters here to not
+    #       introduce another security vulnerability: https://gitlab.com/gitlab-org/gitlab-workhorse/issues/182
     config.filter_parameters += [/token$/, /password/, /secret/, /key$/]
     config.filter_parameters += %i(
       certificate

config/gitlab.yml.example

@@ -114,6 +114,9 @@ production: &base
     # The default is 'shared/cache/archive/' relative to the root of the Rails app.
     # repository_downloads_path: shared/cache/archive/
 
+    ## Impersonation settings
+    impersonation_enabled: true
+
   ## Reply by email
   # Allow users to comment on issues and merge requests by replying to notification emails.
   # For documentation on how to set this up, see http://doc.gitlab.com/ce/administration/reply_by_email.html

config/initializers/1_settings.rb

@@ -166,6 +166,7 @@ Settings.gitlab['domain_whitelist'] ||= []
 Settings.gitlab['import_sources'] ||= Gitlab::ImportSources.values
 Settings.gitlab['trusted_proxies'] ||= []
 Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config', 'no_todos_messages.yml'))
+Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil?
 Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil?
 
 #

config/initializers/devise.rb

@@ -103,6 +103,9 @@ Devise.setup do |config|
   # Send a notification email when the user's password is changed
   config.send_password_change_notification = true
 
+  # Send a notification email when the user's email is changed
+  config.send_email_changed_notification = true
+
   # ==> Configuration for :validatable
   # Range for password length. Default is 6..128.
   config.password_length = 8..128

config/initializers/doorkeeper.rb

@@ -48,6 +48,13 @@ Doorkeeper.configure do
   #
   force_ssl_in_redirect_uri false
 
+  # Specify what redirect URI's you want to block during Application creation.
+  # Any redirect URI is whitelisted by default.
+  #
+  # You can use this option in order to forbid URI's with 'javascript' scheme
+  # for example.
+  forbid_redirect_uri { |uri| %w[data vbscript javascript].include?(uri.scheme.to_s.downcase) }
+
   # Provide support for an owner to be assigned to each registered application (disabled by default)
   # Optional parameter confirmation: true (default false) if you want to enforce ownership of
   # a registered application

db/migrate/20181108091549_cleanup_environments_external_url.rb

+# frozen_string_literal: true
+
+class CleanupEnvironmentsExternalUrl < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    update_column_in_batches(:environments, :external_url, nil) do |table, query|
+      query.where(table[:external_url].matches('javascript://%'))
+    end
+  end
+
+  def down
+  end
+end

db/post_migrate/20181026091631_migrate_forbidden_redirect_uris.rb

+# frozen_string_literal: true
+
+class MigrateForbiddenRedirectUris < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  FORBIDDEN_SCHEMES = %w[data:// vbscript:// javascript://]
+  NEW_URI = 'http://forbidden-scheme-has-been-overwritten'
+
+  disable_ddl_transaction!
+
+  def up
+    update_forbidden_uris(:oauth_applications)
+    update_forbidden_uris(:oauth_access_grants)
+  end
+
+  def down
+    # noop
+  end
+
+  private
+
+  def update_forbidden_uris(table_name)
+    update_column_in_batches(table_name, :redirect_uri, NEW_URI) do |table, query|
+      where_clause = FORBIDDEN_SCHEMES.map do |scheme|
+        table[:redirect_uri].matches("#{scheme}%")
+      end.inject(&:or)
+
+      query.where(where_clause)
+    end
+  end
+end

doc/api/README.md

@@ -232,6 +232,43 @@ For more information, refer to the
 Impersonation tokens are used exactly like regular personal access tokens, and can be passed in either the
 `private_token` parameter or the `Private-Token` header.
 
+#### Disable impersonation
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/40385) in GitLab
+11.6.
+
+By default, impersonation is enabled. To disable impersonation, GitLab must be
+reconfigured:
+
+**For Omnibus installations**
+
+1. Edit `/etc/gitlab/gitlab.rb`:
+
+    ```ruby
+    gitlab_rails['impersonation_enabled'] = false
+    ```
+
+1. Save the file and [reconfigure](../administration/restart_gitlab.md#omnibus-gitlab-reconfigure)
+   GitLab for the changes to take effect.
+
+To re-enable impersonation, remove this configuration and reconfigure GitLab.
+
+---
+
+**For installations from source**
+
+1. Edit `config/gitlab.yml`:
+
+    ```yaml
+    gitlab:
+      impersonation_enabled: false
+    ```
+
+1. Save the file and [restart](../administration/restart_gitlab.md#installations-from-source)
+   GitLab for the changes to take effect.
+
+To re-enable impersonation, remove this configuration and restart GitLab.
+
 ### Sudo
 
 NOTE: **Note:**
@@ -547,7 +584,7 @@ When you try to access an API URL that does not exist you will receive 404 Not F
 ```
 HTTP/1.1 404 Not Found
 Content-Type: application/json
-{
+{ f
     "error": "404 Not Found"
 }
 ```

doc/api/commits.md

@@ -87,7 +87,7 @@ POST /projects/:id/repository/commits
 | `action` | string | yes | The action to perform, `create`, `delete`, `move`, `update`, `chmod`|
 | `file_path` | string | yes | Full path to the file. Ex. `lib/class.rb` |
 | `previous_path` | string | no | Original full path to the file being moved. Ex. `lib/class1.rb`. Only considered for `move` action. |
-| `content` | string | no | File content, required for all except `delete` and `chmod`. Optional for `move` |
+| `content` | string | no | File content, required for all except `delete`, `chmod`, and `move`. Move actions that do not specify `content` will preserve the existing file content, and any other value of `content` will overwrite the file content. |
 | `encoding` | string | no | `text` or `base64`. `text` is default. |
 | `last_commit_id` | string | no | Last known file commit id. Will be only considered in update, move and delete actions. |
 | `execute_filemode` | boolean | no | When `true/false` enables/disables the execute flag on the file. Only considered for `chmod` action. |

doc/api/merge_requests.md

@@ -1286,6 +1286,62 @@ If the rebase operation fails, the response will include the following:
 }
 ```
 
+## Rebase a merge request
+
+Automatically rebase the `source_branch` of the merge request against its
+`target_branch`.
+
+If you don't have permissions to push to the merge request's source branch -
+you'll get a `403 Forbidden` response.
+
+```
+PUT /projects/:id/merge_requests/:merge_request_iid/rebase
+```
+
+| Attribute           | Type    | Required | Description                          |
+| ---------           | ----    | -------- | -----------                          |
+| `id`                | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user                  |
+| `merge_request_iid` | integer | yes      | The internal ID of the merge request |
+
+```bash
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/76/merge_requests/1/rebase
+```
+
+This is an asynchronous request. The API will return an empty `202 Accepted`
+response if the request is enqueued successfully.
+
+You can poll the [Get single MR](#get-single-mr) endpoint with the
+`include_rebase_in_progress` parameter to check the status of the
+asynchronous request.
+
+If the rebase operation is ongoing, the response will include the following:
+
+```json
+{
+  "rebase_in_progress": true
+  "merge_error": null
+}
+```
+
+Once the rebase operation has completed successfully, the response will include
+the following:
+
+```json
+{
+  "rebase_in_progress": false,
+  "merge_error": null,
+}
+```
+
+If the rebase operation fails, the response will include the following:
+
+```json
+{
+  "rebase_in_progress": false,
+  "merge_error": "Rebase failed. Please rebase locally",
+}
+```
+
 ## Comments on merge requests
 
 Comments are done via the [notes](notes.md) resource.

doc/development/code_review.md

@@ -53,6 +53,8 @@ from teams other than your own.
 
 #### Security requirements
 
+   1. If your merge request is processing, storing, or transferring any kind of [RED or ORANGE data][https://docs.google.com/document/d/15eNKGA3zyZazsJMldqTBFbYMnVUSQSpU14lo22JMZQY/edit] (this is a confidential document), it must be
+      **approved by a [Security Engineer][team]**.
    1. If your merge request involves implementing, utilizing, or is otherwise related to any type of authentication, authorization, or session handling mechanism, it must be
       **approved by a [Security Engineer][team]**.
    1. If your merge request has a goal which requires a cryptographic function such as: confidentiality, integrity, authentication, or non-repudiation, it must be
@@ -85,6 +87,23 @@ If an author is unsure if a merge request needs a domain expert's opinion, that'
 usually a pretty good sign that it does, since without it the required level of
 confidence in their solution will not have been reached.
 
+Before the review, the author is requested to submit comments on the merge
+request diff alerting the reviewer to anything important as well as for anything
+that demands further explanation or attention.  Examples of content that may
+warrant a comment could be:
+
+- The addition of a linting rule (Rubocop, JS etc)
+- The addition of a library (Ruby gem, JS lib etc)
+- Where not obvious, a link to the parent class or method
+- Any benchmarking performed to complement the change
+- Potentially insecure code
+
+Do not add these comments directly to the source code, unless the
+reviewer requires you to do so.
+
+This
+[saves reviewers time and helps authors catch mistakes earlier](https://www.ibm.com/developerworks/rational/library/11-proven-practices-for-peer-review/index.html#__RefHeading__97_174136755).
+
 ### The responsibility of the maintainer
 
 Maintainers are responsible for the overall health, quality, and consistency of

doc/development/fe_guide/graphql.md

+# GraphQL
+
+We use [Apollo] and [Vue Apollo][vue-apollo] for working with GraphQL
+on the frontend.
+
+In order to use GraphQL, you need to enable the `graphql` feature flag,
+read more about [Feature Flags][feature-flags].
+
+## Apollo Client
+
+To save duplicated clients getting created in different apps, we have a
+[default client][defualt-client] that should be used. This setups the
+Apollo client with the correct URL and also sets the CSRF headers.
+
+## GraphQL Queries
+
+To save query compilation at runtime, webpack can directly import `.graphql`
+files. This allows webpack to preprocess the query at compile time instead
+of the client doing compilation of queries.
+
+## Usage in Vue
+
+To use Vue Apollo, import the [Vue Apollo][vue-apollo] plugin as well
+as the default client. This should be created at the same point
+the Vue application is mounted.
+
+```javascript
+import Vue from 'vue';
+import VueApollo from 'vue-apollo';
+import defaultClient from '~/lib/graphql';
+Vue.use(VueApollo);
+
+const apolloProvider = new VueApollo({
+  defaultClient,
+});
+
+new Vue({
+  ...,
+  apolloProvider,
+  ...
+});
+```
+
+Read more about [Vue Apollo][vue-apollo] in the [Vue Apollo documentation][vue-apollo-docs].
+
+### Testing
+
+With [Vue test utils][vue-test-utils] it is easy to quickly test components that
+fetch GraphQL queries. The simplest way is to use `shallowMount` and then set
+the data on the component
+
+```javascript
+it('tests apollo component', () => {
+  const vm = shallowMount(App);
+
+  vm.setData({
+    ...mock data
+  });
+});
+```
+
+## Usage outside of Vue
+
+It is also possible to use GraphQL outside of Vue by directly importing
+and using the default client with queries.
+
+```javascript
+import defaultClient from '~/lib/graphql';
+import query from './query.graphql';
+
+defaultClient.query(query)
+  .then(result => console.log(result));
+```
+
+Read more about the [Apollo] client in the [Apollo documentation][apollo-client-docs].
+
+[Apollo]: https://www.apollographql.com/
+[vue-apollo]: https://github.com/Akryum/vue-apollo/
+[vue-apollo-docs]: https://akryum.github.io/vue-apollo/
+[feature-flags]: ../feature_flags.md
+[default-client]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/lib/graphql.js
+[apollo-client-docs]: https://www.apollographql.com/docs/tutorial/client.html
+[vue-test-utils]: https://vue-test-utils.vuejs.org/

doc/development/fe_guide/index.md

@@ -54,6 +54,9 @@ Vuex specific design patterns and practices.
 ## [Axios](axios.md)
 Axios specific practices and gotchas.
 
+## [GraphQL](graphql.md)
+How to use GraphQL
+
 ## [Icons and Illustrations](icons.md)
 How we use SVG for our Icons and Illustrations.
 

doc/raketasks/backup_restore.md

@@ -604,6 +604,8 @@ If you fail to restore this encryption key file along with the application data
 backup, users with two-factor authentication enabled and GitLab Runners will
 lose access to your GitLab server.
 
+You may also want to restore any TLS keys, certificates, or [SSH host keys](https://superuser.com/questions/532040/copy-ssh-keys-from-one-server-to-another-server/532079#532079).
+
 Depending on your case, you might want to run the restore command with one or
 more of the following options:
 

doc/topics/authentication/index.md

@@ -44,6 +44,5 @@ This page gathers all the resources for the topic **Authentication** within GitL
 
 - [Kanboard Plugin GitLab Authentication](https://github.com/kanboard/plugin-gitlab-auth)
 - [Jenkins GitLab OAuth Plugin](https://wiki.jenkins.io/display/JENKINS/GitLab+OAuth+Plugin)
-- [Set up Gitlab CE with Active Directory authentication](https://www.caseylabs.com/setup-gitlab-ce-with-active-directory-authentication/)
 - [How to customize GitLab to support OpenID authentication](http://eric.van-der-vlist.com/blog/2013/11/23/how-to-customize-gitlab-to-support-openid-authentication/)
 - [Openshift - Configuring Authentication and User Agent](https://docs.openshift.org/latest/install_config/configuring_authentication.html#GitLab)

doc/user/project/clusters/index.md

@@ -239,13 +239,10 @@ by GitLab before installing any of the above applications.
 ## Getting the external IP address
 
 NOTE: **Note:**
-You need a load balancer installed in your cluster in order to obtain the
-external IP address with the following procedure. It can be deployed using the
-[**Ingress** application](#installing-applications).
-
-NOTE: **Note:**
-Knative will include its own load balancer in the form of [Istio](https://istio.io).
-At this time, to determine the external IP address, you will need to follow the manual approach.
+With the following procedure, a load balancer must be installed in your cluster
+to obtain the external IP address. You can use either
+[Ingress](#installing-applications), or Knative's own load balancer
+([Istio](https://istio.io)) if using [Knative](#installing-applications).
 
 In order to publish your web application, you first need to find the external IP
 address associated to your load balancer.
@@ -254,7 +251,7 @@ address associated to your load balancer.
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17052) in GitLab 10.6.
 
-If you installed the Ingress [via the **Applications**](#installing-applications),
+If you [installed Ingress or Knative](#installing-applications),
 you should see the Ingress IP address on this same page within a few minutes.
 If you don't see this, GitLab might not be able to determine the IP address of
 your ingress application in which case you should manually determine it.

doc/user/project/merge_requests/resolve_conflicts.md

-# Merge conflict resolution
+# Merge request conflict resolution
 
-> [Introduced][ce-5479] in GitLab 8.11.
+Merge conflicts occur when two branches have different changes that cannot be
+merged automatically.
 
-When a merge request has conflicts, GitLab may provide the option to resolve
-those conflicts in the GitLab UI. (See
-[conflicts available for resolution](#conflicts-available-for-resolution) for
-more information on when this is available.) If this is an option, you will see
-a **resolve these conflicts** link in the merge request widget:
+Git is able to automatically merge changes between branches in most cases, but
+there are situations where Git will require your assistance to resolve the
+conflicts manually. Typically, this is necessary when people change the same
+parts of the same files.
+
+GitLab will prevent merge requests from being merged until all conflicts are
+resolved. Conflicts can be resolved locally, or in many cases within GitLab
+(see [conflicts available for resolution](#conflicts-available-for-resolution)
+for information on when this is available).
 
 ![Merge request widget](img/merge_request_widget.png)
 
+NOTE: **Note:**
+GitLab resolves conflicts by creating a merge commit in the source branch that
+is not automatically merged into the target branch. This allows the merge
+commit to be reviewed and tested before the changes are merged, preventing
+unintended changes entering the target branch without review or breaking the
+build.
+
+## Resolve conflicts: interactive mode
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5479) in GitLab 8.11.
+
 Clicking this will show a list of files with conflicts, with conflict sections
 highlighted:
 
@@ -21,9 +37,9 @@ request into the source branch, resolving the conflicts using the options
 chosen. If the source branch is `feature` and the target branch is `master`,
 this is similar to performing `git checkout feature; git merge master` locally.
 
-## Merge conflict editor
+## Resolve conflicts: inline editor
 
-> Introduced in GitLab 8.13.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6374) in GitLab 8.13.
 
 The merge conflict resolution editor allows for more complex merge conflicts,
 which require the user to manually modify a file in order to resolve a conflict,
@@ -50,5 +66,3 @@ Additionally, GitLab does not detect conflicts in renames away from a path. For
 example, this will not create a conflict: on branch `a`, doing `git mv file1
 file2`; on branch `b`, doing `git mv file1 file3`. Instead, both files will be
 present in the branch after the merge request is merged.
-
-[ce-5479]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5479

doc/workflow/notifications.md

@@ -64,6 +64,8 @@ Below is the table of events users can be notified of:
 |------------------------------|-------------------------------------------------------------------|------------------------------|
 | New SSH key added            | User                                                              | Security email, always sent. |
 | New email added              | User                                                              | Security email, always sent. |
+| Email changed                | User                                                              | Security email, always sent. |
+| Password changed             | User                                                              | Security email, always sent. |
 | New user created             | User                                                              | Sent on user creation, except for omniauth (LDAP)|
 | User added to project        | User                                                              | Sent when user is added to project |
 | Project access level changed | User                                                              | Sent when user project access level is changed |

lib/api/api_guard.rb

@@ -95,6 +95,7 @@ module API
                          Gitlab::Auth::TokenNotFoundError,
                          Gitlab::Auth::ExpiredError,
                          Gitlab::Auth::RevokedError,
+                         Gitlab::Auth::ImpersonationDisabled,
                          Gitlab::Auth::InsufficientScopeError]
 
         base.__send__(:rescue_from, *error_classes, oauth2_bearer_token_error_handler) # rubocop:disable GitlabSecurity/PublicSend
@@ -122,6 +123,11 @@ module API
                 :invalid_token,
                 "Token was revoked. You have to re-authorize from the user.")
 
+            when Gitlab::Auth::ImpersonationDisabled
+              Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new(
+                :invalid_token,
+                "Token is an impersonation token but impersonation was disabled.")
+
             when Gitlab::Auth::InsufficientScopeError
               # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2)
               # does not include WWW-Authenticate header, which breaks the standard.

lib/banzai/filter/spaced_link_filter.rb

@@ -17,6 +17,9 @@ module Banzai
     # This is a small extension to the CommonMark spec.  If they start allowing
     # spaces in urls, we could then remove this filter.
     #
+    # Note: Filter::SanitizationFilter should always be run sometime after this filter
+    # to prevent XSS attacks
+    #
     class SpacedLinkFilter < HTML::Pipeline::Filter
       include ActionView::Helpers::TagHelper
 

lib/banzai/pipeline/gfm_pipeline.rb

@@ -14,13 +14,16 @@ module Banzai
       def self.filters
         @filters ||= FilterArray[
           Filter::PlantumlFilter,
+
+          # Must always be before the SanitizationFilter to prevent XSS attacks
+          Filter::SpacedLinkFilter,
+
           Filter::SanitizationFilter,
           Filter::SyntaxHighlightFilter,
 
           Filter::MathFilter,
           Filter::ColorFilter,
           Filter::MermaidFilter,
-          Filter::SpacedLinkFilter,
           Filter::VideoLinkFilter,
           Filter::ImageLazyLoadFilter,
           Filter::ImageLinkFilter,

lib/gitlab/auth/user_auth_finders.rb

@@ -7,6 +7,7 @@ module Gitlab
     TokenNotFoundError = Class.new(AuthenticationError)
     ExpiredError = Class.new(AuthenticationError)
     RevokedError = Class.new(AuthenticationError)
+    ImpersonationDisabled = Class.new(AuthenticationError)
     UnauthorizedError = Class.new(AuthenticationError)
 
     class InsufficientScopeError < AuthenticationError
@@ -69,6 +70,8 @@ module Gitlab
           raise ExpiredError
         when AccessTokenValidationService::REVOKED
           raise RevokedError
+        when AccessTokenValidationService::IMPERSONATION_DISABLED
+          raise ImpersonationDisabled
         end
       end
 

lib/gitlab/gitaly_client/operation_service.rb

@@ -385,7 +385,8 @@ module Gitlab
           file_path: encode_binary(action[:file_path]),
           previous_path: encode_binary(action[:previous_path]),
           base64_content: action[:encoding] == 'base64',
-          execute_filemode: !!action[:execute_filemode]
+          execute_filemode: !!action[:execute_filemode],
+          infer_content: !!action[:infer_content]
         )
       rescue RangeError
         raise ArgumentError, "Unknown action '#{action[:action]}'"

lib/gitlab/url_blocker.rb

 # frozen_string_literal: true
 
 require 'resolv'
+require 'ipaddress'
 
 module Gitlab
   class UrlBlocker
@@ -10,11 +11,8 @@ module Gitlab
       def validate!(url, allow_localhost: false, allow_local_network: true, enforce_user: false, ports: [], protocols: [])
         return true if url.nil?
 
-        begin
-          uri = Addressable::URI.parse(url)
-        rescue Addressable::URI::InvalidURIError
-          raise BlockedUrlError, "URI is invalid"
-        end
+        # Param url can be a string, URI or Addressable::URI
+        uri = parse_url(url)
 
         # Allow imports from the GitLab instance itself but only from the configured ports
         return true if internal?(uri)
@@ -26,7 +24,9 @@ module Gitlab
         validate_hostname!(uri.hostname)
 
         begin
-          addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM)
+          addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr|
+            addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
+          end
         rescue SocketError
           return true
         end
@@ -49,6 +49,18 @@ module Gitlab
 
       private
 
+      def parse_url(url)
+        raise Addressable::URI::InvalidURIError if multiline?(url)
+
+        Addressable::URI.parse(url)
+      rescue Addressable::URI::InvalidURIError, URI::InvalidURIError
+        raise BlockedUrlError, 'URI is invalid'
+      end
+
+      def multiline?(url)
+        CGI.unescape(url.to_s) =~ /\n|\r/
+      end
+
       def validate_port!(port, ports)
         return if port.blank?
         # Only ports under 1024 are restricted
@@ -73,13 +85,14 @@ module Gitlab
 
       def validate_hostname!(value)
         return if value.blank?
+        return if IPAddress.valid?(value)
         return if value =~ /\A\p{Alnum}/
 
-        raise BlockedUrlError, "Hostname needs to start with an alphanumeric character"
+        raise BlockedUrlError, "Hostname or IP address invalid"
       end
 
       def validate_localhost!(addrs_info)
-        local_ips = ["127.0.0.1", "::1", "0.0.0.0"]
+        local_ips = ["::", "0.0.0.0"]
         local_ips.concat(Socket.ip_address_list.map(&:ip_address))
 
         return if (local_ips & addrs_info.map(&:ip_address)).empty?
@@ -94,7 +107,7 @@ module Gitlab
       end
 
       def validate_local_network!(addrs_info)
-        return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
+        return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? || addr.ipv6_unique_local? }
 
         raise BlockedUrlError, "Requests to the local network are not allowed"
       end
@@ -111,11 +124,13 @@ module Gitlab
       end
 
       def internal_web?(uri)
+        uri.scheme == config.gitlab.protocol &&
           uri.hostname == config.gitlab.host &&
           (uri.port.blank? || uri.port == config.gitlab.port)
       end
 
       def internal_shell?(uri)
+        uri.scheme == 'ssh' &&
           uri.hostname == config.gitlab_shell.ssh_host &&
           (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
       end

locale/gitlab.pot

@@ -188,6 +188,9 @@ msgstr ""
 msgid "%{usage_ping_link_start}Learn more%{usage_ping_link_end} about what information is shared with GitLab Inc."
 msgstr ""
 
+msgid "%{user_name} profile page"
+msgstr ""
+
 msgid "+ %{count} more"
 msgstr ""
 
@@ -1363,6 +1366,9 @@ msgstr ""
 msgid "Business metrics (Custom)"
 msgstr ""
 
+msgid "By %{user_name}"
+msgstr ""
+
 msgid "ByAuthor|by"
 msgstr ""
 
@@ -3016,6 +3022,9 @@ msgstr ""
 msgid "Edit application"
 msgstr ""
 
+msgid "Edit comment"
+msgstr ""
+
 msgid "Edit environment"
 msgstr ""
 
@@ -4470,6 +4479,9 @@ msgstr ""
 msgid "ImageDiffViewer|Swipe"
 msgstr ""
 
+msgid "Impersonation has been disabled"
+msgstr ""
+
 msgid "Import"
 msgstr ""
 
@@ -4592,6 +4604,9 @@ msgstr ""
 msgid "Interested parties can even contribute by pushing commits if they want to."
 msgstr ""
 
+msgid "Internal"
+msgstr ""
+
 msgid "Internal - The group and any internal projects can be viewed by any logged in user."
 msgstr ""
 
@@ -5738,6 +5753,9 @@ msgstr ""
 msgid "Notes|Show history only"
 msgstr ""
 
+msgid "Nothing here."
+msgstr ""
+
 msgid "Notification events"
 msgstr ""
 
@@ -6256,6 +6274,9 @@ msgstr ""
 msgid "Prioritized label"
 msgstr ""
 
+msgid "Private"
+msgstr ""
+
 msgid "Private - Project access must be granted explicitly to each user."
 msgstr ""
 
@@ -6823,6 +6844,9 @@ msgstr ""
 msgid "Pseudonymizer data collection"
 msgstr ""
 
+msgid "Public"
+msgstr ""
+
 msgid "Public - The group and any public projects can be viewed without any authentication."
 msgstr ""
 

rubocop/cop/migration/add_reference.rb

@@ -8,7 +8,7 @@ module RuboCop
       class AddReference < RuboCop::Cop::Cop
         include MigrationHelpers
 
-        MSG = '`add_reference` requires `index: true`'
+        MSG = '`add_reference` requires `index: true` or `index: { options... }`'
 
         def on_send(node)
           return unless in_migration?(node)
@@ -33,7 +33,12 @@ module RuboCop
         private
 
         def index_enabled?(pair)
-          hash_key_type(pair) == :sym && hash_key_name(pair) == :index && pair.children[1].true_type?
+          return unless hash_key_type(pair) == :sym
+          return unless hash_key_name(pair) == :index
+
+          index = pair.children[1]
+
+          index.true_type? || index.hash_type?
         end
 
         def hash_key_type(pair)

spec/controllers/admin/users_controller_spec.rb

@@ -264,5 +264,17 @@ describe Admin::UsersController do
         expect(flash[:alert]).to eq("You are now impersonating #{user.username}")
       end
     end
+
+    context "when impersonation is disabled" do
+      before do
+        stub_config_setting(impersonation_enabled: false)
+      end
+
+      it "shows error page" do
+        post :impersonate, id: user.username
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
   end
 end

spec/controllers/dashboard/todos_controller_spec.rb

@@ -42,6 +42,16 @@ describe Dashboard::TodosController do
       end
     end
 
+    context 'group authorization' do
+      it 'renders 404 when user does not have read access on given group' do
+        unauthorized_group = create(:group, :private)
+
+        get :index, group_id: unauthorized_group.id
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context 'when using pagination' do
       let(:last_page) { user.todos.page.total_pages }
       let!(:issues) { create_list(:issue, 3, project: project, assignees: [user]) }

spec/controllers/oauth/applications_controller_spec.rb

@@ -40,6 +40,23 @@ describe Oauth::ApplicationsController do
         expect(response).to have_gitlab_http_status(302)
         expect(response).to redirect_to(profile_path)
       end
+
+      context 'redirect_uri' do
+        render_views
+
+        it 'shows an error for a forbidden URI' do
+          invalid_uri_params = {
+            doorkeeper_application: {
+              name: 'foo',
+              redirect_uri: 'javascript://alert()'
+            }
+          }
+
+          post :create, invalid_uri_params
+
+          expect(response.body).to include 'Redirect URI is forbidden by the server'
+        end
+      end
     end
   end
 

spec/controllers/projects/environments_controller_spec.rb

@@ -217,7 +217,10 @@ describe Projects::EnvironmentsController do
       end
 
       it 'loads the terminals for the environment' do
-        expect_any_instance_of(Environment).to receive(:terminals)
+        # In EE we have to stub EE::Environment since it overwrites the
+        # "terminals" method.
+        expect_any_instance_of(defined?(EE) ? EE::Environment : Environment)
+          .to receive(:terminals)
 
         get :terminal, environment_params
       end
@@ -240,7 +243,9 @@ describe Projects::EnvironmentsController do
 
       context 'and valid id' do
         it 'returns the first terminal for the environment' do
-          expect_any_instance_of(Environment)
+          # In EE we have to stub EE::Environment since it overwrites the
+          # "terminals" method.
+          expect_any_instance_of(defined?(EE) ? EE::Environment : Environment)
             .to receive(:terminals)
             .and_return([:fake_terminal])
 

spec/controllers/projects/milestones_controller_spec.rb

@@ -143,11 +143,27 @@ describe Projects::MilestonesController do
   end
 
   describe '#promote' do
+    let(:group) { create(:group) }
+
+    before do
+      project.update(namespace: group)
+    end
+
+    context 'when user does not have permission to promote milestone' do
+      before do
+        group.add_guest(user)
+      end
+
+      it 'renders 404' do
+        post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
     context 'promotion succeeds' do
       before do
-        group = create(:group)
         group.add_developer(user)
-        milestone.project.update(namespace: group)
       end
 
       it 'shows group milestone' do
@@ -166,12 +182,17 @@ describe Projects::MilestonesController do
       end
     end
 
-    context 'promotion fails' do
-      it 'shows project milestone' do
+    context 'when user cannot admin group milestones' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'renders 404' do
+        project.update(namespace: user.namespace)
+
         post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid
 
-        expect(response).to redirect_to(project_milestone_path(project, milestone))
-        expect(flash[:alert]).to eq('Promotion failed - Project does not belong to a group.')
+        expect(response).to have_gitlab_http_status(404)
       end
     end
   end

spec/factories/ci/builds.rb

@@ -308,7 +308,7 @@ FactoryBot.define do
 
     trait :with_runner_session do
       after(:build) do |build|
-        build.build_runner_session(url: 'ws://localhost')
+        build.build_runner_session(url: 'https://localhost')
       end
     end
   end

spec/features/admin/admin_users_spec.rb

@@ -205,77 +205,120 @@ describe "Admin::Users" do
     describe 'Impersonation' do
       let(:another_user) { create(:user) }
 
-      before do
-        visit admin_user_path(another_user)
-      end
-
       context 'before impersonating' do
+        subject { visit admin_user_path(user_to_visit) }
+
+        let(:user_to_visit) { another_user }
+
+        context 'for other users' do
           it 'shows impersonate button for other users' do
+            subject
+
             expect(page).to have_content('Impersonate')
           end
+        end
+
+        context 'for admin itself' do
+          let(:user_to_visit) { current_user }
 
           it 'does not show impersonate button for admin itself' do
-          visit admin_user_path(current_user)
+            subject
 
             expect(page).not_to have_content('Impersonate')
           end
+        end
 
-        it 'does not show impersonate button for blocked user' do
+        context 'for blocked user' do
+          before do
             another_user.block
+          end
 
-          visit admin_user_path(another_user)
+          it 'does not show impersonate button for blocked user' do
+            subject
 
             expect(page).not_to have_content('Impersonate')
+          end
+        end
 
-          another_user.activate
+        context 'when impersonation is disabled' do
+          before do
+            stub_config_setting(impersonation_enabled: false)
+          end
+
+          it 'does not show impersonate button' do
+            subject
+
+            expect(page).not_to have_content('Impersonate')
+          end
         end
       end
 
       context 'when impersonating' do
+        subject { click_link 'Impersonate' }
+
         before do
-          click_link 'Impersonate'
+          visit admin_user_path(another_user)
         end
 
         it 'logs in as the user when impersonate is clicked' do
+          subject
+
           expect(page.find(:css, '.header-user .profile-link')['data-user']).to eql(another_user.username)
         end
 
         it 'sees impersonation log out icon' do
-          icon = first('.fa.fa-user-secret')
+          subject
 
+          icon = first('.fa.fa-user-secret')
           expect(icon).not_to be nil
         end
 
+        context 'a user with an expired password' do
+          before do
+            another_user.update(password_expires_at: Time.now - 5.minutes)
+          end
+
+          it 'does not redirect to password change page' do
+            subject
+
+            expect(current_path).to eq('/')
+          end
+        end
+      end
+
+      context 'ending impersonation' do
+        subject { find(:css, 'li.impersonation a').click }
+
+        before do
+          visit admin_user_path(another_user)
+          click_link 'Impersonate'
+        end
+
         it 'logs out of impersonated user back to original user' do
-          find(:css, 'li.impersonation a').click
+          subject
 
           expect(page.find(:css, '.header-user .profile-link')['data-user']).to eq(current_user.username)
         end
 
         it 'is redirected back to the impersonated users page in the admin after stopping' do
-          find(:css, 'li.impersonation a').click
+          subject
 
           expect(current_path).to eq("/admin/users/#{another_user.username}")
         end
-      end
 
-      context 'when impersonating a user with an expired password' do
+        context 'a user with an expired password' do
           before do
             another_user.update(password_expires_at: Time.now - 5.minutes)
-          click_link 'Impersonate'
-        end
-
-        it 'does not redirect to password change page' do
-          expect(current_path).to eq('/')
           end
 
           it 'is redirected back to the impersonated users page in the admin after stopping' do
-          find(:css, 'li.impersonation a').click
+            subject
 
             expect(current_path).to eq("/admin/users/#{another_user.username}")
           end
         end
       end
+    end
 
     describe 'Two-factor Authentication status' do
       it 'shows when enabled' do

spec/features/issues/user_comments_on_issue_spec.rb

@@ -40,6 +40,18 @@ describe "User comments on issue", :js do
 
       expect(page.find('pre code').text).to eq code_block_content
     end
+
+    it "does not render html content in mermaid" do
+      html_content = "<img onerror=location=`javascript\\u003aalert\\u0028document.domain\\u0029` src=x>"
+      mermaid_content = "graph LR\n  B-->D(#{html_content});"
+      comment = "```mermaid\n#{mermaid_content}\n```"
+
+      add_note(comment)
+
+      wait_for_requests
+
+      expect(page.find('svg.mermaid')).to have_content html_content
+    end
   end
 
   context "when editing comments" do

spec/features/issues/user_sees_breadcrumb_links_spec.rb

 require 'rails_helper'
 
-describe 'New issue breadcrumbs' do
+describe 'New issue breadcrumb' do
   let(:project) { create(:project) }
   let(:user) { project.creator }
 
   before do
     sign_in(user)
-    visit new_project_issue_path(project)
+    visit(new_project_issue_path(project))
   end
 
-  it 'display a link to project issues and new issue pages' do
+  it 'displays link to project issues and new issue' do
     page.within '.breadcrumbs' do
       expect(find_link('Issues')[:href]).to end_with(project_issues_path(project))
       expect(find_link('New')[:href]).to end_with(new_project_issue_path(project))

spec/features/markdown/mermaid_spec.rb

@@ -18,7 +18,7 @@ describe 'Mermaid rendering', :js do
     visit project_issue_path(project, issue)
 
     %w[A B C D].each do |label|
-      expect(page).to have_selector('svg foreignObject', text: label)
+      expect(page).to have_selector('svg text', text: label)
     end
   end
 end

spec/features/merge_request/user_sees_breadcrumb_links_spec.rb

 require 'rails_helper'
 
-describe 'New merge request breadcrumbs' do
+describe 'New merge request breadcrumb' do
   let(:project) { create(:project, :repository) }
   let(:user) { project.creator }
 
   before do
     sign_in(user)
-    visit project_new_merge_request_path(project)
+    visit(project_new_merge_request_path(project))
   end
 
-  it 'display a link to project merge requests and new merge request pages' do
+  it 'displays link to project merge requests and new merge request' do
     page.within '.breadcrumbs' do
       expect(find_link('Merge Requests')[:href]).to end_with(project_merge_requests_path(project))
       expect(find_link('New')[:href]).to end_with(project_new_merge_request_path(project))

spec/features/milestones/user_promotes_milestone_spec.rb

+require 'rails_helper'
+
+describe 'User promotes milestone' do
+  set(:group) { create(:group) }
+  set(:user) { create(:user) }
+  set(:project) { create(:project, namespace: group) }
+  set(:milestone) { create(:milestone, project: project) }
+
+  context 'when user can admin group milestones' do
+    before do
+      group.add_developer(user)
+      sign_in(user)
+      visit(project_milestones_path(project))
+    end
+
+    it "shows milestone promote button" do
+      expect(page).to have_selector('.js-promote-project-milestone-button')
+    end
+  end
+
+  context 'when user cannot admin group milestones' do
+    before do
+      project.add_developer(user)
+      sign_in(user)
+      visit(project_milestones_path(project))
+    end
+
+    it "does not show milestone promote button" do
+      expect(page).not_to have_selector('.js-promote-project-milestone-button')
+    end
+  end
+end

spec/features/milestones/user_sees_breadcrumb_links_spec.rb

+require 'rails_helper'
+
+describe 'New project milestone breadcrumb' do
+  let(:project) { create(:project) }
+  let(:milestone) { create(:milestone, project: project) }
+  let(:user) { project.creator }
+
+  before do
+    sign_in(user)
+    visit(new_project_milestone_path(project))
+  end
+
+  it 'displays link to project milestones and new project   milestone' do
+    page.within '.breadcrumbs' do
+      expect(find_link('Milestones')[:href]).to end_with(project_milestones_path(project))
+      expect(find_link('New')[:href]).to end_with(new_project_milestone_path(project))
+    end
+  end
+end

spec/features/projects/commits/user_browses_commits_spec.rb

@@ -4,10 +4,9 @@ describe 'User browses commits' do
   include RepoHelpers
 
   let(:user) { create(:user) }
-  let(:project) { create(:project, :repository, namespace: user.namespace) }
+  let(:project) { create(:project, :public, :repository, namespace: user.namespace) }
 
   before do
-    project.add_maintainer(user)
     sign_in(user)
   end
 
@@ -127,6 +126,26 @@ describe 'User browses commits' do
         .and have_selector('entry summary', text: commit.description[0..10].delete("\r\n"))
     end
 
+    context 'when a commit links to a confidential issue' do
+      let(:confidential_issue) { create(:issue, confidential: true, title: 'Secret issue!', project: project) }
+
+      before do
+        project.repository.create_file(user, 'dummy-file', 'dummy content',
+                                       branch_name: 'feature',
+                                       message: "Linking #{confidential_issue.to_reference}")
+      end
+
+      context 'when the user cannot see confidential issues but was cached with a link', :use_clean_rails_memory_store_fragment_caching do
+        it 'does not render the confidential issue' do
+          visit project_commits_path(project, 'feature')
+          sign_in(create(:user))
+          visit project_commits_path(project, 'feature')
+
+          expect(page).not_to have_link(href: project_issue_path(project, confidential_issue))
+        end
+      end
+    end
+
     context 'master branch' do
       before do
         visit_commits_page

spec/features/projects/environments/environment_spec.rb

@@ -165,8 +165,14 @@ describe 'Environment' do
 
                 context 'web terminal', :js do
                   before do
-                    # Stub #terminals as it causes js-enabled feature specs to render the page incorrectly
-                    allow_any_instance_of(Environment).to receive(:terminals) { nil }
+                    # Stub #terminals as it causes js-enabled feature specs to
+                    # render the page incorrectly
+                    #
+                    # In EE we have to stub EE::Environment since it overwrites
+                    # the "terminals" method.
+                    allow_any_instance_of(defined?(EE) ? EE::Environment : Environment)
+                      .to receive(:terminals) { nil }
+
                     visit terminal_project_environment_path(project, environment)
                   end
 

spec/features/projects/labels/user_sees_breadcrumb_links_spec.rb

+require 'rails_helper'
+
+describe 'New project label breadcrumb' do
+  let(:project) { create(:project) }
+  let(:user) { project.creator }
+
+  before do
+    sign_in(user)
+    visit(project_labels_path(project))
+  end
+
+  it 'displays link to project labels and new project label' do
+    page.within '.breadcrumbs' do
+      expect(find_link('Labels')[:href]).to end_with(project_labels_path(project))
+    end
+  end
+end

spec/features/projects/wiki/user_creates_wiki_page_spec.rb

@@ -85,7 +85,7 @@ describe "User creates wiki page" do
         expect(current_path).to eq(project_wiki_path(project, "test"))
 
         page.within(:css, ".nav-text") do
-          expect(page).to have_content("Test").and have_content("Create Page")
+          expect(page).to have_content("test").and have_content("Create Page")
         end
 
         click_link("Home")
@@ -97,7 +97,7 @@ describe "User creates wiki page" do
         expect(current_path).to eq(project_wiki_path(project, "api"))
 
         page.within(:css, ".nav-text") do
-          expect(page).to have_content("Create").and have_content("Api")
+          expect(page).to have_content("Create").and have_content("api")
         end
 
         click_link("Home")
@@ -109,7 +109,7 @@ describe "User creates wiki page" do
         expect(current_path).to eq(project_wiki_path(project, "raketasks"))
 
         page.within(:css, ".nav-text") do
-          expect(page).to have_content("Create").and have_content("Rake")
+          expect(page).to have_content("Create").and have_content("rake")
         end
       end
 
@@ -200,7 +200,7 @@ describe "User creates wiki page" do
             click_button("Create page")
           end
 
-          expect(page).to have_content("Foo")
+          expect(page).to have_content("foo")
                      .and have_content("Last edited by #{user.name}")
                      .and have_content("My awesome wiki!")
         end
@@ -215,7 +215,7 @@ describe "User creates wiki page" do
           end
 
           # Commit message field should have correct value.
-          expect(page).to have_field("wiki[message]", with: "Create spaces in the name")
+          expect(page).to have_field("wiki[message]", with: "Create Spaces in the name")
 
           page.within(".wiki-form") do
             fill_in(:wiki_content, with: "My awesome wiki!")
@@ -246,7 +246,7 @@ describe "User creates wiki page" do
             click_button("Create page")
           end
 
-          expect(page).to have_content("Hyphens in the name")
+          expect(page).to have_content("hyphens in the name")
                      .and have_content("Last edited by #{user.name}")
                      .and have_content("My awesome wiki!")
         end
@@ -293,7 +293,7 @@ describe "User creates wiki page" do
             click_button("Create page")
           end
 
-          expect(page).to have_content("Foo")
+          expect(page).to have_content("foo")
                      .and have_content("Last edited by #{user.name}")
                      .and have_content("My awesome wiki!")
         end
@@ -311,7 +311,7 @@ describe "User creates wiki page" do
       it 'renders a default sidebar when there is no customized sidebar' do
         visit(project_wikis_path(project))
 
-        expect(page).to have_content('Another')
+        expect(page).to have_content('another')
         expect(page).to have_content('More Pages')
       end
 

spec/features/projects/wiki/user_updates_wiki_page_spec.rb

@@ -39,11 +39,11 @@ describe 'User updates wiki page' do
         end
 
         expect(current_path).to include('one/two/three-test')
-        expect(find('.wiki-pages')).to have_content('Three')
+        expect(find('.wiki-pages')).to have_content('three')
 
-        first(:link, text: 'Three').click
+        first(:link, text: 'three').click
 
-        expect(find('.nav-text')).to have_content('Three')
+        expect(find('.nav-text')).to have_content('three')
 
         click_on('Edit')
 

spec/features/projects/wiki/user_views_wiki_page_spec.rb

@@ -38,7 +38,7 @@ describe 'User views a wiki page' do
     it 'shows the history of a page that has a path', :js do
       expect(current_path).to include('one/two/three-test')
 
-      first(:link, text: 'Three').click
+      first(:link, text: 'three').click
       click_on('Page history')
 
       expect(current_path).to include('one/two/three-test')
@@ -50,11 +50,11 @@ describe 'User views a wiki page' do
 
     it 'shows an old version of a page', :js do
       expect(current_path).to include('one/two/three-test')
-      expect(find('.wiki-pages')).to have_content('Three')
+      expect(find('.wiki-pages')).to have_content('three')
 
-      first(:link, text: 'Three').click
+      first(:link, text: 'three').click
 
-      expect(find('.nav-text')).to have_content('Three')
+      expect(find('.nav-text')).to have_content('three')
 
       click_on('Edit')
 

spec/lib/banzai/pipeline/gfm_pipeline_spec.rb

@@ -104,5 +104,17 @@ describe Banzai::Pipeline::GfmPipeline do
 
       expect(output).to include("src=\"test%20image.png\"")
     end
+
+    it 'sanitizes the fixed link' do
+      markdown_xss = "[xss](javascript: alert%28document.domain%29)"
+      output = described_class.to_html(markdown_xss, project: project)
+
+      expect(output).not_to include("javascript")
+
+      markdown_xss = "<invalidtag>\n[xss](javascript:alert%28document.domain%29)"
+      output = described_class.to_html(markdown_xss, project: project)
+
+      expect(output).not_to include("javascript")
+    end
   end
 end

spec/lib/gitlab/auth/user_auth_finders_spec.rb

@@ -279,5 +279,20 @@ describe Gitlab::Auth::UserAuthFinders do
         expect { validate_access_token!(scopes: [:sudo]) }.to raise_error(Gitlab::Auth::InsufficientScopeError)
       end
     end
+
+    context 'with impersonation token' do
+      let(:personal_access_token) { create(:personal_access_token, :impersonation, user: user) }
+
+      context 'when impersonation is disabled' do
+        before do
+          stub_config_setting(impersonation_enabled: false)
+          allow_any_instance_of(described_class).to receive(:access_token).and_return(personal_access_token)
+        end
+
+        it 'returns Gitlab::Auth::ImpersonationDisabled' do
+          expect { validate_access_token! }.to raise_error(Gitlab::Auth::ImpersonationDisabled)
+        end
+      end
+    end
   end
 end

spec/lib/gitlab/url_blocker_spec.rb

@@ -10,8 +10,8 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?(import_url)).to be false
     end
 
-    it 'allows imports from configured SSH host and port' do
-      import_url = "http://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
+    it 'allows mirroring from configured SSH host and port' do
+      import_url = "ssh://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git"
       expect(described_class.blocked_url?(import_url)).to be false
     end
 
@@ -29,24 +29,46 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git', protocols: ['http'])).to be true
     end
 
+    it 'returns true for bad protocol on configured web/SSH host and ports' do
+      web_url = "javascript://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.port}/t.git%0aalert(1)"
+      expect(described_class.blocked_url?(web_url)).to be true
+
+      ssh_url = "javascript://#{Gitlab.config.gitlab_shell.ssh_host}:#{Gitlab.config.gitlab_shell.ssh_port}/t.git%0aalert(1)"
+      expect(described_class.blocked_url?(ssh_url)).to be true
+    end
+
     it 'returns true for localhost IPs' do
+      expect(described_class.blocked_url?('https://[0:0:0:0:0:0:0:0]/foo/foo.git')).to be true
       expect(described_class.blocked_url?('https://0.0.0.0/foo/foo.git')).to be true
-      expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true
-      expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true
+      expect(described_class.blocked_url?('https://[::]/foo/foo.git')).to be true
     end
 
     it 'returns true for loopback IP' do
       expect(described_class.blocked_url?('https://127.0.0.2/foo/foo.git')).to be true
+      expect(described_class.blocked_url?('https://127.0.0.1/foo/foo.git')).to be true
+      expect(described_class.blocked_url?('https://[::1]/foo/foo.git')).to be true
     end
 
     it 'returns true for alternative version of 127.0.0.1 (0177.1)' do
       expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true
     end
 
+    it 'returns true for alternative version of 127.0.0.1 (017700000001)' do
+      expect(described_class.blocked_url?('https://017700000001:65535/foo/foo.git')).to be true
+    end
+
     it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do
       expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true
     end
 
+    it 'returns true for alternative version of 127.0.0.1 (0x7f.0.0.1)' do
+      expect(described_class.blocked_url?('https://0x7f.0.0.1:65535/foo/foo.git')).to be true
+    end
+
+    it 'returns true for alternative version of 127.0.0.1 (0x7f000001)' do
+      expect(described_class.blocked_url?('https://0x7f000001:65535/foo/foo.git')).to be true
+    end
+
     it 'returns true for alternative version of 127.0.0.1 (2130706433)' do
       expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true
     end
@@ -55,6 +77,27 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true
     end
 
+    it 'returns true for alternative version of 127.0.0.1 (127.0.1)' do
+      expect(described_class.blocked_url?('https://127.0.1:65535/foo/foo.git')).to be true
+    end
+
+    context 'with ipv6 mapped address' do
+      it 'returns true for localhost IPs' do
+        expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:0.0.0.0]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:0.0.0.0]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:0:0]/foo/foo.git')).to be true
+      end
+
+      it 'returns true for loopback IPs' do
+        expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.1]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:127.0.0.1]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:7f00:1]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[0:0:0:0:0:ffff:127.0.0.2]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:127.0.0.2]/foo/foo.git')).to be true
+        expect(described_class.blocked_url?('https://[::ffff:7f00:2]/foo/foo.git')).to be true
+      end
+    end
+
     it 'returns true for a non-alphanumeric hostname' do
       stub_resolv
 
@@ -78,7 +121,22 @@ describe Gitlab::UrlBlocker do
     end
 
     context 'when allow_local_network is' do
-      let(:local_ips) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
+      let(:local_ips) do
+        [
+          '192.168.1.2',
+          '[0:0:0:0:0:ffff:192.168.1.2]',
+          '[::ffff:c0a8:102]',
+          '10.0.0.2',
+          '[0:0:0:0:0:ffff:10.0.0.2]',
+          '[::ffff:a00:2]',
+          '172.16.0.2',
+          '[0:0:0:0:0:ffff:172.16.0.2]',
+          '[::ffff:ac10:20]',
+          '[feef::1]',
+          '[fee2::]',
+          '[fc00:bf8b:e62c:abcd:abcd:aaaa:aaaa:aaaa]'
+        ]
+      end
       let(:fake_domain) { 'www.fakedomain.fake' }
 
       context 'true (default)' do
@@ -109,10 +167,14 @@ describe Gitlab::UrlBlocker do
           expect(described_class).not_to be_blocked_url('http://169.254.168.100')
         end
 
-        # This is blocked due to the hostname check: https://gitlab.com/gitlab-org/gitlab-ce/issues/50227
-        it 'blocks IPv6 link-local endpoints' do
-          expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]')
-          expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]')
+        it 'allows IPv6 link-local endpoints' do
+          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.169.254]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a9fe]')
+          expect(described_class).not_to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:169.254.168.100]')
+          expect(described_class).not_to be_blocked_url('http://[::ffff:a9fe:a864]')
+          expect(described_class).not_to be_blocked_url('http://[fe80::c800:eff:fe74:8]')
         end
       end
 
@@ -135,14 +197,20 @@ describe Gitlab::UrlBlocker do
         end
 
         it 'blocks IPv6 link-local endpoints' do
+          expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.169.254]', allow_local_network: false)
           expect(described_class).to be_blocked_url('http://[::ffff:169.254.169.254]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a9fe]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[0:0:0:0:0:ffff:169.254.168.100]', allow_local_network: false)
           expect(described_class).to be_blocked_url('http://[::ffff:169.254.168.100]', allow_local_network: false)
-          expect(described_class).to be_blocked_url('http://[FE80::C800:EFF:FE74:8]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[::ffff:a9fe:a864]', allow_local_network: false)
+          expect(described_class).to be_blocked_url('http://[fe80::c800:eff:fe74:8]', allow_local_network: false)
         end
       end
 
       def stub_domain_resolv(domain, ip)
-        allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)])
+        address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)
+        allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address])
+        allow(address).to receive(:ipv6_v4mapped?).and_return(false)
       end
 
       def unstub_domain_resolv
@@ -183,6 +251,36 @@ describe Gitlab::UrlBlocker do
     end
   end
 
+  describe '#validate_hostname!' do
+    let(:ip_addresses) do
+      [
+        '2001:db8:1f70::999:de8:7648:6e8',
+        'FE80::C800:EFF:FE74:8',
+        '::ffff:127.0.0.1',
+        '::ffff:169.254.168.100',
+        '::ffff:7f00:1',
+        '0:0:0:0:0:ffff:0.0.0.0',
+        'localhost',
+        '127.0.0.1',
+        '127.000.000.001',
+        '0x7f000001',
+        '0x7f.0.0.1',
+        '0x7f.0.0.1',
+        '017700000001',
+        '0177.1',
+        '2130706433',
+        '::',
+        '::1'
+      ]
+    end
+
+    it 'does not raise error for valid Ip addresses' do
+      ip_addresses.each do |ip|
+        expect { described_class.send(:validate_hostname!, ip) }.not_to raise_error
+      end
+    end
+  end
+
   # Resolv does not support resolving UTF-8 domain names
   # See https://bugs.ruby-lang.org/issues/4270
   def stub_resolv

spec/migrations/cleanup_environments_external_url_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20181108091549_cleanup_environments_external_url.rb')
+
+describe CleanupEnvironmentsExternalUrl, :migration do
+  let(:environments)    { table(:environments) }
+  let(:invalid_entries) { environments.where(environments.arel_table[:external_url].matches('javascript://%')) }
+  let(:namespaces)      { table(:namespaces) }
+  let(:projects)        { table(:projects) }
+
+  before do
+    namespace = namespaces.create(name: 'foo', path: 'foo')
+    project = projects.create!(namespace_id: namespace.id)
+
+    environments.create!(id: 1, project_id: project.id, name: 'poisoned', slug: 'poisoned', external_url: 'javascript://alert("1")')
+  end
+
+  it 'clears every environment with a javascript external_url' do
+    expect do
+      subject.up
+    end.to change { invalid_entries.count }.from(1).to(0)
+  end
+
+  it 'do not removes environments' do
+    expect do
+      subject.up
+    end.not_to change { environments.count }
+  end
+end

spec/migrations/migrate_forbidden_redirect_uris_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20181026091631_migrate_forbidden_redirect_uris.rb')
+
+describe MigrateForbiddenRedirectUris, :migration do
+  let(:oauth_application) { table(:oauth_applications) }
+  let(:oauth_access_grant) { table(:oauth_access_grants) }
+
+  let!(:control_app) { oauth_application.create(random_params) }
+  let!(:control_access_grant) { oauth_application.create(random_params) }
+  let!(:forbidden_js_app) { oauth_application.create(random_params.merge(redirect_uri: 'javascript://alert()')) }
+  let!(:forbidden_vb_app) { oauth_application.create(random_params.merge(redirect_uri: 'VBSCRIPT://alert()')) }
+  let!(:forbidden_access_grant) { oauth_application.create(random_params.merge(redirect_uri: 'vbscript://alert()')) }
+
+  context 'oauth application' do
+    it 'migrates forbidden javascript URI' do
+      expect { migrate! }.to change { forbidden_js_app.reload.redirect_uri }.to('http://forbidden-scheme-has-been-overwritten')
+    end
+
+    it 'migrates forbidden VBScript URI' do
+      expect { migrate! }.to change { forbidden_vb_app.reload.redirect_uri }.to('http://forbidden-scheme-has-been-overwritten')
+    end
+
+    it 'does not migrate a valid URI' do
+      expect { migrate! }.not_to change { control_app.reload.redirect_uri }
+    end
+  end
+
+  context 'access grant' do
+    it 'migrates forbidden VBScript URI' do
+      expect { migrate! }.to change { forbidden_access_grant.reload.redirect_uri }.to('http://forbidden-scheme-has-been-overwritten')
+    end
+
+    it 'does not migrate a valid URI' do
+      expect { migrate! }.not_to change { control_access_grant.reload.redirect_uri }
+    end
+  end
+
+  def random_params
+    {
+      name: 'test',
+      secret: 'test',
+      uid: Doorkeeper::OAuth::Helpers::UniqueToken.generate,
+      redirect_uri: 'http://valid.com'
+    }
+  end
+end

spec/models/project_services/prometheus_service_spec.rb

@@ -13,6 +13,23 @@ describe PrometheusService, :use_clean_rails_memory_store_caching do
     it { is_expected.to belong_to :project }
   end
 
+  context 'redirects' do
+    it 'does not follow redirects' do
+      redirect_to = 'https://redirected.example.com'
+      redirect_req_stub = stub_prometheus_request(prometheus_query_url('1'), status: 302, headers: { location: redirect_to })
+      redirected_req_stub = stub_prometheus_request(redirect_to, body: { 'status': 'success' })
+
+      result = service.test
+
+      # result = { success: false, result: error }
+      expect(result[:success]).to be_falsy
+      expect(result[:result]).to be_instance_of(Gitlab::PrometheusClient::Error)
+
+      expect(redirect_req_stub).to have_been_requested
+      expect(redirected_req_stub).not_to have_been_requested
+    end
+  end
+
   describe 'Validations' do
     context 'when manual_configuration is enabled' do
       before do

spec/models/project_spec.rb

@@ -245,6 +245,7 @@ describe Project do
       it { is_expected.to validate_presence_of(:mirror_user) }
     end
 
+    describe 'import_url' do
       it 'does not allow an invalid URI as import_url' do
         project = build(:project, import_url: 'invalid://')
 
@@ -317,6 +318,22 @@ describe Project do
         expect(project.errors[:import_url].first).to include('Username needs to start with an alphanumeric character')
       end
 
+      include_context 'invalid urls'
+
+      it 'does not allow urls with CR or LF characters' do
+        project = build(:project)
+
+        aggregate_failures do
+          urls_with_CRLF.each do |url|
+            project.import_url = url
+
+            expect(project).not_to be_valid
+            expect(project.errors.full_messages.first).to match(/is blocked: URI is invalid/)
+          end
+        end
+      end
+    end
+
     it 'creates import state when mirror gets enabled' do
       project2 = create(:project)
 

spec/models/remote_mirror_spec.rb

@@ -174,7 +174,15 @@ describe RemoteMirror do
     end
 
     context 'with remote mirroring enabled' do
+      it 'defaults to disabling only protected branches' do
+        expect(remote_mirror.only_protected_branches?).to be_falsey
+      end
+
       context 'with only protected branches enabled' do
+        before do
+          remote_mirror.only_protected_branches = true
+        end
+
         context 'when it did not update in the last minute' do
           it 'schedules a RepositoryUpdateRemoteMirrorWorker to run now' do
             expect(RepositoryUpdateRemoteMirrorWorker).to receive(:perform_async).with(remote_mirror.id, Time.now)

spec/requests/api/applications_spec.rb

@@ -25,7 +25,7 @@ describe API::Applications, :api do
 
       it 'does not allow creating an application with the wrong redirect_uri format' do
         expect do
-          post api('/applications', admin_user), name: 'application_name', redirect_uri: 'wrong_url_format', scopes: ''
+          post api('/applications', admin_user), name: 'application_name', redirect_uri: 'http://', scopes: ''
         end.not_to change { Doorkeeper::Application.count }
 
         expect(response).to have_gitlab_http_status(400)
@@ -33,6 +33,16 @@ describe API::Applications, :api do
         expect(json_response['message']['redirect_uri'][0]).to eq('must be an absolute URI.')
       end
 
+      it 'does not allow creating an application with a forbidden URI format' do
+        expect do
+          post api('/applications', admin_user), name: 'application_name', redirect_uri: 'javascript://alert()', scopes: ''
+        end.not_to change { Doorkeeper::Application.count }
+
+        expect(response).to have_gitlab_http_status(400)
+        expect(json_response).to be_a Hash
+        expect(json_response['message']['redirect_uri'][0]).to eq('is forbidden by the server.')
+      end
+
       it 'does not allow creating an application without a name' do
         expect do
           post api('/applications', admin_user), redirect_uri: 'http://application.url', scopes: ''

spec/requests/api/helpers_spec.rb

@@ -209,6 +209,19 @@ describe API::Helpers do
 
         expect { current_user }.to raise_error Gitlab::Auth::ExpiredError
       end
+
+      context 'when impersonation is disabled' do
+        let(:personal_access_token) { create(:personal_access_token, :impersonation, user: user) }
+
+        before do
+          stub_config_setting(impersonation_enabled: false)
+          env[Gitlab::Auth::UserAuthFinders::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+        end
+
+        it 'does not allow impersonation tokens' do
+          expect { current_user }.to raise_error Gitlab::Auth::ImpersonationDisabled
+        end
+      end
     end
 
     describe "when authenticating using a job token" do

spec/rubocop/cop/migration/add_reference_spec.rb

@@ -29,7 +29,7 @@ describe RuboCop::Cop::Migration::AddReference do
       expect_offense(<<~RUBY)
         call do
           add_reference(:projects, :users)
-          ^^^^^^^^^^^^^ `add_reference` requires `index: true`
+          ^^^^^^^^^^^^^ `add_reference` requires `index: true` or `index: { options... }`
         end
       RUBY
     end
@@ -38,7 +38,7 @@ describe RuboCop::Cop::Migration::AddReference do
       expect_offense(<<~RUBY)
         def up
           add_reference(:projects, :users, index: false)
-          ^^^^^^^^^^^^^ `add_reference` requires `index: true`
+          ^^^^^^^^^^^^^ `add_reference` requires `index: true` or `index: { options... }`
         end
       RUBY
     end
@@ -50,5 +50,13 @@ describe RuboCop::Cop::Migration::AddReference do
         end
       RUBY
     end
+
+    it 'does not register an offense when the index is unique' do
+      expect_no_offenses(<<~RUBY)
+        def up
+          add_reference(:projects, :users, index: { unique: true } )
+        end
+      RUBY
+    end
   end
 end

spec/services/ci/archive_trace_service_spec.rb

+require 'spec_helper'
+
+describe Ci::ArchiveTraceService, '#execute' do
+  subject { described_class.new.execute(job) }
+
+  context 'when job is finished' do
+    let(:job) { create(:ci_build, :success, :trace_live) }
+
+    it 'creates an archived trace' do
+      expect { subject }.not_to raise_error
+
+      expect(job.reload.job_artifacts_trace).to be_exist
+    end
+
+    context 'when trace is already archived' do
+      let!(:job) { create(:ci_build, :success, :trace_artifact) }
+
+      it 'ignores an exception' do
+        expect { subject }.not_to raise_error
+      end
+
+      it 'does not create an archived trace' do
+        expect { subject }.not_to change { Ci::JobArtifact.trace.count }
+      end
+    end
+  end
+
+  context 'when job is running' do
+    let(:job) { create(:ci_build, :running, :trace_live) }
+
+    it 'increments Prometheus counter, sends crash report to Sentry and ignore an error for continuing to archive' do
+      expect(Gitlab::Sentry)
+        .to receive(:track_exception)
+        .with(::Gitlab::Ci::Trace::ArchiveError,
+              issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
+              extra: { job_id: job.id } ).once
+
+      expect(Rails.logger)
+        .to receive(:error)
+        .with("Failed to archive trace. id: #{job.id} message: Job is not finished yet")
+        .and_call_original
+
+      expect(Gitlab::Metrics)
+        .to receive(:counter)
+        .with(:job_trace_archive_failed_total, "Counter of failed attempts of trace archiving")
+        .and_call_original
+
+      expect { subject }.not_to raise_error
+    end
+  end
+end

spec/services/files/multi_service_spec.rb

@@ -122,26 +122,47 @@ describe Files::MultiService do
       let(:action) { 'move' }
       let(:new_file_path) { 'files/ruby/new_popen.rb' }
 
+      let(:result) { subject.execute }
+      let(:blob) { repository.blob_at_branch(branch_name, new_file_path) }
+
       context 'when original file has been updated' do
         before do
           update_file(original_file_path)
         end
 
         it 'rejects the commit' do
-          results = subject.execute
-
-          expect(results[:status]).to eq(:error)
-          expect(results[:message]).to match(original_file_path)
+          expect(result[:status]).to eq(:error)
+          expect(result[:message]).to match(original_file_path)
         end
       end
 
-      context 'when original file have not been updated' do
+      context 'when original file has not been updated' do
         it 'moves the file' do
-          results = subject.execute
-          blob = project.repository.blob_at_branch(branch_name, new_file_path)
+          expect(result[:status]).to eq(:success)
+          expect(blob).to be_present
+          expect(blob.data).to eq(file_content)
+        end
 
-          expect(results[:status]).to eq(:success)
+        context 'when content is nil' do
+          let(:file_content) { nil }
+
+          it 'moves the existing content untouched' do
+            original_content = repository.blob_at_branch(branch_name, original_file_path).data
+
+            expect(result[:status]).to eq(:success)
             expect(blob).to be_present
+            expect(blob.data).to eq(original_content)
+          end
+        end
+
+        context 'when content is an empty string' do
+          let(:file_content) { '' }
+
+          it 'moves the file and empties it' do
+            expect(result[:status]).to eq(:success)
+            expect(blob).not_to be_nil
+            expect(blob.data).to eq('')
+          end
         end
       end
     end

spec/support/helpers/prometheus_helpers.rb

@@ -49,11 +49,11 @@ module PrometheusHelpers
     "https://prometheus.example.com/api/v1/series?#{query}"
   end
 
-  def stub_prometheus_request(url, body: {}, status: 200)
+  def stub_prometheus_request(url, body: {}, status: 200, headers: {})
     WebMock.stub_request(:get, url)
       .to_return({
         status: status,
-        headers: { 'Content-Type' => 'application/json' },
+        headers: { 'Content-Type' => 'application/json' }.merge(headers),
         body: body.to_json
       })
   end

spec/support/shared_contexts/url_shared_context.rb

+shared_context 'invalid urls' do
+  let(:urls_with_CRLF) do
+    ["http://127.0.0.1:333/pa\rth",
+     "http://127.0.0.1:333/pa\nth",
+     "http://127.0a.0.1:333/pa\r\nth",
+     "http://127.0.0.1:333/path?param=foo\r\nbar",
+     "http://127.0.0.1:333/path?param=foo\rbar",
+     "http://127.0.0.1:333/path?param=foo\nbar",
+     "http://127.0.0.1:333/pa%0dth",
+     "http://127.0.0.1:333/pa%0ath",
+     "http://127.0a.0.1:333/pa%0d%0th",
+     "http://127.0.0.1:333/pa%0D%0Ath",
+     "http://127.0.0.1:333/path?param=foo%0Abar",
+     "http://127.0.0.1:333/path?param=foo%0Dbar",
+     "http://127.0.0.1:333/path?param=foo%0D%0Abar"]
+  end
+end

spec/validators/url_validator_spec.rb

+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe UrlValidator do
@@ -6,6 +8,30 @@ describe UrlValidator do
 
   include_examples 'url validator examples', described_class::DEFAULT_PROTOCOLS
 
+  describe 'validations' do
+    include_context 'invalid urls'
+
+    let(:validator) { described_class.new(attributes: [:link_url]) }
+
+    it 'returns error when url is nil' do
+      expect(validator.validate_each(badge, :link_url, nil)).to be_nil
+      expect(badge.errors.first[1]).to eq 'must be a valid URL'
+    end
+
+    it 'returns error when url is empty' do
+      expect(validator.validate_each(badge, :link_url, '')).to be_nil
+      expect(badge.errors.first[1]).to eq 'must be a valid URL'
+    end
+
+    it 'does not allow urls with CR or LF characters' do
+      aggregate_failures do
+        urls_with_CRLF.each do |url|
+          expect(validator.validate_each(badge, :link_url, url)[0]).to eq 'is blocked: URI is invalid'
+        end
+      end
+    end
+  end
+
   context 'by default' do
     let(:validator) { described_class.new(attributes: [:link_url]) }
 

spec/workers/archive_trace_worker_spec.rb

@@ -5,10 +5,11 @@ describe ArchiveTraceWorker do
     subject { described_class.new.perform(job&.id) }
 
     context 'when job is found' do
-      let(:job) { create(:ci_build) }
+      let(:job) { create(:ci_build, :trace_live) }
 
       it 'executes service' do
-        expect_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!)
+        expect_any_instance_of(Ci::ArchiveTraceService)
+          .to receive(:execute).with(job)
 
         subject
       end
@@ -18,7 +19,8 @@ describe ArchiveTraceWorker do
       let(:job) { nil }
 
       it 'does not execute service' do
-        expect_any_instance_of(Gitlab::Ci::Trace).not_to receive(:archive!)
+        expect_any_instance_of(Ci::ArchiveTraceService)
+          .not_to receive(:execute)
 
         subject
       end

spec/workers/ci/archive_traces_cron_worker_spec.rb

@@ -30,6 +30,13 @@ describe Ci::ArchiveTracesCronWorker do
 
     it_behaves_like 'archives trace'
 
+    it 'executes service' do
+      expect_any_instance_of(Ci::ArchiveTraceService)
+        .to receive(:execute).with(build)
+
+      subject
+    end
+
     context 'when a trace had already been archived' do
       let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) }
       let!(:build2) { create(:ci_build, :success, :trace_live) }
@@ -46,11 +53,12 @@ describe Ci::ArchiveTracesCronWorker do
       let!(:build) { create(:ci_build, :success, :trace_live) }
 
       before do
+        allow(Gitlab::Sentry).to receive(:track_exception)
         allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!).and_raise('Unexpected error')
       end
 
       it 'puts a log' do
-        expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Unexpected error")
+        expect(Rails.logger).to receive(:error).with("Failed to archive trace. id: #{build.id} message: Unexpected error")
 
         subject
       end