Merge branch 'gitaly-disk-access-4' into 'master'

Deny repository disk access in development and test

See merge request gitlab-org/gitlab-ce!19716

app/models/project_services/gemnasium_service.rb

@@ -43,13 +43,18 @@ class GemnasiumService < Service
   def execute(data)
     return unless supported_events.include?(data[:object_kind])
+    # Gitaly: this class will be removed https://gitlab.com/gitlab-org/gitlab-ee/issues/6010
+    repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      project.repository.path_to_repo
+    end
+
     Gemnasium::GitlabService.execute(
       ref: data[:ref],
       before: data[:before],
       after: data[:after],
       token: token,
       api_key: api_key,
-      repo: project.repository.path_to_repo # Gitaly: fixed by https://gitlab.com/gitlab-org/security-products/gemnasium-migration/issues/9
+      repo: repo_path
     )
   end
 end

db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb

@@ -12,7 +12,9 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
     end
     def repository_storage_path
-      Gitlab.config.repositories.storages[repository_storage].legacy_disk_path
+      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+        Gitlab.config.repositories.storages[repository_storage].legacy_disk_path
+      end
     end
     def repository_path

db/migrate/20161226122833_remove_dot_git_from_usernames.rb

@@ -64,7 +64,9 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration
     # we rename suffix instead of removing it
     path = path.sub(/\.git\z/, '_git')
-    check_routes(path.dup, 0, path)
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      check_routes(path.dup, 0, path)
+    end
   end
   def check_routes(base, counter, path)

lib/gitlab/gitaly_client.rb

@@ -33,11 +33,6 @@ module Gitlab
     MAXIMUM_GITALY_CALLS = 35
     CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze
-    # We have a mechanism to let GitLab automatically opt in to all Gitaly
-    # features. We want to be able to exclude some features from automatic
-    # opt-in. That is what EXPLICIT_OPT_IN_REQUIRED is for.
-    EXPLICIT_OPT_IN_REQUIRED = [Gitlab::GitalyClient::StorageSettings::DISK_ACCESS_DENIED_FLAG].freeze
-
     MUTEX = Mutex.new
     class << self
@@ -249,7 +244,7 @@ module Gitlab
       when MigrationStatus::OPT_OUT
         true
       when MigrationStatus::OPT_IN
-        opt_into_all_features? && !EXPLICIT_OPT_IN_REQUIRED.include?(feature_name)
+        opt_into_all_features? && !explicit_opt_in_required.include?(feature_name)
       else
         false
       end
@@ -259,6 +254,13 @@ module Gitlab
       false
     end
+    # We have a mechanism to let GitLab automatically opt in to all Gitaly
+    # features. We want to be able to exclude some features from automatic
+    # opt-in. This function has an override in EE.
+    def self.explicit_opt_in_required
+      []
+    end
+
     # opt_into_all_features? returns true when the current environment
     # is one in which we opt into features automatically
     def self.opt_into_all_features?

lib/system_check/orphans/repository_check.rb

@@ -5,16 +5,18 @@ module SystemCheck
       attr_accessor :orphans
       def multi_check
-        Gitlab.config.repositories.storages.each do |storage_name, repository_storage|
-          storage_path = repository_storage.legacy_disk_path
-          $stdout.puts
-          $stdout.puts "* Storage: #{storage_name} (#{storage_path})".color(:yellow)
-          repositories = disk_repositories(storage_path)
-          orphans = (repositories - fetch_repositories(storage_name))
-          print_orphans(orphans, storage_name)
+        Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          Gitlab.config.repositories.storages.each do |storage_name, repository_storage|
+            storage_path = repository_storage.legacy_disk_path
+            $stdout.puts
+            $stdout.puts "* Storage: #{storage_name} (#{storage_path})".color(:yellow)
+            repositories = disk_repositories(storage_path)
+            orphans = (repositories - fetch_repositories(storage_name))
+            print_orphans(orphans, storage_name)
+          end
         end
       end

spec/controllers/projects_controller_spec.rb

@@ -296,16 +296,22 @@ describe ProjectsController do
     shared_examples_for 'updating a project' do
       context 'when only renaming a project path' do
         it "sets the repository to the right path after a rename" do
-          original_repository_path = project.repository.path
+          original_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+            project.repository.path
+          end
           expect { update_project path: 'renamed_path' }
             .to change { project.reload.path }
           expect(project.path).to include 'renamed_path'
+          assign_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+            assigns(:repository).path
+          end
+
           if project.hashed_storage?(:repository)
-            expect(assigns(:repository).path).to eq(original_repository_path)
+            expect(assign_repository_path).to eq(original_repository_path)
           else
-            expect(assigns(:repository).path).to include(project.path)
+            expect(assign_repository_path).to include(project.path)
           end
           expect(response).to have_gitlab_http_status(302)

spec/helpers/projects_helper_spec.rb

@@ -280,7 +280,11 @@ describe ProjectsHelper do
   describe '#sanitizerepo_repo_path' do
     let(:project) { create(:project, :repository) }
-    let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path }
+    let(:storage_path) do
+      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+        Gitlab.config.repositories.storages.default.legacy_disk_path
+      end
+    end
     before do
       allow(Settings.shared).to receive(:[]).with('path').and_return('/base/repo/export/path')

spec/lib/gitlab/git_access_wiki_spec.rb

@@ -52,7 +52,9 @@ describe Gitlab::GitAccessWiki do
       context 'when the wiki repository does not exist' do
         it 'returns not found' do
           wiki_repo = project.wiki.repository
-          FileUtils.rm_rf(wiki_repo.path)
+          Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+            FileUtils.rm_rf(wiki_repo.path)
+          end
           # Sanity check for rm_rf
           expect(wiki_repo.exists?).to eq(false)

spec/migrations/migrate_process_commit_worker_jobs_spec.rb

@@ -6,7 +6,11 @@ require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_
 describe MigrateProcessCommitWorkerJobs do
   let(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
   let(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
-  let(:commit) { project.commit.raw.rugged_commit }
+  let(:commit) do
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      project.commit.raw.rugged_commit
+    end
+  end
   describe 'Project' do
     describe 'find_including_path' do

spec/migrations/turn_nested_groups_into_regular_groups_for_mysql_spec.rb

@@ -49,10 +49,14 @@ describe TurnNestedGroupsIntoRegularGroupsForMysql do
     end
     it 'renames the repository of any projects' do
-      expect(updated_project.repository.path)
+      repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+        updated_project.repository.path
+      end
+
+      expect(repo_path)
         .to end_with("#{parent_group.name}-#{child_group.name}/#{updated_project.path}.git")
-      expect(File.directory?(updated_project.repository.path)).to eq(true)
+      expect(File.directory?(repo_path)).to eq(true)
     end
     it 'creates a redirect route for renamed projects' do

spec/models/project_spec.rb

@@ -2943,7 +2943,7 @@ describe Project do
         project.rename_repo
-        expect(project.repository.rugged.config['gitlab.fullpath']).to eq(project.full_path)
+        expect(rugged_config['gitlab.fullpath']).to eq(project.full_path)
       end
     end
@@ -3104,7 +3104,7 @@ describe Project do
       it 'updates project full path in .git/config' do
         project.rename_repo
-        expect(project.repository.rugged.config['gitlab.fullpath']).to eq(project.full_path)
+        expect(rugged_config['gitlab.fullpath']).to eq(project.full_path)
       end
     end
@@ -3525,13 +3525,13 @@ describe Project do
     it 'writes full path in .git/config when key is missing' do
       project.write_repository_config
-      expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path
+      expect(rugged_config['gitlab.fullpath']).to eq project.full_path
     end
     it 'updates full path in .git/config when key is present' do
       project.write_repository_config(gl_full_path: 'old/path')
-      expect { project.write_repository_config }.to change { project.repository.rugged.config['gitlab.fullpath'] }.from('old/path').to(project.full_path)
+      expect { project.write_repository_config }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path)
     end
     it 'does not raise an error with an empty repository' do
@@ -3817,4 +3817,10 @@ describe Project do
       let(:uploader_class) { AttachmentUploader }
     end
   end
+
+  def rugged_config
+    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      project.repository.rugged.config
+    end
+  end
 end

spec/models/project_wiki_spec.rb

@@ -188,7 +188,11 @@ describe ProjectWiki do
       before do
         subject.wiki # Make sure the wiki repo exists
-        BareRepoOperations.new(subject.repository.path_to_repo).commit_file(image, 'image.png')
+        repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          subject.repository.path_to_repo
+        end
+
+        BareRepoOperations.new(repo_path).commit_file(image, 'image.png')
       end
       it 'returns the latest version of the file if it exists' do

spec/models/remote_mirror_spec.rb

@@ -74,7 +74,9 @@ describe RemoteMirror do
         mirror.update_attribute(:url, 'http://foo:baz@test.com')
-        config = repo.raw_repository.rugged.config
+        config = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          repo.raw_repository.rugged.config
+        end
         expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com')
       end

spec/requests/api/internal_spec.rb

@@ -522,7 +522,6 @@ describe API::Internal do
     context 'the project path was changed' do
       let(:project) { create(:project, :repository, :legacy_storage) }
-      let!(:old_path_to_repo) { project.repository.path_to_repo }
       let!(:repository) { project.repository }
       before do

spec/services/projects/create_service_spec.rb

@@ -272,8 +272,11 @@ describe Projects::CreateService, '#execute' do
   it 'writes project full path to .git/config' do
     project = create_project(user, opts)
-    expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path
+    rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      project.repository.rugged
+    end
+    expect(rugged.config['gitlab.fullpath']).to eq project.full_path
   end
   def create_project(user, opts)

spec/support/gitaly.rb

@@ -9,7 +9,7 @@ RSpec.configure do |config|
       # Use 'and_wrap_original' to make sure the arguments are valid
       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original do |m, *args|
         m.call(*args)
-        !Gitlab::GitalyClient::EXPLICIT_OPT_IN_REQUIRED.include?(args.first)
+        !Gitlab::GitalyClient.explicit_opt_in_required.include?(args.first)
       end
     end
   end

spec/workers/repository_remove_remote_worker_spec.rb

@@ -44,7 +44,9 @@ describe RepositoryRemoveRemoteWorker do
   end
   def create_remote_branch(remote_name, branch_name, target)
-    rugged = project.repository.rugged
+    rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+      project.repository.rugged
+    end
     rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id)
   end
 end