fix(auth): require the request on findSessionOrFail

findSessionOrFail took `req` as optional, so the quiet default was the
unscoped lookup: a route that forgot to pass it compiled cleanly and, in
multi-user mode, returned another user's session. `req` is now required,
and a caller with no request in flight must use the explicit, greppable
findSessionUnscopedOrFail instead.

Every current caller in src/ already passes req (verified by grep and
tsc), so behaviour is unchanged; the new helper has no production
callers and exists so the next request-less caller does not make `req`
optional again.

Tests pin the unscoped helper's behaviour and assert, from the source
AST, that `req` is a required parameter (tsc does not typecheck test/).
This commit is contained in:
Aamer Akhter committed 2026-10-22 17:23:03 -04:00
1 parent 002de32a08
commit 4f9274af3a
2 files changed
+53 -5

No files matched your search

+26 -4
View File
@@ -324,11 +324,33 @@ function sessionNotFoundError(sessionId: string): Error & { statusCode: number;
* Look up a session by ID or throw a structured error.
* Replaces the pattern: `const session = sessions.get(id); if (!session) return createErrorResponse(...)`.
*
* When `req` is passed in multi-user mode, a session the caller does not own is
* reported as NOT_FOUND (never 403), so existence of other users' sessions is not
* leaked. Single-user / admin callers are unaffected.
* In multi-user mode a session the caller does not own is reported as NOT_FOUND
* (never 403), so existence of other users' sessions is not leaked. Single-user /
* admin callers are unaffected (`canAccessOwned` short-circuits when multi-user is off).
*
* `req` is REQUIRED. When it was optional, forgetting it compiled cleanly and
* silently performed an UNSCOPED lookup. A caller with no request in flight must
* say so at the call site via {@link findSessionUnscopedOrFail}.
*/
export function findSessionOrFail(ctx: SessionPort, sessionId: string, req?: FastifyRequest): Session {
export function findSessionOrFail(ctx: SessionPort, sessionId: string, req: FastifyRequest): Session {
return lookupSessionOrFail(ctx, sessionId, req);
}
/**
* Look up a session WITHOUT ownership scoping. Deliberately verbose and greppable:
* every call is a decision to skip the multi-user ownership check and should carry
* a comment saying why (e.g. a startup path with no request in flight). Inside a
* request handler use `findSessionOrFail(ctx, id, req)` instead.
*
* It has no production callers today: every lookup in `src/` is request-scoped. It
* exists so the next genuinely request-less caller has somewhere to go other than
* making `req` optional again.
*/
export function findSessionUnscopedOrFail(ctx: SessionPort, sessionId: string): Session {
return lookupSessionOrFail(ctx, sessionId, undefined);
}
function lookupSessionOrFail(ctx: SessionPort, sessionId: string, req: FastifyRequest | undefined): Session {
const session = ctx.sessions.get(sessionId);
if (!session) throw sessionNotFoundError(sessionId);
if (req && !canAccessOwned(getAuthUser(req), session.owner)) throw sessionNotFoundError(sessionId);