security: harden external navigation and preload URL handling

[Mehdi-Bl]

Summary

• validate external URLs in preload bridge and block non-http(s) protocols before shell.openExternal
• add main-process navigation guards using setWindowOpenHandler and will-navigate deny-by-default behavior
• add shared navigation-guard utility with unit tests
• update integration/stress BrowserWindow mocks and test catalog entries

Validation

• npm run lint
• npm test -- --runInBand
• xvfb-run -a npm run e2e:playwright:ci
• npm run stress:metrics
• make sonar

Performance Snapshot

From dist/benchmarks/summary.json latest scenarios:

• fs:getDirectoryTree-large-flat (fileCount=5000): p50=38.814ms, p95=58.732ms, p99=58.732ms
• fs:getDirectoryTree-event-loop-lag: p50=2ms, p95=5ms, p99=5ms

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[sourcery-ai]

Sorry @Mehdi-Bl, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Harden external navigation and preload URL handling with protocol validation

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Add URL validation guards for external navigation in main process and preload bridge
• Implement setWindowOpenHandler and will-navigate event handlers with deny-by-default policy
• Create shared navigation-guard utility validating http(s) protocols and in-app file URLs
• Add comprehensive unit tests for navigation guards and preload external URL blocking

File Changes

1. src/main/index.ts ✨ Enhancement +31/-1

Add navigation guards and external URL handler

src/main/index.ts

---

2. src/main/preload.ts ✨ Enhancement +8/-1

Validate external URLs before shell.openExternal

src/main/preload.ts

---

3. src/main/security/navigation-guard.ts ✨ Enhancement +25/-0

New shared navigation URL validation utilities

src/main/security/navigation-guard.ts

---

View more (6)

4. tests/integration/main-process/handlers.test.ts 🧪 Tests +5/-0

Update BrowserWindow mock with navigation handlers

tests/integration/main-process/handlers.test.ts

---

5. tests/integration/main-process/xml-export-e2e.test.ts 🧪 Tests +5/-0

Update BrowserWindow mock with navigation handlers

tests/integration/main-process/xml-export-e2e.test.ts

---

6. tests/stress/main-process/ipc-latency.stress.test.ts 🧪 Tests +5/-0

Update BrowserWindow mock with navigation handlers

tests/stress/main-process/ipc-latency.stress.test.ts

---

7. tests/unit/main/navigation-guard.test.ts 🧪 Tests +36/-0

New unit tests for navigation guard functions

tests/unit/main/navigation-guard.test.ts

---

8. tests/unit/main/preload.test.ts 🧪 Tests +51/-0

New unit tests for preload external URL guard

tests/unit/main/preload.test.ts

---

9. tests/catalog.md 📝 Documentation +2/-0

Document new navigation and preload guard tests

tests/catalog.md

---

[gemini-code-assist]

Summary of Changes

Hello @Mehdi-Bl, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the application's security posture by implementing comprehensive navigation guards. It ensures that all external URL navigations are strictly limited to safe http and https protocols, preventing potential attacks via malicious schemes. Additionally, it establishes clear rules for in-app navigation, preventing unintended redirects and maintaining application integrity.

Highlights

• External URL Validation: Implemented robust URL validation in the preload script to restrict shell.openExternal to only http(s) protocols, preventing potential malicious scheme exploitation.
• Main-Process Navigation Guards: Introduced main-process navigation guards using Electron's setWindowOpenHandler and will-navigate events to control both external and in-app navigations, ensuring only allowed URLs are processed.
• Shared Navigation Guard Utility: Developed a new shared utility module (navigation-guard.ts) containing core logic for URL validation, complete with dedicated unit tests.
• Test Infrastructure Updates: Updated existing integration and stress tests to properly mock the new Electron BrowserWindow and shell APIs, and added new entries to the test catalog.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• src/main/index.ts
  • Imported shell module from Electron.
  • Imported isAllowedExternalNavigationUrl and isAllowedInAppNavigationUrl from the new security module.
  • Defined openAllowedExternalUrl utility function to safely open external URLs after validation.
  • Configured mainWindow.webContents.setWindowOpenHandler to deny new window openings and redirect to openAllowedExternalUrl.
  • Added a mainWindow.webContents.on('will-navigate') listener to prevent unauthorized in-app navigations and redirect external ones.
