fix(vpn): lock every hub mutation; resolve duplicate keys inside the lock

- ConfigureWireguard and RemoveWireguardPeer mutate the hub too, and were not
  taking the lock, so a sync could still read the interface around them and
  write stale state. Both now hold wireguard:hub, which is what makes the
  read/mutation guarantee actually hold.
- The duplicate-key check ran before the allocation lock, so two concurrent
  creations with the same key both passed it and the loser hit the unique index
  as an unhandled 500. The check moved inside the lock, with the index kept as
  a caught backstop for any writer that does not take it — reproduced in a test
  by inserting a colliding row from within the allocation call.

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

app/Livewire/Admin/Vpn.php

@@ -7,6 +7,7 @@ use App\Provisioning\Jobs\ApplyVpnPeer;
 use App\Provisioning\Jobs\SyncVpnPeers;
 use App\Services\Wireguard\Keypair;
 use App\Services\Wireguard\WireguardHub;
+use Illuminate\Database\QueryException;
 use Illuminate\Support\Facades\Cache;
 use Livewire\Attributes\Layout;
 use Livewire\Attributes\On;
@@ -65,30 +66,45 @@ class Vpn extends Component
         $keypair = $ownKey ? null : Keypair::generate();
         $publicKey = $ownKey ? trim($this->publicKey) : $keypair->publicKey;
 
-        // withTrashed: a revoked peer still holds the unique key until the hub
-        // 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;
-        }
-
         $hub = app(WireguardHub::class);
 
         // Same lock the host pipeline holds while it reserves an address
         // (ConfigureWireguard). Addresses come from one subnet but live in two
         // tables, so neither unique index can catch the other's insert — the
         // shared lock is what keeps a host and an access off the same tunnel IP.
-        $peer = Cache::lock('wireguard:allocate', 30)->block(10, fn () => VpnPeer::create([
+        try {
+            $peer = Cache::lock('wireguard:allocate', 30)->block(10, function () use ($hub, $publicKey) {
+                // Inside the lock: checking before it would let two concurrent
+                // requests both pass and the loser hit the unique index as a
+                // 500. withTrashed, because a revoked peer keeps its key until
+                // the hub confirms removal.
+                $existing = VpnPeer::withTrashed()->where('public_key', $publicKey)->first();
+                if ($existing !== null) {
+                    return $existing->trashed() ? 'pending_removal' : 'duplicate_key';
+                }
+
+                return VpnPeer::create([
                     'name' => trim($this->name),
                     'public_key' => $publicKey,
                     'allowed_ip' => $hub->allocateIp(),
                     'enabled' => true,
                     'present' => false,
                     'created_by' => auth()->id(),
-        ]));
+                ]);
+            });
+        } catch (QueryException) {
+            // Backstop: the unique index caught a writer that did not take this
+            // lock. Report it like any other duplicate instead of a 500.
+            $this->addError('publicKey', __('vpn.duplicate_key'));
+
+            return;
+        }
+
+        if (is_string($peer)) {
+            $this->addError('publicKey', __('vpn.'.$peer));
+
+            return;
+        }
 
         ApplyVpnPeer::dispatch($peer->public_key, $peer->allowed_ip, true);
 

app/Provisioning/Jobs/RemoveWireguardPeer.php

@@ -3,6 +3,7 @@
 namespace App\Provisioning\Jobs;
 
 use App\Services\Wireguard\WireguardHub;
+use Illuminate\Support\Facades\Cache;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use Illuminate\Foundation\Bus\Dispatchable;
@@ -30,6 +31,9 @@ class RemoveWireguardPeer implements ShouldQueue
 
     public function handle(WireguardHub $hub): void
     {
-        $hub->removePeer($this->publicKey);
+        // Every hub mutation takes this lock so SyncVpnPeers cannot read the
+        // interface around it and then write what it saw — a peer removed here
+        // would otherwise be reported back as a live access.
+        Cache::lock('wireguard:hub', 30)->block(10, fn () => $hub->removePeer($this->publicKey));
     }
 }

app/Provisioning/Steps/Host/ConfigureWireguard.php

@@ -61,7 +61,9 @@ class ConfigureWireguard extends HostStep
             return StepResult::retry(20, 'bringing up wg0 failed');
         }
 
-        $this->hub->addPeer($publicKey, $wgIp);
+        // See RemoveWireguardPeer: hub mutations are serialized against the
+        // reconciliation that feeds the VPN console.
+        Cache::lock('wireguard:hub', 30)->block(10, fn () => $this->hub->addPeer($publicKey, $wgIp));
 
         // Store the pubkey now (so removal can always clean up the hub peer), but
         // record the wg_peer resource only AFTER the handshake verifies — the

tests/Feature/Admin/VpnTest.php

@@ -337,3 +337,50 @@ it('serializes hub reads against hub mutations', function () {
     expect(microtime(true) - $started)->toBeGreaterThan(0.8)
         ->and($peer->fresh()->present)->toBeTrue();
 });
+
+it('reports a lost creation race as a validation error, not a 500', function () {
+    // A hub whose allocation inserts a colliding row mid-flight: exactly what a
+    // concurrent request that skipped the lock would do between our check and
+    // our insert.
+    $key = Keypair::generate()->publicKey;
+    $hub = new class($key) extends FakeWireguardHub
+    {
+        public function __construct(private string $collidingKey) {}
+
+        public function allocateIp(): string
+        {
+            VpnPeer::create([
+                'name' => 'gleichzeitig',
+                'public_key' => $this->collidingKey,
+                'allowed_ip' => '10.66.0.90',
+                'enabled' => true,
+            ]);
+
+            return '10.66.0.91';
+        }
+    };
+    app()->instance(WireguardHub::class, $hub);
+
+    Livewire::actingAs(operator('Owner'))->test(Vpn::class)
+        ->set('name', 'verloren')
+        ->set('publicKey', $key)
+        ->call('create')
+        ->assertHasErrors('publicKey');
+
+    // Only the winner's row exists.
+    expect(VpnPeer::query()->count())->toBe(1)
+        ->and(VpnPeer::query()->first()->name)->toBe('gleichzeitig');
+});
+
+it('keeps host peer removal behind the same hub lock', function () {
+    $hub = vpnHub();
+    $hub->addPeer('HOSTKEY', '10.66.0.30');
+
+    expect(Illuminate\Support\Facades\Cache::lock('wireguard:hub', 1)->get())->toBeTrue();
+
+    $started = microtime(true);
+    (new App\Provisioning\Jobs\RemoveWireguardPeer('HOSTKEY'))->handle($hub);
+
+    expect(microtime(true) - $started)->toBeGreaterThan(0.8)
+        ->and($hub->peerIps())->toBe([]);
+});