fix(vpn): a stale enable job can no longer resurrect a deleted access

If an add job was retried after the access had been revoked and its tombstone
purged, it found no row and trusted its own enabled=true payload — re-adding a
key that no longer had a row anyone could revoke it with. A missing row can only
mean the access is gone, so the captured payload may now remove but never
enable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
feat/portal-design
nexxo 2026-07-25 21:50:57 +02:00
parent 1b193ba4ea
commit 9137eae6df
2 changed files with 20 additions and 5 deletions

View File

@ -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) {

View File

@ -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);
});