Honor NODE_EXTRA_CA_CERTS in the default fetch path#1803
Honor NODE_EXTRA_CA_CERTS in the default fetch path#1803raashish1601 wants to merge 3 commits intoopenai:masterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcf34cdb73
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/internal/shims.ts
Outdated
| } | ||
|
|
||
| function applyNodeExtraCACertificates(): void { | ||
| if (!readEnv('NODE_EXTRA_CA_CERTS')) return; |
There was a problem hiding this comment.
This early return makes the fix dependent on process.env.NODE_EXTRA_CA_CERTS still being present at client construction time, but Node loads extra certs at process start and tls.getCACertificates('extra') can remain populated even after that env var is removed/scrubbed. In that common hardening pattern, the SDK will skip setDefaultCACertificates, so the default-fetch path still won’t trust those extra roots and requests to private/corporate CAs can fail. The merge should be driven by getCACertificates('extra') availability, not a fresh env read.
Useful? React with 👍 / 👎.
1 participant
Summary
NODE_EXTRA_CA_CERTSinto Node''s default TLS CA store before the SDK adopts the default globalfetchtests/index.test.tsFixes #1758.
Testing
node node_modules/jest/bin/jest.js tests/index.test.ts --runInBandnode node_modules/prettier/bin/prettier.cjs --check README.md src/internal/shims.ts tests/index.test.tsnode node_modules/eslint/bin/eslint.js src/internal/shims.ts tests/index.test.ts