Skip to content

https: honor per-request TLS options over agent options - #66288

Open
axedos wants to merge 1 commit into
nodejs:mainfrom
axedos:https-per-request-tls-override
Open

axedos wants to merge 1 commit into
nodejs:mainfrom
axedos:https-per-request-tls-override

Conversation

@axedos

@axedos axedos commented Sep 25, 2026

Copy link
Copy Markdown

Per-request TLS options passed to https.request() / https.get() are silently
ignored when the https.Agent in use was constructed with the same option.
http.Agent merges the request options over the agent's own options, letting
the agent's value win.

That is surprising and can weaken security for TLS options in particular: a
per-request rejectUnauthorized: true on an agent configured with
rejectUnauthorized: false still sends the request with certificate
verification disabled. The same applies to ca and servername.

CVE-2026-58040 already handled this for checkServerIdentity by tagging the
request so the socket is not reused. This change generalizes that approach:

when a per-request rejectUnauthorized, ca, or servername differs from the
agent's value, request() captures the override and createConnection()
re-applies it, and the socket is not reused.

Refs: 52a8ace880d

Per-request rejectUnauthorized/ca/servername were silently ignored when the
https.Agent set the same option, because http.Agent merges request options
over the agent's own, letting the agent win. Capture the per-request overrides
and re-apply them in createConnection(), mirroring the checkServerIdentity
handling from CVE-2026-58040.

Refs: nodejs@52a8ace880d
Signed-off-by: axedos <acceleratingssoul@proton.me>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added https Issues and PRs related to the https subsystem. needs-ci PRs that need a full CI run. labels Sep 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome to Node.js, and thank you for your first contribution!

Before review, please take a moment to read:

Please make sure every commit is signed off. For a first pull request, GitHub Actions require collaborator approval and Jenkins CI must be started by a collaborator or triager, so an initial wait is normal.

@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.35%. Comparing base (bc6e1ad) to head (7145186).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66288      +/-   ##
==========================================
- Coverage   90.37%   90.35%   -0.02%     
==========================================
  Files         790      790              
  Lines      273807   273850      +43     
  Branches    52367    52389      +22     
==========================================
- Hits       247446   247441       -5     
- Misses      16869    16918      +49     
+ Partials     9492     9491       -1     
Files with missing lines Coverage Δ
lib/https.js 98.11% <100.00%> (+0.90%) ⬆️

... 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

https Issues and PRs related to the https subsystem. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants