From 1c9227bbb436062e6b6bfd97d6207e83c944025f Mon Sep 17 00:00:00 2001 From: SylvainDruaux Date: Thu, 29 Oct 2026 17:59:22 +0100 Subject: [PATCH] fix(security): limit what pages can open, load and be granted A page shown in Stacki, including a previewed site and any third-party script it runs, could ask the operating system to open any kind of link, use privacy-sensitive permissions that Electron grants without asking, and navigate the main window away from Stacki's own page, which then received the preload's window.avb API. Links that would open a new window now reach the browser only when they are http or https, the same rule as the existing shell:openExternal handler, so a page can no longer launch apps or local files. The main window stays on Stacki's own page: the same file when packaged, the same origin as the dev server in development; other navigations are cancelled and web links open in the browser instead. Camera, microphone, location, screen capture, clipboard reading, USB, serial, HID and idle detection are refused for every page, Stacki's own interface included, since Stacki uses none of them today. A future feature that needs one, such as voice input, should allow it in electron/app/windowSecurity.ts for Stacki's own page only, keeping it refused for previewed sites. Everything else works as before: web links open in the browser, clipboard writing (the copy buttons), fullscreen and notifications are still granted, and previewed sites keep their own links, anchors, scripts and API requests (cookie consent, analytics, a headless CMS), since navigation inside preview iframes is not affected. The rules live in electron/app/windowSecurity.ts as plain functions, with tests for accepted values, every refused kind and malformed input. --- electron/app/windowSecurity.ts | 47 ++++++++++++ electron/main.ts | 37 +++++++++- test/electron/app/windowSecurity.test.js | 94 ++++++++++++++++++++++++ 3 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 electron/app/windowSecurity.ts create mode 100644 test/electron/app/windowSecurity.test.js diff --git a/electron/app/windowSecurity.ts b/electron/app/windowSecurity.ts new file mode 100644 index 000000000..94e70b3ec --- /dev/null +++ b/electron/app/windowSecurity.ts @@ -0,0 +1,47 @@ +// What the app's windows may open, load and be granted. Kept apart from +// main.ts, which wires these rules to electron, so each one is tested alone. + +const BROWSER_PROTOCOLS: ReadonlySet = new Set(['http:', 'https:']); + +// A link the user's browser may open. Any other scheme (file:, javascript:, an +// app's own link) would let a page choose what the operating system launches. +export function isBrowserLink(url: string): boolean { + if (!URL.canParse(url)) { + return false; + } + return BROWSER_PROTOCOLS.has(new URL(url).protocol); +} + +// Whether a navigation of the app's window stays on Stacki's own page: the same +// file in a packaged build, the same origin as the dev server in development. +export function isAppPage(url: string, appUrl: string): boolean { + if (!URL.canParse(url) || !URL.canParse(appUrl)) { + return false; + } + const target = new URL(url); + const app = new URL(appUrl); + if (app.protocol === 'file:') { + return target.protocol === 'file:' && target.pathname === app.pathname; + } + return target.origin === app.origin; +} + +// Permissions that reveal the user or reach their devices. Electron grants every +// request by default, and a previewed site may run scripts the user never read; +// Stacki itself uses none of these, so they are refused for every page. +const PRIVATE_PERMISSIONS: ReadonlySet = new Set([ + 'clipboard-read', + 'deprecated-sync-clipboard-read', + 'display-capture', + 'geolocation', + 'geolocation-approximate', + 'hid', + 'idle-detection', + 'media', + 'serial', + 'usb', +]); + +export function isPrivatePermission(permission: string): boolean { + return PRIVATE_PERMISSIONS.has(permission); +} diff --git a/electron/main.ts b/electron/main.ts index 7406a5ab5..59dddb18c 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -39,6 +39,7 @@ import { net as enet, nativeImage, clipboard, + session, } from 'electron'; import * as path from 'path'; import * as fs from 'fs'; @@ -46,6 +47,7 @@ import { pathToFileURL } from 'url'; import { stopAllServices } from './content/contentConfig'; import * as thumbs from './project/thumbs'; import { openingBounds } from './app/windowBounds'; +import { isAppPage, isBrowserLink, isPrivatePermission } from './app/windowSecurity'; import { registerTerminalHandlers, cleanupTerminals } from './terminal/terminal'; import { watchProject } from './project/projectWatcher'; import { autoUpdater } from 'electron-updater'; @@ -183,6 +185,30 @@ function registerAssetProtocol() { }); } +// Every window shares the default session, so one rule covers the app and its previews. +function refusePrivatePermissions() { + session.defaultSession.setPermissionRequestHandler((_contents, permission, callback) => { + callback(!isPrivatePermission(permission)); + }); + session.defaultSession.setPermissionCheckHandler( + (_contents, permission) => !isPrivatePermission(permission), + ); +} + +// The preload hands Stacki's API to whatever page this window shows, so the +// window never leaves Stacki's own page; web links open in the browser instead. +function keepOnAppPage(window: Window, appUrl: string) { + window.webContents.on('will-navigate', (event) => { + if (isAppPage(event.url, appUrl)) { + return; + } + event.preventDefault(); + if (isBrowserLink(event.url)) { + openExternalLink(event.url); + } + }); +} + async function serveFile(abs: string, request: Request) { const response = await enet.fetch(pathToFileURL(abs).toString(), { headers: request.headers }); // The renderer is a different origin from this scheme, and font loading is @@ -270,9 +296,15 @@ function createWindow() { }); mainWindow.webContents.setWindowOpenHandler(({ url }) => { - openExternalLink(url); + if (isBrowserLink(url)) { + openExternalLink(url); + } return { action: 'deny' }; }); + keepOnAppPage( + mainWindow, + process.env['VITE_DEV_SERVER_URL'] ?? pathToFileURL(RUNTIME_PATHS.rendererIndex).toString(), + ); // Somebody closed the window. Whatever was open is over: the next window is a // fresh start and belongs on the welcome screen, not back in the project this @@ -297,7 +329,7 @@ function openExternalLink(url: string) { } ipcMain.handle('shell:openExternal', async (_event, url) => { - if (/^https?:\/\//.test(url)) { + if (isBrowserLink(url)) { openExternalLink(url); } return { ok: true as const }; @@ -452,6 +484,7 @@ app.whenReady().then( settings = readSettings(); setApplicationIcon(); registerAssetProtocol(); + refusePrivatePermissions(); buildMenu(); createWindow(); autoUpdates.start(); diff --git a/test/electron/app/windowSecurity.test.js b/test/electron/app/windowSecurity.test.js new file mode 100644 index 000000000..3a9aa9117 --- /dev/null +++ b/test/electron/app/windowSecurity.test.js @@ -0,0 +1,94 @@ +// Goal: the app's windows hand the operating system only what the user's +// browser should open, whatever a page asks for. +// Method: run each rule from its built output against accepted values, every +// refused kind, and malformed text, so a loosened rule fails here first. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { + isAppPage, + isBrowserLink, + isPrivatePermission, +} = require('#dist/electron/app/windowSecurity.js'); + +const PACKAGED_PAGE = 'file:///Applications/Stacki.app/Contents/Resources/app.asar/index.html'; +const DEV_PAGE = 'http://localhost:5173/'; + +test('web links open in the browser', () => { + assert.equal(isBrowserLink('https://example.com/page?query=1'), true); + assert.equal(isBrowserLink('http://localhost:4321/'), true); + assert.equal(isBrowserLink('HTTPS://EXAMPLE.COM'), true); +}); + +test('every other link kind is refused', () => { + for (const url of [ + 'file:///Applications/Calculator.app', + 'javascript:alert(1)', + 'mailto:someone@example.com', + 'vscode://file/etc/hosts', + 'data:text/html,

