security: app-side auth & public-surface hardening — token revocation, cookie flags, admin MFA + anonymized-reviewer masking, enumeration, throttles, upload validation

Problem

Bench publishes anonymous reviews of named judges, so account compromise and identity unmasking carry real retaliation risk. Six gaps share an app-side auth/abuse theme: credentials that survive password reset, a JS-readable 30-day token cookie, a single-password admin panel that exposes anonymized reviewers' real identities, an enumerable registration endpoint, unthrottled full-table-scan public endpoints, and an upload pipeline trusting client-declared MIME. Framework root causes are filed upstream where applicable; this issue tracks the app-side fixes.

Findings

• Password reset/change never revoke existing tokens (high). Login mints a 60-min access + 30-day refresh token, but neither resetPassword() (storage/framework/core/auth/src/password/reset.ts:199-275) nor app/Actions/Me/ChangePasswordAction.ts:83-90 revokes anything — the latter's comment (lines 27-31) documents this deliberately. revokeAllTokens/revokeOtherTokens (storage/framework/core/auth/src/tokens.ts:694,712) have zero callers and there is no "log out everywhere" endpoint, so a stolen refresh token survives the victim's recovery action for up to 30 days.

• Bearer token in a non-HttpOnly, non-Secure, 30-day cookie (medium). resources/stores/auth.ts:37-44 writes auth-token via document.cookie (XSS = token theft); the Secure flag exists only as a comment (auth.ts:41-42). The auth-user cookie mirrors full user JSON with the same flags, and admin sessions reuse this plumbing (resources/stores/admin.ts).

• Admin panel is the anonymity-unmasking surface, behind a single password (medium). app/Actions/Admin/Reviews/ReviewIndexAction.ts:81,140 returns user: { id, name, email } for every review including anonymized ones. Admin login (app/Actions/Admin/Auth/AdminLoginAction.ts:40-67) is email+password only; no MFA/step-up exists anywhere, though app/Models/User.ts:35 declares usePasskey: true with a passkeys migration, unwired.

• User enumeration on registration (medium). storage/framework/core/auth/src/register.ts:33-35 throws a distinct 409 "Email already exists" (pre-bcrypt, adding a timing signal); login and reset are correctly uniform, making this the only enumerable auth surface.

• Public search/index endpoints: no throttle, full table scans (medium). routes/api.ts:112-123 (/judges, /judges/search, /court-houses) carry no throttle while login/reviews/comments do; JudgeSearchAction.ts:37-40 and JudgeIndexAction.ts:22-25 run Judge.all() + CourtHouse.all() per request — a cheap unauthenticated DoS amplifier and unmetered judge-directory scrape.

• Photo upload trusts client MIME, no throttle or quota (low). app/Helpers/reviewPhotos.ts:54-57 validates the client-supplied file.type (UploadReviewPhotosAction.ts:84); the route (routes/api.ts:166-169) has no throttle and no per-user byte quota, so a delete+reupload loop yields unbounded sharp work on 8MB inputs.

Acceptance criteria

• revokeAllTokens (or revokeOtherTokens) called after the password update in both the reset flow and ChangePasswordAction
• POST /api/me/logout-all wired to revokeAllTokens
• Secure flag actually applied when APP_URL is https; cookie max-age shortened to access-token TTL with refresh renewal — or server-set HttpOnly cookie + CSRF adopted (or the risk explicitly documented)
• Admin login requires a second factor (email OTP minimum; passkeys are the existing unwired path)
• ReviewIndexAction masks name/email for anonymized reviews; reveal is an explicit per-row action logged to moderation_logs
• Registration returns a uniform response for existing emails (disambiguate out-of-band via email)
• /judges, /judges/search, /court-houses throttled and queries bounded (SQL LIKE + LIMIT, no Judge.all() in-memory filter)
• Uploads validated by magic bytes (or sharp decode treated as canonical), route throttled, per-user aggregate photo/byte quota enforced

From the 2026-06-10 production-readiness audit.

Upstream (framework root causes):

• https://github.com/stacksjs/stacks/issues/1947 — password reset never revokes tokens
• https://github.com/stacksjs/stacks/issues/1953 — register() enumeration oracle

Closing notes

All eight criteria are done. Three of them (token revocation, logout-all, the cookie) were already satisfied before this pass and were just unticked — the audit is in the commits below.

Decisions worth recording

Masked authors keep a pseudonym. Nulling the author outright would have removed a moderator's ability to tell that several anonymous reviews are the same person, which is usually the question they actually have — and would have pushed them to reveal every time, making the reveal log noise. Masked rows carry author_ref, an HMAC of the user id, stable per author and carrying no identity. It must be keyed: user ids are small sequential integers, so an unkeyed digest is reversible offline by anyone who can count.

The reveal's audit write is not best-effort. Every other logModeration call in the codebase swallows its errors so moderation is never blocked by logging. This one refuses the reveal if the log cannot be written, because a reveal whose record failed is exactly the silent de-anonymization the control exists to prevent.

Email, not TOTP, for the second factor. The framework already ships TOTP and it is stronger. It also requires every admin to enrol first, and a required factor nobody has enrolled in is an admin panel nobody can log into. Swapping in verifyTwoFactorLoginCode once enrolment exists is a one-line change; the rest of the flow is unchanged. Mail failure fails closed, except in a development environment (allowlisted env names, not !== production) where the code goes to the console.

Registration's 409 was already unreachable. register() now hashes before checking and returns a generic 422 (stacksjs/stacks#1953), and the app's isUniqueViolation catch does not match that HttpError — verified by probing the live path, not by reading it. It was removed anyway, since it was one config key from being reachable and the behavioural tests structurally cannot see it. A third test reads the source for that reason.

Deliberately not done

/judges still returns the whole directory. The query is now a single bounded LEFT JOIN instead of Judge.all() + CourtHouse.all() (29ms → 12ms on the real 2,740-judge dataset), but the response is still every judge, because eight components derive from the full array client-side — the directory's own filter and pagination, the category counts, the courthouse pages, the review form's picker, and a single-judge lookup that does .find() over it. Capping the rows would silently truncate all of them. Server-side pagination is the right end state and is a frontend change, not a security fix. Both list endpoints now carry a loud ceiling so an unbounded SELECT cannot become the thing that falls over without anyone having touched it.

Admin comment moderation was left as it is. CommentIndexAction resolves the author deliberately (bench-review#51). Whether it should adopt the same mask-plus-reveal shape is a judgement call that belongs with that issue rather than being decided inside this one.

Found on the way

Admin login never installed the session cookie. LoginAction and RegisterAction call withSessionCookie; AdminLoginAction returned a bare JSON token, so an admin session lived only in one tab's memory and did not survive a reload — while the comment in resources/stores/admin.ts still claimed persistAuth wrote the cookie. An app-side gap from when the HttpOnly work landed, not a framework one. Fixed as part of the MFA rework, which installs the cookie on the verify step.

A judge's location rendered as ", DC" whenever the courthouse had a state and no city, which is most of the federal appellate courts in the import. Fixed in e4a8a5bf.

All eight criteria are met and on main (72030b3f…c37c78a4), CI green. Three of them — token revocation, logout-all, the HttpOnly cookie — were already satisfied before this pass and were only unticked.

Highlights, decisions and the two things deliberately left undone are written up in the issue body above.