diff --git a/ts/packages/agents/browser/src/agent/browserActionHandler.mts b/ts/packages/agents/browser/src/agent/browserActionHandler.mts index 1a412517e8..2fbe1176a0 100644 --- a/ts/packages/agents/browser/src/agent/browserActionHandler.mts +++ b/ts/packages/agents/browser/src/agent/browserActionHandler.mts @@ -592,8 +592,31 @@ async function updateBrowserContext( } if (!context.agentContext.viewProcess) { - context.agentContext.viewProcess = - await createViewServiceHost(context); + const viewProcess = await createViewServiceHost(context); + if (viewProcess) { + context.agentContext.viewProcess = viewProcess; + // Defensive cleanup if the child crashes mid-session. + // The dispatcher's PortRegistrar leaves stale entries + // bounded to "until respawn or session end", but a + // crashed child should release its registration eagerly + // so the entry doesn't shadow a fresh bind. The + // identity guard prevents a late-firing `exit` event on + // a previously-replaced process from clobbering a newer + // registration; the explicit disable path (which also + // releases) is naturally idempotent under `?.release()`. + viewProcess.once("exit", () => { + if (context.agentContext.viewProcess !== viewProcess) { + return; + } + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = undefined; + context.agentContext.viewProcess = undefined; + // Reset cached port so respawn forks with arg "0" + // (OS-assigned) instead of trying to re-bind the + // stale port — mirrors the disable/close paths. + context.agentContext.localHostPort = 0; + }); + } } if (context.agentContext.browserSchemaEnabled) { @@ -783,6 +806,8 @@ async function updateBrowserContext( if (context.agentContext.viewProcess) { context.agentContext.viewProcess.kill(); context.agentContext.viewProcess = undefined; + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = undefined; // Reset to OS-assigned so a subsequent re-enable forks // server.mjs with arg "0" instead of the stale port. The // killed child may still hold the old port for a brief @@ -806,6 +831,8 @@ async function closeBrowserContext( if (context.agentContext.viewProcess) { context.agentContext.viewProcess.kill(); context.agentContext.viewProcess = undefined; + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = undefined; context.agentContext.localHostPort = 0; } } @@ -2471,7 +2498,9 @@ async function createViewServiceHost( childProcess.on("message", function (message: any) { if (message?.type === "Success") { context.agentContext.localHostPort = message.port; - context.setLocalHostPort(message.port); + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = + context.registerPort("view", message.port); resolve(childProcess); } else if (message === "Failure") { resolve(undefined); diff --git a/ts/packages/agents/browser/src/agent/browserActions.mts b/ts/packages/agents/browser/src/agent/browserActions.mts index b766c3c0f1..2ec2606c86 100644 --- a/ts/packages/agents/browser/src/agent/browserActions.mts +++ b/ts/packages/agents/browser/src/agent/browserActions.mts @@ -38,6 +38,10 @@ export type BrowserActionContext = { index: IndexData | undefined; viewProcess?: ChildProcess | undefined; localHostPort: number; + // Handle returned by sessionContext.registerPort for the views + // server (PDF viewer / per-session HTML views). Released on + // updateBrowserContext(false, ...) or closeAgentContext. + viewPortRegistration?: { release: () => void } | undefined; webFlowStore?: WebFlowStore | undefined; choiceManager?: ChoiceManager | undefined; lastInferredActions?: any[] | undefined; diff --git a/ts/packages/agents/browser/src/views/server/core/baseServer.ts b/ts/packages/agents/browser/src/views/server/core/baseServer.ts index 9474391730..440c28f4e1 100644 --- a/ts/packages/agents/browser/src/views/server/core/baseServer.ts +++ b/ts/packages/agents/browser/src/views/server/core/baseServer.ts @@ -7,6 +7,7 @@ import path from "path"; import { fileURLToPath } from "url"; import { FeatureConfig, ServerConfig, SSEManager } from "./types.js"; import { SSEManagerImpl } from "./sseManager.js"; +import { isAllowedViewOrigin } from "./originAllowlist.js"; import registerDebug from "debug"; const debug = registerDebug("typeagent:views:server:core"); @@ -37,6 +38,22 @@ export class BaseServer { * Setup core middleware */ private setupMiddleware(): void { + // Origin allowlist — runs before everything else so non-loopback + // requests get HTTP 403 without consuming rate-limit budget or + // touching feature routes. The bind is loopback-only, but a + // malicious local web page in any browser tab can still hit + // `http://localhost:` via fetch/XHR/iframe; the gate stops + // that. + this.app.use((req, res, next) => { + const origin = req.headers.origin; + if (isAllowedViewOrigin(origin)) { + next(); + return; + } + debug(`Rejecting request from origin ${origin}`); + res.status(403).send("Origin not allowed"); + }); + // Rate limiting const limiter = rateLimit({ windowMs: this.config.rateLimitWindow || 60000, diff --git a/ts/packages/agents/browser/src/views/server/core/originAllowlist.ts b/ts/packages/agents/browser/src/views/server/core/originAllowlist.ts new file mode 100644 index 0000000000..a7937aa920 --- /dev/null +++ b/ts/packages/agents/browser/src/views/server/core/originAllowlist.ts @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { createAgentOriginAllowlist } from "websocket-utils/originAllowlist"; + +/** + * Origin allowlist for the browser views server (PDF viewer + other + * per-session HTML views forked as a child process by the browser + * agent). + * + * Distinct from `agents/browser/src/agent/originAllowlist.mts`, which + * gates the WS bridge used by the typeagent Chrome/Edge extension. The + * views server is consumed by: + * - the Electron shell's inline browser (origin + * `http(s)://localhost(:port)` / `127.0.0.1` / `[::1]`), + * - external loopback browser tabs the user opens manually, + * - same-origin XHR/fetch from the served HTML (Origin absent or the + * server's own loopback origin). + * + * No browser-extension scheme is accepted here — this listener does not + * back any extension UI. See {@link createAgentOriginAllowlist} for the + * shared loopback + no-Origin baseline. + * + * `Origin: "null"` (sent by `file://` pages and sandboxed iframes) is + * rejected — only regular browser tabs and same-origin fetches are + * legitimate clients, so an opaque-origin caller is necessarily + * something we do not want to honor. + * + * Anything else is rejected with HTTP 403 before the route handler runs. + */ +export const isAllowedViewOrigin = createAgentOriginAllowlist({ + allowNullOrigin: false, +}); diff --git a/ts/packages/agents/browser/test/unit/agentWebSocketServer.test.ts b/ts/packages/agents/browser/test/unit/agentWebSocketServer.test.ts index adcde8c398..9e4696fde3 100644 --- a/ts/packages/agents/browser/test/unit/agentWebSocketServer.test.ts +++ b/ts/packages/agents/browser/test/unit/agentWebSocketServer.test.ts @@ -63,6 +63,7 @@ jest.mock("debug", () => { import { AgentWebSocketServer } from "../../src/agent/agentWebSocketServer.mjs"; import { isAllowedAgentOrigin } from "../../src/agent/originAllowlist.mjs"; +import { createAgentOriginAllowlist } from "websocket-utils/originAllowlist"; function makeMockSocket() { const handlers: Record = {}; @@ -425,3 +426,69 @@ describe("isAllowedAgentOrigin", () => { expect(isAllowedAgentOrigin("not a url")).toBe(false); }); }); + +// Direct tests of the shared factory cover the policy knobs we expose +// (`allowNullOrigin`, the `string[]` header normalization). The +// agent-side wrapper above tests the default `allowNullOrigin: true` +// branch as a side effect; these tests add explicit coverage for the +// view-server posture where `"null"` must be rejected. +describe("createAgentOriginAllowlist", () => { + describe("allowNullOrigin option", () => { + it('rejects Origin: "null" when allowNullOrigin is false', () => { + const allow = createAgentOriginAllowlist({ + allowNullOrigin: false, + }); + expect(allow("null")).toBe(false); + }); + it("still allows missing/empty Origin when allowNullOrigin is false", () => { + const allow = createAgentOriginAllowlist({ + allowNullOrigin: false, + }); + expect(allow(undefined)).toBe(true); + expect(allow("")).toBe(true); + }); + it("still allows loopback origins when allowNullOrigin is false", () => { + const allow = createAgentOriginAllowlist({ + allowNullOrigin: false, + }); + expect(allow("http://localhost:1234")).toBe(true); + expect(allow("http://127.0.0.1")).toBe(true); + expect(allow("http://[::1]:5173")).toBe(true); + }); + it('accepts Origin: "null" by default (backwards-compat)', () => { + const allow = createAgentOriginAllowlist(); + expect(allow("null")).toBe(true); + }); + it('accepts Origin: "null" when allowNullOrigin is explicitly true', () => { + const allow = createAgentOriginAllowlist({ + allowNullOrigin: true, + }); + expect(allow("null")).toBe(true); + }); + }); + describe("string[] header normalization", () => { + // Node's TS header types claim repeated headers may surface as + // `string[]`. At runtime the parser joins repeated Origin + // headers into a single comma-separated string, so the array + // form is not expected — but if it ever does arrive, anything + // other than a single entry is inherently ambiguous and must be + // rejected. + it("rejects an empty array", () => { + const allow = createAgentOriginAllowlist(); + expect(allow([])).toBe(false); + }); + it("rejects an array with more than one entry", () => { + const allow = createAgentOriginAllowlist(); + expect( + allow(["http://localhost", "https://evil.example.com"]), + ).toBe(false); + // Even two loopback entries are ambiguous and rejected. + expect(allow(["http://localhost", "http://127.0.0.1"])).toBe(false); + }); + it("normalizes a single-element array to that entry", () => { + const allow = createAgentOriginAllowlist(); + expect(allow(["http://localhost:1234"])).toBe(true); + expect(allow(["https://evil.example.com"])).toBe(false); + }); + }); +}); diff --git a/ts/packages/agents/markdown/package.json b/ts/packages/agents/markdown/package.json index dd4f885131..85d63086db 100644 --- a/ts/packages/agents/markdown/package.json +++ b/ts/packages/agents/markdown/package.json @@ -57,6 +57,7 @@ "telemetry": "workspace:*", "typechat": "^0.1.1", "unist-util-visit": "^4.1.2", + "websocket-utils": "workspace:*", "ws": "^8.20.1", "y-prosemirror": "^1.2.3", "y-protocols": "^1.0.5", diff --git a/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts b/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts index a7f7255d32..d4158d0385 100644 --- a/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts +++ b/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts @@ -44,6 +44,10 @@ type MarkdownActionContext = { currentFileName?: string | undefined; viewProcess?: ChildProcess | undefined; localHostPort: number; + // Handle returned by sessionContext.registerPort for the markdown + // preview / Yjs WebSocket server. Released on + // updateMarkdownContext(false, ...). + viewPortRegistration?: { release: () => void } | undefined; }; async function handleUICommand( @@ -302,9 +306,26 @@ async function updateMarkdownContext( context.agentContext.localHostPort, ); if (result) { - context.agentContext.viewProcess = result.process; + const viewProcess = result.process; + context.agentContext.viewProcess = viewProcess; context.agentContext.localHostPort = result.port; - context.setLocalHostPort(result.port); + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = + context.registerPort("view", result.port); + // Defensive cleanup if the child crashes mid-session. + // The identity guard prevents a late-firing `exit` + // event on a previously-replaced process from + // clobbering a newer registration; the explicit + // disable path (which also releases) is naturally + // idempotent under `?.release()`. + viewProcess.once("exit", () => { + if (context.agentContext.viewProcess !== viewProcess) { + return; + } + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = undefined; + context.agentContext.viewProcess = undefined; + }); } } } @@ -314,6 +335,8 @@ async function updateMarkdownContext( if (context.agentContext.viewProcess) { context.agentContext.viewProcess.kill(); context.agentContext.viewProcess = undefined; + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = undefined; } } } diff --git a/ts/packages/agents/markdown/src/view/route/originAllowlist.ts b/ts/packages/agents/markdown/src/view/route/originAllowlist.ts new file mode 100644 index 0000000000..ce4e850ff3 --- /dev/null +++ b/ts/packages/agents/markdown/src/view/route/originAllowlist.ts @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { createAgentOriginAllowlist } from "websocket-utils/originAllowlist"; + +/** + * Origin allowlist for the markdown view server (HTTP preview + Yjs + * collaboration WebSocket). + * + * Accepts only the shared loopback + no-Origin baseline documented on + * {@link createAgentOriginAllowlist} (loopback browser tabs, the + * Electron shell's inline browser, same-origin XHR/fetch from the + * preview page, and Node `ws`/HTTP clients). The server binds to + * localhost, so this is loopback-restricted at the OS level. + * + * `Origin: "null"` (sent by `file://` pages and sandboxed iframes) is + * rejected — the preview server only ever serves regular browser tabs + * and same-origin fetches, so an opaque-origin caller is necessarily + * something we do not want to honor. + * + * Anything else is rejected with HTTP 403 (HTTP routes) or 403 on the + * upgrade response (WebSocket). + */ +export const isAllowedViewOrigin = createAgentOriginAllowlist({ + allowNullOrigin: false, +}); diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index e611c764e0..38fae7f7db 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -18,6 +18,7 @@ import * as encoding from "lib0/encoding"; import * as decoding from "lib0/decoding"; import registerDebug from "debug"; import sanitizeFilename from "sanitize-filename"; +import { isAllowedViewOrigin } from "./originAllowlist.js"; const debug = registerDebug("typeagent:markdown:service"); @@ -26,6 +27,23 @@ const port = parseInt(process.argv[2]); if (isNaN(port)) { throw new Error("Port must be a number"); } + +// Origin allowlist — runs before everything else so non-loopback +// requests get HTTP 403 without consuming rate-limit budget or hitting +// the route handlers. The server binds to localhost, but any local +// browser tab can still hit `http://localhost:` via fetch/XHR; +// the gate stops cross-origin reads of the live document and uploaded +// files. The Y.js WebSocket upgrade is gated separately in +// createYjsWSServer. +app.use((req, res, next) => { + const origin = req.headers.origin; + if (isAllowedViewOrigin(origin)) { + next(); + return; + } + debug(`Rejecting request from origin ${origin}`); + res.status(403).send("Origin not allowed"); +}); const limiter = rateLimit({ windowMs: 60000, max: 100, // limit each IP to 100 requests per windowMs @@ -2211,6 +2229,24 @@ function createYjsWSServer(server: http.Server): WebSocketServer { server.on("upgrade", (request, socket, head) => { try { + // Origin gate — same allowlist as the HTTP middleware. The + // Yjs WS carries the full document content + edit stream, + // so loopback bind alone isn't enough; any local web page + // could otherwise open a WS to localhost: and read / + // mutate the live doc. + const origin = request.headers.origin as string | undefined; + if (!isAllowedViewOrigin(origin)) { + debug(`Rejecting WS upgrade from origin ${origin}`); + socket.write( + "HTTP/1.1 403 Forbidden\r\n" + + "Connection: close\r\n" + + "Content-Length: 0\r\n" + + "\r\n", + ); + socket.destroy(); + return; + } + // Extract room name from URL path const url = new URL( request.url || "/", diff --git a/ts/packages/agents/montage/package.json b/ts/packages/agents/montage/package.json index 9dd7b2ecd3..db474dacff 100644 --- a/ts/packages/agents/montage/package.json +++ b/ts/packages/agents/montage/package.json @@ -44,6 +44,7 @@ "sharp": "^0.33.5", "typeagent": "workspace:*", "typechat": "^0.1.1", + "websocket-utils": "workspace:*", "winreg": "^1.2.5" }, "devDependencies": { diff --git a/ts/packages/agents/montage/src/agent/montageActionHandler.ts b/ts/packages/agents/montage/src/agent/montageActionHandler.ts index fd1c1abb0d..a9df81d835 100644 --- a/ts/packages/agents/montage/src/agent/montageActionHandler.ts +++ b/ts/packages/agents/montage/src/agent/montageActionHandler.ts @@ -60,6 +60,10 @@ type MontageActionContext = { imageCollection?: im.ImageCollection | undefined; viewProcess?: ChildProcess | undefined; localHostPort: number; + // Handle returned by sessionContext.registerPort for the gallery + // view server. Released on updateMontageContext(false, ...) or + // closeMontageContext. + viewPortRegistration?: { release: () => void } | undefined; searchSettings: { minScore: number; exactMatch: boolean; @@ -233,6 +237,8 @@ async function closeMontageContext( if (context.agentContext.viewProcess) { context.agentContext.viewProcess.kill(); context.agentContext.viewProcess = undefined; + context.agentContext.viewPortRegistration?.release(); + context.agentContext.viewPortRegistration = undefined; } } @@ -297,8 +303,28 @@ async function updateMontageContext( agentContext.localHostPort, ); if (viewServiceResult) { - agentContext.viewProcess = viewServiceResult.process; - context.setLocalHostPort(viewServiceResult.port); + const viewProcess = viewServiceResult.process; + agentContext.viewProcess = viewProcess; + agentContext.localHostPort = viewServiceResult.port; + agentContext.viewPortRegistration?.release(); + agentContext.viewPortRegistration = context.registerPort( + "view", + viewServiceResult.port, + ); + // Defensive cleanup if the child crashes mid-session. + // The identity guard prevents a late-firing `exit` + // event on a previously-replaced process from + // clobbering a newer registration; the explicit + // disable path (which also releases) is naturally + // idempotent under `?.release()`. + viewProcess.once("exit", () => { + if (agentContext.viewProcess !== viewProcess) { + return; + } + agentContext.viewPortRegistration?.release(); + agentContext.viewPortRegistration = undefined; + agentContext.viewProcess = undefined; + }); } // send the folder info @@ -333,6 +359,8 @@ async function updateMontageContext( if (agentContext.viewProcess) { agentContext.viewProcess.kill(); agentContext.viewProcess = undefined; + agentContext.viewPortRegistration?.release(); + agentContext.viewPortRegistration = undefined; } } } diff --git a/ts/packages/agents/montage/src/route/originAllowlist.ts b/ts/packages/agents/montage/src/route/originAllowlist.ts new file mode 100644 index 0000000000..f2f245e22d --- /dev/null +++ b/ts/packages/agents/montage/src/route/originAllowlist.ts @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { createAgentOriginAllowlist } from "websocket-utils/originAllowlist"; + +/** + * Origin allowlist for the montage view server. + * + * Accepts only the shared loopback + no-Origin baseline documented on + * {@link createAgentOriginAllowlist} (loopback browser tabs, the + * Electron shell's inline browser, same-origin XHR/fetch from the + * gallery page, and Node `ws`/HTTP clients). The server binds to + * localhost, so this is loopback-restricted at the OS level. + * + * `Origin: "null"` (sent by `file://` pages and sandboxed iframes) is + * rejected — the gallery only ever serves regular browser tabs and + * same-origin fetches, so an opaque-origin caller is necessarily + * something we do not want to honor. + * + * Anything else is rejected with HTTP 403 before the route handler runs. + */ +export const isAllowedViewOrigin = createAgentOriginAllowlist({ + allowNullOrigin: false, +}); diff --git a/ts/packages/agents/montage/src/route/route.ts b/ts/packages/agents/montage/src/route/route.ts index 54d86bb371..f6565dbed2 100644 --- a/ts/packages/agents/montage/src/route/route.ts +++ b/ts/packages/agents/montage/src/route/route.ts @@ -10,6 +10,7 @@ import path from "path"; import sharp from "sharp"; import fs from "node:fs"; import registerDebug from "debug"; +import { isAllowedViewOrigin } from "./originAllowlist.js"; const debug = registerDebug("typeagent:agent:montage:route"); const app: Express = express(); @@ -33,6 +34,21 @@ let rootImageFolder: string; // The last message sent to the clients let lastMessage: any = {}; +// Origin allowlist — runs before everything else so non-loopback +// requests get HTTP 403 without consuming rate-limit budget or hitting +// the route handlers. The server binds to localhost, but any local +// browser tab can still hit `http://localhost:` via fetch/XHR; +// the gate stops cross-origin reads of indexed images and folders. +app.use((req, res, next) => { + const origin = req.headers.origin; + if (isAllowedViewOrigin(origin)) { + next(); + return; + } + debug(`Rejecting request from origin ${origin}`); + res.status(403).send("Origin not allowed"); +}); + // limit request rage const limiter = rateLimit({ windowMs: 1000, diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts index 876252e914..ca885921f1 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts @@ -55,6 +55,19 @@ import { RegistrationId, } from "./portRegistrar.js"; +/** + * Role string used by agents that have migrated off the legacy + * `setLocalHostPort` shim and onto explicit + * `sessionContext.registerPort("view", port)` for their per-session + * local view (browser views, montage gallery, markdown preview). + * + * The legacy "default" role and this "view" role coexist: lookups for + * `getLocalHostPort` / `getSharedLocalHostPort` try "default" first + * (back-compat) and fall back to "view" (new pattern). This lets + * cross-agent site lookup keep working through the migration. + */ +const LOCAL_VIEW_ROLE = "view"; + const debug = registerDebug("typeagent:dispatcher:agents"); const debugError = registerDebug("typeagent:dispatcher:agents:error"); const debugLoad = registerDebug("typeagent:dispatcher:agents:load"); @@ -435,7 +448,13 @@ export class AppAgentManager implements ActionConfigProvider { } public getLocalHostPort(appAgentName: string) { - return this.portRegistrar.lookup(appAgentName, DEFAULT_ROLE); + // Fall back to the "view" role for agents migrated off the + // legacy `setLocalHostPort` shim (browser-views, montage, + // markdown). Lookup order matches `getSharedLocalHostPort`. + return ( + this.portRegistrar.lookup(appAgentName, DEFAULT_ROLE) ?? + this.portRegistrar.lookup(appAgentName, LOCAL_VIEW_ROLE) + ); } /** @@ -490,7 +509,13 @@ export class AppAgentManager implements ActionConfigProvider { ); } - const port = this.portRegistrar.lookup(target, DEFAULT_ROLE); + // Lookup order: "default" first for back-compat with legacy + // `setLocalHostPort` callers, then "view" for agents that + // migrated to the explicit `registerPort("view", ...)` pattern + // (browser-views, montage, markdown). + const port = + this.portRegistrar.lookup(target, DEFAULT_ROLE) ?? + this.portRegistrar.lookup(target, LOCAL_VIEW_ROLE); if (port === undefined) { throw new Error(`Local view not available for agent '${target}'.`); } diff --git a/ts/packages/utils/webSocketUtils/src/originAllowlist.ts b/ts/packages/utils/webSocketUtils/src/originAllowlist.ts index fa94e6c19a..134839eb73 100644 --- a/ts/packages/utils/webSocketUtils/src/originAllowlist.ts +++ b/ts/packages/utils/webSocketUtils/src/originAllowlist.ts @@ -13,9 +13,15 @@ * parser preserves IPv6 brackets in `hostname`, so we match the * bracketed form, while the unbracketed form is accepted defensively * against URL parser/serializer differences across runtimes). - * - A missing/empty/`"null"` Origin is allowed — Node `ws` clients and - * other non-browser callers don't send Origin, and the listener binds - * to loopback so this remains OS-level restricted. + * - A missing/empty Origin is allowed — Node `ws` clients and other + * non-browser callers don't send Origin, and the listener binds to + * loopback so this remains OS-level restricted. + * - An `Origin: "null"` header (sent by `file://` pages and sandboxed/ + * opaque-origin documents) is allowed only when + * {@link OriginAllowlistOptions.allowNullOrigin} is `true` (the + * default, for backwards compatibility with agent-side bridges that + * may see opaque-origin clients). View-server gates that only ever + * serve browser tabs should set this to `false`. * - All other `http(s)` origins are rejected; non-`http(s)` schemes are * rejected unless explicitly named in {@link extensionSchemes}. * @@ -25,6 +31,14 @@ * matched as a literal prefix against the Origin string (e.g. * `"chrome-extension://"` accepts `chrome-extension://abc123`). * + * The returned predicate accepts the raw Node header value type + * (`string | string[] | undefined`). In practice Node combines repeated + * Origin headers into a single comma-joined string at the parser level, + * so the array form is not expected at runtime — but if it ever + * appears, an array of length other than 1 is rejected outright and a + * single-element array is normalized to its sole entry. Repeated + * Origins are inherently ambiguous and should not be trusted. + * * Returned predicates are pure and reusable; build once per agent and * call from the WS server's `verifyClient` so denied clients are * rejected with HTTP 403 before the `connection` event fires. @@ -37,6 +51,15 @@ export type OriginAllowlistOptions = { * include the trailing `://`. */ extensionSchemes?: readonly string[]; + /** + * Whether to accept `Origin: "null"` (the opaque-origin sentinel + * sent by `file://` pages and sandboxed iframes). Defaults to + * `true` for backwards compatibility with agent-side bridges. View + * servers that only intend to serve same-origin browser tabs should + * set this to `false` so a malicious `file://` page can't read + * loopback responses. + */ + allowNullOrigin?: boolean; }; /** @@ -46,21 +69,40 @@ export type OriginAllowlistOptions = { */ export function createAgentOriginAllowlist( options: OriginAllowlistOptions = {}, -): (origin: string | undefined) => boolean { +): (origin: string | string[] | undefined) => boolean { const schemes = options.extensionSchemes ?? []; - return (origin: string | undefined): boolean => { - if (origin === undefined || origin === "" || origin === "null") { + const allowNullOrigin = options.allowNullOrigin ?? true; + return (origin: string | string[] | undefined): boolean => { + // Node's header types claim repeated headers may surface as + // `string[]`. In practice the parser joins repeated Origin + // headers into a single comma-separated string, but if an array + // ever does arrive, reject anything other than a single entry — + // multiple Origins are inherently ambiguous and the safer + // posture is to drop the request. + let header: string | undefined; + if (Array.isArray(origin)) { + if (origin.length !== 1) { + return false; + } + header = origin[0]; + } else { + header = origin; + } + if (header === undefined || header === "") { // No Origin header: legitimate for Node `ws` and other // non-browser clients. return true; } + if (header === "null") { + return allowNullOrigin; + } for (const scheme of schemes) { - if (origin.startsWith(scheme)) { + if (header.startsWith(scheme)) { return true; } } try { - const u = new URL(origin); + const u = new URL(header); if (u.protocol !== "http:" && u.protocol !== "https:") { return false; } diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 0c356272b0..60a64fb592 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -2533,6 +2533,9 @@ importers: unist-util-visit: specifier: ^4.1.2 version: 4.1.2 + websocket-utils: + specifier: workspace:* + version: link:../../utils/webSocketUtils ws: specifier: ^8.20.1 version: 8.20.1 @@ -2645,6 +2648,9 @@ importers: typechat: specifier: ^0.1.1 version: 0.1.1(typescript@5.4.5)(zod@3.25.76) + websocket-utils: + specifier: workspace:* + version: link:../../utils/webSocketUtils winreg: specifier: ^1.2.5 version: 1.2.5