fix(security): harden Electron MCP navigation boundaries

apps/code/src/main/bootstrap.ts

@@ -42,7 +42,9 @@ app.setName(isDev ? "PostHog Code (Development)" : "PostHog Code");
 
 // Set userData path for @posthog/code
 const appDataPath = app.getPath("appData");
-const userDataPath = path.join(appDataPath, "@posthog", appName);
+const userDataPath =
+  process.env.POSTHOG_E2E_USER_DATA_DIR ??
+  path.join(appDataPath, "@posthog", appName);
 app.setPath("userData", userDataPath);
 
 // Export the electron-derived state to env so utility singletons (utils/*,

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

@@ -18,13 +18,33 @@ vi.mock("./utils/logger.js", () => ({
   },
 }));
 
-import { setupExternalLinkHandlers } from "./external-links";
+import {
+  setupExternalLinkHandlers,
+  setupExternalLinkPermissionHandlers,
+} from "./external-links";
 
 type WindowOpenHandler = (details: { url: string }) => { action: string };
 type WillNavigateHandler = (
   event: { preventDefault: () => void },
   url: string,
 ) => void;
+type WillFrameNavigateHandler = (details: {
+  preventDefault: () => void;
+  isMainFrame: boolean;
+  url: string;
+}) => void;
+type PermissionCheckHandler = (
+  webContents: unknown,
+  permission: string,
+  requestingOrigin: string,
+  details: { isMainFrame: boolean },
+) => boolean;
+type PermissionRequestHandler = (
+  webContents: unknown,
+  permission: string,
+  callback: (permissionGranted: boolean) => void,
+  details: Record<string, unknown>,
+) => void;
 
 // Packaged renderer served from a file: URL, and dev renderer from the Vite origin.
 const PROD_HOME = new URL(
@@ -35,24 +55,60 @@ const DEV_HOME = new URL("http://localhost:5173");
 function setup(appHome: URL) {
   let windowOpenHandler: WindowOpenHandler | undefined;
   let willNavigateHandler: WillNavigateHandler | undefined;
+  let willFrameNavigateHandler: WillFrameNavigateHandler | undefined;
   const window = {
     webContents: {
       setWindowOpenHandler: (handler: WindowOpenHandler) => {
         windowOpenHandler = handler;
       },
-      on: (event: string, handler: WillNavigateHandler) => {
-        if (event === "will-navigate") willNavigateHandler = handler;
+      on: (
+        event: string,
+        handler: WillNavigateHandler | WillFrameNavigateHandler,
+      ) => {
+        if (event === "will-navigate") {
+          willNavigateHandler = handler as WillNavigateHandler;
+        }
+        if (event === "will-frame-navigate") {
+          willFrameNavigateHandler = handler as WillFrameNavigateHandler;
+        }
       },
     },
   };
   setupExternalLinkHandlers(
     window as unknown as Parameters<typeof setupExternalLinkHandlers>[0],
     appHome,
   );
-  if (!windowOpenHandler || !willNavigateHandler) {
+  if (!windowOpenHandler || !willNavigateHandler || !willFrameNavigateHandler) {
     throw new Error("Handlers were not registered");
   }
-  return { windowOpenHandler, willNavigateHandler };
+  return {
+    windowOpenHandler,
+    willNavigateHandler,
+    willFrameNavigateHandler,
+  };
+}
+
+function setupPermissionHandlers() {
+  let permissionCheckHandler: PermissionCheckHandler | undefined;
+  let permissionRequestHandler: PermissionRequestHandler | undefined;
+  const session = {
+    setPermissionCheckHandler: (handler: PermissionCheckHandler) => {
+      permissionCheckHandler = handler;
+    },
+    setPermissionRequestHandler: (handler: PermissionRequestHandler) => {
+      permissionRequestHandler = handler;
+    },
+  };
+
+  setupExternalLinkPermissionHandlers(
+    session as unknown as Parameters<
+      typeof setupExternalLinkPermissionHandlers
+    >[0],
+  );
+  if (!permissionCheckHandler || !permissionRequestHandler) {
+    throw new Error("Permission handlers were not registered");
+  }
+  return { permissionCheckHandler, permissionRequestHandler };
 }
 
 const SAFE_URLS = [
@@ -78,120 +134,199 @@ beforeEach(() => {
   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);
+describe("external link policies", () => {
+  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 });
+      const result = windowOpenHandler({ url });
 
-    expect(result).toEqual({ action: "deny" });
-    expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(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);
+    it.each(UNSAFE_URLS)("blocks %s without opening it", (url) => {
+      const { windowOpenHandler } = setup(PROD_HOME);
 
-    const result = windowOpenHandler({ url });
+      const result = windowOpenHandler({ url });
 
-    expect(result).toEqual({ action: "deny" });
-    expect(mockOpenExternal).not.toHaveBeenCalled();
-    expect(mockWarn).toHaveBeenCalledOnce();
-  });
+      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);
+    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));
+      windowOpenHandler({ url: "https://posthog.com" });
+      await Promise.resolve();
 
-    expect(mockWarn).toHaveBeenCalledOnce();
+      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();
+  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/index.html?source=reload",
+    ])("treats renderer entry file %s as internal navigation", (url) => {
+      const { willNavigateHandler } = setup(PROD_HOME);
+      const preventDefault = vi.fn();
 
-    willNavigateHandler({ preventDefault }, url);
+      willNavigateHandler({ preventDefault }, url);
 
-    expect(preventDefault).not.toHaveBeenCalled();
-    expect(mockOpenExternal).not.toHaveBeenCalled();
-  });
+      expect(preventDefault).not.toHaveBeenCalled();
+      expect(mockOpenExternal).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      "file:///etc/passwd",
+      "file:///Applications/PostHog.app/resources/renderer/other/index.html",
+      "file:///Applications/PostHog.app/resources/renderer/main_window/assets/app.js",
+      "file://attacker.example/Applications/PostHog.app/resources/renderer/main_window/index.html",
+      "file:///Applications/PostHog.app/resources/renderer/main_window/index.html%2F..%2Fpayload.html",
+    ])("blocks non-entry file %s without opening it externally", (url) => {
+      const { willNavigateHandler } = setup(PROD_HOME);
+      const preventDefault = vi.fn();
+
+      willNavigateHandler({ preventDefault }, url);
 
-  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();
+      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 }, url);
+      willNavigateHandler({ preventDefault }, "https://posthog.com");
 
