diff --git a/apps/planner/app/lib/url-validation.server.test.ts b/apps/planner/app/lib/url-validation.server.test.ts index ad5dc07..afc67c8 100644 --- a/apps/planner/app/lib/url-validation.server.test.ts +++ b/apps/planner/app/lib/url-validation.server.test.ts @@ -44,6 +44,59 @@ describe("validateFetchUrl", () => { }); }); +describe("validateFetchUrl — private-address blocking (production)", () => { + beforeEach(() => { + vi.unstubAllEnvs(); + // Production-without-E2E is the only mode that blocks (mirrors the + // requireSecret guard). Tests otherwise run as NODE_ENV=test → off. + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("E2E", ""); + }); + + const blocked = [ + "http://127.0.0.1/x", + "http://localhost:3000/x", + "http://sub.localhost/x", + "http://169.254.169.254/latest/meta-data/", // cloud metadata + "http://10.0.0.5/x", + "http://172.16.0.1/x", + "http://172.31.255.254/x", + "http://192.168.1.1/x", + "http://100.64.0.1/x", // CGNAT + "http://0.0.0.0/x", + "http://[::1]/x", + "http://[fc00::1]/x", + "http://[fe80::1]/x", + "http://[::ffff:127.0.0.1]/x", + ]; + for (const url of blocked) { + it(`blocks ${url}`, () => { + expect(validateFetchUrl(url).ok).toBe(false); + }); + } + + it("still allows public hosts", () => { + expect(validateFetchUrl("https://journal.trails.cool/api/cb").ok).toBe(true); + expect(validateFetchUrl("http://203.0.113.10/x").ok).toBe(true); // public IP literal + expect(validateFetchUrl("http://172.15.0.1/x").ok).toBe(true); // just outside RFC1918 /12 + expect(validateFetchUrl("http://172.32.0.1/x").ok).toBe(true); + }); + + it("an explicit allowlist overrides private blocking (operator decision)", () => { + expect( + validateFetchUrl("http://10.0.0.2:3000/cb", { allowedHosts: ["10.0.0.2:3000"] }).ok, + ).toBe(true); + }); + + it("does not block private hosts outside production (dev/e2e localhost flow)", () => { + vi.stubEnv("NODE_ENV", "development"); + expect(validateFetchUrl("http://localhost:3000/cb").ok).toBe(true); + vi.stubEnv("NODE_ENV", "production"); + vi.stubEnv("E2E", "true"); + expect(validateFetchUrl("http://localhost:3000/cb").ok).toBe(true); + }); +}); + describe("validateRedirectUrl", () => { it("accepts an absolute https URL", () => { expect(validateRedirectUrl("https://trails.cool/r/123").ok).toBe(true); diff --git a/apps/planner/app/lib/url-validation.server.ts b/apps/planner/app/lib/url-validation.server.ts index 84e2964..70795d9 100644 --- a/apps/planner/app/lib/url-validation.server.ts +++ b/apps/planner/app/lib/url-validation.server.ts @@ -16,6 +16,60 @@ export interface UrlValidationResult { url?: URL; } +/** + * Whether to reject callback hosts pointing at private / loopback / + * link-local / cloud-metadata ranges. Active in real production only — + * mirrors the `requireSecret` guard in the journal's config.server.ts. + * Dev and E2E legitimately point the callback at the journal on + * `localhost`, so blocking there would break the save flow. + */ +function blockPrivateAddresses(): boolean { + return process.env.NODE_ENV === "production" && process.env.E2E !== "true"; +} + +function isBlockedIpv4(ip: string): boolean { + const o = ip.split(".").map((n) => Number(n)); + // Malformed dotted-quad → treat as blocked (fail safe). + if (o.length !== 4 || o.some((n) => !Number.isInteger(n) || n < 0 || n > 255)) { + return true; + } + const [a, b] = o as [number, number, number, number]; + if (a === 0 || a === 10 || a === 127) return true; // this-host, RFC1918 /8, loopback + if (a === 169 && b === 254) return true; // link-local incl. 169.254.169.254 metadata + if (a === 172 && b >= 16 && b <= 31) return true; // RFC1918 /12 + if (a === 192 && b === 168) return true; // RFC1918 /16 + if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT 100.64/10 + return false; +} + +/** + * Literal private/loopback/link-local hosts. Catches IP literals and + * `localhost`; it does NOT resolve DNS, so a public name that resolves + * to a private IP (DNS-rebinding-style SSRF) is not blocked here — set + * PLANNER_CALLBACK_ALLOWED_HOSTS to close that where the callback host + * is known ahead of time. + */ +function isBlockedHost(hostname: string): boolean { + // Node keeps IPv6 brackets on URL.hostname (e.g. "[::1]"); strip them. + const h = hostname.toLowerCase().replace(/^\[|\]$/g, ""); + if (h === "localhost" || h.endsWith(".localhost")) return true; + if (h.includes(":")) { + // IPv6. + if (h === "::1" || h === "::") return true; + if (h.startsWith("fc") || h.startsWith("fd")) return true; // ULA fc00::/7 + if (h.startsWith("fe8") || h.startsWith("fe9") || h.startsWith("fea") || h.startsWith("feb")) { + return true; // link-local fe80::/10 + } + // IPv4-mapped (::ffff:a.b.c.d, which Node may render in hex form). + // Reaching IPv4 through a mapped address is inherently suspect for a + // callback target, so block the whole prefix. + if (h.startsWith("::ffff:")) return true; + return false; + } + if (/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.test(h)) return isBlockedIpv4(h); + return false; +} + /** * Validate a URL string for use as a fetch target. Requires absolute * http(s) URL. If `allowedHosts` is provided, the host must be on the @@ -39,9 +93,16 @@ export function validateFetchUrl( return { ok: false, reason: `disallowed scheme ${parsed.protocol}` }; } if (opts.allowedHosts && opts.allowedHosts.length > 0) { + // An explicit allowlist is the operator's decision and the strongest + // control; a host that matches it is trusted even if it's private. if (!opts.allowedHosts.includes(parsed.host)) { return { ok: false, reason: `host ${parsed.host} not on allowlist` }; } + } else if (blockPrivateAddresses() && isBlockedHost(parsed.hostname)) { + return { + ok: false, + reason: `host ${parsed.hostname} is a private/loopback/link-local address`, + }; } return { ok: true, url: parsed }; } diff --git a/apps/planner/app/routes/api.save-to-journal.ts b/apps/planner/app/routes/api.save-to-journal.ts index 6f7acc5..29ac08e 100644 --- a/apps/planner/app/routes/api.save-to-journal.ts +++ b/apps/planner/app/routes/api.save-to-journal.ts @@ -16,6 +16,7 @@ import { data } from "react-router"; import type { Route } from "./+types/api.save-to-journal"; import { getSession } from "~/lib/sessions"; import { fetchWithTimeout } from "~/lib/http.server"; +import { validateFetchUrl, getCallbackAllowedHosts } from "~/lib/url-validation.server"; interface SaveRequestBody { sessionId?: unknown; @@ -51,6 +52,15 @@ export async function action({ request }: Route.ActionArgs) { return data({ error: "session has no journal callback" }, { status: 400 }); } + // Defense in depth: re-validate immediately before the outbound fetch. + // Guards sessions persisted before callbackUrl validation existed, and + // narrows the window for a host that was public at create time but + // resolves private now. + const v = validateFetchUrl(session.callbackUrl, { allowedHosts: getCallbackAllowedHosts() }); + if (!v.ok) { + return data({ error: "session callback URL is not allowed" }, { status: 400 }); + } + let resp: Response; try { resp = await fetchWithTimeout(session.callbackUrl, { diff --git a/apps/planner/app/routes/api.sessions.ts b/apps/planner/app/routes/api.sessions.ts index 5fbd2d4..2756dba 100644 --- a/apps/planner/app/routes/api.sessions.ts +++ b/apps/planner/app/routes/api.sessions.ts @@ -4,6 +4,7 @@ import { createSession, listSessions } from "~/lib/sessions"; import { parseGpxAsync, extractWaypoints } from "@trails-cool/gpx"; import { withDb } from "@trails-cool/db"; import type { Waypoint } from "@trails-cool/types"; +import { validateFetchUrl, getCallbackAllowedHosts } from "~/lib/url-validation.server"; export async function action({ request }: Route.ActionArgs) { if (request.method !== "POST") { @@ -17,6 +18,20 @@ export async function action({ request }: Route.ActionArgs) { gpx?: string; }; + // callbackUrl becomes a server-side fetch target on save-to-journal, + // so an unvalidated value here is an SSRF sink. The /new loader + // already validates the query-param form; this is the programmatic + // JSON entry point and must do the same. + if (callbackUrl !== undefined) { + if (typeof callbackUrl !== "string") { + return data({ error: "callbackUrl must be a string" }, { status: 400 }); + } + const v = validateFetchUrl(callbackUrl, { allowedHosts: getCallbackAllowedHosts() }); + if (!v.ok) { + return data({ error: `Invalid callback URL: ${v.reason}` }, { status: 400 }); + } + } + return withDb(async () => { const session = await createSession({ callbackUrl, callbackToken });