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);