-    expect(preventDefault).toHaveBeenCalledOnce();
-    expect(mockOpenExternal).not.toHaveBeenCalled();
-    expect(mockWarn).toHaveBeenCalledOnce();
+      expect(preventDefault).toHaveBeenCalledOnce();
+      expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(
+        "https://posthog.com",
+      );
+    });
   });
 
-  it("routes an external https link to the browser", () => {
-    const { willNavigateHandler } = setup(PROD_HOME);
-    const preventDefault = vi.fn();
+  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 }, "https://posthog.com");
+        willNavigateHandler({ preventDefault }, url);
 
-    expect(preventDefault).toHaveBeenCalledOnce();
-    expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(
-      "https://posthog.com",
+        expect(preventDefault).not.toHaveBeenCalled();
+        expect(mockOpenExternal).not.toHaveBeenCalled();
+      },
     );
-  });
-});
 
-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) => {
+    // Origin lookalikes must be handled as external URLs: userinfo resolving to
+    // another host, a longer port, and a scheme change.
+    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();
+    });
+  });
+
+  describe("will-frame-navigate (subframes)", () => {
+    it.each([
+      "mcp-sandbox://proxy",
+      "about:blank",
+      "about:srcdoc",
+      "https://example.com/embed",
+      "http://localhost:3000/embed",
+      "blob:mcp-sandbox://proxy/1234",
+      "data:text/html,<p>embedded</p>",
+    ])("allows browser-contained navigation to %s", (url) => {
+      const { willFrameNavigateHandler } = setup(PROD_HOME);
+      const preventDefault = vi.fn();
+
+      willFrameNavigateHandler({ preventDefault, isMainFrame: false, url });
+
       expect(preventDefault).not.toHaveBeenCalled();
       expect(mockOpenExternal).not.toHaveBeenCalled();
-    },
-  );
+    });
+
+    it.each([
+      "smb://attacker.example/share",
+      "file:///etc/passwd",
+      "mailto:attacker@example.com",
+      "custom-scheme://payload",
+      "javascript:alert(1)",
+      "not a url",
+    ])("blocks external application navigation to %s", (url) => {
+      const { willFrameNavigateHandler } = setup(PROD_HOME);
+      const preventDefault = vi.fn();
 
-  // 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);
-  });
+      willFrameNavigateHandler({ preventDefault, isMainFrame: false, url });
 
-  it("blocks an unsafe scheme in dev too", () => {
-    const { willNavigateHandler } = setup(DEV_HOME);
-    const preventDefault = vi.fn();
+      expect(preventDefault).toHaveBeenCalledOnce();
+      expect(mockOpenExternal).not.toHaveBeenCalled();
+      expect(mockWarn).toHaveBeenCalledOnce();
+    });
 
-    willNavigateHandler({ preventDefault }, "file:///etc/passwd");
+    it("leaves main-frame navigation to the main-frame handler", () => {
+      const { willFrameNavigateHandler } = setup(PROD_HOME);
+      const preventDefault = vi.fn();
 
-    expect(preventDefault).toHaveBeenCalledOnce();
-    expect(mockOpenExternal).not.toHaveBeenCalled();
-    expect(mockWarn).toHaveBeenCalledOnce();
+      willFrameNavigateHandler({
+        preventDefault,
+        isMainFrame: true,
+        url: "custom-scheme://payload",
+      });
+
+      expect(preventDefault).not.toHaveBeenCalled();
+      expect(mockWarn).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("session permission handlers", () => {
+    it.each([
+      { permission: "openExternal", expected: false },
+      { permission: "media", expected: true },
+    ])(
+      "returns $expected for $permission permission checks and requests",
+      ({ permission, expected }) => {
+        const { permissionCheckHandler, permissionRequestHandler } =
+          setupPermissionHandlers();
+        const callback = vi.fn();
+
+        expect(
+          permissionCheckHandler(null, permission, "mcp-sandbox://proxy", {
+            isMainFrame: false,
+          }),
+        ).toBe(expected);
+        permissionRequestHandler({}, permission, callback, {});
+
+        expect(callback).toHaveBeenCalledExactlyOnceWith(expected);
+      },
+    );
   });
 });