Merge pull request #2370 from modelcontextprotocol/v2/feat/1862-sandbox-public-url
feat(web): configurable public addresses for the MCP Apps listeners
This commit is contained in:
Cliff Hall
GitHub
commit
fd6e60148b
12 changed files with 998 additions and 19 deletions
|
|
@ -19,7 +19,7 @@ The `server/` directory holds the Node-only backend:
|
|||
- **`web-server-config.ts`** — env parsing, the `GET /api/config` payload, the startup banner, the default origin allow-list.
|
||||
- **`resolve-bind-host.ts`** — the shared bind-host guard (refuses an all-interfaces `HOST` unless `DANGEROUSLY_BIND_ALL_INTERFACES`), used by both bind points (`web-server-config.ts` + `vite.config.ts`); see [Host binding & the origin allow-list](#host-binding--the-origin-allow-list).
|
||||
- **`inject-auth-token.ts`** — embeds the API token into the served `index.html` (see [Auth token](#auth-token)).
|
||||
- **`sandbox-controller.ts`** — the MCP Apps sandbox HTTP server; **`app-origin-controller.ts`** — the dedicated app-origin server for `_meta.ui.domain` (see [MCP App dedicated origins](#mcp-app-dedicated-origins-metauidomain)); **`ensure-web-build.ts`** — builds `dist/` on demand for prod `--web`; **`vite-base-config.ts`** — shared `optimizeDeps` exclusions.
|
||||
- **`sandbox-controller.ts`** — the MCP Apps sandbox HTTP server; **`app-origin-controller.ts`** — the dedicated app-origin server for `_meta.ui.domain` (see [MCP App dedicated origins](#mcp-app-dedicated-origins-metauidomain)); **`public-address.ts`** — validates `MCP_SANDBOX_FULL_ADDRESS` / `MCP_APP_ORIGIN_FULL_ADDRESS`, the public addresses those two advertise behind a reverse proxy; **`ensure-web-build.ts`** — builds `dist/` on demand for prod `--web`; **`vite-base-config.ts`** — shared `optimizeDeps` exclusions.
|
||||
- **`browser-externalized-builtin-gate.ts`** — Vite-agnostic build-gate logic that fails `vite build` when a Node built-in reaches the browser bundle (#1769); the thin Vite plugin wiring lives in `vite.config.ts`. It sits under `server/` (rather than `src/`) as the home for Node-only, build-time tooling — it's imported by the Vite config, never by the browser — alongside the other `vite-*` config helpers here.
|
||||
|
||||
## Development
|
||||
|
|
@ -417,7 +417,22 @@ The guard blocks only the **wildcard** all-interfaces addresses. Binding a **spe
|
|||
|
||||
The bind-host guard and the `ALLOWED_ORIGINS` allow-list apply to both the prod server and `--dev`. Note that in **`--dev`** the Vite dev server _additionally_ enforces its own `server.allowedHosts` Host-header check, whose default accepts loopback and IP-literal hosts. The host you **bind** is auto-allowed (Vite adds the resolved `server.host` — which this config sets from `HOST` — to the allow-list), so `HOST=<hostname>` works out of the box under `--dev` too. What needs an explicit `server.allowedHosts` entry is reaching the dev server at a **different** name than the one bound — e.g. a wildcard bind reached by hostname, or a reverse-proxy domain. For those, prefer the prod server (`mcp-inspector --web`) or add the host to `server.allowedHosts`.
|
||||
|
||||
**MCP Apps caveats.** The MCP Apps sandbox runs on a **separate** port — `MCP_SANDBOX_PORT`, defaulting to a fixed **`6275`** (#2008; it was OS-assigned before, which meant it changed every run and so could never be named in a `forwardPorts` / `-p` / tunnel config written ahead of time). For the Apps tab to work off loopback, that sandbox port must be independently reachable from the browser — expose/forward `6275` alongside `6274`, or set `MCP_SANDBOX_PORT` to pick another. If the port is already taken the sandbox falls back to an OS-assigned one and warns, so a second Inspector still gets a working Apps tab locally — but the forwarded port is then wrong, which is what the warning tells you. (Under a `0.0.0.0` wildcard bind the sandbox URL is advertised as `localhost`, which is reachable — a wildcard bind serves loopback — so only the port needs handling.) Also note the sandbox iframe is gated by a `frame-ancestors` CSP, and **a bracketed IPv6 literal is not a valid CSP host-source** — so MCP Apps requires browsing the app at a name or IPv4 (`localhost`, `127.0.0.1`, a hostname, a LAN IPv4), **not** a bare `http://[::1]:…` address. Finally, the sandbox URL is always `http://` — so **behind TLS** (an `https://` app page) the browser blocks the `http://…/sandbox` iframe as mixed content and MCP Apps can't render; the Apps tab needs a plain-`http` app origin today.
|
||||
**MCP Apps caveats.** The MCP Apps sandbox runs on a **separate** port — `MCP_SANDBOX_PORT`, defaulting to a fixed **`6275`** (#2008; it was OS-assigned before, which meant it changed every run and so could never be named in a `forwardPorts` / `-p` / tunnel config written ahead of time). For the Apps tab to work off loopback, that sandbox port must be independently reachable from the browser — expose/forward `6275` alongside `6274`, or set `MCP_SANDBOX_PORT` to pick another. If the port is already taken the sandbox falls back to an OS-assigned one and warns, so a second Inspector still gets a working Apps tab locally — but the forwarded port is then wrong, which is what the warning tells you. (Under a `0.0.0.0` wildcard bind the sandbox URL is advertised as `localhost`, which is reachable — a wildcard bind serves loopback — so only the port needs handling.) Also note the sandbox iframe is gated by a `frame-ancestors` CSP, and **a bracketed IPv6 literal is not a valid CSP host-source** — so MCP Apps requires browsing the app at a name or IPv4 (`localhost`, `127.0.0.1`, a hostname, a LAN IPv4), **not** a bare `http://[::1]:…` address. Finally, the sandbox URL is built from the bind, so it is always `http://<bind host>:<port>/sandbox` — which is wrong **behind a reverse proxy or TLS**, where the browser reaches the listener at a public hostname (and an `https://` app page blocks an `http://` iframe as mixed content).
|
||||
|
||||
**Behind a reverse proxy: `MCP_SANDBOX_FULL_ADDRESS` and `MCP_APP_ORIGIN_FULL_ADDRESS`** (#1862). These set the address the browser is _told_ to use; the process still listens on `MCP_SANDBOX_PORT` / `MCP_APP_ORIGIN_PORT`, and routing the public address to that listener is the proxy's job.
|
||||
|
||||
```sh
|
||||
ALLOWED_ORIGINS=https://inspector.example.com \
|
||||
MCP_SANDBOX_FULL_ADDRESS=https://inspector-sandbox.example.com/sandbox \
|
||||
MCP_APP_ORIGIN_FULL_ADDRESS=https://inspector-apps.example.com \
|
||||
mcp-inspector --web
|
||||
```
|
||||
|
||||
- **Each needs its own origin** — its own hostname, or its own port. The MCP Apps spec requires the sandbox origin to differ from the host's, so the natural single-hostname layout (`https://inspector.example.com/sandbox` routed to `6275`) is **refused** at boot with a warning naming the collision, as is an app origin equal to the Inspector's or the sandbox's. A refused or malformed value (no `http(s)` scheme, credentials, a query, a wildcard, a bracketed IPv6 literal, or a path on the app origin) is warned and ignored, and the bind-derived address is advertised instead.
|
||||
- `MCP_SANDBOX_FULL_ADDRESS` may be a bare origin; `/sandbox` is appended. `MCP_APP_ORIGIN_FULL_ADDRESS` must be an origin only — documents are served at `<origin>/app-document/<id>`.
|
||||
- **`ALLOWED_ORIGINS` must name the public Inspector origin.** It already had to for connects; it is now also the sandbox proxy's embedder allow-list — the proxy checks the embedding page's origin against exactly the list its `frame-ancestors` header admits, rather than a hardcoded `http://localhost` pattern that no `https://` page could match. It **replaces** the defaults, so add the loopback origins too if you still browse locally.
|
||||
- Set **both** if a server you inspect declares `_meta.ui.domain`: with only the sandbox overridden, that app is still served from `http://<bind host>:6278`, which an `https` page blocks and a remote browser cannot reach — and that frame stays blank with no fallback. A plain-`http` address under an `https` `ALLOWED_ORIGINS` entry is advertised but warned about for the same reason.
|
||||
- The web client itself still needs a secure context off loopback (`crypto.randomUUID` is unavailable over plain HTTP on a non-`localhost` host), so serve the Inspector over HTTPS when it is not on `localhost`.
|
||||
|
||||
In every case, exposing the Inspector beyond loopback also means anyone who can reach it can drive its backend — keep authentication on (do **not** set `DANGEROUSLY_OMIT_AUTH`) and prefer a specific bind address over the wildcard.
|
||||
|
||||
|
|
|
|||
|
|
@ -162,6 +162,14 @@ export interface AppOriginControllerOptions {
|
|||
* loopback, exactly as the sandbox proxy's own `frame-ancestors` does.
|
||||
*/
|
||||
embedderOrigins?: string[];
|
||||
/**
|
||||
* The origin to serve published documents under instead of the bind-derived
|
||||
* one — the operator's `MCP_APP_ORIGIN_FULL_ADDRESS`, already validated by
|
||||
* `public-address.ts` (#1862). Only adopted once the listener is up, so a
|
||||
* failed bind still makes {@link AppOriginController.publish} return null
|
||||
* and the renderer keeps its srcdoc fallback.
|
||||
*/
|
||||
publicOrigin?: string;
|
||||
}
|
||||
|
||||
/** A document handed to {@link AppOriginController.publish}. */
|
||||
|
|
@ -237,9 +245,17 @@ export function createAppOriginController(
|
|||
// Same defaulting rationale as the sandbox controller: never the *name*
|
||||
// `localhost`, which resolves to a single address family and would put this
|
||||
// listener on a different family than the web server (#1951).
|
||||
const { port, host = DEFAULT_BIND_HOST, embedderOrigins } = options;
|
||||
const {
|
||||
port,
|
||||
host = DEFAULT_BIND_HOST,
|
||||
embedderOrigins,
|
||||
publicOrigin,
|
||||
} = options;
|
||||
let server: Server | null = null;
|
||||
let origin: string | null = null;
|
||||
// Tracked separately from `origin`: a public origin carries the public port
|
||||
// (or none), not the one this listener bound.
|
||||
let boundPort = 0;
|
||||
|
||||
// Insertion-ordered, so the first key is the oldest entry.
|
||||
const documents = new Map<string, StoredDocument>();
|
||||
|
|
@ -268,7 +284,7 @@ export function createAppOriginController(
|
|||
return {
|
||||
async start(): Promise<{ port: number; url: string }> {
|
||||
if (server && origin) {
|
||||
return { port: parseInt(new URL(origin).port, 10), url: origin };
|
||||
return { port: boundPort, url: origin };
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
let settled = false;
|
||||
|
|
@ -312,6 +328,43 @@ export function createAppOriginController(
|
|||
// dedicated-origin rendering down. Retry once on an OS-assigned port,
|
||||
// loudly — whoever pinned the port to forward it needs to know.
|
||||
let retriedDynamic = false;
|
||||
|
||||
/**
|
||||
* The origin to publish under once bound: the operator's public origin
|
||||
* (#1862) when it is still safe to use, else the bind-derived one.
|
||||
*
|
||||
* Two reasons to refuse it here rather than only at config time:
|
||||
* - A reverse proxy routes the public origin to one stable port, so it
|
||||
* is only usable on a FIXED port: not an explicit `0`, not a
|
||||
* config-time collision resolved to `0`, and not the EADDRINUSE
|
||||
* fallback, where someone else holds the pinned port.
|
||||
* - It must differ from every trusted ancestor — the sandbox proxy and
|
||||
* the Inspector page, i.e. `embedderOrigins`. The frame is granted
|
||||
* `allow-same-origin` on this path, so sharing either origin hands
|
||||
* the app that realm. Config time cannot see a bind-derived sandbox
|
||||
* origin (it is known only once that listener binds, possibly on a
|
||||
* fallback port); the callers pass the real one here.
|
||||
*/
|
||||
const adoptedOrigin = (derived: string): string => {
|
||||
if (!publicOrigin) return derived;
|
||||
if (port === 0 || retriedDynamic) {
|
||||
console.warn(
|
||||
`App origin: not using MCP_APP_ORIGIN_FULL_ADDRESS, since the listener is not on a fixed ` +
|
||||
`port for it to route to; serving app documents from ${derived} instead.`,
|
||||
);
|
||||
return derived;
|
||||
}
|
||||
if (embedderOrigins?.includes(publicOrigin)) {
|
||||
console.warn(
|
||||
`App origin: not using MCP_APP_ORIGIN_FULL_ADDRESS=${publicOrigin}: it is the sandbox's or ` +
|
||||
`the Inspector UI's origin, and a dedicated app origin must differ from both; ` +
|
||||
`serving app documents from ${derived} instead.`,
|
||||
);
|
||||
return derived;
|
||||
}
|
||||
return publicOrigin;
|
||||
};
|
||||
|
||||
server.on("error", (err: NodeJS.ErrnoException) => {
|
||||
if (err.code === "EADDRINUSE") {
|
||||
if (!retriedDynamic && port !== 0) {
|
||||
|
|
@ -364,7 +417,8 @@ export function createAppOriginController(
|
|||
const urlHost = isAllInterfacesHost(canonicalHost)
|
||||
? "localhost"
|
||||
: canonicalHost;
|
||||
origin = `http://${urlHost}:${actualPort}`;
|
||||
origin = adoptedOrigin(`http://${urlHost}:${actualPort}`);
|
||||
boundPort = actualPort;
|
||||
settle({ port: actualPort, url: origin });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
220
clients/web/server/public-address.ts
Normal file
220
clients/web/server/public-address.ts
Normal file
|
|
@ -0,0 +1,220 @@
|
|||
/**
|
||||
* Public addresses for the MCP Apps listeners (#1862): the URL the browser is
|
||||
* told to load the sandbox proxy from, and the origin a `_meta.ui.domain` app
|
||||
* document is served from, when either differs from the address the process
|
||||
* binds.
|
||||
*
|
||||
* ## Why an override exists
|
||||
*
|
||||
* Both listeners advertise a URL built from their own bind — `http://localhost:
|
||||
* 6275/sandbox` under a wildcard bind. That is right whenever the browser shares
|
||||
* the process's view of the network (a desktop, a container published on the
|
||||
* same port numbers, an SSH tunnel), and wrong behind a reverse proxy or an
|
||||
* ingress, where the browser reaches the listener at a public hostname and
|
||||
* frequently over HTTPS. The port knobs (`MCP_SANDBOX_PORT`,
|
||||
* `MCP_APP_ORIGIN_PORT`) cannot express a scheme or a hostname, so:
|
||||
*
|
||||
* - `MCP_SANDBOX_FULL_ADDRESS` — the sandbox proxy URL, e.g.
|
||||
* `https://inspector-sandbox.example.com/sandbox`. A bare origin gets
|
||||
* `/sandbox` appended, since that is the only path the listener serves.
|
||||
* - `MCP_APP_ORIGIN_FULL_ADDRESS` — the app-origin listener's origin, e.g.
|
||||
* `https://inspector-apps.example.com`. An origin only: documents are served
|
||||
* at `<origin>/app-document/<id>`, so a path would name nothing.
|
||||
*
|
||||
* Neither changes what the process listens on; the operator routes the public
|
||||
* address to the listener.
|
||||
*
|
||||
* ## Why the values are validated rather than passed through
|
||||
*
|
||||
* The sandbox origin is an isolation boundary, not merely an address. The MCP
|
||||
* Apps spec requires host ≠ sandbox origin, and today that holds structurally —
|
||||
* the listener is on its own port of the same process, so it cannot be
|
||||
* misconfigured. An operator-supplied address makes it configurable, and the
|
||||
* most natural reverse-proxy layout (`https://inspector.example.com/sandbox`
|
||||
* routed to `:6275`) is exactly the one that collapses it. The proxy page's own
|
||||
* self-test and its #2056 `src` check both fail closed, but their symptom is a
|
||||
* blank frame and a console throw; refusing here names the cause at boot.
|
||||
*
|
||||
* So a value is **refused** — warned and ignored, falling back to the
|
||||
* bind-derived address, matching the warn-and-drop precedent `ALLOWED_ORIGINS`
|
||||
* and `MCP_SANDBOX_PORT` set — when it is not an absolute `http(s)` URL, carries
|
||||
* credentials, a query, a fragment, a wildcard, or a bracketed IPv6 literal (not
|
||||
* a valid CSP host-source, and the sandbox origin is emitted into published
|
||||
* documents' `frame-ancestors`), or when its origin equals an Inspector UI
|
||||
* origin (or, for the app origin, the sandbox's). A plain-`http` address under
|
||||
* an `https` UI is only **warned**: the browser blocks it as mixed content, but
|
||||
* `ALLOWED_ORIGINS` can legitimately list both schemes at once, so it is not
|
||||
* provably wrong.
|
||||
*
|
||||
* Not `DANGEROUSLY_`-prefixed: it widens no bind and disables no check, and the
|
||||
* configurations that would be dangerous are rejected rather than renamed.
|
||||
*/
|
||||
|
||||
export const SANDBOX_FULL_ADDRESS_ENV = "MCP_SANDBOX_FULL_ADDRESS";
|
||||
export const APP_ORIGIN_FULL_ADDRESS_ENV = "MCP_APP_ORIGIN_FULL_ADDRESS";
|
||||
|
||||
/** The only path the sandbox listener serves (see `sandbox-controller.ts`). */
|
||||
const SANDBOX_PATH = "/sandbox";
|
||||
|
||||
/** A parsed address, or the reason it was refused. */
|
||||
type Parsed = { url: URL } | { reason: string };
|
||||
|
||||
/**
|
||||
* Shape checks shared by both addresses. Returns the reason for the first
|
||||
* failure, so the warning names one concrete thing to fix.
|
||||
*/
|
||||
function parsePublicUrl(raw: string): Parsed {
|
||||
let url: URL;
|
||||
try {
|
||||
url = new URL(raw);
|
||||
} catch {
|
||||
return { reason: "it is not an absolute URL" };
|
||||
}
|
||||
if (url.protocol !== "http:" && url.protocol !== "https:") {
|
||||
return { reason: "it needs an http:// or https:// scheme" };
|
||||
}
|
||||
if (url.username || url.password) {
|
||||
return { reason: "it must not carry credentials" };
|
||||
}
|
||||
// `new URL` accepts `*` in a hostname; it is never a reachable address.
|
||||
if (url.hostname.includes("*")) {
|
||||
return { reason: "it must not contain a wildcard" };
|
||||
}
|
||||
if (url.hostname.startsWith("[")) {
|
||||
return {
|
||||
reason:
|
||||
"a bracketed IPv6 literal is not a valid CSP host-source — use a hostname or an IPv4 address",
|
||||
};
|
||||
}
|
||||
if (url.search || url.hash) {
|
||||
return { reason: "it must not carry a query string or fragment" };
|
||||
}
|
||||
return { url };
|
||||
}
|
||||
|
||||
/**
|
||||
* The value as it may appear in a log line. Never the raw string: the values
|
||||
* refused for carrying credentials, a query or a path are exactly the ones
|
||||
* likely to hold a secret, and echoing them would leak what the check refused.
|
||||
* Only scheme and host are shown — that is enough to identify which address was
|
||||
* refused — and an unparseable value is not echoed at all.
|
||||
*/
|
||||
export function describeForLog(raw: string): string {
|
||||
try {
|
||||
const u = new URL(raw);
|
||||
return `"${u.protocol}${u.host ? `//${u.host}` : ""}"`;
|
||||
} catch {
|
||||
return "(unparseable value, not echoed)";
|
||||
}
|
||||
}
|
||||
|
||||
function warnIgnored(env: string, raw: string, reason: string): void {
|
||||
console.warn(
|
||||
`Ignoring ${env}=${describeForLog(raw)}: ${reason}. Advertising the bind-derived address instead.`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Warn when a plain-`http` address will be loaded by an `https` UI. Not a
|
||||
* refusal — see the header comment.
|
||||
*/
|
||||
function warnMixedContent(
|
||||
env: string,
|
||||
url: URL,
|
||||
allowedOrigins: readonly string[],
|
||||
): void {
|
||||
if (
|
||||
url.protocol === "http:" &&
|
||||
allowedOrigins.some((o) => o.startsWith("https:"))
|
||||
) {
|
||||
console.warn(
|
||||
`${env} is plain http (${url.origin}) but ALLOWED_ORIGINS includes an https origin: ` +
|
||||
`a browser on the https UI blocks it as mixed content and MCP Apps will not render there.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The public sandbox proxy URL from {@link SANDBOX_FULL_ADDRESS_ENV}, or
|
||||
* `undefined` when it is unset, blank, or refused (warned).
|
||||
*
|
||||
* @param allowedOrigins The resolved Inspector UI origins — the sandbox must
|
||||
* not share an origin with any of them.
|
||||
*/
|
||||
export function resolveSandboxPublicUrl(
|
||||
raw: string | undefined,
|
||||
allowedOrigins: readonly string[],
|
||||
): string | undefined {
|
||||
const value = raw?.trim();
|
||||
if (!value) return undefined;
|
||||
const parsed = parsePublicUrl(value);
|
||||
if ("reason" in parsed) {
|
||||
warnIgnored(SANDBOX_FULL_ADDRESS_ENV, value, parsed.reason);
|
||||
return undefined;
|
||||
}
|
||||
const { url } = parsed;
|
||||
// A bare origin names the listener, not the page; the page is `/sandbox`.
|
||||
if (url.pathname === "/") url.pathname = SANDBOX_PATH;
|
||||
if (allowedOrigins.includes(url.origin)) {
|
||||
warnIgnored(
|
||||
SANDBOX_FULL_ADDRESS_ENV,
|
||||
value,
|
||||
`its origin ${url.origin} is also an Inspector UI origin (ALLOWED_ORIGINS), and the MCP Apps ` +
|
||||
`sandbox must be on a different origin from its host — route it to its own hostname or port`,
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
warnMixedContent(SANDBOX_FULL_ADDRESS_ENV, url, allowedOrigins);
|
||||
return url.href;
|
||||
}
|
||||
|
||||
/**
|
||||
* The public app-origin from {@link APP_ORIGIN_FULL_ADDRESS_ENV}, or
|
||||
* `undefined` when it is unset, blank, or refused (warned).
|
||||
*
|
||||
* @param allowedOrigins The resolved Inspector UI origins.
|
||||
* @param sandboxUrl The overridden sandbox URL, when there is one. This is the
|
||||
* early, config-time check; a bind-derived sandbox origin is only known once
|
||||
* that listener binds (possibly on a fallback port), so the authoritative
|
||||
* comparison against it is made by the app-origin controller at start, which
|
||||
* receives the real sandbox and Inspector origins as `embedderOrigins`.
|
||||
*/
|
||||
export function resolveAppOriginPublicOrigin(
|
||||
raw: string | undefined,
|
||||
allowedOrigins: readonly string[],
|
||||
sandboxUrl: string | undefined,
|
||||
): string | undefined {
|
||||
const value = raw?.trim();
|
||||
if (!value) return undefined;
|
||||
const parsed = parsePublicUrl(value);
|
||||
if ("reason" in parsed) {
|
||||
warnIgnored(APP_ORIGIN_FULL_ADDRESS_ENV, value, parsed.reason);
|
||||
return undefined;
|
||||
}
|
||||
const { url } = parsed;
|
||||
if (url.pathname !== "/") {
|
||||
warnIgnored(
|
||||
APP_ORIGIN_FULL_ADDRESS_ENV,
|
||||
value,
|
||||
"it must be an origin only, with no path — app documents are served at <origin>/app-document/<id>",
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
// Same-origin with the host or the proxy is precisely what the dedicated
|
||||
// origin must never be: the inner frame is granted `allow-same-origin` on
|
||||
// this path, so sharing either origin hands the app that realm.
|
||||
const trusted = sandboxUrl
|
||||
? [...allowedOrigins, new URL(sandboxUrl).origin]
|
||||
: allowedOrigins;
|
||||
if (trusted.includes(url.origin)) {
|
||||
warnIgnored(
|
||||
APP_ORIGIN_FULL_ADDRESS_ENV,
|
||||
value,
|
||||
`its origin ${url.origin} is also the Inspector UI's or the sandbox's, and a dedicated app ` +
|
||||
`origin must differ from both — route it to its own hostname or port`,
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
warnMixedContent(APP_ORIGIN_FULL_ADDRESS_ENV, url, allowedOrigins);
|
||||
return url.origin;
|
||||
}
|
||||
|
|
@ -29,6 +29,14 @@ export interface SandboxControllerOptions {
|
|||
* list (only a hand-constructed caller or a test) falls back to loopback-only.
|
||||
*/
|
||||
allowedOrigins?: string[];
|
||||
/**
|
||||
* The URL to advertise instead of the bind-derived one — the operator's
|
||||
* `MCP_SANDBOX_FULL_ADDRESS`, already validated by `public-address.ts`
|
||||
* (#1862). It changes what {@link SandboxController.getUrl} returns, never
|
||||
* what is bound; and it is only advertised once the listener is actually up,
|
||||
* so a failed bind still reports no sandbox rather than a dead URL.
|
||||
*/
|
||||
publicUrl?: string;
|
||||
}
|
||||
|
||||
// Loopback fallback. NB: no `http://[::1]:*` — a **bracketed IPv6 literal is not
|
||||
|
|
@ -86,11 +94,59 @@ export function sandboxFrameAncestors(allowedOrigins?: string[]): string {
|
|||
* derive it here rather than each rolling one.
|
||||
*/
|
||||
export function frameAncestorsDirective(origins?: string[]): string {
|
||||
const valid = (origins ?? []).filter((o) => CSP_HOST_SOURCE.test(o));
|
||||
const valid = validHostSources(origins);
|
||||
const sources = valid.length > 0 ? valid : LOOPBACK_FRAME_ANCESTORS;
|
||||
return `frame-ancestors ${sources.join(" ")}`;
|
||||
}
|
||||
|
||||
/** The entries of `origins` that are well-formed CSP host-sources. */
|
||||
function validHostSources(origins?: string[]): string[] {
|
||||
return (origins ?? []).filter((o) => CSP_HOST_SOURCE.test(o));
|
||||
}
|
||||
|
||||
/**
|
||||
* The marker in `static/sandbox_proxy.html` that {@link renderSandboxProxyHtml}
|
||||
* replaces, together with the `null` it stands in front of. Matching the pair
|
||||
* means an unrendered page is still valid JavaScript, and it falls back to the
|
||||
* loopback check rather than to accepting anything.
|
||||
*/
|
||||
export const EMBEDDER_ORIGINS_PLACEHOLDER =
|
||||
"/*INSPECTOR_EMBEDDER_ORIGINS*/ null";
|
||||
|
||||
/**
|
||||
* The JavaScript literal the proxy page's referrer check reads as its exact
|
||||
* embedder allow-list (#1862).
|
||||
*
|
||||
* It is derived from the SAME filtered list as {@link sandboxFrameAncestors},
|
||||
* so the referrer check and the `frame-ancestors` header admit one set. Before
|
||||
* this the page hardcoded `http://localhost|127.0.0.1`, which no `https://`
|
||||
* referrer can match — so the proxy threw on every HTTPS deployment regardless
|
||||
* of `ALLOWED_ORIGINS`. When nothing valid remains the literal is `null`, and
|
||||
* the page falls back to that loopback check, mirroring the header's own
|
||||
* loopback fallback.
|
||||
*
|
||||
* The value lands inside a `<script>`, so `<` is escaped: `CSP_HOST_SOURCE`
|
||||
* does not exclude it, and a `</script>` inside a string literal would close
|
||||
* the element regardless of the quotes around it.
|
||||
*/
|
||||
export function embedderOriginsLiteral(origins?: string[]): string {
|
||||
const valid = validHostSources(origins);
|
||||
if (valid.length === 0) return "null";
|
||||
return JSON.stringify(valid).replace(/</g, "\\u003c");
|
||||
}
|
||||
|
||||
/** Substitute the embedder allow-list into the proxy page's source. */
|
||||
export function renderSandboxProxyHtml(
|
||||
html: string,
|
||||
origins?: string[],
|
||||
): string {
|
||||
// Function-form replacement: a string replacement would interpret `$&` and
|
||||
// friends, and the literal is operator-derived.
|
||||
return html.replace(EMBEDDER_ORIGINS_PLACEHOLDER, () =>
|
||||
embedderOriginsLiteral(origins),
|
||||
);
|
||||
}
|
||||
|
||||
export interface SandboxController {
|
||||
start(): Promise<{ port: number; url: string }>;
|
||||
close(): Promise<void>;
|
||||
|
|
@ -156,9 +212,12 @@ export function createSandboxController(
|
|||
// `localhost` — a name resolves to one address family and would reintroduce
|
||||
// the #1951 split (web on IPv4, sandbox on IPv6) for any future call site
|
||||
// that omits `host`. Both call sites pass `config.sandboxHost` today.
|
||||
const { port, host = DEFAULT_BIND_HOST, allowedOrigins } = options;
|
||||
const { port, host = DEFAULT_BIND_HOST, allowedOrigins, publicUrl } = options;
|
||||
let server: Server | null = null;
|
||||
let sandboxUrl: string | null = null;
|
||||
// Tracked separately from the URL: an advertised public URL carries the
|
||||
// public port (or none at all), not the one this listener bound.
|
||||
let boundPort = 0;
|
||||
|
||||
// Defense-in-depth for the proxy page itself. Only `frame-ancestors` is set
|
||||
// here — fetch directives (`default-src`, `connect-src`, etc.) are
|
||||
|
|
@ -175,7 +234,10 @@ export function createSandboxController(
|
|||
let sandboxHtml: string;
|
||||
try {
|
||||
const sandboxHtmlPath = join(__dirname, "../static/sandbox_proxy.html");
|
||||
sandboxHtml = readFileSync(sandboxHtmlPath, "utf-8");
|
||||
sandboxHtml = renderSandboxProxyHtml(
|
||||
readFileSync(sandboxHtmlPath, "utf-8"),
|
||||
allowedOrigins,
|
||||
);
|
||||
} catch (e) {
|
||||
sandboxHtml =
|
||||
"<!DOCTYPE html><html><body>Sandbox not loaded: " +
|
||||
|
|
@ -186,8 +248,7 @@ export function createSandboxController(
|
|||
return {
|
||||
async start(): Promise<{ port: number; url: string }> {
|
||||
if (server && sandboxUrl) {
|
||||
const p = parseInt(new URL(sandboxUrl).port, 10);
|
||||
return { port: p, url: sandboxUrl };
|
||||
return { port: boundPort, url: sandboxUrl };
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
// Guard so a `listen` error followed by a (theoretically possible)
|
||||
|
|
@ -277,7 +338,24 @@ export function createSandboxController(
|
|||
const urlHost = isAllInterfacesHost(canonicalHost)
|
||||
? "localhost"
|
||||
: canonicalHost;
|
||||
sandboxUrl = `http://${urlHost}:${actualPort}/sandbox`;
|
||||
const derivedUrl = `http://${urlHost}:${actualPort}/sandbox`;
|
||||
// An operator-supplied public URL (#1862) replaces the derived one
|
||||
// only now that the listener it routes to is actually up — and only
|
||||
// when that listener is on a FIXED port. A reverse proxy routes the
|
||||
// public URL to one stable port; an OS-assigned port (an explicit
|
||||
// `0`, a config-time collision resolved to `0`, or the EADDRINUSE
|
||||
// fallback, where someone else holds the pinned port) is not it, so
|
||||
// advertising the public URL would send the browser to the wrong
|
||||
// listener or none. The derived URL at least works locally.
|
||||
const onFixedPort = port !== 0 && !retriedDynamic;
|
||||
if (publicUrl && !onFixedPort) {
|
||||
console.warn(
|
||||
`Sandbox: not advertising MCP_SANDBOX_FULL_ADDRESS, since the sandbox is not on a fixed ` +
|
||||
`port for it to route to; advertising ${derivedUrl} instead.`,
|
||||
);
|
||||
}
|
||||
sandboxUrl = publicUrl && onFixedPort ? publicUrl : derivedUrl;
|
||||
boundPort = actualPort;
|
||||
settle({ port: actualPort, url: sandboxUrl });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ export async function startHonoServer(
|
|||
port: config.sandboxPort,
|
||||
host: config.sandboxHost,
|
||||
allowedOrigins: config.allowedOrigins,
|
||||
publicUrl: config.sandboxPublicUrl,
|
||||
});
|
||||
await sandboxController.start();
|
||||
// The dedicated origin apps declaring `_meta.ui.domain` are served from
|
||||
|
|
@ -54,6 +55,7 @@ export async function startHonoServer(
|
|||
const appOriginController = createAppOriginController({
|
||||
port: config.appOriginPort,
|
||||
host: config.sandboxHost,
|
||||
publicOrigin: config.appOriginPublicOrigin,
|
||||
embedderOrigins: appDocumentEmbedders(
|
||||
sandboxController.getUrl(),
|
||||
config.allowedOrigins,
|
||||
|
|
|
|||
|
|
@ -69,6 +69,7 @@ export function honoMiddlewarePlugin(config: WebServerConfig): Plugin {
|
|||
port: config.sandboxPort,
|
||||
host: config.sandboxHost,
|
||||
allowedOrigins: config.allowedOrigins,
|
||||
publicUrl: config.sandboxPublicUrl,
|
||||
});
|
||||
await sandboxController.start();
|
||||
// The dedicated origin apps declaring `_meta.ui.domain` are served from
|
||||
|
|
@ -77,6 +78,7 @@ export function honoMiddlewarePlugin(config: WebServerConfig): Plugin {
|
|||
const appOriginController = createAppOriginController({
|
||||
port: config.appOriginPort,
|
||||
host: config.sandboxHost,
|
||||
publicOrigin: config.appOriginPublicOrigin,
|
||||
embedderOrigins: appDocumentEmbedders(
|
||||
sandboxController.getUrl(),
|
||||
config.allowedOrigins,
|
||||
|
|
|
|||
|
|
@ -21,6 +21,12 @@ import { readInspectorVersionSafe } from "../../../core/node/version.ts";
|
|||
import { resolveSandboxPort } from "./sandbox-controller.js";
|
||||
import { resolveAppOriginPort } from "./app-origin-controller.js";
|
||||
import { resolveBindHostname } from "./resolve-bind-host.js";
|
||||
import {
|
||||
APP_ORIGIN_FULL_ADDRESS_ENV,
|
||||
resolveAppOriginPublicOrigin,
|
||||
resolveSandboxPublicUrl,
|
||||
SANDBOX_FULL_ADDRESS_ENV,
|
||||
} from "./public-address.js";
|
||||
import {
|
||||
canonicalUrlHost,
|
||||
isAllInterfacesHost,
|
||||
|
|
@ -74,6 +80,17 @@ export interface WebServerConfig {
|
|||
* address the sandbox uses.
|
||||
*/
|
||||
appOriginPort: number;
|
||||
/**
|
||||
* Public sandbox proxy URL advertised in place of the bind-derived one
|
||||
* (`MCP_SANDBOX_FULL_ADDRESS`, #1862), or undefined when unset or refused.
|
||||
* See `public-address.ts` for what is refused and why.
|
||||
*/
|
||||
sandboxPublicUrl?: string;
|
||||
/**
|
||||
* Public origin published app documents are served under
|
||||
* (`MCP_APP_ORIGIN_FULL_ADDRESS`, #1862), or undefined when unset or refused.
|
||||
*/
|
||||
appOriginPublicOrigin?: string;
|
||||
logger: Logger | undefined;
|
||||
/** When true, open browser after server starts. */
|
||||
autoOpen: boolean;
|
||||
|
|
@ -460,6 +477,22 @@ export function buildWebServerConfig(
|
|||
}
|
||||
})
|
||||
.filter((o): o is string => o !== null);
|
||||
const allowedOrigins = configuredOrigins?.length
|
||||
? configuredOrigins
|
||||
: defaultAllowedOrigins(hostname, port);
|
||||
|
||||
// Resolved after the allow-list because both are checked against it: neither
|
||||
// listener may share an origin with the Inspector page (#1862). The sandbox
|
||||
// goes first so the app origin can be checked against it too.
|
||||
const sandboxPublicUrl = resolveSandboxPublicUrl(
|
||||
process.env[SANDBOX_FULL_ADDRESS_ENV],
|
||||
allowedOrigins,
|
||||
);
|
||||
const appOriginPublicOrigin = resolveAppOriginPublicOrigin(
|
||||
process.env[APP_ORIGIN_FULL_ADDRESS_ENV],
|
||||
allowedOrigins,
|
||||
sandboxPublicUrl,
|
||||
);
|
||||
|
||||
return {
|
||||
port,
|
||||
|
|
@ -471,12 +504,12 @@ export function buildWebServerConfig(
|
|||
writable,
|
||||
initialServers,
|
||||
storageDir: process.env.MCP_STORAGE_DIR,
|
||||
allowedOrigins: configuredOrigins?.length
|
||||
? configuredOrigins
|
||||
: defaultAllowedOrigins(hostname, port),
|
||||
allowedOrigins,
|
||||
sandboxPort,
|
||||
sandboxHost: hostname,
|
||||
appOriginPort,
|
||||
sandboxPublicUrl,
|
||||
appOriginPublicOrigin,
|
||||
logger,
|
||||
autoOpen: resolveAutoOpen(),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -23,6 +23,18 @@ import {
|
|||
} from "../../../../server/app-origin-controller.js";
|
||||
import { RUNNER_OAUTH_CALLBACK_DEFAULT_PORT } from "@inspector/core/auth/node/runner-oauth-callback.js";
|
||||
|
||||
/** A port that was free a moment ago, for tests that need a FIXED port. */
|
||||
async function freePort(): Promise<number> {
|
||||
const probe: Server = createServer();
|
||||
await new Promise<void>((resolve) =>
|
||||
probe.listen(0, "127.0.0.1", () => resolve()),
|
||||
);
|
||||
const addr = probe.address();
|
||||
const port = typeof addr === "object" && addr !== null ? addr.port : 0;
|
||||
await new Promise<void>((resolve) => probe.close(() => resolve()));
|
||||
return port;
|
||||
}
|
||||
|
||||
describe("resolveAppOriginPort", () => {
|
||||
const saved = process.env.MCP_APP_ORIGIN_PORT;
|
||||
afterEach(() => {
|
||||
|
|
@ -178,6 +190,73 @@ describe("createAppOriginController", () => {
|
|||
await expect(res.text()).resolves.toBe("<!doctype html><p>app</p>");
|
||||
});
|
||||
|
||||
it("publishes under a public origin while still serving the bound listener (#1862)", async () => {
|
||||
const publicOrigin = "https://apps.example.com";
|
||||
// A fixed port: a public origin is only used where a proxy can route.
|
||||
const port = await freePort();
|
||||
controller = createAppOriginController({
|
||||
port,
|
||||
host: "127.0.0.1",
|
||||
publicOrigin,
|
||||
});
|
||||
const first = await controller.start();
|
||||
expect(first.port).toBe(port);
|
||||
expect(first.url).toBe(publicOrigin);
|
||||
expect(controller.getOrigin()).toBe(publicOrigin);
|
||||
// The cached start reports the BOUND port; parsing one out of a public
|
||||
// origin with no explicit port would yield NaN.
|
||||
expect(await controller.start()).toEqual(first);
|
||||
|
||||
const published = controller.publish({ html: "<p>proxied</p>" })!;
|
||||
expect(published.url.startsWith(`${publicOrigin}/app-document/`)).toBe(
|
||||
true,
|
||||
);
|
||||
// The reverse proxy's job is routing the public path to this listener;
|
||||
// simulate it by fetching the same path on the bound port.
|
||||
const path = new URL(published.url).pathname;
|
||||
const res = await fetch(`http://127.0.0.1:${first.port}${path}`);
|
||||
expect(res.status).toBe(200);
|
||||
await expect(res.text()).resolves.toBe("<p>proxied</p>");
|
||||
});
|
||||
|
||||
it("does not use a public origin on an OS-assigned port (port 0)", async () => {
|
||||
// Both an explicit MCP_APP_ORIGIN_PORT=0 and a config-time collision with
|
||||
// CLIENT_PORT / MCP_SANDBOX_PORT (which buildWebServerConfig resolves to 0)
|
||||
// land here: there is no stable port for a reverse proxy to route to.
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
try {
|
||||
controller = createAppOriginController({
|
||||
port: 0,
|
||||
host: "127.0.0.1",
|
||||
publicOrigin: "https://apps.example.com",
|
||||
});
|
||||
const { url, port } = await controller.start();
|
||||
expect(url).toBe(`http://127.0.0.1:${port}`);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("not using MCP_APP_ORIGIN_FULL_ADDRESS"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not adopt a public origin when the listener never bound", async () => {
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
try {
|
||||
controller = createAppOriginController({
|
||||
port: 0,
|
||||
host: "203.0.113.1", // TEST-NET-3: not assigned to any local interface
|
||||
publicOrigin: "https://apps.example.com",
|
||||
});
|
||||
expect(await controller.start()).toEqual({ port: 0, url: "" });
|
||||
expect(controller.getOrigin()).toBeNull();
|
||||
// So the renderer takes its srcdoc fallback instead of a dead URL.
|
||||
expect(controller.publish({ html: "<p>x</p>" })).toBeNull();
|
||||
} finally {
|
||||
errorSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("serves only frame-ancestors when the document carries no csp", async () => {
|
||||
controller = createAppOriginController({ port: 0, host: "127.0.0.1" });
|
||||
await controller.start();
|
||||
|
|
@ -350,6 +429,54 @@ describe("createAppOriginController", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("does not use a public origin after falling back off its pinned port (#1862)", async () => {
|
||||
const { port, release } = await claimPort();
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
try {
|
||||
controller = createAppOriginController({
|
||||
port,
|
||||
host: "127.0.0.1",
|
||||
publicOrigin: "https://apps.example.com",
|
||||
});
|
||||
const result = await controller.start();
|
||||
expect(result.port).not.toBe(port);
|
||||
expect(result.url).toBe(`http://127.0.0.1:${result.port}`);
|
||||
expect(
|
||||
controller.publish({ html: "<p>x</p>" })!.url.startsWith(result.url),
|
||||
).toBe(true);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("not using MCP_APP_ORIGIN_FULL_ADDRESS"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
await release();
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a public origin equal to a trusted ancestor, including a bind-derived sandbox (#1862)", async () => {
|
||||
// Config time cannot see the bind-derived sandbox origin; the callers pass
|
||||
// it in embedderOrigins, so the controller is where the collision is caught.
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
try {
|
||||
controller = createAppOriginController({
|
||||
port: await freePort(),
|
||||
host: "127.0.0.1",
|
||||
publicOrigin: "http://127.0.0.1:6275",
|
||||
embedderOrigins: appDocumentEmbedders("http://127.0.0.1:6275/sandbox", [
|
||||
"http://localhost:6274",
|
||||
]),
|
||||
});
|
||||
const result = await controller.start();
|
||||
expect(result.url).toBe(`http://127.0.0.1:${result.port}`);
|
||||
expect(controller.getOrigin()).not.toBe("http://127.0.0.1:6275");
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("must differ from both"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("degrades to publish()===null when listen fails for a non-EADDRINUSE reason", async () => {
|
||||
// The backends await start() during boot; if it ever stopped resolving the
|
||||
// whole web server would hang. Instead it resolves empty and every publish
|
||||
|
|
|
|||
196
clients/web/src/test/integration/server/public-address.test.ts
Normal file
196
clients/web/src/test/integration/server/public-address.test.ts
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
/**
|
||||
* Public addresses for the MCP Apps listeners (#1862): `MCP_SANDBOX_FULL_ADDRESS`
|
||||
* and `MCP_APP_ORIGIN_FULL_ADDRESS`, as parsed by `server/public-address.ts`.
|
||||
*
|
||||
* Pinned here: what each accepts and how it is normalized (`/sandbox` appended
|
||||
* to a bare origin, the app origin kept origin-only), every refusal reason, the
|
||||
* same-origin refusals that keep the sandbox and a dedicated app origin off the
|
||||
* Inspector's own origin, the mixed-content warning, and that a refused value
|
||||
* never leaks a secret (credentials, query, fragment, path) into the log. The
|
||||
* listener-side rules — fixed port only, no trusted-ancestor collision — are in
|
||||
* the sandbox and app-origin controller suites, where the bound port is known.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import {
|
||||
resolveAppOriginPublicOrigin,
|
||||
resolveSandboxPublicUrl,
|
||||
} from "../../../../server/public-address.js";
|
||||
|
||||
const UI = ["http://localhost:6274", "http://127.0.0.1:6274"];
|
||||
|
||||
describe("public addresses (#1862)", () => {
|
||||
let warnSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
/** The single warning line, or undefined when nothing was warned. */
|
||||
function warned(): string | undefined {
|
||||
return (
|
||||
warnSpy.mock.calls.map((c: unknown[]) => String(c[0])).join("\n") ||
|
||||
undefined
|
||||
);
|
||||
}
|
||||
|
||||
describe("resolveSandboxPublicUrl", () => {
|
||||
it.each([[undefined], [""], [" "]])(
|
||||
"is unset, silently, for %j",
|
||||
(raw) => {
|
||||
expect(resolveSandboxPublicUrl(raw, UI)).toBeUndefined();
|
||||
expect(warned()).toBeUndefined();
|
||||
},
|
||||
);
|
||||
|
||||
it("advertises a full URL as given", () => {
|
||||
expect(
|
||||
resolveSandboxPublicUrl(
|
||||
"https://inspector-sandbox.example.com/sandbox",
|
||||
["https://inspector.example.com"],
|
||||
),
|
||||
).toBe("https://inspector-sandbox.example.com/sandbox");
|
||||
expect(warned()).toBeUndefined();
|
||||
});
|
||||
|
||||
it("appends /sandbox to a bare origin, the only path the listener serves", () => {
|
||||
expect(
|
||||
resolveSandboxPublicUrl(" https://sb.example.com ", [
|
||||
"https://inspector.example.com",
|
||||
]),
|
||||
).toBe("https://sb.example.com/sandbox");
|
||||
});
|
||||
|
||||
it("keeps a non-root path, which a reverse proxy may rewrite", () => {
|
||||
expect(
|
||||
resolveSandboxPublicUrl("https://sb.example.com/mcp/sandbox", UI),
|
||||
).toBe("https://sb.example.com/mcp/sandbox");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["not a url", "not an absolute URL"],
|
||||
["ftp://sb.example.com/sandbox", "http:// or https://"],
|
||||
["https://user:pw@sb.example.com/sandbox", "credentials"],
|
||||
["https://*.example.com/sandbox", "wildcard"],
|
||||
["http://[::1]:6275/sandbox", "IPv6"],
|
||||
["https://sb.example.com/sandbox?x=1", "query string or fragment"],
|
||||
["https://sb.example.com/sandbox#frag", "query string or fragment"],
|
||||
])("refuses %s", (raw, reason) => {
|
||||
expect(resolveSandboxPublicUrl(raw, UI)).toBeUndefined();
|
||||
expect(warned()).toContain("MCP_SANDBOX_FULL_ADDRESS");
|
||||
expect(warned()).toContain(reason);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["https://user:hunter2@sb.example.com/sandbox", "hunter2"],
|
||||
["https://sb.example.com/sandbox?token=hunter2", "hunter2"],
|
||||
["https://sb.example.com/sandbox#hunter2", "hunter2"],
|
||||
["not a url hunter2", "hunter2"],
|
||||
])("never echoes a secret from a refused value (%s)", (raw, secret) => {
|
||||
expect(resolveSandboxPublicUrl(raw, UI)).toBeUndefined();
|
||||
expect(warned()).toContain("MCP_SANDBOX_FULL_ADDRESS");
|
||||
expect(warned()).not.toContain(secret);
|
||||
});
|
||||
|
||||
it("never echoes a path, which can carry a secret too", () => {
|
||||
expect(
|
||||
resolveAppOriginPublicOrigin(
|
||||
"https://apps.example.com/hunter2",
|
||||
["https://inspector.example.com"],
|
||||
undefined,
|
||||
),
|
||||
).toBeUndefined();
|
||||
expect(warned()).toContain('"https://apps.example.com"');
|
||||
expect(warned()).not.toContain("hunter2");
|
||||
});
|
||||
|
||||
it("refuses an origin shared with the Inspector UI — the same-origin collapse", () => {
|
||||
// The natural reverse-proxy layout: one hostname, /sandbox routed to 6275.
|
||||
// The spec requires host != sandbox origin, so this must not be advertised.
|
||||
expect(
|
||||
resolveSandboxPublicUrl("https://inspector.example.com/sandbox", [
|
||||
"https://inspector.example.com",
|
||||
]),
|
||||
).toBeUndefined();
|
||||
expect(warned()).toContain("different origin from its host");
|
||||
});
|
||||
|
||||
it("advertises, but warns about, plain http under an https UI", () => {
|
||||
expect(
|
||||
resolveSandboxPublicUrl("http://sb.example.com/sandbox", [
|
||||
"https://inspector.example.com",
|
||||
]),
|
||||
).toBe("http://sb.example.com/sandbox");
|
||||
expect(warned()).toContain("mixed content");
|
||||
});
|
||||
|
||||
it("does not warn about http when every UI origin is http", () => {
|
||||
resolveSandboxPublicUrl("http://sb.example.com/sandbox", UI);
|
||||
expect(warned()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveAppOriginPublicOrigin", () => {
|
||||
const SANDBOX = "https://sb.example.com/sandbox";
|
||||
const HOST = ["https://inspector.example.com"];
|
||||
|
||||
it("is unset, silently, for a blank value", () => {
|
||||
expect(resolveAppOriginPublicOrigin(" ", HOST, SANDBOX)).toBeUndefined();
|
||||
expect(warned()).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns the canonical origin", () => {
|
||||
expect(
|
||||
resolveAppOriginPublicOrigin(
|
||||
"https://Apps.Example.com/",
|
||||
HOST,
|
||||
SANDBOX,
|
||||
),
|
||||
).toBe("https://apps.example.com");
|
||||
expect(warned()).toBeUndefined();
|
||||
});
|
||||
|
||||
it("refuses a path, since documents are served at the origin root", () => {
|
||||
expect(
|
||||
resolveAppOriginPublicOrigin(
|
||||
"https://apps.example.com/x",
|
||||
HOST,
|
||||
SANDBOX,
|
||||
),
|
||||
).toBeUndefined();
|
||||
expect(warned()).toContain("origin only");
|
||||
});
|
||||
|
||||
it("refuses a malformed value", () => {
|
||||
expect(
|
||||
resolveAppOriginPublicOrigin("javascript:alert(1)", HOST, SANDBOX),
|
||||
).toBeUndefined();
|
||||
expect(warned()).toContain("MCP_APP_ORIGIN_FULL_ADDRESS");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["the Inspector UI", "https://inspector.example.com"],
|
||||
["the sandbox proxy", "https://sb.example.com"],
|
||||
])("refuses an origin shared with %s", (_label, raw) => {
|
||||
expect(resolveAppOriginPublicOrigin(raw, HOST, SANDBOX)).toBeUndefined();
|
||||
expect(warned()).toContain("must differ from both");
|
||||
});
|
||||
|
||||
it("checks only the UI origins when the sandbox URL is bind-derived", () => {
|
||||
expect(
|
||||
resolveAppOriginPublicOrigin("https://sb.example.com", HOST, undefined),
|
||||
).toBe("https://sb.example.com");
|
||||
});
|
||||
|
||||
it("warns about plain http under an https UI", () => {
|
||||
expect(
|
||||
resolveAppOriginPublicOrigin("http://apps.example.com", HOST, SANDBOX),
|
||||
).toBe("http://apps.example.com");
|
||||
expect(warned()).toContain("mixed content");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,13 +1,84 @@
|
|||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
import { createServer, type Server } from "node:http";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { dirname, join } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import {
|
||||
createSandboxController,
|
||||
DEFAULT_SANDBOX_PORT,
|
||||
EMBEDDER_ORIGINS_PLACEHOLDER,
|
||||
embedderOriginsLiteral,
|
||||
renderSandboxProxyHtml,
|
||||
resolveSandboxPort,
|
||||
sandboxFrameAncestors,
|
||||
} from "../../../../server/sandbox-controller.js";
|
||||
|
||||
/** A port that was free a moment ago, for tests that need a FIXED port. */
|
||||
async function freePort(): Promise<number> {
|
||||
const probe: Server = createServer();
|
||||
await new Promise<void>((resolve) =>
|
||||
probe.listen(0, "127.0.0.1", () => resolve()),
|
||||
);
|
||||
const addr = probe.address();
|
||||
const port = typeof addr === "object" && addr !== null ? addr.port : 0;
|
||||
await new Promise<void>((resolve) => probe.close(() => resolve()));
|
||||
return port;
|
||||
}
|
||||
|
||||
const PROXY_PAGE = join(
|
||||
dirname(fileURLToPath(import.meta.url)),
|
||||
"../../../../static/sandbox_proxy.html",
|
||||
);
|
||||
|
||||
describe("embedder allow-list injection (#1862)", () => {
|
||||
it("emits the same filtered list frame-ancestors admits", () => {
|
||||
const origins = [
|
||||
"https://inspector.example.com",
|
||||
"http://[::1]:6274", // dropped by both
|
||||
"http://a:1; sandbox", // dropped by both
|
||||
];
|
||||
expect(embedderOriginsLiteral(origins)).toBe(
|
||||
'["https://inspector.example.com"]',
|
||||
);
|
||||
expect(sandboxFrameAncestors(origins)).toBe(
|
||||
"frame-ancestors https://inspector.example.com",
|
||||
);
|
||||
});
|
||||
|
||||
it.each([[undefined], [[]], [["http://[::1]:6274"]]])(
|
||||
"is null — the page's loopback fallback — when nothing valid remains (%j)",
|
||||
(origins) => {
|
||||
expect(embedderOriginsLiteral(origins as string[] | undefined)).toBe(
|
||||
"null",
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("escapes < so a value cannot close the <script> element", () => {
|
||||
const literal = embedderOriginsLiteral(["http://x</script><b>:1"]);
|
||||
expect(literal).not.toContain("<");
|
||||
// Still the same value once parsed back as JavaScript/JSON.
|
||||
expect(JSON.parse(literal)).toEqual(["http://x</script><b>:1"]);
|
||||
});
|
||||
|
||||
it("does not interpret replacement patterns in the literal", () => {
|
||||
expect(
|
||||
renderSandboxProxyHtml(`a ${EMBEDDER_ORIGINS_PLACEHOLDER} b`, [
|
||||
"http://$&.example:1",
|
||||
]),
|
||||
).toBe('a ["http://$&.example:1"] b');
|
||||
});
|
||||
|
||||
it("finds exactly one placeholder in the shipped proxy page", () => {
|
||||
// If the page's marker drifts from the constant, the substitution silently
|
||||
// no-ops and every non-loopback deployment is refused again.
|
||||
const page = readFileSync(PROXY_PAGE, "utf-8");
|
||||
expect(page.split(EMBEDDER_ORIGINS_PLACEHOLDER)).toHaveLength(2);
|
||||
expect(page).not.toContain("ALLOWED_REFERRER_PATTERN");
|
||||
});
|
||||
});
|
||||
|
||||
describe("sandboxFrameAncestors", () => {
|
||||
it("derives the directive from the provided allow-list", () => {
|
||||
expect(
|
||||
|
|
@ -198,6 +269,85 @@ describe("createSandboxController", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("serves the proxy page with the allow-list injected", async () => {
|
||||
const controller = createSandboxController({
|
||||
port: 0,
|
||||
allowedOrigins: ["https://inspector.example.com"],
|
||||
});
|
||||
try {
|
||||
const { url } = await controller.start();
|
||||
const body = await (await fetch(url)).text();
|
||||
expect(body).toContain(
|
||||
'const ALLOWED_EMBEDDER_ORIGINS = ["https://inspector.example.com"];',
|
||||
);
|
||||
expect(body).not.toContain(EMBEDDER_ORIGINS_PLACEHOLDER);
|
||||
} finally {
|
||||
await controller.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("advertises a public URL in place of the bind-derived one (#1862)", async () => {
|
||||
const publicUrl = "https://sb.example.com/sandbox";
|
||||
// A fixed port: a public URL is only advertised where a proxy can route.
|
||||
const port = await freePort();
|
||||
const controller = createSandboxController({
|
||||
port,
|
||||
host: "127.0.0.1",
|
||||
publicUrl,
|
||||
});
|
||||
try {
|
||||
const first = await controller.start();
|
||||
expect(first.url).toBe(publicUrl);
|
||||
expect(controller.getUrl()).toBe(publicUrl);
|
||||
// The port is still the bound one — the public URL has none of its own.
|
||||
expect(first.port).toBe(port);
|
||||
expect(
|
||||
(await fetch(`http://127.0.0.1:${first.port}/sandbox`)).status,
|
||||
).toBe(200);
|
||||
// The cached second start must report the bound port, not parse one out
|
||||
// of the public URL (which would be NaN here).
|
||||
expect(await controller.start()).toEqual(first);
|
||||
} finally {
|
||||
await controller.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not advertise a public URL on an OS-assigned port (port 0)", async () => {
|
||||
// MCP_SANDBOX_PORT=0 leaves no stable port for a reverse proxy to route to.
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const controller = createSandboxController({
|
||||
port: 0,
|
||||
host: "127.0.0.1",
|
||||
publicUrl: "https://sb.example.com/sandbox",
|
||||
});
|
||||
try {
|
||||
const { url, port } = await controller.start();
|
||||
expect(url).toBe(`http://127.0.0.1:${port}/sandbox`);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("not advertising MCP_SANDBOX_FULL_ADDRESS"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
await controller.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not advertise a public URL when the listener never bound", async () => {
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
const controller = createSandboxController({
|
||||
port: 0,
|
||||
host: "203.0.113.1", // TEST-NET-3: not assigned to any local interface
|
||||
publicUrl: "https://sb.example.com/sandbox",
|
||||
});
|
||||
try {
|
||||
expect(await controller.start()).toEqual({ port: 0, url: "" });
|
||||
expect(controller.getUrl()).toBeNull();
|
||||
} finally {
|
||||
errorSpy.mockRestore();
|
||||
await controller.close();
|
||||
}
|
||||
});
|
||||
|
||||
it("advertises localhost in the sandbox URL for a wildcard bind", async () => {
|
||||
// 0.0.0.0 isn't reachable from the browser, but a wildcard bind serves
|
||||
// loopback — so the URL handed to the client (and printed in the banner)
|
||||
|
|
@ -379,6 +529,31 @@ describe("createSandboxController", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("does not advertise a public URL after falling back off its pinned port (#1862)", async () => {
|
||||
// The reverse proxy routes the public URL to the pinned port, which another
|
||||
// process holds — so the public URL would reach the wrong listener.
|
||||
const { port, release } = await claimPort();
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
const controller = createSandboxController({
|
||||
port,
|
||||
host: "127.0.0.1",
|
||||
publicUrl: "https://sb.example.com/sandbox",
|
||||
});
|
||||
try {
|
||||
const result = await controller.start();
|
||||
expect(result.port).not.toBe(port);
|
||||
expect(result.url).toBe(`http://127.0.0.1:${result.port}/sandbox`);
|
||||
expect(controller.getUrl()).toBe(result.url);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("not advertising MCP_SANDBOX_FULL_ADDRESS"),
|
||||
);
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
await controller.close();
|
||||
await release();
|
||||
}
|
||||
});
|
||||
|
||||
it("resolves with empty values when even the dynamic retry fails", async () => {
|
||||
// The Vite plugin awaits start() in configureServer; if start() ever stops
|
||||
// resolving, the entire dev backend hangs. This pins the resolve-on-error
|
||||
|
|
|
|||
|
|
@ -27,6 +27,8 @@ const MUTATED_ENV_KEYS = [
|
|||
"ALLOWED_ORIGINS",
|
||||
"MCP_SANDBOX_PORT",
|
||||
"MCP_APP_ORIGIN_PORT",
|
||||
"MCP_SANDBOX_FULL_ADDRESS",
|
||||
"MCP_APP_ORIGIN_FULL_ADDRESS",
|
||||
"SERVER_PORT",
|
||||
"MCP_LOG_FILE",
|
||||
"MCP_AUTO_OPEN_ENABLED",
|
||||
|
|
@ -342,6 +344,40 @@ describe("buildWebServerConfigFromEnv", () => {
|
|||
},
|
||||
);
|
||||
|
||||
it("leaves the public addresses unset by default (#1862)", () => {
|
||||
const cfg = buildWebServerConfigFromEnv();
|
||||
expect(cfg.sandboxPublicUrl).toBeUndefined();
|
||||
expect(cfg.appOriginPublicOrigin).toBeUndefined();
|
||||
});
|
||||
|
||||
it("resolves both public addresses against the resolved allow-list", () => {
|
||||
process.env.ALLOWED_ORIGINS = "https://inspector.example.com";
|
||||
process.env.MCP_SANDBOX_FULL_ADDRESS = "https://sb.example.com";
|
||||
process.env.MCP_APP_ORIGIN_FULL_ADDRESS = "https://apps.example.com";
|
||||
const cfg = buildWebServerConfigFromEnv();
|
||||
expect(cfg.sandboxPublicUrl).toBe("https://sb.example.com/sandbox");
|
||||
expect(cfg.appOriginPublicOrigin).toBe("https://apps.example.com");
|
||||
});
|
||||
|
||||
it("refuses public addresses that collapse onto the Inspector or sandbox origin", () => {
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
try {
|
||||
// The DEFAULT allow-list is what the sandbox is checked against when
|
||||
// ALLOWED_ORIGINS is unset — so the loopback UI origin is refused too.
|
||||
process.env.MCP_SANDBOX_FULL_ADDRESS = "http://localhost:6274/sandbox";
|
||||
expect(buildWebServerConfigFromEnv().sandboxPublicUrl).toBeUndefined();
|
||||
|
||||
// The app origin is checked against the overridden sandbox's origin.
|
||||
process.env.MCP_SANDBOX_FULL_ADDRESS = "https://sb.example.com/sandbox";
|
||||
process.env.MCP_APP_ORIGIN_FULL_ADDRESS = "https://sb.example.com";
|
||||
const cfg = buildWebServerConfigFromEnv();
|
||||
expect(cfg.sandboxPublicUrl).toBe("https://sb.example.com/sandbox");
|
||||
expect(cfg.appOriginPublicOrigin).toBeUndefined();
|
||||
} finally {
|
||||
warnSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("leaves an explicit dynamic app-origin port alone", () => {
|
||||
// 0 means "OS-assigned" for both, so two zeros are not a collision.
|
||||
process.env.CLIENT_PORT = "6274";
|
||||
|
|
|
|||
|
|
@ -76,8 +76,19 @@
|
|||
return allowList.join("; ");
|
||||
}
|
||||
|
||||
const ALLOWED_REFERRER_PATTERN =
|
||||
/^http:\/\/(localhost|127\.0\.0\.1)(:|\/|$)/;
|
||||
// The exact embedder origins this proxy accepts, injected by the sandbox
|
||||
// controller when it serves this page (see `embedderOriginsLiteral` in
|
||||
// server/sandbox-controller.ts). It is the same list the served
|
||||
// `frame-ancestors` header admits, so the two checks agree by
|
||||
// construction — an https or public-hostname Inspector page is accepted
|
||||
// exactly when the backend's ALLOWED_ORIGINS names it (#1862).
|
||||
//
|
||||
// `null` means the controller had no valid list (a hand-constructed
|
||||
// caller or a test); the header then falls back to the loopback family,
|
||||
// and so does this check.
|
||||
const ALLOWED_EMBEDDER_ORIGINS = /*INSPECTOR_EMBEDDER_ORIGINS*/ null;
|
||||
const LOOPBACK_EMBEDDER_PATTERN =
|
||||
/^http:\/\/(localhost|127\.0\.0\.1)(:\d+)?$/;
|
||||
|
||||
if (window.self === window.top) {
|
||||
throw new Error("This file is only to be used in an iframe sandbox.");
|
||||
|
|
@ -87,9 +98,24 @@
|
|||
throw new Error("No referrer, cannot validate embedding site.");
|
||||
}
|
||||
|
||||
if (!document.referrer.match(ALLOWED_REFERRER_PATTERN)) {
|
||||
// Compare the referrer's parsed ORIGIN, never the raw string: a pattern
|
||||
// over the full referrer is a prefix/substring match an attacker page's
|
||||
// path or query can be shaped to satisfy.
|
||||
let referrerOrigin = null;
|
||||
try {
|
||||
referrerOrigin = new URL(document.referrer).origin;
|
||||
} catch (e) {
|
||||
referrerOrigin = null;
|
||||
}
|
||||
|
||||
if (
|
||||
!referrerOrigin ||
|
||||
!(Array.isArray(ALLOWED_EMBEDDER_ORIGINS)
|
||||
? ALLOWED_EMBEDDER_ORIGINS.includes(referrerOrigin)
|
||||
: LOOPBACK_EMBEDDER_PATTERN.test(referrerOrigin))
|
||||
) {
|
||||
throw new Error(
|
||||
`Embedding domain not allowed in referrer ${document.referrer}. (Consider updating the validation logic to allow your domain.)`,
|
||||
`Embedding origin ${referrerOrigin} is not allowed to load the MCP Apps sandbox. Add it to the Inspector's ALLOWED_ORIGINS (e.g. ALLOWED_ORIGINS=https://inspector.example.com) and restart.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue