Two from Codex:
- The job accepted any stored instance with a host. A closed one's VMID can have
been reused on the same machine, and the reset would then land on another
customer's VM. It now requires a live instance, and the action is not offered
for anything else.
- An unreachable Proxmox or guest agent throws rather than returning an exit
code, so nothing was ever written to the handoff and the modal polled forever.
The throw is caught, and a failed() handler covers anything that still escapes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Impersonation borrows the customer's portal session; this is access to their
installation, which is a different thing and now a different button.
Nextcloud has no passwordless admin jump, so this does the only honest thing it
offers: it resets OUR managed admin account inside that installation and hands
the credentials over once. The customer's own accounts are untouched, and the
next request sets a new password again.
- New capability instances.adminlogin, Owner and Admin only — stronger than
impersonation, because it hands over control rather than a session.
- The operator's own password is required every time, rate-limited: taking over
a customer's installation is not something an unattended browser should manage
on its own.
- The reset runs on the provisioning worker (it owns the tunnel and the Proxmox
credentials); the console polls a handoff token, so the credentials never
enter a Livewire snapshot.
- A silent instance is reported as such instead of appearing to succeed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Once a key is free, someone can legitimately create a new access with it. The
forced revocation looked the key up and would have deleted that person's access
and pulled their key off the hub. It now acts only on an adoption artifact — a
system peer with no owner and no creator — and leaves anything else alone.
Re-issuing also threw when VPN_CONFIG_KEY was missing, which is exactly the
situation the console tells people to fix by re-issuing. It now hands the new
config over once instead of storing it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex found that a sync landing between the key swap and its removal would see
the old key as unknown. Writing the test showed something worse than the
duplicate he predicted: the adoption cannot even happen — the address still
belongs to the live access — so the insert violated the unique index and took
the ENTIRE reconciliation down with it. One stale key would have stopped every
peer's state from updating.
The sync now recognises that case and queues the removal instead of adopting,
and a key with no row of its own is revoked with force: it can only ever be
removed, never kept, so the removal cannot be talked out of it by a row that
should not exist.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A registration finishing while the purge waited on the run locks wrote
dns_record_id after the purge had already loaded the host, so it skipped the
deletion and then deleted the row holding the only id.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Swallowing it and carrying on deleted the row that held the only reference to
that record, publishing the machine's management address for good. The purge now
fails instead — and is retried, since it re-reads the host and its other steps
are idempotent — so a broken DNS provider becomes a visible failed job rather
than a silent leak.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex was right that my never-reuse claim did not hold:
- The lock was released with only a candidate in hand, so two concurrent
onboardings in one datacenter could pick the same name and overwrite each
other's record. The name is now persisted on the host while still locked.
- The number was derived from the hosts that happen to exist, so removing the
highest one handed that number straight back out — a cached name would then
resolve to a different machine. The counter is stored per datacenter, floored
by what is actually in use so a wiped settings store cannot reissue live names.
- PurgeHost deleted the row that carried the record id, leaving the machine's
management address published with nothing left to clean it up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Defaulting missing netin/netout to zero reset the baseline, so the next real
sample added the entire cumulative counter again — a customer throttled for
traffic that was counted twice. Such a sample is skipped and logged now.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- update.sh restarted the long-lived services before clearing the caches, so
they booted from the old cached config and kept it for the life of the
process — the release's configuration never reached the workers.
- install.sh put the Gitea token in the clone URL, which is visible in the
process list to every local user while the clone runs. It goes through a
short-lived askpass helper now.
- The collector's uniqueness lock expired at exactly the collection interval,
so a delayed queue could run two collectors on the same baseline and count
the same delta twice — throttling a customer for traffic they never used.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tying the release to the creation of the new month's row meant a single failed
call was never retried: the row exists from then on, and the current period
looks unthrottled, so nothing could detect the stale NIC limit. A customer would
have stayed slow for a whole month they had paid for. Checked on every sample
now, with a test that makes the first release fail.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two from Codex:
- A throttled instance kept its NIC limit into the new month: the new period
row starts unthrottled, so neither branch of enforce() fired and nothing ever
took the limit off. A customer would have stayed slow through a month they
had already paid for.
- PublicSiteGate exempted livewire/* wholesale. That endpoint is shared by the
console and the portal, so a signed-in customer could keep driving portal
components — billing included — while the portal was supposed to be offline.
Operators pass the normal check anyway, which is all the console needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Customers can now see what they have used and what is left, and the service
slows down rather than stopping when the allowance is gone — a slow Nextcloud
gets a top-up, a dead one gets a cancellation.
- instance_traffic keeps one row per instance per month. Proxmox counters are
cumulative since the VM last started, so usage is the difference between two
samples, and a counter that went backwards means a restart, not a refund.
- Outbound is what counts: inbound is free at our providers and egress is what
Hetzner's 20 TB per server applies to.
- CollectInstanceTraffic samples every 15 minutes, warns once per threshold
(80/95 %) rather than on every run, and at 100 % limits the VM's NIC via
Proxmox — released again as soon as the customer tops up or the month rolls
over.
- The dashboard gets a full-width band (it throttles the service, so it is not
a tile among tiles) with the top-up offer right next to the warning.
- Bytes are formatted in SI units now: allowances are computed in SI, so
dividing by 1024 made a 1000 GB plan read as "931 GB".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codex spotted the migration labelling sync-adopted peers as staff with no
owner, which breaks the invariant the same migration introduces. The live sync
had the identical hole — it never set kind at all, so every adopted peer landed
on the 'staff' default. Both now use system for a peer nobody is behind, and
promote it to host once the link is known.
.env.example carried ADMIN_HOSTS twice; phpdotenv keeps the first, so a fresh
checkout would have got the list without localhost and 404'd its own console.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The FK only nulled host_id, so the adopted peer stayed enabled-but-absent: shown
as "wird angewendet" forever, and re-enabling it would have put the deleted
host's key and address back on the hub. PurgeHost now tombstones that row, which
hands it to the same revocation machinery as everything else.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- A host's wg_pubkey may not be in vpn_peers yet, so the duplicate check passed
and an operator could create an access with it. ApplyVpnPeer would then run
wg set with a freshly allocated address, rewriting that host's allowed-ip and
cutting CluPilot's management tunnel to a live machine. Creation now rejects
keys already held by a host, checked under the allocation lock.
- A sync landing between addPeer() and the step storing wg_pubkey adopted the
peer as "unknown", and a later sync filled in host_id but left the name, so
the page showed that host as unknown forever. The name is now filled in when
the link becomes known — only for sync-adopted rows, so an access an operator
named keeps its name.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- 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>
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>
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>
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>
The console can now create, block and remove VPN accesses, and shows who is
actually connected — traffic counters, source endpoint and last handshake.
Design notes:
- The web container has no wg0, so the page never talks to WireGuard. Live
state is copied in by SyncVpnPeers on the provisioning queue (the only worker
whose container owns the interface); the page polls the database and merely
nudges that job, throttled so many open tabs cannot flood the queue.
- enabled (operator intent) and present (observed on the hub) are stored
separately, so a sync can never quietly undo a block and drift stays visible
as "wird angewendet".
- The sync adopts peers the host pipeline registered, naming them after their
host — otherwise "who is on the VPN" would have blind spots.
- Keypairs are generated in PHP via libsodium rather than shelling out to
wg genkey (no interface in this container, and it keeps generation testable).
The private key is shown once and never stored.
- allocateIp() now also considers vpn_peers, which would otherwise be handed a
tunnel address a host already holds.
- vpn.manage is a new capability held by Owner/Admin only, and it hides the nav
entry too — a VPN access reaches the management network.
Verified end to end against the real hub, not just mocks: created an access in
the browser, connected with the generated config, watched the console flip to
"Verbunden" with real counters, then blocked it and confirmed the peer was
gone from wg0 and the client could no longer handshake.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- ConfigureWireguard checks setup command results and only records the wg_peer
resource after the handshake verifies, so a transient setup failure re-runs
the full setup instead of getting stuck on the idempotent path.
- Host removal now deactivates immediately and queues PurgeHost, which deletes
runs under the runner lock (waiting out a long step) — no 15s LockTimeout error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- PhpseclibRemoteShell sets a command timeout (default 2000s) so apt
full-upgrade/install don't hit phpseclib's ~10s default and fail.
- Host removal dispatches RemoveWireguardPeer to the provisioning queue, which
runs in the worker that owns wg0 (the web container can't manage WireGuard).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Dedicated 'provisioning' queue connection (retry_after 2400) + worker
(--timeout=2100); AdvanceRunJob tries=1, timeout=2100 on that queue.
- Run lock TTL raised to 2100s so a long step can't be run concurrently by a
duplicate tick-dispatched job.
- StartHostOnboarding creates host+run in a transaction, dispatches after commit.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nexxo