From c9ad74a620672803ee7e1a93a1208d1781ecafc5 Mon Sep 17 00:00:00 2001
From: enricobuehler <buehler@unom.io>
Date: Wed, 10 Jun 2026 18:55:41 +0000
Subject: [PATCH] =?UTF-8?q?fix(web):=20harden=20BFF=20auth=20=E2=80=94=20a?=
 =?UTF-8?q?dversarial-review=20fixes?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 web/server/util/auth.ts  | 36 +++++++++++++++++++++++++++++++-----
 web/src/routes/login.tsx |  6 ++++--
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/web/server/util/auth.ts b/web/server/util/auth.ts
index 9f32b03..80c2919 100644
--- a/web/server/util/auth.ts
+++ b/web/server/util/auth.ts
@@ -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
 }
diff --git a/web/src/routes/login.tsx b/web/src/routes/login.tsx
index f4d4fd2..e2e2e5c 100644
--- a/web/src/routes/login.tsx
+++ b/web/src/routes/login.tsx
@@ -38,8 +38,10 @@ function LoginPage() {
         setBusy(false)
         return
       }
-      // Full reload to the target so SSR re-runs WITH the new session cookie.
-      window.location.href = next && next.startsWith('/') ? next : '/'
+      // Full reload to the target so SSR re-runs WITH the new session cookie. Only a
+      // same-origin path — reject protocol-relative/absolute URLs (open-redirect guard).
+      const safe = next && next.startsWith('/') && !next.startsWith('//') ? next : '/'
+      window.location.href = safe
     } catch {
       setError(true)
       setBusy(false)