Bring in security changes from the 9.2.5 release

Ran:
 - git format-patch v9.2.2..v9.2.5 --stdout > patchfile.patch
 - git checkout -b 9-2-5-security-patch origin/v9.2.2
 - git apply patchfile.patch
 - git commit
 - [Got the sha ref for the commit]
 - git checkout -b upstream-9-2-security master
 - git cherry-pick <SHA of the patchfile commit>
 - [Resolved conflicts]
 - git cherry-pick --continue

28 files changed, 746 insertions, 41 deletions

diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb
index 2c9d1ffc9c2..4c3a5ec49ef 100644
--- a/spec/controllers/autocomplete_controller_spec.rb
+++ b/spec/controllers/autocomplete_controller_spec.rb
@@ -170,22 +170,32 @@ describe AutocompleteController do
     end
 
     context 'author of issuable included' do
-      before do
-        sign_in(user)
-      end
-
       let(:body) { JSON.parse(response.body) }
 
-      it 'includes the author' do
-        get(:users, author_id: non_member.id)
+      context 'authenticated' do
+        before do
+          sign_in(user)
+        end
+
+        it 'includes the author' do
+          get(:users, author_id: non_member.id)
+
+          expect(body.first["username"]).to eq non_member.username
+        end
+
+        it 'rejects non existent user ids' do
+          get(:users, author_id: 99999)
 
-        expect(body.first["username"]).to eq non_member.username
+          expect(body.collect { |u| u['id'] }).not_to include(99999)
+        end
       end
 
-      it 'rejects non existent user ids' do
-        get(:users, author_id: 99999)
+      context 'without authenticating' do
+        it 'returns empty result' do
+          get(:users, author_id: non_member.id)
 
-        expect(body.collect { |u| u['id'] }).not_to include(99999)
+          expect(body).to be_empty
+        end
       end
     end
 
diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb
new file mode 100644
index 00000000000..1383420fb44
--- /dev/null
+++ b/spec/factories/uploads.rb
@@ -0,0 +1,8 @@
+FactoryGirl.define do
+  factory :upload do
+    model { build(:project) }
+    path { "uploads/system/project/avatar/avatar.jpg" }
+    size 100.kilobytes
+    uploader "AvatarUploader"
+  end
+end
diff --git a/spec/features/admin/admin_appearance_spec.rb b/spec/features/admin/admin_appearance_spec.rb
index 96d715ef383..595366ce352 100644
--- a/spec/features/admin/admin_appearance_spec.rb
+++ b/spec/features/admin/admin_appearance_spec.rb
@@ -63,11 +63,11 @@ feature 'Admin Appearance', feature: true do
   end
 
   def logo_selector
-    '//img[@src^="/uploads/appearance/logo"]'
+    '//img[@src^="/uploads/system/appearance/logo"]'
   end
 
   def header_logo_selector
-    '//img[@src^="/uploads/appearance/header_logo"]'
+    '//img[@src^="/uploads/system/appearance/header_logo"]'
   end
 
   def logo_fixture
diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
index f88a515f7fc..d9d6f2e2382 100644
--- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb
@@ -18,7 +18,7 @@ feature 'User uploads avatar to group', feature: true do
 
     visit group_path(group)
 
