feat: load OS CA certificates at startup for corporate environments#3283
feat: load OS CA certificates at startup for corporate environments#3283jeanfbrito merged 4 commits intodevfrom
Conversation
Document the overridden-settings.json configuration for bypassing SSL certificate validation in Outlook calendar sync, covering file paths per platform, default behavior, and usage scenarios.
Use Node.js 24's native tls.setDefaultCACertificates() to combine system-trusted and bundled Mozilla CAs at boot time. This allows all Node.js HTTPS connections (Outlook sync, version checks, etc.) to trust certificates from the OS trust store without disabling TLS verification. Enabled by default, configurable via useSystemCertificates in overridden-settings.json. The existing allowInsecureOutlookConnections remains as a last-resort fallback.
Merge the separate outlook-calendar-insecure-connections.md and system-ca-certificates.md into a unified corporate-certificate-configuration.md covering both approaches (system CAs and insecure bypass) in one place.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
WalkthroughLoad OS trust-store CA certificates at startup (optional via Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main (startup)
participant SysCert as systemCertificates.applySystemCertificates()
participant NodeTLS as Node TLS (tls.getCACertificates / tls.setDefaultCACertificates)
participant Data as App Data Dispatcher
participant Outlook as Outlook Sync
Main->>SysCert: invoke applySystemCertificates()
SysCert->>NodeTLS: getCACertificates('system')
alt system CAs found
NodeTLS-->>SysCert: list of system CAs
SysCert->>NodeTLS: setDefaultCACertificates(merged CAs)
SysCert-->>Main: status { applied: true, certCount }
else none or disabled
NodeTLS-->>SysCert: no system CAs / disabled
SysCert-->>Main: status { applied: false, certCount: 0 }
end
Main->>Data: dispatch allowInsecureOutlookConnections (normalized)
Data-->>Main: if insecure && status.applied -> log warning
Main->>Outlook: Outlook sync uses Node TLS defaults for connection verification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/corporate-certificate-configuration.md`:
- Line 72: The sentence is misleading: the loaders in src/systemCertificates.ts
and src/app/main/data.ts look for a settings file placed alongside app.asar
(next to the app bundle), not generically “outside the app bundle” for
system-wide deployment; update the docs line to explicitly say the override file
must be located beside app.asar in the app directory (alongside the app bundle)
so admins know where the loaders (src/systemCertificates.ts and
src/app/main/data.ts) will read it from.
In `@src/systemCertificates.ts`:
- Around line 17-43: readUseSystemCertificatesSetting currently returns based on
the first file that contains useSystemCertificates, which conflicts with
src/app/main/data.ts where overrides are merged as
{...userDataOverriddenSettings, ...appAsarOverriddenSettings} (app.asar wins).
Change readUseSystemCertificatesSetting to read and parse both paths in
locations, merge them with the same precedence (userData first, then appAsar
overrides), then evaluate the resulting merged.useSystemCertificates value
(treating missing or non-false as true) and return that. Update references to
the locations array and the function readUseSystemCertificatesSetting only—no
other behavior changes.
- Around line 64-65: The current code replaces Node's entire CA list by calling
tls.getCACertificates('bundled') and then
tls.setDefaultCACertificates([...systemCerts, ...bundledCerts]); instead call
tls.getCACertificates() with no arguments to retrieve the runtime's actual
default CA list (which includes bundled + NODE_EXTRA_CA_CERTS + system CAs),
merge your systemCerts into that list (deduplicate if needed) and pass the
combined array to tls.setDefaultCACertificates so you preserve existing default
CAs; update references around tls.getCACertificates,
tls.setDefaultCACertificates, and the systemCerts variable accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe1c4c27-a151-469b-b93f-b32f6dc55657
📒 Files selected for processing (4)
docs/corporate-certificate-configuration.mdsrc/app/main/data.tssrc/main.tssrc/systemCertificates.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check (windows-latest)
- GitHub Check: check (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from@rocket.chat/fuselagefor all UI work and only create custom components when Fuselage doesn't provide what's needed
CheckTheme.d.tsfor valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux
Files:
src/main.tssrc/app/main/data.tssrc/systemCertificates.ts
🧠 Learnings (2)
📚 Learning: 2026-03-06T19:31:11.433Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/*(!preload).ts?(x) : Always use the centralized logger from `logger.ts` (outlookLog, outlookDebug, outlookError, outlookWarn, outlookEventDetail) instead of console.log() for Outlook Calendar module logging
Applied to files:
src/app/main/data.ts
📚 Learning: 2026-03-06T19:31:11.433Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: src/outlookCalendar/AGENTS.md:0-0
Timestamp: 2026-03-06T19:31:11.433Z
Learning: Applies to src/outlookCalendar/**/preload.ts : Keep preload.ts logging minimal since it cannot access the verbose logging toggle from Redux Store and all logs always appear
Applied to files:
src/app/main/data.ts
- Align settings precedence: read both override files and let appAsar win, matching the merge order in data.ts - Use getCACertificates() (no arg) to preserve NODE_EXTRA_CA_CERTS alongside system and bundled CAs - Clarify doc wording for app ASAR override location
Summary
tls.setDefaultCACertificates()API — zero external dependenciesuseSystemCertificatesinoverridden-settings.jsonallowInsecureOutlookConnectionsremains as a last-resort fallback; a warning is logged when both are activeWhy now
Electron has always had a split TLS architecture: the Chromium renderer trusts the OS certificate store, but Node.js in the main process uses a hardcoded Mozilla CA bundle — ignoring corporate CAs entirely. This has been a long-standing pain point (electron/electron#11741), and our previous workaround (
allowInsecureOutlookConnectionsfrom #3191) simply disabled all TLS verification.Node.js addressed this in two steps:
tls.getCACertificates('system')to read OS-trusted certificatestls.setDefaultCACertificates()to replace the default CA bundle programmaticallyOur upgrade to Electron 40 (Node.js 24.11.1) made both APIs available, enabling a proper solution with no third-party dependencies (previously this required packages like
win-ca,mac-ca, orsystem-ca).How it works
At boot, before any HTTPS connections, the app calls:
This sets the default CA bundle for all TLS contexts that don't specify their own
caoption — covering plain axios, customhttps.Agentinstances (EWS/NTLM), and any future HTTP client code. The bundled Mozilla CAs are preserved alongside system CAs, so nothing that worked before will break.Test plan
yarn buildcompiles without errorsyarn lintpassesSystem CA certificates: loaded N certificates from OS trust storeallowInsecureOutlookConnections)useSystemCertificates: falseinoverridden-settings.jsondisables the featureallowInsecureOutlookConnectionsand system CAs are active, a warning is loggedCORE-1363
Summary by CodeRabbit
New Features
Documentation