• src/main/preload.ts
  • Imported isAllowedExternalNavigationUrl.
  • Modified the electronShellApi.shell.openExternal function to validate URLs using isAllowedExternalNavigationUrl before opening, throwing an error for blocked URLs.
• src/main/security/navigation-guard.ts
  • Added a new file containing isAllowedExternalNavigationUrl to check for http or https protocols.
  • Added isAllowedInAppNavigationUrl to ensure in-app navigations are restricted to about:blank or the main renderer index file.
• tests/catalog.md
  • Added entries for tests/unit/main/navigation-guard.test.ts and tests/unit/main/preload.test.ts.
• tests/integration/main-process/handlers.test.ts
  • Updated Electron BrowserWindow mock to include webContents.setWindowOpenHandler and webContents.on.
  • Added mock for shell.openExternal.
• tests/integration/main-process/xml-export-e2e.test.ts
  • Updated Electron BrowserWindow mock to include webContents.setWindowOpenHandler and webContents.on.
  • Added mock for shell.openExternal.
• tests/stress/main-process/ipc-latency.stress.test.ts
  • Updated Electron BrowserWindow mock to include webContents.setWindowOpenHandler and webContents.on.
  • Added mock for shell.openExternal.
• tests/unit/main/navigation-guard.test.ts
  • Added a new file with unit tests for isAllowedExternalNavigationUrl and isAllowedInAppNavigationUrl, covering various valid and invalid URL scenarios.
• tests/unit/main/preload.test.ts
  • Added a new file with unit tests for the preload script's shell.openExternal guard, verifying it allows https and blocks non-http(s) URLs.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues found.

Scanned Files

None

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

✅ 1. about:blank bypasses will-navigate guard 🐞 Bug ⛨ Security

Description

isAllowedInAppNavigationUrl unconditionally returns true for 'about:blank', so the
will-navigate handler never calls event.preventDefault() for that URL. Any JavaScript in the
renderer can navigate the main BrowserWindow to about:blank (e.g., `window.location =
'about:blank'`), completely erasing the application UI — a denial-of-service against the app's own
content.

Code

src/main/security/navigation-guard.ts[R10-15]

