fix(vpn): serialize hub reads against hub mutations

A sync could read the hub snapshot just before ApplyVpnPeer removed a peer and
purged its tombstone, then adopt what it had seen as a fresh enabled access —
restoring a revoked one. Both jobs now hold Cache::lock('wireguard:hub') around
read-decide-mutate, so a reconciliation can never straddle a removal. Retry
removals are dispatched after the lock is released, because the sync queue
driver runs them inline and they take the same lock.

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

app/Provisioning/Jobs/ApplyVpnPeer.php

@@ -4,6 +4,7 @@ namespace App\Provisioning\Jobs;
 
 use App\Models\VpnPeer;
 use App\Services\Wireguard\WireguardHub;
+use Illuminate\Support\Facades\Cache;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
@@ -37,6 +38,13 @@ class ApplyVpnPeer implements ShouldQueue
     }
 
     public function handle(WireguardHub $hub): void
+    {
+        // Shared with SyncVpnPeers so a reconciliation cannot read the hub
+        // around this mutation and then act on what it saw.
+        Cache::lock('wireguard:hub', 30)->block(10, fn () => $this->apply($hub));
+    }
+
+    private function apply(WireguardHub $hub): void
     {
         // Read the CURRENT desired state rather than the one captured at
         // dispatch time. A retried block job would otherwise undo an unblock

app/Provisioning/Jobs/SyncVpnPeers.php

@@ -5,6 +5,7 @@ namespace App\Provisioning\Jobs;
 use App\Models\Host;
 use App\Models\VpnPeer;
 use App\Services\Wireguard\WireguardHub;
+use Illuminate\Support\Facades\Cache;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldBeUnique;
 use Illuminate\Contracts\Queue\ShouldQueue;
@@ -39,6 +40,26 @@ class SyncVpnPeers implements ShouldBeUnique, ShouldQueue
     }
 
     public function handle(WireguardHub $hub): void
+    {
+        // Reading the hub and reconciling must not straddle a peer removal: a
+        // snapshot taken before ApplyVpnPeer removes a peer and purges its
+        // tombstone would otherwise be adopted afterwards as a fresh, enabled
+        // access — restoring exactly what was revoked. ApplyVpnPeer holds the
+        // same lock, so read and mutation are mutually exclusive.
+        $retryRemoval = [];
+        Cache::lock('wireguard:hub', 30)->block(10, function () use ($hub, &$retryRemoval) {
+            $this->reconcile($hub, $retryRemoval);
+        });
+
+        // Dispatched outside the lock: with the sync queue driver this executes
+        // inline, and ApplyVpnPeer takes the very same lock.
+        foreach ($retryRemoval as $publicKey) {
+            ApplyVpnPeer::dispatch($publicKey, null, false);
+        }
+    }
+
+    /** @param list<string> $retryRemoval collected, dispatched by the caller */
+    private function reconcile(WireguardHub $hub, array &$retryRemoval): void
     {
         $snapshots = $hub->peers();
         $now = now();
@@ -56,7 +77,7 @@ class SyncVpnPeers implements ShouldBeUnique, ShouldQueue
             // 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);
+                $retryRemoval[] = $publicKey;
 
                 continue;
             }

tests/Feature/Admin/VpnTest.php

@@ -321,3 +321,19 @@ it('never re-adds a peer whose row is gone, however stale the job', function ()
     expect($hub->peerIps())->toBe([])
         ->and(VpnPeer::withTrashed()->count())->toBe(0);
 });
+
+it('serializes hub reads against hub mutations', function () {
+    $hub = vpnHub();
+    $peer = VpnPeer::factory()->create();
+    $hub->addPeer($peer->public_key, $peer->allowed_ip);
+
+    // A removal in flight holds this lock. A sync must wait for it instead of
+    // acting on a snapshot taken around the mutation.
+    expect(Illuminate\Support\Facades\Cache::lock('wireguard:hub', 1)->get())->toBeTrue();
+
+    $started = microtime(true);
+    (new SyncVpnPeers)->handle($hub);
+
+    expect(microtime(true) - $started)->toBeGreaterThan(0.8)
+        ->and($peer->fresh()->present)->toBeTrue();
+});