diff --git a/app/Provisioning/Jobs/ApplyVpnPeer.php b/app/Provisioning/Jobs/ApplyVpnPeer.php index b35f4cf..1dd6676 100644 --- a/app/Provisioning/Jobs/ApplyVpnPeer.php +++ b/app/Provisioning/Jobs/ApplyVpnPeer.php @@ -44,11 +44,11 @@ class ApplyVpnPeer implements ShouldQueue // SyncVpnPeers only observes, it never re-applies intent. $peer = VpnPeer::withTrashed()->where('public_key', $this->publicKey)->first(); - // No row, or a revoked one, means the peer must be off the hub. The - // constructor payload only matters when the row is already gone. - $enabled = $peer === null - ? $this->enabled && $this->allowedIp !== null - : (! $peer->trashed() && $peer->enabled); + // A missing row can only mean the access is gone — deleted, tombstone + // already purged. A stale enable payload must never be able to re-add a + // key that no longer has a row to revoke it with, so the captured state + // can only ever remove, never enable. + $enabled = $peer !== null && ! $peer->trashed() && $peer->enabled; $ip = $peer?->allowed_ip ?? $this->allowedIp; if ($enabled && $ip !== null) { diff --git a/tests/Feature/Admin/VpnTest.php b/tests/Feature/Admin/VpnTest.php index ae2fade..9f13bef 100644 --- a/tests/Feature/Admin/VpnTest.php +++ b/tests/Feature/Admin/VpnTest.php @@ -306,3 +306,18 @@ it('holds the same allocation lock the host pipeline uses', function () { expect(VpnPeer::query()->count())->toBe(1) ->and($elapsed)->toBeGreaterThan(0.8); // it waited for the lock to expire }); + +it('never re-adds a peer whose row is gone, however stale the job', function () { + $hub = vpnHub(); + $peer = VpnPeer::factory()->create(); + + // The add job is retried late — after the access was revoked and its + // tombstone already purged by a sync that saw the peer gone. + $staleAdd = ApplyVpnPeer::for($peer); + $peer->forceDelete(); + + $staleAdd->handle($hub); + + expect($hub->peerIps())->toBe([]) + ->and(VpnPeer::withTrashed()->count())->toBe(0); +});