Skip to content

fix(security): harden Electron IPC and shell.openExternal against abuse#162

Open
EnragedAntelope wants to merge 1 commit intoLuqP2:mainfrom
EnragedAntelope:claude/security-review-fixes-Gb3tS
Open

fix(security): harden Electron IPC and shell.openExternal against abuse#162
EnragedAntelope wants to merge 1 commit intoLuqP2:mainfrom
EnragedAntelope:claude/security-review-fixes-Gb3tS

Conversation

@EnragedAntelope
Copy link

I had Claude review for security. I could not build like you can so test before full implementation:

  • Validate URL protocol in shell.openExternal to only allow http/https, preventing malicious protocol handler invocation (e.g. file://, smb://)
  • Add will-navigate handler to block renderer navigation to external URLs
  • Add path validation to write-file IPC handler (was previously unrestricted, allowing writes to arbitrary filesystem locations)
  • Restrict open-cache-location handler to app userData directory
image

- Validate URL protocol in shell.openExternal to only allow http/https,
  preventing malicious protocol handler invocation (e.g. file://, smb://)
- Add will-navigate handler to block renderer navigation to external URLs
- Add path validation to write-file IPC handler (was previously unrestricted,
  allowing writes to arbitrary filesystem locations)
- Restrict open-cache-location handler to app userData directory

https://claude.ai/code/session_01HWTeMeasbEW9JPY2d1dcp2
@LuqP2
Copy link
Owner

LuqP2 commented Mar 11, 2026

Thanks for this, and sorry for the delay on my side. I reviewed it and it looks solid. I’m merging it into the next release branch. Thanks!!

@LuqP2
Copy link
Owner

LuqP2 commented Mar 11, 2026

Quick follow-up: I realized that the new write-file restriction will break export to folders that are not already indexed. So I’m holding off on merging until I patch that flow properly. The hardening itself still looks good, but, but I need to make sure export remains authorized when the user explicitly picks a location through the native dialog.

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.

3 participants