saveServer() wrote host/port/encryption without touching any mailbox, so
the console kept showing every mailbox as verified against a server
nothing had tested since. Mirrors EditMailbox::save()'s existing guard on
a single mailbox's own identity, just at server scope: only clears when a
field actually changed, so re-saving unedited values leaves a real
verification alone.
Both places now delegate the actual clear to new Mailbox::
invalidateVerification()/invalidateAllVerifications() methods rather than
touching last_verified_at directly, sharing the mutation. The "did this
change" comparison stays separate in each caller - one diffs a loaded
model's attributes, the other diffs persisted settings against incoming
scalars, and forcing them through one function would add machinery no
actual duplication justifies.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
savePurposes() only checked that each key was a nonempty string, so a
stale or tampered mailbox key (reachable by posting straight to
/livewire/update) saved successfully while MailboxResolver silently
returned null for it. Every purpose's key must now name a mailbox that
exists; system's must additionally be active, since every unmapped or
inactive OTHER purpose falls through to it.
EditMailbox::save() also cleared last_verified_at only when the
password changed, but smtpUsername() depends on address and username
too (falling back to address when username is blank) — changing
either while leaving the password untouched kept showing a successful
verification for an identity nothing had actually tested.
I3: nothing stopped an operator unchecking Aktiv on the mailbox that
mail.purpose.system points at. MailboxResolver::active() filters an
inactive mailbox at BOTH the direct lookup and the system fallback, so
that one checkbox could take mail.purpose.system to null and every
purpose without its own mapping down with it — the existing
system_required guard on savePurposes() reads as if this were covered,
but it only blocks the empty-mapping path, not this one.
EditMailbox::save() now refuses active=false against the mailbox 'system'
currently points at, the same way savePurposes() already refuses to leave
'system' empty.
I4: Admin\Mail::saveServer() writes mail.host — the platform's outbound
relay for every purpose mailbox at once — and CloudReady::toMail() puts a
customer's Nextcloud admin password in cleartext in the message body. So a
signed-in mail.manage session was already a credential-interception
primitive, the same threat Admin\Secrets' second gate exists for. Both
saveServer() and EditMailbox::save() (only when a new password is actually
being typed) now also require passwordRecentlyConfirmed(), following the
ConfirmsPassword pattern Secrets.php and Settings.php's two-factor actions
already use — including their per-action UI shape (an inline confirm form
scoped to the gated action), not a whole-page lock, so address, display
name, username, no-reply, active and the purpose mapping stay reachable
without confirming, per the brief's capability split.
Both gated and ungated paths are covered by tests, and both guards were
mutated away and confirmed to fail before being restored.
The console announced an update for "spätestens um 15:21" while the clock on
the wall said 17:21. Not a formatting slip in one string: fourteen views and
four components printed database values straight out, so every absolute time in
the product was two hours out all summer and one hour out all winter.
Two of those views looked as though they had handled it. They called
->timezone(config('app.timezone'))
which reads like "convert to local time" and, app.timezone being the STORAGE
zone that must stay UTC, converts to UTC — a no-op wearing the costume of a
fix, which is worse than no call at all because it stops the next person
looking. That exact string is now banned by test.
Worse than the labels were two FORMS. Maintenance windows and plan versions
filled their datetime-local fields from UTC and parsed what came back as UTC. A
datetime-local field carries no offset — it is the digits a person reads off
their own clock — so an operator typing 21:00 scheduled a window for 23:00
their time, and nothing anywhere said so. App\Support\LocalTime now holds
toField() and fromField() side by side, because getting one end right is not
half a fix, it is a fresh bug.
The mechanism is one Carbon macro, ->local(). It copies before converting:
Illuminate\Support\Carbon is mutable, so without that, rendering a timestamp
would rewrite the model attribute as a side effect and anything comparing or
saving it afterwards would be an hour or two out. Both Carbon classes get the
same body — Carbon keeps macros in one global table, so the second registration
replaces the first for every Carbon class, and two different bodies meant the
immutable version, safe for itself, silently became the mutable one's
implementation too. My own test caught that.
The existing tests had not caught any of this because they built their expected
values with the same wrong call the views used — a test that recomputes the
implementation asserts nothing. They now assert the wall clock, and the queued
update additionally asserts that the UTC time is NOT shown.
Recorded as R19 in CLAUDE.md and enforced by tests/Feature/DisplayTimezoneTest:
no Blade or Livewire component may format an absolute time without ->local(),
the UTC no-op is banned, storage stays UTC, both sides of the DST boundary are
checked, ->local() must not mutate, and a form field must round-trip to the
same instant.
APP_DISPLAY_TIMEZONE, default Europe/Vienna. 615 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The console has been unreachable over the VPN, and the cause was the readiness
probe rather than the tunnel. The gateway binds to the WireGuard hub address
alone; the probe asked 127.0.0.1, where nothing has ever listened. It could only
fail, so VPN_READY stayed false, and on that basis the application withheld the
`DNS = 10.66.0.1` line from every client config it issued. A phone then built
the tunnel, asked its normal resolver for the console hostname, got the public
address and had its connection closed — indistinguishable from "this site does
not exist", which is exactly what it looked like.
Probing over TLS on the bare address would not have worked either: the site
matches on the console's hostname, so a request without SNI is offered no
certificate. The gateway now answers a plain-HTTP health port on the hub
address, which removes TLS, SNI and name resolution from the question and
answers only what is being asked — is this gateway listening, in this network
namespace, right now. Caddy refuses to start when the certificate is unreadable,
so a health port that answers still proves the whole file loaded.
Also here, all found while looking:
- Icons pushed their label onto a second line and rendered a size larger than
asked for. Tailwind's preflight makes an svg display:block, and `.size-4` and
`.size-5` have equal specificity, so stylesheet order decided — and it emits
size-4 first. Every icon written as 16px was silently 20px. Recorded as R18.
- Four error pages printed `errors.404.hint` in place of a sentence: the lang
files give those codes a null hint and Laravel returns the key for a null line.
- The Developer role had no label in either language, so the dropdown showed
`admin_settings.role_developer`.
- The secrets area held one key, which is not worth a password gate. It now
carries the credentials that actually stop the business when they expire —
DNS, monitoring, SMTP — and the test button appears only where a checker
exists, instead of reporting on Stripe whatever was being looked at.
- The update button said nothing about when a queued run would start or where a
running one had got to; both are now shown, and a failure names its step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The owner's phone shows a blank page on the VPN. That page is the reverse proxy
refusing the request, and it refuses because the phone is not on the tunnel: the
client config routes only the management subnet, so a request to the console's
public hostname leaves over the mobile network and arrives from a carrier
address. It is also why the peer reads "last contact: never" — WireGuard only
handshakes when it has traffic to send, and nothing is ever routed in.
The obvious fixes are both wrong. Adding the server's public address to
AllowedIPs routes the WireGuard endpoint into the tunnel it is trying to
establish. A public DNS record pointing at 10.66.0.1 publishes the internal
subnet to anyone enumerating the domain — the owner raised that himself, and he
was right.
So the console is served INSIDE the tunnel instead. A small resolver and a small
gateway share the hub container's network namespace, which is where wg0 lives,
and answer on 10.66.0.1 directly. A client therefore needs no route beyond the
subnet it already has, the host's docker bridge is never exposed, and the
request reaches the application with its real 10.66.0.x source — which is what
the console's own allowlist checks. The gateway reuses the certificate the
public Caddy already renews: a certificate is bound to the name, not to the
address serving it, so nothing new is issued and nothing internal reaches a
public zone.
Most of this commit is the arithmetic of not lying about it. Review found
fourteen ways the arrangement could report itself working while it was not:
enabling the compose profile after the services were started rather than before;
starting a gateway with empty certificate paths, which can only crash; treating
"container created" as "container running"; carrying a stale readiness marker
through a failed restart; reusing the previous hostname's certificate after a
rename; leaving the profile enabled when the hostname was cleared, in a script
that exits early precisely when nothing else would fix it; reading the renewal
timestamp as a user who cannot traverse Caddy's storage; and a `find | head`
that aborts the whole installer under pipefail the moment two issuers hold a
certificate for the same name.
One of them was mine and worth naming: to let the container read the
certificate, I had made the TLS private key world-readable. On a multi-user host
that hands the console's identity to every local account.
The client is told about the resolver only when both services are confirmed
running, because a config naming a resolver that does not exist takes the
device's entire name resolution with it for as long as the tunnel is up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The page shown while the site is hidden was still in the old style — a coloured
square and system fonts — on the two hostnames a visitor is most likely to try.
It now uses the site's own plate, typeface and registration marks, still
entirely self-contained because it is shown precisely when the asset build may
not exist. Both it and the error pages lighten the accent in dark mode: the tone
is chosen for contrast against paper, and on the ink plate the small text is the
first thing to stop being readable.
The VPN investigation ended somewhere useful, and not where it started.
The blank page on the phone is the reverse proxy refusing the request — `respond
404` sends no body. It refuses because the phone is not on the tunnel: the
client config lists only the management subnet in AllowedIPs, so a request to
the console's public hostname goes out over the mobile network and arrives from
a carrier address. It also explains "last contact: never", since WireGuard only
performs a handshake when it has traffic to send, and nothing is ever routed
into the tunnel.
The obvious fix — add the server's public address to AllowedIPs — is written
down here as a comment and a test rather than as code, because it cannot work.
The WireGuard endpoint is that same address, so routing it into the tunnel
routes the handshake packets into the tunnel they are trying to establish. The
result is a loop and a connection that never comes up: the same blank page, now
with no way in at all.
Reaching the console over the VPN needs an address INSIDE the subnet — the proxy
answering on the hub address, and a name that resolves to it. That is a
deployment change, not something a config line can express.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Changing a key meant a shell, a file edit and a cache rebuild — and the person
who owns the Stripe account is not necessarily the person who owns the server.
Two gates, not one. The capability decides who may open the page; every operator
has console.view, and that must not mean "can read the payment key". The
password decides whether this SESSION may see or change anything, because the
realistic threat is not a stranger but an unlocked machine, and a session is
exactly what that hands over. Both are re-checked server-side on every action —
a Livewire action is reachable by anyone who can post to /livewire/update.
The value is stored encrypted under a key of its own, SECRETS_KEY, and the vault
refuses to work without it rather than falling back to APP_KEY: rotating APP_KEY
is ordinary maintenance and would otherwise destroy every stored credential,
discovered when Stripe stops answering. It is read where it is used, not
overlaid onto config at boot — an overlay adds a query to every request
including the public site, and leaves queue workers holding whatever was true
when they started. It is never shown again, only outlined, and never enters a
Livewire property that would carry it to the browser and back in the snapshot.
A registry, not an env editor: a form that can set any environment variable is a
privilege-escalation primitive, and one bad value bricks the installation with
no way back through that same form.
The test button is the part that matters. It reports which Stripe account the
key belongs to, whether it is LIVE or test — the most expensive mistake here is
pasting one where the other belongs, and both look identical in a form — and
which webhook endpoints exist with the events each subscribes to. A key can be
perfectly valid while the endpoint listens for the wrong five events, and
nothing fails until a payment goes unrecorded.
The webhook signing secret deliberately stays in .env. It is read on every
incoming payment event; in the database, a database problem becomes silently
failing signature checks.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three gaps, all of the "half-built" kind.
Nobody could change a password. Fortify's updatePasswords feature was switched
off, so an account created with a generated password kept it until someone
opened a shell on the server — including the owner's own. Both settings pages
have the form now, sharing one concern, because two copies of a password rule is
how one of them ends up weaker. It goes through Fortify's own action rather than
hashing here, and the current password is required: an unlocked machine should
not be two keystrokes away from locking its owner out.
The datacenter edit form had no active switch. The column and the scope existed,
and the list page has a toggle — but the form you open to change the thing did
not offer the one lifecycle action a location actually needs. Switching one off
now says what it does NOT do: existing hosts keep running. Once, on the actual
transition, and as the only message — the generic "saved" toast would otherwise
replace it, and an already-inactive location would have announced its own
deactivation every time its name was edited.
The staff table's actions cell is empty for your own row, because you cannot
revoke yourself. With a one-person team the column was therefore always blank
and read as broken rather than as a rule.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The console was unreachable, and not for the reason it looked like. Two causes,
both mine.
A failed `artisan optimize` — the one that hit duplicate route names — left a
broken route cache behind, so every request to the console host answered 500.
Rebuilt.
Underneath that: the reverse proxy has its own console allowlist, hard-coded,
and it runs BEFORE the application. Everything added on the console's own
access page was therefore ineffective, and when the owner's address changed
they were turned away by the proxy before reaching the page that would have
fixed it. The proxy now imports a fragment generated from the same list the
console manages, regenerated by the host agent and reloaded when it changes.
Getting that safe took most of the review. It refuses to rewrite an ambiguous
Caddyfile rather than replacing some other site's matcher; it never falls back
to a loopback-only list when the application cannot be reached, because that
list validates cleanly and locks out every remote operator; it retries a reload
that failed instead of assuming it worked; and an installation that upgrades
without rerunning the installer is told, because otherwise the whole mechanism
is invisibly absent.
Prices are entered in euros. The form asked for cents, so €799 was typed as
79900 and one slipped digit was a factor of ten on an invoice. Conversion
happens in one place, on the string rather than through a float — (float)
'79.90' × 100 is 7989.999… and casting truncates to 7989, one cent short on
exactly the prices people charge — and it refuses an amount the column cannot
hold instead of failing at the database.
Every error code has a page now, in the site's own language and typeface,
self-contained so it still renders when the asset manifest is the thing that
broke. There was only a bare white 404.
The VPN list stops being a seven-column table nobody could fit: names broke
across two lines and so did the headings. These are attributes of one access,
not quantities compared down a column, so each access is a row — identity on
one line, measurements in mono on the next.
The status page says what it measures rather than what it promises. "New orders
are delivered without failures" read as a marketing claim on a page whose only
job is to be believed, and said nothing about the last 24 hours.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three things the owner asked for, and one the review found underneath them.
The console leaves /admin. It has its own route file now, registered before
routes/web.php because once it has a hostname to itself it sits at the ROOT of
that host — where `/` and `/settings` also exist for the portal, and the first
matching route wins. Where it mounts is one decision in one place: `/` on the
console hostname where it has one to itself, `/admin` on any host otherwise.
Names stay admin.* in both modes, so nothing that builds a URL has to know.
Exclusivity is its own switch, not a consequence of having a hostname. The
first attempt made "this host is the console" follow from ADMIN_HOSTS, and a
development machine lists its own IP there so the console works without DNS —
which took the public site and the portal off that machine entirely. The
switch also registers the console on every listed hostname, canonical last, so
the alternates that exist as recovery paths keep working.
The status page moves out of /legal, where it sat beside the imprint and the
terms. Nothing about the current health of the platform is a legal document. It
is a real page now: portal, instances, provisioning and backups, each derived
from records, aggregate only, and a component with no signal reports "unknown"
rather than "operational".
That last rule is what exposed the real bug. monitoring_targets.status was
written once — 'up', at provisioning — and nothing ever updated it. Both the
console's notices and the new public page read it, so an outage would have been
published as healthy indefinitely. There is a sync job now, on a five-minute
schedule, and a checked_at column so a verdict can go stale instead of standing
forever.
The monitoring contract had to grow a third state for that to be honest.
isHealthy() answers true when no monitoring is configured and false when the
monitor is unreachable; recording that boolean would have published either a
fleet-wide all-clear nobody measured or a fleet-wide outage that was really one
broken monitor. health() returns null for both, the recorder leaves the old
verdict to go stale, and isHealthy() keeps its forgiving semantics for the one
caller that wants them — the provisioning acceptance check, which must not fail
a delivery because monitoring is not set up.
Backups are counted from the instances that need protecting rather than from
the backup rows that exist, so an instance with no schedule at all cannot be
missing from the arithmetic that declares the estate protected.
Also: the update panel polls itself, offers the button only when there is
something to install, and opens the log while a run is in progress.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The panel had no way to update the installation, and could not have one on its
own: it runs as www-data inside a container, and the update restarts that
container. A button that shelled out would kill its own web server
mid-response, and would need the application to hold host-level credentials —
a far worse thing to own than an out-of-date checkout.
So the button is a mailbox. The panel writes a request into the checkout; a
systemd timer on the host, running as the service account, consumes it, runs
deploy/update.sh and writes back what happened. The same timer answers the
question the panel cannot answer alone — is there anything to update — which
needs a git fetch, and therefore credentials the application does not have.
The parts that were wrong before review, all of which would have shipped as a
button that looks fine and does nothing:
- The agent was only installed by install.sh, which the installed base never
runs again. Every existing server would have shown the button permanently
disabled. It has its own root entry point now, install.sh delegates to it,
and update.sh says so when the unit is missing.
- On a release-pinned server the checkout is detached, so it followed
"origin/HEAD" and then ran an update that deliberately stays on the pinned
tag: updates advertised, nothing applied. It now compares TAGS in release
mode and passes the target release through.
- On a branch other than main it advertised that branch's commits and then
deployed main's, because update.sh defaults to main.
- A request written while the agent was down was executed whenever the agent
next started, days later.
- A single status file meant the routine five-minute check overwrote a failed
update with "idle" before anyone saw it.
- An agent that had been stopped left the button enabled forever, because a
status file written once counted as an agent.
- The agent's error messages were German strings rendered into an English
interface; it reports codes now and the panel translates them.
Also: the add-address button in the console-access panel was as tall as the
field plus its hint, because the hint was a sibling inside the flex row.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three console pages were fiction. The front page claimed 42 customers, 39
instances, four hosts named pve-fsn-1..3 and €7,842 a month, over a twelve-month
growth curve; the instance list held seven invented machines; the revenue page
reported churn and a trend for a business with no recorded history. All of it
was hard-coded. It read like a running company and measured nothing.
They now read the database. Two figures are gone rather than approximated —
the revenue trend, which needs a monthly history nobody records, and churn,
which needs a base the data cannot supply. ARR stays, labelled as the
projection it is. The green "all systems normal" badge is computed from the
notice list instead of asserted, and the notices themselves come from failed
runs, hosts reporting errors or gone quiet, and monitoring that is down.
Host load is the one number that had to agree with something else: placement
counts the VM disk allocation, ignores a failed instance that never got a VM,
and subtracts the host's reserve. A dashboard doing its own arithmetic would
show a host as comfortable while orders were already being refused on it, so it
uses the host's own accounting — with the filter moved into a scope both share,
and the sum preloaded so listing hosts stays one query.
The instance list drops the Nextcloud version column: that version is not
recorded anywhere, and a column filled with a plausible number is worse than no
column. Statuses the lifecycle writes but nobody had translated no longer
render as "admin.status.failed".
The price sheet also gains the included traffic, which the catalogue has always
carried and the page simply never printed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The allowlist is managed in the console, which is fine until the address you
manage it from changes — and then the page that would fix the problem is the
page the problem blocks. Every gate needs a door that does not depend on
itself, and on a server that door is a shell:
php artisan clupilot:console-access show
php artisan clupilot:console-access allow 203.0.113.7
php artisan clupilot:console-access open
The address check moved to RestrictConsoleNetwork::isNetwork() so the command
and the console apply the same rule. Codex caught the version that did not: a
typo like 203.0.113.9/99 was stored, reported as success, and matched nothing —
leaving whoever was recovering still locked out, now believing they were not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RestrictAdminHost answers under which NAME the console responds — and the
caller picks the Host header, so it can never answer who is asking. This does:
a network gate on the client address, which behind a trusted proxy is not
something the client chooses.
The management VPN is always in the list and cannot be removed — it is the one
way in that survives a bad entry. Beyond that the owner keeps their own
addresses in the console, because being away from the VPN should not mean being
locked out, and the person who needs to change that list is the person sitting
in front of it.
Two refusals rather than warnings, since by the time a warning renders the
request that would show it has already been rejected: switching the restriction
ON is refused unless the address doing the switching is already covered, and
removing the entry you are sitting behind is refused. Entries are validated as
an address or CIDR — a typo that matches nothing is how someone locks
themselves out while believing they have not.
404, never 403.
Registered as persistent Livewire middleware, and verified in a browser that it
holds there: with the settings page open, narrowing the list turned the next
action from 200 into 404. Codex read the path guard as skipping
/livewire/update; Livewire in fact replays middleware against a duplicate of
the request carrying the original component's path, so the guard matches and
the client address is preserved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The owner can now do from the console what only a config edit could do before:
create a plan line, draft a version, price it, publish it into a window, and
pull a plan out of the shop.
Two pages and one modal, because the catalogue has exactly two levels. The
plan list is a name, a rank and a kill switch. Everything that can be got
wrong — capabilities, prices, windows — lives on the versions, where the page
is built around the one rule that matters: a draft is freely editable, a
published version is not touchable at all. So drafting and publishing are
separate acts, and publishing says plainly that it is final.
The console refuses everything the catalogue refuses, on the form rather than
as a stack trace: an overlapping window, a window that ends before it starts,
a version with no price for a term we sell on, and — new here — a version with
no VM template, which would be bought and then fail provisioning every time.
Numbers are bounded on both sides, because a mistyped price otherwise
overflows the column and answers with a 500 instead of saying which field was
wrong. Performance class and features are picked from the keys we have labels
for; free text would be frozen at publication and shown to customers as a raw
translation key forever.
Concurrency, since two admins share one catalogue: draft numbers are allocated
under a lock on the family, publication is an atomic conditional claim, and
discarding a draft is one statement conditional on it still being a draft —
otherwise a draft published in the meantime could be deleted out from under
the customers now contracted to it.
`plans.manage` (Owner and Admin) guards the pages themselves, not only the
buttons, and the modal authorises itself — prices, drafts and unreleased plans
are not something to leave readable to anyone who types the URL.
Also fixes the shared checkbox, which ticked in the browser's own blue on
every page that used one: a native checkbox ignores text colour and needs
accent-color.
Verified in the browser: withdrawing a plan in the console removes it from the
customer's billing page immediately, and restoring it brings it back. The
public landing page carries no catalogue-driven plan list, so there is nothing
there to hide.
421 tests green. Codex review clean after six rounds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Plans lived in config/provisioning.php, where the owner cannot reach them and
where a plan has no history. They now live in plan_families / plan_versions /
plan_prices — a name that never moves, what the plan WAS at a point in time,
and a price per version and term with its own Stripe Price id.
- Availability is computed on every read (available_from <= now < until,
half-open, UTC) plus a per-family sales kill switch. Nothing schedules a plan
into or out of sale: a job that fails to run is a plan that silently
misbehaves.
- Overlaps crash rather than resolve. currentVersion() uses sole(), and
scheduling takes a lock on the family and rejects an overlapping window —
two versions on sale at once would decide a customer's price by row order.
- A version is frozen from publication, not from first sale, and so is its
price: a checkout is not instant, and an amount edited between the session
opening and the webhook landing would contract someone at a price they were
never quoted. Repricing publishes a new version, as Stripe requires anyway.
- Neither a published version, its price, nor a family with customers can be
deleted, and a family key cannot be renamed. All of those would null the
provenance off existing contracts.
- Subscriptions and orders record plan_version_id. A historical reference
resolved by plan NAME hands back today's terms, which is the split-brain one
level up. A checkout that carried its version is honoured even after the
window closes — they paid for what they were shown.
Switched atomically and failing closed: config('provisioning.plans') and
plan_features are gone, and nothing falls back to them. A fallback would
resurrect a plan the owner had just switched off. The seed lives in the
migration, and PlanCatalogueTest pins what the config catalogue sold as the
shadow comparison. `php artisan plans:check` reports overlaps, gaps and
missing prices before a customer finds them.
Contract-backed displays, which were reading the live catalogue:
seat limits, the cloud card, the plan card, and admin MRR — the last three now
divide a yearly contract down via Subscription::monthlyPriceCents(), since all
three label the figure per month.
Two recovery gaps closed along the way: the order now commits before the
contract is opened (so a contract failure can never erase the record of a
payment), a Stripe retry repairs a missing contract and restarts a run stranded
with no_subscription, and TickProvisioning sweeps runs left pending by a crash
rather than only running/waiting ones.
402 tests green. Codex review clean after thirteen rounds. Verified in the
browser: portal, billing and console render unchanged off the new catalogue.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The handoff stayed in the cache for its full ten minutes, so any replayed
component request could fetch the plaintext again — "shown once" was a figure
of speech. It is consumed on the first read and the token dropped; the payload
reaches the view and nothing else.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
An access created without storing its config had no download button and no
explanation — it looked broken. It now shows a dimmed button that says why
(the private key only exists on your device), and every staff access gets
"Re-issue": a new keypair, old key off the hub before the new one goes on,
so two peers never claim the same tunnel address at once. A stored config is
re-encrypted in step, or the owner would download a key the hub no longer
accepts.
Host peers are excluded: their key belongs to the machine's own wg0, and
swapping it here would cut the host off with nothing left to repair it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alpha_dash accepted eu_west, -edge and edge-, none of which are valid DNS
labels — every host in such a datacenter would have failed registration and,
because DNS is non-fatal, silently ended up without a name. The console now
requires a proper label, and the step normalises anything created before that
rule so existing rows still get a usable name.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
While the product is still being built, the marketing site and the customer
portal should not be reachable — but they must stay reachable for us.
- A toggle in the console (site.manage, Owner/Admin) stored in a new
app_settings table, because this has to be flippable without a deploy.
- Outsiders get a placeholder with 503 + noindex, not 200: a 200 invites search
engines to index the placeholder as the site's content, which is far harder
to undo than to prevent.
- Anyone on the management VPN, and any signed-in operator, sees the real site.
The console, Livewire's endpoint, the Stripe webhook and the health check are
always reachable — otherwise the switch could only be flipped once.
- robots.txt is generated by the app and follows the switch. It had to stop
being a static file: nginx short-circuited it and Laravel's stock file said
'Disallow:' with an empty value, so crawlers were never told anything.
- Settings reads fall back to the default when the table is unavailable. The
gate reads one on every request, so a deploy running new code before migrate
would otherwise answer the entire site with a 500.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The action labels wrapped to two lines and crushed the column — now icon
buttons with tooltips, like every other admin table. The form column was too
narrow for its content: the datetime fields silently clipped their own value,
so they are stacked and full width now, with duration chips because typing an
end timestamp by hand is the fiddliest part of the form. Host rows show a real
selected state and a per-datacenter count, and the impact column says
"1 Host · 1 Kunde" instead of "Host(s) · Kunde(n)".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Encryption and the write ran after the transaction had committed and the peer
had been dispatched to the hub. A failure there left a live access whose config
was never stored — unrecoverable, and the operator would create a second one
not knowing why. Both now happen in the same insert; nothing is dispatched
unless it succeeded.
Re-verified in the browser afterwards: create with storage, wrong password
refused, right password reveals the config.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The download counter was read-modify-written, so two concurrent retrievals
recorded one. It is an audit trail — under-reporting is the failure mode it
must not have.
The creation-race test broke when issuing moved into a transaction: its
simulated competitor writes inside our transaction, so the rollback removes
that row too. It now asserts what actually matters (the loser is told, and
creates nothing) and states what a single-connection sqlite test cannot show.
This test was red in the previous commit — my mistake for committing before
reading the run.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A revocation committing between the operator check and the insert would find no
peer to remove and still leave the revoked colleague with a brand-new tunnel.
Issuing now runs in a transaction that locks the owner row — the same row
revokeStaff() locks — so the two cannot interleave.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Creating a second access with a supplied key skipped the config branch, so the
first access's private configuration stayed on screen under the new name — and
someone would have handed it out.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Once revealed, the plaintext sits in the handoff cache for ten minutes, and the
modal read it from the token alone. A former owner could keep pulling the key
out of an open modal after revocation — the exact window purgeSecret() exists to
close. Every request now re-checks the policy and drops the handoff when it no
longer holds.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A worker could pick the job up mid-transaction, still see the peer as active,
leave it on the hub — and never be asked again, because the committed state is
only a soft delete. A revoked colleague would keep their tunnel until some
later reconciliation noticed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reworked after a design consultation with Codex, which pushed back on my first
proposal in three useful ways.
Ownership and rights:
- vpn.manage split into vpn.view.all and vpn.manage.all. Seeing an access is
not managing it, and neither is holding its private key.
- Record-level rules live in VpnPeerPolicy, not in permissions: an access
belongs to a person, and ownership is what grants sight of it. Every operator
reaches the page, but sees only their own unless they may see all.
- Issuing an access is not self-service — it reaches the management network, so
it needs vpn.manage.all even for oneself.
- New Developer role: sees everything, manages nothing. Writing code does not
imply authority over other people's access.
- kind (staff|host|system) replaces a null user_id that had to mean two
different things at once.
Config storage, opt-in per access:
- Downloading is owner-only — explicitly NOT for view.all or manage.all. An
admin who needs access revokes this one and issues their own, which keeps the
record of who holds what honest.
- The password is asked on EVERY retrieval, rate-limited. Laravel's
password.confirm keeps a 15-minute session stamp, which would authorise
unlimited later downloads from an unattended browser.
- Stored under VPN_CONFIG_KEY, not APP_KEY: a leaked application key must not
also hand over the management network. Purged on revocation and when a staff
member is revoked — console taken away, tunnel left open was the worst case.
- The plaintext never enters a Livewire snapshot (the page polls every five
seconds); the component carries an opaque handle instead.
Also: copy button works without HTTPS again (navigator.clipboard only exists in
a secure context, so over http it silently did nothing), plus config download
as a file and a QR code for the mobile apps.
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>
Two more from Codex:
- Addresses come from one subnet but live in two tables (hosts.wg_ip and
vpn_peers.allowed_ip), so a concurrent host onboarding and console creation
could each see the same address as free and both insert — neither unique
index sees the other. Creation now waits on Cache::lock('wireguard:allocate'),
the same lock ConfigureWireguard already holds while reserving a host address.
- mount() runs once, so revoking vpn.manage left an open tab polling every five
seconds and still receiving fresh peer state. Authorization now also runs on
hydration, which is what the poll goes through.
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>
- ResolvesCustomer trait replaces 5 duplicated customer() lookups; portal actions
now SAY why nothing happened (operator accounts have no Customer) instead of
returning silently — the actual cause of 'dead buttons', '0/0 seats' and
'settings not saving' when signed in as admin
- portal layout: explicit notice for a login without a linked customer
- /cloud: per-instance maintenance badge + window details; seat note 'owner = seat 1'
- maintenance form: sectioned card, hints, placeholder, styled datetime fields,
grouped host picker with per-datacenter select-all + selection counter
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- isOperator()/EnsureAdmin/broadcast fall back to is_admin so a legacy admin is
never locked out by a stale permission cache
- published modal: centered max-w-lg card (dynamic modalWidth class was purged
by Tailwind → full-width)
- datacenter edit moved to a modal (no row-height jump); location is now a
country dropdown (config/countries.php) instead of free text
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nexxo
Claude