Merge remote-tracking branch 'upstream/master' into ce-to-ee-2017-07-19

CHANGELOG-EE.md

 Please view this file on the master branch, on stable branches it's out of date.
 
+## 9.3.8 (2017-07-19)
+
+- Escape symbols in exported CSV columns to prevent command execution in Microsoft Excel.
+- Prevent mirror user to be assigned to users other than the current one.
+
 ## 9.3.7 (2017-07-18)
 
 - No changes.
@@ -72,6 +77,11 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Fix: /unassign by default unassigns everyone. Implement /reassign command.
 - Speed up checking for approvers remaining.
 
+## 9.2.8 (2017-07-19)
+
+- Escape symbols in exported CSV columns to prevent command execution in Microsoft Excel.
+- Prevent mirror user to be assigned to users other than the current one.
+
 ## 9.2.7 (2017-06-21)
 
 - Geo: fixed Dynamic Backoff strategy that was not being used by workers. !2128
@@ -128,6 +138,11 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Feature availability check using feature list AND license addons.
 - Disable mirror workers for Geo secondaries.
 
+## 9.1.8 (2017-07-19)
+
+- Escape symbols in exported CSV columns to prevent command execution in Microsoft Excel.
+- Prevent mirror user to be assigned to users other than the current one.
+
 ## 9.1.7 (2017-06-07)
 
 - No changes.
@@ -207,6 +222,11 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Show user cohorts data when usage ping is enabled.
 - Visualise Canary Deployments.
 
+## 9.0.11 (2017-07-19)
+
+- Escape symbols in exported CSV columns to prevent command execution in Microsoft Excel.
+- Prevent mirror user to be assigned to users other than the current one.
+
 ## 9.0.10 (2017-06-07)
 
 - No changes.
@@ -299,6 +319,10 @@ Please view this file on the master branch, on stable branches it's out of date.
 - [Elasticsearch] More efficient search.
 - Get Geo secondaries nodes statuses over AJAX.
 
+## 8.17.7 (2017-07-19)
+
+- Prevent mirror user to be assigned to users other than the current one.
+
 ## 8.17.6 (2017-05-05)
 
 - Respect project features when searching alternative branches with elasticsearch enabled.

CHANGELOG.md

@@ -2,6 +2,13 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 9.3.8 (2017-07-19)
+
+- Improve support for external issue references. !12485
+- Renders 404 if given project is not readable by the user on Todos dashboard.
+- Use uploads/system directory for personal snippets.
+- Remove uploads/appearance symlink. A leftover from a previous migration.
+
 ## 9.3.7 (2017-07-18)
 
 - Prevent bad data being added to application settings when Redis is unavailable. !12750
@@ -264,6 +271,13 @@ entry.
 - Remove foreigh key on ci_trigger_schedules only if it exists.
 - Allow translation of Pipeline Schedules.
 
+## 9.2.8 (2017-07-19)
+
+- Improve support for external issue references. !12485
+- Renders 404 if given project is not readable by the user on Todos dashboard.
+- Fix incorrect project authorizations.
+- Remove uploads/appearance symlink. A leftover from a previous migration.
+
 ## 9.2.7 (2017-06-21)
 
 - Reinstate is_admin flag in users api when authenticated user is an admin. !12211 (rickettm)
@@ -510,6 +524,13 @@ entry.
 - Fix preemptive scroll bar on user activity calendar.
 - Pipeline chat notifications convert seconds to minutes and hours.
 
+## 9.1.8 (2017-07-19)
+
+- Improve support for external issue references. !12485
+- Renders 404 if given project is not readable by the user on Todos dashboard.
+- Fix incorrect project authorizations.
+- Remove uploads/appearance symlink. A leftover from a previous migration.
+
 ## 9.1.7 (2017-06-07)
 
 - No changes.
@@ -823,6 +844,12 @@ entry.
 - Only send chat notifications for the default branch.
 - Don't fill in the default kubernetes namespace.
 
+## 9.0.11 (2017-07-19)
+
+- Renders 404 if given project is not readable by the user on Todos dashboard.
+- Fix incorrect project authorizations.
+- Remove uploads/appearance symlink. A leftover from a previous migration.
+
 ## 9.0.10 (2017-06-07)
 
 - No changes.
@@ -1193,6 +1220,11 @@ entry.
 - Change development tanuki favicon colors to match logo color order.
 - API issues - support filtering by iids.
 
+## 8.17.7 (2017-07-19)
+
+- Renders 404 if given project is not readable by the user on Todos dashboard.
+- Fix incorrect project authorizations.
+
 ## 8.17.6 (2017-05-05)
 
 - Enforce project features when searching blobs and wikis.

Gemfile

@@ -172,6 +172,9 @@ gem 'rainbow', '~> 2.2'
 # GitLab settings
 gem 'settingslogic', '~> 2.0.9'
 
+# Linear-time regex library for untrusted regular expressions
+gem 're2', '~> 1.0.0'
+
 # Misc
 
 gem 'version_sorter', '~> 2.1.0'

Gemfile.lock

@@ -686,6 +686,7 @@ GEM
       debugger-ruby_core_source (~> 1.3)
     rdoc (4.2.2)
       json (~> 1.4)
+    re2 (1.0.0)
     recaptcha (3.0.0)
       json
     recursive-open-struct (1.0.0)
@@ -1094,6 +1095,7 @@ DEPENDENCIES
   raindrops (~> 0.18)
   rblineprof (~> 0.3.6)
   rdoc (~> 4.2)
+  re2 (~> 1.0.0)
   recaptcha (~> 3.0)
   redcarpet (~> 3.4)
   redis (~> 3.2)

app/assets/javascripts/dispatcher.js

 /* eslint-disable func-names, space-before-function-paren, no-var, prefer-arrow-callback, wrap-iife, no-shadow, consistent-return, one-var, one-var-declaration-per-line, camelcase, default-case, no-new, quotes, no-duplicate-case, no-case-declarations, no-fallthrough, max-len */
+/* global ProjectSelect */
 /* global ShortcutsNavigation */
 /* global IssuableIndex */
 /* global ShortcutsIssuable */
@@ -163,6 +164,9 @@ import AuditLogs from './audit_logs';
           shortcut_handler = new ShortcutsIssuable();
           new ZenMode();
           break;
+        case 'dashboard:milestones:index':
+          new ProjectSelect();
+          break;
         case 'projects:milestones:show':
         case 'groups:milestones:show':
         case 'dashboard:milestones:show':
@@ -172,6 +176,7 @@ import AuditLogs from './audit_logs';
         case 'groups:issues':
         case 'groups:merge_requests':
           new UsersSelect();
+          new ProjectSelect();
           break;
         case 'dashboard:todos:index':
           new Todos();
@@ -266,6 +271,7 @@ import AuditLogs from './audit_logs';
           break;
         case 'dashboard:issues':
         case 'dashboard:merge_requests':
+          new ProjectSelect();
           new UsersSelect();
           break;
         case 'projects:commit:show':

app/assets/javascripts/group_name.js

@@ -5,12 +5,15 @@ export default class GroupName {
   constructor() {
     this.titleContainer = document.querySelector('.js-title-container');
     this.title = this.titleContainer.querySelector('.title');
-    this.titleWidth = this.title.offsetWidth;
-    this.groupTitle = this.titleContainer.querySelector('.group-title');
-    this.groups = this.titleContainer.querySelectorAll('.group-path');
-    this.toggle = null;
-    this.isHidden = false;
-    this.init();
+
+    if (this.title) {
+      this.titleWidth = this.title.offsetWidth;
+      this.groupTitle = this.titleContainer.querySelector('.group-title');
+      this.groups = this.titleContainer.querySelectorAll('.group-path');
+      this.toggle = null;
+      this.isHidden = false;
+      this.init();
+    }
   }
 
   init() {

app/assets/javascripts/project_select.js

@@ -110,6 +110,14 @@ import Api from './api';
           dropdownCssClass: "ajax-project-dropdown"
         });
       });
+
+      $('.new-project-item-select-button').on('click', function() {
+        $('.project-item-select', this.parentNode).select2('open');
+      });
+
+      $('.project-item-select').on('click', function() {
+        window.location = `${$(this).val()}/${this.dataset.relativePath}`;
+      });
     }
 
     return ProjectSelect;

app/assets/stylesheets/new_nav.scss

@@ -330,7 +330,7 @@ header.navbar-gitlab-new {
     white-space: nowrap;
 
     > a {
-      &:last-of-type {
+      &:last-of-type:not(:first-child) {
         font-weight: 600;
       }
     }
@@ -384,6 +384,7 @@ header.navbar-gitlab-new {
       &::after {
         content: "/";
         margin: 0 2px 0 5px;
+        color: rgba($black, .65);
       }
     }
 
@@ -396,3 +397,13 @@ header.navbar-gitlab-new {
     color: $gl-text-color;
   }
 }
+
+.top-area {
+  .nav-controls-new-nav {
+    .dropdown {
+      @media (min-width: $screen-sm-min) {
+        margin-right: 0;
+      }
+    }
+  }
+}

app/assets/stylesheets/pages/commits.scss

@@ -178,6 +178,10 @@
   .merge-request-branches & {
     flex-direction: column;
   }
+
+  .project_namespace {
+    color: $gl-text-color-secondary;
+  }
 }
 
 .commit-content {

app/controllers/admin/geo_nodes_controller.rb

 class Admin::GeoNodesController < Admin::ApplicationController
   before_action :check_license, except: [:index, :destroy]
-  before_action :load_node, only: [:destroy, :repair, :toggle, :status]
+  before_action :load_node, only: [:edit, :update, :destroy, :repair, :toggle, :status]
 
   def index
     @nodes = GeoNode.all.order(:id)
@@ -22,6 +22,14 @@ class Admin::GeoNodesController < Admin::ApplicationController
     end
   end
 
+  def update
+    if @node.update_attributes(geo_node_params.except(:geo_node_key_attributes))
+      redirect_to admin_geo_nodes_path, notice: 'Geo Node was successfully updated.'
+    else
+      render 'edit'
+    end
+  end
+
   def destroy
     @node.destroy
 

app/controllers/concerns/issuable_collections.rb

@@ -32,10 +32,10 @@ module IssuableCollections
 
   def filter_params
     set_sort_order_from_cookie
-    set_default_scope
     set_default_state
 
-    @filter_params = params.dup
+    # Skip irrelevant Rails routing params
+    @filter_params = params.dup.except(:controller, :action, :namespace_id)
     @filter_params[:sort] ||= default_sort_order
 
     @sort = @filter_params[:sort]
@@ -55,10 +55,6 @@ module IssuableCollections
     @filter_params.permit(IssuableFinder::VALID_PARAMS)
   end
 
-  def set_default_scope
-    params[:scope] = 'all' if params[:scope].blank?
-  end
-
   def set_default_state
     params[:state] = 'opened' if params[:state].blank?
   end

app/controllers/concerns/safe_mirror_params.rb

+module SafeMirrorParams
+  extend ActiveSupport::Concern
+
+  included do
+    helper_method :default_mirror_users
+  end
+
+  private
+
+  def valid_mirror_user?(mirror_params)
+    return true unless mirror_params[:mirror_user_id].present?
+
+    default_mirror_users.map(&:id).include?(mirror_params[:mirror_user_id].to_i)
+  end
+
+  def default_mirror_users
+    [current_user, @project.mirror_user].compact.uniq
+  end
+end

app/controllers/dashboard/todos_controller.rb

 class Dashboard::TodosController < Dashboard::ApplicationController
   include ActionView::Helpers::NumberHelper
 
+  before_action :authorize_read_project!, only: :index
   before_action :find_todos, only: [:index, :destroy_all]
 
   def index
@@ -49,6 +50,15 @@ class Dashboard::TodosController < Dashboard::ApplicationController
 
   private
 
+  def authorize_read_project!
+    project_id = params[:project_id]
+
+    if project_id.present?
+      project = Project.find(project_id)
+      render_404 unless can?(current_user, :read_project, project)
+    end
+  end
+
   def find_todos
     @todos ||= TodosFinder.new(current_user, params).execute
   end

app/controllers/projects/imports_controller.rb

 class Projects::ImportsController < Projects::ApplicationController
   include ContinueParams
+  include SafeMirrorParams
 
   # Authorize
   before_action :authorize_admin_project!
@@ -11,7 +12,7 @@ class Projects::ImportsController < Projects::ApplicationController
   end
 
   def create
-    if @project.update_attributes(import_params)
+    if @project.update_attributes(safe_import_params)
       @project.reload.import_schedule
     end
 
@@ -67,4 +68,10 @@ class Projects::ImportsController < Projects::ApplicationController
   def import_params
     params.require(:project).permit(:import_url, :mirror, :mirror_user_id)
   end
+
+  def safe_import_params
+    return import_params if valid_mirror_user?(import_params)
+
+    import_params.merge(mirror_user_id: current_user.id)
+  end
 end

app/controllers/projects/mirrors_controller.rb

 class Projects::MirrorsController < Projects::ApplicationController
   include RepositorySettingsRedirect
+  include SafeMirrorParams
   # Authorize
   before_action :authorize_admin_project!, except: [:update_now]
   before_action :authorize_push_code!, only: [:update_now]
@@ -12,7 +13,7 @@ class Projects::MirrorsController < Projects::ApplicationController
   end
 
   def update
-    if @project.update_attributes(mirror_params)
+    if @project.update_attributes(safe_mirror_params)
       if @project.mirror?
         @project.force_import_job!
 
@@ -51,4 +52,10 @@ class Projects::MirrorsController < Projects::ApplicationController
     params.require(:project).permit(:mirror, :import_url, :mirror_user_id,
                                     :mirror_trigger_builds, remote_mirrors_attributes: [:url, :id, :enabled])
   end
+
+  def safe_mirror_params
+    return mirror_params if valid_mirror_user?(mirror_params)
+
+    mirror_params.merge(mirror_user_id: current_user.id)
+  end
 end

app/controllers/projects/settings/repository_controller.rb

 module Projects
   module Settings
     class RepositoryController < Projects::ApplicationController
+      include SafeMirrorParams
+
       before_action :authorize_admin_project!
 
       prepend ::EE::Projects::Settings::RepositoryController

app/finders/issuable_finder.rb

@@ -20,9 +20,9 @@
 #
 class IssuableFinder
   include CreatedAtFilter
-  
+
   NONE = '0'.freeze
-  IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page].freeze
+  IRRELEVANT_PARAMS_FOR_CACHE_KEY = %i[utf8 sort page state].freeze
 
   SCALAR_PARAMS = %i(scope state group_id project_id milestone_title assignee_id search label_name sort assignee_username author_id author_username authorized_only due_date iids non_archived weight).freeze
   ARRAY_PARAMS = { label_name: [], iids: [] }.freeze
