From bbee57a81a51d5f2aed0fddf27cf9c8294871c8e Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:40:53 +0200 Subject: [PATCH] fix(webhooks): scope manual webhook lockouts to repository and branch Failed manual webhook deliveries were counted per provider and source IP. On Coolify Cloud many customers share the Git provider IP, so one repository with a wrong secret locked out valid deliveries of all other repositories for 60 seconds. The failure key now also contains the repository and branch. Guessing the secret of one application stays limited to 30 tries per minute, and a lockout rejects all deliveries in that scope, also correct ones. A GitLab request without a token no longer counts as a failure. Co-Authored-By: Claude Opus 5.5 --- app/Http/Controllers/Webhook/Bitbucket.php | 12 +- .../MatchesManualWebhookApplications.php | 14 +- .../ThrottlesManualWebhookFailures.php | 34 ++-- app/Http/Controllers/Webhook/Gitea.php | 15 +- app/Http/Controllers/Webhook/Github.php | 35 ++-- app/Http/Controllers/Webhook/Gitlab.php | 19 +- .../AdvisorySecurityRegressionTest.php | 21 ++- tests/Feature/Webhook/WebhookHmacTest.php | 168 ++++++++++++++---- 8 files changed, 224 insertions(+), 94 deletions(-) diff --git a/app/Http/Controllers/Webhook/Bitbucket.php b/app/Http/Controllers/Webhook/Bitbucket.php index 7b252c5201..ced9a7a05c 100644 --- a/app/Http/Controllers/Webhook/Bitbucket.php +++ b/app/Http/Controllers/Webhook/Bitbucket.php @@ -20,10 +20,6 @@ class Bitbucket extends Controller public function manual(Request $request) { - if ($this->hasTooManyManualWebhookFailures($request, 'bitbucket')) { - return $this->tooManyManualWebhookFailuresResponse($request, 'bitbucket'); - } - try { $return_payloads = collect([]); $payload = $request->collect(); @@ -76,9 +72,13 @@ class Bitbucket extends Controller 'message' => 'Nothing to do. Invalid repository.', ]); } + $failure_key = $this->manualWebhookFailureRateLimitKey($request, 'bitbucket', $full_name, $branch); + if ($this->hasTooManyManualWebhookFailures($failure_key)) { + return $this->tooManyManualWebhookFailuresResponse($failure_key); + } $applications = $this->manualWebhookApplications(Application::query()->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'bitbucket'); + return $this->unauthenticatedManualWebhookResponse($failure_key); } foreach ($applications as $application) { $webhook_secret = data_get($application, 'manual_webhook_secret_bitbucket'); @@ -279,7 +279,7 @@ class Bitbucket extends Controller } } - return $this->manualWebhookResponse($return_payloads, $request, 'bitbucket'); + return $this->manualWebhookResponse($return_payloads, $failure_key); } catch (Exception $e) { return handleError($e); } diff --git a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php index 080b4a1725..b987da88c5 100644 --- a/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php +++ b/app/Http/Controllers/Webhook/Concerns/MatchesManualWebhookApplications.php @@ -4,7 +4,6 @@ namespace App\Http\Controllers\Webhook\Concerns; use App\Models\Application; use Illuminate\Database\Eloquent\Builder; -use Illuminate\Http\Request; use Illuminate\Http\Response; use Illuminate\Support\Collection; @@ -68,20 +67,25 @@ trait MatchesManualWebhookApplications /** * Respond to a delivery that could not be authenticated (no matching * application or no signature) and count it as a failed attempt. + * + * Deliveries without a matching application are counted too: the failure + * key is scoped to the repository and branch, so this cannot lock out other + * applications, and it keeps the 429 response from revealing which + * repositories exist in this instance. */ - protected function unauthenticatedManualWebhookResponse(Request $request, string $provider): Response + protected function unauthenticatedManualWebhookResponse(string $failureKey): Response { - $this->recordManualWebhookFailure($request, $provider); + $this->recordManualWebhookFailure($failureKey); return response([$this->unauthenticatedManualWebhookFailurePayload()]); } - protected function manualWebhookResponse(Collection $payloads, Request $request, string $provider): Response + protected function manualWebhookResponse(Collection $payloads, string $failureKey): Response { $failure = $this->unauthenticatedManualWebhookFailurePayload(); $authorizedPayloads = $payloads->reject(fn (array $payload): bool => $payload === $failure)->values(); if ($authorizedPayloads->isEmpty() && $payloads->isNotEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, $provider); + return $this->unauthenticatedManualWebhookResponse($failureKey); } return response($authorizedPayloads); diff --git a/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php b/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php index fe2832eb1a..e86684b9f6 100644 --- a/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php +++ b/app/Http/Controllers/Webhook/Concerns/ThrottlesManualWebhookFailures.php @@ -9,9 +9,12 @@ use Illuminate\Support\Facades\RateLimiter; /** * Throttles manual webhook deliveries that fail authentication. * - * Only failed authentication attempts are counted, per provider and client IP, - * so valid signed deliveries from shared IPs (Cloudflare, Git host IP pools) - * are never dropped because of unrelated traffic. + * Failures are counted per provider, client IP, repository and branch filter. + * The repository and branch come from the payload and select the applications + * whose secrets are checked, so a guesser can only exhaust the bucket of the + * applications it targets. Git hosts deliver from shared egress IPs; one + * misconfigured repository (wrong secret, deleted application, untracked + * branch) therefore cannot lock out deliveries for other repositories. */ trait ThrottlesManualWebhookFailures { @@ -19,19 +22,28 @@ trait ThrottlesManualWebhookFailures protected const int MANUAL_WEBHOOK_FAILURE_DECAY_SECONDS = 60; - protected function manualWebhookFailureRateLimitKey(Request $request, string $provider): string + /** + * @param string $fullName Canonical repository path from manualWebhookRepositoryFullName(). + * @param mixed $branch Branch used to select applications, or null when all branches match. + */ + protected function manualWebhookFailureRateLimitKey(Request $request, string $provider, string $fullName, mixed $branch): string { - return "manual-webhook-failures:{$provider}:".auth_rate_limit_ip($request); + $scope = json_encode([ + mb_strtolower($fullName), + is_scalar($branch) ? (string) $branch : null, + ]); + + return "manual-webhook-failures:{$provider}:".auth_rate_limit_ip($request).':'.hash('sha256', (string) $scope); } - protected function hasTooManyManualWebhookFailures(Request $request, string $provider): bool + protected function hasTooManyManualWebhookFailures(string $failureKey): bool { - return RateLimiter::tooManyAttempts($this->manualWebhookFailureRateLimitKey($request, $provider), self::MANUAL_WEBHOOK_MAX_FAILURES); + return RateLimiter::tooManyAttempts($failureKey, self::MANUAL_WEBHOOK_MAX_FAILURES); } - protected function tooManyManualWebhookFailuresResponse(Request $request, string $provider): Response + protected function tooManyManualWebhookFailuresResponse(string $failureKey): Response { - $retryAfter = RateLimiter::availableIn($this->manualWebhookFailureRateLimitKey($request, $provider)); + $retryAfter = RateLimiter::availableIn($failureKey); return response([ 'status' => 'failed', @@ -39,8 +51,8 @@ trait ThrottlesManualWebhookFailures ], 429)->header('Retry-After', (string) max($retryAfter, 1)); } - protected function recordManualWebhookFailure(Request $request, string $provider): void + protected function recordManualWebhookFailure(string $failureKey): void { - RateLimiter::hit($this->manualWebhookFailureRateLimitKey($request, $provider), self::MANUAL_WEBHOOK_FAILURE_DECAY_SECONDS); + RateLimiter::hit($failureKey, self::MANUAL_WEBHOOK_FAILURE_DECAY_SECONDS); } } diff --git a/app/Http/Controllers/Webhook/Gitea.php b/app/Http/Controllers/Webhook/Gitea.php index 975e990973..40f015b41d 100644 --- a/app/Http/Controllers/Webhook/Gitea.php +++ b/app/Http/Controllers/Webhook/Gitea.php @@ -21,10 +21,6 @@ class Gitea extends Controller public function manual(Request $request) { - if ($this->hasTooManyManualWebhookFailures($request, 'gitea')) { - return $this->tooManyManualWebhookFailuresResponse($request, 'gitea'); - } - try { $return_payloads = collect([]); $x_gitea_delivery = request()->header('X-Gitea-Delivery'); @@ -69,17 +65,22 @@ class Gitea extends Controller if ($full_name === null) { return response('Nothing to do. Invalid repository.'); } + $matched_branch = $x_gitea_event === 'pull_request' ? $base_branch : $branch; + $failure_key = $this->manualWebhookFailureRateLimitKey($request, 'gitea', $full_name, $matched_branch); + if ($this->hasTooManyManualWebhookFailures($failure_key)) { + return $this->tooManyManualWebhookFailuresResponse($failure_key); + } $applications = Application::query(); if ($x_gitea_event === 'push') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'gitea'); + return $this->unauthenticatedManualWebhookResponse($failure_key); } } if ($x_gitea_event === 'pull_request') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $base_branch), $full_name); if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'gitea'); + return $this->unauthenticatedManualWebhookResponse($failure_key); } } foreach ($applications as $application) { @@ -285,7 +286,7 @@ class Gitea extends Controller } } - return $this->manualWebhookResponse($return_payloads, $request, 'gitea'); + return $this->manualWebhookResponse($return_payloads, $failure_key); } catch (Exception $e) { return handleError($e); } diff --git a/app/Http/Controllers/Webhook/Github.php b/app/Http/Controllers/Webhook/Github.php index e00c9cf4a4..fb85b148ef 100644 --- a/app/Http/Controllers/Webhook/Github.php +++ b/app/Http/Controllers/Webhook/Github.php @@ -25,10 +25,6 @@ class Github extends Controller public function manual(Request $request) { - if ($this->hasTooManyManualWebhookFailures($request, 'github')) { - return $this->tooManyManualWebhookFailuresResponse($request, 'github'); - } - try { $return_payloads = collect([]); $x_github_delivery = request()->header('X-GitHub-Delivery'); @@ -79,21 +75,22 @@ class Github extends Controller if ($full_name === null) { return response('Nothing to do. Invalid repository.'); } - $applications = Application::query(); - if ($x_github_event === 'push') { - $applications = $this->manualWebhookApplications($applications->where('git_branch', $branch), $full_name); - if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'github'); - } + $matched_branch = match (true) { + $x_github_event === 'push' => $branch, + $action === 'closed' => null, + default => $base_branch, + }; + $failure_key = $this->manualWebhookFailureRateLimitKey($request, 'github', $full_name, $matched_branch); + if ($this->hasTooManyManualWebhookFailures($failure_key)) { + return $this->tooManyManualWebhookFailuresResponse($failure_key); } - if ($x_github_event === 'pull_request') { - if ($action !== 'closed') { - $applications->where('git_branch', $base_branch); - } - $applications = $this->manualWebhookApplications($applications, $full_name); - if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'github'); - } + $applications = Application::query(); + if ($x_github_event === 'push' || $action !== 'closed') { + $applications->where('git_branch', $matched_branch); + } + $applications = $this->manualWebhookApplications($applications, $full_name); + if ($applications->isEmpty()) { + return $this->unauthenticatedManualWebhookResponse($failure_key); } $applicationsByServer = $applications->groupBy(function ($app) { return $app->destination->server_id; @@ -243,7 +240,7 @@ class Github extends Controller } } - return $this->manualWebhookResponse($return_payloads, $request, 'github'); + return $this->manualWebhookResponse($return_payloads, $failure_key); } catch (Exception $e) { return handleError($e); } diff --git a/app/Http/Controllers/Webhook/Gitlab.php b/app/Http/Controllers/Webhook/Gitlab.php index e95a98b230..2eec721bb9 100644 --- a/app/Http/Controllers/Webhook/Gitlab.php +++ b/app/Http/Controllers/Webhook/Gitlab.php @@ -336,10 +336,6 @@ class Gitlab extends Controller public function manual(Request $request) { - if ($this->hasTooManyManualWebhookFailures($request, 'gitlab')) { - return $this->tooManyManualWebhookFailuresResponse($request, 'gitlab'); - } - try { $return_payloads = collect([]); $payload = $request->collect(); @@ -356,12 +352,14 @@ class Gitlab extends Controller return response($return_payloads); } + // A delivery without a token does not try a secret, so it is not + // counted as a failed attempt. if (empty($x_gitlab_token)) { auditLogWebhookFailure('gitlab', 'webhook_token_missing', [ 'event' => $x_gitlab_event, ]); - return $this->unauthenticatedManualWebhookResponse($request, 'gitlab'); + return response([$this->unauthenticatedManualWebhookFailurePayload()]); } if ($x_gitlab_event === 'push') { @@ -412,17 +410,22 @@ class Gitlab extends Controller return response($return_payloads); } + $matched_branch = $x_gitlab_event === 'merge_request' ? $base_branch : $branch; + $failure_key = $this->manualWebhookFailureRateLimitKey($request, 'gitlab', $full_name, $matched_branch); + if ($this->hasTooManyManualWebhookFailures($failure_key)) { + return $this->tooManyManualWebhookFailuresResponse($failure_key); + } $applications = Application::query(); if ($x_gitlab_event === 'push') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $branch), $full_name); if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'gitlab'); + return $this->unauthenticatedManualWebhookResponse($failure_key); } } if ($x_gitlab_event === 'merge_request') { $applications = $this->manualWebhookApplications($applications->where('git_branch', $base_branch), $full_name); if ($applications->isEmpty()) { - return $this->unauthenticatedManualWebhookResponse($request, 'gitlab'); + return $this->unauthenticatedManualWebhookResponse($failure_key); } } foreach ($applications as $application) { @@ -631,7 +634,7 @@ class Gitlab extends Controller } } - return $this->manualWebhookResponse($return_payloads, $request, 'gitlab'); + return $this->manualWebhookResponse($return_payloads, $failure_key); } catch (Exception $e) { return handleError($e); } diff --git a/tests/Feature/AdvisorySecurityRegressionTest.php b/tests/Feature/AdvisorySecurityRegressionTest.php index 62af383364..c9d9469bb7 100644 --- a/tests/Feature/AdvisorySecurityRegressionTest.php +++ b/tests/Feature/AdvisorySecurityRegressionTest.php @@ -67,19 +67,26 @@ it('throttles only failed authentication on manual webhook routes', function (st { use MatchesManualWebhookApplications; - public function reply(array $payloads, Request $request, string $provider): int + public function key(Request $request, string $provider): string { - return $this->manualWebhookResponse(collect($payloads), $request, $provider)->getStatusCode(); + return $this->manualWebhookFailureRateLimitKey($request, $provider, 'test-org/test-repo', 'main'); + } + + public function reply(array $payloads, string $failureKey): int + { + return $this->manualWebhookResponse(collect($payloads), $failureKey)->getStatusCode(); } }; + $failureKey = $helper->key($request, $provider); expect($route->gatherMiddleware())->not->toContain('throttle:60,1'); + expect($failureKey)->toStartWith("manual-webhook-failures:{$provider}:192.0.2.44:"); - $helper->reply([['status' => 'success', 'message' => 'queued']], $request, $provider); - expect(RateLimiter::attempts("manual-webhook-failures:{$provider}:192.0.2.44"))->toBe(0); + $helper->reply([['status' => 'success', 'message' => 'queued']], $failureKey); + expect(RateLimiter::attempts($failureKey))->toBe(0); - $helper->reply([['status' => 'failed', 'message' => 'Invalid signature.']], $request, $provider); - expect(RateLimiter::attempts("manual-webhook-failures:{$provider}:192.0.2.44"))->toBe(1); + $helper->reply([['status' => 'failed', 'message' => 'Invalid signature.']], $failureKey); + expect(RateLimiter::attempts($failureKey))->toBe(1); })->with(['github', 'gitlab', 'bitbucket', 'gitea']); it('does not reveal how many applications share a manual webhook repository', function () { @@ -89,7 +96,7 @@ it('does not reveal how many applications share a manual webhook repository', fu public function reply(array $payloads): string { - return $this->manualWebhookResponse(collect($payloads), Request::create('/webhooks/source/github/events/manual', 'POST'), 'github')->getContent(); + return $this->manualWebhookResponse(collect($payloads), 'manual-webhook-failures:test')->getContent(); } }; $failure = ['status' => 'failed', 'message' => 'Invalid signature.']; diff --git a/tests/Feature/Webhook/WebhookHmacTest.php b/tests/Feature/Webhook/WebhookHmacTest.php index 842fc00266..557569f31d 100644 --- a/tests/Feature/Webhook/WebhookHmacTest.php +++ b/tests/Feature/Webhook/WebhookHmacTest.php @@ -1,6 +1,8 @@ match(Request::create("/webhooks/source/{$provider}/events/manual", 'POST')); expect(collect($route->gatherMiddleware())->filter(fn (mixed $middleware): bool => is_string($middleware) && str_starts_with($middleware, 'throttle')))->toBeEmpty(); })->with(['github', 'gitlab', 'bitbucket', 'gitea']); -function sendManualWebhookPush(TestCase $test, string $provider, Application $application, bool $validSignature = true, string $ip = '203.0.113.10', string $repository = 'test-org/test-repo'): TestResponse +function sendManualWebhookPush(TestCase $test, string $provider, Application $application, bool $validSignature = true, string $ip = '203.0.113.10', string $repository = 'test-org/test-repo', string $branch = 'main'): TestResponse { $secret = $validSignature ? $application->{"manual_webhook_secret_{$provider}"} : 'wrong-secret'; $server = ['REMOTE_ADDR' => $ip, 'CONTENT_TYPE' => 'application/json']; @@ -30,7 +39,7 @@ function sendManualWebhookPush(TestCase $test, string $provider, Application $ap if ($provider === 'gitlab') { $payload = json_encode([ 'object_kind' => 'push', - 'ref' => 'refs/heads/main', + 'ref' => "refs/heads/{$branch}", 'project' => ['path_with_namespace' => $repository], 'after' => 'abc123', 'commits' => [], @@ -43,7 +52,7 @@ function sendManualWebhookPush(TestCase $test, string $provider, Application $ap if ($provider === 'bitbucket') { $payload = json_encode([ - 'push' => ['changes' => [['new' => ['name' => 'main', 'target' => ['hash' => 'abc123']]]]], + 'push' => ['changes' => [['new' => ['name' => $branch, 'target' => ['hash' => 'abc123']]]]], 'repository' => ['full_name' => $repository], ]); @@ -54,7 +63,7 @@ function sendManualWebhookPush(TestCase $test, string $provider, Application $ap } $payload = json_encode([ - 'ref' => 'refs/heads/main', + 'ref' => "refs/heads/{$branch}", 'repository' => ['full_name' => $repository], 'after' => 'abc123', 'commits' => [], @@ -67,6 +76,46 @@ function sendManualWebhookPush(TestCase $test, string $provider, Application $ap ], $payload); } +function manualWebhookFailureKey(string $provider, string $ip = '203.0.113.10', string $repository = 'test-org/test-repo', ?string $branch = 'main'): string +{ + $helper = new class + { + use MatchesManualWebhookApplications; + + public function key(Request $request, string $provider, string $repository, ?string $branch): string + { + return $this->manualWebhookFailureRateLimitKey($request, $provider, $this->manualWebhookRepositoryFullName($repository), $branch); + } + }; + + return $helper->key(Request::create('/', 'POST', server: ['REMOTE_ADDR' => $ip]), $provider, $repository, $branch); +} + +function lockOutManualWebhookRepository(TestCase $test, string $provider, Application $application, string $repository = 'test-org/test-repo', string $branch = 'main'): void +{ + for ($i = 0; $i < 30; $i++) { + $response = sendManualWebhookPush($test, $provider, $application, validSignature: false, repository: $repository, branch: $branch); + + $response->assertOk(); + expect($response->getContent())->toContain('Invalid signature'); + } + + sendManualWebhookPush($test, $provider, $application, validSignature: false, repository: $repository, branch: $branch) + ->assertStatus(429) + ->assertHeader('Retry-After'); +} + +function makeWebhookApplicationServerFunctional(Application $application): Application +{ + $application->destination->server->settings->update([ + 'is_reachable' => true, + 'is_usable' => true, + 'force_disabled' => false, + ]); + + return $application->refresh(); +} + describe('Manual Webhook Failed Authentication Rate Limiting', function () { test('valid signed deliveries are never throttled', function (string $provider) { $application = createApplicationWithWebhook(); @@ -78,24 +127,91 @@ describe('Manual Webhook Failed Authentication Rate Limiting', function () { expect($response->getContent())->not->toContain('Invalid signature'); } - expect(RateLimiter::attempts("manual-webhook-failures:{$provider}:203.0.113.10"))->toBe(0); + expect(RateLimiter::attempts(manualWebhookFailureKey($provider)))->toBe(0); })->with(['github', 'gitlab', 'bitbucket', 'gitea']); test('repeated invalid signatures are throttled after 30 failures', function (string $provider) { $application = createApplicationWithWebhook(); + lockOutManualWebhookRepository($this, $provider, $application); + + expect(RateLimiter::tooManyAttempts(manualWebhookFailureKey($provider), 30))->toBeTrue(); + })->with(['github', 'gitlab', 'bitbucket', 'gitea']); + + test('a locked out repository rejects every delivery for that repository and branch before verification', function (string $provider) { + Queue::fake(); + $application = makeWebhookApplicationServerFunctional(createApplicationWithWebhook()); + + lockOutManualWebhookRepository($this, $provider, $application); + + sendManualWebhookPush($this, $provider, $application, validSignature: false)->assertStatus(429); + // A correct guess during the lockout must not succeed, otherwise the + // lockout does not limit how fast a secret can be guessed. + sendManualWebhookPush($this, $provider, $application)->assertStatus(429); + sendManualWebhookPush($this, $provider, $application, validSignature: false, repository: 'TEST-ORG/Test-Repo.git')->assertStatus(429); + + expect(ApplicationDeploymentQueue::query()->where('application_id', $application->id)->exists())->toBeFalse(); + })->with(['github', 'gitlab', 'bitbucket', 'gitea']); + + test('a wrong secret for one repository does not block a signed delivery for another repository from the same IP', function (string $provider) { + Queue::fake(); + $misconfigured = createApplicationWithWebhook(repo: 'other-tenant/misconfigured-repo', overrides: ['name' => 'misconfigured-app']); + $application = makeWebhookApplicationServerFunctional(createApplicationWithWebhook()); + + lockOutManualWebhookRepository($this, $provider, $misconfigured, repository: 'other-tenant/misconfigured-repo'); + + $response = sendManualWebhookPush($this, $provider, $application); + + $response->assertOk(); + expect($response->getContent())->toContain('Deployment queued'); + expect(ApplicationDeploymentQueue::query()->where('application_id', $application->id)->exists())->toBeTrue(); + + sendManualWebhookPush($this, $provider, $misconfigured, validSignature: false, repository: 'other-tenant/misconfigured-repo')->assertStatus(429); + })->with(['github', 'gitlab', 'bitbucket', 'gitea']); + + test('deliveries for unknown repositories do not block signed deliveries for known repositories', function (string $provider) { + Queue::fake(); + $application = makeWebhookApplicationServerFunctional(createApplicationWithWebhook()); + + for ($i = 0; $i < 40; $i++) { + $response = sendManualWebhookPush($this, $provider, $application, validSignature: false, repository: 'deleted-org/deleted-repo'); + + expect($response->getStatusCode())->toBeIn([200, 429]); + } + + $response = sendManualWebhookPush($this, $provider, $application); + + $response->assertOk(); + expect($response->getContent())->toContain('Deployment queued'); + expect(RateLimiter::attempts(manualWebhookFailureKey($provider)))->toBe(0); + })->with(['github', 'gitlab', 'bitbucket', 'gitea']); + + test('pushes to untracked branches do not block the deployed branch', function (string $provider) { + Queue::fake(); + $application = makeWebhookApplicationServerFunctional(createApplicationWithWebhook()); + + for ($i = 0; $i < 40; $i++) { + sendManualWebhookPush($this, $provider, $application, branch: 'feature/untracked'); + } + + $response = sendManualWebhookPush($this, $provider, $application); + + $response->assertOk(); + expect($response->getContent())->toContain('Deployment queued'); + })->with(['github', 'gitlab', 'bitbucket', 'gitea']); + + test('unknown repositories respond like invalid signatures and are still throttled', function () { + $application = createApplicationWithWebhook(); + for ($i = 0; $i < 30; $i++) { - $response = sendManualWebhookPush($this, $provider, $application, validSignature: false); + $response = sendManualWebhookPush($this, 'github', $application, repository: 'unknown-org/unknown-repo'); $response->assertOk(); expect($response->getContent())->toContain('Invalid signature'); } - $response = sendManualWebhookPush($this, $provider, $application, validSignature: false); - - $response->assertStatus(429); - $response->assertHeader('Retry-After'); - })->with(['github', 'gitlab', 'bitbucket', 'gitea']); + sendManualWebhookPush($this, 'github', $application, repository: 'unknown-org/unknown-repo')->assertStatus(429); + }); test('valid deliveries do not count when another matching application has a different secret', function () { $application = createApplicationWithWebhook(); @@ -108,31 +224,24 @@ describe('Manual Webhook Failed Authentication Rate Limiting', function () { expect($response->getContent())->not->toContain('Invalid signature'); } - expect(RateLimiter::attempts('manual-webhook-failures:github:203.0.113.10'))->toBe(0); + expect(RateLimiter::attempts(manualWebhookFailureKey('github')))->toBe(0); }); - test('deliveries for unknown repositories count as failed authentication', function () { - $application = createApplicationWithWebhook(); - - for ($i = 0; $i < 30; $i++) { - sendManualWebhookPush($this, 'github', $application, repository: 'unknown-org/unknown-repo')->assertOk(); - } - - sendManualWebhookPush($this, 'github', $application)->assertStatus(429); - }); - - test('gitlab deliveries without a token count as failed authentication', function () { + test('gitlab deliveries without a token are rejected and do not count as failed authentication', function () { createApplicationWithWebhook(); - for ($i = 0; $i < 30; $i++) { - $this->postJson('/webhooks/source/gitlab/events/manual', [ + for ($i = 0; $i < 40; $i++) { + $response = $this->postJson('/webhooks/source/gitlab/events/manual', [ 'object_kind' => 'push', 'ref' => 'refs/heads/main', 'project' => ['path_with_namespace' => 'test-org/test-repo'], - ])->assertOk(); + ]); + + $response->assertOk(); + expect($response->getContent())->toContain('Invalid signature'); } - expect(RateLimiter::tooManyAttempts('manual-webhook-failures:gitlab:127.0.0.1', 30))->toBeTrue(); + expect(RateLimiter::attempts(manualWebhookFailureKey('gitlab', '127.0.0.1')))->toBe(0); }); test('ping deliveries do not count as failures', function () { @@ -155,10 +264,7 @@ describe('Manual Webhook Failed Authentication Rate Limiting', function () { test('failures on one provider do not block another provider', function () { $application = createApplicationWithWebhook(); - for ($i = 0; $i < 30; $i++) { - sendManualWebhookPush($this, 'github', $application, validSignature: false); - } - sendManualWebhookPush($this, 'github', $application, validSignature: false)->assertStatus(429); + lockOutManualWebhookRepository($this, 'github', $application); foreach (['gitlab', 'bitbucket', 'gitea'] as $provider) { $response = sendManualWebhookPush($this, $provider, $application);