security: fix open redirect vulnerability in login flow

apps/web/__tests__/helpers.ts

@@ -146,6 +146,8 @@ export function getMockMessage({
   snippet = "Test message",
   textPlain = "Test content",
   textHtml = "<p>Test content</p>",
+  labelIds = [],
+  attachments = [],
 }: {
   id?: string;
   threadId?: string;
@@ -156,6 +158,8 @@ export function getMockMessage({
   snippet?: string;
   textPlain?: string;
   textHtml?: string;
+  labelIds?: string[];
+  attachments?: any[];
 } = {}) {
   return {
     id,
@@ -170,9 +174,9 @@ export function getMockMessage({
     snippet,
     textPlain,
     textHtml,
-    attachments: [],
+    attachments,
     inline: [],
-    labelIds: [],
+    labelIds,
     subject,
     date: new Date().toISOString(),
   };

apps/web/app/(app)/[emailAccountId]/calendars/page.tsx

@@ -6,16 +6,15 @@ import { CalendarConnections } from "./CalendarConnections";
 import { CalendarSettings } from "./CalendarSettings";
 import { TimezoneDetector } from "./TimezoneDetector";
 import { CALENDAR_ONBOARDING_RETURN_COOKIE } from "@/utils/calendar/constants";
+import { isInternalPath } from "@/utils/path";
 
 export default async function CalendarsPage() {
   const cookieStore = await cookies();
   const returnPathCookie = cookieStore.get(CALENDAR_ONBOARDING_RETURN_COOKIE);
 
   if (returnPathCookie?.value) {
     const returnPath = decodeURIComponent(returnPathCookie.value);
-    const isInternalPath =
-      returnPath.startsWith("/") && !returnPath.startsWith("//");
-    if (isInternalPath) {
+    if (isInternalPath(returnPath)) {
       redirect(returnPath);
     }
   }

apps/web/app/(landing)/login/LoginForm.tsx

@@ -17,6 +17,7 @@ import {
 import { signIn } from "@/utils/auth-client";
 import { WELCOME_PATH } from "@/utils/config";
 import { toastError } from "@/components/Toast";
+import { isInternalPath } from "@/utils/path";
 
 export function LoginForm() {
   const searchParams = useSearchParams();
@@ -27,11 +28,12 @@ export function LoginForm() {
 
   const handleGoogleSignIn = async () => {
     setLoadingGoogle(true);
+    const callbackURL = next && isInternalPath(next) ? next : WELCOME_PATH;
     try {
       await signIn.social({
         provider: "google",
         errorCallbackURL: "/login/error",
-        callbackURL: next && next.length > 0 ? next : WELCOME_PATH,
+        callbackURL,
       });
     } catch (error) {
       console.error("Error signing in with Google:", error);
@@ -46,11 +48,12 @@ export function LoginForm() {
 
   const handleMicrosoftSignIn = async () => {
     setLoadingMicrosoft(true);
+    const callbackURL = next && isInternalPath(next) ? next : WELCOME_PATH;
     try {
       await signIn.social({
         provider: "microsoft",
         errorCallbackURL: "/login/error",
-        callbackURL: next && next.length > 0 ? next : WELCOME_PATH,
+        callbackURL,
       });
     } catch (error) {
       console.error("Error signing in with Microsoft:", error);

apps/web/app/(landing)/login/page.tsx

@@ -9,6 +9,7 @@ import { env } from "@/env";
 import { Button } from "@/components/ui/button";
 import { WELCOME_PATH } from "@/utils/config";
 import { CrispChatLoggedOutVisible } from "@/components/CrispChat";
+import { isInternalPath } from "@/utils/path";
 
 export const metadata: Metadata = {
   title: "Log in | Inbox Zero",
@@ -22,8 +23,8 @@ export default async function AuthenticationPage(props: {
   const searchParams = await props.searchParams;
   const session = await auth();
   if (session?.user && !searchParams?.error) {
-    if (searchParams?.next) {
-      redirect(searchParams?.next);
+    if (searchParams?.next && isInternalPath(searchParams.next)) {
+      redirect(searchParams.next);
     } else {
       redirect(WELCOME_PATH);
     }

apps/web/app/api/clean/route.test.ts

@@ -3,6 +3,7 @@ import { cleanThread } from "./route";
 import { GmailLabel } from "@/utils/gmail/label";
 import type { ParsedMessage } from "@/utils/types";
 import { CleanAction } from "@/generated/prisma/enums";
+import { getMockMessage } from "@/__tests__/helpers";
 
 vi.mock("server-only", () => ({}));
 
@@ -65,34 +66,6 @@ const mockLogger = {
   trace: vi.fn(),
 };
 
-function createMockMessage(
-  overrides: Partial<ParsedMessage> & { labelIds?: string[] } = {},
-): ParsedMessage {
-  const { headers: headerOverrides, ...restOverrides } = overrides;
-  const defaultHeaders = {
-    from: "sender@example.com",
-    to: "user@example.com",
-    subject: "Test Subject",
-    date: new Date().toISOString(),
-  };
-  const headers = { ...defaultHeaders, ...headerOverrides };
-
-  return {
-    id: restOverrides.id || "msg-1",
-    threadId: "thread-1",
-    historyId: "12345",
-    snippet: "Test snippet",
-    subject: headers.subject,
-    date: new Date().toISOString(),
-    internalDate: String(Date.now()),
-    inline: [],
-    headers,
-    labelIds: restOverrides.labelIds || [],
-    attachments: restOverrides.attachments || [],
-    ...restOverrides,
-  };
-}
-
 function getDefaultParams() {
   return {
     emailAccountId: "email-account-id",
@@ -139,24 +112,18 @@ describe("cleanThread", () => {
   describe("maybe-receipt should not break loop early", () => {
     it("should skip thread when message 1 is maybe-receipt but message 2 is starred", async () => {
       const messages = [
-        createMockMessage({
+        getMockMessage({
           id: "msg-1",
-          headers: {
-            from: "store@example.com",
-            to: "user@example.com",
-            subject: "Payment confirmation",
-            date: new Date().toISOString(),
-          },
+          from: "store@example.com",
+          to: "user@example.com",
+          subject: "Payment confirmation",
           labelIds: [],
         }),
-        createMockMessage({
+        getMockMessage({
           id: "msg-2",
-          headers: {
-            from: "user@example.com",
-            to: "store@example.com",
-            subject: "Re: Payment confirmation",
-            date: new Date().toISOString(),
-          },
+          from: "user@example.com",
+          to: "store@example.com",
+          subject: "Re: Payment confirmation",
           labelIds: [GmailLabel.STARRED],
         }),
       ];
@@ -175,24 +142,18 @@ describe("cleanThread", () => {
 
     it("should skip thread when message 1 is maybe-receipt but message 2 is user's reply (conversation)", async () => {
       const messages = [
-        createMockMessage({
+        getMockMessage({
           id: "msg-1",
-          headers: {
-            from: "store@example.com",
-            to: "user@example.com",
-            subject: "Payment confirmation",
-            date: new Date().toISOString(),
-          },
+          from: "store@example.com",
+          to: "user@example.com",
+          subject: "Payment confirmation",
           labelIds: [],
         }),
-        createMockMessage({
+        getMockMessage({
           id: "msg-2",
-          headers: {
-            from: "user@example.com",
-            to: "store@example.com",
-            subject: "Re: Payment confirmation",
-            date: new Date().toISOString(),
-          },
+          from: "user@example.com",
+          to: "store@example.com",
+          subject: "Re: Payment confirmation",
           labelIds: [GmailLabel.SENT],
         }),
       ];
@@ -211,24 +172,18 @@ describe("cleanThread", () => {
 
     it("should skip thread when message 1 is maybe-receipt but message 2 has attachments", async () => {
       const messages = [
-        createMockMessage({
+        getMockMessage({
           id: "msg-1",
-          headers: {
-            from: "store@example.com",
-            to: "user@example.com",
-            subject: "Payment confirmation",
-            date: new Date().toISOString(),
-          },
+          from: "store@example.com",
+          to: "user@example.com",
+          subject: "Payment confirmation",
           labelIds: [],
         }),
-        createMockMessage({
+        getMockMessage({
           id: "msg-2",
-          headers: {
-            from: "store@example.com",
-            to: "user@example.com",
-            subject: "Invoice attached",
-            date: new Date().toISOString(),
-          },
+          from: "store@example.com",
+          to: "user@example.com",
+          subject: "Invoice attached",
           labelIds: [],
           attachments: [
             {
@@ -261,24 +216,18 @@ describe("cleanThread", () => {
 
     it("should call LLM when maybe-receipt found and no skip conditions in other messages", async () => {
       const messages = [
-        createMockMessage({
+        getMockMessage({
           id: "msg-1",
-          headers: {
-            from: "store@example.com",
-            to: "user@example.com",
-            subject: "Payment confirmation",
-            date: new Date().toISOString(),
-          },
+          from: "store@example.com",
+          to: "user@example.com",
+          subject: "Payment confirmation",
           labelIds: [],
         }),
-        createMockMessage({
+        getMockMessage({
           id: "msg-2",
-          headers: {
-            from: "store@example.com",
-            to: "user@example.com",
-            subject: "Shipping update",
-            date: new Date().toISOString(),
-          },
+          from: "store@example.com",
+          to: "user@example.com",
+          subject: "Shipping update",
           labelIds: [],
         }),
       ];

apps/web/utils/path.test.ts

@@ -0,0 +1,39 @@
+import { describe, it, expect } from "vitest";
+import { isInternalPath } from "./path";
+
+describe("isInternalPath", () => {
+  it("should return true for valid internal paths", () => {
+    expect(isInternalPath("/")).toBe(true);
+    expect(isInternalPath("/dashboard")).toBe(true);
+    expect(isInternalPath("/settings/profile")).toBe(true);
+    expect(isInternalPath("/a")).toBe(true);
+  });
+
+  it("should return false for external URLs", () => {
+    expect(isInternalPath("https://example.com")).toBe(false);
+    expect(isInternalPath("http://example.com")).toBe(false);
+    expect(isInternalPath("ftp://example.com")).toBe(false);
+    expect(isInternalPath("javascript:alert(1)")).toBe(false);
+  });
+
+  it("should return false for protocol-relative URLs", () => {
+    expect(isInternalPath("//example.com")).toBe(false);
+    expect(isInternalPath("//")).toBe(false);
+  });
+
+  it("should return false for backslash bypass attempts", () => {
+    expect(isInternalPath("/\\example.com")).toBe(false);
+    expect(isInternalPath("/\\")).toBe(false);
+  });
+
+  it("should return false for empty, null, or undefined paths", () => {
+    expect(isInternalPath("")).toBe(false);
+    expect(isInternalPath(null)).toBe(false);
+    expect(isInternalPath(undefined)).toBe(false);
+  });
+
+  it("should return false for paths not starting with a slash", () => {
+    expect(isInternalPath("dashboard")).toBe(false);
+    expect(isInternalPath("settings/profile")).toBe(false);
+  });
+});

apps/web/utils/path.ts

@@ -2,3 +2,10 @@ export const prefixPath = (emailAccountId: string, path: `/${string}`) => {
   if (emailAccountId) return `/${emailAccountId}${path}`;
   return path;
 };
+
+export function isInternalPath(path: string | null | undefined): boolean {
+  if (!path) return false;
+  return (
+    path.startsWith("/") && !path.startsWith("//") && !path.startsWith("/\\")
+  );
+}