TL;DR
Three session-related security findings from a downstream audit (stacksjs/stacks#1878). Filing as a tracker so the upstream fix lands cleanly and Stacks can drop its pantry-side workarounds.
Se-1: File-store sanitization silently collides session IDs (HIGH)
src/session/file-store.ts:35. The SID sanitizer is sid.replace(/[^a-zA-Z0-9_-]/g, '') — invalid characters get stripped instead of the request being rejected. Both 'a/b' and 'ab' resolve to the same file. A hostile or buggy client can collide with another user's session by submitting a malformed SID.
Suggested fix: strict validation against ^[A-Za-z0-9_-]{1,128}$. Throw or return null on mismatch so the session is treated as missing (caller regenerates a fresh SID).
private filePath(sid: string): string {
if (!/^[a-zA-Z0-9_-]{1,128}$/.test(sid)) {
throw new Error('[session/file-store] invalid session id')
}
return join(this.dir, `${sid}.json`)
}Se-2: No loginAs(userId) / forgetUser() helpers — session fixation surface (HIGH)
src/session/index.ts:184. regenerate() exists but the framework provides no convention-over-config helper for the "session must be regenerated on auth state change" pattern. Apps that forget to call regenerate() in their login handler are vulnerable to session-fixation — an attacker plants a known SID on the victim's browser, victim logs in, attacker hijacks.
Suggested fix: add two methods to SessionManager:
/** Mark the session as belonging to a newly-authed user.
* Regenerates the SID so a pre-auth fixed cookie can't ride along. */
async loginAs(userId: number | string): Promise<string> {
const newId = await this.regenerate()
this.data.userId = userId
await this.store.set(newId, this.data)
return newId
}
/** Mark the session as logged out. Regenerates the SID and drops
* the userId; flash messages / intended URL persist. */
async forgetUser(): Promise<string> {
delete this.data.userId
return this.regenerate()
}regenerate() should also keep data.id in sync with the new SID so consumers reading session.data.id after a regenerate see the right value.
Se-3: CSRF tokens live in a process-wide static map (HIGH)
src/middleware/csrf.ts:6. Tokens are stored in Csrf.tokens = new Map<string, string>(). Problems:
- Static map is lost on server restart — every user gets a 419 on the first form submit after a deploy.
- No per-session isolation — two users can collide on the same token (low probability but undetected).
Csrf.tokens.set(token, token)is wasteful — the map's value duplicates the key.
Suggested fix: persist the token inside the session payload under a reserved key (_csrf). Fall back to the static map only when no session middleware is installed.
const CSRF_SESSION_KEY = '_csrf'
private storeToken(req: EnhancedRequest, token: string): void {
const session = (req as any).session
if (session?.data) {
session.data[CSRF_SESSION_KEY] = token
return
}
Csrf.tokens.set(token, token) // fallback only
}
private tokenIsKnown(req: EnhancedRequest, token: string): boolean {
const session = (req as any).session
if (session?.data) return session.data[CSRF_SESSION_KEY] === token
return Csrf.tokens.has(token)
}Why upstream
Stacks pinned a local-pantry copy of bun-router and applied these patches there as a workaround for the audit. The pantry is gitignored so the fixes don't survive a clean checkout; landing them in stacksjs/bun-router lets every consumer benefit and unblocks Stacks dropping the pantry patches.
No urgency — Stacks runs with the patched pantry locally. Stable upstream API for the new loginAs / forgetUser helpers is the long-term goal.
Related: stacksjs/stacks#1878 Se-4 / Se-5 / Se-6
These additional session findings are deferred (encryption-at-rest, concurrent-write protection, cookie SameSite defaults). Worth tracking but not blocking.