Skip to content

fix(desktop): renderer boot error boundary and electron hardening#2816

Open
charlesvien wants to merge 1 commit into
refactor/arch-di-boundary-fixesfrom
fix/renderer-boot-and-electron-security
Open

fix(desktop): renderer boot error boundary and electron hardening#2816
charlesvien wants to merge 1 commit into
refactor/arch-di-boundary-fixesfrom
fix/renderer-boot-and-electron-security

Conversation

@charlesvien

@charlesvien charlesvien commented Jun 21, 2026

Copy link
Copy Markdown
Member

Problem

Two reliability and security gaps from the package split surfaced, deliberately split out from #2813 because they touch runtime-sensitive Electron config and warrant their own review.

  1. If boot(), a module-scope import or an Inversify resolution throws during renderer startup, the user gets a blank window with no error and no recovery short of force-quitting.
  2. The main window runs with nodeIntegration: true, which applies in production, not just dev.

Changes

  • Add a BootErrorBoundary (apps/code/src/renderer/components/BootErrorBoundary.tsx) around the root React tree, plus a try/catch around the synchronous bootstrap in main.tsx and a .catch on the async boot(container). Any of those failing now renders a minimal, theme-independent error screen with a Reload button instead of a blank window, and logs the failure.
  • Set webPreferences.nodeIntegration: false on the main window. contextIsolation: true is already set (so the renderer main world never had Node anyway) and the renderer is Vite-bundled with zero node:*/require usage in the main world, so this is defense in depth with no functional change.

Scope note: the dev-only webSecurity: false override is left in place. It does not affect production (production already runs with webSecurity: true) and flipping it risks breaking dev resource loading. The remaining dev/prod parity gap is deferred.

How did you test this?

All run locally in the branch worktree:

  • pnpm typecheck: 22/22 packages pass
  • pnpm lint: clean
  • pnpm test: apps/code 151 pass
  • node scripts/check-host-boundaries.mjs: no new violations

Not yet done: a manual smoke test of the running app (dev and packaged). The nodeIntegration flip is low risk given contextIsolation: true, but it changes runtime Electron config, so a human should confirm the window still loads and agents/terminals work before merge. This PR is left as a draft for that reason.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

@charlesvien charlesvien changed the title harden renderer boot and electron security fix(desktop): renderer boot error boundary and electron hardening Jun 21, 2026

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit d311bc4.

@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
apps/code/src/renderer/main.tsx:97-99
When `boot(container)` rejects, the `.catch()` only logs the error but the already-mounted React tree continues to run. Users either see a broken app or, if the boot failure cascades into a render crash, a `BootErrorBoundary`-generated message that doesn't match the root cause. The synchronous failure path (the outer `catch`) correctly renders `BootErrorScreen`; the async path should do the same.

```suggestion
  boot(container).catch((error: unknown) => {
    bootLog.error("Renderer boot sequence failed", error);
    root.render(<BootErrorScreen error={error} />);
  });
```

Reviews (1): Last reviewed commit: "harden renderer boot and electron securi..." | Re-trigger Greptile

Comment thread apps/code/src/renderer/main.tsx
@charlesvien charlesvien force-pushed the refactor/arch-di-boundary-fixes branch from a8670ec to 9943d59 Compare June 21, 2026 00:30
@charlesvien charlesvien force-pushed the fix/renderer-boot-and-electron-security branch from 7822906 to 6c5ac61 Compare June 21, 2026 00:30
@charlesvien charlesvien marked this pull request as ready for review June 21, 2026 00:31
@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Reviews (2): Last reviewed commit: "harden renderer boot and electron securi..." | Re-trigger Greptile

@charlesvien charlesvien added the Stamphog This will request an autostamp by stamphog on small changes label Jun 21, 2026
github-actions[bot]
github-actions Bot previously approved these changes Jun 21, 2026

@github-actions github-actions Bot 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.

No showstoppers. Security hardening (nodeIntegration: false) is correct given contextIsolation: true and a preload script were already in place. The boot error boundary and async failure handling are well-structured fixes. The resolved bot comment was addressed.

@charlesvien charlesvien force-pushed the fix/renderer-boot-and-electron-security branch from 6c5ac61 to 8272637 Compare June 21, 2026 05:11
@charlesvien charlesvien force-pushed the refactor/arch-di-boundary-fixes branch from 9943d59 to 908dceb Compare June 21, 2026 05:11
@github-actions github-actions Bot dismissed their stale review June 21, 2026 05:12

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

github-actions[bot]
github-actions Bot previously approved these changes Jun 21, 2026

@github-actions github-actions Bot 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.

No showstoppers. The nodeIntegration: false hardening is correct given contextIsolation: true and a preload script are already in place. The boot error boundary is well-structured, and the previously flagged greptile concern about the async boot() failure path not rendering the error screen has been addressed in the current diff.

@charlesvien charlesvien changed the base branch from refactor/arch-di-boundary-fixes to graphite-base/2816 June 21, 2026 06:07
@charlesvien charlesvien force-pushed the fix/renderer-boot-and-electron-security branch from 8272637 to 8ee15e2 Compare June 21, 2026 06:08
@charlesvien charlesvien changed the base branch from graphite-base/2816 to refactor/arch-di-boundary-fixes June 21, 2026 06:08
@github-actions github-actions Bot dismissed their stale review June 21, 2026 06:08

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

github-actions[bot]
github-actions Bot previously approved these changes Jun 21, 2026

@github-actions github-actions Bot 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.

Security hardening (nodeIntegration: false) is correct with contextIsolation: true and a preload script already in place. The error boundary is well-structured, and the previously flagged async boot() failure path now correctly renders the error screen.

@charlesvien charlesvien force-pushed the fix/renderer-boot-and-electron-security branch from 8ee15e2 to 769293d Compare June 21, 2026 06:32
@github-actions github-actions Bot dismissed their stale review June 21, 2026 06:32

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

github-actions[bot]
github-actions Bot previously approved these changes Jun 21, 2026

@github-actions github-actions Bot 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.

Security hardening (nodeIntegration: false) is correct with contextIsolation: true and a preload script already in place. The error boundary is well-structured, async boot failure now properly renders the error screen, and the greptile-flagged concern is addressed.

@charlesvien charlesvien force-pushed the fix/renderer-boot-and-electron-security branch from 769293d to d311bc4 Compare June 21, 2026 08:53
@charlesvien charlesvien force-pushed the refactor/arch-di-boundary-fixes branch from 9e7ce55 to 800c58e Compare June 21, 2026 08:53
@github-actions github-actions Bot dismissed their stale review June 21, 2026 08:53

New commits pushed (delta classified non_linear_history) — stamphog approval dismissed; re-review running automatically.

@github-actions github-actions Bot 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.

Security hardening (nodeIntegration: false) is correct — contextIsolation: true and a preload script were already in place, making this a fix to a contradictory config. The error boundary and async boot failure handling are well-structured additions with proper test coverage, and the greptile-flagged async path concern is addressed.

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

Labels

Stamphog This will request an autostamp by stamphog on small changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant