fix(packaged): forward NODE_EXTRA_CA_CERTS to sidecars - #8559
leorivastech wants to merge 1 commit into
Conversation
The packaged daemon is spawned with an allowlisted environment, so an extra CA bundle set for the app never reached the process that makes BYOK model-discovery and connection-test requests. Behind a TLS-inspecting proxy those requests failed with SELF_SIGNED_CERT_IN_CHAIN. Forward NODE_EXTRA_CA_CERTS only. Certificate verification stays on; NODE_OPTIONS and NODE_TLS_REJECT_UNAUTHORIZED are still dropped. Fixes nexu-io#8157
lefarcen
added
risk/high
|
Thanks @leorivastech — the narrow allowlist change and focused regression coverage make the intent easy to follow. I've routed this to a reviewer. |
lefarcen
added
the
needs-validation
|
This change affects BYOK model discovery and connection testing, so it is queued for QA validation before merge. Nothing is needed from you right now; we’ll update the PR once validation is complete. |
|
Thanks @lefarcen! |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
This correctly forwards NODE_EXTRA_CA_CERTS through the packaged child allowlist, so the daemon receives an explicitly configured corporate CA bundle while unsafe TLS and runtime overrides remain excluded. I traced the resolver through the daemon spawn path and reviewed the focused regression coverage. Thanks for keeping this compatibility fix narrow and well-covered.
This branch has not been deployed
3 participants
Fixes #8157
Why
Behind a TLS-inspecting proxy (Cato in the report), every BYOK "Fetch models" and "Test" request in the desktop app fails with
SELF_SIGNED_CERT_IN_CHAIN, even when the app is launched withNODE_EXTRA_CA_CERTSpointing at the corporate root CA. As @lefarcen found in the issue, the packaged app builds the daemon's environment from an allowlist, andNODE_EXTRA_CA_CERTSwasn't on it. So the daemon, which is what makes those requests, never got the CA bundle. #6802 fixed the same handoff forOD_ALLOWED_INTERNAL_HOSTS.What users will see
If the app is started with
NODE_EXTRA_CA_CERTSset, BYOK model discovery and the connection test trust that CA on top of the normal ones. Nothing changes for anyone who doesn't set it.On macOS, apps opened from Finder don't get your shell's variables, so it has to be set for the session first, for example
launchctl setenv NODE_EXTRA_CA_CERTS /path/to/corp-ca.pemand then open the app, or run the binary from a terminal with the variable set.Surface area
NODE_EXTRA_CA_CERTSto its sidecars. No newOD_*variable.Screenshots
No UI change.
Bug fix verification
apps/packaged/tests/sidecars.test.ts, "forwards NODE_EXTRA_CA_CERTS to the daemon without forwarding TLS or runtime overrides". It callsresolvePackagedChildBaseEnvwith the same arguments the daemon spawn uses.main, green here: yes. A second test checks that an empty value isn't forwarded; that one already passes onmainand is just a guard.This only adds a CA bundle. Verification stays on:
NODE_TLS_REJECT_UNAUTHORIZEDandNODE_OPTIONSare still dropped, and the test checks that.--use-system-cais left out, like the issue says.I also checked the runtime by hand, since on macOS and Windows the daemon runs as Electron in run-as-node mode. With Electron 41.3.0 and a local HTTPS server signed by a throwaway CA,
fetchand an undiciAgentfailed withUNABLE_TO_VERIFY_LEAF_SIGNATUREwithout the variable and returned 200 with it, including with a path that has spaces. Setting it after startup didn't help, so it has to be in the spawn env, which is what this does. A missing file only prints a Node warning and the daemon keeps running. On Linux the daemon runs on the bundled Node, which reads the variable the same way.Validation
pnpm --filter @open-design/packaged test(24 files, 356 tests)pnpm typecheckpnpm guard