From fd04564602fcc30199ee30aa4c378d7485421f64 Mon Sep 17 00:00:00 2001 From: iris Date: Fri, 31 Jul 2026 01:20:00 +0200 Subject: [PATCH] admin: expire sessions server-side, mask unconfigured-password 500 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two low-severity findings from the #47 review round (#52): 1. validSessions was a bare Set — the 7-day cookie maxAge was a browser-side hint only, so a token stayed valid forever server-side until an explicit /admin/logout or a process restart. Now a Map, checked and pruned lazily on lookup, with an opportunistic full sweep on login so an abandoned session doesn't linger in memory indefinitely either. 2. /admin/login on a misconfigured deploy (ADMIN_PASSWORD unset) returned the literal string "ADMIN_PASSWORD not set" to an unauthenticated caller, bypassing the generic-500 masking every other 500 in the app goes through (see index.ts's error handler) — minor recon value for anyone probing. Now logged server-side and masked like any other internal error. --- server/src/routes/admin.ts | 41 +++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/server/src/routes/admin.ts b/server/src/routes/admin.ts index 62c7b6c..ab1430b 100644 --- a/server/src/routes/admin.ts +++ b/server/src/routes/admin.ts @@ -18,11 +18,34 @@ function isValidCents(v: unknown): v is number { // cookie only yields one revocable session (not the shared secret), and // logout actually invalidates it. In-memory: a restart naturally logs // everyone out too, which is fine here (single-process deploy). -const validSessions = new Set(); +// +// Maps each token to its own expiry (ms epoch) so the 7-day cookie maxAge +// is honoured server-side too, not just as a hint the browser can ignore — +// previously a token stayed valid indefinitely until an explicit +// /admin/logout or a process restart. +const SESSION_MAXAGE_SECONDS = 60 * 60 * 24 * 7; +const validSessions = new Map(); function isAuthed(req: FastifyRequest): boolean { const token = req.cookies?.[SESSION_COOKIE]; - return !!token && validSessions.has(token); + if (!token) return false; + const expires = validSessions.get(token); + if (expires === undefined) return false; + if (Date.now() > expires) { + validSessions.delete(token); + return false; + } + return true; +} + +// Opportunistic sweep of expired tokens — called on login (a rare path +// relative to isAuthed, which runs on every authed request) so an +// abandoned session's memory doesn't linger forever between lookups. +function pruneExpiredSessions(): void { + const now = Date.now(); + for (const [token, expires] of validSessions) { + if (now > expires) validSessions.delete(token); + } } // Constant-time comparison for the admin password — hashed first so a @@ -255,17 +278,25 @@ function computeStats(db: DB) { export function registerAdminRoutes(app: FastifyInstance, db: DB) { app.post<{ Body: { password?: string } }>('/admin/login', async (req, reply) => { const expected = process.env.ADMIN_PASSWORD; - if (!expected) return sendProblem(req, reply, 500, 'Internal Server Error', 'ADMIN_PASSWORD not set'); + if (!expected) { + // Masked like every other 500 (see index.ts's error handler) — the + // literal "ADMIN_PASSWORD not set" used to go straight to an + // unauthenticated caller, confirming a misconfigured deploy (no + // password will ever work) to anyone probing. + req.log.error('ADMIN_PASSWORD not set'); + return sendProblem(req, reply, 500, 'Internal Server Error', 'internal error'); + } if (!safeEqual(req.body?.password ?? '', expected)) { return sendProblem(req, reply, 401, 'Unauthorized', 'wrong password'); } + pruneExpiredSessions(); const token = randomBytes(32).toString('hex'); - validSessions.add(token); + validSessions.set(token, Date.now() + SESSION_MAXAGE_SECONDS * 1000); reply.setCookie(SESSION_COOKIE, token, { path: '/', httpOnly: true, sameSite: 'lax', - maxAge: 60 * 60 * 24 * 7, + maxAge: SESSION_MAXAGE_SECONDS, }); return { ok: true }; });