Skip to content

Secure About window with contextBridge preload instead of nodeIntegration#14

Merged
Samuel3 merged 7 commits into
feature/add-testsfrom
copilot/sub-pr-5-585b9761-d211-4735-b740-15f4db4c4681
Jun 1, 2026
Merged

Secure About window with contextBridge preload instead of nodeIntegration#14
Samuel3 merged 7 commits into
feature/add-testsfrom
copilot/sub-pr-5-585b9761-d211-4735-b740-15f4db4c4681

Conversation

Copilot AI commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

The About window was created with nodeIntegration: true and contextIsolation: false, weakening renderer isolation. This replaces it with a secure preload bridge following the same pattern already used by the Release Notes window.

Changes

  • MenuTemplate.js: Set nodeIntegration: false, contextIsolation: true, and wire in about-preload.js
  • about-preload.js (new): Exposes version, i18n, and IPC progress listener via contextBridge; locale is whitelisted (en, de) before use in file path to prevent path traversal
  • About.js: Replace all require('electron'), require('../js/i18n'), and require('../package.json') with window.aboutAPI.*
  • about.html: Replace inline require() script blocks with regular <script src> tags
// Before
aboutWindow = new BrowserWindow({
  webPreferences: { nodeIntegration: true, contextIsolation: false }
});

// After
aboutWindow = new BrowserWindow({
  webPreferences: {
    nodeIntegration: false,
    contextIsolation: true,
    preload: path.join(__dirname, 'about-preload.js')
  }
});

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits March 2, 2026 19:26
…ndow

Co-authored-by: Samuel3 <15921086+Samuel3@users.noreply.github.com>
Co-authored-by: Samuel3 <15921086+Samuel3@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix tests based on feedback from PR #5 review Secure About window with contextBridge preload instead of nodeIntegration Mar 2, 2026
@Samuel3 Samuel3 requested a review from Copilot March 2, 2026 19:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Electron “About” window by removing renderer Node.js access (nodeIntegration) and switching to a contextBridge-based preload API, aligning it with the existing secure approach used by the Release Notes window.

Changes:

  • Updates the About window BrowserWindow config to nodeIntegration: false, contextIsolation: true, and adds an about-preload.js preload.
  • Adds about-preload.js to expose a minimal window.aboutAPI surface (version, i18n lookup, progress listener).
  • Refactors About.js and about.html to remove require() usage in the renderer and load scripts via <script src>.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
app/sites/about.html Switches renderer bootstrapping from inline require() to static script tags.
app/js/about-preload.js New preload bridge exposing version/i18n/progress via contextBridge.
app/js/MenuTemplate.js Secures About window webPreferences and wires in the new preload.
app/js/About.js Migrates renderer code to use window.aboutAPI instead of Node/Electron requires.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/js/about-preload.js Outdated
Comment thread app/js/about-preload.js Outdated
Comment thread app/js/about-preload.js Outdated
@Samuel3

Samuel3 commented Mar 30, 2026

Copy link
Copy Markdown
Owner

@copilot please fix findings and solve merge conflict

Copilot AI commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please fix findings and solve merge conflict

Done in commit acc1b3f. Merged feature/add-tests into this branch, resolved the about-preload.js conflict by adopting the base branch's require('./i18n') approach, and applied all three review fixes: locale type validation, onProgress unsubscribe, and deduplication of locale logic.

Copilot AI requested a review from Samuel3 March 30, 2026 17:52
@Samuel3

Samuel3 commented Mar 30, 2026

Copy link
Copy Markdown
Owner

@copilot please fix merge conflict

…kage-lock.json conflicts

Agent-Logs-Url: https://github.com/Samuel3/LetterCreator/sessions/e4012f10-97cd-47f1-aef5-c14bf1fc407f

Co-authored-by: Samuel3 <15921086+Samuel3@users.noreply.github.com>

Copilot AI commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please fix merge conflict

Resolved in commit 042c4ee. Merged the latest feature/add-tests commits (Store.js callback guard + new store tests), resolving conflicts in about-preload.js (keeping the unsubscribe function and correct ../package.json path) and package-lock.json (taking the base branch version). All 50 tests pass.

@Samuel3

Samuel3 commented Mar 30, 2026

Copy link
Copy Markdown
Owner

@copilot please fix merge conflict in app/js/about-preload.js

Co-authored-by: Samuel3 <15921086+Samuel3@users.noreply.github.com>

Copilot AI commented Mar 30, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please fix merge conflict in app/js/about-preload.js

Resolved in commit 87e18d7. Created a proper two-parent merge commit incorporating feature/add-tests. The about-preload.js conflict is resolved by aligning with the base branch's ../../package.json path while keeping the onProgress unsubscribe improvement. All 50 tests pass.

@Samuel3 Samuel3 merged commit 87e18d7 into feature/add-tests Jun 1, 2026
2 checks passed
@Samuel3 Samuel3 deleted the copilot/sub-pr-5-585b9761-d211-4735-b740-15f4db4c4681 branch June 1, 2026 10:07
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