@@ -94,8 +94,14 @@ class IssuableFinder
     execute.find_by!(*params)
   end
 
-  def state_counter_cache_key(state)
-    Digest::SHA1.hexdigest(state_counter_cache_key_components(state).flatten.join('-'))
+  def state_counter_cache_key
+    cache_key(state_counter_cache_key_components)
+  end
+
+  def clear_caches!
+    state_counter_cache_key_components_permutations.each do |components|
+      Rails.cache.delete(cache_key(components))
+    end
   end
 
   def group
@@ -447,12 +453,19 @@ class IssuableFinder
     params[:scope] == 'created-by-me' || params[:scope] == 'authored' || params[:scope] == 'assigned-to-me'
   end
 
-  def state_counter_cache_key_components(state)
+  def state_counter_cache_key_components
     opts = params.with_indifferent_access
-    opts[:state] = state
     opts.except!(*IRRELEVANT_PARAMS_FOR_CACHE_KEY)
     opts.delete_if { |_, value| value.blank? }
 
     ['issuables_count', klass.to_ability_name, opts.sort]
   end
+
+  def state_counter_cache_key_components_permutations
+    [state_counter_cache_key_components]
+  end
+
+  def cache_key(components)
+    Digest::SHA1.hexdigest(components.flatten.join('-'))
+  end
 end

app/finders/issues_finder.rb

@@ -75,7 +75,7 @@ class IssuesFinder < IssuableFinder
     current_user.blank? || for_counting || params[:for_counting]
   end
 
-  def state_counter_cache_key_components(state)
+  def state_counter_cache_key_components
     extra_components = [
       user_can_see_all_confidential_issues?,
       user_cannot_see_confidential_issues?(for_counting: true)
@@ -84,6 +84,16 @@ class IssuesFinder < IssuableFinder
     super + extra_components
   end
 
+  def state_counter_cache_key_components_permutations
+    # Ignore the last two, as we'll provide both options for them.
+    components = super.first[0..-3]
+
+    [
+      components + [false, true],
+      components + [true, false]
+    ]
+  end
+
   def by_assignee(items)
     if assignee
       items.assigned_to(assignee)

app/helpers/breadcrumbs_helper.rb

+module BreadcrumbsHelper
+  def add_to_breadcrumbs(text, link)
+    @breadcrumbs_extra_links ||= []
+    @breadcrumbs_extra_links.push({
+      text: text,
+      link: link
+    })
+  end
+
+  def breadcrumb_title_link
+    return @breadcrumb_link if @breadcrumb_link
+
+    if controller.available_action?(:index)
+      url_for(action: "index")
+    else
+      request.path
+    end
+  end
+
+  def breadcrumb_title(title)
+    return if defined?(@breadcrumb_title)
+
+    @breadcrumb_title = title
+  end
+end

app/helpers/issuables_helper.rb

@@ -243,7 +243,7 @@ module IssuablesHelper
 
   def issuables_count_for_state(issuable_type, state, finder: nil)
     finder ||= public_send("#{issuable_type}_finder")
-    cache_key = finder.state_counter_cache_key(state)
+    cache_key = finder.state_counter_cache_key
 
     @counts ||= {}
     @counts[cache_key] ||= Rails.cache.fetch(cache_key, expires_in: 2.minutes) do

app/helpers/mirror_helper.rb

@@ -4,4 +4,8 @@ module MirrorHelper
     message << "<br>To discard the local changes and overwrite the branch with the upstream version, delete it here and choose 'Update Now' above." if can?(current_user, :push_code, @project)
     message
   end
+
+  def options_for_mirror_user
+    options_from_collection_for_select(default_mirror_users, :id, :name, @project.mirror_user_id || current_user.id)
+  end
 end

app/helpers/page_layout_helper.rb

@@ -4,6 +4,10 @@ module PageLayoutHelper
 
     @page_title.push(*titles.compact) if titles.any?
 
+    if show_new_nav? && titles.any? && !defined?(@breadcrumb_title)
+      @breadcrumb_title = @page_title.last
+    end
+
     # Segments are seperated by middot
     @page_title.join(" \u00b7 ")
   end

app/models/geo/base_registry.rb

 class Geo::BaseRegistry < ActiveRecord::Base
   self.abstract_class = true
 
-  if Gitlab::Geo.secondary_role_enabled? && Gitlab::Geo.geo_database_configured?
+  if Gitlab::Geo.geo_database_configured?
     establish_connection Rails.configuration.geo_database
   end
+
+  def self.connection
+    raise 'Geo secondary database is not configured' unless Gitlab::Geo.geo_database_configured?
+
+    super
+  end
 end

app/services/boards/issues/list_service.rb

@@ -33,17 +33,12 @@ module Boards
       end
 
       def filter_params
-        set_default_scope
         set_project
         set_state
 
         params
       end
 
-      def set_default_scope
-        params[:scope] = 'all'
-      end
-
       def set_project
         params[:project_id] = project.id
       end

app/services/issuable_base_service.rb

@@ -185,7 +185,7 @@ class IssuableBaseService < BaseService
       after_create(issuable)
       issuable.create_cross_references!(current_user)
       execute_hooks(issuable)
-      invalidate_cache_counts(issuable.assignees, issuable)
+      invalidate_cache_counts(issuable, users: issuable.assignees)
     end
 
     issuable
@@ -242,12 +242,12 @@ class IssuableBaseService < BaseService
           old_assignees: old_assignees
         )
 
-        if old_assignees != issuable.assignees
-          new_assignees = issuable.assignees.to_a
-          affected_assignees = (old_assignees + new_assignees) - (old_assignees & new_assignees)
-          invalidate_cache_counts(affected_assignees.compact, issuable)
-        end
+        new_assignees = issuable.assignees.to_a
+        affected_assignees = (old_assignees + new_assignees) - (old_assignees & new_assignees)
 
+        # Don't clear the project cache, because it will be handled by the
+        # appropriate service (close / reopen / merge / etc.).
+        invalidate_cache_counts(issuable, users: affected_assignees.compact, skip_project_cache: true)
         after_update(issuable)
         issuable.create_new_cross_references!(current_user)
         execute_hooks(issuable, 'update')
@@ -341,9 +341,18 @@ class IssuableBaseService < BaseService
     create_labels_note(issuable, old_labels) if issuable.labels != old_labels
   end
 
-  def invalidate_cache_counts(users, issuable)
+  def invalidate_cache_counts(issuable, users: [], skip_project_cache: false)
     users.each do |user|
       user.public_send("invalidate_#{issuable.model_name.singular}_cache_counts")
     end
+
+    unless skip_project_cache
+      case issuable
+      when Issue
+        IssuesFinder.new(nil, project_id: issuable.project_id).clear_caches!
+      when MergeRequest
+        MergeRequestsFinder.new(nil, project_id: issuable.target_project_id).clear_caches!
+      end
+    end
   end
 end

app/services/issues/close_service.rb

@@ -28,7 +28,7 @@ module Issues
         notification_service.close_issue(issue, current_user) if notifications
         todo_service.close_issue(issue, current_user)
         execute_hooks(issue, 'close')
-        invalidate_cache_counts(issue.assignees, issue)
+        invalidate_cache_counts(issue, users: issue.assignees)
       end
 
       issue

app/services/issues/reopen_service.rb

@@ -8,7 +8,7 @@ module Issues
         create_note(issue)
         notification_service.reopen_issue(issue, current_user)
         execute_hooks(issue, 'reopen')
-        invalidate_cache_counts(issue.assignees, issue)
+        invalidate_cache_counts(issue, users: issue.assignees)
       end
 
       issue

app/services/merge_requests/close_service.rb

@@ -13,7 +13,7 @@ module MergeRequests
         notification_service.close_mr(merge_request, current_user)
         todo_service.close_merge_request(merge_request, current_user)
         execute_hooks(merge_request, 'close')
-        invalidate_cache_counts(merge_request.assignees, merge_request)
+        invalidate_cache_counts(merge_request, users: merge_request.assignees)
       end
 
       merge_request

app/services/merge_requests/post_merge_service.rb

@@ -13,7 +13,7 @@ module MergeRequests
       create_note(merge_request)
       notification_service.merge_mr(merge_request, current_user)
       execute_hooks(merge_request, 'merge')
-      invalidate_cache_counts(merge_request.assignees, merge_request)
+      invalidate_cache_counts(merge_request, users: merge_request.assignees)
     end
 
     private

app/services/merge_requests/reopen_service.rb

@@ -10,7 +10,7 @@ module MergeRequests
         execute_hooks(merge_request, 'reopen')
         merge_request.reload_diff(current_user)
         merge_request.mark_as_unchecked
-        invalidate_cache_counts(merge_request.assignees, merge_request)
+        invalidate_cache_counts(merge_request, users: merge_request.assignees)
       end
 
       merge_request

app/uploaders/personal_file_uploader.rb

@@ -3,6 +3,10 @@ class PersonalFileUploader < FileUploader
     File.join(CarrierWave.root, model_path(model))
   end
 
+  def self.base_dir
+    File.join(root_dir, 'system')
+  end
+
   private
 
   def secure_url

app/views/admin/applications/edit.html.haml

 - page_title "Edit", @application.name, "Applications"
+
 %h3.page-title Edit application
 - @url = admin_application_path(@application)
 = render 'form', application: @application

app/views/admin/applications/new.html.haml

+- breadcrumb_title "Applications"
 - page_title "New Application"
+
 %h3.page-title New application
 - @url = admin_applications_path
 = render 'form', application: @application

app/views/admin/broadcast_messages/edit.html.haml

+- breadcrumb_title "Messages"
 - page_title "Broadcast Messages"
 
 = render 'form'

app/views/admin/broadcast_messages/index.html.haml

+- breadcrumb_title "Messages"
 - page_title "Broadcast Messages"
 
 %h3.page-title

app/views/admin/geo_nodes/_form.html.haml

-= form_for geo_node, as: :geo_node, url: admin_geo_nodes_path, html: { class: 'form-horizontal' } do |f|
-  - if geo_node.errors.any?
-    .alert.alert-danger
-      - geo_node.errors.full_messages.each do |msg|
-        %p= msg
+- disable_key_edit = local_assigns.fetch(:disable_key_edit, false)
+
+= form_errors(geo_node)
+.form-group
+  .col-sm-offset-2.col-sm-10
+    .checkbox
+      = form.label :primary do
+        = form.check_box :primary
+        %strong This is a primary node
+.form-group
+  = form.label :url, 'URL', class: 'control-label'
+  .col-sm-10
+    = form.text_field :url, class: 'form-control'
+
+= form.fields_for :geo_node_key, geo_node.geo_node_key, include_id: !disable_key_edit do |fg|
   .form-group
-    .col-sm-offset-2.col-sm-10
-      .checkbox
-        = f.label :primary do
-          = f.check_box :primary
-          %strong This is a primary node
-  .form-group
-    = f.label :url, 'URL', class: 'control-label'
+    = fg.label :key, 'Public Key', class: 'control-label'
     .col-sm-10
-      = f.text_field :url, class: 'form-control'
-  = f.fields_for :geo_node_key, geo_node.geo_node_key do |fg|
-    .form-group
-      = fg.label :key, 'Public Key', class: 'control-label'
-      .col-sm-10
-        = fg.text_area :key, class: 'form-control thin_area', rows: 5
+      = fg.text_area :key, class: 'form-control thin_area', rows: 5, disabled: disable_key_edit
+      - unless disable_key_edit
         %p.help-block
           Paste a machine public key here for the GitLab user this node runs on. Read more about how to generate it
           = link_to "here", help_page_path("ssh/README")
-  .form-actions
-    = f.submit 'Add Node', class: 'btn btn-create'
-%hr

app/views/admin/geo_nodes/edit.html.haml

+- page_title 'Edit Geo Node'
+%h3.page-title
+  Edit Geo Node
+
+= form_for [:admin, @node], html: { class: 'form-horizontal' } do |f|
+  = render partial: 'form', locals: { form: f, geo_node: @node, disable_key_edit: true }
+  .form-actions
+    = f.submit 'Save changes', class: 'btn btn-create'
+    = link_to 'Cancel', admin_geo_nodes_path, class: 'btn btn-cancel'
+
+%hr

app/views/admin/geo_nodes/index.html.haml

@@ -8,7 +8,13 @@
 
 %hr
 
-= render partial: 'form', locals: {geo_node: @node} if Gitlab::Geo.license_allows?
+- if Gitlab::Geo.license_allows?
+  = form_for [:admin, @node], as: :geo_node, url: admin_geo_nodes_path, html: { class: 'form-horizontal' } do |f|
+    = render partial: 'form', locals: { form: f, geo_node: @node }
+    .form-actions
+      = f.submit 'Add Node', class: 'btn btn-create'
+
+%hr
 
 - if @nodes.any?
   .panel.panel-default
@@ -52,11 +58,12 @@
                 %p
                   .js-health
 
