gitaly/hook: Fix detection of force-deletion-only changes with SHA256

The reference-transaction hook needs to be able to detect changes which
only consists of force deletions. To do so, it checks whether all
updates start with the force-deletion prefix of two zero object IDs.
This prefix is currently using the SHA1 object hash though, which breaks
compatibility with the SHA256 object format.

Use the new object format embedded in the hooks payload to look up the
actual object format used by the repository. Fix the tests to not use
hardcoded values, either.

2 files changed, 18 insertions, 12 deletions

diff --git a/internal/gitaly/hook/referencetransaction.go b/internal/gitaly/hook/referencetransaction.go
index a63ece62b..1ab5c9fd2 100644
--- a/internal/gitaly/hook/referencetransaction.go
+++ b/internal/gitaly/hook/referencetransaction.go
@@ -12,10 +12,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/transaction/voting"
 )
 
-// forceDeletionPrefix is the prefix of a queued reference transaction which deletes a
-// reference without checking its current value.
-var forceDeletionPrefix = fmt.Sprintf("%[1]s %[1]s ", git.ObjectHashSHA1.ZeroOID.String())
-
 //nolint:revive // This is unintentionally missing documentation.
 func (m *GitLabHookManager) ReferenceTransactionHook(ctx context.Context, state ReferenceTransactionState, env []string, stdin io.Reader) error {
 	payload, err := git.HooksPayloadFromEnv(env)
@@ -23,6 +19,11 @@ func (m *GitLabHookManager) ReferenceTransactionHook(ctx context.Context, state
 		return fmt.Errorf("extracting hooks payload: %w", err)
 	}
 
+	objectHash, err := git.ObjectHashByFormat(payload.ObjectFormat)
+	if err != nil {
+		return fmt.Errorf("looking up object hash: %w", err)
+	}
+
 	changes, err := io.ReadAll(stdin)
 	if err != nil {
 		return fmt.Errorf("reading stdin from request: %w", err)
@@ -62,7 +63,7 @@ func (m *GitLabHookManager) ReferenceTransactionHook(ctx context.Context, state
 	// The workaround is thus clear: we simply do not cast a vote on any reference transaction
 	// which consists only of force-deletions -- the vote will instead only happen on the loose
 	// backend transaction, which contains the full record of all refs which are to be updated.
-	if isForceDeletionsOnly(bytes.NewReader(changes)) {
+	if isForceDeletionsOnly(objectHash, bytes.NewReader(changes)) {
 		return nil
 	}
 
@@ -76,7 +77,11 @@ func (m *GitLabHookManager) ReferenceTransactionHook(ctx context.Context, state
 }
 
 // isForceDeletionsOnly determines whether the given changes only consist of force-deletions.
-func isForceDeletionsOnly(changes io.Reader) bool {
+func isForceDeletionsOnly(objectHash git.ObjectHash, changes io.Reader) bool {
+	// forceDeletionPrefix is the prefix of a queued reference transaction which deletes a
+	// reference without checking its current value.
+	forceDeletionPrefix := fmt.Sprintf("%[1]s %[1]s ", objectHash.ZeroOID)
+
 	scanner := bufio.NewScanner(changes)
 
 	for scanner.Scan() {
diff --git a/internal/gitaly/hook/transactions_test.go b/internal/gitaly/hook/transactions_test.go
index 47d7de567..ff12b7f31 100644
--- a/internal/gitaly/hook/transactions_test.go
+++ b/internal/gitaly/hook/transactions_test.go
@@ -62,7 +62,7 @@ func TestHookManager_stopCalled(t *testing.T) {
 		return hookManager.PreReceiveHook(ctx, repo, nil, []string{hooksPayload}, strings.NewReader("changes"), io.Discard, io.Discard)
 	}
 	updateFunc := func(t *testing.T) error {
-		return hookManager.UpdateHook(ctx, repo, "ref", git.ObjectHashSHA1.ZeroOID.String(), git.ObjectHashSHA1.ZeroOID.String(), []string{hooksPayload}, io.Discard, io.Discard)
+		return hookManager.UpdateHook(ctx, repo, "ref", gittest.DefaultObjectHash.ZeroOID.String(), gittest.DefaultObjectHash.ZeroOID.String(), []string{hooksPayload}, io.Discard, io.Discard)
 	}
 	postReceiveFunc := func(t *testing.T) error {
 		return hookManager.PostReceiveHook(ctx, repo, nil, []string{hooksPayload}, strings.NewReader("changes"), io.Discard, io.Discard)
@@ -127,9 +127,10 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) {
 	ctx, cancel := context.WithCancel(testhelper.Context(t))
 	cfg := testcfg.Build(t)
 
-	repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
+	repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{
 		SkipCreationViaService: true,
 	})
+	commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("master"))
 
 	mockTxMgr := transaction.MockManager{
 		VoteFn: func(ctx context.Context, _ txinfo.Transaction, _ voting.Vote, _ voting.Phase) error {
@@ -155,7 +156,7 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) {
 	).Env()
 	require.NoError(t, err)
 
-	changes := fmt.Sprintf("%s %s refs/heads/master", strings.Repeat("1", 40), git.ObjectHashSHA1.ZeroOID)
+	changes := fmt.Sprintf("%s %s refs/heads/master", commitID, gittest.DefaultObjectHash.ZeroOID)
 
 	cancel()
 
@@ -164,8 +165,8 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) {
 }
 
 func TestIsForceDeletionsOnly(t *testing.T) {
-	anyOID := strings.Repeat("1", 40)
-	zeroOID := git.ObjectHashSHA1.ZeroOID.String()
+	anyOID := strings.Repeat("1", gittest.DefaultObjectHash.EncodedLen())
+	zeroOID := gittest.DefaultObjectHash.ZeroOID.String()
 
 	forceDeletion := fmt.Sprintf("%s %s refs/heads/force-delete", zeroOID, zeroOID)
 	forceUpdate := fmt.Sprintf("%s %s refs/heads/force-update", zeroOID, anyOID)
@@ -208,7 +209,7 @@ func TestIsForceDeletionsOnly(t *testing.T) {
 		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
-			actual := isForceDeletionsOnly(strings.NewReader(tc.changes))
+			actual := isForceDeletionsOnly(gittest.DefaultObjectHash, strings.NewReader(tc.changes))
 			require.Equal(t, tc.expected, actual)
 		})
 	}