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

apps/code/src/main/external-links.test.ts

@@ -0,0 +1,197 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mockOpenExternal = vi.hoisted(() => vi.fn(() => Promise.resolve()));
+const mockWarn = vi.hoisted(() => vi.fn());
+
+vi.mock("electron", () => ({
+  shell: { openExternal: mockOpenExternal },
+}));
+
+vi.mock("./utils/logger.js", () => ({
+  logger: {
+    scope: () => ({
+      info: vi.fn(),
+      error: vi.fn(),
+      warn: mockWarn,
+      debug: vi.fn(),
+    }),
+  },
+}));
+
+import { setupExternalLinkHandlers } from "./external-links";
+
+type WindowOpenHandler = (details: { url: string }) => { action: string };
+type WillNavigateHandler = (
+  event: { preventDefault: () => void },
+  url: string,
+) => void;
+
+// Packaged renderer served from a file: URL, and dev renderer from the Vite origin.
+const PROD_HOME = new URL(
+  "file:///Applications/PostHog.app/resources/renderer/main_window/index.html",
+);
+const DEV_HOME = new URL("http://localhost:5173");
+
+function setup(appHome: URL) {
+  let windowOpenHandler: WindowOpenHandler | undefined;
+  let willNavigateHandler: WillNavigateHandler | undefined;
+  const window = {
+    webContents: {
+      setWindowOpenHandler: (handler: WindowOpenHandler) => {
+        windowOpenHandler = handler;
+      },
+      on: (event: string, handler: WillNavigateHandler) => {
+        if (event === "will-navigate") willNavigateHandler = handler;
+      },
+    },
+  };
+  setupExternalLinkHandlers(
+    window as unknown as Parameters<typeof setupExternalLinkHandlers>[0],
+    appHome,
+  );
+  if (!windowOpenHandler || !willNavigateHandler) {
+    throw new Error("Handlers were not registered");
+  }
+  return { windowOpenHandler, willNavigateHandler };
+}
+
+const SAFE_URLS = [
+  "https://posthog.com/docs",
+  "http://example.com",
+  "mailto:support@posthog.com",
+];
+
+// Schemes that dispatch to OS-registered handlers: smb/file enable NTLM
+// credential theft on Windows, ms-msdt-class handlers take attacker args,
+// and custom schemes deep-link into arbitrary installed apps.
+const UNSAFE_URLS = [
+  "smb://attacker.example/share",
+  "file:///etc/passwd",
+  "ms-msdt://id/PCWDiagnostic",
+  "custom-scheme://payload",
+  "javascript:alert(1)",
+  "not a url",
+];
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  mockOpenExternal.mockImplementation(() => Promise.resolve());
+});
+
+describe("window open handler", () => {
+  it.each(SAFE_URLS)("opens %s externally and denies the window", (url) => {
+    const { windowOpenHandler } = setup(PROD_HOME);
+
+    const result = windowOpenHandler({ url });
+
+    expect(result).toEqual({ action: "deny" });
+    expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(url);
+  });
+
+  it.each(UNSAFE_URLS)("blocks %s without opening it", (url) => {
+    const { windowOpenHandler } = setup(PROD_HOME);
+
+    const result = windowOpenHandler({ url });
+
+    expect(result).toEqual({ action: "deny" });
+    expect(mockOpenExternal).not.toHaveBeenCalled();
+    expect(mockWarn).toHaveBeenCalledOnce();
+  });
+
+  it("swallows an openExternal rejection instead of leaving it unhandled", async () => {
+    mockOpenExternal.mockImplementationOnce(() =>
+      Promise.reject(new Error("no handler")),
+    );
+    const { windowOpenHandler } = setup(PROD_HOME);
+
+    windowOpenHandler({ url: "https://posthog.com" });
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
+    expect(mockWarn).toHaveBeenCalledOnce();
+  });
+});
+
+describe("will-navigate (packaged, file: home)", () => {
+  it.each([
+    "file:///Applications/PostHog.app/resources/renderer/main_window/index.html",
+    "file:///Applications/PostHog.app/resources/renderer/main_window/index.html#/tasks/1",
+    "file:///Applications/PostHog.app/resources/renderer/main_window/assets/app.js",
+  ])("treats in-app file %s as internal navigation", (url) => {
+    const { willNavigateHandler } = setup(PROD_HOME);
+    const preventDefault = vi.fn();
+
+    willNavigateHandler({ preventDefault }, url);
+
+    expect(preventDefault).not.toHaveBeenCalled();
+    expect(mockOpenExternal).not.toHaveBeenCalled();
+  });
+
+  it.each([
+    "file:///etc/passwd",
+    "file:///Applications/PostHog.app/resources/renderer/other/index.html",
+  ])("blocks out-of-app file %s (not opened externally either)", (url) => {
+    const { willNavigateHandler } = setup(PROD_HOME);
+    const preventDefault = vi.fn();
+
+    willNavigateHandler({ preventDefault }, url);
+
+    expect(preventDefault).toHaveBeenCalledOnce();
+    expect(mockOpenExternal).not.toHaveBeenCalled();
+    expect(mockWarn).toHaveBeenCalledOnce();
+  });
+
+  it("routes an external https link to the browser", () => {
+    const { willNavigateHandler } = setup(PROD_HOME);
+    const preventDefault = vi.fn();
+
+    willNavigateHandler({ preventDefault }, "https://posthog.com");
+
+    expect(preventDefault).toHaveBeenCalledOnce();
+    expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(
+      "https://posthog.com",
+    );
+  });
+});
+
+describe("will-navigate (dev server, http: home)", () => {
+  it.each(["http://localhost:5173/", "http://localhost:5173/sessions/42"])(
+    "treats same-origin dev URL %s as internal navigation",
+    (url) => {
+      const { willNavigateHandler } = setup(DEV_HOME);
+      const preventDefault = vi.fn();
+
+      willNavigateHandler({ preventDefault }, url);
+
+      expect(preventDefault).not.toHaveBeenCalled();
+      expect(mockOpenExternal).not.toHaveBeenCalled();
+    },
+  );
+
+  // The old startsWith check treated these as in-app, so an attacker origin
+  // could load inside the app window. They must now be punted to the browser:
+  // userinfo that resolves to another host, a longer port, and a scheme swap.
+  it.each([
+    "http://localhost:5173@evil.example/",
+    "http://localhost:51730/",
+    "https://localhost:5173/",
+  ])("does not treat lookalike origin %s as internal", (url) => {
+    const { willNavigateHandler } = setup(DEV_HOME);
+    const preventDefault = vi.fn();
+
+    willNavigateHandler({ preventDefault }, url);
+
+    expect(preventDefault).toHaveBeenCalledOnce();
+    expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(url);
+  });
+
+  it("blocks an unsafe scheme in dev too", () => {
+    const { willNavigateHandler } = setup(DEV_HOME);
+    const preventDefault = vi.fn();
+
+    willNavigateHandler({ preventDefault }, "file:///etc/passwd");
+
+    expect(preventDefault).toHaveBeenCalledOnce();
+    expect(mockOpenExternal).not.toHaveBeenCalled();
+    expect(mockWarn).toHaveBeenCalledOnce();
+  });
+});