-          - if Gitlab::Geo.primary?
+          - unless Gitlab::Geo.secondary?
             .node-actions
               - if Gitlab::Geo.license_allows?
                 - if node.missing_oauth_application?
                   = link_to "Repair authentication", repair_admin_geo_node_path(node), method: :post, title: 'OAuth application is missing', class: 'btn btn-default btn-sm'
                 - if node.secondary?
                   = toggle_node_button(node)
+                = link_to "Edit", edit_admin_geo_node_path(node), class: 'btn btn-sm'
               = link_to "Remove", admin_geo_node_path(node), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm'

app/views/dashboard/_groups_head.html.haml

+- if show_new_nav? && current_user.can_create_group?
+  - content_for :breadcrumbs_extra do
+    = link_to "New group", new_group_path, class: "btn btn-new"
+
 .top-area
   %ul.nav-links
     = nav_link(page: dashboard_groups_path) do
@@ -6,9 +10,8 @@
     = nav_link(page: explore_groups_path) do
       = link_to explore_groups_path, title: 'Explore public groups' do
         Explore public groups
-  .nav-controls
+  .nav-controls{ class: ("nav-controls-new-nav" if show_new_nav?) }
     = render 'shared/groups/search_form'
     = render 'shared/groups/dropdown'
     - if current_user.can_create_group?
-      = link_to new_group_path, class: "btn btn-new" do
-        New group
+      = link_to "New group", new_group_path, class: "btn btn-new #{("visible-xs" if show_new_nav?)}"

app/views/dashboard/_projects_head.html.haml

 = content_for :flash_message do
   = render 'shared/project_limit'
+
+- if show_new_nav? && current_user.can_create_project?
+  - content_for :breadcrumbs_extra do
+    = link_to "New project", new_project_path, class: 'btn btn-new'
+
 .top-area.scrolling-tabs-container.inner-page-scroll-tabs
   .fade-left= icon('angle-left')
   .fade-right= icon('angle-right')
@@ -14,9 +19,8 @@
       = link_to explore_root_path, title: 'Explore', data: {placement: 'right'} do
         Explore projects
 
-  .nav-controls
+  .nav-controls{ class: ("nav-controls-new-nav" if show_new_nav?) }
     = render 'shared/projects/search_form'
     = render 'shared/projects/dropdown'
     - if current_user.can_create_project?
-      = link_to new_project_path, class: 'btn btn-new' do
-        New project
+      = link_to "New project", new_project_path, class: "btn btn-new #{("visible-xs" if show_new_nav?)}"

app/views/dashboard/_snippets_head.html.haml

+- if show_new_nav? && current_user
+  - content_for :breadcrumbs_extra do
+    = link_to "New snippet", new_snippet_path, class: "btn btn-new", title: "New snippet"
+
 .top-area
   %ul.nav-links
     = nav_link(page: dashboard_snippets_path, html_options: {class: 'home'}) do
@@ -8,6 +12,5 @@
         Explore Snippets
 
   - if current_user
-    .nav-controls.hidden-xs
-      = link_to new_snippet_path, class: "btn btn-new", title: "New snippet" do
-        New snippet
+    .nav-controls.hidden-xs{ class: ("hidden-sm hidden-md hidden-lg" if show_new_nav?) }
+      = link_to "New snippet", new_snippet_path, class: "btn btn-new", title: "New snippet"

app/views/dashboard/issues.html.haml

+- @hide_top_links = true
 - page_title "Issues"
 - header_title  "Issues", issues_dashboard_path(assignee_id: current_user.id)
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{current_user.name} issues")
 
+- if show_new_nav?
+  - content_for :breadcrumbs_extra do
+    = link_to params.merge(rss_url_options), class: 'btn has-tooltip append-right-10', title: 'Subscribe' do
+      = icon('rss')
+    = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue", with_feature_enabled: 'issues'
+
 .top-area
   = render 'shared/issuable/nav', type: :issues
-  .nav-controls
+  .nav-controls{ class: ("visible-xs" if show_new_nav?) }
     = link_to params.merge(rss_url_options), class: 'btn has-tooltip', title: 'Subscribe' do
       = icon('rss')
     = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue", with_feature_enabled: 'issues'

app/views/dashboard/merge_requests.html.haml

+- @hide_top_links = true
 - page_title "Merge Requests"
 - header_title  "Merge Requests", merge_requests_dashboard_path(assignee_id: current_user.id)
 
+- if show_new_nav?
+  - content_for :breadcrumbs_extra do
+    = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request", with_feature_enabled: 'merge_requests'
+
 .top-area
   = render 'shared/issuable/nav', type: :merge_requests
-  .nav-controls
+  .nav-controls{ class: ("visible-xs" if show_new_nav?) }
     = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request", with_feature_enabled: 'merge_requests'
 
 = render 'shared/issuable/filter', type: :merge_requests

app/views/dashboard/milestones/index.html.haml

@@ -2,10 +2,14 @@
 - page_title 'Milestones'
 - header_title 'Milestones', dashboard_milestones_path
 
+- if show_new_nav?
+  - content_for :breadcrumbs_extra do
+    = render 'shared/new_project_item_select', path: 'milestones/new', label: 'New milestone', include_groups: true
+
 .top-area
   = render 'shared/milestones_filter', counts: @milestone_states
 
-  .nav-controls
+  .nav-controls{ class: ("visible-xs" if show_new_nav?) }
     = render 'shared/new_project_item_select', path: 'milestones/new', label: 'New milestone', include_groups: true
 
 .milestones

app/views/dashboard/projects/index.html.haml

 - @no_container = true
 - @hide_top_links = true
-- @breadcrumb_title = "Projects"
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, dashboard_projects_url(rss_url_options), title: "All activity")

app/views/dashboard/projects/starred.html.haml

+- @hide_top_links = true
 - @no_container = true
-
+- breadcrumb_title "Projects"
 - page_title "Starred Projects"
 - header_title "Projects", dashboard_projects_path
 

app/views/dashboard/todos/index.html.haml

+- @hide_top_links = true
 - page_title "Todos"
 - header_title "Todos", dashboard_todos_path
 

app/views/explore/groups/index.html.haml

+- @hide_top_links = true
 - page_title    "Groups"
 - header_title  "Groups", dashboard_groups_path
 

app/views/explore/projects/index.html.haml

+- @hide_top_links = true
 - page_title    "Projects"
 - header_title  "Projects", dashboard_projects_path
 

app/views/explore/projects/starred.html.haml

+- @hide_top_links = true
 - page_title    "Projects"
 - header_title  "Projects", dashboard_projects_path
 

app/views/explore/projects/trending.html.haml

+- @hide_top_links = true
 - page_title    "Projects"
 - header_title  "Projects", dashboard_projects_path
 

app/views/explore/snippets/index.html.haml

+- @hide_top_links = true
 - page_title    "Snippets"
 - header_title  "Snippets", snippets_path
 

app/views/groups/issues.html.haml

 - page_title "Issues"
+- group_issues_exists = group_issues(@group).exists?
 = render "head_issues"
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, params.merge(rss_url_options), title: "#{@group.name} issues")
 
-- if group_issues(@group).exists?
+- if show_new_nav? && group_issues_exists
+  - content_for :breadcrumbs_extra do
+    = link_to params.merge(rss_url_options), class: 'btn btn-default append-right-10' do
+      = icon('rss')
+    = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue"
+
+- if group_issues_exists
   .top-area
     = render 'shared/issuable/nav', type: :issues
-    .nav-controls
+    .nav-controls{ class: ("visible-xs" if show_new_nav?) }
       = link_to params.merge(rss_url_options), class: 'btn' do
         = icon('rss')
         %span.icon-label

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

 - page_title 'Labels'
+- if show_new_nav? && can?(current_user, :admin_label, @group)
+  - content_for :breadcrumbs_extra do
+    = link_to "New label", new_group_label_path(@group), class: "btn btn-new"
+
 = render "groups/head_issues"
 
+
 .top-area.adjust
   .nav-text
     Labels can be applied to issues and merge requests. Group labels are available for any project within the group.
 
-  .nav-controls
+  .nav-controls{ class: ("visible-xs" if show_new_nav?) }
     - if can?(current_user, :admin_label, @group)
-      = link_to new_group_label_path(@group), class: "btn btn-new" do
-        New label
+      = link_to "New label", new_group_label_path(@group), class: "btn btn-new"
 
 .labels
   .other-labels

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

+- breadcrumb_title "Labels"
 - page_title 'New Label'
 - header_title group_title(@group, 'Labels', group_labels_path(@group))
 

app/views/groups/merge_requests.html.haml

 - page_title "Merge Requests"
 
+- if show_new_nav? && current_user
+  - content_for :breadcrumbs_extra do
+    = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request"
+
 - if @group_merge_requests.empty?
   = render 'shared/empty_states/merge_requests', project_select_button: true
 - else
   .top-area
     = render 'shared/issuable/nav', type: :merge_requests
     - if current_user
-      .nav-controls
+      .nav-controls{ class: ("visible-xs" if show_new_nav?) }
         = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request"
 
   = render 'shared/issuable/filter', type: :merge_requests

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

 - page_title "Milestones"
+- if show_new_nav? && can?(current_user, :admin_milestones, @group)
+  - content_for :breadcrumbs_extra do
+    = link_to "New milestone", new_group_milestone_path(@group), class: "btn btn-new"
+
 = render "groups/head_issues"
 
 .top-area
   = render 'shared/milestones_filter', counts: @milestone_states
 
-  .nav-controls
+  .nav-controls{ class: ("visible-xs" if show_new_nav?) }
     - if can?(current_user, :admin_milestones, @group)
-      = link_to new_group_milestone_path(@group), class: "btn btn-new" do
-        New milestone
+      = link_to "New milestone", new_group_milestone_path(@group), class: "btn btn-new"
 
 .milestones
   %ul.content-list

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

+- breadcrumb_title "Milestones"
 - page_title "Milestones"
 - header_title group_title(@group, "Milestones", group_milestones_path(@group))
 

app/views/groups/new.html.haml

+- @breadcrumb_link = dashboard_groups_path
+- breadcrumb_title "Groups"
+- @hide_top_links = true
 - page_title    'New Group'
 - header_title  "Groups", dashboard_groups_path
 

app/views/groups/show.html.haml

 - @no_container = true
+- breadcrumb_title "Group"
 
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, group_url(@group, rss_url_options), title: "#{@group.name} activity")

app/views/layouts/_page.html.haml

@@ -15,7 +15,8 @@
       - if show_new_nav?
         - if content_for?(:new_global_flash)
           = yield :new_global_flash
-        = render "layouts/nav/breadcrumbs"
+        - unless @hide_breadcrumbs
+          = render "layouts/nav/breadcrumbs"
       = render "layouts/flash"
       = yield :flash_message
     %div{ class: "#{(container_class unless @no_container)} #{@content_class}" }

app/views/layouts/nav/_breadcrumbs.html.haml

-- breadcrumb_title = @breadcrumb_title || controller.controller_name.humanize
+- breadcrumb_link = breadcrumb_title_link
 - hide_top_links = @hide_top_links || false
 
 %nav.breadcrumbs{ role: "navigation" }
@@ -8,12 +8,16 @@
         .title
           = link_to "GitLab", root_path
           \/
+          - if content_for?(:header_title_before)
+            = yield :header_title_before
+            \/
           = header_title
       %h2.breadcrumbs-sub-title
         %ul.list-unstyled
-          - if content_for?(:sub_title_before)
-            = yield :sub_title_before
-          %li= link_to breadcrumb_title, request.path
+          - if @breadcrumbs_extra_links
+            - @breadcrumbs_extra_links.each do |extra|
+              %li= link_to extra[:text], extra[:link]
+          %li= link_to @breadcrumb_title, breadcrumb_link
     - if content_for?(:breadcrumbs_extra)
       .breadcrumbs-extra.hidden-xs= yield :breadcrumbs_extra
     = yield :header_content

app/views/layouts/nav/_new_admin_sidebar.html.haml

@@ -13,7 +13,7 @@
         = nav_link(controller: :dashboard, html_options: {class: 'home'}) do
           = link_to admin_root_path, title: 'Overview' do
             %span
-              Overview
+              Dashboard
         = nav_link(controller: [:admin, :projects]) do
           = link_to admin_projects_path, title: 'Projects' do
             %span

app/views/layouts/nav/_new_dashboard.html.haml

@@ -3,7 +3,7 @@
     = link_to dashboard_projects_path, title: 'Projects', class: 'dashboard-shortcuts-projects' do
       Projects
 
-  = nav_link(controller: [:groups, 'groups/milestones', 'groups/group_members']) do
+  = nav_link(controller: ['dashboard/groups', 'explore/groups']) do
     = link_to dashboard_groups_path, class: 'dashboard-shortcuts-groups', title: 'Groups' do
       Groups
 

app/views/profiles/show.html.haml

+- breadcrumb_title "Profile"
 - @content_class = "limit-container-width" unless fluid_layout
 = render 'profiles/head'
 

app/views/profiles/two_factor_auths/show.html.haml

 - page_title 'Two-Factor Authentication', 'Account'
-- header_title "Two-Factor Authentication", profile_two_factor_auth_path
+- if show_new_nav?
+  - add_to_breadcrumbs("Account", profile_account_path)
+- else
+  - header_title "Two-Factor Authentication", profile_two_factor_auth_path
 - @content_class = "limit-container-width" unless fluid_layout
+
 = render 'profiles/head'
 
 - if inject_u2f_api?

app/views/projects/activity.html.haml

 - @no_container = true
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Project", project_path(@project))
+
 - page_title "Activity"
 = render "projects/head"
 

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

+- breadcrumb_title "Repository"
 - @no_container = true
 - page_title "Edit", @blob.path, @ref
 - content_for :page_specific_javascripts do

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

+- breadcrumb_title "Repository"
 - page_title "New File", @path.presence, @ref
 - content_for :page_specific_javascripts do
   = page_specific_javascript_tag('lib/ace.js')

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

+- breadcrumb_title "Repository"
 - @no_container = true
 
 - page_title @blob.path, @ref

