Four templates, one layout. Two of them — the maintenance announcement and its
cancellation — still wore Laravel's markdown component, which brings its own
logo, its own button and its own footer: the product was sending mail that
looked like two different companies, one of which was the framework.
New: an order confirmation, sent when the payment lands rather than when
provisioning finishes. Those are minutes apart at best and can be much longer,
and somebody who has just been charged and heard nothing assumes the worst
about both the charge and the product. It deliberately promises no time for the
cloud itself — provisioning takes what it takes, CloudReady announces the end,
and a promised minute that slips is worse than no promise. Queued after the
transaction, never inside it, and never allowed to fail the webhook: Stripe
retries anything that is not a 2xx, and a retry would re-enter provisioning
over a mail server having a bad minute.
The finished-cloud mail now names what was actually built — address, plan,
storage, location — because somebody who ordered a fortnight ago is checking it
against what they bought, and that is a comparison rather than a sentence. Still
no credential in it.
Two tests earn their place. One holds every template on the shared layout, so a
new one written in a hurry cannot copy-paste its way back to the framework's.
The other proves the finished-cloud mail carries no password — asserted against
the data the mail is given and the HTML it renders, not against the template's
source, because the first version of that test tripped over the word in a
comment and would have passed a template that leaked the value through a
variable. It tested the prose, not the mail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Since 1.2.0 the agent has ended with a non-zero status on every tick, before
writing anything. The console showed a check that never finished and a
last-checked time frozen at the moment of the deployment, and the update button
could not be reached at all — the one mechanism that would have delivered the
fix.
Two instances of the same trap, both mine, both under `set -Eeuo pipefail`:
The tag count was `release_version_gt … && echo`. The LAST iteration is almost
always a tag that is not newer, so the loop ended non-zero, pipefail carried it
out of the pipeline, the command substitution inherited it, and set -e ended
the agent. `if` rather than `&&` is the whole fix: an if whose condition is
false still returns 0.
The newest tag came from `git tag … | head -1`. head exits after one line, git
takes SIGPIPE, pipefail does the rest. install-agent.sh has carried a written
warning about exactly this since it was written; I read it and wrote it anyway.
Both now live in deploy/lib/release.sh, because the reason neither was caught
is that no PHP test can reach a bash pipeline.
tests/Feature/ReleaseComparisonTest.php runs the real functions in a real
throwaway repository under the agent's own set -Eeuo pipefail, and asserts the
EXIT CODE as well as the answer — an answer nobody lives to read is not an
answer. The SIGPIPE case needs four hundred tags to reproduce, because a short
list finishes writing before head leaves.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ownership repair added in 1.1.1 sampled the owner of each top-level
directory and skipped the recursion when it already matched. node_modules was
owned by www-data; node_modules/.vite-temp, left behind by an earlier root
build, was not. So the repair walked past it, `npm run build` failed with
EACCES, and the deployment stopped in maintenance mode — on the release whose
whole point was that this could not happen.
A directory's owner says nothing about its contents. It now walks each tree
once with find and changes only the entries that are wrong, which is also
cheaper than the detect-then-chown-everything it replaces.
Verified against the shape that actually failed: a root-owned file inside a
www-data-owned node_modules, repaired.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Registration opened an account that could sign in immediately on an address
nobody had proved was theirs — and anybody can type a stranger's into a
registration form. Both halves were commented out: the Fortify feature and
MustVerifyEmail on the model.
`verified` sits on the whole portal group rather than on the pages that spend
money. An unconfirmed address is not a billing problem to be caught at
checkout; it is an account that may not belong to the person holding it, and
the servers, the users and the backups behind it are worth as much to whoever
typed the address as to whoever owns it.
The mail is a Mailable in this product's design rather than Laravel's
notification, which would arrive with the framework's logo, button and footer
as the first thing anybody ever receives from CluPilot. It also has to leave
through the SYSTEM mailbox like every other account mail, which the
notification path does not do on its own.
The link is signed and expires, and the address is part of what is signed — so
a link issued to a mistyped address stops working the moment the address is
corrected, rather than confirming an address nobody can read. Tests cover the
tampered link, the expired one, the corrected address, and one account trying
to confirm another.
The waiting page is ours (Fortify has views off) and offers the only two things
that move somebody forward: send it again, or sign out and register with the
address they meant. The resend is throttled, because otherwise it is a button
that makes the server send mail to an arbitrary address as fast as it can be
clicked. Somebody already confirmed is redirected away from it — the `verified`
middleware only guards the other direction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the device work: the warning mail now has somewhere to send people.
A warning whose only advice is "if this was not you, take action" names a
problem and hands back nothing to do about it.
Both sides get their own component rather than one shared with the guard passed
in. Operators and customers are two guards and two tables (R21) — a component
taking the guard as input is one forged property away from listing and ending
the other side's sessions. Every query in SessionRegistry is scoped by guard as
well as by id for the same reason, and three tests hold that line, because the
unscoped version looks exactly like a working feature while doing it.
TouchLoginSession keeps each row pointing at the session it describes. Both
sign-in paths raise the Login event BEFORE session()->regenerate(), so the row
is written against an id that is dead a moment later — and the once-a-minute
throttle would have held the correction back for the first minute of every
session, hiding the session its reader is sitting in. A changed id is therefore
never throttled.
A session counts as live only if the framework's row exists AND was touched
within the session lifetime. The join alone is not enough: Laravel collects
expired sessions by lottery, so on a quiet installation rows sit there for days
and the list would offer somebody a laptop they shut last month.
Ending the others goes through the product's own modal (R23) and leaves the
asking session alone: signing somebody out of the page they are using to sign
other people out is not what the button says.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sessions lived in Redis, where they cannot be enumerated — one key per session,
no index by user — so "where am I signed in" and "end that one" were not
answerable at all. The database driver keeps a row per session and makes ending
one a delete. It costs every currently signed-in person one forced sign-in,
once, which is why it is happening now rather than later.
The warning is only worth anything if it is rare, so the two obvious signals
are deliberately not used. Not the IP: it changes on every train, every café,
every mobile handover, and behind a VPN it is not theirs to begin with. Not the
user-agent: Chrome ships every four weeks and the string changes each time,
which would be twelve warnings a year per customer, describing nothing. A
device is a long-lived signed cookie holding a random token, and nothing else
is consulted. The honest cost — cleared cookies, a private window, a second
browser — is written into the mail itself, because a reader who thinks the
warning is wrong stops reading the next one.
Three things the tests found or hold:
The first device on an account is new and must not warn. Replying to somebody
creating an account by telling them they signed in from a new device is noise
on the one message that has to stay worth reading.
token_hash was globally unique, and the shared-browser test failed on it. Two
people on one machine share one cookie and must each own a device row against
it; the obvious repair — issue a fresh token for the second — would orphan the
first person's row and warn them about their own desk. Unique is now per
identity.
Recording a device may never block a sign-in. A safeguard that locks the owner
out when it breaks has stopped being one, so the listener logs and swallows,
and a test drops the tables to prove it.
Sessions are linked by a uuid carried inside the session payload rather than by
the session id, which Laravel rotates on every sign-in and every regenerate().
Laravel's own sessions.user_id cannot carry the link either: it is filled from
the default guard, while operators and customers are two guards and two tables
(R21) — operator 1 and customer 1 would each be offered the other's sessions.
Also lands the shared mail layout the rest of the templates will use: tables
and inline styles for Outlook, no image and no web font because half of all
clients block remote content, and the button in --accent-active rather than the
brand orange, which is 2.9:1 against white.
Not tagged: the session list is not built yet, and without a tag no server can
install this.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The password no longer travels through third-party mail servers or sits
forever in an inbox: it moves onto the instance (encrypted, same as
nc_admin_ref) and shows on the dashboard until the customer clicks
"Notiert", which deletes it for good.
The 500 on the VPN config download was never in the VPN code. storage/logs/
laravel.log was owned by root, mode 644, since 27.07 04:16 — the moment a
failed `artisan optimize` wrote its error there during a deployment. From then
on the application could not append to its own log: Monolog threw on every
attempt, and a throw while logging is a 500 on any page that logs, with nothing
written down to say why. The error page said "Er wurde protokolliert". It was
not.
It surfaced on the config download because that is one of the few pages writing
a log line on its way through — on success and on a wrong password alike. Pages
that log nothing were unaffected, which is exactly why it looked like a VPN
fault for days.
`docker compose exec` is root unless told otherwise, and every deployment
script relied on that default — including the agent's allowlist sync, which
runs every minute. docker/entrypoint.sh had it right all along and drops to
www-data for precisely these commands. Now so does everything else, help text
included: telling an operator to run artisan as root is how the file ends up
owned by root in the first place.
update.sh also repairs what an earlier run left behind, before its first
unprivileged step rather than after — a root-owned log file is not
self-healing, the page that trips over it is nowhere near the deployment that
caused it, and with in_app unprivileged a root-owned vendor/ would break the
next composer step outright.
tests/Feature/DeploymentRunsAsTheAppUserTest.php holds the line. It was checked
against the previous commit and finds all nine places that were wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Asked for directly: a commit on main must change nothing on a server, and only
a tag whose version is higher than the installed one may be installed.
The agent asked two different questions before this. A pinned server compared
tags; everything else counted commits on its branch — so a console following
main offered an update for every push, verified or not, and "Jetzt
aktualisieren" meant "take whatever is on the branch right now". Landing on main
and being released are different events, and only the second is a decision
somebody made.
It is one question now, regardless of what the checkout is attached to: is there
a tag whose version is higher than the deployed one? Compared with sort -V
against the version in the deployment manifest — not the VERSION file, which an
update moves before it migrates, so a run that failed in between would leave the
checkout claiming a version it never finished installing. A run always targets
that tag; the branch path is gone. A request that arrives with nothing to
install is answered rather than obeyed, before anything announces a run.
The console follows: the button is disabled with nothing released, says so in a
line rather than leaving a grey button to be guessed at, names the version it
would install rather than a count, and refuses the action server-side as well —
a Livewire action is a public endpoint, and one place a rule may not live is
only in the disabled attribute of a button.
VERSION becomes 1.1.0, to be tagged as the first release this mechanism can see.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Clicking "Einrichten" generated a secret and showed the QR with no exit:
leaving meant navigating away, which left an unconfirmed enrolment on the
account that nothing on the console ever displayed. cancelSetup() clears
it — it was never confirmed, so it never protected anything.
The page was also one card with four states stacked into it. Now each
state is its own panel, the enrolment step says which step it is and
carries numbered instructions beside the code, and the recovery codes get
their own card with a copy button instead of being a footnote under
whatever came before them.
The code was immutable, which made a typo permanent. It cannot be freely mutable
either, and not only because hosts.datacenter references it: each host's DNS name
was minted from it and registered at the provider, the machine's own hostname was
set from it as it was built, the counter handing out those numbers is keyed by
it, and every past order stores it as plain text with no foreign key to follow.
Renaming a code with hosts in it is not a rename — it is a datacenter called hel
full of machines called fsn-01.
So it is editable exactly while nothing references it, which is the case that
matters: a typo noticed shortly after creating one. Once a host or an order
depends on it the field is disabled and says what is holding it, with the
numbers. The lock is recomputed from the database on every save, never read back
from the property the browser returns.
Alongside it, an optional free-text field for the specific datacenter —
fsn-dc-15. A provider runs several within one location, on separate power and
separate uplinks, and two entries both reading "Falkenstein (fsn)" are not a
choice anybody can make. Nothing keys off it; the code remains the identifier.
Also renames the country loop variables in the edit modal. They were $code and
$name — the component's own properties — and shadowed them for the rest of the
file. Harmless while nothing below the loop read them, which stopped being true
the moment the code became an editable field above it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Checking for updates was a request written into the same mailbox as a real
update, collected by the same agent on the same systemd timer — so asking what
was new was answered on the next tick, and the console, having nothing else to
show, counted down to it. An operator who asked a question was told an update
would start in 3:44.
A systemd path unit now wakes the agent the moment the request file is written,
for a check and for a run alike. PathChanged, not PathExists: PathExists re-arms
as soon as the service goes inactive, so a request the agent could not consume —
it holds a lock for the length of an update — would restart it in a tight loop.
The timer stays as the fallback, and a drop-in disables systemd's start rate
limit, because two triggers on one oneshot service can otherwise wedge the unit
and leave neither of them able to run.
The countdown is gone from both the settings card and the maintenance overlay.
It could not be made honest: its target was recomputed on every poll, and where
the interval the agent reports is shorter than the timer actually installed on
that host it had already gone by, so the fallback moved it forward again each
time. It ran backwards four seconds and snapped back to a full minute, forever —
reported exactly that way. A queued run says it is starting; a queued check says
nothing beyond its badge, because a check starts nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
VpnConfigAccess::reveal() dereferenced Auth::guard('operator')->user()
unguarded, which is null when a modal's session has ended since it was
opened (a Livewire action reaches its component through /livewire/update
on its own, without the page's original route middleware). The same
unguarded pattern was in TwoFactorSetup, InstanceAdminAccess and Settings.
Add App\Livewire\Concerns\ResolvesOperator with currentOperator(), the one
place that resolves the operator and redirects to admin.login when there
isn't one, and use it everywhere the pattern occurred.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A blocked outbound port (587 filtered at a firewall, as on the live
server) left both MailboxTester and MailboxTransport blocking until
PHP's default_socket_timeout (60s) or the reverse proxy cut the
request — a 504 with no error the operator ever saw. Mail::build()
honours a 'timeout' config key by calling SocketStream::setTimeout()
(confirmed by reading MailManager::configureSmtpTransport() and by an
end-to-end run against a genuinely unroutable host), so MailboxTester
now passes 'timeout' => 10 through it. MailboxTransport builds its
EsmtpTransport directly rather than through Mail::build(), so it sets
the same call on the transport's stream, at 30s — a queue worker can
afford to wait longer than an operator watching a browser, and a
timed-out send is retried rather than lost.
The test button also now shows a visible "sending" state while the
request is in flight, matching the wire:loading span pattern already
used on admin/host-create's save button — previously only
wire:loading.attr="disabled" fired, leaving a greyed-out button with
no other sign anything was happening.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
guest:operator's authenticated-user redirect had no console-aware
override, so it fell to Laravel's own default: the named 'dashboard'
route, the portal. An operator revisiting admin.login while already
signed in bounced to the portal's own login instead — in exclusive
mode, to a 404, since dashboard isn't a console route there.
redirectUsersTo() now mirrors the existing redirectGuestsTo() override
right above it, keyed on the same AdminArea::isConsole($request)
condition, so the two read as a pair.
completeLogin() re-checks isActive()/isOperator() immediately before signing
in, not only at the password step. The two-factor challenge previously only
confirmed the operator row still existed (Operator::find() + a null check),
so disabling an operator or revoking every console role in the window between
the password step and the pending code did not stop the login from
completing — the realistic case is an owner locking someone out mid-login,
not a contrived one.
One check, in the static method both OperatorLogin's direct exit and
OperatorTwoFactorChallenge::verify() already share, rather than a second copy
in the challenge. completeLogin() now returns bool instead of void: false
means refused, and neither the guard nor the session was touched. Each caller
turns that into a ValidationException in its own vocabulary — 'email' /
auth.failed for the direct exit (already unreachable in practice, since
authenticate() just checked the same two conditions synchronously; kept for
defense in depth), 'code' / auth.not_an_operator for the challenge, since the
code entered may be genuinely correct and reusing "wrong code" would mislead.
The three sites that refuse to create or rename an operator onto a
customer's email (Admin\Settings::saveAccount(), ::inviteStaff(), and
clupilot:create-operator) all checked Customer::where('email', ...) as
a proxy for "does this address already have a portal login". A users
row with no matching customers row — an email changed on one side
only, or legacy/orphaned data — passed straight through: this dev
database already had one.
Extracted the three copies into Customer::emailTaken(), which checks
both tables directly, so the three sites cannot drift from each other
again.
The setup-route exemption already went through AdminArea::routeIs(),
which matches both admin.<name> and admin.via*.<name>. The logout
exemption next to it used a bare $request->routeIs('admin.logout'),
which matches only the canonical name — on a recovery hostname the
logout route is admin.via0.logout, so the check failed there and an
unenrolled operator's logout POST bounced back to enrolment instead
of being let through, on exactly the host that exists because the
canonical one is not working.
RequireOperatorTwoFactor exempted the whole of admin.settings while
compulsory two-factor was on, on the theory that it was "the page
where two-factor is set up". That component also carries staff
management, site visibility, network restrictions, account changes,
and the two-factor policy switch itself, so an unenrolled operator got
unrestricted access to all of that, not just enrolment.
Enrolment (secret, QR, confirm, recovery codes, regenerate, disable)
moves to its own route and component, admin.two-factor-setup /
Admin\TwoFactorSetup, reachable by every operator regardless of
capability. That is now the only page the middleware exempts besides
logout, and its redirect points there instead. admin.settings goes
back behind the policy like every other console page; only the policy
switch itself stays there, since only someone who already satisfies
it should be the one changing it.
EnsureCustomerActive used session()->invalidate() to sign a suspended
or closed customer out. In shared-host mode the console and the
portal keep their login key in one session, so invalidate() (flush()
+ migrate(true)) silently signed out an operator elsewhere in that
same browser too. Same fix as the console's own /logout route:
regenerate() gets a fresh session id without flushing attributes
belonging to another guard.
confirmPassword() already checked the right account per guard, but the
session marker it stamped, auth.password_confirmed_at, was one flat
key shared by every guard. In shared-host mode the portal and console
guards keep their login in the same session, so confirming a
customer's portal password left the marker in place for an
operator-only gate too. One private method now computes the key for
all three callers so they cannot drift apart again.
PublicSiteGate let anyone through on a bare Auth::guard('operator')->check(),
which stays true for a disabled operator's live session — disabling
someone does not sign them out, so it never stopped this gate from
showing them the real site indefinitely, even though EnsureAdmin
already treats them as inactive. Now requires the same thing EnsureAdmin
does: an active operator holding a console role.
Chosen to merely ignore an invalid operator session here rather than log
it out. This gate only decides what one request sees; logging out would
ripple into whatever else that session is doing (a concurrent console
tab, mid-task), a bigger side effect than a visibility check exists to
have. EnsureAdmin sets the precedent — it also rejects a disabled
operator per request without touching their session.
Customer::ensureUser() enforced uniqueness only within users, so a
customer provisioned (or registering publicly) with an operator's email
got a second, colliding identity for the same address — the exact thing
R21 exists to prevent. Checks Operator by address now, not by session
(the deleted assertNotAdmin() checked who was signed in, which could
never be the right question).
Public registration gets the same guard via a plain unique() validation
rule, reusing Laravel's stock "already taken" message rather than a
dedicated one — naming the collision specifically would tell a public
visitor an address belongs to staff, a worse leak than the generic
refusal every other collision here already gives them.
Impersonation answers 409 instead of crashing into it. The Stripe
webhook keeps recording the paid order (same principle as
openContract()'s own comment: the order is proof money changed hands)
and withholds only the colliding login, logged for an operator to
resolve by hand.
Verified the reverse direction Codex flagged as already covered: two
Admin\Settings actions and the clupilot:create-operator command all
already refuse an operator email that collides with a customer's, and
that check is sound today because every users row is created in
lockstep with a customers row of the same email (CreateNewUser and
ensureUser are the only two creation sites, both confirmed by grep) and
nothing in this app can change a user's email afterwards (Fortify's
updateProfileInformation feature is disabled, and no other code calls
that action).
console.require_2fa exempted admin.settings from the redirect it forces on
an unconfirmed operator, but that page only ever configured the global
policy — nobody could enrol behind it. A newly invited operator stayed
permanently redirected to a page with no way to satisfy the requirement,
the opposite of what the switch was for.
Adds enable/confirm/regenerate/disable to Admin\Settings, reusing Fortify's
own actions exactly as the portal's Settings already does — they only ever
touch the model they're handed, no guard or auth() call inside them, so
pointing ConfirmsPassword's guard at 'operator' is the only change needed.
The new card is deliberately not gated behind the site.manage capability
that hides the rest of this page's Owner-only sections: the compulsory
switch applies to every operator, so enrolment has to be reachable by every
operator too. Dropped the account card's stale hint pointing at a "separate
security area" that never existed.
operatorUserIds() only discovered console users through model_has_roles. A
pre-existing account granted console.view straight through
model_has_permissions, holding no role at all, was invisible to the
worklist — meanwhile the permission's own guard is switched to `operator`
a few lines above, so that account would silently lose console access
while staying behind as a working portal login. Worklist is now the
deduplicated, sorted union of both tables; orderBy('model_id') kept on
each side since carryAcross() remaps in place and the ordering still
matters regardless of which table an id came from.
Where the console and the portal share a host (shared/fallback mode —
this VM's own mode), both guards keep their login key in the SAME
session. session()->invalidate() is flush() + migrate(true) — it
discards every attribute in the session, not only the operator's, so
signing out of the console silently signed a customer out of the
portal too if the same browser carried both. Replaced with
session()->regenerate(): a new session id, so there is no fixation
risk, while attributes belonging to any other guard survive untouched.
ApplicationBuilder::withMiddleware() defaults redirectGuestsTo() to
fn () => route('login') before bootstrap/app.php's own callback ever
runs — always the portal's Fortify page, regardless of which side of
the site turned the guest away. In non-exclusive (shared, this VM's
own mode) that sends a guest bounced off /admin to a sign-in form whose
table holds no operators: no 404 any more, but a sign-in that can never
succeed. Exclusive mode happened to self-correct only because
RestrictAdminHost 404s the wrong host before this ever ran.
Fixed for both modes at once rather than depending on which one a given
deployment runs: ask AdminArea, the one place routes/web.php's own
registration and RestrictAdminHost already agree is the source of truth
for "is this the console".
POST /broadcasting/auth carries only the 'web' middleware group
(Laravel's own withBroadcasting() registers it that way), so
PusherBroadcaster::auth() resolved retrieveUser() on the DEFAULT guard
before the admin.runs channel's own Auth::guard('operator') check ever
ran — found nobody, since an operator is never signed in on 'web', and
threw straight from there. The guard was correct but unreachable.
Fixed by naming both guards on the channel registration itself.
Also adds broadcasting/auth to RestrictAdminHost::SHARED: in exclusive
mode it was neither admin.* by name nor on the shared list, so it 404'd
on the console host too — degrading silently to wire:poll.4s rather
than breaking outright, which is why nothing screamed.
Three faults in the same migration:
- up()'s removal loop was driven by operatorUserIds() (model_has_roles
WHERE model_type=User) via the $carried array — but carryAcross()
repoints model_has_roles to Operator for every operator before a
single users row is deleted. A run that stops partway (the
customer-conflict abort, or a dropped connection on real MariaDB,
which this file deliberately runs without a transaction) leaves that
worklist empty on the next attempt, so `migrate` records the
migration as applied while every leftover users row still grants a
live portal login on the operator's own password. Now driven by
`operators` joined to `users` on email: that finds every
carried-but-not-yet-deleted row regardless of which run carried it.
- operatorUserIds() had no ORDER BY. carryAcross() remaps four owned
columns in place, keyed on the old users.id, while operators.id hands
out new values in the same order the list is walked — unordered, a
later operator's freshly assigned id can equal an earlier operator's
still-unprocessed users.id, and the second remap catches a row the
first one already rewrote. One operator ends up silently owning
another operator's VPN peer. Added ->orderBy('model_id').
- down() restored model_has_roles but not model_has_permissions, so an
operator holding a direct (not role-granted) permission lost it on
rollback. Mirrored the same move carryAcross() does forward.
Also removes seed_roles_and_permissions' now-stale
User::OPERATOR_ROLES/hasAnyRole()/assignRole() bootstrap loop — all
three gone from User per this branch's own work — which only a deep
rollback (down() of the move migration recreates is_admin users) would
ever have reached, but would fatal when it did.
It checked Auth::guard('operator')->check() instead of the users row it
was handed, so with an operator's own session still attached (shared
mode carries the same cookie through both legs of impersonation) it
threw the moment a customer's email already had a users row — a 500 on
the first impersonation of anyone who had signed into the portal
themselves. Deleting it rather than fixing the check: operators live in
`operators` now, never in `users` (R21), so a users row can no longer
BE an operator account for this to guard against. Also drops the dead
`'is_admin' => false` write — the column reads from nowhere any more.
redirect()->to(AdminArea::home()) resolved against whatever host the
leave() POST itself arrived on — the portal's, once the console has a
hostname of its own — landing the operator on the portal's dashboard
instead of back at the console. route('admin.overview') is domain-bound
to the console and generates the correct cross-host URL, matching what
this line did before this branch. The test named after this behaviour
now actually asserts it.
actingAs()/Livewire::actingAs() call Auth::shouldUse(), which made every
existing test for this trait pass identically whether confirmPassword() asked
for the right guard explicitly or just read whichever guard happened to be
default — reverting the explicit guard() call to a bare auth()->user() passed
the full suite. This test signs the operator in without actingAs and
deliberately decouples the ambient default guard from their session before
calling confirmPassword(), so only the explicit resolution can succeed.
Deleted tests/Feature/Auth/ConsoleLoginLandsInTheConsoleTest.php — it tested
the three deleted response classes. Its intent (an operator's sign-in landing
in the console, not the portal) is covered by OperatorLoginTest's "signs an
operator in on the operator guard" and OperatorTwoFactorChallengeTest's "signs
the operator in once the code actually checks out", both confirmed present
before deletion; the route-separation half is covered by the new
ConsoleHostSeparationTest cases added in this commit.
env('MAIL_HOST')/env('MAIL_PORT') returning anything, or MAIL_URL supplying
either, now counts as "supplied" and wins over the sentinel comparison — a
relay genuinely on 127.0.0.1:2525 no longer reads as unconfigured. The
sentinel stays as a fallback for when neither signal says anything (including
under config:cache, where env() goes null and this collapses to today's
behaviour).
MAIL_SCHEME=smtp is Symfony's opportunistic scheme (upgrade to STARTTLS
if offered, don't fail if not) — not a "no encryption" statement.
Mapping it to 'none' silently turned every existing MAIL_SCHEME=smtp
installation's encrypted-when-possible connection into an always-
plaintext one. Maps to 'tls' (STARTTLS required) instead: stricter than
smtp's own default, so a relay lacking STARTTLS now fails loudly rather
than leaking credentials.
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>
MailboxTester required a usable SECRETS_KEY unconditionally, refusing an
unauthenticated relay's test-send on a fresh install with no key yet, even
though real sending never touches the cipher for that mailbox. Gated the
check on authenticates, same as the password-presence check beside it.
Comparison pass across MailboxTester and MailboxTransport turned up two more
of the same shape. MailboxTransport's delegate-cache fingerprint decrypted
$box->password unconditionally, even when unauthenticated and never going to
use it — a mailbox that once authenticated, stored a password, and later had
authenticates unchecked would crash real sending the moment SECRETS_KEY
became unusable. And MailboxTester had no guard against a stored port <1;
Symfony silently reinterprets that as port 25 rather than refusing it, so the
test button could reach whatever happens to listen there and report success
for a configuration MailboxTransport refuses outright.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An SMTP relay with a host and a from address but no username or password
- a trusted local or private-network relay - always used to be seeded as
a placeholder, because the takeover required a username to treat an
account as real, and Mailbox::isConfigured() always required a stored
password. Laravel's own SMTP mailer has supported unauthenticated relays
all along.
Add mailboxes.authenticates (default true, so every existing row keeps
its current behaviour) to tell "does not authenticate" apart from
"authenticates, but nobody has typed the password yet" - the same empty
password column used to mean both. The takeover migration now seeds the
real account off the host/address alone and marks it authenticates=false
only when neither a username nor a password resolved; isConfigured(),
MailboxTransport and MailboxTester all read it before requiring or
sending a password; the edit modal gets a checkbox for it with the
password field's hint updated to match, in both languages.
MAIL_SCHEME=smtps or an smtps:// MAIL_URL on a non-465 port used to be
downgraded to STARTTLS because mail.encryption was derived from the port
alone. Resolve the scheme first (from MAIL_SCHEME directly, or from the
URL's own scheme via the same ConfigurationUrlParser Laravel uses) and
only fall back to the port heuristic when nothing was actually stated -
which is also what keeps this machine's invalid MAIL_SCHEME=tls from
silently steering the result.
isUsable() checked only "is SECRETS_KEY nonempty", so a set-but-malformed
key (wrong length, garbage base64) read back as usable even though
encrypter() rejects it two lines below. EditMailbox::save() and
MailboxTester::run() both gate on isUsable() specifically to avoid an
uncaught RuntimeException reaching the operator; a lying isUsable() meant
that guard did not fire in exactly the configuration it exists for.
Both methods now read resolveKey() — the base64: prefix, the raw-base64
path, the 32-byte check — so they cannot disagree about a value either
one is given. SecretVault::isUsable() and the secrets console page's
"no key" banner both delegate down to this and are covered here too, not
assumed to inherit the fix correctly.
The seed migration read config('mail.mailers.smtp.host') and its three
siblings directly. An install configuring SMTP through MAIL_URL (the
other form config/mail.php supports) never sets those individual keys —
MailManager only substitutes the URL's components in at transport-build
time, inside its own protected getConfig(). Read raw, that left the
migration seeding config/mail.php's own placeholder defaults, which the
UNCONFIGURED_HOST/UNCONFIGURED_PORT guards then read as genuinely unset:
blank host, port 0, an unconfigured mailbox, for an install that was
sending mail successfully.
resolveSmtpConfig() calls the same Illuminate\Support\ConfigurationUrlParser
MailManager::getConfig() itself delegates to, so this is the same
resolution rather than a second copy of its merge order. Verified against
the real (protected) getConfig() via reflection: the URL wins for
host/port/username/password wherever it actually supplies one, and an
unset MAIL_URL leaves this a no-op.
The takeover migration seeded `address` from MAIL_USERNAME and left
`username` null unconditionally, conflating the SMTP login with the
sender address. Whenever the two differ — an API-key username, a
shared relay login — mail would go out From the login instead of the
address the system was actually configured to send from, exactly like
this installation: MAIL_USERNAME is no-reply@clupilot.com but
MAIL_FROM_ADDRESS is hello@clupilot.local.
`address` now reads config('mail.from.address'), falling back to the
SMTP login only when that reads back config/mail.php's own unset
placeholder (env('MAIL_FROM_ADDRESS', 'hello@example.com') — the same
masking problem UNCONFIGURED_HOST already guards against for
MAIL_HOST). `username` is set to the SMTP login only when it actually
differs from the resolved address, since Mailbox::smtpUsername()
already falls back to address when username is null. display_name
now reads config('mail.from.name') instead of a hardcoded literal.
The placeholder domain for the four non-configured mailboxes still
comes from the SMTP login, not from mail.from.address — the two can
name different domains, and only the login's is proven to accept mail
for a given install.
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.
EsmtpTransport's autoTls (default true) and requireTls (default false)
are independent of the implicit-TLS constructor argument: 'tls' left
these at their defaults, so a server that omitted STARTTLS sent
mailbox credentials in the clear while reporting success, and 'none'
would silently upgrade the moment a server offered STARTTLS. Both
MailboxTransport and MailboxTester already duplicated the "ssl, or
port 465 ⇒ implicit" half of this decision independently; MailTlsPolicy
is now the one place both read, for both dimensions.
MailboxTester still builds through Mail::build() rather than a hand-built
transport: MailManager::createSmtpTransport() forwards the whole config
array to EsmtpTransportFactory as DSN options, which already reads
auto_tls/require_tls itself, so the same policy reaches this path
through keys the config array already supports.
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.
Mail::to($x)->queue($mail) resolves the DEFAULT mailer, and
Illuminate\Mail\Mailer::queue() (vendor Mailer.php:482) does
`$view->mailer($this->name)->queue($this->queue)` — overwriting
MaintenanceAnnouncementMail/MaintenanceCancelledMail's own 'cp_maintenance'
with the default mailer's name ('array' in tests, 'log'/whatever
MAIL_MAILER is in prod) before the job is even built. Both mails kept
sending through the old .env account instead of MailboxTransport. CloudReady
was never affected — MailChannel.php:66 preserves a notification's own
mailer.
Fix: name the mailer before queueing — Mail::mailer($mail->mailer)->to(...).
$this->name on the resolved Mailer is then already $mail->mailer, so the
vendor overwrite becomes a no-op instead of a silent downgrade. Confirmed by
reading the vendor source, not assumed.
SenderAddressTest's "names the purpose mailer..." test asserted
$mail->mailer on a bare, never-queued mailable, which proves nothing about
what survives queueing — the constructor sets it right regardless of what
MaintenanceNotifier does with it. Replaced it with one that drives
MaintenanceNotifier::deliver() under Queue::fake() and inspects the pushed
SendQueuedMailable's mailable. Verified it fails against the pre-fix code
first (the pushed job carried mailer = 'array'), then verified the fix
makes it pass, then mutated the fix away and watched it fail again.
MailboxSeedMigrationTest.php had the actual .env MAIL_PASSWORD for
no-reply@clupilot.com hard-coded into a test fixture — replaced with an
obvious dummy. Grepped the whole branch (576cfca..HEAD) and the working
tree for that string and for every other real secret value in .env;
nothing else leaked.
Separately, the seed migration's SECRETS_KEY-missing warning claimed a
password "was NOT copied into the mailbox table" even when a vault row
already existed and its ciphertext WAS carried across a few lines below,
unconditionally on $canEncrypt. An operator debugging a live mail outage
would have been sent to re-enter a password that was already sitting in
the column. The warning now distinguishes the two cases and says which
one actually happened.
It only ever loaded the page and asserted the banner, never a password
save — but its name claimed the crash-on-save case, which is exactly the
promise that let that bug ship unnoticed once. The test that earns the
crash claim is its new neighbour, added for the same finding.
EditMailbox::save() now checks SecretCipher::isUsable() before touching a
typed password and reports it as a form error, instead of letting
SecretCipher::encrypt() throw all the way to Laravel's debug page. Also
switches the mail.manage migration to Role::findOrCreate, matching every
sibling capability migration, so a squashed replay can't hit
RoleDoesNotExist.
The vault's mail.password row is no longer deleted by this migration at
all, in either direction: deleting it unconditionally destroyed a real
password whenever MAIL_USERNAME was empty (nowhere to carry it to), and
even on a successful carry-over a later rollback had nothing left to fall
back to. Reproduced both against real MariaDB, along with a migrations-
table drift that crashed a retry on the mailboxes.key unique constraint —
fixed with Mailbox::firstOrNew() and a transaction around each direction.
down() now goes through the new Settings::forget() instead of a raw
delete, so it stops leaving the settings cache holding what it just
removed. A skipped .env password (SECRETS_KEY still unset) now prints an
operator-visible line instead of migrating clean and failing silently
later. Host/port that resolve to config/mail.php's own placeholder
defaults are treated as unset rather than as a real relay.
The seed migration also guards against SECRETS_KEY being unset at migrate
time (encrypting the .env password would otherwise throw and take the
whole migration down, on a fresh install as much as in the test suite),
and the four tests/Feature/Mail files written before this migration reset
the mailbox table in their beforeEach so they keep testing what they did
before mailboxes.key collided with the seeded rows.
A mutation the reviewer ran (if ($box !== null) -> if (true)) left every
test green: CloudReady hand-rolled the same null-mailbox guard as
SendsFromMailbox, but nothing exercised its null branch. Added the
missing test, then extracted the decision itself (null mailbox -> no
sender fields, no_reply -> no Reply-To) into one shared method so an
Envelope-shaped and a MailMessage-shaped consumer can no longer disagree.
Re-review of the previous fix round found the "cannot disagree" part of
finding 4 was still open. resolution() had unified the log/array/null/
mailbox decision, but inside the mailbox branch describe() returned
$box?->address unconditionally while delegate() additionally required
isConfigured() before using it. A mailbox with active=true, a real
address, and no password produced
'mailbox://support/support@clupilot.com' — reading as entirely healthy
— while send() threw. That is exactly the harm the original finding
named: the log says fine, the mail fails.
resolution() now decides "unconfigured" too, not just "which
non-delivering mode": it calls isConfigured() itself, once, and returns
that verdict alongside the box. describe() and delegate() both branch
on the SAME result again, the same way they already did for log/array/
null — a mailbox with no password now reads as
'support@clupilot.com [unconfigured]' rather than a plain address, and
the address stays visible so an operator can tell WHICH mailbox needs
fixing.
Also renamed a test the re-review flagged as promising more than it
checked: "registers a mailer per purpose without touching the database
at boot" only ever verified the config array's shape — the DB-touching
claim is a separate, already-passing test just below it
(defines every purpose mailer as a plain literal...). Renamed to
"registers a mailer for every purpose". No behaviour change.
Added a test creating a mailbox with no password and asserting the DSN
both names the address AND carries the marker, plus that send() still
throws — proving agreement, not just a matching label painted on
separately. Mutated describe()'s unconfigured branch to drop the
marker: the test failed exactly on the missing 'unconfigured' string
(DSN read as a plain, healthy-looking address again). Reverted, reran,
passes.
Full suite: 667 passed. vendor/bin/pint --test clean on both files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reviewer continued the mutation sweep from Task 4's own commit and
found three more unpinned lines in delegate(), plus two real behaviour
gaps, all inside the same 40-line method:
- The fingerprint refresh (the headline feature: a worker's mailer
must pick up a corrected password without a restart) had no test —
mutating its check to "delegate === null" left the suite green.
- The implicit-TLS third argument to EsmtpTransport had no test either
— mutating it to a hardcoded true left the suite green, because the
only assertion touching it was instanceof EsmtpTransport, which the
hanging variant also satisfies.
- isLogging() only ever checked for 'log', but phpunit.xml sets
MAIL_MAILER=array for the whole suite. Task 5 wires real mailables
to these mailers next; the first feature test that sends one without
Mail::fake() would have opened a real SMTP connection.
- describe() and delegate() each ran their own copy of that guard —
which is exactly how deleting it from ONLY delegate() passed the
whole suite in the first place: __toString() kept reporting "log"
from its own untouched copy.
- An unconfigured host/port (Settings::get('mail.host', '') and a
stored null port casting to 0) built a transport pointed at
smtp://:587 or silently downgraded to plaintext port 25, instead of
refusing the way a missing mailbox already does.
Fixes: replaced isLogging() with resolution(), the one place that now
decides "log, array, null-default, or a real mailbox" — describe() and
delegate() both branch on ITS result, so a guard broken in one cannot
look fine in the other. Added an explicit RuntimeException for a blank
host or non-positive port, guarded the same way as the missing-mailbox
case just above it.
Tests: added coverage for the fingerprint rebuild (reuses the same
delegate until the password actually changes, proven by object
identity), the exact DSN string for both the STARTTLS (587) and
implicit-TLS (465) cases, 'array' and unset-default both landing on a
non-delivering transport, and the blank host/bad port throwing.
Mutated each of these four in turn on the final code and confirmed:
fingerprint check removed -> the reuse test fails (same object handed
back after the password changed); TLS argument hardcoded true -> the
587 DSN test fails ('smtps://...' where 'smtp://...:587' was
expected); NON_DELIVERING narrowed back to ['log'] -> both the array
and unset-default tests fail (a real EsmtpTransport where a safe
transport was expected); host/port guards removed -> both throw
expectations fail. Reverted each, reran, all twelve tests pass again.
Replaced the two tests that reached into delegate() via Closure::bind
to check "log or SMTP" with one behavioural test: point Settings at a
closed loopback port (127.0.0.1:1, so refusal is instant and entirely
local — no DNS, no dependency on this environment's network egress
policy) and assert Log::shouldReceive('debug')->once() while sending
for real through Mail::mailer('cp_support')->raw(...). Mutated
NON_DELIVERING to drop 'log' specifically and confirmed this test
alone catches it: a real connection attempt to the closed port throws
TransportException (Connection refused) in under 300ms. Reverted,
reran, passes again.
Also: the two tests that only checked the config array's shape and a
snapshot of the resolved address promised more than they verified —
CLAUDE.md R19 names a test that recomputes the implementation as
checking nothing. Added a source-level test that the five cp_* config
entries are parenthesis-free literals (same technique
DisplayTimezoneTest/IconLayoutTest use: a property of the file, not of
one run) as the actual "no query at boot" proof, and renamed the
send-time test to what it verifies now that the fingerprint test above
covers the live-refresh claim properly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of Task 4: MailboxResolver::for() only fell back on a missing
mapping or a missing row. Deactivating a mailbox — the operator's
explicit "do not use this" — left it resolvable anyway, so
MailboxTransport reached it, found isConfigured() false, and threw
instead of quietly using the next candidate the way every other
"this mailbox is unusable" case does. Worse, __toString() still
reported the deactivated address as though it were healthy.
named() now runs through a new active() filter at BOTH lookup sites —
the purpose's own mapping and the system fallback — so an inactive
system mailbox is refused exactly like a missing one, not handed out
as the last resort. A missing password stays deliberately unfiltered
here: that is a MailboxTransport concern (loud failure naming the
mailbox), not a resolver fallback trigger — silently rerouting mail
because a password hasn't been typed yet would hide the gap instead
of surfacing it.
Added two tests and mutated active() to a no-op filter to confirm they
catch it: both failed (one expected the system mailbox, got null; the
other expected null, got the inactive mailbox's full attribute dump).
Reverted, reran, both pass again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five purpose-named mailers (cp_maintenance, cp_provisioning, cp_support,
cp_billing, cp_system) land in config/mail.php as plain static entries —
no query runs to build them. Mail::extend('mailbox', ...) in
AppServiceProvider registers a transport that looks up its mailbox
through MailboxResolver only once a mailer is actually resolved, which
for queued mail is inside the worker at send time, never at boot.
Mutation-tested the brief's own three tests by deleting the MAIL_MAILER
guard from MailboxTransport::delegate(): all three stayed green. They
only exercise __toString()/describe(), a code path separate from the one
that actually sends. Added two tests that reach into delegate() itself
(a bound closure, since it's private) and assert the transport it
actually builds: LogTransport while MAIL_MAILER=log, EsmtpTransport once
it isn't. Re-ran the same mutation against the strengthened suite: it
now fails (EsmtpTransport where LogTransport was expected), then passes
again once reverted.
Verified two things against the installed versions before writing this:
LogTransport takes a Psr\Log\LoggerInterface and Log::channel(null)
resolves to the default channel rather than throwing, and
Mail::extend('mailbox', ...) is read by
MailManager::createSymfonyTransport() via $config['transport'] before it
would fall back to createSmtpTransport() — both matched the brief
exactly, no changes needed there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The old test rotated app.key and expected the password to stay
readable, but Laravel's encrypter is a container singleton resolved
once — config()->set('app.key', ...) never rebuilds it, so the
rotation the test performed was invisible to any code path going
through it. Confirmed by mutation: rewiring Mailbox to Crypt::
encryptString/decryptString (genuinely APP_KEY-keyed, the exact
regression this constraint exists to forbid) left the old test green.
Replace it with two tests: a positive proof that rotating SECRETS_KEY
makes a stored password unreadable (the one that discriminates), and
a separate check that rotating APP_KEY does not, with
forgetInstance('encrypter') so that rotation is actually visible to
whatever the password accessor resolves.
Re-ran the same mutation against the new test: it fails (exit 1, the
SECRETS_KEY test reports the expected DecryptException was not
thrown), then passes again once the mutation is reverted.
Mailbox hand-rolled the same creating-time UUID assignment that
App\Models\Concerns\HasUuid already provides to 17 other models under
R11 (URLs address records by UUID, not integer PK), and skipped the
route-key binding that comes with it. Swap onto the trait, and cover
both the assignment and the route key with a test matching the
convention already used for Host/Order.
Also match MailboxFactory's @extends annotation to how the other
factories in the repo write it (Pint's fully_qualified_strict_types
rule turns a fully-qualified docblock reference into an import plus a
short name anyway, so this is what it would end up as either way).
Three things reported together.
── Editing belongs in a modal (R20)
The seats table grew its input fields into the row. It worked and it looked
broken: the row grew, the columns beside it jumped, and a table half in edit
mode reads as a rendering fault rather than as a form. The project already had
the answer — EditDatacenter, whose own header comment says it avoids exactly
that row-height jump — and the seats table simply did not use it.
EditSeat is now a ModalComponent. A modal is reachable WITHOUT the page's route
middleware, so it resolves the customer itself and re-reads the record rather
than trusting a hydrated property: a forged addressEditable would otherwise
open the address of an accepted seat, and the address is the person — editing
it hands one employee's access to another with nobody told. Only an invitation
still in flight can have its address corrected, which is the case that actually
comes up.
The actions column is now always drawn. It used to disappear when the only seat
was the owner, on the reasoning that there was nothing to act on. But every
seat can be renamed, and a column that vanishes does not read as "not
applicable here" — it reads as "this product cannot do that", which is how it
was reported, three times. The owner's row says "Geschützt" rather than leaving
an empty cell.
Also caught here: the edit fields carried class-wide #[Validate] attributes, so
an empty edit form made the INVITE button fail on a field the invite form does
not have. Rules for an action belong to the action.
── The update button
"I cannot run an update, it says everything is current." `behind` is a READING
taken by the agent every five minutes, not the state of the world — push a
commit and the console insists it is up to date and refuses to act. The agent
does its own fetch before deciding, so asking against a stale reading costs one
fetch and finds nothing; being locked out costs the deployment. The button is
now offered whenever an agent is alive and no run is in flight, labelled for
what it does rather than for what the last reading said.
── The update never announced that it had finished
Because the thing being watched restarts the thing doing the watching. Mid-run
every wire:poll request fails, and what answers afterwards is a new build being
questioned by the old page's JavaScript — so the card sat on "läuft" until
somebody reloaded by hand. A small Alpine watcher now asks a plain JSON
endpoint (no Livewire, no component state, no assets), treats a failed request
as the restart rather than as a fault worth giving up over, and reloads once
the build it is looking at is no longer the build it started with.
── Support
The page was a decorated placeholder: a button that raised a toast saying the
form was "only hinted at in the prototype", three invented ticket titles living
in the translation file, and no way to see what became of anything. It looked
thin because nothing on it was real.
It now leads with the customer's own requests — what somebody arriving here
wants to know is what they asked and whether anyone answered — with contact
details moved to the side where they belong. The form attaches the plan, the
instance and who is asking automatically: making a customer describe their own
server back to the people who built it is the part of support people hate. FAQ
answers end where the thing can actually be done, and "is it me or is it you"
is answered by a link to the status page.
637 tests. R20 recorded in CLAUDE.md and enforced by EditInModalTest: no page
view may grow an input field inside a <td>.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
Availability is now counted from our own monitoring answers. Uptime Kuma knows
the figure, but the REST bridge in front of it only answers "healthy, yes or
no", and waiting for the bridge to grow an endpoint would have left the panel
without the number indefinitely. The sync already asks on a schedule; counting
the answers gives a percentage that is ours and explainable.
An unanswered check is not a failed one — the counter only moves when a real
answer arrived, so our monitoring going down does not become the customer's
outage on the one figure they are asked to trust. An instance nobody has ever
checked shows no availability rather than 100 %.
The downgrade path existed nowhere. Going down is not the mirror of going up:
it can ask an instance to hold more than the target plan allows. Smaller plans
are now listed even when they cannot be taken, with the obstacle in the
customer's own numbers — "you have 31 users, this package allows 10" — because
a greyed-out button that does not name what is in the way is the thing people
ring about. Storage is checked against the last real reading rather than the
contractual allowance, or anyone who once bought a large plan could never leave
it. The limit is re-checked in the action: a rule enforced only in markup is
not enforced.
And the smaller corrections from this round:
- The cloud tab carried a hardcoded "B" as the instance's initial, and the PHP
and MariaDB versions, which tell a tenant nothing they can act on.
- "EU — Serverstandort im Angebot festgehalten" under a label already reading
"Serverstandort" was two sentences saying nothing. It is the country.
- Support promised a reply "binnen 4 Std." with nothing behind it. What is true
everywhere else is an answer on the same working day.
- The actions column appeared even when no row in the table had an action.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The panel could not show the storage ring or the transfer trend because nothing
measured them. instance_traffic keeps one row per billing period — right for an
allowance, useless as a series — and storage consumption was never sampled at
all, so the card could only ever state what was sold rather than what is used.
instance_metrics holds one row per instance per day, written by the traffic
collector on the visit it already makes. A second scheduler entry against the
same Proxmox API would have doubled the load and the failure modes for no gain.
Disk usage comes from `df` inside the guest rather than from Proxmox's own disk
figure, which reports the allocated image: that would show every customer a full
500 GB from their first day.
The rule the whole thing is built on: a reading that could not be taken is not a
reading of zero. The disk columns are nullable and left untouched when the guest
agent does not answer, so yesterday's figure stands instead of the ring dropping
to empty — which would tell a customer their data had vanished. Missing days
stay missing in the series rather than being filled with zeroes, which would
draw an outage that never happened. Both are covered by tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The console was a second product. admin-tokens.css held a complete dark set —
its own surfaces, its own orange, its own status triad — so every shared
component had to work in two worlds, and the two halves of the application
drifted apart in ways nobody could see from inside either one. That file now
carries density and nothing else: smaller type, tighter rows, tighter corners.
Colour comes from one place.
The tokens themselves are rebuilt around what the redesign settled on:
- Radii scaled to object size (9/11/16/22) instead of one value everywhere,
which is what made the interface read as a design-system specification
rather than as a product.
- Warm, broad, low shadows. A neutral grey drop shadow belongs to a different
design language and reads as cold on this ground.
- --accent-press, which several rules already referenced and nothing defined.
An undefined var() does not make a browser skip the declaration; it computes
to , and for background-color that is transparent — the accent button
lost its fill on hover and left white text on white.
- No serif. A serif headline is what made every page read as a document, which
is the impression the whole redesign exists to remove. --font-serif now
points at the sans so nothing breaks while call sites are cleaned up.
The two shells are one shell with two configurations, and both finally have a
mobile navigation: the sidebar used to simply disappear below 900px, leaving no
way to move around the application at all on a phone.
The customer dashboard is bound to real records — instance, seats, traffic,
backups, contract — instead of the fixtures it shipped with. Where something is
not measured, storage consumption, it says what was contractually agreed rather
than inventing a figure. This is the sheet a customer forwards to their auditor.
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>
An empty 404 is itself information: it says something terminates TLS here and
chose not to answer. The console and the websocket endpoint now close the
connection instead, which is what a hostname that serves nothing looks like.
The websocket endpoint answers a genuine upgrade — verified live, 101 — and
nothing else; a browser opening it gets a closed connection.
Reviewing that turned up two holes that mattered more than the thing being
reviewed.
The compose defaults published the application and Reverb on every interface.
Docker publishes ports ahead of UFW, so those backends were reachable from the
internet with the firewall closed — and reaching a backend directly skips the
proxy, and with it every hostname and address rule keeping the console private.
The defaults are loopback now, and an update rebinds an existing installation
that is behind a proxy actually running and holding 443. A development box
without one is left alone, because taking its port away would look like the
machine had broken.
And the console's own "open" switch was emitting 0.0.0.0/0 into the proxy's
allowlist. One click would have put the console on the public internet, which
is the exact state the owner's rule exists to prevent. Switching it off relaxes
the application's check; the proxy keeps its list, always.
The reference proxy config carries the arrangement: QUIC early data off, since
an address-based decision taken on 0-RTT can answer 425 and some browsers do
not retry — an intermittent lockout from the console is the worst kind — and
explicit handling for plain HTTP, whose automatic redirect otherwise announces
both hidden names to anyone who asks.
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>
Clicking Save made every button on the console's settings page appear to fire.
Seven `wire:loading.attr="disabled"` had no `wire:target`, and without one
Livewire applies the loading state to ANY request on the component — so one save
put all of them into their disabled state simultaneously. Each names its own
action now.
Two-factor for customers. Fortify's endpoints already existed; only the screen
was missing. Setting up requires re-entering the password first, and every
action re-checks that server-side rather than relying on the button not being on
screen — a Livewire action is reachable by anyone who can post to
/livewire/update. The confirmation marker is Laravel's own session key, so it
and the framework's password.confirm middleware mean the same thing rather than
drifting apart.
The secret never enters a Livewire property. Component state travels to the
browser and back in the snapshot; the QR image is derived from the secret, the
secret is not in it — and there is a test that says so.
The status page moves to the ROOT of its hostname: status.clupilot.com/status
says the same word twice. That needed the domain-bound `/` registered BEFORE the
landing page, because Laravel takes the first match and the landing route is
host-agnostic — so the status host would have served the marketing site.
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>
Registering the console once per accepted hostname reused one set of route
names, and Laravel refuses to serialise two routes under the same name — so
`artisan optimize` failed on the live server the moment the separation was
switched on, mid-deploy.
Only the canonical registration carries the `admin.` names now; the alternates
answer under `admin.viaN.*`. They exist to be MATCHED — they are the addresses
someone locked out reaches for — never to have URLs generated for them, so
route() keeps producing the canonical hostname.
That renaming then broke every exact route-name check, which is how the console
navigation decides what is active: reached through a recovery address, nothing
in the sidebar was marked, exactly when something is already going wrong. The
check goes through AdminArea now and matches either form.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>