Skip to content

fix(security): 2 improvements across 2 files#513

Open
tomaioo wants to merge 2 commits into
lacymorrow:mainfrom
tomaioo:fix/security/unsafe-content-security-policy-with-unsa
Open

fix(security): 2 improvements across 2 files#513
tomaioo wants to merge 2 commits into
lacymorrow:mainfrom
tomaioo:fix/security/unsafe-content-security-policy-with-unsa

Conversation

@tomaioo

@tomaioo tomaioo commented May 21, 2026

Copy link
Copy Markdown

Summary

fix(security): 2 improvements across 2 files

Problem

Severity: High | File: src/renderer/chooser.html:L6

The chooser.html file uses a very permissive Content-Security-Policy that allows 'unsafe-inline' for scripts and styles, and 'unsafe-eval' for scripts. This significantly weakens the security benefits of CSP and could allow XSS attacks if any user-controlled data reaches the renderer.

Solution

Remove 'unsafe-inline' and 'unsafe-eval' from the CSP. Use nonce or hash-based CSP for inline scripts. If 'unsafe-eval' is needed for a specific library, consider using a safer alternative or sandboxing that code.

Changes

  • src/renderer/chooser.html (modified)
  • src/renderer/index.html (modified)

tomaioo added 2 commits May 20, 2026 23:11
- Security: Unsafe Content Security Policy with 'unsafe-inline' and 'unsafe-eval'
- Security: Unsafe Content Security Policy with 'unsafe-inline' and 'unsafe-eval'

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
- Security: Unsafe Content Security Policy with 'unsafe-inline' and 'unsafe-eval'
- Security: Unsafe Content Security Policy with 'unsafe-inline' and 'unsafe-eval'

Signed-off-by: tomaioo <203048277+tomaioo@users.noreply.github.com>
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@lacymorrow

Copy link
Copy Markdown
Owner

Reviewed the CSP changes. A couple of notes before merging:

feather-icons eval dependency: vendor/feather/feather-icons.min.js contains Function('return this')() in its UMD global detection. In Electron renderers with node integration enabled, typeof module === 'object' is true so feather uses the CommonJS path and this eval code is likely bypassed. But it's worth testing to confirm the renderer loads correctly after this change.

chooser.html script-src change: The new CSP adds https: to script-src in chooser.html (script-src 'self' https:). That actually broadens script loading to any HTTPS URL, which is more permissive than intended for a local renderer. Recommend using just 'self' there to match index.html.

Suggest testing the renderer (both the main window and the crosshair chooser) with these CSP changes before merging.

@lacymorrow

Copy link
Copy Markdown
Owner

Update on my earlier concern: the Function('return this')() call in feather-icons is in a fallback chain:

t(typeof globalThis === 'object' && globalThis) ||
t(typeof window === 'object' && window) ||
t(typeof self === 'object' && self) ||
Function('return this')()

In any modern Chromium/Electron renderer, globalThis resolves first and short-circuits the rest. The eval path is never reached. Removing unsafe-eval is safe.

One remaining point: the new chooser.html CSP uses script-src 'self' https: which allows scripts from any HTTPS origin. That's wider than needed — script-src 'self' would match index.html and be more consistent. Not a blocker since no external scripts are loaded in practice.

Overall this looks good to merge as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants