45 security findings, one missing CSS class, and the middleware order that undid every scope in the platform

The Starting Point

Two halves. Morning: a feedback module — the fifteenth package, and mostly a registration. Afternoon: work through a security review that had produced 45 findings, one critical.

Also, first thing, repairing something I broke yesterday.

What Actually Happened

I locked us out of the dev environment

Yesterday, proving that the audit trail withholds a password, I changed the password on the only administrator account in the live dev database. And then discovered nothing could repair it.

The ensure-admin command sets a password only when it creates the administrator — deliberately, so a deploy never overwrites one somebody chose. And the set-password link lives on a page you must be signed in to reach.

So a deployment whose only administrator loses their password had no supported way back in.

There's a reset command now. Refused outside local without a force flag, and it says what to use instead rather than only refusing. It switches a deactivated account back on, because an account that's off cannot sign in however good its password is. And it records the reset in the trail, with the password withheld by the redaction rules rather than by the command remembering to leave it out.

Lesson filed under "don't verify destructively against the account you need".

A button that was there and CSS that wasn't

The feedback module shipped with a floating button on every page. Enabled, and no button anywhere.

The markup was on every page. fixed bottom-4 right-4 had never been compiled — two utility classes the platform hadn't used before, in a view added after the last asset build. So the control sat unpositioned at the foot of the document where nobody would find it.

And this class of fault is invisible to the entire test suite, because every suite runs withoutVite(). A view naming a utility class that was never compiled passes every assertion and renders wrong in a browser.

So bin/check builds the assets now, between static analysis and the suites. About half a second, and it means a green check implies the CSS on disk matches the views on disk rather than the views as they were the last time somebody remembered to build.

That's the third thing this week a green suite had no opinion about, after two navigation faults. Same pattern every time: the assertion was about the markup and the fault was in what the browser does with it.

Then the security review

45 findings. Going through them turned up one genuinely frightening thing and a long tail of the same shapes repeating.

The critical one: middleware ordering.

Laravel's SubstituteBindings ran ahead of the business-unit context middleware. And the unit scope stands aside when no unit is set — a deliberate decision from very early on, so that a migration or a console command doesn't silently hide rows.

Put those together: every module route that binds a record resolved records from any business unit. Not one module. Every route in the platform that takes a {record}.

The fix is registering three context middlewares ahead of bindings in the priority list. One place, every module at once.

The regression test has a detail worth keeping: it requests the foreign record first, because the unit context is a singleton and a request that has already set it masks the ordering. Checked by reverting the fix and watching all eight cases fail.

The second critical: an administrator could act on any account. A policy on every action that takes a user — never the vendor account unless you are it, never somebody outside your units, never somebody holding a grant in a unit you can't see or a permission you lack.

And with it: no changing your own access, and only permissions you actually hold can be granted — directly or through a role. A role administrator adds only what they hold.

The shape that keeps repeating. An unguarded download appeared for the fifth time in this port — project files, boards, approval documents, case message bodies, and now feedback attachments. The source streams any file to any signed-in user with no check of any kind.

At five occurrences it stops being a finding and becomes a pattern. Which is an argument about the framework, not about any of those modules.

Things that only exist because the platform is multi-tenant, and every one is a place the source simply never had to think:

scheduled reports ran outside any unit — each runs inside its own now, and one with no owner unit is skipped
mailboxes polled outside any unit, so an emailed case could land anywhere; case references matched exactly, within the unit, and threads never cross units
a bill document could print another unit's customer
money could be allocated from a payment the actor couldn't see
the approver picker offered colleagues from everywhere

And a few that are just good hygiene done properly:

CSV formula injection — cells neutralised in exports and scheduled CSVs, leaving plain negative numbers alone, which is the bit naive implementations get wrong
bearer links stripped from the stored copy of every message and from the log line; the log transport refuses to run in production
cached provider tokens encrypted
failed jobs pruned daily
whoever raises an approval cannot approve it, and once anybody decides, the content, documents, approvers and quorum are fixed until resubmission — and the page stops offering what would be refused
a plan item's parent guarded on create and edit as it already was on move, and the rollup survives a cycle — the recursion that would otherwise loop forever

Content-Security-Policy, in two stages

Stage one: 23 inline handlers stopped echoing Blade into JavaScript. The worst was the case desk's "Reply to this", which put an inbound sender's display name into an onclick. An attacker controls that string entirely.