app/views/projects/boards/_show.html.haml

@@ -4,8 +4,7 @@
 - page_title "Boards"
 
 - if show_new_nav?
-  - content_for :sub_title_before do
-    %li= link_to "Issues", project_issues_path(@project)
+  - add_to_breadcrumbs("Issues", project_issues_path(@project))
 
 - content_for :page_specific_javascripts do
   = webpack_bundle_tag 'common_vue'

app/views/projects/branches/index.html.haml

@@ -2,6 +2,9 @@
 - page_title "Branches"
 = render "projects/commits/head"
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
+
 %div{ class: container_class }
   .top-area.adjust
     .nav-text

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

 - ref = local_assigns.fetch(:ref)
+- show_project_name = local_assigns.fetch(:show_project_name, false)
 - if @note_counts
   - note_count = @note_counts.fetch(commit.id, 0)
 - else
@@ -34,6 +35,9 @@
           - commit_timeago = time_ago_with_tooltip(commit.committed_date)
           - commit_text =  _('%{commit_author_link} committed %{commit_timeago}') % { commit_author_link: commit_author_link, commit_timeago: commit_timeago }
           #{ commit_text.html_safe }
+        - if show_project_name
+          %span.project_namespace
+            = project.name_with_namespace
 
 
       .commit-actions.flex-row.hidden-xs

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

 - @no_container = true
+- breadcrumb_title _("Commits")
 
 - page_title _("Commits"), @ref
 = content_for :meta_tags do
   = auto_discovery_link_tag(:atom, project_commits_url(@project, @ref, rss_url_options), title: "#{@project.name}:#{@ref} commits")
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
+
 = content_for :sub_nav do
   = render "head"
 

app/views/projects/compare/index.html.haml

 - @no_container = true
 - page_title "Compare"
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
 = render "projects/commits/head"
 
 %div{ class: container_class }

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

 - @no_container = true
+- breadcrumb_title "Compare"
 - page_title "#{params[:from]}...#{params[:to]}"
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
 = render "projects/commits/head"
 
 %div{ class: container_class }

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

 - @no_container = true
 - page_title "Cycle Analytics"
+- if show_new_nav?
+  - add_to_breadcrumbs("Project", project_path(@project))
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('common_vue')
   = page_specific_javascript_bundle_tag('cycle_analytics')

app/views/projects/environments/index.html.haml

@@ -2,6 +2,9 @@
 - page_title "Environments"
 = render "projects/pipelines/head"
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Pipelines", project_pipelines_path(@project))
+
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('common_vue')
   = page_specific_javascript_bundle_tag("environments")

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

 - @no_container = true
+- breadcrumb_title "Environments"
 - page_title 'New Environment'
 = render "projects/pipelines/head"
 

app/views/projects/graphs/charts.html.haml

 - @no_container = true
 - page_title "Charts"
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('common_d3')
   = page_specific_javascript_bundle_tag('graphs')

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

@@ -3,6 +3,10 @@
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('common_d3')
   = page_specific_javascript_bundle_tag('graphs')
+
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
+
 = render 'projects/commits/head'
 
 %div{ class: container_class }

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

+- breadcrumb_title "Issues"
 - page_title "New Issue"
 
 %h3.page-title

app/views/projects/jobs/index.html.haml

@@ -2,6 +2,9 @@
 - page_title "Jobs"
 = render "projects/pipelines/head"
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Pipelines", project_pipelines_path(@project))
+
 %div{ class: container_class }
   .top-area
     - build_path_proc = ->(scope) { project_jobs_path(@project, scope: scope) }

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

 - @no_container = true
 - page_title "Labels"
 - hide_class = ''
+
+- if show_new_nav? && can?(current_user, :admin_label, @project)
+  - content_for :breadcrumbs_extra do
+    = link_to "New label", new_namespace_project_label_path(@project.namespace, @project), class: "btn btn-new"
+
 = render "shared/mr_head"
 
 - if @labels.exists? || @prioritized_labels.exists?
@@ -9,7 +14,7 @@
       .nav-text
         Labels can be applied to issues and merge requests. Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging.
 
-      .nav-controls
+      .nav-controls{ class: ("visible-xs" if show_new_nav?) }
         - if can?(current_user, :admin_label, @project)
           = link_to new_project_label_path(@project), class: "btn btn-new" do
             New label

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

 - @no_container = true
+- breadcrumb_title "Labels"
 - page_title "New Label"
 = render "shared/mr_head"
 

app/views/projects/merge_requests/creations/new.html.haml

+- breadcrumb_title "Merge Requests"
 - page_title "New Merge Request"
 
 - if @merge_request.can_be_created && !params[:change_branches]

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

 - @no_container = true
 - page_title 'Milestones'
+
+- if show_new_nav?
+  - content_for :breadcrumbs_extra do
+    = link_to "New milestone", new_namespace_project_milestone_path(@project.namespace, @project), class: 'btn btn-new', title: 'New milestone'
+
 = render "shared/mr_head"
 
 %div{ class: container_class }

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

 - @no_container = true
+- breadcrumb_title "Milestones"
 - page_title "New Milestone"
 = render "shared/mr_head"
 

app/views/projects/mirrors/_show.html.haml

@@ -36,12 +36,11 @@
         = render "projects/mirrors/instructions"
       .form-group
         = f.label :mirror_user_id, "Mirror user", class: "label-light"
-        = users_select_tag("project[mirror_user_id]", class: 'input-large', selected: @project.mirror_user_id || current_user.id,
-          first_user: true, current_user: true, push_code_to_protected_branches: true)
+        = select_tag('project[mirror_user_id]', options_for_mirror_user, class: "select2 lg", required: true)
         .help-block
           This user will be the author of all events in the activity feed that are the result of an update,
           like new branches being created or new commits being pushed to existing branches.
-          They need to have at least master access to this project.
+          You can only assign yourself to be the mirror user.
       - if @project.builds_enabled?
         = render "shared/mirror_trigger_builds_setting", f: f
       = f.submit 'Save changes', class: 'btn btn-create', name: 'update_remote_mirror'

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

+- breadcrumb_title "Graph"
 - page_title "Graph", @ref
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('network')
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
 = render "projects/commits/head"
 = render "head"
 %div{ class: container_class }

app/views/projects/new.html.haml

+- @breadcrumb_link = dashboard_projects_path
+- breadcrumb_title "Projects"
+- @hide_top_links = true
 - page_title    'New Project'
 - header_title  "Projects", dashboard_projects_path
 - visibility_level = params.dig(:project, :visibility_level) || default_project_visibility

app/views/projects/pipeline_schedules/index.html.haml

+- breadcrumb_title "Schedules"
+
 - content_for :page_specific_javascripts do
   = webpack_bundle_tag 'common_vue'
   = webpack_bundle_tag 'schedules_index'
 
 - @no_container = true
 - page_title _("Pipeline Schedules")
+
+- if show_new_nav? && can?(current_user, :create_pipeline_schedule, @project)
+  - content_for :breadcrumbs_extra do
+    = link_to _('New schedule'), new_namespace_project_pipeline_schedule_path(@project.namespace, @project), class: 'btn btn-create'
+
+  - add_to_breadcrumbs("Pipelines", project_pipelines_path(@project))
+
 = render "projects/pipelines/head"
 
 %div{ class: container_class }
@@ -13,7 +22,7 @@
     = render "tabs", schedule_path_proc: schedule_path_proc, all_schedules: @all_schedules, scope: @scope
 
     - if can?(current_user, :create_pipeline_schedule, @project)
-      .nav-controls
+      .nav-controls{ class: ("visible-xs" if show_new_nav?) }
         = link_to new_project_pipeline_schedule_path(@project), class: 'btn btn-create' do
           %span= _('New schedule')
 

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

+- breadcrumb_title "Schedules"
+- @breadcrumb_link = namespace_project_pipeline_schedules_path(@project.namespace, @project)
 - page_title _("New Pipeline Schedule")
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Pipelines", project_pipelines_path(@project))
+
 %h3.page-title
   = _("Schedule a new pipeline")
 %hr

app/views/projects/pipelines/charts.html.haml

 - @no_container = true
 - page_title _("Charts"), _("Pipelines")
+- if show_new_nav?
+  - add_to_breadcrumbs("Pipelines", project_pipelines_path(@project))
 - content_for :page_specific_javascripts do
   = page_specific_javascript_bundle_tag('common_d3')
   = page_specific_javascript_bundle_tag('graphs')

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

+- breadcrumb_title "Pipelines"
 - page_title "New Pipeline"
 
 %h3.page-title

app/views/projects/project_members/index.html.haml

 - page_title "Members"
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Settings", edit_project_path(@project))
+
 .row.prepend-top-default
   .col-lg-12
     %h4

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

+- breadcrumb_title "Integrations"
 - page_title @service.title, "Services"
+
+- if show_new_nav?
+  - add_to_breadcrumbs("Settings", edit_project_path(@project))
+
 = render "projects/settings/head"
 = render 'form'

app/views/projects/settings/ci_cd/show.html.haml

 - @content_class = "limit-container-width" unless fluid_layout
 - page_title "Pipelines"
+
+- if show_new_nav?
+  - add_to_breadcrumbs("Settings", edit_project_path(@project))
+
 = render "projects/settings/head"
 
 = render 'projects/runners/index'

app/views/projects/settings/integrations/show.html.haml

 - @content_class = "limit-container-width" unless fluid_layout
 - page_title 'Integrations'
+- if show_new_nav?
+  - add_to_breadcrumbs("Settings", edit_project_path(@project))
 = render "projects/settings/head"
 = render 'projects/hooks/index'
 = render 'projects/services/index'

app/views/projects/settings/repository/show.html.haml

 - page_title "Repository"
 - @content_class = "limit-container-width" unless fluid_layout
+
+- if show_new_nav?
+  - add_to_breadcrumbs("Settings", edit_project_path(@project))
+
 = render "projects/settings/head"
 
 - content_for :page_specific_javascripts do

app/views/projects/show.html.haml

 - @no_container = true
+- breadcrumb_title "Project"
 - @content_class = "limit-container-width" unless fluid_layout
 - flash_message_container = show_new_nav? ? :new_global_flash : :flash_message
 

app/views/projects/snippets/index.html.haml

 - page_title "Snippets"
 
+- if show_new_nav? && can?(current_user, :create_project_snippet, @project)
+  - content_for :breadcrumbs_extra do
+    = link_to "New snippet", new_namespace_project_snippet_path(@project.namespace, @project), class: "btn btn-new", title: "New snippet"
+
 - if current_user
   .top-area
     - include_private = @project.team.member?(current_user) || current_user.admin?
     = render partial: 'snippets/snippets_scope_menu', locals: { subject: @project, include_private: include_private }
 
-    .nav-controls.hidden-xs
+    .nav-controls{ class: ("visible-xs" if show_new_nav?) }
       - if can?(current_user, :create_project_snippet, @project)
-        = link_to new_project_snippet_path(@project), class: "btn btn-new", title: "New snippet" do
-          New snippet
-
-- if can?(current_user, :create_project_snippet, @project)
-  .visible-xs
-     
-    = link_to new_project_snippet_path(@project), class: "btn btn-new btn-block", title: "New snippet" do
-      New snippet
+        = link_to "New snippet", new_project_snippet_path(@project), class: "btn btn-new", title: "New snippet"
 
 = render 'snippets/snippets'

app/views/projects/tags/index.html.haml

@@ -3,6 +3,9 @@
 - page_title "Tags"
 = render "projects/commits/head"
 
+- if show_new_nav?
+  - add_to_breadcrumbs("Repository", project_tree_path(@project))
+
 .flex-list{ class: container_class }
   .top-area.adjust
     .nav-text.row-main-content

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

 - @no_container = true
+- breadcrumb_title _("Repository")
 - @content_class = "limit-container-width" unless fluid_layout
 
 - page_title @path.presence || _("Files"), @ref

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

 - @content_class = "limit-container-width limit-container-width-sm" unless fluid_layout
+- breadcrumb_title "Wiki"
 - page_title @page.title.capitalize, "Wiki"
 
 .wiki-page-header.has-sidebar-toggle

app/views/search/results/_commit.html.haml

-= render 'projects/commits/commit', project: commit.project, commit: commit, ref: nil
+= render 'projects/commits/commit', project: commit.project, commit: commit, ref: nil, show_project_name: true

app/views/search/show.html.haml

+- @hide_top_links = true
+- breadcrumb_title "Search"
 - page_title @search_term
 
 .prepend-top-default

app/views/shared/_mirror_trigger_builds_setting.html.haml

@@ -6,3 +6,4 @@
       Trigger pipelines when branches or tags are updated from the upstream repository.
       Depending on the activity of the upstream repository, this may greatly increase the load on your CI runners.
       Only enable this if you know they can handle the load.
+      <strong>CI will run using the credentials assigned above.</strong>

app/views/shared/_new_project_item_select.html.haml

 - if @projects.any?
   .project-item-select-holder
     = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at' }, with_feature_enabled: local_assigns[:with_feature_enabled]
-    %a.btn.btn-new.new-project-item-select-button
+    %a.btn.btn-new.new-project-item-select-button{ data: { relative_path: local_assigns[:path] } }
       = local_assigns[:label]
       = icon('caret-down')
-
-  :javascript
-    $('.new-project-item-select-button').on('click', function() {
-      $('.project-item-select').select2('open');
-    });
-
-    var relativePath = '#{local_assigns[:path]}';
-
-    $('.project-item-select').on('click', function() {
-      window.location = $(this).val() + '/' + relativePath;
-    });
-
-    new ProjectSelect()

app/views/snippets/new.html.haml

+- @hide_top_links = true
+- breadcrumb_title "Snippets"
 - page_title "New Snippet"
 %h3.page-title
   New Snippet

app/views/snippets/show.html.haml

