From a864cac95b59a0809909ddd1411ec5225338b4fb Mon Sep 17 00:00:00 2001 From: Maciej Ugorski Date: Tue, 23 Jun 2026 16:36:09 +0200 Subject: [PATCH] fix(bridge): add Host/Origin guard + bearer token to HTTP bridge - Reject non-loopback Host on every route (kills DNS rebinding) - Reject non-loopback Origin (kills browser CSRF; CLI sends none) - Require a per-instance bearer token on /call, /tools, /last-snapshot; /health stays token-free for port detection - Generate the token in runBridge, persist it 0600 in the PID file, and read it back in the client to attach Authorization on protected calls --- src/bridge.ts | 127 +++++++++++++++++++++++++++-- src/client.ts | 25 +++++- test/bridge.test.ts | 194 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 337 insertions(+), 9 deletions(-) diff --git a/src/bridge.ts b/src/bridge.ts index 96e5baf..8a79827 100644 --- a/src/bridge.ts +++ b/src/bridge.ts @@ -24,6 +24,7 @@ import { type ServerResponse, } from "node:http"; import { existsSync, mkdirSync, unlinkSync, writeFileSync } from "node:fs"; +import { randomBytes, timingSafeEqual } from "node:crypto"; import { extractPageOrigin } from "./snapshot.js"; import { createRequire } from "node:module"; import { dirname, join, resolve } from "node:path"; @@ -103,9 +104,19 @@ export async function isBridgeClientConnected( } } -function writePidFile(port: number): void { +function writePidFile(port: number, token: string): void { mkdirSync(STATE_DIR, { recursive: true }); - writeFileSync(PID_FILE, JSON.stringify({ pid: process.pid, port })); + // Unlink first: writeFileSync's `mode` only applies on create, so overwriting + // a stale file would keep its looser perms and expose the token. + try { + unlinkSync(PID_FILE); + } catch { + // Didn't exist — fine + } + // 0600: only the owning user may read the auth token. + writeFileSync(PID_FILE, JSON.stringify({ pid: process.pid, port, token }), { + mode: 0o600, + }); } function removePidFile(): void { @@ -187,6 +198,94 @@ function writeJson( res.end(JSON.stringify(payload)); } +// --------------------------------------------------------------------------- +// Access control — the bridge can run arbitrary in-browser script, so a loopback +// bind is not enough. Each request is verified on: +// - Host: loopback only — rejects requests arriving under another hostname (DNS rebinding). +// - Origin: absent or loopback only — rejects browser cross-site calls (which always carry one). +// - Bearer token: required on every route except /health — gates local non-browser callers. +// --------------------------------------------------------------------------- + +const LOOPBACK_HOSTNAMES = new Set(["127.0.0.1", "localhost", "[::1]", "::1"]); + +/** True when a Host/Origin host component (with optional port) is loopback. */ +export function isLoopbackHost(hostHeader: string | undefined): boolean { + if (!hostHeader) return false; + let hostname: string; + if (hostHeader.startsWith("[")) { + // IPv6 literal, e.g. "[::1]:9225" → keep the bracketed form. + const end = hostHeader.indexOf("]"); + hostname = end === -1 ? hostHeader : hostHeader.slice(0, end + 1); + } else { + hostname = hostHeader.split(":")[0]; + } + return LOOPBACK_HOSTNAMES.has(hostname); +} + +/** Browsers always attach Origin; the CLI never does. Absent = trusted; present = must be loopback. */ +export function isAllowedOrigin(originHeader: string | undefined): boolean { + if (originHeader === undefined) return true; + try { + return isLoopbackHost(new URL(originHeader).host); + } catch { + return false; + } +} + +function timingSafeStrEqual(a: string, b: string): boolean { + const bufA = Buffer.from(a); + const bufB = Buffer.from(b); + if (bufA.length !== bufB.length) return false; + return timingSafeEqual(bufA, bufB); +} + +/** Extract the token from an `Authorization: Bearer ` header. */ +export function extractBearerToken( + authHeader: string | undefined, +): string | null { + if (!authHeader) return null; + const match = /^Bearer\s+(.+)$/i.exec(authHeader.trim()); + return match ? match[1] : null; +} + +export interface BridgeAccessResult { + ok: boolean; + status?: number; + error?: string; +} + +/** + * Gate on a loopback Host and an absent-or-loopback Origin; protected routes + * also require the bearer token (null token skips the check, for tests). + */ +export function checkRequestAccess( + req: IncomingMessage, + token: string | null, + requireToken: boolean, +): BridgeAccessResult { + if (!isLoopbackHost(req.headers.host)) { + return { ok: false, status: 403, error: "forbidden: invalid Host header" }; + } + if (!isAllowedOrigin(req.headers.origin)) { + return { + ok: false, + status: 403, + error: "forbidden: cross-origin request rejected", + }; + } + if (requireToken && token) { + const provided = extractBearerToken(req.headers.authorization); + if (!provided || !timingSafeStrEqual(provided, token)) { + return { ok: false, status: 401, error: "unauthorized" }; + } + } + return { ok: true }; +} + +export function generateBridgeToken(): string { + return randomBytes(32).toString("hex"); +} + async function handleToolsRequest( client: BridgeClient, res: ServerResponse, @@ -282,9 +381,18 @@ export async function handleBridgeRequest( req: IncomingMessage, res: ServerResponse, captureNextId?: () => Promise, + token: string | null = null, ): Promise { res.setHeader("Content-Type", "application/json"); + // Host/Origin guard runs on every route, including /health. + const baseAccess = checkRequestAccess(req, token, false); + if (!baseAccess.ok) { + writeJson(res, baseAccess.status ?? 403, { error: baseAccess.error }); + return; + } + + // /health is token-free so the CLI can detect a running bridge before it knows the token. if (req.method === "GET" && req.url === "/health") { if (await isBridgeClientConnected(client)) { writeJson(res, 200, { status: "ok", server: "opera-browser-cli" }); @@ -294,6 +402,13 @@ export async function handleBridgeRequest( return; } + // Every remaining route exposes browser state or automation — require the token. + const tokenAccess = checkRequestAccess(req, token, true); + if (!tokenAccess.ok) { + writeJson(res, tokenAccess.status ?? 401, { error: tokenAccess.error }); + return; + } + if (req.method === "GET" && req.url === "/last-snapshot") { if (lastSnapshot === null) { writeJson(res, 404, { error: "no snapshot cached" }); @@ -324,9 +439,10 @@ export async function handleBridgeRequest( export function createBridgeServer( client: BridgeClient, captureNextId?: () => Promise, + token: string | null = null, ): Server { return createServer((req, res) => { - void handleBridgeRequest(client, req, res, captureNextId); + void handleBridgeRequest(client, req, res, captureNextId, token); }); } @@ -511,9 +627,10 @@ export async function runBridge(port = DEFAULT_PORT): Promise { await client.connect(transport); logBridgeMessage("Connected to opera-devtools-mcp"); - const server = createBridgeServer(client, captureNextId); + const token = generateBridgeToken(); + const server = createBridgeServer(client, captureNextId, token); server.listen(port, "127.0.0.1", () => { - writePidFile(port); + writePidFile(port, token); logBridgeMessage(`Listening on http://127.0.0.1:${port}`); writeReadySignal(); }); diff --git a/src/client.ts b/src/client.ts index f7610a3..2e9f9c8 100644 --- a/src/client.ts +++ b/src/client.ts @@ -86,6 +86,7 @@ export class CdpError extends AxiError { interface PidInfo { pid: number; port: number; + token?: string; } function readPidFile(): PidInfo | null { @@ -101,6 +102,11 @@ function readPidFile(): PidInfo | null { } } +/** Read the bridge's per-instance auth token from the PID file, if present. */ +function readBridgeToken(): string | null { + return readPidFile()?.token ?? null; +} + function isProcessAlive(pid: number): boolean { try { process.kill(pid, 0); @@ -114,10 +120,18 @@ function httpGet( port: number, path: string, timeoutMs = 2000, + token?: string | null, ): Promise { return new Promise((resolve, reject) => { const req = request( - { hostname: "127.0.0.1", port, path, method: "GET", timeout: timeoutMs }, + { + hostname: "127.0.0.1", + port, + path, + method: "GET", + timeout: timeoutMs, + headers: token ? { Authorization: `Bearer ${token}` } : undefined, + }, (res) => { let data = ""; res.on("data", (chunk) => (data += chunk)); @@ -139,6 +153,7 @@ function httpPost( body: unknown, timeoutMs = 120_000, onLog?: (message: string) => void, + token?: string | null, ): Promise { return new Promise((resolve, reject) => { const payload = JSON.stringify(body); @@ -152,6 +167,7 @@ function httpPost( headers: { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(payload), + ...(token ? { Authorization: `Bearer ${token}` } : {}), }, }, (res) => { @@ -342,7 +358,8 @@ export async function callTool( : undefined; try { - const resp = await httpPost(port, "/call", { name, args }, timeoutMs, onLog); + const token = readBridgeToken(); + const resp = await httpPost(port, "/call", { name, args }, timeoutMs, onLog, token); const data = JSON.parse(resp); if (data.error) { throw new Error(data.error); @@ -447,7 +464,7 @@ export async function getLastSnapshot(): Promise { const pidInfo = readPidFile(); if (!pidInfo || !isProcessAlive(pidInfo.pid)) return null; try { - const resp = await httpGet(pidInfo.port, "/last-snapshot", 2000); + const resp = await httpGet(pidInfo.port, "/last-snapshot", 2000, pidInfo.token); const data = JSON.parse(resp) as { error?: string } & Partial; if (data.error || !data.raw) return null; return { raw: data.raw, pageUrl: data.pageUrl ?? null, capturedAt: data.capturedAt ?? 0 }; @@ -470,6 +487,8 @@ export async function getSessionSnapshotIfRunning(): Promise { "/call", { name: "take_snapshot", args: {} }, 5000, + undefined, + pidInfo.token, ); const data = JSON.parse(resp); if (data.error) return null; diff --git a/test/bridge.test.ts b/test/bridge.test.ts index e68471b..ff4d3eb 100644 --- a/test/bridge.test.ts +++ b/test/bridge.test.ts @@ -4,11 +4,16 @@ import type { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdi import type { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js"; import { buildTransportArgs, + checkRequestAccess, + extractBearerToken, extractToolText, + generateBridgeToken, getErrorMessage, getLastSnapshotCache, handleBridgeRequest, + isAllowedOrigin, isBridgeClientConnected, + isLoopbackHost, parseBridgeCallPayload, resolveBridgeScript, resetLastSnapshotCache, @@ -212,10 +217,16 @@ function makeMockTransport() { return transport; } -function makeMockRequest(method: string, url: string, body = ""): IncomingMessage { +function makeMockRequest( + method: string, + url: string, + body = "", + headers: Record = { host: "127.0.0.1:9225" }, +): IncomingMessage { return { method, url, + headers, [Symbol.asyncIterator]: async function* () { yield body; }, } as unknown as IncomingMessage; } @@ -310,6 +321,187 @@ describe("wrapTransportForIdCapture", () => { }); }); +// --------------------------------------------------------------------------- +// Access control — Host / Origin guard + bearer token +// --------------------------------------------------------------------------- + +describe("isLoopbackHost", () => { + it("accepts loopback hostnames with or without a port", () => { + expect(isLoopbackHost("127.0.0.1:9225")).toBe(true); + expect(isLoopbackHost("localhost:9225")).toBe(true); + expect(isLoopbackHost("127.0.0.1")).toBe(true); + expect(isLoopbackHost("[::1]:9225")).toBe(true); + }); + + it("rejects non-loopback and missing hosts", () => { + expect(isLoopbackHost("mario.evil.example:9225")).toBe(false); + expect(isLoopbackHost("127.0.0.1.evil.example")).toBe(false); + expect(isLoopbackHost(undefined)).toBe(false); + expect(isLoopbackHost("")).toBe(false); + }); +}); + +describe("isAllowedOrigin", () => { + it("allows an absent Origin (non-browser caller)", () => { + expect(isAllowedOrigin(undefined)).toBe(true); + }); + + it("allows loopback origins", () => { + expect(isAllowedOrigin("http://localhost:9225")).toBe(true); + expect(isAllowedOrigin("http://127.0.0.1:5173")).toBe(true); + }); + + it("rejects web origins and the null origin", () => { + expect(isAllowedOrigin("https://mario.evil.example")).toBe(false); + expect(isAllowedOrigin("null")).toBe(false); + }); +}); + +describe("extractBearerToken", () => { + it("parses a Bearer token case-insensitively", () => { + expect(extractBearerToken("Bearer abc123")).toBe("abc123"); + expect(extractBearerToken("bearer abc123")).toBe("abc123"); + }); + + it("returns null for missing or malformed headers", () => { + expect(extractBearerToken(undefined)).toBeNull(); + expect(extractBearerToken("Basic abc123")).toBeNull(); + }); +}); + +describe("checkRequestAccess", () => { + const reqWith = (headers: Record) => + ({ headers } as unknown as IncomingMessage); + + it("rejects a forged (non-loopback) Host header", () => { + const result = checkRequestAccess( + reqWith({ host: "mario.evil.example:9225" }), + null, + false, + ); + expect(result.ok).toBe(false); + expect(result.status).toBe(403); + }); + + it("rejects a cross-origin request even with a loopback Host", () => { + const result = checkRequestAccess( + reqWith({ host: "127.0.0.1:9225", origin: "https://mario.evil.example" }), + null, + false, + ); + expect(result.ok).toBe(false); + expect(result.status).toBe(403); + }); + + it("requires a valid bearer token on protected routes", () => { + const headers = { host: "127.0.0.1:9225" }; + expect(checkRequestAccess(reqWith(headers), "secret", true).status).toBe(401); + expect( + checkRequestAccess( + reqWith({ ...headers, authorization: "Bearer wrong" }), + "secret", + true, + ).status, + ).toBe(401); + expect( + checkRequestAccess( + reqWith({ ...headers, authorization: "Bearer secret" }), + "secret", + true, + ).ok, + ).toBe(true); + }); + + it("accepts a loopback request with no Origin and no token required", () => { + expect( + checkRequestAccess(reqWith({ host: "127.0.0.1:9225" }), null, false).ok, + ).toBe(true); + }); +}); + +describe("generateBridgeToken", () => { + it("produces a 64-char hex token that differs each call", () => { + const a = generateBridgeToken(); + const b = generateBridgeToken(); + expect(a).toMatch(/^[0-9a-f]{64}$/); + expect(a).not.toBe(b); + }); +}); + +describe("handleBridgeRequest access control", () => { + const okClient: BridgeClient = { + listTools: async () => ({ tools: [{ name: "click", description: "" }] }), + callTool: async () => ({ content: [{ type: "text", text: "ok" }] }), + close: async () => {}, + }; + + it("rejects a forged-Host /call with 403 before dispatching", async () => { + let called = false; + const spyClient: BridgeClient = { + ...okClient, + callTool: async () => { called = true; return { content: [] }; }, + }; + const req = makeMockRequest( + "POST", + "/call", + JSON.stringify({ name: "list_pages", args: {} }), + { host: "mario.evil.example:9225" }, + ); + const mock = makeMockResponse(); + await handleBridgeRequest(spyClient, req, mock.res, undefined, "secret"); + expect(mock.res.statusCode).toBe(403); + expect(called).toBe(false); + }); + + it("rejects a cross-origin /call with 403", async () => { + const req = makeMockRequest( + "POST", + "/call", + JSON.stringify({ name: "list_pages", args: {} }), + { host: "127.0.0.1:9225", origin: "https://mario.evil.example" }, + ); + const mock = makeMockResponse(); + await handleBridgeRequest(okClient, req, mock.res, undefined, "secret"); + expect(mock.res.statusCode).toBe(403); + }); + + it("rejects /call without the bearer token (401)", async () => { + const req = makeMockRequest( + "POST", + "/call", + JSON.stringify({ name: "list_pages", args: {} }), + { host: "127.0.0.1:9225" }, + ); + const mock = makeMockResponse(); + await handleBridgeRequest(okClient, req, mock.res, undefined, "secret"); + expect(mock.res.statusCode).toBe(401); + }); + + it("accepts /call with a valid bearer token", async () => { + const req = makeMockRequest( + "POST", + "/call", + JSON.stringify({ name: "list_pages", args: {} }), + { host: "127.0.0.1:9225", authorization: "Bearer secret" }, + ); + const mock = makeMockResponse(); + await handleBridgeRequest(okClient, req, mock.res, undefined, "secret"); + expect(JSON.parse(mock.endPayload)).toEqual({ result: "ok" }); + }); + + it("serves /health without a token but still enforces the Host guard", async () => { + const good = makeMockRequest("GET", "/health", "", { host: "127.0.0.1:9225" }); + const goodMock = makeMockResponse(); + await handleBridgeRequest(okClient, good, goodMock.res, undefined, "secret"); + expect(goodMock.res.statusCode).toBe(200); + + const bad = makeMockRequest("GET", "/health", "", { host: "evil.example" }); + const badMock = makeMockResponse(); + await handleBridgeRequest(okClient, bad, badMock.res, undefined, "secret"); + expect(badMock.res.statusCode).toBe(403); + }); +}); + // --------------------------------------------------------------------------- // handleBridgeRequest — streaming path // ---------------------------------------------------------------------------