-    expect(page).to have_selector(%Q(img[src$="/uploads/group/avatar/#{group.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[src$="/uploads/system/group/avatar/#{group.id}/dk.png"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(group.reload.avatar.file).to exist
diff --git a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
index 0dfd29045e5..eb8dbd76aab 100644
--- a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
+++ b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb
@@ -16,7 +16,7 @@ feature 'User uploads avatar to profile', feature: true do
 
     visit user_path(user)
 
-    expect(page).to have_selector(%Q(img[src$="/uploads/user/avatar/#{user.id}/dk.png"]))
+    expect(page).to have_selector(%Q(img[src$="/uploads/system/user/avatar/#{user.id}/dk.png"]))
 
     # Cheating here to verify something that isn't user-facing, but is important
     expect(user.reload.avatar.file).to exist
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 785fb724132..49df91b236f 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -1,3 +1,4 @@
+# coding: utf-8
 require 'spec_helper'
 
 describe ApplicationHelper do
@@ -58,13 +59,13 @@ describe ApplicationHelper do
   describe 'project_icon' do
     it 'returns an url for the avatar' do
       project = create(:empty_project, avatar: File.open(uploaded_image_temp_path))
-      avatar_url = "/uploads/project/avatar/#{project.id}/banana_sample.gif"
+      avatar_url = "/uploads/system/project/avatar/#{project.id}/banana_sample.gif"
 
       expect(helper.project_icon(project.full_path).to_s).
         to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
 
       allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
-      avatar_url = "#{gitlab_host}/uploads/project/avatar/#{project.id}/banana_sample.gif"
+      avatar_url = "#{gitlab_host}/uploads/system/project/avatar/#{project.id}/banana_sample.gif"
 
       expect(helper.project_icon(project.full_path).to_s).
         to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
@@ -84,12 +85,12 @@ describe ApplicationHelper do
     it 'returns an url for the avatar' do
       user = create(:user, avatar: File.open(uploaded_image_temp_path))
 
-      avatar_url = "/uploads/user/avatar/#{user.id}/banana_sample.gif"
+      avatar_url = "/uploads/system/user/avatar/#{user.id}/banana_sample.gif"
 
       expect(helper.avatar_icon(user.email).to_s).to match(avatar_url)
 
       allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host)
-      avatar_url = "#{gitlab_host}/uploads/user/avatar/#{user.id}/banana_sample.gif"
+      avatar_url = "#{gitlab_host}/uploads/system/user/avatar/#{user.id}/banana_sample.gif"
 
       expect(helper.avatar_icon(user.email).to_s).to match(avatar_url)
     end
@@ -102,7 +103,7 @@ describe ApplicationHelper do
       user = create(:user, avatar: File.open(uploaded_image_temp_path))
 
       expect(helper.avatar_icon(user.email).to_s).
-        to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
+        to match("/gitlab/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
     end
 
     it 'calls gravatar_icon when no User exists with the given email' do
@@ -116,7 +117,7 @@ describe ApplicationHelper do
         user = create(:user, avatar: File.open(uploaded_image_temp_path))
 
         expect(helper.avatar_icon(user).to_s).
-          to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
+          to match("/uploads/system/user/avatar/#{user.id}/banana_sample.gif")
       end
     end
   end
diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
index cd112dbb2fb..c68e4f56b05 100644
--- a/spec/helpers/emails_helper_spec.rb
+++ b/spec/helpers/emails_helper_spec.rb
@@ -52,7 +52,7 @@ describe EmailsHelper do
         )
 
         expect(header_logo).to eq(
-          %{<img style="height: 50px" src="/uploads/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
+          %{<img style="height: 50px" src="/uploads/system/appearance/header_logo/#{appearance.id}/dk.png" alt="Dk" />}
         )
       end
     end
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index c8b0d86425f..0337afa4452 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -9,7 +9,7 @@ describe GroupsHelper do
       group.avatar = fixture_file_upload(avatar_file_path)
       group.save!
       expect(group_icon(group.path).to_s).
-        to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
+        to match("/uploads/system/group/avatar/#{group.id}/banana_sample.gif")
     end
 
     it 'gives default avatar_icon when no avatar is present' do
diff --git a/spec/helpers/page_layout_helper_spec.rb b/spec/helpers/page_layout_helper_spec.rb
index 2cc0b40b2d0..dff2784f21f 100644
--- a/spec/helpers/page_layout_helper_spec.rb
+++ b/spec/helpers/page_layout_helper_spec.rb
@@ -60,7 +60,7 @@ describe PageLayoutHelper do
     %w(project user group).each do |type|
       context "with @#{type} assigned" do
         it "uses #{type.titlecase} avatar if available" do
-          object = double(avatar_url: 'http://example.com/uploads/avatar.png')
+          object = double(avatar_url: 'http://example.com/uploads/system/avatar.png')
           assign(type, object)
 
           expect(helper.page_image).to eq object.avatar_url
diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js
index 24335614e09..bfd8b8648a6 100644
--- a/spec/javascripts/notes_spec.js
+++ b/spec/javascripts/notes_spec.js
@@ -461,6 +461,45 @@ import '~/notes';
       });
     });
 
