From 13298591d81ecf5083712d98ce79d53e8bdfa3aa Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:38:30 +0100 Subject: [PATCH] fix(api): throttle API clients that keep failing and notify the token's team - Return 429 after 30 failed (401/403/404) requests per minute for one token or IP, also for unknown API routes (API_FAILED_REQUEST_RATE_LIMIT) - Notify the team once a day when a token reaches the limit - Deduplicate ability_denied and scope_denied audit events for 5 minutes --- app/Http/Kernel.php | 2 + .../Middleware/ThrottleFailedApiRequests.php | 118 ++++++++++++++++ .../ApiTokenFailingNotification.php | 128 ++++++++++++++++++ app/Traits/HasNotificationSettings.php | 1 + bootstrap/helpers/audit.php | 13 +- config/api.php | 2 + .../views/emails/api-token-failing.blade.php | 9 ++ .../Feature/AuditFailureDeduplicationTest.php | 6 +- 8 files changed, 272 insertions(+), 7 deletions(-) create mode 100644 app/Http/Middleware/ThrottleFailedApiRequests.php create mode 100644 app/Notifications/ApiTokenFailingNotification.php create mode 100644 resources/views/emails/api-token-failing.blade.php diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index e897919e71..09156692e6 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -22,6 +22,7 @@ use App\Http\Middleware\PreventRequestsDuringMaintenance; use App\Http\Middleware\RedirectIfAuthenticated; use App\Http\Middleware\RequirePassword; use App\Http\Middleware\RequireTeamTwoFactor; +use App\Http\Middleware\ThrottleFailedApiRequests; use App\Http\Middleware\TrimStrings; use App\Http\Middleware\TrustProxies; use App\Http\Middleware\ValidateSignature; @@ -58,6 +59,7 @@ class Kernel extends HttpKernel AddServerTimingHeaders::class, TrustProxies::class, HandleCors::class, + ThrottleFailedApiRequests::class, PreventRequestsDuringMaintenance::class, ValidatePostSize::class, TrimStrings::class, diff --git a/app/Http/Middleware/ThrottleFailedApiRequests.php b/app/Http/Middleware/ThrottleFailedApiRequests.php new file mode 100644 index 0000000000..18b40f27dd --- /dev/null +++ b/app/Http/Middleware/ThrottleFailedApiRequests.php @@ -0,0 +1,118 @@ + */ + private const BLOCK_SECONDS = [60, 300, 900]; + + private const STRIKE_MEMORY_SECONDS = 3600; + + private const NOTIFICATION_INTERVAL_SECONDS = 86400; + + public function handle(Request $request, Closure $next): Response + { + $limit = (int) config('api.failed_request_rate_limit'); + if ($limit <= 0 || ! $request->is('api/*') || $request->is('api/health', 'api/v1/health')) { + return $next($request); + } + + $key = $this->key($request); + + $retryAfter = $this->blockedFor($key); + if ($retryAfter > 0) { + return response()->json([ + 'message' => "Too many failed API requests. Retry after {$retryAfter} seconds.", + ], 429, ['Retry-After' => $retryAfter]); + } + + $response = $next($request); + + if ($this->isCountedFailure($request, $response->getStatusCode()) + && RateLimiter::hit($key, self::DECAY_SECONDS) >= $limit) { + $this->block($key); + $this->notifyTeam($request, $response->getStatusCode()); + } + + return $response; + } + + private function isCountedFailure(Request $request, int $statusCode): bool + { + return in_array($statusCode, [401, 403], true) + || ($statusCode === 404 && ! $request->bearerToken()); + } + + private function blockedFor(string $key): int + { + return max(0, (int) Cache::get("{$key}:blocked-until", 0) - now()->getTimestamp()); + } + + /** + * Block the client, longer each time it reaches the limit again within one hour. + */ + private function block(string $key): void + { + $strikes = (int) Cache::get("{$key}:strikes", 0) + 1; + Cache::put("{$key}:strikes", $strikes, self::STRIKE_MEMORY_SECONDS); + + $seconds = self::BLOCK_SECONDS[min($strikes, count(self::BLOCK_SECONDS)) - 1]; + Cache::put("{$key}:blocked-until", now()->getTimestamp() + $seconds, $seconds); + RateLimiter::clear($key); + } + + private function key(Request $request): string + { + $bearerToken = $request->bearerToken(); + + return 'api-failed-requests:'.($bearerToken + ? 'token:'.hash('sha256', $bearerToken) + : 'ip:'.client_ip($request)); + } + + private function notifyTeam(Request $request, int $statusCode): void + { + try { + $bearerToken = $request->bearerToken(); + $token = $bearerToken ? PersonalAccessToken::findToken($bearerToken) : null; + if (! $token instanceof PersonalAccessToken || ! $token->team_id) { + return; + } + + if (! Cache::add("api-token-failing-notified:{$token->id}", true, self::NOTIFICATION_INTERVAL_SECONDS)) { + return; + } + + Team::find($token->team_id)?->notify(new ApiTokenFailingNotification( + token: $token, + failedRequests: (int) config('api.failed_request_rate_limit'), + lastRequest: $request->method().' /'.$request->path().' → '.$statusCode, + )); + } catch (Throwable $e) { + report($e); + } + } +} diff --git a/app/Notifications/ApiTokenFailingNotification.php b/app/Notifications/ApiTokenFailingNotification.php new file mode 100644 index 0000000000..0579bdd9a0 --- /dev/null +++ b/app/Notifications/ApiTokenFailingNotification.php @@ -0,0 +1,128 @@ +onQueue('high'); + $this->tokenName = $token->name; + $this->manageUrl = base_url().'/security/api-tokens'; + } + + public function via(object $notifiable): array + { + return $notifiable->getEnabledChannels('api_token_failing'); + } + + private function summary(): string + { + return "API token '{$this->tokenName}' sent {$this->failedRequests} failed requests in one minute. Coolify now answers its requests with 429 (Too Many Requests) until the failures stop."; + } + + private function action(): string + { + return 'Find the client that uses this token and fix its requests, give the token the permissions it needs, or revoke the token.'; + } + + public function toMail(): MailMessage + { + $mail = new MailMessage; + $mail->subject("Coolify: API token '{$this->tokenName}' sends many failed requests"); + $mail->view('emails.api-token-failing', [ + 'summary' => $this->summary(), + 'action' => $this->action(), + 'lastRequest' => $this->lastRequest, + 'manageUrl' => $this->manageUrl, + ]); + + return $mail; + } + + public function toDiscord(): DiscordMessage + { + $message = new DiscordMessage( + title: '🔑 API token sends many failed requests', + description: $this->summary()."\n\n**Action Required:** ".$this->action(), + color: DiscordMessage::warningColor(), + ); + + $message->addField('Last failed request', $this->lastRequest); + $message->addField('Manage tokens', "[Open API Tokens]({$this->manageUrl})"); + + return $message; + } + + public function toTelegram(): array + { + return [ + 'message' => "Coolify: {$this->summary()}\n\nLast failed request: {$this->lastRequest}\n\nAction Required: {$this->action()}", + 'buttons' => [ + [ + 'text' => 'Manage API tokens', + 'url' => $this->manageUrl, + ], + ], + ]; + } + + public function toPushover(): PushoverMessage + { + $message = e($this->summary()).'

'; + $message .= 'Last failed request: '.e($this->lastRequest).'

'; + $message .= 'Action Required: '.e($this->action()); + + return new PushoverMessage( + title: 'API token sends many failed requests', + level: 'warning', + message: $message, + buttons: [ + [ + 'text' => 'Manage API tokens', + 'url' => $this->manageUrl, + ], + ], + ); + } + + public function toSlack(): SlackMessage + { + $description = $this->summary()."\n\n"; + $description .= "Last failed request: {$this->lastRequest}\n\n"; + $description .= '*Action Required:* '.$this->action()."\n\n"; + $description .= "Manage tokens: {$this->manageUrl}"; + + return new SlackMessage( + title: '🔑 API token sends many failed requests', + description: $description, + color: SlackMessage::warningColor(), + ); + } + + public function toWebhook(): array + { + return [ + 'success' => false, + 'message' => $this->summary(), + 'event' => 'api_token_failing', + 'token_name' => $this->tokenName, + 'failed_requests' => $this->failedRequests, + 'last_request' => $this->lastRequest, + 'url' => $this->manageUrl, + ]; + } +} diff --git a/app/Traits/HasNotificationSettings.php b/app/Traits/HasNotificationSettings.php index 9333eb5042..c317d6e0ea 100644 --- a/app/Traits/HasNotificationSettings.php +++ b/app/Traits/HasNotificationSettings.php @@ -20,6 +20,7 @@ trait HasNotificationSettings 'ssl_certificate_renewal', 'hetzner_deletion_failure', 'api_token_expiring', + 'api_token_failing', ]; /** diff --git a/bootstrap/helpers/audit.php b/bootstrap/helpers/audit.php index a83afc675f..d6c83c8e54 100644 --- a/bootstrap/helpers/audit.php +++ b/bootstrap/helpers/audit.php @@ -73,7 +73,7 @@ if (! function_exists('auditChangedFields')) { if (! function_exists('auditFailureDeduplicationWindow')) { /** - * Seconds during which repeated unauthenticated failures from one source are recorded once. + * Seconds during which repeated unauthenticated or permission failures from one source are recorded once. * * Failed logins use a short window so an ongoing brute force attempt stays visible * (one row per email and source IP each minute, on top of the login rate limit). @@ -88,6 +88,7 @@ if (! function_exists('auditFailureDeduplicationWindow')) { $event === 'webhook.sentinel.signature_failed' && in_array(data_get($context, 'reason'), ['server_not_found', 'subscription_unpaid'], true) => 3600, $event === 'api.auth.unauthenticated', + preg_match('/^(api|mcp)\.auth\.(ability|scope)_denied$/', $event) === 1, preg_match('/^webhook\.[a-z0-9_-]+\.signature_failed$/i', $event) === 1 => 300, default => null, }; @@ -96,8 +97,8 @@ if (! function_exists('auditFailureDeduplicationWindow')) { if (! function_exists('shouldRecordAuditEvent')) { /** - * Keep only the first unauthenticated failure per event, source IP, reason, server and - * attempted email in each deduplication window. All other events are always recorded. + * Keep only the first failure per event, source IP, reason, server, attempted email, token and + * required abilities in each deduplication window. All other events are always recorded. * * @param array $context */ @@ -110,12 +111,16 @@ if (! function_exists('shouldRecordAuditEvent')) { try { $request = app()->bound('request') ? request() : null; + $tokenId = data_get($context, 'token_id'); $fingerprint = hash('sha256', json_encode([ $event, - $request ? client_ip($request) : null, + // The token identifies the client, so a token used from many IPs gets one row. + $tokenId === null && $request ? client_ip($request) : null, data_get($context, 'reason'), data_get($context, 'server_uuid'), Str::lower(trim((string) data_get($context, 'attempted_email'))), + $tokenId, + data_get($context, 'required_abilities'), ])); return Cache::add("audit-failure-dedup:{$fingerprint}", true, $window); diff --git a/config/api.php b/config/api.php index 83bac8f03f..d448e4aa50 100644 --- a/config/api.php +++ b/config/api.php @@ -2,4 +2,6 @@ return [ 'rate_limit' => env('API_RATE_LIMIT', 200), + // Failed requests (401, 403, 404) per minute for one token or source IP before it gets 429. 0 turns it off. + 'failed_request_rate_limit' => env('API_FAILED_REQUEST_RATE_LIMIT', 30), ]; diff --git a/resources/views/emails/api-token-failing.blade.php b/resources/views/emails/api-token-failing.blade.php new file mode 100644 index 0000000000..8fbe0becd6 --- /dev/null +++ b/resources/views/emails/api-token-failing.blade.php @@ -0,0 +1,9 @@ + +{{ $summary }} + +Last failed request: {{ $lastRequest }} + +{{ $action }} + +Manage your API tokens [here]({{ $manageUrl }}). + diff --git a/tests/Feature/AuditFailureDeduplicationTest.php b/tests/Feature/AuditFailureDeduplicationTest.php index 2fb9a53343..76152e8d34 100644 --- a/tests/Feature/AuditFailureDeduplicationTest.php +++ b/tests/Feature/AuditFailureDeduplicationTest.php @@ -85,11 +85,11 @@ test('failed logins record one audit event per email and source ip in each windo ->toBe(['victim@example.com', 'other@example.com', 'victim@example.com']); }); -test('events outside the unauthenticated failure set are never deduplicated', function () { +test('events outside the failure set are never deduplicated', function () { auditLog('webhook.github.deployment_queued', ['reason' => 'push']); auditLog('webhook.github.deployment_queued', ['reason' => 'push']); - auditLog('api.auth.ability_denied', ['reason' => 'missing ability']); - auditLog('api.auth.ability_denied', ['reason' => 'missing ability']); + auditLog('api.private_key.created', ['reason' => 'created']); + auditLog('api.private_key.created', ['reason' => 'created']); expect(AuditEvent::query()->count())->toBe(4); });