+- @hide_top_links = true
 - @content_class = "limit-container-width limited-inner-width-container" unless fluid_layout
 - page_title "#{@snippet.title} (#{@snippet.to_reference})", "Snippets"
 

app/views/users/show.html.haml

+- @hide_top_links = true
+- @hide_breadcrumbs = true
 - page_title       @user.name
 - page_description @user.bio
 - content_for :page_specific_javascripts do

app/workers/geo_file_download_dispatch_worker.rb

@@ -22,7 +22,7 @@ class GeoFileDownloadDispatchWorker
   #    files, excluding ones in progress.
   # 5. Quit when we have scheduled all downloads or exceeded an hour.
   def perform
-    return unless Gitlab::Geo.secondary_role_enabled?
+    return unless Gitlab::Geo.geo_database_configured?
     return unless Gitlab::Geo.secondary?
 
     @start_time = Time.now

app/workers/geo_repository_sync_worker.rb

@@ -15,7 +15,7 @@ class GeoRepositorySyncWorker
   end
 
   def perform
-    return unless Gitlab::Geo.secondary_role_enabled?
+    return unless Gitlab::Geo.geo_database_configured?
     return unless Gitlab::Geo.primary_node.present?
 
     logger.info "Started Geo repository sync scheduler"

changelogs/unreleased-ee/1639-show-project-names-for-commit-search-result-with-elasticsearch-global-search.yml

+---
+title: Shows project names for commits in elasticsearch global search
+merge_request: 2434
+author:

changelogs/unreleased-ee/2826-remove-geo-roles.yml

+---
+title: 'Geo: Implement alternative to geo_{primary|secondary}_role in gitlab.yml'
+merge_request: 2352
+author:

changelogs/unreleased/adam-external-issue-references-spike.yml

----
-title: Improve support for external issue references
-merge_request: 12485
-author:

config/gitlab.yml.example

@@ -644,12 +644,6 @@ production: &base
     ip_whitelist:
       - 127.0.0.0/8
 
-  ## GitLab Geo settings (EE-only)
-  geo_primary_role:
-    enabled: false
-  geo_secondary_role:
-    enabled: false
-
   #
   # 5. Extra customization
   # ==========================
@@ -780,10 +774,6 @@ test:
         user_filter: ''
         group_base: 'ou=groups,dc=example,dc=com'
         admin_group: ''
-  geo_primary_role:
-    enabled: true
-  geo_secondary_role:
-    enabled: false
 
 staging:
   <<: *base

config/initializers/1_settings.rb

