security: harden auth next-path validation (#4587)

* security: harden auth next-path validation

* chore: address auth review nits

backend/app/gateway/routers/auth.py

@@ -827,7 +827,8 @@ async def oauth_callback(
     # ── Issue DeerFlow session ───────────────────────────────────────
     token = create_access_token(str(user.id), token_version=user.token_version)
 
-    redirect_target = state_payload.next_path or "/workspace"
+    # Revalidate as defense-in-depth if future state writers populate this target.
+    redirect_target = validate_next_param(state_payload.next_path) or "/workspace"
     frontend_base = oidc_config.frontend_base_url or ""
     callback_redirect = f"{frontend_base}/auth/callback?next={urllib.parse.quote(redirect_target)}"
 
@@ -855,7 +856,8 @@ def validate_next_param(next_param: str | None) -> str | None:
     """Validate and sanitize the ``next`` redirect parameter.
 
     Only allows relative paths starting with ``/``. Rejects protocol-relative
-    URLs (``//``), absolute URLs, and URLs with embedded protocols.
+    URLs (``//``), absolute URLs, URLs with embedded protocols, and backslashes
+    that URL parsers may reinterpret as forward slashes.
     """
     if not next_param:
         return None
@@ -863,6 +865,8 @@ def validate_next_param(next_param: str | None) -> str | None:
         return None
     if next_param.startswith("//") or next_param.startswith("http://") or next_param.startswith("https://"):
         return None
+    if "\\" in next_param:
+        return None
     if ":" in next_param:
         return None
     return next_param

backend/tests/test_auth.py

@@ -1130,8 +1130,13 @@ def test_authenticate_skips_rehash_for_v2_hash():
     mock_repo.update_user.assert_not_called()
 
 
-def test_validate_next_param_rejects_colon_paths():
+def test_validate_next_param_rejects_unsafe_paths():
     from app.gateway.routers.auth import validate_next_param
 
     assert validate_next_param("/workspace") == "/workspace"
+    assert validate_next_param("/workspace/chats/new?tab=recent#top") == "/workspace/chats/new?tab=recent#top"
     assert validate_next_param("/:evil") is None
+    assert validate_next_param("/\\evil.example") is None
+    assert validate_next_param("/foo\\bar") is None
+    assert validate_next_param("//evil.example") is None
+    assert validate_next_param("https://evil.example") is None

frontend/src/app/(auth)/auth/callback/page.tsx

@@ -1,19 +1,9 @@
 "use client";
 
 import { useRouter, useSearchParams } from "next/navigation";
-import { useEffect, useState, useCallback, useRef } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 
-/**
- * Validates the next parameter — only allows relative paths starting with /.
- */
-function validateNextParam(next: string | null): string {
-  if (!next) return "/workspace";
-  if (!next.startsWith("/") || next.startsWith("//")) return "/workspace";
-  if (next.startsWith("http://") || next.startsWith("https://"))
-    return "/workspace";
-  if (next.includes(":")) return "/workspace";
-  return next;
-}
+import { resolveAuthNextPath } from "@/core/auth/next-path";
 
 export default function AuthCallbackPage() {
   const router = useRouter();
@@ -27,7 +17,7 @@ export default function AuthCallbackPage() {
     if (calledRef.current) return;
     calledRef.current = true;
 
-    const next = validateNextParam(searchParams.get("next"));
+    const next = resolveAuthNextPath(searchParams.get("next"));
 
     try {
       const res = await fetch("/api/v1/auth/me", { credentials: "include" });

frontend/src/app/(auth)/login/page.tsx

@@ -10,6 +10,7 @@ import { Button } from "@/components/ui/button";
 import { FlickeringGrid } from "@/components/ui/flickering-grid";
 import { Input } from "@/components/ui/input";
 import { useAuth } from "@/core/auth/AuthProvider";
+import { resolveAuthNextPath } from "@/core/auth/next-path";
 import {
   loadRememberLoginPreference,
   saveRememberLoginPreference,
@@ -22,39 +23,6 @@ import {
 import { parseAuthError } from "@/core/auth/types";
 import { useI18n } from "@/core/i18n/hooks";
 
-/**
- * Validate next parameter
- * Prevent open redirect attacks
- * Per RFC-001: Only allow relative paths starting with /
- */
-function validateNextParam(next: string | null): string | null {
-  if (!next) {
-    return null;
-  }
-
-  // Need start with / (relative path)
-  if (!next.startsWith("/")) {
-    return null;
-  }
-
-  // Disallow protocol-relative URLs
-  if (
-    next.startsWith("//") ||
-    next.startsWith("http://") ||
-    next.startsWith("https://")
-  ) {
-    return null;
-  }
-
-  // Disallow URLs with different protocols (e.g., javascript:, data:, etc)
-  if (next.includes(":") && !next.startsWith("/")) {
-    return null;
-  }
-
-  // Valid relative path
-  return next;
-}
-
 export default function LoginPage() {
   const router = useRouter();
   const searchParams = useSearchParams();
@@ -94,7 +62,7 @@ export default function LoginPage() {
 
   // Get next parameter for validated redirect
   const nextParam = searchParams.get("next");
-  const redirectPath = validateNextParam(nextParam) ?? "/workspace";
+  const redirectPath = resolveAuthNextPath(nextParam);
   const regularSignupAllowed = canCreateRegularAccount({
     // A failed probe must not expose registration while the system's setup
     // state is unknown. Existing users can still sign in normally.

frontend/src/core/auth/next-path.ts

@@ -0,0 +1,26 @@
+export const DEFAULT_AUTH_NEXT_PATH = "/workspace";
+
+export function validateAuthNextPath(
+  nextPath: string | null | undefined,
+): string | null {
+  if (!nextPath) {
+    return null;
+  }
+  if (!nextPath.startsWith("/")) {
+    return null;
+  }
+  if (nextPath.startsWith("//")) {
+    return null;
+  }
+  if (nextPath.includes("\\") || nextPath.includes(":")) {
+    return null;
+  }
+  return nextPath;
+}
+
+export function resolveAuthNextPath(
+  nextPath: string | null | undefined,
+  fallback = DEFAULT_AUTH_NEXT_PATH,
+): string {
+  return validateAuthNextPath(nextPath) ?? fallback;
+}

frontend/tests/unit/core/auth/next-path.test.ts

@@ -0,0 +1,30 @@
+import { describe, expect, test } from "@rstest/core";
+
+import {
+  resolveAuthNextPath,
+  validateAuthNextPath,
+} from "@/core/auth/next-path";
+
+describe("auth next path validation", () => {
+  test("accepts local absolute paths", () => {
+    expect(validateAuthNextPath("/workspace")).toBe("/workspace");
+    expect(validateAuthNextPath("/workspace/chats/new?tab=recent#top")).toBe(
+      "/workspace/chats/new?tab=recent#top",
+    );
+  });
+
+  test("rejects external and ambiguous redirects", () => {
+    expect(validateAuthNextPath(null)).toBeNull();
+    expect(validateAuthNextPath("workspace")).toBeNull();
+    expect(validateAuthNextPath("//evil.example")).toBeNull();
+    expect(validateAuthNextPath("https://evil.example")).toBeNull();
+    expect(validateAuthNextPath("/:evil")).toBeNull();
+    expect(validateAuthNextPath("/\\evil.example")).toBeNull();
+    expect(validateAuthNextPath("/foo\\bar")).toBeNull();
+  });
+
+  test("falls back for unsafe paths", () => {
+    expect(resolveAuthNextPath("/\\evil.example")).toBe("/workspace");
+    expect(resolveAuthNextPath("/safe", "/fallback")).toBe("/safe");
+  });
+});