+const isAboutBlank = (url: string): boolean => url === 'about:blank';
+
+export const isAllowedInAppNavigationUrl = (url: string, rendererIndexUrl: string): boolean => {
+  if (isAboutBlank(url)) {
+    return true;
+  }

Evidence

Lines 10–15 of navigation-guard.ts make isAllowedInAppNavigationUrl return true immediately for
about:blank. The will-navigate handler in index.ts only calls event.preventDefault() when this
function returns false — so about:blank navigation is never blocked. The setWindowOpenHandler
already denies all popups unconditionally (line 183), so there is no legitimate path through which
about:blank would appear in a will-navigate event on the main window. The unit test at
navigation-guard.test.ts line 26 confirms and locks in this behavior.

src/main/security/navigation-guard.ts[10-15]
src/main/index.ts[186-193]
src/main/index.ts[181-184]
tests/unit/main/navigation-guard.test.ts[26-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `isAllowedInAppNavigationUrl` function unconditionally allows `about:blank`, which is used as the gate in the `will-navigate` handler. This means any renderer-side code can navigate the main BrowserWindow to `about:blank`, erasing the entire application UI.
## Issue Context
- `setWindowOpenHandler` already returns `{ action: 'deny' }` for all popups, so `about:blank` will never legitimately appear in a `will-navigate` event on the main window.
- The `about:blank` allowance has no valid use case in the `will-navigate` guard.
## Fix Focus Areas
- src/main/security/navigation-guard.ts[10-15]
- tests/unit/main/navigation-guard.test.ts[26-26]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

✅ 2. Optional chaining silently skips security guards 🐞 Bug ⛯ Reliability

Description

Both setWindowOpenHandler and the will-navigate event listener are registered using optional
chaining (?.). If either method is absent — due to a version mismatch, future API change, or
environment issue — the entire security boundary is silently skipped with no error or warning
logged, leaving the application completely unguarded.

Code

src/main/index.ts[R181-186]

+  mainWindow.webContents.setWindowOpenHandler?.(({ url }) => {
+    openAllowedExternalUrl(url);
+    return { action: 'deny' };
+  });
+
+  mainWindow.webContents.on?.('will-navigate', (event, url) => {

Evidence

Lines 181 and 186 use ?. on setWindowOpenHandler and on, both of which are always defined on
Electron's WebContents (on is inherited from Node's EventEmitter). The inconsistency is
visible on line 179 where mainWindow.setMenu(null) is called on the same object without optional
chaining. The integration test mocks explicitly add both methods (handlers.test.ts lines 39–40),
confirming the code depends on their presence — but ?. hides this dependency and makes a silent
no-op possible.

src/main/index.ts[181-186]
src/main/index.ts[179-179]
tests/integration/main-process/handlers.test.ts[39-40]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Security-critical event handlers (`setWindowOpenHandler` and `will-navigate`) are registered with optional chaining (`?.`). If either method is absent, the guards are silently not registered — no error, no warning, no log.
## Issue Context
- `setWindowOpenHandler` and `on` are always defined on Electron's `WebContents`.
- The same `mainWindow` object calls `setMenu(null)` without optional chaining on line 179, showing the inconsistency.
- The integration test mocks explicitly define both methods, confirming they are expected to be present.
## Fix Focus Areas
- src/main/index.ts[181-181]
- src/main/index.ts[186-186]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

✅ 3. Case-sensitive pathname comparison breaks on Windows 🐞 Bug ⛯ Reliability

Description

The in-app navigation check compares file URL pathnames with strict equality (===). On Windows,
filesystem paths are case-insensitive, so a drive-letter casing difference between the
startup-computed RENDERER_INDEX_URL and the URL reported by Electron's will-navigate event would
cause the guard to incorrectly reject the renderer's own index URL, silently blocking legitimate
navigation.

Code

src/main/security/navigation-guard.ts[21]

+    return targetUrl.protocol === 'file:' && targetUrl.pathname === rendererUrl.pathname;

Evidence

RENDERER_INDEX_URL is computed once at startup via pathToFileURL(RENDERER_INDEX_PATH) (index.ts
line 129). If Electron reports the will-navigate URL with a different drive-letter casing (e.g.,
file:///c:/... vs file:///C:/...), the strict === comparison on navigation-guard.ts line 21
fails. The guard then calls event.preventDefault() and passes the URL to openAllowedExternalUrl,
which also rejects it (it's file:, not http/https), silently blocking the navigation. The unit
tests only cover Linux-style paths and provide no coverage for Windows drive-letter casing.

src/main/security/navigation-guard.ts[21-21]
src/main/index.ts[129-129]
tests/unit/main/navigation-guard.test.ts[10-11]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The pathname comparison in `isAllowedInAppNavigationUrl` uses strict equality (`===`), which is case-sensitive. On Windows, path casing differences between the startup-computed renderer URL and the URL reported by Electron's `will-navigate` event can cause the guard to incorrectly block legitimate navigation to the renderer's own index.
## Issue Context
- `RENDERER_INDEX_URL` is computed once at startup from `pathToFileURL(RENDERER_INDEX_PATH)` in index.ts.
- Windows file paths are case-insensitive; drive-letter casing (e.g., `C:` vs `c:`) can differ between path resolution contexts.
- The unit tests only cover Linux-style paths and do not test Windows drive-letter casing.
## Fix Focus Areas
- src/main/security/navigation-guard.ts[21-21]
- tests/unit/main/navigation-guard.test.ts[21-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[gemini-code-assist]

Code Review

This pull request introduces significant security hardening for external navigation and URL handling, which is a great improvement. The changes correctly validate external URLs in the preload bridge, blocking non-HTTP(S) protocols. In the main process, navigation is now guarded by setWindowOpenHandler and a will-navigate listener, implementing a deny-by-default behavior which is a security best practice for Electron applications. The new shared navigation-guard utility is well-designed and thoroughly tested. The updates to mocks and test catalogs are also correctly handled. Overall, the implementation is solid and effectively mitigates risks associated with URL handling and navigation.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
84.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud