Skip to content

Commit 9265994

Browse files
Merging e2b5aae into trunk-temp/pr-3464/49d6f01a-ff12-4103-8c8d-2234e5ff956f
2 parents b647382 + e2b5aae commit 9265994

6 files changed

Lines changed: 368 additions & 115 deletions

File tree

apps/code/src/main/bootstrap.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ app.setName(isDev ? "PostHog Code (Development)" : "PostHog Code");
4242

4343
// Set userData path for @posthog/code
4444
const appDataPath = app.getPath("appData");
45-
const userDataPath = path.join(appDataPath, "@posthog", appName);
45+
const userDataPath =
46+
process.env.POSTHOG_E2E_USER_DATA_DIR ??
47+
path.join(appDataPath, "@posthog", appName);
4648
app.setPath("userData", userDataPath);
4749

4850
// Export the electron-derived state to env so utility singletons (utils/*,

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

Lines changed: 223 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,33 @@ vi.mock("./utils/logger.js", () => ({
1818
},
1919
}));
2020

21-
import { setupExternalLinkHandlers } from "./external-links";
21+
import {
22+
setupExternalLinkHandlers,
23+
setupExternalLinkPermissionHandlers,
24+
} from "./external-links";
2225

2326
type WindowOpenHandler = (details: { url: string }) => { action: string };
2427
type WillNavigateHandler = (
2528
event: { preventDefault: () => void },
2629
url: string,
2730
) => void;
31+
type WillFrameNavigateHandler = (details: {
32+
preventDefault: () => void;
33+
isMainFrame: boolean;
34+
url: string;
35+
}) => void;
36+
type PermissionCheckHandler = (
37+
webContents: unknown,
38+
permission: string,
39+
requestingOrigin: string,
40+
details: { isMainFrame: boolean },
41+
) => boolean;
42+
type PermissionRequestHandler = (
43+
webContents: unknown,
44+
permission: string,
45+
callback: (permissionGranted: boolean) => void,
46+
details: Record<string, unknown>,
47+
) => void;
2848

2949
// Packaged renderer served from a file: URL, and dev renderer from the Vite origin.
3050
const PROD_HOME = new URL(
@@ -35,24 +55,60 @@ const DEV_HOME = new URL("http://localhost:5173");
3555
function setup(appHome: URL) {
3656
let windowOpenHandler: WindowOpenHandler | undefined;
3757
let willNavigateHandler: WillNavigateHandler | undefined;
58+
let willFrameNavigateHandler: WillFrameNavigateHandler | undefined;
3859
const window = {
3960
webContents: {
4061
setWindowOpenHandler: (handler: WindowOpenHandler) => {
4162
windowOpenHandler = handler;
4263
},
43-
on: (event: string, handler: WillNavigateHandler) => {
44-
if (event === "will-navigate") willNavigateHandler = handler;
64+
on: (
65+
event: string,
66+
handler: WillNavigateHandler | WillFrameNavigateHandler,
67+
) => {
68+
if (event === "will-navigate") {
69+
willNavigateHandler = handler as WillNavigateHandler;
70+
}
71+
if (event === "will-frame-navigate") {
72+
willFrameNavigateHandler = handler as WillFrameNavigateHandler;
73+
}
4574
},
4675
},
4776
};
4877
setupExternalLinkHandlers(
4978
window as unknown as Parameters<typeof setupExternalLinkHandlers>[0],
5079
appHome,
5180
);
52-
if (!windowOpenHandler || !willNavigateHandler) {
81+
if (!windowOpenHandler || !willNavigateHandler || !willFrameNavigateHandler) {
5382
throw new Error("Handlers were not registered");
5483
}
55-
return { windowOpenHandler, willNavigateHandler };
84+
return {
85+
windowOpenHandler,
86+
willNavigateHandler,
87+
willFrameNavigateHandler,
88+
};
89+
}
90+
91+
function setupPermissionHandlers() {
92+
let permissionCheckHandler: PermissionCheckHandler | undefined;
93+
let permissionRequestHandler: PermissionRequestHandler | undefined;
94+
const session = {
95+
setPermissionCheckHandler: (handler: PermissionCheckHandler) => {
96+
permissionCheckHandler = handler;
97+
},
98+
setPermissionRequestHandler: (handler: PermissionRequestHandler) => {
99+
permissionRequestHandler = handler;
100+
},
101+
};
102+
103+
setupExternalLinkPermissionHandlers(
104+
session as unknown as Parameters<
105+
typeof setupExternalLinkPermissionHandlers
106+
>[0],
107+
);
108+
if (!permissionCheckHandler || !permissionRequestHandler) {
109+
throw new Error("Permission handlers were not registered");
110+
}
111+
return { permissionCheckHandler, permissionRequestHandler };
56112
}
57113

58114
const SAFE_URLS = [
@@ -78,120 +134,199 @@ beforeEach(() => {
78134
mockOpenExternal.mockImplementation(() => Promise.resolve());
79135
});
80136

81-
describe("window open handler", () => {
82-
it.each(SAFE_URLS)("opens %s externally and denies the window", (url) => {
83-
const { windowOpenHandler } = setup(PROD_HOME);
137+
describe("external link policies", () => {
138+
describe("window open handler", () => {
139+
it.each(SAFE_URLS)("opens %s externally and denies the window", (url) => {
140+
const { windowOpenHandler } = setup(PROD_HOME);
84141

85-
const result = windowOpenHandler({ url });
142+
const result = windowOpenHandler({ url });
86143

87-
expect(result).toEqual({ action: "deny" });
88-
expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(url);
89-
});
144+
expect(result).toEqual({ action: "deny" });
145+
expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(url);
146+
});
90147

91-
it.each(UNSAFE_URLS)("blocks %s without opening it", (url) => {
92-
const { windowOpenHandler } = setup(PROD_HOME);
148+
it.each(UNSAFE_URLS)("blocks %s without opening it", (url) => {
149+
const { windowOpenHandler } = setup(PROD_HOME);
93150

94-
const result = windowOpenHandler({ url });
151+
const result = windowOpenHandler({ url });
95152

96-
expect(result).toEqual({ action: "deny" });
97-
expect(mockOpenExternal).not.toHaveBeenCalled();
98-
expect(mockWarn).toHaveBeenCalledOnce();
99-
});
153+
expect(result).toEqual({ action: "deny" });
154+
expect(mockOpenExternal).not.toHaveBeenCalled();
155+
expect(mockWarn).toHaveBeenCalledOnce();
156+
});
100157

101-
it("swallows an openExternal rejection instead of leaving it unhandled", async () => {
102-
mockOpenExternal.mockImplementationOnce(() =>
103-
Promise.reject(new Error("no handler")),
104-
);
105-
const { windowOpenHandler } = setup(PROD_HOME);
158+
it("swallows an openExternal rejection instead of leaving it unhandled", async () => {
159+
mockOpenExternal.mockImplementationOnce(() =>
160+
Promise.reject(new Error("no handler")),
161+
);
162+
const { windowOpenHandler } = setup(PROD_HOME);
106163

107-
windowOpenHandler({ url: "https://posthog.com" });
108-
await new Promise((resolve) => setTimeout(resolve, 0));
164+
windowOpenHandler({ url: "https://posthog.com" });
165+
await Promise.resolve();
109166

110-
expect(mockWarn).toHaveBeenCalledOnce();
167+
expect(mockWarn).toHaveBeenCalledOnce();
168+
});
111169
});
112-
});
113170

114-
describe("will-navigate (packaged, file: home)", () => {
115-
it.each([
116-
"file:///Applications/PostHog.app/resources/renderer/main_window/index.html",
117-
"file:///Applications/PostHog.app/resources/renderer/main_window/index.html#/tasks/1",
118-
"file:///Applications/PostHog.app/resources/renderer/main_window/assets/app.js",
119-
])("treats in-app file %s as internal navigation", (url) => {
120-
const { willNavigateHandler } = setup(PROD_HOME);
121-
const preventDefault = vi.fn();
171+
describe("will-navigate (packaged, file: home)", () => {
172+
it.each([
173+
"file:///Applications/PostHog.app/resources/renderer/main_window/index.html",
174+
"file:///Applications/PostHog.app/resources/renderer/main_window/index.html#/tasks/1",
175+
"file:///Applications/PostHog.app/resources/renderer/main_window/index.html?source=reload",
176+
])("treats renderer entry file %s as internal navigation", (url) => {
177+
const { willNavigateHandler } = setup(PROD_HOME);
178+
const preventDefault = vi.fn();
122179

123-
willNavigateHandler({ preventDefault }, url);
180+
willNavigateHandler({ preventDefault }, url);
124181

125-
expect(preventDefault).not.toHaveBeenCalled();
126-
expect(mockOpenExternal).not.toHaveBeenCalled();
127-
});
182+
expect(preventDefault).not.toHaveBeenCalled();
183+
expect(mockOpenExternal).not.toHaveBeenCalled();
184+
});
185+
186+
it.each([
187+
"file:///etc/passwd",
188+
"file:///Applications/PostHog.app/resources/renderer/other/index.html",
189+
"file:///Applications/PostHog.app/resources/renderer/main_window/assets/app.js",
190+
"file://attacker.example/Applications/PostHog.app/resources/renderer/main_window/index.html",
191+
"file:///Applications/PostHog.app/resources/renderer/main_window/index.html%2F..%2Fpayload.html",
192+
])("blocks non-entry file %s without opening it externally", (url) => {
193+
const { willNavigateHandler } = setup(PROD_HOME);
194+
const preventDefault = vi.fn();
195+
196+
willNavigateHandler({ preventDefault }, url);
128197

129-
it.each([
130-
"file:///etc/passwd",
131-
"file:///Applications/PostHog.app/resources/renderer/other/index.html",
132-
])("blocks out-of-app file %s (not opened externally either)", (url) => {
133-
const { willNavigateHandler } = setup(PROD_HOME);
134-
const preventDefault = vi.fn();
198+
expect(preventDefault).toHaveBeenCalledOnce();
199+
expect(mockOpenExternal).not.toHaveBeenCalled();
200+
expect(mockWarn).toHaveBeenCalledOnce();
201+
});
202+
203+
it("routes an external https link to the browser", () => {
204+
const { willNavigateHandler } = setup(PROD_HOME);
205+
const preventDefault = vi.fn();
135206

136-
willNavigateHandler({ preventDefault }, url);
207+
willNavigateHandler({ preventDefault }, "https://posthog.com");
137208

138-
expect(preventDefault).toHaveBeenCalledOnce();
139-
expect(mockOpenExternal).not.toHaveBeenCalled();
140-
expect(mockWarn).toHaveBeenCalledOnce();
209+
expect(preventDefault).toHaveBeenCalledOnce();
210+
expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(
211+
"https://posthog.com",
212+
);
213+
});
141214
});
142215

143-
it("routes an external https link to the browser", () => {
144-
const { willNavigateHandler } = setup(PROD_HOME);
145-
const preventDefault = vi.fn();
216+
describe("will-navigate (dev server, http: home)", () => {
217+
it.each(["http://localhost:5173/", "http://localhost:5173/sessions/42"])(
218+
"treats same-origin dev URL %s as internal navigation",
219+
(url) => {
220+
const { willNavigateHandler } = setup(DEV_HOME);
221+
const preventDefault = vi.fn();
146222

147-
willNavigateHandler({ preventDefault }, "https://posthog.com");
223+
willNavigateHandler({ preventDefault }, url);
148224

149-
expect(preventDefault).toHaveBeenCalledOnce();
150-
expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(
151-
"https://posthog.com",
225+
expect(preventDefault).not.toHaveBeenCalled();
226+
expect(mockOpenExternal).not.toHaveBeenCalled();
227+
},
152228
);
153-
});
154-
});
155229

156-
describe("will-navigate (dev server, http: home)", () => {
157-
it.each(["http://localhost:5173/", "http://localhost:5173/sessions/42"])(
158-
"treats same-origin dev URL %s as internal navigation",
159-
(url) => {
230+
// Origin lookalikes must be handled as external URLs: userinfo resolving to
231+
// another host, a longer port, and a scheme change.
232+
it.each([
233+
"http://localhost:5173@evil.example/",
234+
"http://localhost:51730/",
235+
"https://localhost:5173/",
236+
])("does not treat lookalike origin %s as internal", (url) => {
160237
const { willNavigateHandler } = setup(DEV_HOME);
161238
const preventDefault = vi.fn();
162239

163240
willNavigateHandler({ preventDefault }, url);
164241

242+
expect(preventDefault).toHaveBeenCalledOnce();
243+
expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(url);
244+
});
245+
246+
it("blocks an unsafe scheme in dev too", () => {
247+
const { willNavigateHandler } = setup(DEV_HOME);
248+
const preventDefault = vi.fn();
249+
250+
willNavigateHandler({ preventDefault }, "file:///etc/passwd");
251+
252+
expect(preventDefault).toHaveBeenCalledOnce();
253+
expect(mockOpenExternal).not.toHaveBeenCalled();
254+
expect(mockWarn).toHaveBeenCalledOnce();
255+
});
256+
});
257+
258+
describe("will-frame-navigate (subframes)", () => {
259+
it.each([
260+
"mcp-sandbox://proxy",
261+
"about:blank",
262+
"about:srcdoc",
263+
"https://example.com/embed",
264+
"http://localhost:3000/embed",
265+
"blob:mcp-sandbox://proxy/1234",
266+
"data:text/html,<p>embedded</p>",
267+
])("allows browser-contained navigation to %s", (url) => {
268+
const { willFrameNavigateHandler } = setup(PROD_HOME);
269+
const preventDefault = vi.fn();
270+
271+
willFrameNavigateHandler({ preventDefault, isMainFrame: false, url });
272+
165273
expect(preventDefault).not.toHaveBeenCalled();
166274
expect(mockOpenExternal).not.toHaveBeenCalled();
167-
},
168-
);
275+
});
276+
277+
it.each([
278+
"smb://attacker.example/share",
279+
"file:///etc/passwd",
280+
"mailto:attacker@example.com",
281+
"custom-scheme://payload",
282+
"javascript:alert(1)",
283+
"not a url",
284+
])("blocks external application navigation to %s", (url) => {
285+
const { willFrameNavigateHandler } = setup(PROD_HOME);
286+
const preventDefault = vi.fn();
169287

170-
// The old startsWith check treated these as in-app, so an attacker origin
171-
// could load inside the app window. They must now be punted to the browser:
172-
// userinfo that resolves to another host, a longer port, and a scheme swap.
173-
it.each([
174-
"http://localhost:5173@evil.example/",
175-
"http://localhost:51730/",
176-
"https://localhost:5173/",
177-
])("does not treat lookalike origin %s as internal", (url) => {
178-
const { willNavigateHandler } = setup(DEV_HOME);
179-
const preventDefault = vi.fn();
180-
181-
willNavigateHandler({ preventDefault }, url);
182-
183-
expect(preventDefault).toHaveBeenCalledOnce();
184-
expect(mockOpenExternal).toHaveBeenCalledExactlyOnceWith(url);
185-
});
288+
willFrameNavigateHandler({ preventDefault, isMainFrame: false, url });
186289

187-
it("blocks an unsafe scheme in dev too", () => {
188-
const { willNavigateHandler } = setup(DEV_HOME);
189-
const preventDefault = vi.fn();
290+
expect(preventDefault).toHaveBeenCalledOnce();
291+
expect(mockOpenExternal).not.toHaveBeenCalled();
292+
expect(mockWarn).toHaveBeenCalledOnce();
293+
});
190294

191-
willNavigateHandler({ preventDefault }, "file:///etc/passwd");
295+
it("leaves main-frame navigation to the main-frame handler", () => {
296+
const { willFrameNavigateHandler } = setup(PROD_HOME);
297+
const preventDefault = vi.fn();
192298

193-
expect(preventDefault).toHaveBeenCalledOnce();
194-
expect(mockOpenExternal).not.toHaveBeenCalled();
195-
expect(mockWarn).toHaveBeenCalledOnce();
299+
willFrameNavigateHandler({
300+
preventDefault,
301+
isMainFrame: true,
302+
url: "custom-scheme://payload",
303+
});
304+
305+
expect(preventDefault).not.toHaveBeenCalled();
306+
expect(mockWarn).not.toHaveBeenCalled();
307+
});
308+
});
309+
310+
describe("session permission handlers", () => {
311+
it.each([
312+
{ permission: "openExternal", expected: false },
313+
{ permission: "media", expected: true },
314+
])(
315+
"returns $expected for $permission permission checks and requests",
316+
({ permission, expected }) => {
317+
const { permissionCheckHandler, permissionRequestHandler } =
318+
setupPermissionHandlers();
319+
const callback = vi.fn();
320+
321+
expect(
322+
permissionCheckHandler(null, permission, "mcp-sandbox://proxy", {
323+
isMainFrame: false,
324+
}),
325+
).toBe(expected);
326+
permissionRequestHandler({}, permission, callback, {});
327+
328+
expect(callback).toHaveBeenCalledExactlyOnceWith(expected);
329+
},
330+
);
196331
});
197332
});

0 commit comments

Comments
 (0)