fix(vpn): stop revoked accesses coming back and stale jobs undoing intent

Two lifecycle holes Codex found in the new VPN management:

1. Delete removed the row before the hub knew. If the removal job was delayed
   or failed, SyncVpnPeers saw a peer it did not recognise and adopted it back
   as a live access — silently restoring what an operator had just revoked, and
   with no row left to revoke it again. Deletion is now a soft-delete
   tombstone: the row survives until the hub confirms the peer is gone, the
   sync retries the removal instead of adopting, and the tombstone is purged
   only once the peer is really absent. It also keeps the key and the tunnel
   address reserved meanwhile, so neither is handed out twice.

2. ApplyVpnPeer applied the state captured at dispatch. A retried block job
   could therefore undo a later unblock, and nothing would repair it — the sync
   only observes state, it never re-applies intent. The job now resolves the
   current desired state from the row and treats its payload as a fallback for
   the case where the row is already gone.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

app/Livewire/Admin/ConfirmDeleteVpnPeer.php

@@ -34,10 +34,13 @@ class ConfirmDeleteVpnPeer extends ModalComponent
 
         $peer = VpnPeer::query()->where('uuid', $this->uuid)->first();
         if ($peer !== null) {
-            // Remove from the hub first: if the row went first and the job then
+            // Soft-delete, so the row survives as a tombstone until the hub has
-            // failed, the peer would keep its tunnel with nothing left to show it.
+            // actually dropped the peer. A hard delete here would let the next
-            ApplyVpnPeer::dispatch($peer->public_key, null, false);
+            // sync adopt the still-present peer back as a live access —
+            // silently restoring what was just revoked. ApplyVpnPeer purges the
+            // tombstone once removal succeeded; SyncVpnPeers retries if not.
             $peer->delete();
+            ApplyVpnPeer::dispatch($peer->public_key, null, false);
         }
 
         $this->dispatch('vpn-peer-deleted');

app/Livewire/Admin/Vpn.php

@@ -55,8 +55,12 @@ class Vpn extends Component
         $keypair = $ownKey ? null : Keypair::generate();
         $publicKey = $ownKey ? trim($this->publicKey) : $keypair->publicKey;
 
