Skip to content

Commit 648d3b8

Browse files
committed
fix(browser): harden and restore webview navigation
Generated-By: PostHog Code Task-Id: b23f944d-dd7f-465b-9fed-1a1d028f35e1
1 parent 18fed73 commit 648d3b8

23 files changed

Lines changed: 471 additions & 40 deletions
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
3+
describe("browserViewService", () => {
4+
beforeEach(() => {
5+
vi.resetModules();
6+
});
7+
8+
afterEach(() => {
9+
vi.unstubAllEnvs();
10+
});
11+
12+
it.each([
13+
["true", true],
14+
["false", false],
15+
])("defaults to %s in development mode", async (isDev, expected) => {
16+
vi.stubEnv("POSTHOG_CODE_IS_DEV", isDev);
17+
const { browserViewService } = await import("./service");
18+
19+
expect(browserViewService.isEnabled()).toBe(expected);
20+
});
21+
22+
it("updates the attachment gate", async () => {
23+
vi.stubEnv("POSTHOG_CODE_IS_DEV", "false");
24+
const { browserViewService } = await import("./service");
25+
26+
browserViewService.setEnabled(true);
27+
28+
expect(browserViewService.isEnabled()).toBe(true);
29+
});
30+
});
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { isDevBuild } from "../../utils/env";
2+
3+
class BrowserViewService {
4+
private enabled = isDevBuild();
5+
6+
isEnabled(): boolean {
7+
return this.enabled;
8+
}
9+
10+
setEnabled(enabled: boolean): void {
11+
this.enabled = enabled;
12+
}
13+
}
14+
15+
export const browserViewService = new BrowserViewService();

apps/code/src/main/trpc/router.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import { uiRouter } from "@posthog/host-router/routers/ui.router";
5050
import { updatesRouter } from "@posthog/host-router/routers/updates.router";
5151
import { usageMonitorRouter } from "@posthog/host-router/routers/usage-monitor.router";
5252
import { workspaceRouter } from "@posthog/host-router/routers/workspace.router";
53+
import { browserViewRouter } from "./routers/browser-view";
5354
import { devRouter } from "./routers/dev";
5455
import { discordPresenceRouter } from "./routers/discord-presence";
5556
import { encryptionRouter } from "./routers/encryption";
@@ -64,6 +65,7 @@ export const trpcRouter = router({
6465
auth: authRouter,
6566
autoresearch: autoresearchRouter,
6667
browserTabs: browserTabsRouter,
68+
browserView: browserViewRouter,
6769
canvasData: canvasDataRouter,
6870
canvasTemplates: canvasTemplatesRouter,
6971
channelTasks: channelTasksRouter,
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { z } from "zod";
2+
import { browserViewService } from "../../services/browser-view/service";
3+
import { publicProcedure, router } from "../trpc";
4+
5+
export const browserViewRouter = router({
6+
setEnabled: publicProcedure
7+
.input(z.object({ enabled: z.boolean() }))
8+
.mutation(({ input }) => browserViewService.setEnabled(input.enabled)),
9+
});

apps/code/src/main/utils/webview-attach-policy.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BROWSER_WEBVIEW_PARTITION } from "@posthog/shared/constants";
1+
import { BROWSER_WEBVIEW_PARTITION } from "@shared/browser-view";
22
import { describe, expect, it } from "vitest";
33
import {
44
hardenWebviewPreferences,

apps/code/src/main/utils/webview-attach-policy.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BROWSER_WEBVIEW_PARTITION } from "@posthog/shared/constants";
1+
import { BROWSER_WEBVIEW_PARTITION } from "@shared/browser-view";
22
import { isAllowedWebviewNavigation } from "./webview-navigation-guard";
33

44
interface WebviewSecurityPreferences {

apps/code/src/main/utils/webview-navigation-guard.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from "vitest";
22
import {
33
isAllowedWebviewNavigation,
4+
isAllowedWebviewRequest,
45
isBlockedWebviewHost,
56
} from "./webview-navigation-guard";
67

@@ -25,6 +26,25 @@ describe("isBlockedWebviewHost", () => {
2526
});
2627
});
2728

