Two security fixes from the recent audit. Independent of #76 — no overlapping files, so they can merge in either order.
1. Unauthenticated full CRUD on /api/requests
app/Models/Request.ts carried useApi: true, and useApi.routes is
opt-OUT: omitting it makes the ORM generator emit index, show, store,
update, destroy and a bulk-delete, with no 401 path on any of the
writes. storage/framework/orm/routes.ts resolves the authed user only to
feed the optional authedFill hook, and its sole 401 branch sits inside
if (own.enforced), which needs an ownership config this model never
declared.
The requests table is request telemetry — ip_address, user_agent, path,
status_code:
- Readable, it is an unmetered log of who visited what. On an app whose
premise is anonymous reviews that is a de-anonymisation primitive: correlate
an IP against a
POST /api/reviewshit and the anonymous author is named. - Writable, anyone can forge or wipe telemetry that the framework dashboard
renders (
defaults/routes/dashboard.ts→Dashboard/Infrastructure/RequestIndexAction).
No live data leak — the table has no rows and nothing in this app reads or writes it. The endpoints were real regardless.
Fix is routes: [].
Two things worth recording:
app/Models/commerce/*(~29 models, sameuseApiwith noroutes) are inert: the loader atstorage/framework/orm/routes.ts:19-33doesreaddirSyncoverapp/Modelsthenif (st.isDirectory()) continue— it never recurses. Don't rely on it; a loader change would expose all of them at once.This arrived with
c5131e97("scaffold sync; commerce + email models"), i.e. after the earlier lockdown pass. So re-audit after every scaffold sync:for f in $(find app/Models -maxdepth 1 -name '*.ts'); do grep -q useApi "$f" && ! grep -qE '^\s*routes:\s*\[' "$f" && echo "OPEN: $f" done
2. The session token was readable from JavaScript
resources/stores/auth.ts took the token out of the login JSON and did
document.cookie = 'auth-token=…'. A cookie written that way can never be
HttpOnly — the entire point of document.cookie is that scripts write it,
and whatever a script can write a script can read. The token sat there for 30
days, readable by any injected script. This app renders user-submitted rich
HTML, so that turns a single XSS into session theft — and the reviewer
anonymity the product promises is downstream of those sessions.
This did not need the auth redesign the notes assumed
The framework already does cookie auth end-to-end; the app simply wasn't using it. Nothing server-side had to be taught anything new:
authCookie()(@stacksjs/auth) serialises the cookie and always includesHttpOnly, resolving the name throughauthCookieName(), the lifetime fromconfig.auth.tokenExpiry, andSecurefromshouldSecureAuthCookie()— which deliberately omitsSecureon loopback so local http sessions keep working.requestToken()— whichAuth.user(),Auth.logout()and theauthmiddleware all funnel through — checksbearerToken(), then theAuthorizationheader, then falls back to the cookie. A cookie-only request already authenticated.
config/auth.ts sets defaultTokenName: 'auth-token' and declares no cookie
block, so the resolved name is auth-token — already what the store and
resources/middleware/auth.ts key off. The three stay in lock-step for free.
So the change is just moving the write to the server
app/Support/sessionCookie.ts—withSessionCookie/withClearedSessionCookie, bothCache-Control: no-store, since a cached auth response would hand one visitor another's session.app/Actions/Auth/{Login,Register,Logout}Action.ts— app-local overrides, behaviourally identical to the framework defaults except for theSet-Cookie. App actions shadow defaults, so this needs no edit to the vendoredstorage/framework/tree that a sync would revert. JSON bodies stay byte-compatible with the framework shape for non-browser clients.resources/stores/auth.ts— no longer writes or reads the credential. A module-levelsessionTokenmirrors it in memory for theAuthorizationheader within the tab that logged in; it is never persisted and dies on reload, after which the cookie carries the session alone.clearAuthCookies()now only clears the non-credential user cookie, because adocument.cookieexpiry for anHttpOnlycookie is ignored by the browser — the server clears that one.
Two consequences worth stating
isAuthenticated()is now keyed offuser, nottoken. The token is in-memory only, so it is empty after a reload while the session is still perfectly valid. Two callers usedtoken()as an "am I signed in" flag and would have regressed on every hard refresh — the notifications store (the header bell silently stops fetching) andAdminShell(a valid admin session bounced to/admin/login). Both now readisAuthenticated().- The host's same-origin
/api/*proxy is now load-bearing for auth, not just convenience: the cookie isSameSite=Laxand is only sent same-site. That proxy is already in the deploy contract (DEPLOY.md, landing in #76).
CSRF posture is unchanged, not worsened. requestToken() already accepted
the cookie before this, so cookie-borne requests were always possible, and
SameSite=Lax is what withholds it on cross-site POST/PUT/DELETE. That is what
keeps the blanket .skipCsrf() in routes/api.ts defensible, and it holds as
long as no GET mutates state — currently true, and worth keeping true.
Verification
lint | PASS |
typecheck:app | PASS (0 errors) |
tests/unit/session-cookie.test.ts | 8 tests, 23 assertions, PASS |
gate | 16/17 — unchanged; the one failure is the pre-existing empty-database check |
The strongest check: the built bundle now contains zero auth-token= cookie
writes, where it previously wrote the credential on every login.
The new tests cover the cookie contract (HttpOnly, resolved name, server-side
round-trip through authCookieToken(), expiry on clear, loopback Secure
handling), both response helpers, and the routes: [] opt-out from part 1.
I confirmed app actions genuinely shadow framework defaults rather than
trusting the comment that says so: this repo's app/Actions/Auth/VerifyEmailAction.ts
is already a distinct 33-line override of the 25-line default, routed through
the same 'Actions/Auth/…' string form, and that is the one the flow runs.
Not verified
A real login → reload → still-signed-in round trip. That needs the dev server and a seeded user (the database is empty), so it is worth exercising before deploy. The failure mode if the cookie somehow isn't sent is graceful — the user appears logged out rather than locked out, and can sign in again.
Also note tests/feature/ui.test.ts fails on main already (document is not
defined — a placeholder stub with no DOM registered), so the test job stays
red independently of this PR.
🤖 Generated with Claude Code