@@ -349,10 +349,6 @@ Settings.pages['external_https']  ||= false unless Settings.pages['external_http
 # Geo
 #
 Settings.gitlab['geo_status_timeout'] ||= 10
-Settings['geo_primary_role'] ||= Settingslogic.new({})
-Settings.geo_primary_role['enabled'] = false if Settings.geo_primary_role['enabled'].nil?
-Settings['geo_secondary_role'] ||= Settingslogic.new({})
-Settings.geo_secondary_role['enabled'] = false if Settings.geo_secondary_role['enabled'].nil?
 
 #
 # Git LFS

config/initializers/geo.rb

-if File.exist?(Rails.root.join('config/database_geo.yml')) &&
-    Gitlab::Geo.secondary_role_enabled?
+if File.exist?(Rails.root.join('config/database_geo.yml'))
   Rails.application.configure do
     config.geo_database = config_for(:database_geo)
   end
 end
 
 begin
-  # Avoid using the database if this is run in a Rake task
-  if Gitlab::Geo.primary_role_enabled?
+  if Gitlab::Geo.primary?
     Gitlab::Geo.current_node&.update_clone_url!
   end
 rescue => e

config/routes/admin.rb

@@ -125,7 +125,7 @@ namespace :admin do
     get :download, on: :member
   end
 
-  resources :geo_nodes, only: [:index, :create, :destroy] do
+  resources :geo_nodes, only: [:index, :create, :edit, :update, :destroy] do
     member do
       post :repair
       post :toggle

config/routes/uploads.rb

@@ -5,12 +5,12 @@ scope path: :uploads do
       constraints:  { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ }
 
   # show uploads for models, snippets (notes) available for now
-  get ':model/:id/:secret/:filename',
+  get 'system/:model/:id/:secret/:filename',
     to: 'uploads#show',
     constraints: { model: /personal_snippet/, id: /\d+/, filename: /[^\/]+/ }
 
   # show temporary uploads
-  get 'temp/:secret/:filename',
+  get 'system/temp/:secret/:filename',
     to: 'uploads#show',
     constraints: { filename: /[^\/]+/ }
 

db/post_migrate/20170406111121_clean_upload_symlinks.rb

@@ -6,7 +6,7 @@ class CleanUploadSymlinks < ActiveRecord::Migration
   disable_ddl_transaction!
 
   DOWNTIME = false
-  DIRECTORIES_TO_MOVE = %w(user project note group appeareance)
+  DIRECTORIES_TO_MOVE = %w(user project note group appearance)
 
   def up
     return unless file_storage?

db/post_migrate/20170612071012_move_personal_snippets_files.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+class MovePersonalSnippetsFiles < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    return unless file_storage?
+
+    @source_relative_location = File.join('/uploads', 'personal_snippet')
+    @destination_relative_location = File.join('/uploads', 'system', 'personal_snippet')
+
+    move_personal_snippet_files
+  end
+
+  def down
+    return unless file_storage?
+
+    @source_relative_location = File.join('/uploads', 'system', 'personal_snippet')
+    @destination_relative_location = File.join('/uploads', 'personal_snippet')
+
+    move_personal_snippet_files
+  end
+
+  def move_personal_snippet_files
+    query = "SELECT uploads.path, uploads.model_id, snippets.description FROM uploads "\
+            "INNER JOIN snippets ON snippets.id = uploads.model_id WHERE uploader = 'PersonalFileUploader'"
+    select_all(query).each do |upload|
+      secret = upload['path'].split('/')[0]
+      file_name = upload['path'].split('/')[1]
+
+      next unless move_file(upload['model_id'], secret, file_name)
+      update_markdown(upload['model_id'], secret, file_name, upload['description'])
+    end
+  end
+
+  def move_file(snippet_id, secret, file_name)
+    source_dir = File.join(base_directory, @source_relative_location, snippet_id.to_s, secret)
+    destination_dir = File.join(base_directory, @destination_relative_location, snippet_id.to_s, secret)
+
+    source_file_path = File.join(source_dir, file_name)
+    destination_file_path = File.join(destination_dir, file_name)
+
+    unless File.exist?(source_file_path)
+      say "Source file `#{source_file_path}` doesn't exist. Skipping."
+      return
+    end
+
+    say "Moving file #{source_file_path} -> #{destination_file_path}"
+
+    FileUtils.mkdir_p(destination_dir)
+    FileUtils.move(source_file_path, destination_file_path)
+
+    true
+  end
+
+  def update_markdown(snippet_id, secret, file_name, description)
+    source_markdown_path = File.join(@source_relative_location, snippet_id.to_s, secret, file_name)
+    destination_markdown_path = File.join(@destination_relative_location, snippet_id.to_s, secret, file_name)
+
+    source_markdown = "](#{source_markdown_path})"
+    destination_markdown = "](#{destination_markdown_path})"
+
+    if description.present?
+      description = description.gsub(source_markdown, destination_markdown)
+      quoted_description = quote_string(description)
+
+      execute("UPDATE snippets SET description = '#{quoted_description}', description_html = NULL "\
+              "WHERE id = #{snippet_id}")
+    end
+
+    query = "SELECT id, note FROM notes WHERE noteable_id = #{snippet_id} "\
+            "AND noteable_type = 'Snippet' AND note IS NOT NULL"
+    select_all(query).each do |note|
+      text = note['note'].gsub(source_markdown, destination_markdown)
+      quoted_text = quote_string(text)
+
+      execute("UPDATE notes SET note = '#{quoted_text}', note_html = NULL WHERE id = #{note['id']}")
+    end
+  end
+
+  def base_directory
+    File.join(Rails.root, 'public')
+  end
+
+  def file_storage?
+    CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+  end
+end

db/post_migrate/20170613111224_clean_appearance_symlinks.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class CleanAppearanceSymlinks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    return unless file_storage?
+
+    symlink_location = File.join(old_upload_dir, dir)
+
+    return unless File.symlink?(symlink_location)
+    say "removing symlink: #{symlink_location}"
+    FileUtils.rm(symlink_location)
+  end
+
+  def down
+    return unless file_storage?
+
+    symlink = File.join(old_upload_dir, dir)
+    destination = File.join(new_upload_dir, dir)
+
+    return if File.directory?(symlink)
+    return unless File.directory?(destination)
+
+    say "Creating symlink #{symlink} -> #{destination}"
+    FileUtils.ln_s(destination, symlink)
+  end
+
+  def file_storage?
+    CarrierWave::Uploader::Base.storage == CarrierWave::Storage::File
+  end
+
+  def dir
+    'appearance'
+  end
+
+  def base_directory
+    Rails.root
+  end
+
+  def old_upload_dir
+    File.join(base_directory, "public", "uploads")
+  end
+
+  def new_upload_dir
+    File.join(base_directory, "public", "uploads", "system")
+  end
+end

lib/csv_builder.rb

@@ -79,9 +79,9 @@ class CsvBuilder
   def row(object)
     attributes.map do |attribute|
       if attribute.respond_to?(:call)
-        attribute.call(object)
+        excel_sanitize(attribute.call(object))
       else
-        object.public_send(attribute)
+        excel_sanitize(object.public_send(attribute))
       end
     end
   end
@@ -100,4 +100,11 @@ class CsvBuilder
       end
     end
   end
+
+  def excel_sanitize(line)
+    return if line.nil?
+
+    line.prepend("'") if line =~ /^[=\+\-@;]/
+    line
+  end
 end

lib/gitlab/ci/trace/stream.rb

@@ -69,12 +69,12 @@ module Gitlab
           return unless valid?
           return unless regex
 
-          regex = Regexp.new(regex)
+          regex = Gitlab::UntrustedRegexp.new(regex)
 
           match = ""
 
           reverse_line do |line|
-            matches = line.scan(regex)
+            matches = regex.scan(line)
             next unless matches.is_a?(Array)
             next if matches.empty?
 

lib/gitlab/geo.rb

@@ -31,7 +31,12 @@ module Gitlab
     end
 
     def self.enabled?
-      self.cache_value(:geo_node_enabled) { GeoNode.exists? }
+      GeoNode.connected? && self.cache_value(:geo_node_enabled) { GeoNode.exists? }
+    rescue => e
+      # We can't use the actual classes in rescue because we load only one of them based on database supported
+      raise e unless %w(PG::UndefinedTable Mysql2::Error).include? e.class.name
+
+      false
     end
 
     def self.current_node_enabled?
@@ -41,14 +46,6 @@ module Gitlab
       Gitlab::Geo.current_node.reload.enabled?
     end
 
-    def self.primary_role_enabled?
-      Gitlab.config.geo_primary_role['enabled']
-    end
-
-    def self.secondary_role_enabled?
-      Gitlab.config.geo_secondary_role['enabled']
-    end
-
     def self.geo_database_configured?
       Rails.configuration.respond_to?(:geo_database)
     end
@@ -113,9 +110,9 @@ module Gitlab
     end
 
     def self.configure_cron_jobs!
-      if self.primary_role_enabled?
+      if self.primary?
         self.configure_primary_jobs!
-      elsif self.secondary_role_enabled?
+      elsif self.secondary?
         self.configure_secondary_jobs!
       else
         self.enable_all_cron_jobs!

lib/gitlab/geo/health_check.rb

@@ -5,7 +5,6 @@ module Gitlab
         raise NotImplementedError.new('Geo is only compatible with PostgreSQL') unless Gitlab::Database.postgresql?
 
         return '' unless Gitlab::Geo.secondary?
-        return 'The Geo secondary role is disabled.' unless Gitlab::Geo.secondary_role_enabled?
         return 'The Geo database configuration file is missing.' unless Gitlab::Geo.geo_database_configured?
         return 'The Geo node has a database that is not configured for streaming replication with the primary node.' unless self.database_secondary?
 

lib/gitlab/route_map.rb

@@ -18,7 +18,11 @@ module Gitlab
       mapping = @map.find { |mapping| mapping[:source] === path }
       return unless mapping
 
-      path.sub(mapping[:source], mapping[:public])
+      if mapping[:source].is_a?(String)
+        path.sub(mapping[:source], mapping[:public])
+      else
+        mapping[:source].replace(path, mapping[:public])
+      end
     end
 
     private
@@ -35,7 +39,7 @@ module Gitlab
         source_pattern = source_pattern[1...-1].gsub('\/', '/')
 
         begin
-          source_pattern = /\A#{source_pattern}\z/
+          source_pattern = Gitlab::UntrustedRegexp.new('\A' + source_pattern + '\z')
         rescue RegexpError => e
           raise FormatError, "Route map entry source is not a valid regular expression: #{e}"
         end

lib/gitlab/untrusted_regexp.rb

+module Gitlab
+  # An untrusted regular expression is any regexp containing patterns sourced
+  # from user input.
+  #
+  # Ruby's built-in regular expression library allows patterns which complete in
+  # exponential time, permitting denial-of-service attacks.
+  #
+  # Not all regular expression features are available in untrusted regexes, and
+  # there is a strict limit on total execution time. See the RE2 documentation
+  # at https://github.com/google/re2/wiki/Syntax for more details.
+  class UntrustedRegexp
+    delegate :===, to: :regexp
+
+    def initialize(pattern)
+      @regexp = RE2::Regexp.new(pattern, log_errors: false)
+
+      raise RegexpError.new(regexp.error) unless regexp.ok?
+    end
+
+    def replace_all(text, rewrite)
+      RE2.GlobalReplace(text, regexp, rewrite)
+    end
+
+    def scan(text)
+      scan_regexp.scan(text).map do |match|
+        if regexp.number_of_capturing_groups == 0
+          match.first
+        else
+          match
+        end
+      end
+    end
+
+    def replace(text, rewrite)
+      RE2.Replace(text, regexp, rewrite)
+    end
+
+    private
+
+    attr_reader :regexp
+
+    # RE2 scan operates differently to Ruby scan when there are no capture
+    # groups, so work around it
+    def scan_regexp
+      @scan_regexp ||=
+        if regexp.number_of_capturing_groups == 0
+          RE2::Regexp.new('(' + regexp.source + ')')
+        else
+          regexp
+        end
+    end
+  end
+end

scripts/prepare_build.sh

@@ -67,6 +67,3 @@ if [ "$SETUP_DB" != "false" ]; then
     # EE-only
     bundle exec rake geo:db:drop geo:db:create geo:db:schema:load geo:db:migrate
 fi
-
-# EE-only
-sed -i -e '/geo_secondary_role\:/ {' -e 'n; s/enabled\: false/enabled\: true/' -e '}' config/gitlab.yml

spec/controllers/admin/geo_nodes_controller_spec.rb

@@ -20,7 +20,10 @@ describe Admin::GeoNodesController, :postgresql do
 
   describe '#index' do
     render_views
-    subject { get :index }
+
+    def go
+      get :index
+    end
 
     context 'with add-on license available' do
       before do
@@ -28,7 +31,7 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'renders creation form' do
-        expect(subject).to render_template(partial: 'admin/geo_nodes/_form')
+        expect(go).to render_template(partial: 'admin/geo_nodes/_form')
       end
     end
 
@@ -38,16 +41,16 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'does not render the creation form' do
-        expect(subject).not_to render_template(partial: 'admin/geo_nodes/_form')
+        expect(go).not_to render_template(partial: 'admin/geo_nodes/_form')
       end
 
       it 'displays a flash message' do
-        subject
+        go
         expect(controller).to set_flash.now[:alert].to('You need a different license to enable Geo replication')
       end
 
       it 'does not redirects to the license page' do
-        subject
+        go
         expect(response).not_to redirect_to(admin_license_path)
       end
     end
@@ -55,7 +58,8 @@ describe Admin::GeoNodesController, :postgresql do
 
   describe '#destroy' do
     let!(:geo_node) { create(:geo_node) }
-    subject do
+
+    def go
       delete(:destroy, id: geo_node)
     end
 
@@ -65,7 +69,7 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'deletes the node' do
-        expect { subject }.to change { GeoNode.count }.by(-1)
+        expect { go }.to change { GeoNode.count }.by(-1)
       end
     end
 
@@ -75,19 +79,22 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'deletes the node' do
-        expect { subject }.to change { GeoNode.count }.by(-1)
+        expect { go }.to change { GeoNode.count }.by(-1)
       end
     end
   end
 
   describe '#create' do
     let(:geo_node_attributes) { { url: 'http://example.com', geo_node_key_attributes: { key: SSHKeygen.generate } } }
-    subject { post :create, geo_node: geo_node_attributes }
+
+    def go
+      post :create, geo_node: geo_node_attributes
+    end
 
     context 'without add-on license' do
       before do
         allow(Gitlab::Geo).to receive(:license_allows?) { false }
-        subject
+        go
       end
 
       it_behaves_like 'unlicensed geo action'
@@ -99,18 +106,53 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'creates the node' do
-        expect { subject }.to change { GeoNode.count }.by(1)
+        expect { go }.to change { GeoNode.count }.by(1)
+      end
+    end
+  end
+
+  describe '#update' do
+    let(:geo_node_attributes) { { url: 'http://example.com', geo_node_key_attributes: attributes_for(:key) } }
+    let(:geo_node) { create(:geo_node) }
+    let!(:original_fingerprint) { geo_node.geo_node_key.fingerprint }
+
+    def go
+      post :update, id: geo_node, geo_node: geo_node_attributes
+    end
+
+    context 'without add-on license' do
+      before do
+        allow(Gitlab::Geo).to receive(:license_allows?) { false }
+        go
+      end
+
+      it_behaves_like 'unlicensed geo action'
+    end
+
+    context 'with add-on license' do
+      before do
+        allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
+        go
+      end
+
+      it 'updates the node without changing the key' do
+        geo_node.reload
+
+        expect(geo_node.url.chomp('/')).to eq(geo_node_attributes[:url])
+        expect(geo_node.geo_node_key.fingerprint).to eq(original_fingerprint)
       end
     end
   end
 
   describe '#repair' do
     let(:geo_node) { create(:geo_node) }
-    subject { post :repair, id: geo_node }
+    def go
+      post :repair, id: geo_node
+    end
 
     before do
       allow(Gitlab::Geo).to receive(:license_allows?) { false }
-      subject
+      go
     end
 
     it_behaves_like 'unlicensed geo action'

spec/controllers/dashboard/todos_controller_spec.rb

@@ -12,6 +12,36 @@ describe Dashboard::TodosController do
   end
 
   describe 'GET #index' do
+    context 'project authorization' do
+      it 'renders 404 when user does not have read access on given project' do
+        unauthorized_project = create(:empty_project, :private)
+
+        get :index, project_id: unauthorized_project.id
+
+        expect(response).to have_http_status(404)
+      end
+
+      it 'renders 404 when given project does not exists' do
+        get :index, project_id: 999
+
+        expect(response).to have_http_status(404)
+      end
+
+      it 'renders 200 when filtering for "any project" todos' do
+        get :index, project_id: ''
+
+        expect(response).to have_http_status(200)
+      end
+
+      it 'renders 200 when user has access on given project' do
+        authorized_project = create(:empty_project, :public)
+
+        get :index, project_id: authorized_project.id
+
+        expect(response).to have_http_status(200)
+      end
+    end
+
     context 'when using pagination' do
       let(:last_page) { user.todos.page.total_pages }
       let!(:issues) { create_list(:issue, 2, project: project, assignees: [user]) }

spec/controllers/projects/imports_controller_spec.rb

@@ -2,16 +2,15 @@ require 'spec_helper'
 
 describe Projects::ImportsController do
   let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+
+  before do
+    sign_in(user)
+    project.team << [user, :master]
+  end
 
   describe 'GET #show' do
     context 'when repository does not exists' do
-      let(:project) { create(:empty_project) }
-
-      before do
-        sign_in(user)
-        project.team << [user, :master]
-      end
-
       it 'renders template' do
         get :show, namespace_id: project.namespace.to_param, project_id: project
 
@@ -28,11 +27,6 @@ describe Projects::ImportsController do
     context 'when repository exists' do
       let(:project) { create(:project_empty_repo, import_url: 'https://github.com/vim/vim.git') }
 
-      before do
-        sign_in(user)
-        project.team << [user, :master]
-      end
-
       context 'when import is in progress' do
         before do
           project.update_attribute(:import_status, :started)
@@ -125,4 +119,23 @@ describe Projects::ImportsController do
       end
     end
   end
+
+  context 'POST #create' do
+    context 'mirror user is not the current user' do
+      it 'should only assign the current user' do
+        allow_any_instance_of(EE::Project).to receive(:add_import_job)
+
+        new_user = create(:user)
+        project.add_master(new_user)
+
+        post :create, namespace_id: project.namespace.to_param,
+                      project_id: project,
+                      project: { mirror: true, mirror_user_id: new_user.id, import_url: 'http://local.dev' },
+                      format: :json
+
+        expect(project.reload.mirror).to eq(true)
+        expect(project.reload.mirror_user.id).to eq(user.id)
+      end
+    end
+  end
 end

spec/controllers/projects/mirrors_controller_spec.rb

@@ -71,6 +71,20 @@ describe Projects::MirrorsController do
           expect(project.reload.mirror).to eq(true)
           expect(project.reload.import_url).to eq('http://local.dev')
         end
+
+        context 'mirror user is not the current user' do
+          it 'should only assign the current user' do
+            expect_any_instance_of(EE::Project).to receive(:force_import_job!)
+
+            new_user = create(:user)
+            project.add_master(new_user)
+
+            do_put(project, mirror: true, mirror_user_id: new_user.id, import_url: 'http://local.dev')
+
+            expect(project.reload.mirror).to eq(true)
+            expect(project.reload.mirror_user.id).to eq(project.owner.id)
+          end
+        end
       end
     end
   end

spec/controllers/snippets_controller_spec.rb

@@ -186,8 +186,8 @@ describe SnippetsController do
     end
 
     context 'when the snippet description contains a file' do
-      let(:picture_file) { '/temp/secret56/picture.jpg' }
-      let(:text_file) { '/temp/secret78/text.txt' }
+      let(:picture_file) { '/system/temp/secret56/picture.jpg' }
+      let(:text_file) { '/system/temp/secret78/text.txt' }
       let(:description) do
         "Description with picture: ![picture](/uploads#{picture_file}) and "\
         "text: [text.txt](/uploads#{text_file})"
@@ -208,8 +208,8 @@ describe SnippetsController do
         snippet = subject
 
         expected_description = "Description with picture: "\
-          "![picture](/uploads/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\
-          "text: [text.txt](/uploads/personal_snippet/#{snippet.id}/secret78/text.txt)"
+          "![picture](/uploads/system/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\
+          "text: [text.txt](/uploads/system/personal_snippet/#{snippet.id}/secret78/text.txt)"
 
         expect(snippet.description).to eq(expected_description)
       end

spec/controllers/uploads_controller_spec.rb

@@ -102,7 +102,7 @@ describe UploadsController do
           subject
 
           expect(response.body).to match '\"alt\":\"rails_sample\"'
-          expect(response.body).to match "\"url\":\"/uploads/temp"
+          expect(response.body).to match "\"url\":\"/uploads/system/temp"
         end
 
         it 'does not create an Upload record' do
@@ -119,7 +119,7 @@ describe UploadsController do
           subject
 
           expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
-          expect(response.body).to match "\"url\":\"/uploads/temp"
+          expect(response.body).to match "\"url\":\"/uploads/system/temp"
         end
 
         it 'does not create an Upload record' do

spec/features/admin/admin_geo_nodes_spec.rb

+require 'spec_helper'
+
+RSpec.describe 'admin Geo Nodes', type: :feature do
+  let!(:geo_node) { create(:geo_node) }
+
+  before do
+    allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
+    sign_in(create(:admin))
+  end
+
+  it 'show all public Geo Nodes' do
+    visit admin_geo_nodes_path
+
+    page.within(find('.geo-nodes', match: :first)) do
+      expect(page).to have_content(geo_node.url)
+    end
+  end
+
+  describe 'create a new Geo Nodes' do
+    let(:new_ssh_key) { attributes_for(:key)[:key] }
+
+    before do
+      visit admin_geo_nodes_path
+    end
+
+    it 'creates a new Geo Node' do
+      check 'This is a primary node'
+      fill_in 'geo_node_url', with: 'https://test.gitlab.com'
+      fill_in 'geo_node_geo_node_key_attributes_key', with: new_ssh_key
+      click_button 'Add Node'
+
+      expect(current_path).to eq admin_geo_nodes_path
+
+      page.within(find('.geo-nodes', match: :first)) do
+        expect(page).to have_content(geo_node.url)
+      end
+    end
+  end
+
+  describe 'update an existing Geo Node' do
+    before do
+      visit admin_geo_nodes_path
+      page.within(find('.node-actions', match: :first)) do
+        page.click_link('Edit')
+      end
+    end
+
+    it 'updates an existing Geo Node' do
+      fill_in 'URL', with: 'http://newsite.com'
+      check 'This is a primary node'
+      click_button 'Save changes'
+
+      expect(current_path).to eq admin_geo_nodes_path
+
+      page.within(find('.geo-nodes', match: :first)) do
+        expect(page).to have_content('http://newsite.com')
+        expect(page).to have_content('Primary')
+      end
+    end
+  end
+
+  describe 'remove an existing Geo Node' do
+    before do
+      visit admin_geo_nodes_path
+    end
+
+    it 'removes an existing Geo Node' do
+      page.within(find('.node-actions', match: :first)) do
+        page.click_link('Remove')
+      end
+
+      expect(current_path).to eq admin_geo_nodes_path
+      expect(page).not_to have_css('.geo-nodes')
+    end
+  end
+end

spec/features/dashboard/issues_spec.rb

@@ -62,7 +62,7 @@ RSpec.describe 'Dashboard Issues', feature: true do
 
     it 'state filter tabs work' do
       find('#state-closed').click
-      expect(page).to have_current_path(issues_dashboard_url(assignee_id: current_user.id, scope: 'all', state: 'closed'), url: true)
+      expect(page).to have_current_path(issues_dashboard_url(assignee_id: current_user.id, state: 'closed'), url: true)
     end
 
     it_behaves_like "it has an RSS button with current_user's RSS token"

spec/features/es_global_search_spec.rb

@@ -93,6 +93,7 @@ feature 'Global elastic search', feature: true do
       select_filter("Commits")
 
       expect(page).to have_selector('.commit-row-description')
+      expect(page).to have_selector('.project_namespace')
     end
   end
 end

spec/features/projects/issuable_counts_caching_spec.rb

+require 'spec_helper'
+
+describe 'Issuable counts caching', :use_clean_rails_memory_store_caching do
+  let!(:member) { create(:user) }
+  let!(:member_2) { create(:user) }
+  let!(:non_member) { create(:user) }
+  let!(:project) { create(:empty_project, :public) }
+  let!(:open_issue) { create(:issue, project: project) }
+  let!(:confidential_issue) { create(:issue, :confidential, project: project, author: non_member) }
+  let!(:closed_issue) { create(:issue, :closed, project: project) }
+
+  before do
+    project.add_developer(member)
+    project.add_developer(member_2)
+  end
+
+  it 'caches issuable counts correctly for non-members' do
+    # We can't use expect_any_instance_of because that uses a single instance.
+    counts = 0
+
+    allow_any_instance_of(IssuesFinder).to receive(:count_by_state).and_wrap_original do |m, *args|
+      counts += 1
+
+      m.call(*args)
+    end
+
+    aggregate_failures 'only counts once on first load with no params, and caches for later loads' do
+      expect { visit project_issues_path(project) }
+        .to change { counts }.by(1)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+    end
+
+    aggregate_failures 'uses counts from cache on load from non-member' do
+      sign_in(non_member)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_out(non_member)
+    end
+
+    aggregate_failures 'does not use the same cache for a member' do
+      sign_in(member)
+
+      expect { visit project_issues_path(project) }
+        .to change { counts }.by(1)
+
+      sign_out(member)
+    end
+
+    aggregate_failures 'uses the same cache for all members' do
+      sign_in(member_2)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_out(member_2)
+    end
+
+    aggregate_failures 'shares caches when params are passed' do
+      expect { visit project_issues_path(project, author_username: non_member.username) }
+        .to change { counts }.by(1)
+
+      sign_in(member)
+
+      expect { visit project_issues_path(project, author_username: non_member.username) }
+        .to change { counts }.by(1)
+
+      sign_in(non_member)
+
+      expect { visit project_issues_path(project, author_username: non_member.username) }
+        .not_to change { counts }
+
+      sign_in(member_2)
+
+      expect { visit project_issues_path(project, author_username: non_member.username) }
+        .not_to change { counts }
+
+      sign_out(member_2)
+    end
+
+    aggregate_failures 'resets caches on issue close' do
+      Issues::CloseService.new(project, member).execute(open_issue)
+
+      expect { visit project_issues_path(project) }
+        .to change { counts }.by(1)
+
+      sign_in(member)
+
+      expect { visit project_issues_path(project) }
+        .to change { counts }.by(1)
+
+      sign_in(non_member)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_in(member_2)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_out(member_2)
+    end
+
+    aggregate_failures 'does not reset caches on issue update' do
+      Issues::UpdateService.new(project, member, title: 'new title').execute(open_issue)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_in(member)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_in(non_member)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_in(member_2)
+
+      expect { visit project_issues_path(project) }
+        .not_to change { counts }
+
+      sign_out(member_2)
+    end
+  end
+end

spec/features/projects/settings/ee/push_rules_settings_spec.rb

@@ -60,7 +60,7 @@ describe 'Project settings > [EE] repository', feature: true do
 
         click_button('Save changes')
 
-        expect(find('.select2-chosen')).to have_content(user2.name)
+        expect(find('.select2-chosen')).to have_content(user.name)
       end
     end
   end

spec/features/snippets/user_creates_snippet_spec.rb

@@ -41,7 +41,7 @@ feature 'User creates snippet', :js, feature: true do
       expect(page).to have_content('My Snippet')
 
       link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
-      expect(link).to match(%r{/uploads/temp/\h{32}/banana_sample\.gif\z})
+      expect(link).to match(%r{/uploads/system/temp/\h{32}/banana_sample\.gif\z})
 
       visit(link)
       expect(page.status_code).to eq(200)
@@ -59,7 +59,7 @@ feature 'User creates snippet', :js, feature: true do
     wait_for_requests
 
     link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
-    expect(link).to match(%r{/uploads/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
+    expect(link).to match(%r{/uploads/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
 
     visit(link)
     expect(page.status_code).to eq(200)
@@ -84,7 +84,7 @@ feature 'User creates snippet', :js, feature: true do
     end
     expect(page).to have_content('Hello World!')
     link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
-    expect(link).to match(%r{/uploads/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
+    expect(link).to match(%r{/uploads/system/personal_snippet/#{Snippet.last.id}/\h{32}/banana_sample\.gif\z})
 
     visit(link)
     expect(page.status_code).to eq(200)

spec/features/snippets/user_edits_snippet_spec.rb

@@ -33,7 +33,7 @@ feature 'User edits snippet', :js, feature: true do
     wait_for_requests
 
     link = find('a.no-attachment-icon img[alt="banana_sample"]')['src']
-    expect(link).to match(%r{/uploads/personal_snippet/#{snippet.id}/\h{32}/banana_sample\.gif\z})
+    expect(link).to match(%r{/uploads/system/personal_snippet/#{snippet.id}/\h{32}/banana_sample\.gif\z})
   end
 
   it 'updates the snippet to make it internal' do

spec/lib/csv_builder_spec.rb

@@ -82,4 +82,22 @@ describe CsvBuilder, lib: true do
   it 'allows lamdas to look up more complicated data' do
     expect(csv_data).to include 'rewsna'
   end
+
+  describe 'excel sanitization' do
+    let(:dangerous_title) { double(title: "=cmd|' /C calc'!A0 title", description: "*safe_desc") }
+    let(:dangerous_desc) { double(title: "*safe_title", description: "=cmd|' /C calc'!A0 desc") }
+    let(:fake_relation) { FakeRelation.new([dangerous_title, dangerous_desc]) }
+    let(:subject) { CsvBuilder.new(fake_relation, 'Title' => 'title', 'Description' => 'description') }
+    let(:csv_data) { subject.render }
+
+    it 'sanitizes dangerous characters at the beginning of a column' do
+      expect(csv_data).to include "'=cmd|' /C calc'!A0 title"
+      expect(csv_data).to include "'=cmd|' /C calc'!A0 desc"
+    end
+
+    it 'does not sanitize safe symbols at the beginning of a column' do
+      expect(csv_data).not_to include "'*safe_desc"
+      expect(csv_data).not_to include "'*safe_title"
+    end
+  end
 end

spec/lib/gitlab/ci/trace/stream_spec.rb

@@ -293,5 +293,12 @@ describe Gitlab::Ci::Trace::Stream do
 
       it { is_expected.to eq("65") }
     end
+
+    context 'malicious regexp' do
+      let(:data) { malicious_text }  
+      let(:regex) { malicious_regexp }
+
+      include_examples 'malicious regexp'
+    end
   end
 end

spec/lib/gitlab/geo/health_check_spec.rb

@@ -8,7 +8,6 @@ describe Gitlab::Geo::HealthCheck, :postgresql do
   describe '.perform_checks' do
     it 'returns a string if database is not fully migrated' do
       allow(Gitlab::Geo).to receive(:secondary?) { true }
-      allow(Gitlab::Geo).to receive(:secondary_role_enabled?).and_return(true)
       allow(described_class).to receive(:geo_database_configured?).and_return(true)
       allow(described_class).to receive(:database_secondary?).and_return(true)
       allow(described_class).to receive(:get_database_version).and_return('20170101')
@@ -26,37 +25,32 @@ describe Gitlab::Geo::HealthCheck, :postgresql do
       expect(subject.perform_checks).to be_blank
     end
 
-    it 'returns an error when secondary role is disabled' do
-      allow(Gitlab::Geo).to receive(:secondary?) { true }
-      allow(Gitlab::Geo).to receive(:secondary_role_enabled?).and_return(false)
-
-      expect(subject.perform_checks).not_to be_blank
-    end
-
     it 'returns an error when database is not configured for streaming replication' do
       allow(Gitlab::Geo).to receive(:secondary?) { true }
       allow(Gitlab::Database).to receive(:postgresql?) { true }
       allow(ActiveRecord::Base).to receive_message_chain(:connection, :execute, :first, :fetch) { 'f' }
 
-      expect(subject.perform_checks).not_to be_blank
+      expect(subject.perform_checks).to include('not configured for streaming replication')
     end
 
     it 'returns an error when configuration file is missing for tracking DB' do
       allow(Rails.configuration).to receive(:respond_to?).with(:geo_database) { false }
 
-      expect(subject.perform_checks).not_to be_blank
+      expect(subject.perform_checks).to include('database configuration file is missing')
     end
 
     it 'returns an error when Geo database version does not match the latest migration version' do
+      allow(described_class).to receive(:database_secondary?).and_return(true)
       allow(subject).to receive(:get_database_version) { 1 }
 
-      expect(subject.perform_checks).not_to be_blank
+      expect(subject.perform_checks).to match(/Current Geo database version \([0-9]+\) does not match latest migration \([0-9]+\)/)
     end
 
     it 'returns an error when latest migration version does not match the Geo database version' do
+      allow(described_class).to receive(:database_secondary?).and_return(true)
       allow(subject).to receive(:get_migration_version) { 1 }
 
-      expect(subject.perform_checks).not_to be_blank
+      expect(subject.perform_checks).to match(/Current Geo database version \([0-9]+\) does not match latest migration \([0-9]+\)/)
     end
   end
 

spec/lib/gitlab/geo_spec.rb

@@ -25,6 +25,43 @@ describe Gitlab::Geo, lib: true do
     end
   end
 
+  describe 'primary?' do
+    context 'when current node is a primary node' do
+      before(:each) do
+        primary_node
+      end
+
+      it 'returns true' do
+        expect(described_class.primary?).to be_truthy
+      end
+
+      it 'returns false when GeoNode is disabled' do
+        allow(described_class).to receive(:enabled?) { false }
+
+        expect(described_class.primary?).to be_falsey
+      end
+    end
+  end
+
+  describe 'secondary?' do
+    context 'when current node is a secondary node' do
+      before(:each) do
+        secondary_node
+        allow(described_class).to receive(:current_node) { secondary_node }
+      end
+
+      it 'returns true' do
+        expect(described_class.secondary?).to be_truthy
+      end
+
+      it 'returns false when GeoNode is disabled' do
+        allow(described_class).to receive(:enabled?) { false }
+
+        expect(described_class.secondary?).to be_falsey
+      end
+    end
+  end
+
   describe 'enabled?' do
     context 'when any GeoNode exists' do
       before do
@@ -42,6 +79,24 @@ describe Gitlab::Geo, lib: true do
       end
     end
 
+    context 'when there is a database issue' do
+      it 'returns false when database connection is down' do
+        allow(GeoNode).to receive(:connected?) { false }
+
+        expect(described_class.enabled?).to be_falsey
+      end
+
+      it 'returns false when database schema does not contain required tables' do
+        if Gitlab::Database.mysql?
+          allow(GeoNode).to receive(:exists?).and_raise(Mysql2::Error, "Table 'gitlabhq_test.geo_nodes' doesn't exist: SHOW FULL FIELDS FROM `geo_nodes`")
+        else
+          allow(GeoNode).to receive(:exists?).and_raise(PG::UndefinedTable)
+        end
+
+        expect(described_class.enabled?).to be_falsey
+      end
+    end
+
     context 'with RequestStore enabled' do
       before do
         RequestStore.begin!
@@ -142,8 +197,8 @@ describe Gitlab::Geo, lib: true do
     end
 
     it 'activates cron jobs for primary' do
-      allow(described_class).to receive(:primary_role_enabled?).and_return(true)
-      allow(described_class).to receive(:secondary_role_enabled?).and_return(false)
+      allow(described_class).to receive(:primary?).and_return(true)
+      allow(described_class).to receive(:secondary?).and_return(false)
 
       described_class.configure_cron_jobs!
 
@@ -154,8 +209,8 @@ describe Gitlab::Geo, lib: true do
     end
 
     it 'activates cron jobs for secondary' do
-      allow(described_class).to receive(:primary_role_enabled?).and_return(false)
-      allow(described_class).to receive(:secondary_role_enabled?).and_return(true)
+      allow(described_class).to receive(:primary?).and_return(false)
+      allow(described_class).to receive(:secondary?).and_return(true)
 
       described_class.configure_cron_jobs!
 
@@ -166,8 +221,8 @@ describe Gitlab::Geo, lib: true do
     end
 
     it 'deactivates all jobs when Geo is not active' do
-      allow(described_class).to receive(:primary_role_enabled?).and_return(false)
-      allow(described_class).to receive(:secondary_role_enabled?).and_return(false)
+      allow(described_class).to receive(:primary?).and_return(false)
+      allow(described_class).to receive(:secondary?).and_return(false)
 
       described_class.configure_cron_jobs!
 
@@ -178,14 +233,14 @@ describe Gitlab::Geo, lib: true do
     end
 
     it 'reactivates cron jobs when node turns off Geo' do
-      allow(described_class).to receive(:primary_role_enabled?).and_return(false)
-      allow(described_class).to receive(:secondary_role_enabled?).and_return(true)
+      allow(described_class).to receive(:primary?).and_return(false)
+      allow(described_class).to receive(:secondary?).and_return(true)
 
       described_class.configure_cron_jobs!
       expect(Sidekiq::Cron::Job.find('ldap_test')).not_to be_enabled
 
-      allow(described_class).to receive(:primary_role_enabled?).and_return(false)
-      allow(described_class).to receive(:secondary_role_enabled?).and_return(false)
+      allow(described_class).to receive(:primary?).and_return(false)
+      allow(described_class).to receive(:secondary?).and_return(false)
 
       described_class.configure_cron_jobs!
       expect(Sidekiq::Cron::Job.find('ldap_test')).to be_enabled

spec/lib/gitlab/route_map_spec.rb

@@ -55,6 +55,19 @@ describe Gitlab::RouteMap, lib: true do
   end
 
   describe '#public_path_for_source_path' do
+    context 'malicious regexp' do
+      include_examples 'malicious regexp'
+
+      subject do
+        map = described_class.new(<<-"MAP".strip_heredoc)
+        - source: '#{malicious_regexp}'
+          public: '/'
+        MAP
+
+        map.public_path_for_source_path(malicious_text)
+      end
+    end
+
     subject do
       described_class.new(<<-'MAP'.strip_heredoc)
         # Team data

spec/lib/gitlab/untrusted_regexp_spec.rb

+require 'spec_helper'
+
+describe Gitlab::UntrustedRegexp do
+  describe '#initialize' do
+    subject { described_class.new(pattern) }
+
+    context 'invalid regexp' do
+      let(:pattern) { '[' }
+
+      it { expect { subject }.to raise_error(RegexpError) }
+    end
+  end
+
+  describe '#replace_all' do
+    it 'replaces all instances of the match in a string' do
+      result = described_class.new('foo').replace_all('foo bar foo', 'oof')
+
+      expect(result).to eq('oof bar oof')
+    end
+  end
+
+  describe '#replace' do
+    it 'replaces the first instance of the match in a string' do
+      result = described_class.new('foo').replace('foo bar foo', 'oof')
+
+      expect(result).to eq('oof bar foo')
+    end
+  end
+
+  describe '#===' do
+    it 'returns true for a match' do
+      result = described_class.new('foo') === 'a foo here'
+
+      expect(result).to be_truthy
+    end
+
+    it 'returns false for no match' do
+      result = described_class.new('foo') === 'a bar here'
+
+      expect(result).to be_falsy
+    end
+  end
+
+  describe '#scan' do
+    subject { described_class.new(regexp).scan(text) }
+    context 'malicious regexp' do
+      let(:text) { malicious_text }
+      let(:regexp) { malicious_regexp }
+ 
+      include_examples 'malicious regexp'
+    end
+
+    context 'no capture group' do
+      let(:regexp) { '.+' }
+      let(:text) { 'foo' }
+
+      it 'returns the whole match' do
+        is_expected.to eq(['foo'])
+      end
+    end
+
+    context 'one capture group' do
+      let(:regexp) { '(f).+' }
+      let(:text) { 'foo' }
+
+      it 'returns the captured part' do
+        is_expected.to eq([%w[f]])
+      end
+    end
+
+    context 'two capture groups' do
+      let(:regexp) { '(f).(o)' }
+      let(:text) { 'foo' }
+
+      it 'returns the captured parts' do
+        is_expected.to eq([%w[f o]])
+      end
+    end
+  end
+end

spec/migrations/clean_appearance_symlinks_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170613111224_clean_appearance_symlinks.rb')
+
+describe CleanAppearanceSymlinks do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "tests", "clean_appearance_test") }
+  let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+  let(:new_uploads_dir) { File.join(uploads_dir, "system") }
+  let(:original_path) { File.join(new_uploads_dir, 'appearance') }
+  let(:symlink_path) { File.join(uploads_dir, 'appearance') }
+
+  before do
+    FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+    FileUtils.mkdir_p(uploads_dir)
+    allow(migration).to receive(:base_directory).and_return(test_dir)
+    allow(migration).to receive(:say)
+  end
+
+  describe "#up" do
+    before do
+      FileUtils.mkdir_p(original_path)
+      FileUtils.ln_s(original_path, symlink_path)
+    end
+
+    it 'removes the symlink' do
+      migration.up
+
+      expect(File.symlink?(symlink_path)).to be(false)
+    end
+  end
+
+  describe '#down' do
+    before do
+      FileUtils.mkdir_p(File.join(original_path))
+      FileUtils.touch(File.join(original_path, 'dummy.file'))
+    end
+
+    it 'creates a symlink' do
+      expected_path = File.join(symlink_path, "dummy.file")
+      migration.down
+
+      expect(File.exist?(expected_path)).to be(true)
+      expect(File.symlink?(symlink_path)).to be(true)
+    end
+  end
+end

spec/migrations/move_personal_snippets_files_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170612071012_move_personal_snippets_files.rb')
+
+describe MovePersonalSnippetsFiles do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_snippet_files_test") }
+  let(:uploads_dir) { File.join(test_dir, 'uploads') }
+  let(:new_uploads_dir) { File.join(uploads_dir, 'system') }
+
+  before do
+    allow(CarrierWave).to receive(:root).and_return(test_dir)
+    allow(migration).to receive(:base_directory).and_return(test_dir)
+    FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+    allow(migration).to receive(:say)
+  end
+
+  describe "#up" do
+    let(:snippet) do
+      snippet = create(:personal_snippet)
+      create_upload('picture.jpg', snippet)
+      snippet.update(description: markdown_linking_file('picture.jpg', snippet))
+      snippet
+    end
+
+    let(:snippet_with_missing_file) do
+      snippet = create(:snippet)
+      create_upload('picture.jpg', snippet, create_file: false)
+      snippet.update(description: markdown_linking_file('picture.jpg', snippet))
+      snippet
+    end
+
+    it 'moves the files' do
+      source_path = File.join(uploads_dir, model_file_path('picture.jpg', snippet))
+      destination_path = File.join(new_uploads_dir, model_file_path('picture.jpg', snippet))
+
+      migration.up
+
+      expect(File.exist?(source_path)).to be_falsy
+      expect(File.exist?(destination_path)).to be_truthy
+    end
+
+    describe 'updating the markdown' do
+      it 'includes the new path when the file exists' do
+        secret = "secret#{snippet.id}"
+        file_location = "/uploads/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg"
+
+        migration.up
+
+        expect(snippet.reload.description).to include(file_location)
+      end
+
+      it 'does not update the markdown when the file is missing' do
+        secret = "secret#{snippet_with_missing_file.id}"
+        file_location = "/uploads/personal_snippet/#{snippet_with_missing_file.id}/#{secret}/picture.jpg"
+
+        migration.up
+
+        expect(snippet_with_missing_file.reload.description).to include(file_location)
+      end
+
+      it 'updates the note markdown' do
+        secret = "secret#{snippet.id}"
+        file_location = "/uploads/system/personal_snippet/#{snippet.id}/#{secret}/picture.jpg"
+        markdown = markdown_linking_file('picture.jpg', snippet)
+        note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown}")
+
+        migration.up
+
+        expect(note.reload.note).to include(file_location)
+      end
+    end
+  end
+
+  describe "#down" do
+    let(:snippet) do
+      snippet = create(:personal_snippet)
+      create_upload('picture.jpg', snippet, in_new_path: true)
+      snippet.update(description: markdown_linking_file('picture.jpg', snippet, in_new_path: true))
+      snippet
+    end
+
+    let(:snippet_with_missing_file) do
+      snippet = create(:personal_snippet)
+      create_upload('picture.jpg', snippet, create_file: false, in_new_path: true)
+      snippet.update(description: markdown_linking_file('picture.jpg', snippet, in_new_path: true))
+      snippet
+    end
+
+    it 'moves the files' do
+      source_path = File.join(new_uploads_dir, model_file_path('picture.jpg', snippet))
+      destination_path = File.join(uploads_dir, model_file_path('picture.jpg', snippet))
+
+      migration.down
+
+      expect(File.exist?(source_path)).to be_falsey
+      expect(File.exist?(destination_path)).to be_truthy
+    end
+
+    describe 'updating the markdown' do
+      it 'includes the new path when the file exists' do
+        secret = "secret#{snippet.id}"
+        file_location = "/uploads/personal_snippet/#{snippet.id}/#{secret}/picture.jpg"
+
+        migration.down
+
+        expect(snippet.reload.description).to include(file_location)
+      end
+
+      it 'keeps the markdown as is when the file is missing' do
+        secret = "secret#{snippet_with_missing_file.id}"
+        file_location = "/uploads/system/personal_snippet/#{snippet_with_missing_file.id}/#{secret}/picture.jpg"
+
+        migration.down
+
+        expect(snippet_with_missing_file.reload.description).to include(file_location)
+      end
+
+      it 'updates the note markdown' do
+        markdown = markdown_linking_file('picture.jpg', snippet, in_new_path: true)
+        secret = "secret#{snippet.id}"
+        file_location = "/uploads/personal_snippet/#{snippet.id}/#{secret}/picture.jpg"
+        note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown}")
+
+        migration.down
+
+        expect(note.reload.note).to include(file_location)
+      end
+    end
+  end
+
+  describe '#update_markdown' do
+    it 'escapes sql in the snippet description' do
+      migration.instance_variable_set('@source_relative_location', '/uploads/personal_snippet')
+      migration.instance_variable_set('@destination_relative_location', '/uploads/system/personal_snippet')
+
+      secret = '123456789'
+      filename = 'hello.jpg'
+      snippet = create(:personal_snippet)
+
+      path_before = "/uploads/personal_snippet/#{snippet.id}/#{secret}/#{filename}"
+      path_after = "/uploads/system/personal_snippet/#{snippet.id}/#{secret}/#{filename}"
+      description_before = "Hello world; ![image](#{path_before})'; select * from users;"
+      description_after = "Hello world; ![image](#{path_after})'; select * from users;"
+
+      migration.update_markdown(snippet.id, secret, filename, description_before)
+
+      expect(snippet.reload.description).to eq(description_after)
+    end
+  end
+
+  def create_upload(filename, snippet, create_file: true, in_new_path: false)
+    secret = "secret#{snippet.id}"
+    absolute_path = if in_new_path
+                      File.join(new_uploads_dir, model_file_path(filename, snippet))
+                    else
+                      File.join(uploads_dir, model_file_path(filename, snippet))
+                    end
+
+    if create_file
+      FileUtils.mkdir_p(File.dirname(absolute_path))
+      FileUtils.touch(absolute_path)
+    end
+
+    create(:upload, model: snippet, path: "#{secret}/#{filename}", uploader: PersonalFileUploader)
+  end
+
+  def markdown_linking_file(filename, snippet, in_new_path: false)
+    markdown =  "![#{filename.split('.')[0]}]"
+    markdown += '(/uploads'
+    markdown += '/system' if in_new_path
+    markdown += "/#{model_file_path(filename, snippet)})"
+    markdown
+  end
+
+  def model_file_path(filename, snippet)
+    secret = "secret#{snippet.id}"
+
+    File.join('personal_snippet', snippet.id.to_s, secret, filename)
+  end
+end

spec/support/db_cleaner.rb

@@ -34,7 +34,7 @@ RSpec.configure do |config|
   end
 
   def setup_database_cleaner
-    if Gitlab::Geo.secondary_role_enabled?
+    if Gitlab::Geo.geo_database_configured?
       DatabaseCleaner[:active_record, { connection: Geo::BaseRegistry }]
     end
 

spec/support/malicious_regexp_shared_examples.rb

+shared_examples 'malicious regexp' do
+  let(:malicious_text)  { 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!' }
+  let(:malicious_regexp) { '(?i)^(([a-z])+.)+[A-Z]([a-z])+$' }
+
+  it 'takes under a second' do
+    expect { Timeout.timeout(1) { subject } }.not_to raise_error
+  end
+end

spec/uploaders/file_mover_spec.rb

@@ -4,11 +4,11 @@ describe FileMover do
   let(:filename) { 'banana_sample.gif' }
   let(:file) { fixture_file_upload(Rails.root.join('spec', 'fixtures', filename)) }
   let(:temp_description) do
-    'test ![banana_sample](/uploads/temp/secret55/banana_sample.gif) same ![banana_sample]'\
-    '(/uploads/temp/secret55/banana_sample.gif)'
+    'test ![banana_sample](/uploads/system/temp/secret55/banana_sample.gif) same ![banana_sample]'\
+    '(/uploads/system/temp/secret55/banana_sample.gif)'
   end
   let(:temp_file_path) { File.join('secret55', filename).to_s }
-  let(:file_path) { File.join('uploads', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s }
+  let(:file_path) { File.join('uploads', 'system', 'personal_snippet', snippet.id.to_s, 'secret55', filename).to_s }
 
   let(:snippet) { create(:personal_snippet, description: temp_description) }
 
@@ -28,8 +28,8 @@ describe FileMover do
 
         expect(snippet.reload.description)
           .to eq(
-            "test ![banana_sample](/uploads/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"\
-            " same ![banana_sample](/uploads/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"
+            "test ![banana_sample](/uploads/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"\
+            " same ![banana_sample](/uploads/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif)"
           )
       end
 
@@ -50,8 +50,8 @@ describe FileMover do
 
         expect(snippet.reload.description)
           .to eq(
-            "test ![banana_sample](/uploads/temp/secret55/banana_sample.gif)"\
-            " same ![banana_sample](/uploads/temp/secret55/banana_sample.gif)"
+            "test ![banana_sample](/uploads/system/temp/secret55/banana_sample.gif)"\
+            " same ![banana_sample](/uploads/system/temp/secret55/banana_sample.gif)"
           )
       end
 

spec/uploaders/personal_file_uploader_spec.rb

@@ -10,7 +10,7 @@ describe PersonalFileUploader do
 
       dynamic_segment = "personal_snippet/#{snippet.id}"
 
-      expect(described_class.absolute_path(upload)).to end_with("#{dynamic_segment}/secret/foo.jpg")
+      expect(described_class.absolute_path(upload)).to end_with("/system/#{dynamic_segment}/secret/foo.jpg")
     end
   end
 
@@ -19,7 +19,7 @@ describe PersonalFileUploader do
       uploader = described_class.new(snippet, 'secret')
 
       allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name'))
-      expected_url = "/uploads/personal_snippet/#{snippet.id}/secret/file_name"
+      expected_url = "/uploads/system/personal_snippet/#{snippet.id}/secret/file_name"
 
       expect(uploader.to_h).to eq(
         alt: 'file_name',

spec/workers/geo_file_download_dispatch_worker_spec.rb

@@ -16,7 +16,7 @@ describe GeoFileDownloadDispatchWorker do
     it 'does not schedule anything when secondary role is disabled' do
       create(:lfs_object, :with_file)
 
-      allow(Gitlab::Geo).to receive(:secondary_role_enabled?) { false }
+      allow(Gitlab::Geo).to receive(:geo_database_configured?) { false }
 
       expect(GeoFileDownloadWorker).not_to receive(:perform_async)
 

spec/workers/geo_repository_sync_worker_spec.rb

@@ -38,8 +38,8 @@ describe GeoRepositorySyncWorker do
       subject.perform
     end
 
-    it 'does not perform Geo::ProjectSyncWorker when secondary role is disabled' do
-      allow(Gitlab::Geo).to receive(:secondary_role_enabled?) { false }
+    it 'does not perform Geo::ProjectSyncWorker when no geo database is configured' do
+      allow(Gitlab::Geo).to receive(:geo_database_configured?) { false }
 
       expect(Geo::ProjectSyncWorker).not_to receive(:perform_in)