From 21280a6ba861fd40c732856c525f54f269965720 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Wed, 20 May 2026 17:28:59 -0400 Subject: [PATCH 1/3] fix: prevent self-referential EventAchievement infinite loops --- .../Resources/EventAchievementResource.php | 12 +++++++ .../Actions/UnlockPlayerAchievementAction.php | 4 +++ .../UnlockPlayerAchievementActionTest.php | 31 +++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/app/Filament/Resources/EventAchievementResource.php b/app/Filament/Resources/EventAchievementResource.php index ea3111c4de..f98b410a2b 100644 --- a/app/Filament/Resources/EventAchievementResource.php +++ b/app/Filament/Resources/EventAchievementResource.php @@ -12,6 +12,7 @@ use App\Models\EventAchievement; use App\Models\Game; use App\Models\User; +use Closure; use Filament\Forms; use Filament\Infolists; use Filament\Pages\Page; @@ -167,6 +168,17 @@ public static function form(Schema $schema): Schema } return $label; + }) + ->rule(function (?EventAchievement $record): Closure { + return function (string $attribute, mixed $value, Closure $fail) use ($record): void { + if ( + $record !== null + && $value !== null + && (int) $value === (int) $record->achievement_id + ) { + $fail('Source achievement cannot be the same as the event achievement.'); + } + }; }), Forms\Components\DatePicker::make('active_from') diff --git a/app/Platform/Actions/UnlockPlayerAchievementAction.php b/app/Platform/Actions/UnlockPlayerAchievementAction.php index e85ca918eb..53f6898fe0 100644 --- a/app/Platform/Actions/UnlockPlayerAchievementAction.php +++ b/app/Platform/Actions/UnlockPlayerAchievementAction.php @@ -39,6 +39,10 @@ public function execute( // also unlock active event achievements associated to the achievement being unlocked if ($hardcore && $user->isRanked()) { foreach ($achievement->eventAchievements()->active($timestamp)->get() as $eventAchievement) { + if ($eventAchievement->achievement_id === $achievement->id) { + continue; + } + dispatch(new UnlockPlayerAchievementJob($user->id, $eventAchievement->achievement_id, true, $timestamp, $unlockedBy?->id, $gameHash?->id, $userAgent)) ->onQueue('player-achievements'); } diff --git a/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php b/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php index d6ed15543d..7a63d6326a 100644 --- a/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php +++ b/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php @@ -4,6 +4,8 @@ namespace Tests\Feature\Platform\Actions; +use App\Models\Achievement; +use App\Models\EventAchievement; use App\Models\Game; use App\Models\PlayerGame; use App\Models\PlayerSession; @@ -13,9 +15,12 @@ use App\Platform\Enums\AchievementType; use App\Platform\Events\PlayerAchievementUnlocked; use App\Platform\Events\PlayerGameAttached; +use App\Platform\Jobs\UnlockPlayerAchievementJob; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Queue; use Tests\TestCase; class UnlockPlayerAchievementActionTest extends TestCase @@ -183,6 +188,32 @@ public function testEntireSetManuallyUnlockAwardsBadge(): void $this->assertEquals(0, $user1->playerSessions()->count()); } + public function testFanOutSkipsSelfReferentialEventAchievement(): void + { + // a self-referential EventAchievement (source_achievement_id == achievement_id) would + // dispatch a job re-targeting the same achievement and infinite loop, pinning our infra + $user = User::factory()->create(); + $game = $this->seedGame(achievements: 1); + /** @var Achievement $achievement */ + $achievement = $game->achievements->first(); + + DB::table('event_achievements')->insert([ + 'achievement_id' => $achievement->id, + 'source_achievement_id' => $achievement->id, + 'created_at' => Carbon::now(), + 'updated_at' => Carbon::now(), + ]); + + Queue::fake(); + + (new UnlockPlayerAchievementAction())->execute($user, $achievement, true); + + Queue::assertNotPushed( + UnlockPlayerAchievementJob::class, + fn (UnlockPlayerAchievementJob $job) => in_array(Achievement::class . ':' . $achievement->id, $job->tags(), true) + ); + } + public function testSubsetAchievementThroughCoreSetDoesntCreateSubsetSession(): void { $coreGame = $this->seedGame(achievements: 6); From 95094e2e012ed795cb3f4f089220a9b063d4d446 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Thu, 21 May 2026 18:42:02 -0400 Subject: [PATCH 2/3] fix: address feedback --- .../Actions/UnlockPlayerAchievementAction.php | 4 --- .../UnlockPlayerAchievementActionTest.php | 31 ------------------- 2 files changed, 35 deletions(-) diff --git a/app/Platform/Actions/UnlockPlayerAchievementAction.php b/app/Platform/Actions/UnlockPlayerAchievementAction.php index 53f6898fe0..e85ca918eb 100644 --- a/app/Platform/Actions/UnlockPlayerAchievementAction.php +++ b/app/Platform/Actions/UnlockPlayerAchievementAction.php @@ -39,10 +39,6 @@ public function execute( // also unlock active event achievements associated to the achievement being unlocked if ($hardcore && $user->isRanked()) { foreach ($achievement->eventAchievements()->active($timestamp)->get() as $eventAchievement) { - if ($eventAchievement->achievement_id === $achievement->id) { - continue; - } - dispatch(new UnlockPlayerAchievementJob($user->id, $eventAchievement->achievement_id, true, $timestamp, $unlockedBy?->id, $gameHash?->id, $userAgent)) ->onQueue('player-achievements'); } diff --git a/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php b/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php index 7a63d6326a..d6ed15543d 100644 --- a/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php +++ b/tests/Feature/Platform/Actions/UnlockPlayerAchievementActionTest.php @@ -4,8 +4,6 @@ namespace Tests\Feature\Platform\Actions; -use App\Models\Achievement; -use App\Models\EventAchievement; use App\Models\Game; use App\Models\PlayerGame; use App\Models\PlayerSession; @@ -15,12 +13,9 @@ use App\Platform\Enums\AchievementType; use App\Platform\Events\PlayerAchievementUnlocked; use App\Platform\Events\PlayerGameAttached; -use App\Platform\Jobs\UnlockPlayerAchievementJob; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Carbon; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; -use Illuminate\Support\Facades\Queue; use Tests\TestCase; class UnlockPlayerAchievementActionTest extends TestCase @@ -188,32 +183,6 @@ public function testEntireSetManuallyUnlockAwardsBadge(): void $this->assertEquals(0, $user1->playerSessions()->count()); } - public function testFanOutSkipsSelfReferentialEventAchievement(): void - { - // a self-referential EventAchievement (source_achievement_id == achievement_id) would - // dispatch a job re-targeting the same achievement and infinite loop, pinning our infra - $user = User::factory()->create(); - $game = $this->seedGame(achievements: 1); - /** @var Achievement $achievement */ - $achievement = $game->achievements->first(); - - DB::table('event_achievements')->insert([ - 'achievement_id' => $achievement->id, - 'source_achievement_id' => $achievement->id, - 'created_at' => Carbon::now(), - 'updated_at' => Carbon::now(), - ]); - - Queue::fake(); - - (new UnlockPlayerAchievementAction())->execute($user, $achievement, true); - - Queue::assertNotPushed( - UnlockPlayerAchievementJob::class, - fn (UnlockPlayerAchievementJob $job) => in_array(Achievement::class . ':' . $achievement->id, $job->tags(), true) - ); - } - public function testSubsetAchievementThroughCoreSetDoesntCreateSubsetSession(): void { $coreGame = $this->seedGame(achievements: 6); From 3a2bc4ba939294b29f11b6774b38a968a2f846c6 Mon Sep 17 00:00:00 2001 From: Wes Copeland Date: Thu, 21 May 2026 18:42:14 -0400 Subject: [PATCH 3/3] chore: missing change --- .../Resources/EventAchievementResource.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/Filament/Resources/EventAchievementResource.php b/app/Filament/Resources/EventAchievementResource.php index f98b410a2b..2b3fe5a982 100644 --- a/app/Filament/Resources/EventAchievementResource.php +++ b/app/Filament/Resources/EventAchievementResource.php @@ -169,14 +169,14 @@ public static function form(Schema $schema): Schema return $label; }) - ->rule(function (?EventAchievement $record): Closure { - return function (string $attribute, mixed $value, Closure $fail) use ($record): void { - if ( - $record !== null - && $value !== null - && (int) $value === (int) $record->achievement_id - ) { - $fail('Source achievement cannot be the same as the event achievement.'); + ->rule(function (): Closure { + return function (string $attribute, mixed $value, Closure $fail): void { + if ($value === null) { + return; + } + + if (EventAchievement::where('achievement_id', (int) $value)->exists()) { + $fail('Source achievement cannot be an event achievement.'); } }; }),