Merge branch 'pks-receive-pack-fsck-timezone' into 'master'

Allow commits with invalid timezones to be pushed

Closes gitlab#31583

See merge request gitlab-org/gitaly!1947

3 files changed, 67 insertions, 0 deletions

diff --git a/changelogs/unreleased/pks-receive-pack-fsck-timezone.yml b/changelogs/unreleased/pks-receive-pack-fsck-timezone.yml
new file mode 100644
index 000000000..bb0b4faa3
--- /dev/null
+++ b/changelogs/unreleased/pks-receive-pack-fsck-timezone.yml
@@ -0,0 +1,5 @@
+---
+title: Allow commits with invalid timezones to be pushed
+merge_request: 1947
+author:
+type: fixed
diff --git a/internal/git/receivepack.go b/internal/git/receivepack.go
index 5489ff9a5..7c09d96da 100644
--- a/internal/git/receivepack.go
+++ b/internal/git/receivepack.go
@@ -37,5 +37,13 @@ func ReceivePackConfig() []Option {
 		// Because Git itself will append the pool repository directory, the
 		// command ends with a "#". The end result is that Git runs `/bin/sh -c 'exit 0 # /path/to/pool.git`.
 		ValueFlag{"-c", "core.alternateRefsCommand=exit 0 #"},
+
+		// In the past, there was a bug in git that caused users to
+		// create commits with invalid timezones. As a result, some
+		// histories contain commits that do not match the spec. As we
+		// fsck received packfiles by default, any push containing such
+		// a commit will be rejected. As this is a mostly harmless
+		// issue, we add the following flag to ignore this check.
+		ValueFlag{"-c", "receive.fsck.badTimezone=ignore"},
 	}
 }
diff --git a/internal/service/smarthttp/receive_pack_test.go b/internal/service/smarthttp/receive_pack_test.go
index bd34823fe..bac6ed7ad 100644
--- a/internal/service/smarthttp/receive_pack_test.go
+++ b/internal/service/smarthttp/receive_pack_test.go
@@ -273,6 +273,60 @@ func TestFailedReceivePackRequestDueToValidationError(t *testing.T) {
 	}
 }
 
+func TestInvalidTimezone(t *testing.T) {
+	_, localRepoPath, localCleanup := testhelper.NewTestRepoWithWorktree(t)
+	defer localCleanup()
+
+	head := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "rev-parse", "HEAD"))
+	tree := text.ChompBytes(testhelper.MustRunCommand(t, nil, "git", "-C", localRepoPath, "rev-parse", "HEAD^{tree}"))
+
+	buf := new(bytes.Buffer)
+	buf.WriteString("tree " + tree + "\n")
+	buf.WriteString("parent " + head + "\n")
+	buf.WriteString("author Au Thor <author@example.com> 1313584730 +051800\n")
+	buf.WriteString("committer Au Thor <author@example.com> 1313584730 +051800\n")
+	buf.WriteString("\n")
+	buf.WriteString("Commit message\n")
+	commit := text.ChompBytes(testhelper.MustRunCommand(t, buf, "git", "-C", localRepoPath, "hash-object", "-t", "commit", "--stdin", "-w"))
+
+	stdin := strings.NewReader(fmt.Sprintf("^%s\n%s\n", head, commit))
+	pack := testhelper.MustRunCommand(t, stdin, "git", "-C", localRepoPath, "pack-objects", "--stdout", "--revs", "--thin", "--delta-base-offset", "-q")
+
+	pkt := fmt.Sprintf("%s %s refs/heads/master\x00 %s", head, commit, "report-status side-band-64k agent=git/2.12.0")
+	body := &bytes.Buffer{}
+	fmt.Fprintf(body, "%04x%s%s", len(pkt)+4, pkt, pktFlushStr)
+	body.Write(pack)
+
+	_, cleanup := testhelper.CaptureHookEnv(t)
+	defer cleanup()
+
+	server, socket := runSmartHTTPServer(t)
+	defer server.Stop()
+
+	repo, repoPath, cleanup := testhelper.NewTestRepo(t)
+	defer cleanup()
+
+	client, conn := newSmartHTTPClient(t, socket)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	stream, err := client.PostReceivePack(ctx)
+	require.NoError(t, err)
+	firstRequest := &gitalypb.PostReceivePackRequest{
+		Repository:       repo,
+		GlId:             "user-123",
+		GlRepository:     "project-456",
+		GitConfigOptions: []string{"receive.fsckObjects=true"},
+	}
+	response := doPush(t, stream, firstRequest, body)
+
+	expectedResponse := "0030\x01000eunpack ok\n0019ok refs/heads/master\n00000000"
+	require.Equal(t, expectedResponse, string(response), "Expected response to be %q, got %q", expectedResponse, response)
+	testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "show", commit)
+}
+
 func TestPostReceivePackToHooks(t *testing.T) {
 	secretToken := "secret token"
 	glRepository := "some_repo"