X Tutup
The Wayback Machine - https://web.archive.org/web/20250814212439/https://github.com/nodejs/node/pull/59276
Skip to content

cli: add NODE_USE_SYSTEM_CA=1 #59276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 10, 2025

Conversation

joyeecheung
Copy link
Member

Similar to how NODE_USE_ENV_PROXY complements --use-env-proxy, this
complements --use-system-ca. This will allow the setting to be
applied to workers individually in the future.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jul 29, 2025
Copy link

codecov bot commented Jul 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.91%. Comparing base (a73b575) to head (d23c5e8).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #59276   +/-   ##
=======================================
  Coverage   89.91%   89.91%           
=======================================
  Files         655      655           
  Lines      192866   192871    +5     
  Branches    37806    37803    -3     
=======================================
+ Hits       173412   173420    +8     
+ Misses      12015    12002   -13     
- Partials     7439     7449   +10     
Files with missing lines Coverage Δ
src/node.cc 75.10% <100.00%> (+0.17%) ⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

const { child: { stdout: expectedLength } } = spawnSyncAndExitWithoutError(process.execPath, [
'--use-system-ca',
'-p',
`tls.getCACertificates('default').length`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit... just have a preference to avoid template literals when unnecessary. but feel free to ignore.

Suggested change
`tls.getCACertificates('default').length`,
'tls.getCACertificates("default").length',

Copy link
Member Author

@joyeecheung joyeecheung Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have am impression that double quotes in an argument don't work very well on Windows. I could give it a try and see if it works though if the CI comes back happy I would prefer to use single quotes (AFAIK Windows does not do anything special with single quotes).

@bnoordhuis
Copy link
Member

Why not NODE_OPTIONS? That env var was specifically introduced to counter new environment variables getting introduced for every single little feature.

@joyeecheung
Copy link
Member Author

joyeecheung commented Jul 30, 2025

Why not NODE_OPTIONS? That env var was specifically introduced to counter new environment variables getting introduced for every single little feature.

NODE_OPTIONS may not work so well when working with workers: #41103 - if workers controlled by another party need to override a feature to be inherited by other workers, then overriding the setting requires argument parsing on the env var, and they need to be careful to filter out arguments not supported per-worker. It would also be more consistent with the offering of NODE_USE_ENV_PROXY/--use-env-proxy which tend to be where NODE_USE_SYSTEM_CA/--use-system-ca would also be needed.

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 30, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 30, 2025
@nodejs-github-bot
Copy link
Collaborator

@@ -0,0 +1,56 @@
// Env: NODE_USE_SYSTEM_CA=1
Copy link
Member Author

@joyeecheung joyeecheung Jul 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @pmarchini when using this feature (thanks for implementing it by the way) I noticed that it does not yet have integration in test/common/index.js like the one for Flags so that when you run the test directly with node /path/to/test.js without the specified environment variable, the process would automatically pause the evaluation and spawn a child process to run itself with the environment variable instead.

if (process.argv.length === 2 &&
!process.env.NODE_SKIP_FLAG_CHECK &&
isMainThread &&
hasCrypto &&
require('cluster').isPrimary &&
fs.existsSync(process.argv[1])) {
const flags = parseTestFlags();

@nodejs-github-bot

This comment was marked as outdated.

@joyeecheung joyeecheung added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 30, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

Added macros to fix the withoutssl builds. @jasnell @Ethan-Arrowood @RaisinTen can you take a look again? Thanks

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

Not quite sure why there is a new src\quic\data.h(159,5): error : expected identifier in the coverage Windows build. I am guessing it might need a rebase.

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 8, 2025
@nodejs-github-bot
Copy link
Collaborator

Similar to how NODE_USE_ENV_PROXY complements --use-env-proxy, this
complements --use-system-ca. This will allow the setting to be
applied to workers individually in the future.
@joyeecheung
Copy link
Member Author

Not sure how but after the rebase the SEA tests started to fail on macOS x64. Trying to rebase again...

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

CI is (finally) happy. This needs a re-approval after the rebase. @jasnell @legendecas @Ethan-Arrowood @RaisinTen Can you take a look again? Thanks!

Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@joyeecheung joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 10, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 10, 2025
@nodejs-github-bot nodejs-github-bot merged commit ca76b39 into nodejs:main Aug 10, 2025
60 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ca76b39

RafaelGSS pushed a commit that referenced this pull request Aug 11, 2025
Similar to how NODE_USE_ENV_PROXY complements --use-env-proxy, this
complements --use-system-ca. This will allow the setting to be
applied to workers individually in the future.

PR-URL: #59276
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
nodejs-github-bot added a commit that referenced this pull request Aug 11, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Similar to how NODE_USE_ENV_PROXY complements --use-env-proxy, this
complements --use-system-ca. This will allow the setting to be
applied to workers individually in the future.

PR-URL: #59276
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
  * (SEMVER-MINOR) add --use-env-proxy (Joyee Cheung) #59151
  * (SEMVER-MINOR) support `${pid}` placeholder in --cpu-prof-name (Haram Jeong) #59072
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
  * (SEMVER-MINOR) add tls.setDefaultCACertificates() (Joyee Cheung) #58822
deps:
  * update archs files for openssl-3.5.1 (Node.js GitHub Bot) #59234
  * upgrade openssl sources to openssl-3.5.1 (Node.js GitHub Bot) #59234
dns:
  * (SEMVER-MINOR) support max timeout (theanarkh) #58440
doc:
  * update the instruction on how to verify releases (Antoine du Hamel) #59113
esm:
  * (SEMVER-MINOR) unflag --experimental-wasm-modules (Guy Bedford) #57038
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
http,https:
  * (SEMVER-MINOR) add built-in proxy support in http/https.request and Agent (Joyee Cheung) #58980
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
net:
  * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087
test:
  * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980
worker:
  * (SEMVER-MINOR) add web locks api (ishabi) #58666
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
  * (SEMVER-MINOR) add --use-env-proxy (Joyee Cheung) #59151
  * (SEMVER-MINOR) support `${pid}` placeholder in --cpu-prof-name (Haram Jeong) #59072
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
  * (SEMVER-MINOR) add tls.setDefaultCACertificates() (Joyee Cheung) #58822
deps:
  * update archs files for openssl-3.5.1 (Node.js GitHub Bot) #59234
  * upgrade openssl sources to openssl-3.5.1 (Node.js GitHub Bot) #59234
dns:
  * (SEMVER-MINOR) support max timeout (theanarkh) #58440
doc:
  * update the instruction on how to verify releases (Antoine du Hamel) #59113
esm:
  * (SEMVER-MINOR) unflag --experimental-wasm-modules (Guy Bedford) #57038
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
http,https:
  * (SEMVER-MINOR) add built-in proxy support in http/https.request and Agent (Joyee Cheung) #58980
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
net:
  * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087
test:
  * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980
worker:
  * (SEMVER-MINOR) add web locks api (ishabi) #58666
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 12, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 14, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 14, 2025
Notable changes:

cli:
  * (SEMVER-MINOR) add NODE_USE_SYSTEM_CA=1 (Joyee Cheung) #59276
crypto:
  * (SEMVER-MINOR) support ML-DSA KeyObject, sign, and verify (Filip Skokan) #59259
fs:
  * (SEMVER-MINOR) port SonicBoom module to fs module as Utf8Stream (James M Snell) #58897
http:
  * (SEMVER-MINOR) add server.keepAliveTimeoutBuffer option (Haram Jeong) #59243
lib:
  * docs deprecate _http_* (Sebastian Beltran) #59293
zlib:
  * (SEMVER-MINOR) add dictionary support to zstdCompress and zstdDecompress (lluisemper) #59240

PR-URL: #59449
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants
X Tutup