apps/code/src/main/external-links.ts

@@ -0,0 +1,76 @@
+import { isSafeExternalUrl } from "@posthog/shared";
+import { type BrowserWindow, shell } from "electron";
+import { logger } from "./utils/logger";
+
+const log = logger.scope("external-links");
+
+function urlScheme(url: string): string {
+  try {
+    return new URL(url).protocol;
+  } catch {
+    return "<unparseable>";
+  }
+}
+
+// `shell.openExternal` dispatches to whatever app the OS registered for the
+// scheme, so it must never receive a scheme outside the http/https/mailto
+// allowlist: renderer content (including sandboxed MCP apps) can reach these
+// handlers via window.open/navigation with e.g. smb:, file:, or ms-msdt: URLs.
+function openExternalIfSafe(url: string): void {
+  if (!isSafeExternalUrl(url)) {
+    log.warn("Blocked externally-opened URL with disallowed scheme", {
+      scheme: urlScheme(url),
+    });
+    return;
+  }
+  // openExternal rejects when the OS has no handler for the scheme (or the user
+  // dismisses the confirmation prompt on some platforms). Swallow it so a failed
+  // open never surfaces as an unhandled rejection in the main process.
+  shell.openExternal(url).catch((error) => {
+    log.warn("shell.openExternal rejected", { scheme: urlScheme(url), error });
+  });
+}
+
+// A navigation is "in-app" only when it targets the exact renderer origin (dev
+// server) or a file under the packaged renderer directory. Comparing parsed
+// URLs — rather than a startsWith prefix — stops lookalikes like
+// http://localhost:5173.evil.example or file:///etc/passwd from being treated
+// as internal and skipping the external-link scheme check below.
+function isInAppNavigation(target: string, appHome: URL): boolean {
+  let parsed: URL;
+  try {
+    parsed = new URL(target);
+  } catch {
+    return false;
+  }
+
+  if (appHome.protocol === "file:") {
+    // file: origins are all opaque ("null"), so pin to the directory that holds
+    // index.html instead of comparing origins.
+    if (parsed.protocol !== "file:") return false;
+    const appDir = appHome.pathname.slice(
+      0,
+      appHome.pathname.lastIndexOf("/") + 1,
+    );
+    return parsed.pathname.startsWith(appDir);
+  }
+
+  // Dev server (http/https): pin scheme + host + port exactly.
+  return parsed.origin === appHome.origin;
+}
+
+export function setupExternalLinkHandlers(
+  window: BrowserWindow,
+  appHome: URL,
+): void {
+  window.webContents.setWindowOpenHandler(({ url }) => {
+    openExternalIfSafe(url);
+    return { action: "deny" };
+  });
+
+  window.webContents.on("will-navigate", (event, url) => {
+    if (isInAppNavigation(url, appHome)) return;
+    event.preventDefault();
+    openExternalIfSafe(url);
+  });
+}