page

', + 'smb://server/share', + ]) { + assert.equal(isBrowserLink(url), false, url); + } +}); + +test('malformed text is refused', () => { + for (const url of ['', 'example.com', '//example.com', 'https//example.com', ' https:']) { + assert.equal(isBrowserLink(url), false, JSON.stringify(url)); + } +}); + +test("the app's window may reload its own page", () => { + assert.equal(isAppPage(PACKAGED_PAGE, PACKAGED_PAGE), true); + assert.equal(isAppPage(`${PACKAGED_PAGE}#/project`, PACKAGED_PAGE), true); + assert.equal(isAppPage(`${PACKAGED_PAGE}?view=code`, PACKAGED_PAGE), true); + assert.equal(isAppPage(DEV_PAGE, DEV_PAGE), true); + assert.equal(isAppPage('http://localhost:5173/src/main.tsx?reload=1', DEV_PAGE), true); +}); + +test("the app's window may not leave its own page", () => { + for (const [url, appUrl] of [ + ['file:///Users/someone/Desktop/index.html', PACKAGED_PAGE], + ['https://example.com/', PACKAGED_PAGE], + ['http://localhost:4321/', DEV_PAGE], + ['https://localhost:5173/', DEV_PAGE], + ['http://localhost:5173.evil.com/', DEV_PAGE], + ['http://localhost.evil.com:5173/', DEV_PAGE], + ['https://example.com/', DEV_PAGE], + ['file:///Applications/Stacki.app/Contents/Resources/app.asar/index.html', DEV_PAGE], + ['not a url', DEV_PAGE], + [DEV_PAGE, 'not a url'], + ]) { + assert.equal(isAppPage(url, appUrl), false, `${url} from ${appUrl}`); + } +}); + +test('permissions that reveal the user or reach their devices are refused', () => { + for (const permission of [ + 'media', + 'geolocation', + 'geolocation-approximate', + 'display-capture', + 'clipboard-read', + 'deprecated-sync-clipboard-read', + 'hid', + 'serial', + 'usb', + 'idle-detection', + ]) { + assert.equal(isPrivatePermission(permission), true, permission); + } +}); + +test('the permissions Stacki relies on, and other harmless ones, stay granted', () => { + for (const permission of [ + 'clipboard-sanitized-write', + 'fullscreen', + 'notifications', + 'pointerLock', + 'openExternal', + ]) { + assert.equal(isPrivatePermission(permission), false, permission); + } +});