Merge branch '5750-backport-checksum-git-commanderror-exit-status-128' into 'master'

Raise InvalidRepository error for non-valid git repositories

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

5 files changed, 32 insertions, 6 deletions

diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 95fce8ca25f..01781720cd4 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.98.0
+0.99.0
diff --git a/changelogs/unreleased/5750-backport-checksum-git-commanderror-exit-status-128.yml b/changelogs/unreleased/5750-backport-checksum-git-commanderror-exit-status-128.yml
new file mode 100644
index 00000000000..d778b44c110
--- /dev/null
+++ b/changelogs/unreleased/5750-backport-checksum-git-commanderror-exit-status-128.yml
@@ -0,0 +1,6 @@
+---
+title: Raise NoRepository error for non-valid repositories when calculating repository
+  checksum
+merge_request: 18594
+author:
+type: fixed
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index bc61834ff7d..b145001a024 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -30,6 +30,7 @@ module Gitlab
       EMPTY_REPOSITORY_CHECKSUM = '0000000000000000000000000000000000000000'.freeze
 
       NoRepository = Class.new(StandardError)
+      InvalidRepository = Class.new(StandardError)
       InvalidBlobName = Class.new(StandardError)
       InvalidRef = Class.new(StandardError)
       GitError = Class.new(StandardError)
@@ -1584,7 +1585,7 @@ module Gitlab
 
       def checksum
         gitaly_migrate(:calculate_checksum,
-                      status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
+                       status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
           if is_enabled
             gitaly_repository_client.calculate_checksum
           else
@@ -2533,10 +2534,12 @@ module Gitlab
         output, status = run_git(args)
 
         if status.nil? || !status.zero?
-          # Empty repositories return with a non-zero status and an empty output.
-          return EMPTY_REPOSITORY_CHECKSUM if output&.empty?
+          # Non-valid git repositories return 128 as the status code and an error output
+          raise InvalidRepository if status == 128 && output.to_s.downcase =~ /not a git repository/
+          # Empty repositories returns with a non-zero status and an empty output.
+          raise ChecksumError, output unless output.blank?
 
-          raise ChecksumError, output
+          return EMPTY_REPOSITORY_CHECKSUM
         end
 
         refs = output.split("\n")
diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb
index 662b3d6cd0c..132a5947f17 100644
--- a/lib/gitlab/gitaly_client/repository_service.rb
+++ b/lib/gitlab/gitaly_client/repository_service.rb
@@ -292,6 +292,8 @@ module Gitlab
         request  = Gitaly::CalculateChecksumRequest.new(repository: @gitaly_repo)
         response = GitalyClient.call(@storage, :repository_service, :calculate_checksum, request)
         response.checksum.presence
+      rescue GRPC::DataLoss => e
+        raise Gitlab::Git::Repository::InvalidRepository.new(e)
       end
 
       def raw_changes_between(from, to)
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 9f091975959..cce84276fe3 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -2275,7 +2275,22 @@ describe Gitlab::Git::Repository, seed_helper: true do
         expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000'
       end
 
-      it 'raises a no repository exception when there is no repo' do
+      it 'raises Gitlab::Git::Repository::InvalidRepository error for non-valid git repo' do
+        FileUtils.rm_rf(File.join(storage_path, 'non-valid.git'))
+
+        system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --bare #{TEST_REPO_PATH} non-valid.git),
+               chdir: SEED_STORAGE_PATH,
+               out: '/dev/null',
+               err: '/dev/null')
+
+        File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0)
+
+        non_valid = described_class.new('default', 'non-valid.git', '')
+
+        expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository)
+      end
+
+      it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do
         broken_repo = described_class.new('default', 'a/path.git', '')
 
         expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository)