[go: up one dir, main page]

Skip to content

Go implementation of UserCommitFiles does not raise IndexError upon commit failure

From gitlab#322096 (closed):

In the Rugged implementation of UserCommitFiles, adding an invalid file (e.g. .git/hooks/pre-commit) would result in IndexError being caught and raised:

https://gitlab.com/gitlab-org/gitaly/blob/c1bf0cf2a6084c6969459c24bbac5a8a4649e018/ruby/lib/gitlab/git/index.rb#L137-138

However, in the Go implementation, this happens in the apply step, but the Git2Go index exception (https://pkg.go.dev/github.com/libgit2/git2go/v30#ErrClassIndex) isn't handled in any special way in https://gitlab.com/gitlab-org/gitaly/blob/83fc926987b86b692a0acacc2797b210fceb0c93/cmd/gitaly-git2go/commit/commit.go#L66.

We may want to add a git2go.IndexError and handle this in this error handling block: https://gitlab.com/gitlab-org/gitaly/blob/538d03d56cc1ad99a20693b03eaa1ef068c05dbc/internal/gitaly/service/operations/commit_files.go#L81-96

Here's the test that reproduces the problem:

diff --git a/internal/gitaly/service/operations/commit_files_test.go b/internal/gitaly/service/operations/commit_files_test.go
index e72151e9f..d2f7415ec 100644
--- a/internal/gitaly/service/operations/commit_files_test.go
+++ b/internal/gitaly/service/operations/commit_files_test.go
@@ -232,6 +232,18 @@ func testUserCommitFiles(t *testing.T, ctx context.Context) {
 				},
 			},
 		},
+		{
+			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 file without content",
 			steps: []step{

/cc: @samihiltunen

Edited by Stan Hu
To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information