🛡️ Sentinel: [CRITICAL] Fix Host Header Injection in Auth Callback

src/app/auth/callback/route.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { resolveRedirectPath } from "./route";
 import { isInternalUrl } from "~/lib/url";
 
@@ -25,75 +25,60 @@ describe("isInternalUrl", () => {
 });
 
 describe("resolveRedirectPath", () => {
+  beforeEach(() => {
+    vi.stubEnv("NEXT_PUBLIC_SITE_URL", "http://localhost:3000");
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
   it("should accept valid internal path", () => {
-    const result = resolveRedirectPath({
-      nextParam: "/dashboard",
-      origin: "http://localhost:3000",
-      forwardedHost: null,
-    });
+    const result = resolveRedirectPath("/dashboard");
     expect(result).toBe("/dashboard");
   });
 
   it("should reject external URL (open redirect prevention)", () => {
-    const result = resolveRedirectPath({
-      nextParam: "https://evil.com/steal-session",
-      origin: "http://localhost:3000",
-      forwardedHost: null,
-    });
+    const result = resolveRedirectPath("https://evil.com/steal-session");
     expect(result).toBe("/");
   });
 
   it("should reject protocol-relative URL", () => {
-    const result = resolveRedirectPath({
-      nextParam: "//evil.com/phishing",
-      origin: "http://localhost:3000",
-      forwardedHost: null,
-    });
+    const result = resolveRedirectPath("//evil.com/phishing");
     expect(result).toBe("/");
   });
 
   it("should handle paths with query params and hash", () => {
-    const result = resolveRedirectPath({
-      nextParam: "/dashboard?tab=issues#top",
-      origin: "http://localhost:3000",
-      forwardedHost: null,
-    });
+    const result = resolveRedirectPath("/dashboard?tab=issues#top");
     expect(result).toBe("/dashboard?tab=issues#top");
   });
 
   it("should return fallback when nextParam is null", () => {
-    const result = resolveRedirectPath({
-      nextParam: null,
-      origin: "http://localhost:3000",
-      forwardedHost: null,
-    });
+    const result = resolveRedirectPath(null);
     expect(result).toBe("/");
   });
 
-  it("should accept absolute URL matching origin host", () => {
-    const result = resolveRedirectPath({
-      nextParam: "http://localhost:3000/dashboard",
-      origin: "http://localhost:3000",
-      forwardedHost: null,
-    });
+  it("should accept absolute URL matching site url", () => {
+    const result = resolveRedirectPath("http://localhost:3000/dashboard");
     expect(result).toBe("/dashboard");
   });
 
-  it("should accept absolute URL matching forwarded host", () => {
-    const result = resolveRedirectPath({
-      nextParam: "https://app.example.com/dashboard",
-      origin: "http://localhost:3000",
-      forwardedHost: "app.example.com",
-    });
+  it("should reject absolute URL matching a different host (even if it was forwarded host previously)", () => {
+    // We simulate a scenario where attacker provided a different host.
+    // Since resolveRedirectPath now relies on NEXT_PUBLIC_SITE_URL, it should reject this.
+    const result = resolveRedirectPath("https://app.example.com/dashboard");
+    expect(result).toBe("/");
+  });
+
+  it("should accept absolute URL matching configured production site url", () => {
+    vi.stubEnv("NEXT_PUBLIC_SITE_URL", "https://pinpoint.com");
+    const result = resolveRedirectPath("https://pinpoint.com/dashboard");
     expect(result).toBe("/dashboard");
   });
 
-  it("should reject absolute URL not matching origin or forwarded host", () => {
-    const result = resolveRedirectPath({
-      nextParam: "https://evil.com/dashboard",
-      origin: "http://test.local",
-      forwardedHost: "app.example.com",
-    });
+  it("should reject mismatching site url when production url is configured", () => {
+    vi.stubEnv("NEXT_PUBLIC_SITE_URL", "https://pinpoint.com");
+    const result = resolveRedirectPath("http://localhost:3000/dashboard");
     expect(result).toBe("/");
   });
 });

src/app/auth/callback/route.ts

@@ -9,14 +9,9 @@ import { createServerClient, type CookieOptions } from "@supabase/ssr";
 import { NextResponse } from "next/server";
 import { type NextRequest } from "next/server";
 import type { EmailOtpType } from "@supabase/supabase-js";
-import { isInternalUrl } from "~/lib/url";
-
-export function resolveRedirectPath(options: {
-  nextParam: string | null;
-  origin: string;
-  forwardedHost: string | null;
-}): string {
-  const { nextParam, origin, forwardedHost } = options;
+import { getSiteUrl, isInternalUrl } from "~/lib/url";
+
+export function resolveRedirectPath(nextParam: string | null): string {
   const fallback = "/";
 
   if (!nextParam) {
@@ -27,16 +22,13 @@ export function resolveRedirectPath(options: {
     return nextParam;
   }
 
-  // Allow absolute URLs that point to this host only; drop origin to prevent open redirects.
+  // Allow absolute URLs that point to the configured site URL only.
   try {
-    const originHost = new URL(origin).host;
-    const allowedHosts = new Set([originHost]);
-    if (forwardedHost) {
-      allowedHosts.add(forwardedHost);
-    }
+    const siteUrl = getSiteUrl();
+    const siteHost = new URL(siteUrl).host;
+    const parsed = new URL(nextParam);
 
-    const parsed = new URL(nextParam, origin);
-    if (allowedHosts.has(parsed.host)) {
+    if (parsed.host === siteHost) {
       const normalizedPath = `${parsed.pathname}${parsed.search}${parsed.hash}`;
       if (isInternalUrl(normalizedPath)) {
         return normalizedPath;
@@ -63,30 +55,30 @@ export function resolveRedirectPath(options: {
  * @see https://supabase.com/docs/guides/auth/server-side/nextjs
  */
 export async function GET(request: NextRequest): Promise<NextResponse> {
-  const { searchParams, origin } = new URL(request.url);
+  const { searchParams } = new URL(request.url);
   const code = searchParams.get("code");
   const tokenHash = searchParams.get("token_hash");
   const typeParam = searchParams.get("type");
   const nextParam = searchParams.get("next") ?? "/";
 
-  const forwardedHost = request.headers.get("x-forwarded-host");
-  const next = resolveRedirectPath({ nextParam, origin, forwardedHost });
-  const isLocalEnv = process.env.NODE_ENV === "development";
+  // Security: Always use configured site URL for redirects, never trust Host header
+  const next = resolveRedirectPath(nextParam);
+  const siteUrl = getSiteUrl();
+
   const shouldUseLoadingScreen = next === "/reset-password";
   const redirectPath = shouldUseLoadingScreen
     ? `/auth/loading?next=${encodeURIComponent(next)}`
     : next;
 
-  const redirectToTarget = (): NextResponse => {
-    if (isLocalEnv) {
-      return NextResponse.redirect(`${origin}${redirectPath}`);
-    }
+  // Ensure redirectPath starts with / to avoid double slashes if siteUrl has trailing slash
+  // getSiteUrl() typically does not have trailing slash, but good to be safe.
+  // Actually getSiteUrl implementation: return `...` (no trailing slash usually).
+  // But redirectPath from resolveRedirectPath always starts with / due to isInternalUrl check.
 
-    if (forwardedHost) {
-      return NextResponse.redirect(`https://${forwardedHost}${redirectPath}`);
-    }
+  const targetUrl = `${siteUrl}${redirectPath}`;
 
-    return NextResponse.redirect(`${origin}${redirectPath}`);
+  const redirectToTarget = (): NextResponse => {
+    return NextResponse.redirect(targetUrl);
   };
 
   const supabaseClient = createSupabaseClient(request);
@@ -128,8 +120,9 @@ export async function GET(request: NextRequest): Promise<NextResponse> {
   }
 
   // Auth failed or no code - redirect to error page
+  // Use siteUrl for error page redirect too
   return applyCookies(
-    NextResponse.redirect(`${origin}/auth/auth-code-error`),
+    NextResponse.redirect(`${siteUrl}/auth/auth-code-error`),
     pendingCookies
   );
 }