Multi-agent security review of 9856c04 (4 dimensions, 2-skeptic verification):
- CRITICAL functional+security: the session cookie inherited h3's Secure=true default;
browsers DROP Secure cookies over plain http://, so login silently failed on a LAN HTTP
client (worked only on localhost, a secure context — which is why the live test passed).
Now set the cookie attributes explicitly: HttpOnly + SameSite=Lax + Path=/, and Secure
only when PUNKTFUNK_UI_SECURE=1 (behind TLS). Verified: Set-Cookie no longer has Secure.
- Gate bypass: isPublicPath allowlisted any path ending in .json/.css/.png/etc., so
/api/v1/openapi.json (served unauthenticated on the mgmt side too) leaked the whole API
schema through the token-injecting proxy. Now /api is ALWAYS gated and the generic
extension allowlist is gone (client assets are all under /assets/, still allowlisted).
Verified: /api/v1/openapi.json and /api/v1/status.json → 401.
- Session lifetime: added maxAge (7d) — bounds a stolen cookie (cookie Max-Age + iron seal
TTL); previously never expired.
- Open redirect: the post-login `next` accepted protocol-relative `//evil.com`. Hardened
client + added safeNextPath() (same-origin path only).
Re-validated end to end: login assets public (200), /api/openapi.json gated (401), authed
/api/v1/status (200), unauth /→302. tsc + build green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+31
-5
@@ -34,7 +34,22 @@ export function sessionConfig(): SessionConfig {
|
||||
const password = secret && secret.length >= 32
|
||||
? secret
|
||||
: createHash('sha256').update(`punktfunk-session-v1:${uiPassword()}`).digest('hex')
|
||||
return { name: SESSION_NAME, password }
|
||||
return {
|
||||
name: SESSION_NAME,
|
||||
password,
|
||||
// Bounds a stolen/replayed cookie's lifetime (sets the cookie Max-Age AND the iron
|
||||
// seal TTL). 7 days for a single-user console.
|
||||
maxAge: 60 * 60 * 24 * 7,
|
||||
cookie: {
|
||||
httpOnly: true,
|
||||
sameSite: 'lax',
|
||||
path: '/',
|
||||
// h3 defaults Secure to true, which browsers DROP over plain http:// (so login
|
||||
// silently fails on a LAN HTTP server). Only mark Secure when actually behind TLS
|
||||
// (set PUNKTFUNK_UI_SECURE=1 / =true then).
|
||||
secure: /^(1|true)$/i.test(process.env.PUNKTFUNK_UI_SECURE ?? ''),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
/** Constant-time string comparison (avoids leaking the password via timing). */
|
||||
@@ -45,18 +60,29 @@ export function timingSafeEqual(a: string, b: string): boolean {
|
||||
return nodeTimingSafeEqual(ab, bb)
|
||||
}
|
||||
|
||||
/** Paths reachable WITHOUT a session: the login page, the auth endpoints, and static
|
||||
* assets (the login page needs its own CSS/JS). Everything else is gated. */
|
||||
/** Paths reachable WITHOUT a session: the login page, the auth endpoints, and the build's
|
||||
* static assets (the login page needs its own CSS/JS, all of which live under /assets/).
|
||||
* Everything else — crucially ALL of /api — is gated.
|
||||
*
|
||||
* Note: do NOT allowlist by file extension. The client assets are all under /assets/, and a
|
||||
* generic `*.json` allowlist would expose `/api/v1/openapi.json` (and any future
|
||||
* `.json`/`.png` management route) through the proxy unauthenticated. */
|
||||
export function isPublicPath(pathname: string): boolean {
|
||||
if (pathname === '/api' || pathname.startsWith('/api/')) return false // always gated
|
||||
if (pathname === '/login') return true
|
||||
if (pathname.startsWith('/_auth/')) return true
|
||||
if (pathname.startsWith('/assets/')) return true
|
||||
if (pathname === '/favicon.ico' || pathname === '/robots.txt') return true
|
||||
// Vite/TanStack client chunks and source maps requested by the login page.
|
||||
if (/\.(js|css|map|ico|svg|png|woff2?|json)$/.test(pathname)) return true
|
||||
return false
|
||||
}
|
||||
|
||||
/** Validate a post-login redirect target: a same-origin path only. Rejects protocol-
|
||||
* relative (`//evil.com`) and absolute URLs to prevent an open redirect. */
|
||||
export function safeNextPath(next: string | undefined): string {
|
||||
if (!next || !next.startsWith('/') || next.startsWith('//')) return '/'
|
||||
return next
|
||||
}
|
||||
|
||||
export interface SessionData {
|
||||
authenticated?: boolean
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user