From 446a4bc2d239fc6bf4c3af117edc53f65ef70144 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Tue, 14 Jul 2026 10:25:37 +0200 Subject: [PATCH] fix(web): address auth review findings (open redirect, dead code, HMAC per-req) Pre-merge review of the P0 branch: - Open redirect: login redirected to an unvalidated ?next param, so a crafted /login?next=https://evil (or //evil) bounced off-site after login. Now only same-origin relative paths are honored (else fall back to "/"). - Remove the unused useRouter import/binding in the login form. - Memoize expectedToken() keyed on (password, secret) so middleware stops recomputing an HMAC on every gated request. Deferred (low-value/latent, noted): Soulseek cover.jpg isn't retrieved into staging (UI still shows Cover Art Archive art); _retrieve's basename-keyed map could undercount same-named files but source_refs come from a single slskd dir; SignOut visibility rides a server-env read (cosmetic). web 138 tests green, tsc + build clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- web/src/app/login/login-form.tsx | 8 +++++--- web/src/lib/auth.ts | 9 ++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/web/src/app/login/login-form.tsx b/web/src/app/login/login-form.tsx index 7f12e1b..a7134a4 100644 --- a/web/src/app/login/login-form.tsx +++ b/web/src/app/login/login-form.tsx @@ -1,12 +1,14 @@ "use client"; import { useState } from "react"; -import { useRouter, useSearchParams } from "next/navigation"; +import { useSearchParams } from "next/navigation"; export function LoginForm() { - const router = useRouter(); const params = useSearchParams(); - const next = params.get("next") || "/"; + // Only allow same-origin relative paths, so a crafted ?next=https://evil or ?next=//evil + // can't turn login into an open redirect. + const raw = params.get("next") || "/"; + const next = raw.startsWith("/") && !raw.startsWith("//") ? raw : "/"; const [password, setPassword] = useState(""); const [error, setError] = useState(""); const [busy, setBusy] = useState(false); diff --git a/web/src/lib/auth.ts b/web/src/lib/auth.ts index d43e733..edeb2b6 100644 --- a/web/src/lib/auth.ts +++ b/web/src/lib/auth.ts @@ -13,11 +13,16 @@ function toB64(bytes: Uint8Array): string { return btoa(s); } +// Memoize the derived token so the middleware doesn't recompute an HMAC on every request +// (password/secret don't change at runtime; keyed on both so a change still recomputes). +let _cache: { password: string; secret: string; token: string } | null = null; + /** The token a valid session cookie must carry, or "" when auth is disabled (no password). */ export async function expectedToken(): Promise { const password = process.env.LYRA_PASSWORD; if (!password) return ""; const secret = process.env.LYRA_SECRET_KEY ?? "lyra"; + if (_cache && _cache.password === password && _cache.secret === secret) return _cache.token; const enc = new TextEncoder(); const key = await crypto.subtle.importKey( "raw", @@ -27,7 +32,9 @@ export async function expectedToken(): Promise { ["sign"], ); const sig = await crypto.subtle.sign("HMAC", key, enc.encode(password)); - return toB64(new Uint8Array(sig)); + const token = toB64(new Uint8Array(sig)); + _cache = { password, secret, token }; + return token; } /** Constant-time string compare (both operands are our own derived tokens). */