From 63e3c95faead959b08ef89b005d597732738239a Mon Sep 17 00:00:00 2001 From: Your Name Date: Wed, 9 Sep 2026 19:54:24 +0930 Subject: [PATCH] Fix custom filter rate-limit counter never expiring --- .../Controllers/CustomFilterController.php | 14 ++-- .../Feature/Api/CustomFilterRateLimitTest.php | 79 +++++++++++++++++++ 2 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 tests/Feature/Api/CustomFilterRateLimitTest.php diff --git a/app/Http/Controllers/CustomFilterController.php b/app/Http/Controllers/CustomFilterController.php index f6ad93352..e42344638 100644 --- a/app/Http/Controllers/CustomFilterController.php +++ b/app/Http/Controllers/CustomFilterController.php @@ -180,10 +180,11 @@ class CustomFilterController extends Controller } } + // Create the counter with a 1h TTL only on first write, then + // increment. increment() alone would create a TTL-less key, so the + // "per hour" window must be established by the add() before it. + Cache::add($rateKey, 0, 3600); Cache::increment($rateKey); - if (! Cache::has($rateKey)) { - Cache::put($rateKey, 1, 3600); - } Cache::forget("filters:v3:{$profile_id}"); @@ -445,10 +446,11 @@ class CustomFilterController extends Controller } } + // Create the counter with a 1h TTL only on first write, then + // increment. increment() alone would create a TTL-less key, so the + // "per hour" window must be established by the add() before it. + Cache::add($rateKey, 0, 3600); Cache::increment($rateKey); - if (! Cache::has($rateKey)) { - Cache::put($rateKey, 1, 3600); - } Cache::forget("filters:v3:{$pid}"); diff --git a/tests/Feature/Api/CustomFilterRateLimitTest.php b/tests/Feature/Api/CustomFilterRateLimitTest.php new file mode 100644 index 000000000..b13d445d5 --- /dev/null +++ b/tests/Feature/Api/CustomFilterRateLimitTest.php @@ -0,0 +1,79 @@ +setAccessible(true); + $ref->setValue(null, null); + } +} + +function createFilter(string $title, string $keyword) +{ + return test()->postJson('/api/v2/filters', [ + 'title' => $title, + 'context' => ['home'], + 'filter_action' => 'warn', + 'keywords_attributes' => [ + ['keyword' => $keyword, 'whole_word' => true], + ], + ]); +} + +beforeEach(function () { + Cache::flush(); + config(['instance.custom_filters.max_create_per_hour' => 2]); + config(['instance.custom_filters.max_filters_per_user' => 100]); + resetFilterLimitStatics(); +}); + +it('resets the create rate limit after the 1 hour window', function () { + $user = User::factory()->create(); + $user->refresh(); + Passport::actingAs($user, ['read', 'write']); + + // Two creates within the cap succeed. + createFilter('one', 'alpha')->assertOk(); + createFilter('two', 'bravo')->assertOk(); + + // Third exceeds the cap -> 429. + createFilter('three', 'charlie')->assertStatus(429); + + // After the 1h window elapses, the counter must reset and allow writes. + $this->travel(3700)->seconds(); + + createFilter('four', 'delta')->assertOk(); +}); + +it('rejects writes once the cap is reached within the window', function () { + $user = User::factory()->create(); + $user->refresh(); + Passport::actingAs($user, ['read', 'write']); + + createFilter('one', 'alpha')->assertOk(); + createFilter('two', 'bravo')->assertOk(); + createFilter('three', 'charlie')->assertStatus(429); +});