Skip to content

fix(electron): block untrusted window.open and navigations #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ubugeeei merged 1 commit into from
Jun 2, 2026
Merged

fix(electron): block untrusted window.open and navigations #74

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions electron/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
BrowserWindow,
Menu,
ipcMain,
shell,
type IpcMainInvokeEvent,
type MenuItemConstructorOptions,
} from "electron"
Expand Down Expand Up @@ -115,6 +116,35 @@ function assertTrustedIpcSender(event: IpcMainInvokeEvent) {
assertTrustedUrl(senderUrl, "render IPC sender")
}

function isTrustedUrl(rawUrl: string) {
try {
assertTrustedUrl(rawUrl, "url")
return true
} catch {
return false
}
}

function hardenWindow(win: BrowserWindow) {
win.webContents.setWindowOpenHandler(({ url }) => {
if (isTrustedUrl(url)) {
return { action: "allow" }
}
Comment on lines +130 to +132
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject opaque-origin URLs in window-open trust check

This allow branch trusts any URL whose origin is "null", which includes schemes like data: and javascript: (not just your app files). Because isTrustedUrl() delegates to assertTrustedUrl(), a renderer can still open opaque-origin popup URLs inside Electron instead of being denied/externalized, which bypasses the new hardening goal for untrusted window.open targets.

Useful? React with 👍 / 👎.

Comment on lines +129 to +132
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Harden BrowserWindow instances created by allowed popups

Returning { action: "allow" } lets Electron create a new BrowserWindow, but this code never attaches hardenWindow() to that child window. As a result, any allowed popup (trusted initial URL) can later navigate/open untrusted URLs without the will-navigate / will-redirect / setWindowOpenHandler guards applied here, leaving a bypass path for the new protections.

Useful? React with 👍 / 👎.

Comment on lines +130 to +132
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Limit file:// trust when allowing window.open targets

The trust predicate returns true for any file: URL, and this handler now uses that predicate to allow popup creation. That means a renderer can open arbitrary local files (e.g. file:///tmp/...) inside the app instead of being denied/externalized, which undermines the “untrusted window.open” hardening in this commit and expands the set of in-app destinations far beyond your packaged app content.

Useful? React with 👍 / 👎.

void shell.openExternal(url).catch(() => {
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Allowlist protocols before calling shell.openExternal

This forwards every untrusted popup URL directly into shell.openExternal, including attacker-controlled custom protocols. Electron’s security guidance warns this can be abused to invoke arbitrary external handlers/apps, so a compromised renderer (e.g., XSS in any trusted page) can escalate impact from “blocked in-app popup” to launching host-level protocol handlers unless you explicitly restrict allowed schemes (typically https/http, optionally mailto).

Useful? React with 👍 / 👎.

// ignore – we already refused to open it inside the app
})
return { action: "deny" }
})

const guardNavigation = (event: Electron.Event, url: string) => {
if (!isTrustedUrl(url)) {
event.preventDefault()
}
}
win.webContents.on("will-navigate", guardNavigation)
win.webContents.on("will-redirect", guardNavigation)
}

function getBundledBinaryEnv(): NodeJS.ProcessEnv {
const env: NodeJS.ProcessEnv = {}
const ffmpegPath =
Expand Down Expand Up @@ -564,6 +594,7 @@ async function createWindow() {
contextIsolation: true,
},
})
hardenWindow(mainWindow)

if (useDevServer && process.env.VITE_DEV_SERVER_URL) {
assertTrustedUrl(process.env.VITE_DEV_SERVER_URL, "VITE_DEV_SERVER_URL")
Expand Down Expand Up @@ -603,6 +634,7 @@ function createRenderSettingsWindow() {
sandbox: false,
},
})
hardenWindow(renderSettingsWindow)
renderSettingsWindow.setMenu(null)
renderSettingsWindow.setMenuBarVisibility(false)

Expand Down Expand Up @@ -641,6 +673,7 @@ function createRenderProgressWindow() {
preload: resolveRenderPreloadPath(),
},
})
hardenWindow(renderProgressWindow)
renderProgressWindow.setMenu(null)
renderProgressWindow.setMenuBarVisibility(false)

Expand Down
Loading