fix(auth): harden post-login redirect validation

[dobby-coder]

Summary

The post-login redirect query param was passed straight into SvelteKit's redirect() (server load, +page.server.ts) and goto() (client, +page.svelte) without any validation. An attacker could craft a link such as:

• ?redirect=https://evil.example
• ?redirect=//evil.example (protocol-relative)

to bounce a freshly-logged-in user off-site — a classic open redirect (see draft advisory GHSA-mr47-cc7w-r794, tracked in #123).

Changes

• Add a small, co-located, unit-tested helper src/routes/auth/login/safe-redirect.ts exporting safeRedirect(). It only accepts same-origin, path-absolute targets:
  • must start with a single /
  • rejects absolute URLs (https://…), protocol-relative //host, and the /\host backslash variant that browsers normalise to protocol-relative
  • falls back to /portal/dashboard for anything else (including empty/missing)
• Wire the guard into +page.server.ts (redirect()) and into handleSuccess() in +page.svelte (before goto()).
• Add tests/unit/safe-redirect.test.ts covering accepted paths, missing values, non-slash values, protocol-relative URLs, and the backslash bypass.

Testing

• npx vitest run — 99 passed (14 files), including the 5 new cases.
• npm run check (svelte-check) — 0 errors, 0 warnings.
• prettier --check + eslint --max-warnings 0 on the changed files — clean.

Refs #123

[dobby-coder]

Rules Dobby 2 — consolidated review (cycle 1)

Verdict is request-changes — posted as COMMENT only because GitHub blocks self-request-changes on a bot-authored PR. Treat the blocking item below as blocking.

Ran the per-rule compliance checks (incl. repo-specific security notes) and merged them with Review Dobby 2's findings. One blocking security issue remains; the fix is otherwise well-structured and well-tested.

🔴 Blocking: open-redirect control-character bypass still exploitable

safeRedirect() still lets control characters through. safeRedirect('/\t/evil.example') (also /\n/… and /\r/…) starts with a single / and its 2nd char is neither / nor \, so it is accepted and returned unchanged. Browsers strip ASCII TAB/LF/CR from URLs (WHATWG URL spec), so /<tab>/evil.example normalises to //evil.example → https://evil.example/.

I reproduced this on the checked-out branch:

"/\t/evil.example" -> returned "/\t/evil.example" | new URL(out, origin): https://evil.example/
"/\n/evil.example" -> returned "/\n/evil.example" | new URL(out, origin): https://evil.example/
"/\r/evil.example" -> returned "/\r/evil.example" | new URL(out, origin): https://evil.example/

This affects both the server redirect(303, …) path (TAB is a valid Location header char) and the client goto() path — it is the exact class of bypass this PR exists to prevent.

Fix: reject control chars before the slash checks, e.g.

if (/[\u0000-\u001f]/.test(target)) return DEFAULT_REDIRECT;

and add a regression test covering /\t/, /\n/, /\r/.

🟡 Non-blocking: PR title names the vulnerability class

Per the no-public-security-issues rule, don't name the vuln in a public PR title while the advisory (GHSA-mr47-cc7w-r794) is still a draft. Consider a generic title such as fix(auth): harden post-login redirect validation — still conventional-commit compliant.

✅ Passing checks

• Conventional-commit PR title format — OK
• Tests present (rejection + happy paths) — OK
• SvelteKit route exports (logic extracted to sibling safe-redirect.ts) — OK
• Delivered scope matches the /dobby fix request — OK
• No repo-specific security pitfalls introduced — OK

Sending back to the PR Dobby to fix the control-char bypass.