fix(security): validate URL scheme before shell.openExternal (MCP-app sandbox escape)

[zachdotai]

Problem

Untrusted MCP-app HTML could reach shell.openExternal with an arbitrary URL scheme, bypassing the http/https/mailto allowlist we enforce everywhere else (isSafeExternalUrl, McpAppsService.openLink). This let a malicious/compromised MCP server hand the OS any scheme it has a handler for, from inside the "sandbox": smb:// (NTLM hash theft on Windows), ms-msdt:-class handlers, or a registered app deep-link such as zoommtg://… — which macOS silently hands to the Zoom client, auto-joining an attacker-chosen meeting with zero further clicks.

Root cause. The native window-open and navigation handlers in apps/code/src/main/window.ts forwarded the URL straight to the OS with no validation:

window.webContents.setWindowOpenHandler(({ url }) => {
  shell.openExternal(url);        // no scheme check
  return { action: "deny" };
});
window.webContents.on("will-navigate", (event, url) => {
  if (!url.startsWith(appUrl)) { event.preventDefault(); shell.openExternal(url); } // same
});

How the escape works. MCP apps render in a double-iframe sandbox, but untrusted app JS can still reach the unvalidated handler:

1. McpAppHost renders the outer/proxy iframe from mcp-sandbox://proxy with sandbox="… allow-popups allow-popups-to-escape-sandbox".
2. The proxy HTML (packages/shared/src/mcp-sandbox-proxy.ts) creates the inner iframe with allow-same-origin and writes the untrusted app HTML into it via document.write (not srcdoc, to preserve the origin for WebGL/canvas). Because of allow-same-origin + document.write, the inner frame shares the proxy's origin.
3. The inner frame's own sandbox has no allow-popups, so window.open() from the app's own realm is blocked. But since the app is same-origin with its parent, it injects a <script> into window.parent.document. That script runs in the proxy realm, whose sandbox does carry allow-popups, so window.open(<any scheme>) there is permitted.
4. window.open fires setWindowOpenHandler in the main process, which called shell.openExternal(url) unconditionally → the OS dispatches the scheme handler.

The PoC does this andalso fires once on render:

// Inner (untrusted) frame is same-origin with the proxy frame, which carries
// allow-popups. Run window.open in the PROXY realm by injecting a <script>.
function openInProxyRealm(url) {
  var p = window.parent;
  var scr = p.document.createElement("script");
  scr.textContent = "try{window.open(" + JSON.stringify(url) + ",'_blank');}catch(e){}";
  p.document.documentElement.appendChild(scr);
  scr.remove();
}
// zoommtg:// is registered to the Zoom client and is NOT on isSafeExternalUrl's
// allowlist. Pre-fix it reached shell.openExternal and macOS auto-joined the meeting.
openInProxyRealm("zoommtg://zoom.us/join?action=join&confno=…");

Changes

1. Primary (closes the sink): extracted the handlers into apps/code/src/main/external-links.ts and validate every URL with isSafeExternalUrl (packages/shared/src/url.ts — http/https/mailto only) in both setWindowOpenHandler and will-navigate before calling shell.openExternal. Anything else is logged (with the rejected scheme) and dropped.
2. Defense-in-depth (closes the popup path): removed allow-popups and allow-popups-to-escape-sandbox from the MCP-apps outer iframe (packages/ui/src/features/mcp-apps/components/McpAppHost.tsx). The inner iframe never had allow-popups, so legitimate apps could not open popups anyway; apps open links through the ui/open-link bridge, which McpAppsService.openLink already scheme-validates.
3. Tightened the in-app navigation check (review): will-navigate now parses the target URL and compares it against the exact renderer origin (dev server) or the packaged renderer directory (file://), instead of a startsWith prefix. This stops lookalikes such as http://localhost:5173@evil.example/ or http://localhost:51730/ from being treated as internal and skipping the scheme check.
4. Catch and log shell.openExternal rejections (review): it rejects when the OS has no handler for a scheme, so the promise is now caught and logged rather than left to surface as an unhandled rejection.
5. biome.jsonc: added external-links.ts to the allowlist of host files permitted to import electron.

How did you test this?

• Unit (apps/code/src/main/external-links.test.ts, 22 cases): both handlers open https://…/http://…/mailto:… and do not open smb://, ms-msdt://, custom-scheme://, javascript:, or a non-URL; in-app navigation is allowed only for the exact dev origin or a file under the packaged renderer directory, while lookalike origins (…@evil.example, longer port, scheme swap) and out-of-directory file:// paths are punted to the external-link path; an openExternal rejection is swallowed rather than left unhandled.
• Ran pnpm --filter @posthog/code typecheck, pnpm --filter @posthog/ui typecheck, the full apps/code vitest suite (308 passing), Biome on the touched files, and node scripts/check-host-boundaries.mjs (no new violations) — all pass.
• Manual repro: connected an MCP server serving an app resource with the openInProxyRealm snippet. The blocked state is silent, so the PoC carries a small on-page diagnostic panel that confirms each step (inner frame is same-origin with the proxy, the injected script executes in the proxy realm, and window.open's return value). Before the fix, a registered scheme like zoommtg:// reached the sink (Zoom launched / an unregistered scheme showed a macOS "no application set to open the URL" dialog). After the fix, window.open returns null (popups removed) and the main-process log shows Blocked externally-opened URL with disallowed scheme.

Automatic notifications

• Publish to changelog?
• Alert Sales and Marketing teams?

[trunk-io]

😎 Merged manually by @tatoalo - details.

[tatoalo]

hey @zachdotai, thanks a lot for this! Before merging, could you perhaps have a look at tightening the internal-navigation check? We can compare parsed URLs against the exact renderer origin/path, we could also maybe catch and log some shell.openExternal() rejections

[zachdotai]

hey @zachdotai, thanks a lot for this! Before merging, could you perhaps have a look at tightening the internal-navigation check? We can compare parsed URLs against the exact renderer origin/path, we could also maybe catch and log some shell.openExternal() rejections

this is now done!

[tatoalo]

@zachdotai we have commit signing protection enforced in the repo, approved otherwise! 🙌🏻