Merge branch 'smh-fix-user-commit-files-index-error-handling' into 'master'

Fix UserCommitFiles index error handling

Closes #3461

See merge request gitlab-org/gitaly!3165

6 files changed, 32 insertions, 8 deletions

diff --git a/changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml b/changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml
new file mode 100644
index 000000000..bc0063740
--- /dev/null
+++ b/changelogs/unreleased/smh-fix-user-commit-files-index-error-handling.yml
@@ -0,0 +1,5 @@
+---
+title: Fix UserCommitFiles index error handling
+merge_request: 3165
+author:
+type: fixed
diff --git a/cmd/gitaly-git2go/commit/commit.go b/cmd/gitaly-git2go/commit/commit.go
index c8051c7a2..3c80452d2 100644
--- a/cmd/gitaly-git2go/commit/commit.go
+++ b/cmd/gitaly-git2go/commit/commit.go
@@ -64,6 +64,10 @@ func commit(ctx context.Context, params git2go.CommitParams) (string, error) {
 
 	for _, action := range params.Actions {
 		if err := apply(action, repo, index); err != nil {
+			if git.IsErrorClass(err, git.ErrClassIndex) {
+				err = git2go.IndexError(err.Error())
+			}
+
 			return "", fmt.Errorf("apply action %T: %w", action, err)
 		}
 	}
diff --git a/internal/git2go/commit.go b/internal/git2go/commit.go
index 2a9478609..558c82bd9 100644
--- a/internal/git2go/commit.go
+++ b/internal/git2go/commit.go
@@ -7,6 +7,12 @@ import (
 	"fmt"
 )
 
+// IndexError is an error that was produced by performing an invalid operation on the index.
+type IndexError string
+
+// Error returns the error message of the index error.
+func (err IndexError) Error() string { return string(err) }
+
 // InvalidArgumentError is returned when an invalid argument is provided.
 type InvalidArgumentError string
 
diff --git a/internal/git2go/gob.go b/internal/git2go/gob.go
index aa98b7964..4e760b437 100644
--- a/internal/git2go/gob.go
+++ b/internal/git2go/gob.go
@@ -30,6 +30,7 @@ var registeredTypes = map[interface{}]struct{}{
 	FileNotFoundError(""):    {},
 	InvalidArgumentError(""): {},
 	HasConflictsError{}:      {},
+	IndexError(""):           {},
 }
 
 // Result is the serialized result.
diff --git a/internal/gitaly/service/operations/commit_files.go b/internal/gitaly/service/operations/commit_files.go
index 2e8d70f29..8aa3791bf 100644
--- a/internal/gitaly/service/operations/commit_files.go
+++ b/internal/gitaly/service/operations/commit_files.go
@@ -27,10 +27,6 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-type indexError string
-
-func (err indexError) Error() string { return string(err) }
-
 func errorWithStderr(err error, stderr *bytes.Buffer) error {
 	return fmt.Errorf("%w, stderr: %q", err, stderr)
 }
@@ -74,7 +70,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile
 
 			var (
 				response        gitalypb.UserCommitFilesResponse
-				indexError      indexError
+				indexError      git2go.IndexError
 				preReceiveError preReceiveError
 			)
 
@@ -143,7 +139,7 @@ func (s *Server) UserCommitFiles(stream gitalypb.OperationService_UserCommitFile
 
 func validatePath(rootPath, relPath string) (string, error) {
 	if relPath == "" {
-		return "", indexError("You must provide a file path")
+		return "", git2go.IndexError("You must provide a file path")
 	} else if strings.Contains(relPath, "//") {
 		// This is a workaround to address a quirk in porting the RPC from Ruby to Go.
 		// GitLab's QA pipeline runs tests with filepath 'invalid://file/name/here'.
@@ -154,13 +150,13 @@ func validatePath(rootPath, relPath string) (string, error) {
 		//
 		// The Rails code expects to receive an error prefixed with 'invalid path', which is done
 		// here to retain compatibility.
-		return "", indexError(fmt.Sprintf("invalid path: '%s'", relPath))
+		return "", git2go.IndexError(fmt.Sprintf("invalid path: '%s'", relPath))
 	}
 
 	path, err := storage.ValidateRelativePath(rootPath, relPath)
 	if err != nil {
 		if errors.Is(err, storage.ErrRelativePathEscapesRoot) {
-			return "", indexError("Path cannot include directory traversal")
+			return "", git2go.IndexError("Path cannot include directory traversal")
 		}
 
 		return "", err
diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index e72151e9f..43c25aab9 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -103,6 +103,18 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 		steps []step
 	}{
 		{
+			desc: "create file with .git/hooks/pre-commit",
+			steps: []step{
+				{
+					actions: []*gitalypb.UserCommitFilesRequest{
+						createFileHeaderRequest(".git/hooks/pre-commit"),
+						actionContentRequest("content-1"),
+					},
+					indexError: "invalid path: '.git/hooks/pre-commit'",
+				},
+			},
+		},
+		{
 			desc: "create directory",
 			steps: []step{
 				{