-        if (VpnPeer::query()->where('public_key', $publicKey)->exists()) {
+        // withTrashed: a revoked peer still holds the unique key until the hub
-            $this->addError('publicKey', __('vpn.duplicate_key'));
+        // confirms removal, so re-adding it now would hit the index instead of
+        // telling the operator what is actually going on.
+        $existing = VpnPeer::withTrashed()->where('public_key', $publicKey)->first();
+        if ($existing !== null) {
+            $this->addError('publicKey', $existing->trashed() ? __('vpn.pending_removal') : __('vpn.duplicate_key'));
 
             return;
         }

app/Models/VpnPeer.php

@@ -6,11 +6,13 @@ use Illuminate\Database\Eloquent\Concerns\HasUuids;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
+use Illuminate\Database\Eloquent\SoftDeletes;
 
 class VpnPeer extends Model
 {
     use HasFactory;
     use HasUuids;
+    use SoftDeletes;
 
     /** A tunnel is considered live if the last handshake is recent. WireGuard
      *  re-handshakes about every two minutes, so three is the first value that

app/Provisioning/Jobs/ApplyVpnPeer.php

@@ -38,17 +38,38 @@ class ApplyVpnPeer implements ShouldQueue
 
     public function handle(WireguardHub $hub): void
     {
-        if ($this->enabled && $this->allowedIp !== null) {
+        // Read the CURRENT desired state rather than the one captured at
-            $hub->addPeer($this->publicKey, $this->allowedIp);
+        // dispatch time. A retried block job would otherwise undo an unblock
+        // that happened in the meantime, and nothing would ever put it back:
+        // 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);
+        $ip = $peer?->allowed_ip ?? $this->allowedIp;
+
+        if ($enabled && $ip !== null) {
+            $hub->addPeer($this->publicKey, $ip);
         } else {
             $hub->removePeer($this->publicKey);
         }
 
+        if ($peer === null) {
+            return;
+        }
+
+        // The tombstone has served its purpose once the hub has dropped the peer.
+        if ($peer->trashed()) {
+            $peer->forceDelete();
+
+            return;
+        }
+
         // Reflect the change immediately instead of waiting for the next sync,
         // so the console does not sit on "wird angewendet" for a whole minute.
-        VpnPeer::query()->where('public_key', $this->publicKey)->update([
+        $peer->update(['present' => $enabled, 'observed_at' => now()]);
-            'present' => $this->enabled,
-            'observed_at' => now(),
-        ]);
     }
 }

app/Provisioning/Jobs/SyncVpnPeers.php

@@ -50,7 +50,16 @@ class SyncVpnPeers implements ShouldBeUnique, ShouldQueue
             ->pluck('id', 'wg_pubkey');
 
         foreach ($snapshots as $publicKey => $snapshot) {
-            $peer = VpnPeer::query()->where('public_key', $publicKey)->first();
+            $peer = VpnPeer::withTrashed()->where('public_key', $publicKey)->first();
+
+            // Revoked, yet still on the hub: the removal never landed. Retry it
+            // rather than adopting the peer back into the console — otherwise a
+            // failed removal quietly becomes a live access again.
+            if ($peer?->trashed()) {
+                ApplyVpnPeer::dispatch($publicKey, null, false);
+
+                continue;
+            }
             $hostId = $hostsByKey[$publicKey] ?? null;
 
             $observed = [
@@ -83,8 +92,13 @@ class SyncVpnPeers implements ShouldBeUnique, ShouldQueue
             $peer->update($observed + ['host_id' => $peer->host_id ?? $hostId]);
         }
 
+        $seen = array_keys($snapshots);
+
         VpnPeer::query()
-            ->whereNotIn('public_key', array_keys($snapshots))
+            ->whereNotIn('public_key', $seen)
             ->update(['present' => false, 'observed_at' => $now]);
+
+        // A tombstone whose peer is gone from the hub has done its job.
+        VpnPeer::onlyTrashed()->whereNotIn('public_key', $seen)->forceDelete();
     }
 }

app/Services/Wireguard/LocalWireguardHub.php

@@ -21,7 +21,8 @@ class LocalWireguardHub implements WireguardHub
         // access and a host peer would otherwise be given the same tunnel IP.
         $used = array_flip(array_merge(
             Host::query()->whereNotNull('wg_ip')->pluck('wg_ip')->all(),
-            VpnPeer::query()->pluck('allowed_ip')->all(),
+            // withTrashed: a revoked peer keeps its address until the hub drops it.
+            VpnPeer::withTrashed()->pluck('allowed_ip')->all(),
         ));
         $hubIp = (string) config('provisioning.wireguard.hub_ip');
 

database/migrations/2026_07_25_200000_create_vpn_peers_table.php

@@ -34,6 +34,11 @@ return new class extends Migration
             $table->timestamp('observed_at')->nullable();   // when the hub last reported it
             $table->foreignId('created_by')->nullable()->constrained('users')->nullOnDelete();
             $table->timestamps();
+            // Tombstone: a revoked access keeps its row until the hub has
+            // actually dropped the peer. Deleting outright would let the next
+            // sync see the still-present peer and adopt it back as a live
+            // access — silently restoring what an operator just revoked.
+            $table->softDeletes();
         });
     }
 

lang/de/vpn.php

@@ -32,6 +32,7 @@ return [
     'own_key_placeholder' => 'optional',
     'invalid_key' => 'Das ist kein gültiger WireGuard-Schlüssel (32 Byte, Base64).',
     'duplicate_key' => 'Für diesen Schlüssel gibt es bereits einen Zugang.',
+    'pending_removal' => 'Dieser Zugang wird gerade entfernt. Bitte kurz warten.',
     'created' => 'Zugang angelegt.',
     'blocked' => 'Zugang gesperrt.',
     'unblocked' => 'Zugang wieder freigegeben.',

lang/en/vpn.php

@@ -32,6 +32,7 @@ return [
     'own_key_placeholder' => 'optional',
     'invalid_key' => 'That is not a valid WireGuard key (32 bytes, base64).',
     'duplicate_key' => 'An access already exists for this key.',
+    'pending_removal' => 'This access is currently being removed. Please wait a moment.',
     'created' => 'Access created.',
     'blocked' => 'Access blocked.',
     'unblocked' => 'Access re-enabled.',

tests/Feature/Admin/VpnTest.php

@@ -207,3 +207,69 @@ it('does not hand the same tunnel address to a host and a VPN access', function
     // The real allocator, not the fake — this is the collision the fake cannot show.
     expect((new LocalWireguardHub)->allocateIp())->toBe('10.66.0.4');
 });
+
+it('does not let a failed removal resurrect a revoked access', function () {
+    $hub = vpnHub();
+    Queue::fake();
+    $peer = VpnPeer::factory()->create();
+    $hub->addPeer($peer->public_key, $peer->allowed_ip);
+
+    Livewire::actingAs(operator('Owner'))
+        ->test(ConfirmDeleteVpnPeer::class, ['uuid' => $peer->uuid])
+        ->call('delete');
+
+    // Removal is queued but has not run — the peer is still on the hub.
+    expect(VpnPeer::query()->count())->toBe(0)
+        ->and(VpnPeer::withTrashed()->count())->toBe(1);
+
+    (new SyncVpnPeers)->handle($hub);
+
+    // The sync must NOT adopt it back as a live access…
+    expect(VpnPeer::query()->count())->toBe(0);
+    // …it must retry the removal instead.
+    Queue::assertPushed(ApplyVpnPeer::class, fn ($job) => $job->publicKey === $peer->public_key && ! $job->enabled);
+});
+
+it('clears the tombstone once the peer is really gone', function () {
+    $hub = vpnHub();
+    $peer = VpnPeer::factory()->create();
+    $hub->addPeer($peer->public_key, $peer->allowed_ip);
+    $peer->delete();
+
+    (new ApplyVpnPeer($peer->public_key, null, false))->handle($hub);
+
+    expect($hub->peerIps())->toBe([])
+        ->and(VpnPeer::withTrashed()->count())->toBe(0);
+});
+
+it('ignores a stale job that would undo a newer decision', function () {
+    $hub = vpnHub();
+    $peer = VpnPeer::factory()->create();
+    $hub->addPeer($peer->public_key, $peer->allowed_ip);
+
+    // Operator blocks, then immediately unblocks: two jobs are now in flight.
+    $blockJob = new ApplyVpnPeer($peer->public_key, $peer->allowed_ip, false);
+    $peer->update(['enabled' => false]);
+    $peer->update(['enabled' => true]);          // newer intent wins
+
+    // The older block job runs late (a retry, say). It must not cut access.
+    $blockJob->handle($hub);
+
+    expect($hub->peerIps())->toHaveKey($peer->public_key)
+        ->and($peer->fresh()->present)->toBeTrue();
+});
+
+it('keeps a revoked address and key reserved until the hub confirms', function () {
+    vpnHub();
+    $peer = VpnPeer::factory()->create(['allowed_ip' => '10.66.0.2']);
+    $peer->delete();
+
+    // The address is still configured on the hub — handing it out again would
+    // give two machines the same tunnel IP.
+    expect((new LocalWireguardHub)->allocateIp())->not->toBe('10.66.0.2');
+
+    Livewire::actingAs(operator('Owner'))->test(Vpn::class)
+        ->set('name', 'wieder da')->set('publicKey', $peer->public_key)
+        ->call('create')
+        ->assertHasErrors('publicKey');
+});