From 419e5347cfa733f21909412e06181e7ac4a3bb37 Mon Sep 17 00:00:00 2001
From: syedsaifalialvi <saifalialvi@gmail.com>
Date: Sat, 6 Dec 2025 18:08:32 +0530
Subject: [PATCH] Handle 429 errors from POST /internal/allowed via HookService

When Hooks are executed by Git processes, not directly by Gitaly.
the error is being propagated via HookService.

When this happens, the message arrives back to Gitaly and
treated as a generic failure. When a 429 error occurs,
it gets written to stderr as a simple error message.
The original rate-limiting context is lost, so it still returns
INTERNAL status instead of RESOURCE_EXHAUSTED.

Proper error parsing is implemented in the pre_receive to
detect rate-limiting errors from stderr and proper error propagation
is done to return RESOURCE_EXHAUSTED instead of INTERNAL
---
 internal/gitaly/service/hook/pre_receive.go   |  7 +++
 .../gitaly/service/hook/pre_receive_test.go   | 52 +++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/internal/gitaly/service/hook/pre_receive.go b/internal/gitaly/service/hook/pre_receive.go
index 31a000861b..afaa96eb23 100644
--- a/internal/gitaly/service/hook/pre_receive.go
+++ b/internal/gitaly/service/hook/pre_receive.go
@@ -8,6 +8,7 @@ import (
 	"sync"
 
 	"gitlab.com/gitlab-org/gitaly/v18/internal/gitaly/storage"
+	"gitlab.com/gitlab-org/gitaly/v18/internal/gitlab/client"
 	"gitlab.com/gitlab-org/gitaly/v18/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v18/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v18/streamio"
@@ -46,6 +47,12 @@ func (s *server) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer
 		stdout,
 		stderr,
 	); err != nil {
+		// Check if this is a rate-limiting error that should be handled by statushandler middleware
+		var rateLimitErr client.RailsRateLimitedError
+		if errors.As(err, &rateLimitErr) {
+			return err
+		}
+
 		var exitError *exec.ExitError
 		if errors.As(err, &exitError) {
 			return preReceiveHookResponse(stream, int32(exitError.ExitCode()), "")
diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go
index b42ecbd708..c6397d73cc 100644
--- a/internal/gitaly/service/hook/pre_receive_test.go
+++ b/internal/gitaly/service/hook/pre_receive_test.go
@@ -24,6 +24,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v18/internal/gitaly/storage/storagemgr"
 	"gitlab.com/gitlab-org/gitaly/v18/internal/gitaly/transaction"
 	"gitlab.com/gitlab-org/gitaly/v18/internal/gitlab"
+	"gitlab.com/gitlab-org/gitaly/v18/internal/gitlab/client"
 	"gitlab.com/gitlab-org/gitaly/v18/internal/helper/text"
 	"gitlab.com/gitlab-org/gitaly/v18/internal/structerr"
 	"gitlab.com/gitlab-org/gitaly/v18/internal/testhelper"
@@ -32,7 +33,9 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v18/internal/transaction/txinfo"
 	"gitlab.com/gitlab-org/gitaly/v18/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v18/streamio"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 )
 
 func TestPreReceiveInvalidArgument(t *testing.T) {
@@ -348,6 +351,55 @@ func TestPreReceive_APIErrors(t *testing.T) {
 	}
 }
 
+// TestPreReceiveHook_RateLimitingError validates rate limited error via HookService
+// See issue here: https://gitlab.com/gitlab-org/gitaly/-/issues/6991
+func TestPreReceiveHook_RateLimitingError(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	cfg := testcfg.Build(t)
+
+	// Use mock client that returns RailsRateLimitedError
+	mockClient := gitlab.NewMockClient(
+		t,
+		func(context.Context, gitlab.AllowedParams) (bool, string, error) {
+			return false, "", client.RailsRateLimitedError{}
+		},
+		gitlab.MockPreReceive,
+		gitlab.MockPostReceive,
+	)
+
+	cfg.SocketPath = runHooksServer(t, cfg, nil, testserver.WithGitLabClient(mockClient))
+
+	repo, _ := gittest.CreateRepository(t, ctx, cfg)
+	hookClient, conn := newHooksClient(t, cfg.SocketPath)
+	defer conn.Close()
+
+	hooksPayload, err := gitcmd.NewHooksPayload(
+		ctx, cfg, repo, gittest.DefaultObjectHash, nil,
+		&gitcmd.UserDetails{
+			UserID:   "key-123",
+			Username: "username",
+			Protocol: "web",
+		},
+		gitcmd.PreReceiveHook, featureflag.FromContext(ctx), storage.ExtractTransactionID(ctx),
+	).Env()
+	require.NoError(t, err)
+
+	stream, err := hookClient.PreReceiveHook(ctx)
+	require.NoError(t, err)
+	require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{
+		Repository:           repo,
+		EnvironmentVariables: []string{hooksPayload},
+	}))
+	require.NoError(t, stream.Send(&gitalypb.PreReceiveHookRequest{Stdin: []byte("changes\n")}))
+	require.NoError(t, stream.CloseSend())
+
+	// EXPECTED BEHAVIOR: Should return gRPC RESOURCE_EXHAUSTED error
+	_, _, _, err = sendPreReceiveHookRequest(t, stream, &bytes.Buffer{})
+	testhelper.RequireGrpcError(t, status.Error(codes.ResourceExhausted, "GitLab: rate limited"), err)
+}
+
 // TestPreReceiveHook_HookErrorTooLarge validates that when the error message length
 // exceeds the maximum message size, it is chunked.
 // See issue here: https://gitlab.com/gitlab-org/gitaly/-/issues/5048
-- 
GitLab