Skip to content

fix: honor requestTls when proxy is SOCKS5#5417

Merged
mcollina merged 6 commits into
v7.xfrom
backport-42d49559-v7.x
Jun 12, 2026
Merged

fix: honor requestTls when proxy is SOCKS5#5417
mcollina merged 6 commits into
v7.xfrom
backport-42d49559-v7.x

Conversation

@mcollina

@mcollina mcollina commented Jun 12, 2026

Copy link
Copy Markdown
Member

Backports 42d4955 to v7.x.

Also backports related SOCKS5 fixes from main needed around this area:

deepview-autofix and others added 6 commits June 12, 2026 11:31
… routing (#5041)

The Socks5ProxyAgent stored a single Pool keyed implicitly to the first
origin it saw and reused it for every subsequent request. When a second
request targeted a different origin, it was dispatched through the
existing pool whose connect callback tunnelled to the original origin,
causing the request to reach the wrong host (and potentially leaking
headers/credentials intended for origin B to origin A).

Track pools in a Map keyed by origin so each origin gets its own pool
and SOCKS5 tunnel.

Signed-off-by: Nikita Skovoroda <chalkerx@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Nikita Skovoroda <chalkerx@gmail.com>
(cherry picked from commit a516f87)
(cherry picked from commit 1dec881)
Signed-off-by: Matteo Collina <hello@matteocollina.com>
…5099)

Assisted-by: openai:gpt-5.4

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
(cherry picked from commit d76f932)
* fix: use configured connector in Socks5ProxyAgent

Assisted-by: openai:gpt-5.5
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>

* test: use t.after in Socks5ProxyAgent custom connector test

---------

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
(cherry picked from commit 1a45226)
Assited-by: openai:gpt-5.5

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
(cherry picked from commit e2bc508)
ProxyAgent silently dropped opts.requestTls when the configured proxy
URI was socks5:// (or socks://). Socks5ProxyAgent also lacked any
equivalent of requestTls, so even constructed directly it had no way to
configure target HTTPS TLS settings (ca, cert, key, rejectUnauthorized,
servername).

The inner connect callback called tls.connect with
...connectOpts.tls || {} but undici's internal connectOpts never
populates a .tls field, so the spread was a dead expression.

This change:
- Forwards opts.requestTls from ProxyAgent to Socks5ProxyAgent.
- Adds a requestTls constructor option on Socks5ProxyAgent stored in
  the kRequestTls symbol.
- Applies it on the inner tls.connect for the target HTTPS upgrade,
  with socket overriding (must point at the SOCKS5 tunnel) and
  servername defaulting to targetHost but allowing user override
  to match the HTTP-proxy path semantics.
- Documents the new option in Socks5ProxyAgent.md.
- Adds two regression tests covering both the direct Socks5ProxyAgent
  path and the ProxyAgent forwarding path.

Refs GHSA-vmh5-mc38-953g

(cherry picked from commit 42d4955)
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina requested review from metcoder95 and trivikr June 12, 2026 09:54
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.13924% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.99%. Comparing base (85a2405) to head (0d2fceb).

Files with missing lines Patch % Lines
lib/dispatcher/socks5-proxy-agent.js 88.88% 5 Missing ⚠️
lib/core/socks5-client.js 85.71% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             v7.x    #5417      +/-   ##
==========================================
+ Coverage   92.90%   92.99%   +0.09%     
==========================================
  Files         112      112              
  Lines       35909    35945      +36     
==========================================
+ Hits        33362    33428      +66     
+ Misses       2547     2517      -30     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

@mcollina mcollina merged commit 04201f8 into v7.x Jun 12, 2026
38 checks passed
@mcollina mcollina deleted the backport-42d49559-v7.x branch June 12, 2026 13:51
@github-actions github-actions Bot mentioned this pull request Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants