diff --git a/apps/builder/__tests__/proxy-public-url.test.ts b/apps/builder/__tests__/proxy-public-url.test.ts new file mode 100644 index 0000000000..a11c527ec2 --- /dev/null +++ b/apps/builder/__tests__/proxy-public-url.test.ts @@ -0,0 +1,72 @@ +// @vitest-environment node + +import { NextRequest } from "next/server" +import { afterEach, describe, expect, test, vi } from "vitest" + +vi.mock("@/lib/auth/auth", () => ({ + auth: { api: { getSession: vi.fn() } }, +})) + +vi.mock("better-auth/cookies", () => ({ + getSessionCookie: vi.fn(() => null), +})) + +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => new Headers()), +})) + +vi.mock("@/lib/log", () => ({ + httpLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})) + +const originalForcePublicHttps = process.env.FORCE_PUBLIC_HTTPS + +afterEach(() => { + if (originalForcePublicHttps === undefined) { + delete process.env.FORCE_PUBLIC_HTTPS + } else { + process.env.FORCE_PUBLIC_HTTPS = originalForcePublicHttps + } +}) + +/** + * `/api` is a public route, so `proxy` reaches `attachProxyUrl` without + * touching the session. `NextResponse.next({ request: { headers } })` exposes + * the overridden request headers as `x-middleware-request-`. + */ +async function proxyUrlFor(publicHost: string, requestUrl: string) { + delete process.env.FORCE_PUBLIC_HTTPS + const { proxy } = await import("@/proxy") + const response = await proxy( + new NextRequest(requestUrl, { + headers: { "x-forwarded-host": publicHost }, + }), + ) + return response.headers.get("x-middleware-request-x-url") +} + +describe("proxy x-url", () => { + test("keeps the port when the public host carries one", async () => { + expect( + await proxyUrlFor("localhost:3123", "http://internal.test:3000/api/x"), + ).toBe("http://localhost:3123/api/x") + }) + + test("drops the internal port when the public host carries none", async () => { + expect( + await proxyUrlFor("app.example.com", "http://internal.test:3000/api/x"), + ).toBe("http://app.example.com/api/x") + }) + + test("does not mistake a bracketed IPv6 host for one carrying a port", async () => { + expect(await proxyUrlFor("[::1]", "http://internal.test:3000/api/x")).toBe( + "http://[::1]/api/x", + ) + }) + + test("keeps the port of a bracketed IPv6 host that carries one", async () => { + expect( + await proxyUrlFor("[::1]:3123", "http://internal.test:3000/api/x"), + ).toBe("http://[::1]:3123/api/x") + }) +}) diff --git a/apps/builder/src/lib/auth-redirect.ts b/apps/builder/src/lib/auth-redirect.ts index f0cf0fc6f1..e64491208f 100644 --- a/apps/builder/src/lib/auth-redirect.ts +++ b/apps/builder/src/lib/auth-redirect.ts @@ -1,5 +1,6 @@ import { getPublicHostFromRequest, + getPublicPortFromRequest, getPublicProtocolFromRequest, } from "@chatbotx.io/utils" import { env } from "@/env" @@ -67,7 +68,7 @@ export async function rewriteAuthRedirectToPublicHost( target.host = publicHost target.protocol = publicProtocol - target.port = "" + target.port = getPublicPortFromRequest(request) const headers = new Headers(response.headers) headers.set("location", target.toString()) diff --git a/apps/builder/src/proxy.ts b/apps/builder/src/proxy.ts index b34b958f47..686433433a 100644 --- a/apps/builder/src/proxy.ts +++ b/apps/builder/src/proxy.ts @@ -1,6 +1,7 @@ import { getPublicHostFromRequest, getPublicOriginFromRequest, + getPublicPortFromRequest, getPublicProtocolFromRequest, } from "@chatbotx.io/utils" import { getSessionCookie } from "better-auth/cookies" @@ -64,7 +65,7 @@ function attachProxyUrl(request: NextRequest): NextResponse { const originUrl = new URL(request.url) originUrl.host = getPublicHostFromRequest(request) originUrl.protocol = getPublicProtocolFromRequest(request) - originUrl.port = "" + originUrl.port = getPublicPortFromRequest(request) const requestHeaders = new Headers(request.headers) requestHeaders.set("x-url", originUrl.toString()) diff --git a/packages/utils/__tests__/request.test.ts b/packages/utils/__tests__/request.test.ts index 7c0dde5dd9..00bba69890 100644 --- a/packages/utils/__tests__/request.test.ts +++ b/packages/utils/__tests__/request.test.ts @@ -64,4 +64,52 @@ describe("getPublicUrlFromRequest", () => { .protocol, ).toBe("https:") }) + + test("keeps the port when the public host carries one", () => { + delete process.env.FORCE_PUBLIC_HTTPS + + expect( + getPublicUrlFromRequest( + new Request("http://internal.test:3000/callback", { + headers: { "x-forwarded-host": "localhost:3123" }, + }), + ).toString(), + ).toBe("http://localhost:3123/callback") + }) + + test("drops the internal port when the public host carries none", () => { + delete process.env.FORCE_PUBLIC_HTTPS + + expect( + getPublicUrlFromRequest( + new Request("http://internal.test:3000/callback", { + headers: { "x-forwarded-host": "app.example.com" }, + }), + ).toString(), + ).toBe("http://app.example.com/callback") + }) + + test("does not mistake a bracketed IPv6 host for one carrying a port", () => { + delete process.env.FORCE_PUBLIC_HTTPS + + expect( + getPublicUrlFromRequest( + new Request("http://internal.test:3000/callback", { + headers: { "x-forwarded-host": "[::1]" }, + }), + ).toString(), + ).toBe("http://[::1]/callback") + }) + + test("keeps the port of a bracketed IPv6 host that carries one", () => { + delete process.env.FORCE_PUBLIC_HTTPS + + expect( + getPublicUrlFromRequest( + new Request("http://internal.test:3000/callback", { + headers: { "x-forwarded-host": "[::1]:3123" }, + }), + ).toString(), + ).toBe("http://[::1]:3123/callback") + }) }) diff --git a/packages/utils/src/request.ts b/packages/utils/src/request.ts index 8f2b17b343..4ab8aa179f 100644 --- a/packages/utils/src/request.ts +++ b/packages/utils/src/request.ts @@ -11,10 +11,35 @@ export function getPublicUrlFromRequest(request: Request): URL { const url = new URL(request.url) url.host = getPublicHostFromRequest(request) url.protocol = getPublicProtocolFromRequest(request) - url.port = "" + url.port = getPublicPortFromRequest(request) return url } +/** + * The port carried by the public host (`localhost:3123` → `"3123"`), or an + * empty string when it carries none (`app.example.com`). + * + * Assigning `URL.host` a value *without* a port leaves the URL's previous port + * untouched (per the URL spec), so behind a reverse proxy the internal port + * would leak into the public URL. Every caller that assigns `host` must + * therefore assign the port as well — clearing it unconditionally instead + * would drop the port in local development, where the public host legitimately + * is `localhost:3123`. + * + * Parsing is delegated to the URL parser rather than searching for a `":"`: + * the colons inside a bracketed IPv6 literal (`[::1]`) are not port + * separators, and an out-of-range or non-numeric port must clear the port + * rather than leave the internal one in place. + */ +export function getPublicPortFromRequest(request: Request): string { + const host = getPublicHostFromRequest(request) + try { + return new URL(`http://${host}`).port + } catch { + return "" + } +} + export function getPublicProtocolFromRequest( request: Request, ): "http" | "https" {