Harden Electron Navigation, Permission Gates, and Renderer Content Security Policy in Vigil - #2
Open
mozluk wants to merge 2 commits into
Open
Harden Electron Navigation, Permission Gates, and Renderer Content Security Policy in Vigil#2mozluk wants to merge 2 commits into
mozluk wants to merge 2 commits into
Conversation
…curity Policy in Vigil
### Description
This pull request resolves High severity client-side security and privilege escalation findings identified during the workspace audit for the `vigil` desktop application. Previously, arbitrary protocol schemes could reach OS-level handlers via unconstrained `shell.openExternal` calls, renderer sessions lacked a sandbox boundary, permissions were open to default handling, and the renderer omitted a Content Security Policy (CSP). This PR enforces an external URL protocol allowlist, sandboxes the renderer process, registers fail-closed permission and navigation guards across all WebContents, and adds a strict meta CSP.
### Key Changes & Remediations
#### 1. IPC & External Navigation Hardening (`electron/main.ts`)
* **Protocol Allowlist:** Restricted `shell.openExternal` strictly to `ALLOWED_EXTERNAL_PROTOCOLS` (`http:`, `https:`). Non-web schemes (such as `file:`, `ms-msdt:`, or custom protocols) are blocked and warned rather than handed to the host OS.
* **Origin-Checked Internal Navigation:** Added `isInternalUrl` origin verification for `will-navigate` and `will-redirect` to ensure unhandled window navigations cannot load third-party origins into a window with bridge access.
* **Global Navigation Guards:** Enforced `setWindowOpenHandler` on both `mainWindow` and globally within `app.on("web-contents-created")` to intercept and deny popup creation while safely delegating external links to the default browser.
* **Webview Denial:** Intercepted and cancelled `will-attach-webview` events to disallow untrusted guest contexts.
#### 2. Sandbox & Permissions (`electron/main.ts`)
* **Process Sandboxing:** Enabled `sandbox: true` under `BrowserWindow.webPreferences` to restrict OS-level syscall access from compromised renderers.
* **Deny-by-Default Permissions:** Implemented `session.setPermissionRequestHandler` to automatically reject all browser-level permission inquiries (camera, microphone, geolocation, etc.).
#### 3. Renderer Content Security Policy (`electron/renderer/index.html`)
* **Strict Meta CSP:** Introduced a restrictive CSP meta tag enforcing `default-src 'self'`, `object-src 'none'`, `frame-src 'none'`, and `base-uri 'self'` to block remote script injections and untrusted framing[cite: 12].
### How to Review
1. **Scheme Validation:** Check `isSafeExternalUrl` in `electron/main.ts` to confirm only `http:` and `https:` are permitted.
2. **WebContents Interception:** Review `app.on("web-contents-created")` to verify both webview attachment and window creation handlers fail-closed.
3. **CSP Directives:** Inspect `<meta http-equiv="Content-Security-Policy">` in `electron/renderer/index.html` to ensure dangerous sinks (`object-src`, `frame-src`) are disabled[cite: 12].
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 21210cc. Configure here.
Author
|
Good catch. The permission request handler now maintains a strict allowlist ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Description
This pull request resolves High severity client-side security and privilege escalation findings identified during the workspace audit for the
vigildesktop application. Previously, arbitrary protocol schemes could reach OS-level handlers via unconstrainedshell.openExternalcalls, renderer sessions lacked a sandbox boundary, permissions were open to default handling, and the renderer omitted a Content Security Policy (CSP). This PR enforces an external URL protocol allowlist, sandboxes the renderer process, registers fail-closed permission and navigation guards across all WebContents, and adds a strict meta CSP.Key Changes & Remediations
1. IPC & External Navigation Hardening (
electron/main.ts)shell.openExternalstrictly toALLOWED_EXTERNAL_PROTOCOLS(http:,https:). Non-web schemes (such asfile:,ms-msdt:, or custom protocols) are blocked and warned rather than handed to the host OS.isInternalUrlorigin verification forwill-navigateandwill-redirectto ensure unhandled window navigations cannot load third-party origins into a window with bridge access.setWindowOpenHandleron bothmainWindowand globally withinapp.on("web-contents-created")to intercept and deny popup creation while safely delegating external links to the default browser.will-attach-webviewevents to disallow untrusted guest contexts.2. Sandbox & Permissions (
electron/main.ts)sandbox: trueunderBrowserWindow.webPreferencesto restrict OS-level syscall access from compromised renderers.session.setPermissionRequestHandlerto automatically reject all browser-level permission inquiries (camera, microphone, geolocation, etc.).3. Renderer Content Security Policy (
electron/renderer/index.html)default-src 'self',object-src 'none',frame-src 'none', andbase-uri 'self'to block remote script injections and untrusted framing[cite: 12].How to Review
isSafeExternalUrlinelectron/main.tsto confirm onlyhttp:andhttps:are permitted.app.on("web-contents-created")to verify both webview attachment and window creation handlers fail-closed.<meta http-equiv="Content-Security-Policy">inelectron/renderer/index.htmlto ensure dangerous sinks (object-src,frame-src) are disabled[cite: 12].Note
Medium Risk
Security-critical Electron main-process behavior changes (sandbox, navigation, external URLs) could break dev HMR, clipboard onboarding, or link handling if misconfigured; CSP may block unexpected asset or API hosts.
Overview
Hardens the Vigil Electron shell against navigation abuse, privilege escalation, and renderer injection.
Main process (
electron/main.ts):shell.openExternaland IPCopen-externalnow only accepthttp:/https:URLs viaisSafeExternalUrl; invalid schemes are logged and ignored.store-sessionrejects non-string or empty tokens. The renderer runs withsandbox: true. Popups are denied and external links open in the system browser;will-navigate/will-redirectstay in-app only whenisInternalUrlmatches the dev server origin orfile:. A permission handler allows only clipboard read/write for onboarding and denies everything else. Globalweb-contents-createdhandlers block<webview>attachment and repeat the popup policy.Renderer (
renderer/index.html): Adds a meta Content-Security-Policy (needed for packagedfile://loads) that limits scripts, styles, fonts, images, and network connections while locking down objects, frames, forms, and base URLs.Session storage, notifications, and window lifecycle behavior are unchanged aside from clearer error logging when decrypting the stored token.
Reviewed by Cursor Bugbot for commit cf1c7af. Bugbot is set up for automated code reviews on this repo. Configure here.