[PM-41251] Disable nodeIntegration, add preload + IPC layer

[BTreston]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-41251

📔 Objective

Disables Electron's nodeIntegration in the renderer process and establishes a secure IPC boundary. Migrates services that rely on node out of the renderer process and into the main process. These services are now called from the renderer process via the IPC layer.

Big changes:

• nodeIntegration: false renderer no longer has direct Node.js access
• adds preload.ts which exposes a typed window.ipc API via electron's contextBridge so renderer can call its old services
• all services (auth, sync, secure storage, crypto, API) now instantiated in the main process
• adds IPC handlers registered via ipcMain.handle, which is a wrappper around the electron API w/ some extra error handling.
• added more platform utils for main process
• added webpack.preload.mjs config for preload bundle
• adds and updates context and documentation for claude.
• removes a lot of unused code left over the the jslib era of DC.

The renderer process should now be fully sandboxed. all privileged operations that require node run in main and are exposed only through whitelisted IPC channels.

📸 Screenshots

[BTreston]

This all works, but its not perfect. It yanks everything that relies on node out of the renderer process in a pretty unceremonious manner. Theres probably room for improvement but this is the gist what we do in clients... Just looking for general feedback, this minimally resolves the linked issue. I can iterate on any async feedback once I am back from PTO so no rush here.

The bunch of comments are for future me, who will thank me when reading them in 2 weeks. 🤝

[BTreston]

@eliykat curious on your opinion on this pattern. I don't know if I am a fan of the IPC calls as they're done here, maybe we want an abstraction to sit between the IPC calls and these renderer call sites that we can DI into the components. Maybe each main process service that has an IPC channel, similar to what ended up in src-gui/services/electron/electronRendererSecureStorage.service.ts?

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (951d96c) to head (2c43b38).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@     Coverage Diff      @@
##   main   #1212   +/-   ##
============================
============================

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[BTreston]

While this addresses the ticket, this leaves DC in a rather fragile state. GUI build will bork if you accidentally pull in any node api in the renderer process, which is relatively easy to do accidentally (especially with dc_native being a straight up node binary that we manually vendor in) and end up with an arcane error message. Claude is decently good at following the runtime hierarchy based on what comes out of the build to find what node api is causing the issue, but its not a great dev-ex... I'm not sure if that's a big problem or if that's just how it is working with sandboxing in electron.

[BTreston]

While this addresses the ticket, this leaves DC in a rather fragile state. GUI build will bork if you accidentally pull in any node api in the renderer process, which is relatively easy to do accidentally (especially with dc_native being a straight up node binary that we manually vendor in) and end up with an arcane error message. Claude is decently good at following the runtime hierarchy based on what comes out of the build to find what node api is causing the issue, but its not a great dev-ex... I'm not sure if that's a big problem or if that's just how it is working with sandboxing in electron.

I have addressed this with a new linter rule preventing node API and electron imports in the renderer. I think that should take care of this concern

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the one commit since the last pass (2c43b38), which makes WindowMain.registerAppHandlers() return boolean and has Main.bootstrap() return early when the single-instance lock is lost. That closes the previously open finding: a second instance now bails before stateService.init() runs the real StateMigrationService and before createWindowWhenReady(), so it can no longer race keychain migration or open a window in a process that has already called app.quit(). The App Store / Snap Store paths still skip the lock and return true, so their behaviour is unchanged. Also re-verified the dependency surface: the only manifest change is the webpack-node-externals devDependency removal, package-lock.json is regenerated, and no code, webpack, or CI reference to the package remains.

Code Review Details

No new findings in this pass.

Dependency Changes

Package	Change	Ecosystem
webpack-node-externals	Removed	npm

[BTreston]

@coltonhurst If you're available to give the just the IPC implementation here some feedback it would be much appreciated, as I've been told you have some experience with it.

[sven-bitwarden]

Direction seems sound and I haven't seen anything that I could identify as alarming. My only question: I assume the IPC layer entirely is managed by electron, so we have nothing to handle on our side in terms of transient failures (either for the connection as a whole, or on a per-message basis) between the renderer<->main process?

[BTreston]

Direction seems sound and I haven't seen anything that I could identify as alarming. My only question: I assume the IPC layer entirely is managed by electron, so we have nothing to handle on our side in terms of transient failures (either for the connection as a whole, or on a per-message basis) between the renderer<->main process?

Yes, electron manages the IPC wrt the messaging between renderer and main. The only thing we really have to concern ourselves with is the handle() function done in src-gui/main.ts to make sure non-error result objects (like failure messges from sync services) that come across the boundary are treated like errors.

[jrmccannon]

As much as I can tell it looks good.