A guard test now scans every view for {{ inside JS and Alpine attributes.

Stage two, later the same day: enforce script-src with a nonce and no unsafe-inline. An injected <script> or inline handler no longer runs — which is how every XSS in this review would have executed.

That meant 42 inline handlers across 30 views becoming framework directives. 30 of them were confirm-on-submit. The nonce is created before the view renders so the framework's own script tags carry it automatically; the 11 hand-written <script> tags carry it explicitly.

unsafe-eval stays for now, because the standard build of the reactivity library evaluates directives with Function(). Its CSP build is the recorded follow-up — named rather than pretended away.

And it was checked in a browser, not just asserted: headless Chrome signed in as a throwaway admin and walked 30 pages. No CSP violations, no JS errors, and all 28 confirm forms across 11 pages opened their dialog, cancelled on dismiss, and were bound correctly.

Sessions in the clear

A session row holds the password hash the session guard compares, the business unit being acted in, and flashed one-time values such as a new API token or a set-password link.

Laravel's default stores all of that in the clear, and the app inherited the default. Core forces encryption on from a booting callback now — the same way it forces permission teams — so a client repository cannot forget it.

Everybody gets signed out once when it ships. A session written in the clear fails to decrypt and is treated as empty. Worth saying out loud in the deployment notes rather than letting people discover it.

One more, older than the review

Found while fixing something else: filling in a form through its own page never saved anything. The page named each control after the bare field key and the controller reads a namespaced array.

Every test posted the right names by hand. So the whole feature was covered and completely broken. The test now posts exactly what the page renders.

What Worked

A tracker that the dashboard is a view of, not a copy of. One row per finding, its status, and the commit and test that closed it. A script recounts the tracker and redraws the dashboard block between markers — and bin/check fails when they disagree.

That's the only version of this I'd trust. A security summary maintained by hand alongside a list of findings is two documents that will drift, and the summary is the one people read.

One finding is accepted, with its reason written down, rather than quietly closed. And two follow-ups are named.

Feedback scoped by authorship, which is a first for this platform. The source has two positions and no middle: administrators only, or every signed-in person reads everything anybody ever submitted — including the page they were on — because its list isn't scoped at all.

Here it's the record type's own scope, so the list and the record page both honour it rather than one filtering and another not. That needed one small core change: the record type wouldn't accept a null view permission, although null has always been a supported state on the property. Feedback is the first type to want "nothing to read, something to change".

Core gains exactly one new slot for it. The floating button is the single thing the record framework can't draw: it sits on every other page and captures the URL and scroll position of the page it was pressed on, which is the whole feature. Deliberately narrow — an icon, a label, a view — and in core rather than the module because a module overriding core's layout is the top rung of the escalation ladder and this is a bottom-rung problem. A second caller is already visible: "raise a case about this page" is the same shape.

And a correction worth recording: a test I wrote assumed a record type inherits a CSV export, because two of my own documents say so. It doesn't — core's only CSV writer is the blank import template. Both documents corrected, and the gap matters more now than it did: an export that ignored this type's scope is how a hidden record reaches a spreadsheet.

What I Learned

Middleware ordering can silently undo every scope in an application. Mine bound models before the tenant context existed, and the scope's deliberate "stand aside when there's no context" turned that into a platform-wide leak.
A scope that stands aside when unset is a loaded gun. It's the right behaviour for console commands and the wrong one everywhere a request can reach it without context.
withoutVite() makes your whole suite blind to CSS. A class that was never compiled passes every assertion. Build the assets in your check.
At five occurrences, a finding is a pattern — and the argument moves from "fix these five" to "the framework should make this impossible".
Neutralise CSV formula cells but leave negative numbers alone. Naive implementations break every spreadsheet full of legitimate negatives.
A nonce-based CSP means auditing every inline handler, which is real work — 42 across 30 views — and it's the thing that actually stops injected script running.
Walk the pages in a real browser after a CSP change. Assertions about headers don't tell you the confirm dialogs still work.
Encrypt sessions. They hold flashed one-time values, and the default doesn't.
A summary kept by hand next to a list of findings will drift. Generate one from the other and fail the build when they disagree.
Don't verify a destructive behaviour against the only account you have. Obvious in hindsight; cost me a morning.

Tech Notes

Stack: PHP 8.4, Laravel, Blade + Alpine, Tailwind 4, headless Chrome for the CSP walk
Middleware: three context middlewares registered ahead of SubstituteBindings in the priority list
CSP: script-src 'self' 'nonce-…' 'unsafe-eval', no unsafe-inline; nonce created before render so framework tags pick it up; report-only retained for the non-script directives still being worked towards
Guard tests: no on*= attribute or javascript: URL in any view; every <script> carries the nonce; no {{ }} inside JS or reactive attributes
session.encrypt forced from a booting callback in core so a client repo can't forget it
bin/check now runs: format → static analysis → asset build → all suites; also fails if the security tracker and the dashboard disagree
1,469 assertions across fifteen packages

php artisan <platform>:reset-admin-password   # refused outside local without --force
bin/security-status                           # recounts the tracker, redraws the dashboard block