Merge branch 'jc-better-bundle-error' into 'master'

Explicitly check for existing repository in CreateRepositoryFromBundle

Closes #1376

See merge request gitlab-org/gitaly!1980

3 files changed, 51 insertions, 5 deletions

diff --git a/changelogs/unreleased/jc-better-bundle-error.yml b/changelogs/unreleased/jc-better-bundle-error.yml
new file mode 100644
index 000000000..1af60d4c6
--- /dev/null
+++ b/changelogs/unreleased/jc-better-bundle-error.yml
@@ -0,0 +1,5 @@
+---
+title: Explicitly check for existing repository in CreateRepositoryFromBundle
+merge_request: 1980
+author:
+type: changed
diff --git a/internal/service/repository/create_from_bundle.go b/internal/service/repository/create_from_bundle.go
index 07f9da60a..60d4a54c8 100644
--- a/internal/service/repository/create_from_bundle.go
+++ b/internal/service/repository/create_from_bundle.go
@@ -1,6 +1,7 @@
 package repository
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -27,6 +28,15 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 		return status.Errorf(codes.InvalidArgument, "CreateRepositoryFromBundle: empty Repository")
 	}
 
+	repoPath, err := helper.GetPath(repo)
+	if err != nil {
+		return helper.ErrInternal(err)
+	}
+
+	if !isDirEmpty(repoPath) {
+		return helper.ErrPreconditionFailed(errors.New("CreateRepositoryFromBundle: target directory is non-empty"))
+	}
+
 	firstRead := false
 	reader := streamio.NewReader(func() ([]byte, error) {
 		if !firstRead {
@@ -59,11 +69,6 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 		return status.Error(codes.Internal, cleanError)
 	}
 
-	repoPath, err := helper.GetPath(repo)
-	if err != nil {
-		return err
-	}
-
 	cmd, err := git.SafeCmdWithoutRepo(ctx, nil,
 		git.SubCmd{
 			Name: "clone",
@@ -108,6 +113,17 @@ func (s *server) CreateRepositoryFromBundle(stream gitalypb.RepositoryService_Cr
 	return stream.SendAndClose(&gitalypb.CreateRepositoryFromBundleResponse{})
 }
 
+func isDirEmpty(path string) bool {
+	f, err := os.Open(path)
+	if os.IsNotExist(err) {
+		return true
+	}
+
+	_, err = f.Readdir(1)
+
+	return err == io.EOF
+}
+
 func sanitizedError(path, format string, a ...interface{}) string {
 	str := fmt.Sprintf(format, a...)
 	return strings.Replace(str, path, "[REPO PATH]", -1)
diff --git a/internal/service/repository/create_from_bundle_test.go b/internal/service/repository/create_from_bundle_test.go
index e6145757f..f866192f2 100644
--- a/internal/service/repository/create_from_bundle_test.go
+++ b/internal/service/repository/create_from_bundle_test.go
@@ -103,6 +103,31 @@ func TestFailedCreateRepositoryFromBundleRequestDueToValidations(t *testing.T) {
 	testhelper.RequireGrpcError(t, err, codes.InvalidArgument)
 }
 
+func TestFailedCreateRepositoryFromBundle_ExistingDirectory(t *testing.T) {
+	serverSocketPath, stop := runRepoServer(t)
+	defer stop()
+
+	testRepo, _, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	stream, err := client.CreateRepositoryFromBundle(ctx)
+	require.NoError(t, err)
+
+	require.NoError(t, stream.Send(&gitalypb.CreateRepositoryFromBundleRequest{
+		Repository: testRepo,
+	}))
+
+	_, err = stream.CloseAndRecv()
+	testhelper.RequireGrpcError(t, err, codes.FailedPrecondition)
+	testhelper.GrpcErrorHasMessage(err, "CreateRepositoryFromBundle: target directory is non-empty")
+}
+
 func TestSanitizedError(t *testing.T) {
 	testCases := []struct {
 		path     string