29+
describe("isAllowedWebviewRequest", () => {
30+
it.each([
31+
["https://posthog.com", "mainFrame", true],
32+
["http://posthog.com", "mainFrame", false],
33+
["https://posthog.com/app.js", "script", true],
34+
["http://localhost:3000/app.js", "script", true],
35+
["http://posthog.com/app.js", "script", false],
36+
["data:image/png;base64,AA==", "image", true],
37+
["blob:https://posthog.com/id", "xhr", true],
38+
["about:blank", "subFrame", true],
39+
["about:srcdoc", "subFrame", false],
40+
["file:///etc/passwd", "xhr", false],
41+
["custom-scheme://host/path", "xhr", false],
42+
["http://169.254.169.254/latest/meta-data/", "xhr", false],
43+
])("url %j resource %j -> allowed %s", (url, resourceType, allowed) => {
44+
expect(isAllowedWebviewRequest(url, resourceType)).toBe(allowed);
45+
});
46+
});
47+
2848
describe("isAllowedWebviewNavigation", () => {
2949
it.each([
3050
["https://posthog.com", true],

apps/code/src/main/utils/webview-navigation-guard.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,25 @@ export function isAllowedWebviewNavigation(url: string): boolean {
5858
if (parsed.protocol === "https:") return true;
5959
return parsed.protocol === "http:" && isLoopbackHost(parsed.hostname);
6060
}
61+
62+
export function isAllowedWebviewRequest(
63+
url: string,
64+
resourceType: string,
65+
): boolean {
66+
if (resourceType === "mainFrame") {
67+
return isAllowedWebviewNavigation(url);
68+
}
69+
70+
let parsed: URL;
71+
try {
72+
parsed = new URL(url);
73+
} catch {
74+
return false;
75+
}
76+
77+
if (parsed.href === "about:blank") return true;
78+
if (isBlockedWebviewHost(parsed.hostname)) return false;
79+
if (parsed.protocol === "data:" || parsed.protocol === "blob:") return true;
80+
if (parsed.protocol === "https:") return true;
81+
return parsed.protocol === "http:" && isLoopbackHost(parsed.hostname);
82+
}

apps/code/src/main/window.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { DARK_APP_BACKGROUND_COLOR } from "@posthog/shared/constants";
66
import {
77
app,
88
BrowserWindow,
9+
session as electronSession,
910
Menu,
1011
type MenuItemConstructorOptions,
1112
screen,
@@ -17,6 +18,7 @@ import { buildApplicationMenu } from "./menu";
1718
import type { ElectronMainWindow } from "./platform-adapters/electron-main-window";
1819
import { posthogNodeAnalytics } from "./platform-adapters/posthog-analytics";
1920
import { POSTHOG_SESSION_ID_ARG } from "./posthog-session-arg";
21+
import { browserViewService } from "./services/browser-view/service";
2022
import {
2123
encodeDevFlagsForArg,
2224
readDevFlagsSync,
@@ -39,7 +41,7 @@ import {
3941
} from "./utils/webview-attach-policy";
4042
import {
4143
isAllowedWebviewNavigation,
42-
safeProtocol,
44+
isAllowedWebviewRequest,
4345
} from "./utils/webview-navigation-guard";
4446
import { isAllowedWebviewPermission } from "./utils/webview-permission-policy";
4547
import { setupWindowZoom } from "./zoom";
@@ -142,20 +144,34 @@ function hardenWebviewSession(session: Electron.Session): void {
142144
session.setPermissionCheckHandler((_wc, permission) =>
143145
isAllowedWebviewPermission(permission),
144146
);
147+
session.webRequest.onBeforeRequest((details, callback) => {
148+
const allowed = isAllowedWebviewRequest(details.url, details.resourceType);
149+
if (!allowed) {
150+
log.warn("Blocked disallowed webview request", {
151+
resourceType: details.resourceType,
152+
url: details.url,
153+
});
154+
}
155+
callback({ cancel: !allowed });
156+
});
145157
}
146158

147159
function setupWebviewHandlers(window: BrowserWindow): void {
148160
window.webContents.on(
149161
"will-attach-webview",
150162
(event, webPreferences, params) => {
151-
if (!isAllowedWebviewAttachment(params)) {
163+
if (
164+
!browserViewService.isEnabled() ||
165+
!isAllowedWebviewAttachment(params)
166+
) {
152167
event.preventDefault();
153168
log.warn("Blocked disallowed webview attachment", {
154169
src: params.src,
155170
partition: params.partition,
156171
});
157172
return;
158173
}
174+
hardenWebviewSession(electronSession.fromPartition(params.partition));
159175
hardenWebviewPreferences(webPreferences);
160176
},
161177
);
@@ -164,10 +180,10 @@ function setupWebviewHandlers(window: BrowserWindow): void {
164180
hardenWebviewSession(guest.session);
165181

166182
guest.setWindowOpenHandler(({ url }) => {
167-
if (/^https?:$/i.test(safeProtocol(url))) {
168-
shell.openExternal(url);
183+
if (isAllowedWebviewNavigation(url)) {
184+
void shell.openExternal(url);
169185
} else {
170-
log.warn("Blocked webview popup to non-http(s) target", { url });
186+
log.warn("Blocked disallowed webview popup", { url });
171187
}
172188
return { action: "deny" };
173189
});
@@ -318,6 +334,7 @@ export function createWindow(): void {
318334
webPreferences: {
319335
nodeIntegration: false,
320336
contextIsolation: true,
337+
webviewTag: true,
321338
preload: path.join(__dirname, "preload.js"),
322339
enableBlinkFeatures: "GetDisplayMedia",
323340
partition: "persist:main",
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { Contribution } from "@posthog/di/contribution";
2+
import { BROWSER_TAB_FLAG } from "@posthog/shared/constants";
3+
import {
4+
FEATURE_FLAGS,
5+
type FeatureFlags,
6+
} from "@posthog/ui/features/feature-flags/identifiers";
7+
import { trpcClient } from "@renderer/trpc/client";
8+
import { inject, injectable } from "inversify";
9+
10+
@injectable()
11+
export class BrowserViewContribution implements Contribution {
12+
constructor(
13+
@inject(FEATURE_FLAGS) private readonly featureFlags: FeatureFlags,
14+
) {}
15+
16+
start(): void {
17+
const sync = (): void => {
18+
const enabled =
19+
import.meta.env.DEV || this.featureFlags.isEnabled(BROWSER_TAB_FLAG);
20+
void trpcClient.browserView.setEnabled.mutate({ enabled });
21+
};
22+
23+
sync();
24+
this.featureFlags.onFlagsLoaded(sync);
25+
}
26+
}

0 commit comments

Comments
 (0)