diff --git a/apps/web/__tests__/helpers.ts b/apps/web/__tests__/helpers.ts index 514b3b8930..89e3112b7b 100644 --- a/apps/web/__tests__/helpers.ts +++ b/apps/web/__tests__/helpers.ts @@ -146,6 +146,8 @@ export function getMockMessage({ snippet = "Test message", textPlain = "Test content", textHtml = "

Test content

", + 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(), }; diff --git a/apps/web/app/(app)/[emailAccountId]/calendars/page.tsx b/apps/web/app/(app)/[emailAccountId]/calendars/page.tsx index 792af75ab1..969be8146d 100644 --- a/apps/web/app/(app)/[emailAccountId]/calendars/page.tsx +++ b/apps/web/app/(app)/[emailAccountId]/calendars/page.tsx @@ -6,6 +6,7 @@ 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(); @@ -13,9 +14,7 @@ export default async function CalendarsPage() { if (returnPathCookie?.value) { const returnPath = decodeURIComponent(returnPathCookie.value); - const isInternalPath = - returnPath.startsWith("/") && !returnPath.startsWith("//"); - if (isInternalPath) { + if (isInternalPath(returnPath)) { redirect(returnPath); } } diff --git a/apps/web/app/(landing)/login/LoginForm.tsx b/apps/web/app/(landing)/login/LoginForm.tsx index 7881e1f0e1..e958009314 100644 --- a/apps/web/app/(landing)/login/LoginForm.tsx +++ b/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); diff --git a/apps/web/app/(landing)/login/page.tsx b/apps/web/app/(landing)/login/page.tsx index e137bc9e12..fb8cff0a36 100644 --- a/apps/web/app/(landing)/login/page.tsx +++ b/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); } diff --git a/apps/web/app/api/clean/route.test.ts b/apps/web/app/api/clean/route.test.ts index 400b07d9fe..e51d7e0316 100644 --- a/apps/web/app/api/clean/route.test.ts +++ b/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 & { 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: [], }), ]; diff --git a/apps/web/utils/path.test.ts b/apps/web/utils/path.test.ts new file mode 100644 index 0000000000..11bc7ad02a --- /dev/null +++ b/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); + }); +}); diff --git a/apps/web/utils/path.ts b/apps/web/utils/path.ts index 5ba2d5401b..9720d6edd5 100644 --- a/apps/web/utils/path.ts +++ b/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("/\\") + ); +}