From 94571035d9e4424cc47d3647b5495abcb12a3827 Mon Sep 17 00:00:00 2001 From: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com> Date: Thu, 16 Apr 2026 19:39:25 +0300 Subject: [PATCH] fix(socks5-proxy-agent): use per-origin pools to prevent cross-origin routing 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. Co-Authored-By: Claude Co-Authored-By: DeepView Autofix <276251120+deepview-autofix@users.noreply.github.com> Co-Authored-By: Nikita Skovoroda Signed-off-by: Nikita Skovoroda --- lib/dispatcher/socks5-proxy-agent.js | 33 ++++++++++++++-------- test/socks5-proxy-agent.js | 42 ++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/lib/dispatcher/socks5-proxy-agent.js b/lib/dispatcher/socks5-proxy-agent.js index 4a2f65e6eed..8029c8233bd 100644 --- a/lib/dispatcher/socks5-proxy-agent.js +++ b/lib/dispatcher/socks5-proxy-agent.js @@ -17,7 +17,7 @@ const debug = debuglog('undici:socks5-proxy') const kProxyUrl = Symbol('proxy url') const kProxyHeaders = Symbol('proxy headers') const kProxyAuth = Symbol('proxy auth') -const kPool = Symbol('pool') +const kPools = Symbol('pools') const kConnector = Symbol('connector') // Static flag to ensure warning is only emitted once per process @@ -65,8 +65,8 @@ class Socks5ProxyAgent extends DispatcherBase { servername: options.proxyTls?.servername || url.hostname }) - // Pool for the actual HTTP connections (with SOCKS5 tunnel connect function) - this[kPool] = null + // Pools for the actual HTTP connections (with SOCKS5 tunnel connect function), keyed by origin + this[kPools] = new Map() } /** @@ -183,9 +183,11 @@ class Socks5ProxyAgent extends DispatcherBase { debug('dispatching request to', origin, 'via SOCKS5') try { - // Create Pool with custom connect function if we don't have one yet - if (!this[kPool] || this[kPool].destroyed || this[kPool].closed) { - this[kPool] = new Pool(origin, { + const originKey = String(origin) + let pool = this[kPools].get(originKey) + // Create a Pool per origin so requests are not routed to the wrong host + if (!pool || pool.destroyed || pool.closed) { + pool = new Pool(origin, { pipelining: opts.pipelining, connections: opts.connections, connect: async (connectOpts, callback) => { @@ -225,10 +227,11 @@ class Socks5ProxyAgent extends DispatcherBase { } } }) + this[kPools].set(originKey, pool) } - // Dispatch the request through the pool - return this[kPool][kDispatch](opts, handler) + // Dispatch the request through the per-origin pool + return pool[kDispatch](opts, handler) } catch (err) { debug('dispatch error:', err) if (typeof handler.onError === 'function') { @@ -240,15 +243,21 @@ class Socks5ProxyAgent extends DispatcherBase { } async [kClose] () { - if (this[kPool]) { - await this[kPool].close() + const closePromises = [] + for (const pool of this[kPools].values()) { + closePromises.push(pool.close()) } + this[kPools].clear() + await Promise.all(closePromises) } async [kDestroy] (err) { - if (this[kPool]) { - await this[kPool].destroy(err) + const destroyPromises = [] + for (const pool of this[kPools].values()) { + destroyPromises.push(pool.destroy(err)) } + this[kPools].clear() + await Promise.all(destroyPromises) } } diff --git a/test/socks5-proxy-agent.js b/test/socks5-proxy-agent.js index 85185922647..51587f6d4cb 100644 --- a/test/socks5-proxy-agent.js +++ b/test/socks5-proxy-agent.js @@ -225,6 +225,48 @@ test('Socks5ProxyAgent - multiple requests through same proxy', async (t) => { await p.completed }) +test('Socks5ProxyAgent - requests to different origins are routed correctly', async (t) => { + const p = tspl(t, { plan: 4 }) + + // Create two distinct target servers + const serverA = createServer((req, res) => { + res.writeHead(200, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ server: 'A', path: req.url })) + }) + const serverB = createServer((req, res) => { + res.writeHead(200, { 'content-type': 'application/json' }) + res.end(JSON.stringify({ server: 'B', path: req.url })) + }) + + await new Promise((resolve) => serverA.listen(0, '127.0.0.1', resolve)) + await new Promise((resolve) => serverB.listen(0, '127.0.0.1', resolve)) + const portA = serverA.address().port + const portB = serverB.address().port + + const socksServer = new TestSocks5Server() + const socksAddress = await socksServer.listen() + + try { + const proxyWrapper = new Socks5ProxyAgent(`socks5://127.0.0.1:${socksAddress.port}`) + + // First request goes to server A — establishes a pool + const respA = await request(`http://127.0.0.1:${portA}/a`, { dispatcher: proxyWrapper }) + p.equal(respA.statusCode, 200) + p.deepEqual(await respA.body.json(), { server: 'A', path: '/a' }) + + // Second request goes to server B — must NOT reuse the pool from origin A + const respB = await request(`http://127.0.0.1:${portB}/b`, { dispatcher: proxyWrapper }) + p.equal(respB.statusCode, 200) + p.deepEqual(await respB.body.json(), { server: 'B', path: '/b' }, 'request to origin B must reach server B, not server A') + } finally { + await socksServer.close() + serverA.close() + serverB.close() + } + + await p.completed +}) + test('Socks5ProxyAgent - connection failure', async (t) => { const p = tspl(t, { plan: 1 })