apps/code/src/main/window.ts

@@ -1,5 +1,5 @@
 import path from "node:path";
-import { fileURLToPath } from "node:url";
+import { fileURLToPath, pathToFileURL } from "node:url";
 import { createIPCHandler } from "@posthog/electron-trpc/main";
 import { MAIN_WINDOW_SERVICE } from "@posthog/platform/main-window";
 import { DARK_APP_BACKGROUND_COLOR } from "@posthog/shared/constants";
@@ -9,9 +9,9 @@ import {
   Menu,
   type MenuItemConstructorOptions,
   screen,
-  shell,
 } from "electron";
 import { container } from "./di/container";
+import { setupExternalLinkHandlers } from "./external-links";
 import { buildApplicationMenu } from "./menu";
 import type { ElectronMainWindow } from "./platform-adapters/electron-main-window";
 import { posthogNodeAnalytics } from "./platform-adapters/posthog-analytics";
@@ -120,21 +120,6 @@ export function focusMainWindow(reason: string): void {
   }
 }
 
-function setupExternalLinkHandlers(window: BrowserWindow): void {
-  window.webContents.setWindowOpenHandler(({ url }) => {
-    shell.openExternal(url);
-    return { action: "deny" };
-  });
-
-  window.webContents.on("will-navigate", (event, url) => {
-    const appUrl = MAIN_WINDOW_VITE_DEV_SERVER_URL || "file://";
-    if (!url.startsWith(appUrl)) {
-      event.preventDefault();
-      shell.openExternal(url);
-    }
-  });
-}
-
 function setupCrashLogging(window: BrowserWindow): void {
   window.webContents.on("render-process-gone", (_event, details) => {
     log.error("Renderer process gone", {
@@ -341,17 +326,26 @@ export function createWindow(): void {
     },
   });
 
-  setupExternalLinkHandlers(mainWindow);
+  const rendererFilePath = path.join(
+    __dirname,
+    `../renderer/${MAIN_WINDOW_VITE_NAME}/index.html`,
+  );
+  // The URL the renderer is served from, used to tell in-app navigations from
+  // external links. In dev it's the Vite server origin; in prod it's the
+  // packaged index.html file URL.
+  const appHome = MAIN_WINDOW_VITE_DEV_SERVER_URL
+    ? new URL(MAIN_WINDOW_VITE_DEV_SERVER_URL)
+    : pathToFileURL(rendererFilePath);
+
+  setupExternalLinkHandlers(mainWindow, appHome);
   setupEditableContextMenu(mainWindow);
   setupCrashLogging(mainWindow);
   buildApplicationMenu();
 
   if (MAIN_WINDOW_VITE_DEV_SERVER_URL) {
     mainWindow.loadURL(MAIN_WINDOW_VITE_DEV_SERVER_URL);
   } else {
-    mainWindow.loadFile(
-      path.join(__dirname, `../renderer/${MAIN_WINDOW_VITE_NAME}/index.html`),
-    );
+    mainWindow.loadFile(rendererFilePath);
   }
 
   mainWindow.on("closed", () => {

biome.jsonc

@@ -71,13 +71,14 @@
       // Main-process code must not import from "electron" directly. Every Electron
       // API goes through a port in @posthog/platform, implemented by an adapter in
       // apps/code/src/main/platform-adapters/. The only files allowed to import
-      // electron are the adapters and the 8 Electron host files below.
+      // electron are the adapters and the Electron host files below.
       "includes": [
         "apps/code/src/main/**/*.ts",
         "!apps/code/src/main/platform-adapters/**",
         "!apps/code/src/main/bootstrap.ts",
         "!apps/code/src/main/index.ts",
         "!apps/code/src/main/window.ts",
+        "!apps/code/src/main/external-links.ts",
         "!apps/code/src/main/menu.ts",
         "!apps/code/src/main/preload.ts",
         "!apps/code/src/main/deep-links.ts",

packages/ui/src/features/mcp-apps/components/McpAppHost.tsx

@@ -239,7 +239,10 @@ export function McpAppHost({
     <iframe
       ref={setIframeEl}
       src={sandboxProxyUrl}
-      sandbox="allow-scripts allow-same-origin allow-forms allow-popups allow-popups-to-escape-sandbox allow-presentation"
+      // No allow-popups: app JS is same-origin with the proxy realm, so popup
+      // permission here would let it window.open() past the sandbox. Apps
+      // open links via ui/open-link, which the host scheme-validates.
+      sandbox="allow-scripts allow-same-origin allow-forms allow-presentation"
       style={{
         height: displayMode === "fullscreen" ? "100%" : `${iframeHeight}px`,
       }}