diff --git a/app/Jobs/ProcessHqWhitelistJob.php b/app/Jobs/ProcessHqWhitelistJob.php index e507354..b29251b 100644 --- a/app/Jobs/ProcessHqWhitelistJob.php +++ b/app/Jobs/ProcessHqWhitelistJob.php @@ -144,7 +144,7 @@ private function notifyAdmin(Host $hqHost, string $modsecLogs): void 'email' => $adminEmail, ]); - $ttl = (int) (config('unblock.hq.ttl') ?? 7200); + $ttl = (int) config('unblock.hq.ttl'); Mail::to($adminEmail)->send(new HqWhitelistMail( user: $adminUser, diff --git a/app/Services/Firewall/DirectAdminFirewallAnalyzer.php b/app/Services/Firewall/DirectAdminFirewallAnalyzer.php index f38ea97..c3ab146 100644 --- a/app/Services/Firewall/DirectAdminFirewallAnalyzer.php +++ b/app/Services/Firewall/DirectAdminFirewallAnalyzer.php @@ -489,7 +489,7 @@ private function checkAndRemoveFromBfmBlacklist(string $ip, string $sshKeyName): $this->firewallService->checkProblems($this->host, $sshKeyName, 'da_bfm_whitelist_add', $ip); // 3. Track in database for automatic removal after TTL - $ttl = (int) (config('unblock.hq.ttl') ?? 7200); // Default 2 hours + $ttl = (int) config('unblock.hq.ttl'); BfmWhitelistEntry::create([ 'host_id' => $this->host->id, 'ip_address' => $ip, diff --git a/rector.php b/rector.php index da8a120..ba2b4e9 100644 --- a/rector.php +++ b/rector.php @@ -11,6 +11,7 @@ use Rector\TypeDeclaration\Rector\StmtsAwareInterface\DeclareStrictTypesRector; use Rector\ValueObject\PhpVersion; use RectorLaravel\Rector\ClassMethod\MakeModelAttributesAndScopesProtectedRector; +use RectorLaravel\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector; use RectorLaravel\Rector\FuncCall\{AppToResolveRector, RemoveDumpDataDeadCodeRector, ThrowIfAndThrowUnlessExceptionsToUseClassStringRector}; use RectorLaravel\Rector\If_\ThrowIfRector; use RectorLaravel\Set\LaravelSetList; @@ -111,6 +112,21 @@ // consumes scopes through the builder proxy. This app is final, not a // library: no external consumers exist. MakeModelAttributesAndScopesProtectedRector::class, + // Wave 6 (AID-542), split from AID-531 over the ??/?: lesson. The + // rule turns `helper('key') ?? $default` into `helper('key', + // $default)` — NOT ??→?:, but it still has a non-equivalent corner: + // a key present with an explicit null returns $default under ??, + // null under the parameter form. Both sites it flagged used + // `config('unblock.hq.ttl') ?? 7200`, where that corner is + // unreachable by construction (config/unblock.php casts `(int) + // env(..., 14000)`, so the key always exists as int) — which also + // made the 7200 a dead phantom default lying about the real 14000. + // The fallbacks were removed by hand (the default's single owner is + // the config file); the rule stays on as the guard against future + // `config('x') ?? $d`, which is always either dead or a duplicated + // default. If a legitimate present-but-null key ever appears, skip + // that call site here rather than dropping the rule. + ApplyDefaultInsteadOfNullCoalesceRector::class, ]) // Wave 4 (AID-532): LARAVEL_COLLECTION (4 sites, all element types are // scalars/plain arrays so toArray()≡all(); filter(!empty)→reject(empty) diff --git a/tests/Unit/Jobs/ProcessHqWhitelistJobTest.php b/tests/Unit/Jobs/ProcessHqWhitelistJobTest.php index 1217bc6..c96d04a 100644 --- a/tests/Unit/Jobs/ProcessHqWhitelistJobTest.php +++ b/tests/Unit/Jobs/ProcessHqWhitelistJobTest.php @@ -450,12 +450,17 @@ // SCENARIO 8: TTL Configuration // ============================================================================ -test('job uses default TTL when not configured', function () { +test('job uses the config-owned default TTL when env is not set', function () { $hqHost = Host::factory()->create(['hash' => 'test-key']); // ggignore config()->set('unblock.hq.host_id', $hqHost->id); config()->set('unblock.admin_email', 'admin@example.com'); - config()->set('unblock.hq.ttl', null); // ← Not configured + // "Not configured" means the HQ_WHITELIST_TTL env var is absent, in which + // case config/unblock.php resolves `(int) env(..., 14000)` at load time — + // the key always exists as an int, never null. The job carries no local + // fallback anymore (AID-542); the default's single owner is the config + // file, so this test pins that owner's value flowing through untouched. + expect(config('unblock.hq.ttl'))->toBe(14000); $firewallService = Mockery::mock(FirewallService::class); $firewallService->allows('generateSshKey')->andReturn('/tmp/key'); @@ -473,7 +478,7 @@ $job->handle($firewallService); Mail::assertQueued(HqWhitelistMail::class, function ($mail) { - return $mail->ttlSeconds === 7200; // Default 7200s + return $mail->ttlSeconds === 14000; // config/unblock.php default }); });