diff --git a/src/browser/bridge-server.ts b/src/browser/bridge-server.ts index d513d2319..e91555c41 100644 --- a/src/browser/bridge-server.ts +++ b/src/browser/bridge-server.ts @@ -5,6 +5,7 @@ import type { ResolvedBrowserConfig } from "./config.js"; import type { BrowserRouteRegistrar } from "./routes/types.js"; import { isLoopbackHost } from "../gateway/net.js"; import { deleteBridgeAuthForPort, setBridgeAuthForPort } from "./bridge-auth-registry.js"; +import { browserMutationGuardMiddleware } from "./csrf.js"; import { isAuthorizedBrowserRequest } from "./http-auth.js"; import { registerBrowserRoutes } from "./routes/index.js"; import { @@ -49,6 +50,7 @@ export async function startBrowserBridgeServer(params: { next(); }); app.use(express.json({ limit: "1mb" })); + app.use(browserMutationGuardMiddleware()); const authToken = params.authToken?.trim() || undefined; const authPassword = params.authPassword?.trim() || undefined; diff --git a/src/browser/csrf.test.ts b/src/browser/csrf.test.ts new file mode 100644 index 000000000..6f4bedd69 --- /dev/null +++ b/src/browser/csrf.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; +import { shouldRejectBrowserMutation } from "./csrf.js"; + +describe("browser CSRF loopback mutation guard", () => { + it("rejects mutating methods from non-loopback origin", () => { + expect( + shouldRejectBrowserMutation({ + method: "POST", + origin: "https://evil.example", + }), + ).toBe(true); + }); + + it("allows mutating methods from loopback origin", () => { + expect( + shouldRejectBrowserMutation({ + method: "POST", + origin: "http://127.0.0.1:18789", + }), + ).toBe(false); + + expect( + shouldRejectBrowserMutation({ + method: "POST", + origin: "http://localhost:18789", + }), + ).toBe(false); + }); + + it("allows mutating methods without origin/referer (non-browser clients)", () => { + expect( + shouldRejectBrowserMutation({ + method: "POST", + }), + ).toBe(false); + }); + + it("rejects mutating methods with origin=null", () => { + expect( + shouldRejectBrowserMutation({ + method: "POST", + origin: "null", + }), + ).toBe(true); + }); + + it("rejects mutating methods from non-loopback referer", () => { + expect( + shouldRejectBrowserMutation({ + method: "POST", + referer: "https://evil.example/attack", + }), + ).toBe(true); + }); + + it("rejects cross-site mutations via Sec-Fetch-Site when present", () => { + expect( + shouldRejectBrowserMutation({ + method: "POST", + secFetchSite: "cross-site", + }), + ).toBe(true); + }); + + it("does not reject non-mutating methods", () => { + expect( + shouldRejectBrowserMutation({ + method: "GET", + origin: "https://evil.example", + }), + ).toBe(false); + + expect( + shouldRejectBrowserMutation({ + method: "OPTIONS", + origin: "https://evil.example", + }), + ).toBe(false); + }); +}); diff --git a/src/browser/csrf.ts b/src/browser/csrf.ts new file mode 100644 index 000000000..a7755b874 --- /dev/null +++ b/src/browser/csrf.ts @@ -0,0 +1,83 @@ +import type { NextFunction, Request, Response } from "express"; +import { isLoopbackHost } from "../gateway/net.js"; + +function firstHeader(value: string | string[] | undefined): string { + return Array.isArray(value) ? (value[0] ?? "") : (value ?? ""); +} + +function isMutatingMethod(method: string): boolean { + const m = (method || "").trim().toUpperCase(); + return m === "POST" || m === "PUT" || m === "PATCH" || m === "DELETE"; +} + +function isLoopbackUrl(value: string): boolean { + const v = value.trim(); + if (!v || v === "null") { + return false; + } + try { + const parsed = new URL(v); + return isLoopbackHost(parsed.hostname); + } catch { + return false; + } +} + +export function shouldRejectBrowserMutation(params: { + method: string; + origin?: string; + referer?: string; + secFetchSite?: string; +}): boolean { + if (!isMutatingMethod(params.method)) { + return false; + } + + // Strong signal when present: browser says this is cross-site. + // Avoid being overly clever with "same-site" since localhost vs 127.0.0.1 may differ. + const secFetchSite = (params.secFetchSite ?? "").trim().toLowerCase(); + if (secFetchSite === "cross-site") { + return true; + } + + const origin = (params.origin ?? "").trim(); + if (origin) { + return !isLoopbackUrl(origin); + } + + const referer = (params.referer ?? "").trim(); + if (referer) { + return !isLoopbackUrl(referer); + } + + // Non-browser clients (curl/undici/Node) typically send no Origin/Referer. + return false; +} + +export function browserMutationGuardMiddleware() { + return (req: Request, res: Response, next: NextFunction) => { + // OPTIONS is used for CORS preflight. Even if cross-origin, the preflight isn't mutating. + const method = (req.method || "").trim().toUpperCase(); + if (method === "OPTIONS") { + return next(); + } + + const origin = firstHeader(req.headers.origin); + const referer = firstHeader(req.headers.referer); + const secFetchSite = firstHeader(req.headers["sec-fetch-site"]); + + if ( + shouldRejectBrowserMutation({ + method, + origin, + referer, + secFetchSite, + }) + ) { + res.status(403).send("Forbidden"); + return; + } + + next(); + }; +} diff --git a/src/browser/server.ts b/src/browser/server.ts index 2392f9c48..37213692e 100644 --- a/src/browser/server.ts +++ b/src/browser/server.ts @@ -5,6 +5,7 @@ import { loadConfig } from "../config/config.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { resolveBrowserConfig, resolveProfile } from "./config.js"; import { ensureBrowserControlAuth, resolveBrowserControlAuth } from "./control-auth.js"; +import { browserMutationGuardMiddleware } from "./csrf.js"; import { ensureChromeExtensionRelayServer } from "./extension-relay.js"; import { isAuthorizedBrowserRequest } from "./http-auth.js"; import { isPwAiLoaded } from "./pw-ai-state.js"; @@ -56,6 +57,7 @@ export async function startBrowserControlServerFromConfig(): Promise {