+    describe('update comment with script tags', () => {
+      const sampleComment = '<script></script>';
+      const updatedComment = '<script></script>';
+      const note = {
+        id: 1234,
+        html: `<li class="note note-row-1234 timeline-entry" id="note_1234">
+                <div class="note-text">${sampleComment}</div>
+               </li>`,
+        note: sampleComment,
+        valid: true
+      };
+      let $form;
+      let $notesContainer;
+
+      beforeEach(() => {
+        this.notes = new Notes('', []);
+        window.gon.current_username = 'root';
+        window.gon.current_user_fullname = 'Administrator';
+        $form = $('form.js-main-target-form');
+        $notesContainer = $('ul.main-notes-list');
+        $form.find('textarea.js-note-text').html(sampleComment);
+      });
+
+      it('should not render a script tag', () => {
+        const deferred = $.Deferred();
+        spyOn($, 'ajax').and.returnValue(deferred.promise());
+        $('.js-comment-button').click();
+
+        deferred.resolve(note);
+        const $noteEl = $notesContainer.find(`#note_${note.id}`);
+        $noteEl.find('.js-note-edit').click();
+        $noteEl.find('textarea.js-note-text').html(updatedComment);
+        $noteEl.find('.js-comment-save-button').click();
+
+        const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`).find('.js-task-list-container');
+        expect($updatedNoteEl.find('.note-text').text().trim()).toEqual('');
+      });
+    });
+
     describe('getFormData', () => {
       let $form;
       let sampleComment;
diff --git a/spec/javascripts/vue_shared/components/commit_spec.js b/spec/javascripts/vue_shared/components/commit_spec.js
index 050170a54e9..540245fe71e 100644
--- a/spec/javascripts/vue_shared/components/commit_spec.js
+++ b/spec/javascripts/vue_shared/components/commit_spec.js
@@ -22,7 +22,7 @@ describe('Commit component', () => {
         shortSha: 'b7836edd',
         title: 'Commit message',
         author: {
-          avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
+          avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png',
           web_url: 'https://gitlab.com/jschatz1',
           path: '/jschatz1',
           username: 'jschatz1',
@@ -45,7 +45,7 @@ describe('Commit component', () => {
         shortSha: 'b7836edd',
         title: 'Commit message',
         author: {
-          avatar_url: 'https://gitlab.com/uploads/user/avatar/300478/avatar.png',
+          avatar_url: 'https://gitlab.com/uploads/system/user/avatar/300478/avatar.png',
           web_url: 'https://gitlab.com/jschatz1',
           path: '/jschatz1',
           username: 'jschatz1',
diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb
index d5746107ee1..f4f42bfc3ed 100644
--- a/spec/lib/banzai/reference_parser/base_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb
@@ -30,7 +30,7 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
       it 'checks if user can read the resource' do
         link['data-project'] = project.id.to_s
 
-        expect(subject).to receive(:can_read_reference?).with(user, project)
+        expect(subject).to receive(:can_read_reference?).with(user, project, link)
 
         subject.nodes_visible_to_user(user, [link])
       end
diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb
index d217a775802..620875ece20 100644
--- a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb
@@ -4,20 +4,199 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
   include ReferenceParserHelpers
 
   let(:project) { create(:empty_project, :public) }
+
   let(:user) { create(:user) }
-  let(:snippet) { create(:snippet, project: project) }
+  let(:external_user) { create(:user, :external) }
+  let(:project_member) { create(:user) }
+
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  def visible_references(snippet_visibility, user = nil)
+    snippet = create(:project_snippet, snippet_visibility, project: project)
+    link['data-project'] = project.id.to_s
+    link['data-snippet'] = snippet.id.to_s
+
+    subject.nodes_visible_to_user(user, [link])
+  end
+
+  before do
+    project.add_user(project_member, :developer)
+  end
+
   describe '#nodes_visible_to_user' do
-    context 'when the link has a data-issue attribute' do
-      before { link['data-snippet'] = snippet.id.to_s }
+    context 'when a project is public and the snippets feature is enabled for everyone' do
+      before do
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
+      end
+
+      it 'creates a reference for guest for a public snippet' do
+        expect(visible_references(:public)).to eq([link])
+      end
+
+      it 'creates a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to eq([link])
+      end
+
+      it 'creates a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to eq([link])
+      end
+
+      it 'does not create a reference for an external user for an internal snippet' do
+        expect(visible_references(:internal, external_user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is public and the snippets feature is enabled for project team members' do
+      before do
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
+      end
+
+      it 'creates a reference for a project member for a public snippet' do
+        expect(visible_references(:public, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public, nil)).to be_empty
+      end
+
+      it 'does not create a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for an internal snippet' do
+        expect(visible_references(:internal, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is internal and the snippets feature is enabled for everyone' do
+      before do
+        project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL)
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::ENABLED)
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public)).to be_empty
+      end
+
+      it 'does not create a reference for an external user for a public snippet' do
+        expect(visible_references(:public, external_user)).to be_empty
+      end
 
-      it_behaves_like "referenced feature visibility", "snippets"
+      it 'creates a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to eq([link])
+      end
+
+      it 'creates a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to eq([link])
+      end
+
+      it 'does not create a reference for an external user for an internal snippet' do
+        expect(visible_references(:internal, external_user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is internal and the snippets feature is enabled for project team members' do
+      before do
+        project.update_attribute(:visibility, Gitlab::VisibilityLevel::INTERNAL)
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
+      end
+
+      it 'creates a reference for a project member for a public snippet' do
+        expect(visible_references(:public, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public, nil)).to be_empty
+      end
+
+      it 'does not create reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for an internal snippet' do
+        expect(visible_references(:internal, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
+    end
+
+    context 'when a project is private and the snippets feature is enabled for project team members' do
+      before do
+        project.update_attribute(:visibility, Gitlab::VisibilityLevel::PRIVATE)
+        project.project_feature.update_attribute(:snippets_access_level, ProjectFeature::PRIVATE)
+      end
+
+      it 'creates a reference for a project member for a public snippet' do
+        expect(visible_references(:public, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for guest for a public snippet' do
+        expect(visible_references(:public, nil)).to be_empty
+      end
+
+      it 'does not create a reference for a regular user for a public snippet' do
+        expect(visible_references(:public, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for an internal snippet' do
+        expect(visible_references(:internal, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for an internal snippet' do
+        expect(visible_references(:internal, user)).to be_empty
+      end
+
+      it 'creates a reference for a project member for a private snippet' do
+        expect(visible_references(:private, project_member)).to eq([link])
+      end
+
+      it 'does not create a reference for a regular user for a private snippet' do
+        expect(visible_references(:private, user)).to be_empty
+      end
     end
   end
 
   describe '#referenced_by' do
+    let(:snippet) { create(:snippet, project: project) }
     describe 'when the link has a data-snippet attribute' do
       context 'using an existing snippet ID' do
         it 'returns an Array of snippets' do
@@ -31,7 +210,7 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
         it 'returns an empty Array' do
           link['data-snippet'] = ''
 
-          expect(subject.referenced_by([link])).to eq([])
+          expect(subject.referenced_by([link])).to be_empty
         end
       end
     end
diff --git a/spec/lib/gitlab/uploads_transfer_spec.rb b/spec/lib/gitlab/uploads_transfer_spec.rb
new file mode 100644
index 00000000000..109559bb01c
--- /dev/null
+++ b/spec/lib/gitlab/uploads_transfer_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe Gitlab::UploadsTransfer do
+  it 'leaves avatar uploads where they are' do
+    project_with_avatar = create(:empty_project, :with_avatar)
+
+    described_class.new.rename_namespace('project', 'project-renamed')
+
+    expect(File.exist?(project_with_avatar.avatar.path)).to be_truthy
+  end
+end
diff --git a/spec/migrations/clean_upload_symlinks_spec.rb b/spec/migrations/clean_upload_symlinks_spec.rb
new file mode 100644
index 00000000000..cecb3ddac53
--- /dev/null
+++ b/spec/migrations/clean_upload_symlinks_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170406111121_clean_upload_symlinks.rb')
+
+describe CleanUploadSymlinks do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "tests", "move_uploads_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, 'user') }
+  let(:symlink_path) { File.join(uploads_dir, 'user') }
+
+  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
diff --git a/spec/migrations/move_uploads_to_system_dir_spec.rb b/spec/migrations/move_uploads_to_system_dir_spec.rb
new file mode 100644
index 00000000000..37d66452447
--- /dev/null
+++ b/spec/migrations/move_uploads_to_system_dir_spec.rb
@@ -0,0 +1,68 @@
+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170316163845_move_uploads_to_system_dir.rb")
+
+describe MoveUploadsToSystemDir do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "move_uploads_test") }
+  let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+  let(:new_uploads_dir) { File.join(uploads_dir, "system") }
+
+  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(File.join(uploads_dir, 'user'))
+      FileUtils.touch(File.join(uploads_dir, 'user', 'dummy.file'))
+    end
+
+    it 'moves the directory to the new path' do
+      expected_path = File.join(new_uploads_dir, 'user', 'dummy.file')
+
+      migration.up
+
+      expect(File.exist?(expected_path)).to be(true)
+    end
+
+    it 'creates a symlink in the old location' do
+      symlink_path = File.join(uploads_dir, 'user')
+      expected_path = File.join(symlink_path, 'dummy.file')
+
+      migration.up
+
+      expect(File.exist?(expected_path)).to be(true)
+      expect(File.symlink?(symlink_path)).to be(true)
+    end
+  end
+
+  describe "#down" do
+    before do
+      FileUtils.mkdir_p(File.join(new_uploads_dir, 'user'))
+      FileUtils.touch(File.join(new_uploads_dir, 'user', 'dummy.file'))
+    end
+
+    it 'moves the directory to the old path' do
+      expected_path = File.join(uploads_dir, 'user', 'dummy.file')
+
+      migration.down
+
+      expect(File.exist?(expected_path)).to be(true)
+    end
+
+    it 'removes the symlink if it existed' do
+      FileUtils.ln_s(File.join(new_uploads_dir, 'user'), File.join(uploads_dir, 'user'))
+
+      directory = File.join(uploads_dir, 'user')
+      expected_path = File.join(directory, 'dummy.file')
+
+      migration.down
+
+      expect(File.exist?(expected_path)).to be(true)
+      expect(File.symlink?(directory)).to be(false)
+    end
+  end
+end
diff --git a/spec/migrations/rename_system_namespaces_spec.rb b/spec/migrations/rename_system_namespaces_spec.rb
new file mode 100644
index 00000000000..ad1b83d8e2e
--- /dev/null
+++ b/spec/migrations/rename_system_namespaces_spec.rb
@@ -0,0 +1,252 @@
+require "spec_helper"
+require Rails.root.join("db", "migrate", "20170316163800_rename_system_namespaces.rb")
+
+describe RenameSystemNamespaces, truncate: true do
+  let(:migration) { described_class.new }
+  let(:test_dir) { File.join(Rails.root, "tmp", "tests", "rename_namespaces_test") }
+  let(:uploads_dir) { File.join(test_dir, "public", "uploads") }
+  let(:system_namespace) do
+    namespace = build(:namespace, path: "system")
+    namespace.save(validate: false)
+    namespace
+  end
+
+  def save_invalid_routable(routable)
+    routable.__send__(:prepare_route)
+    routable.save(validate: false)
+  end
+
+  before do
+    FileUtils.remove_dir(test_dir) if File.directory?(test_dir)
+    FileUtils.mkdir_p(uploads_dir)
+    FileUtils.remove_dir(TestEnv.repos_path) if File.directory?(TestEnv.repos_path)
+    allow(migration).to receive(:say)
+    allow(migration).to receive(:uploads_dir).and_return(uploads_dir)
+  end
+
+  describe "#system_namespace" do
+    it "only root namespaces called with path `system`" do
+      system_namespace
+      system_namespace_with_parent = build(:namespace, path: 'system', parent: create(:namespace))
+      system_namespace_with_parent.save(validate: false)
+
+      expect(migration.system_namespace.id).to eq(system_namespace.id)
+    end
+  end
+
+  describe "#up" do
+    before do
+      system_namespace
+    end
+
+    it "doesn't break if there are no namespaces called system" do
+      Namespace.delete_all
+
+      migration.up
+    end
+
+    it "renames namespaces called system" do
+      migration.up
+
+      expect(system_namespace.reload.path).to eq("system0")
+    end
+
+    it "renames the route to the namespace" do
+      migration.up
+
+      expect(system_namespace.reload.full_path).to eq("system0")
+    end
+
+    it "renames the route for projects of the namespace" do
+      project = build(:project, path: "project-path", namespace: system_namespace)
+      save_invalid_routable(project)
+
+      migration.up
+
+      expect(project.route.reload.path).to eq("system0/project-path")
+    end
+
+    it "doesn't touch routes of namespaces that look like system" do
+      namespace = create(:group, path: 'systemlookalike')
+      project = create(:project, namespace: namespace, path: 'the-project')
+
+      migration.up
+
+      expect(project.route.reload.path).to eq('systemlookalike/the-project')
+      expect(namespace.route.reload.path).to eq('systemlookalike')
+    end
+
+    it "moves the the repository for a project in the namespace" do
+      project = build(:project, namespace: system_namespace, path: "system-project")
+      save_invalid_routable(project)
+      TestEnv.copy_repo(project)
+      expected_repo = File.join(TestEnv.repos_path, "system0", "system-project.git")
+
+      migration.up
+
+      expect(File.directory?(expected_repo)).to be(true)
+    end
+
+    it "moves the uploads for the namespace" do
+      allow(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0")
+      expect(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0")
+
+      migration.up
+    end
+
+    it "moves the pages for the namespace" do
+      allow(migration).to receive(:move_namespace_folders).with(uploads_dir, "system", "system0")
+      expect(migration).to receive(:move_namespace_folders).with(Settings.pages.path, "system", "system0")
+
+      migration.up
+    end
+
+    describe "clears the markdown cache for projects in the system namespace" do
+      let!(:project) do
+        project = build(:project, namespace: system_namespace)
+        save_invalid_routable(project)
+        project
+      end
+
+      it 'removes description_html from projects' do
+        migration.up
+
+        expect(project.reload.description_html).to be_nil
+      end
+
+      it 'removes issue descriptions' do
+        issue = create(:issue, project: project, description_html: 'Issue description')
+
+        migration.up
+
+        expect(issue.reload.description_html).to be_nil
+      end
+
+      it 'removes merge request descriptions' do
+        merge_request = create(:merge_request,
+                               source_project: project,
+                               target_project: project,
+                               description_html: 'MergeRequest description')
+
+        migration.up
+
+        expect(merge_request.reload.description_html).to be_nil
+      end
+
+      it 'removes note html' do
+        note = create(:note,
+                      project: project,
+                      noteable: create(:issue, project: project),
+                      note_html: 'note description')
+
+        migration.up
+
+        expect(note.reload.note_html).to be_nil
+      end
+
+      it 'removes milestone description' do
+        milestone = create(:milestone,
+                           project: project,
+                           description_html: 'milestone description')
+
+        migration.up
+
+        expect(milestone.reload.description_html).to be_nil
+      end
+    end
+
+    context "system namespace -> subgroup -> system0 project" do
+      it "updates the route of the project correctly" do
+        subgroup = build(:group, path: "subgroup", parent: system_namespace)
+        save_invalid_routable(subgroup)
+        project = build(:project, path: "system0", namespace: subgroup)
+        save_invalid_routable(project)
+
+        migration.up
+
+        expect(project.route.reload.path).to eq("system0/subgroup/system0")
+      end
+    end
+  end
+
+  describe "#move_repositories" do
+    let(:namespace) { create(:group, name: "hello-group") }
+    it "moves a project for a namespace" do
+      create(:project, namespace: namespace, path: "hello-project")
+      expected_path = File.join(TestEnv.repos_path, "bye-group", "hello-project.git")
+
+      migration.move_repositories(namespace, "hello-group", "bye-group")
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it "moves a namespace in a subdirectory correctly" do
+      child_namespace = create(:group, name: "sub-group", parent: namespace)
+      create(:project, namespace: child_namespace, path: "hello-project")
+
+      expected_path = File.join(TestEnv.repos_path, "hello-group", "renamed-sub-group", "hello-project.git")
+
+      migration.move_repositories(child_namespace, "hello-group/sub-group", "hello-group/renamed-sub-group")
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+
+    it "moves a parent namespace with subdirectories" do
+      child_namespace = create(:group, name: "sub-group", parent: namespace)
+      create(:project, namespace: child_namespace, path: "hello-project")
+      expected_path = File.join(TestEnv.repos_path, "renamed-group", "sub-group", "hello-project.git")
+
+      migration.move_repositories(child_namespace, "hello-group", "renamed-group")
+
+      expect(File.directory?(expected_path)).to be(true)
+    end
+  end
+
+  describe "#move_namespace_folders" do
+    it "moves a namespace with files" do
+      source = File.join(uploads_dir, "parent-group", "sub-group")
+      FileUtils.mkdir_p(source)
+      destination = File.join(uploads_dir, "parent-group", "moved-group")
+      FileUtils.touch(File.join(source, "test.txt"))
+      expected_file = File.join(destination, "test.txt")
+
+      migration.move_namespace_folders(uploads_dir, File.join("parent-group", "sub-group"), File.join("parent-group", "moved-group"))
+
+      expect(File.exist?(expected_file)).to be(true)
+    end
+
+    it "moves a parent namespace uploads" do
+      source = File.join(uploads_dir, "parent-group", "sub-group")
+      FileUtils.mkdir_p(source)
+      destination = File.join(uploads_dir, "moved-parent", "sub-group")
+      FileUtils.touch(File.join(source, "test.txt"))
+      expected_file = File.join(destination, "test.txt")
+
+      migration.move_namespace_folders(uploads_dir, "parent-group", "moved-parent")
+
+      expect(File.exist?(expected_file)).to be(true)
+    end
+  end
+
+  describe "#child_ids_for_parent" do
+    it "collects child ids for all levels" do
+      parent = create(:namespace)
+      first_child = create(:namespace, parent: parent)
+      second_child = create(:namespace, parent: parent)
+      third_child = create(:namespace, parent: second_child)
+      all_ids = [parent.id, first_child.id, second_child.id, third_child.id]
+
+      collected_ids = migration.child_ids_for_parent(parent, ids: [parent.id])
+
+      expect(collected_ids).to contain_exactly(*all_ids)
+    end
+  end
+
+  describe "#remove_last_ocurrence" do
+    it "removes only the last occurance of a string" do
+      input = "this/is/system/namespace/with/system"
+
+      expect(migration.remove_last_occurrence(input, "system")).to eq("this/is/system/namespace/with/")
+    end
+  end
+end
diff --git a/spec/migrations/update_upload_paths_to_system_spec.rb b/spec/migrations/update_upload_paths_to_system_spec.rb
new file mode 100644
index 00000000000..7df44515424
--- /dev/null
+++ b/spec/migrations/update_upload_paths_to_system_spec.rb
@@ -0,0 +1,53 @@
+require "spec_helper"
+require Rails.root.join("db", "post_migrate", "20170317162059_update_upload_paths_to_system.rb")
+
+describe UpdateUploadPathsToSystem do
+  let(:migration) { described_class.new }
+
+  before do
+    allow(migration).to receive(:say)
+  end
+
+  describe "#uploads_to_switch_to_new_path" do
+    it "contains only uploads with the old path for the correct models" do
+      _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg")
+      _upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+      _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg")
+      old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+      group_upload = create(:upload, model: create(:group), path: "uploads/group/avatar.jpg")
+
+      expect(Upload.where(migration.uploads_to_switch_to_new_path)).to contain_exactly(old_upload, group_upload)
+    end
+  end
+
+  describe "#uploads_to_switch_to_old_path" do
+    it "contains only uploads with the new path for the correct models" do
+      _upload_for_other_type = create(:upload, model: create(:ci_pipeline), path: "uploads/ci_pipeline/avatar.jpg")
+      upload_with_system_path = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+      _upload_with_other_path = create(:upload, model: create(:empty_project), path: "thelongsecretforafileupload/avatar.jpg")
+      _old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+
+      expect(Upload.where(migration.uploads_to_switch_to_old_path)).to contain_exactly(upload_with_system_path)
+    end
+  end
+
+  describe "#up", truncate: true do
+    it "updates old upload records to the new path" do
+      old_upload = create(:upload, model: create(:empty_project), path: "uploads/project/avatar.jpg")
+
+      migration.up
+
+      expect(old_upload.reload.path).to eq("uploads/system/project/avatar.jpg")
+    end
+  end
+
+  describe "#down", truncate: true do
+    it "updates the new system patsh to the old paths" do
+      new_upload = create(:upload, model: create(:empty_project), path: "uploads/system/project/avatar.jpg")
+
+      migration.down
+
+      expect(new_upload.reload.path).to eq("uploads/project/avatar.jpg")
+    end
+  end
+end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 316bf153660..3d437ca0fcc 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -179,7 +179,7 @@ describe Group, models: true do
     let!(:group) { create(:group, :access_requestable, :with_avatar) }
     let(:user) { create(:user) }
     let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
-    let(:avatar_path) { "/uploads/group/avatar/#{group.id}/dk.png" }
+    let(:avatar_path) { "/uploads/system/group/avatar/#{group.id}/dk.png" }
 
     context 'when avatar file is uploaded' do
       before { group.add_master(user) }
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 0e74f1ab1bd..145c7ad5770 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -43,6 +43,12 @@ describe Namespace, models: true do
         end
       end
 
+      context "is case insensitive" do
+        let(:group) { build(:group, path: "System") }
+
+        it { expect(group).not_to be_valid }
+      end
+
       context 'top-level group' do
         let(:group) { build(:group, path: 'tree') }
 
@@ -178,8 +184,8 @@ describe Namespace, models: true do
       let(:parent) { create(:group, name: 'parent', path: 'parent') }
       let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
       let!(:project) { create(:project_empty_repo, path: 'the-project', namespace: child) }
-      let(:uploads_dir) { File.join(CarrierWave.root, 'uploads') }
-      let(:pages_dir) { TestEnv.pages_path }
+      let(:uploads_dir) { File.join(CarrierWave.root, FileUploader.base_dir) }
+      let(:pages_dir) { File.join(TestEnv.pages_path) }
 
       before do
         FileUtils.mkdir_p(File.join(uploads_dir, 'parent', 'child', 'the-project'))
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 3ed52d42f86..454eeb58ecd 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -812,7 +812,7 @@ describe Project, models: true do
 
     context 'when avatar file is uploaded' do
       let(:project) { create(:empty_project, :with_avatar) }
-      let(:avatar_path) { "/uploads/project/avatar/#{project.id}/dk.png" }
+      let(:avatar_path) { "/uploads/system/project/avatar/#{project.id}/dk.png" }
       let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
 
       it 'shows correct url' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a83726b48a0..d5bd9946ab6 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -987,7 +987,7 @@ describe User, models: true do
 
     context 'when avatar file is uploaded' do
       let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" }
-      let(:avatar_path) { "/uploads/user/avatar/#{user.id}/dk.png" }
+      let(:avatar_path) { "/uploads/system/user/avatar/#{user.id}/dk.png" }
 
       it 'shows correct avatar url' do
         expect(user.avatar_url).to eq(avatar_path)
diff --git a/spec/policies/project_snippet_policy_spec.rb b/spec/policies/project_snippet_policy_spec.rb
index e1771b636b8..ddbed5f781e 100644
--- a/spec/policies/project_snippet_policy_spec.rb
+++ b/spec/policies/project_snippet_policy_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe ProjectSnippetPolicy, models: true do
   let(:regular_user) { create(:user) }
   let(:external_user) { create(:user, :external) }
-  let(:project) { create(:empty_project) }
+  let(:project) { create(:empty_project, :public) }
 
   let(:author_permissions) do
     [
@@ -107,7 +107,7 @@ describe ProjectSnippetPolicy, models: true do
     end
 
     context 'snippet author' do
-      let(:snippet) { create(:project_snippet, :private, author: regular_user) }
+      let(:snippet) { create(:project_snippet, :private, author: regular_user, project: project) }
 
       subject { described_class.abilities(regular_user, snippet).to_set }
 
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb
index 05176c3beaa..6d1f0b24196 100644
--- a/spec/requests/openid_connect_spec.rb
+++ b/spec/requests/openid_connect_spec.rb
@@ -79,7 +79,7 @@ describe 'OpenID Connect requests' do
           'email_verified' => true,
           'website'        => 'https://example.com',
           'profile'        => 'http://localhost/alice',
-          'picture'        => "http://localhost/uploads/user/avatar/#{user.id}/dk.png"
+          'picture'        => "http://localhost/uploads/system/user/avatar/#{user.id}/dk.png"
         })
       end
     end
diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb
index 0657b7e93fe..d75851134ee 100644
--- a/spec/services/projects/participants_service_spec.rb
+++ b/spec/services/projects/participants_service_spec.rb
@@ -13,7 +13,7 @@ describe Projects::ParticipantsService, services: true do
         groups = participants.groups
 
         expect(groups.size).to eq 1
-        expect(groups.first[:avatar_url]).to eq("/uploads/group/avatar/#{group.id}/dk.png")
+        expect(groups.first[:avatar_url]).to eq("/uploads/system/group/avatar/#{group.id}/dk.png")
       end
 
       it 'should return an url for the avatar with relative url' do
@@ -24,7 +24,7 @@ describe Projects::ParticipantsService, services: true do
         groups = participants.groups
 
         expect(groups.size).to eq 1
-        expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/group/avatar/#{group.id}/dk.png")
+        expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/system/group/avatar/#{group.id}/dk.png")
       end
     end
   end
diff --git a/spec/uploaders/attachment_uploader_spec.rb b/spec/uploaders/attachment_uploader_spec.rb
index ea714fb08f0..d82dbe871d5 100644
--- a/spec/uploaders/attachment_uploader_spec.rb
+++ b/spec/uploaders/attachment_uploader_spec.rb
@@ -3,6 +3,17 @@ require 'spec_helper'
 describe AttachmentUploader do
   let(:uploader) { described_class.new(build_stubbed(:user)) }
 
+  describe "#store_dir" do
+    it "stores in the system dir" do
+      expect(uploader.store_dir).to start_with("uploads/system/user")
+    end
+
+    it "uses the old path when using object storage" do
+      expect(described_class).to receive(:file_storage?).and_return(false)
+      expect(uploader.store_dir).to start_with("uploads/user")
+    end
+  end
+
   describe '#move_to_cache' do
     it 'is true' do
       expect(uploader.move_to_cache).to eq(true)
diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb
index c4d558805ab..201fe6949aa 100644
--- a/spec/uploaders/avatar_uploader_spec.rb
+++ b/spec/uploaders/avatar_uploader_spec.rb
@@ -3,6 +3,17 @@ require 'spec_helper'
 describe AvatarUploader do
   let(:uploader) { described_class.new(build_stubbed(:user)) }
 
+  describe "#store_dir" do
+    it "stores in the system dir" do
+      expect(uploader.store_dir).to start_with("uploads/system/user")
+    end
+
+    it "uses the old path when using object storage" do
+      expect(described_class).to receive(:file_storage?).and_return(false)
+      expect(uploader.store_dir).to start_with("uploads/user")
+    end
+  end
+
   describe '#move_to_cache' do
     it 'is false' do
       expect(uploader.move_to_cache).to eq(false)
diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb
index d9113ef4095..47e9365e13d 100644
--- a/spec/uploaders/file_uploader_spec.rb
+++ b/spec/uploaders/file_uploader_spec.rb
@@ -15,6 +15,16 @@ describe FileUploader do
     end
   end
 
+  describe "#store_dir" do
+    it "stores in the namespace path" do
+      project = build_stubbed(:empty_project)
+      uploader = described_class.new(project)
+
+      expect(uploader.store_dir).to include(project.path_with_namespace)
+      expect(uploader.store_dir).not_to include("system")
+    end
+  end
+
   describe 'initialize' do
     it 'generates a secret if none is provided' do
       expect(SecureRandom).to receive(:hex).and_return('secret')