Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 41 additions & 0 deletions app/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ class Notification extends EventEmitter {
},
});
notificationWindow = win;
// Deny window.open + block foreign navigation on the toast window too, so the
// security guards cover EVERY BrowserWindow (#377). allowExternalLinks lets a
// Join/meeting link route out via shell.openExternal.
hardenWindow(win, { allowExternalLinks: true });
win._activeCustomNotification = this;
// Also pin the window to the notification instance so a handler bound to
// THIS notif (e.g. the pre-meeting 'close' dismissal-telemetry handler) can
Expand Down Expand Up @@ -1435,6 +1439,38 @@ function recoverPendingDeletesOnLaunch() {
}
}

// #377 — harden every BrowserWindow against untrusted navigation. The renderer
// routes all real external links through the `open-external` IPC (shell.openExternal),
// so nothing legitimately relies on window.open or on navigating the window away
// from the bundled renderer. Deny window.open outright, and cancel any full-page
// navigation — the SPA uses in-app routing, so a will-navigate here is a
// foreign/injected navigation, never a real route change. Interactive windows still
// let a genuine http(s) link (a target=_blank "Report an issue", or a link
// inside a note) open in the user's browser instead of silently dying.
function hardenWindow(win, { allowExternalLinks = false } = {}) {
const wc = win.webContents;
wc.setWindowOpenHandler(({ url }) => {
// HTTP(S) only — matches the will-navigate branch below and the established
// `open-external` IPC policy (no mailto/other schemes; nothing legitimate in
// the renderer opens a non-http(s) popup). #377 review.
if (allowExternalLinks && /^https?:/i.test(url)) {
shell.openExternal(url);
}
return { action: 'deny' };
});
wc.on('will-navigate', (event) => {
// Electron 42: read the target off the event (the positional `url` arg is
// deprecated). will-navigate only fires on a real document navigation — the
// SPA's hash routing never reaches here, so anything that does is foreign.
const url = event.url;
Comment thread
Optic00 marked this conversation as resolved.
if (url === wc.getURL()) return; // reload of the exact trusted document is fine
event.preventDefault();
if (allowExternalLinks && /^https?:/i.test(url)) {
shell.openExternal(url);
}
});
}

function createWindow(options = {}) {
rendererShortcutReady = false;

Expand Down Expand Up @@ -1480,6 +1516,7 @@ function createWindow(options = {}) {
}

mainWindow = new BrowserWindow(windowOpts);
hardenWindow(mainWindow, { allowExternalLinks: true });

const rendererDist = path.join(__dirname, 'renderer', 'dist', 'index.html');
const hash = process.env.STENOAI_RENDERER_HASH;
Expand Down Expand Up @@ -3693,6 +3730,10 @@ async function renderHtmlToPdf(html) {
// renderer-supplied document to PDF.
},
});
// Background render of renderer-supplied HTML: deny any popup and block any
// navigation the document might attempt (no openExternal — this is not an
// interactive window and must never spawn a browser tab on its own).
hardenWindow(win);
try {
const render = (async () => {
// Load the HTML directly as a data URL (self-contained: CSS, font, and
Expand Down
40 changes: 40 additions & 0 deletions e2e/specs/security-window-guards.t1.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { test, expect } from '../fixtures/electron';

/**
* T1 — renderer-only, mock IPC, no backend. Proves the #377 BrowserWindow
* hardening: the main window's `setWindowOpenHandler` denies every popup, so a
* renderer `window.open(...)` (or a `target="_blank"` link) can never spawn a
* chrome-less in-app Electron window. `about:blank` is used deliberately — it is
* not an http(s)/mailto URL, so the deny path runs without the interactive
* window's `shell.openExternal` fallback launching a real browser tab in CI.
*
* Scope: this asserts the popup-deny guard, which is the primary renderer-
* reachable risk surface. The companion `will-navigate` guard (defense-in-depth
* against full-page navigation away from the bundled renderer) is deliberately
* NOT asserted here: modern Chromium already blocks the only hermetic navigation
* vectors (`data:`/`about:blank`/missing `file:`), so a fail-before-provable test
* isn't achievable without a networked http target (side effects). That guard is
* verified by code review + reasoning, not a test that would pass regardless.
*/
test('window.open is denied — no popup BrowserWindow is created (#377)', async ({
launchApp,
}) => {
const { app, page } = await launchApp({ mockIpc: true });

const windowsBefore = app.windows().length;

// If the deny handler failed, a new window would appear and fire this event.
// We assert the *absence* of that event within a bounded window (event-race,
// not a fixed sleep in the assertion).
const popupAppeared = app
.waitForEvent('window', { timeout: 1500 })
.then(() => true)
.catch(() => false);

await page.evaluate(() => {
window.open('about:blank', '_blank');
});

expect(await popupAppeared).toBe(false);
expect(app.windows().length).toBe(windowsBefore);
});
Loading