fix(socks): forward Agent.connectTimeout to SocksClient and TLS, unify family null (#11842)

Fixes a SOCKS proxy timeout bypass: Agent.connectTimeout now reaches both the SocksClient.createConnection handshake and the TLS buildConnector phases (previously a stalled/blackholed SOCKS connection could hang past the configured budget), and the fetch-socks family===null path is unified onto createSocksDispatcherWithFamily. Verified against a faux RFC 1928 SOCKS server exercising both pre-grant and post-grant stalls. 6/6 focused tests passing. Thanks!

changelog.d/fixes/11842-socks-connect-timeout-unify-family.md

@@ -0,0 +1 @@
+- **fix(socks):** `Agent.connectTimeout` now bounds both SOCKS handshake and TLS connect, and `family === null` no longer falls back to `fetch-socks` ([#11842](https://github.com/diegosouzapw/OmniRoute/pull/11842))

docs/i18n/pl/docs/security/EGRESS_POLICY.md

@@ -190,10 +190,7 @@ export function buildSocksFamilySocketOptions(family: 4 | 6 | null): Record<stri
 }
 ```
 
-`createProxyDispatcher` wybiera connector w zależności od tego, czy rodzina jest przypięta:
-
-- `family === null` (czyli `auto` nad hostname) → stockowe `socksDispatcher` z `fetch-socks`.
-- `family === 4 | 6` → `createSocksDispatcherWithFamily`, które przekazuje `socket_options` do `SocksClient.createConnection`, żeby Happy Eyeballs nie wybrał IPv4 przy polityce egress tylko-IPv6.
+Wszystkie dispatchery SOCKS5 przechodzą przez `createSocksDispatcherWithFamily` niezależnie od `family` (również `null` / `auto` nad hostname): `buildSocksFamilySocketOptions(null)` daje `{}`, a ta sama ścieżka `SocksClient.createConnection` + TLS `buildConnector` jest używana z przypięciem `socket_options`, żeby Happy Eyeballs nie wybrał IPv4 przy polityce egress tylko-IPv6.
 
 Sam support SOCKS5 jest domyślnie włączony (opt-out przez `ENABLE_SOCKS5_PROXY=false`); zob. [PROXY_GUIDE.md → Environment Variables](../ops/PROXY_GUIDE.md#environment-variables).
 

docs/security/EGRESS_POLICY.md

@@ -190,10 +190,7 @@ export function buildSocksFamilySocketOptions(family: 4 | 6 | null): Record<stri
 }
 ```
 
-`createProxyDispatcher` chooses the connector based on whether a family is pinned:
-
-- `family === null` (i.e. `auto` over a hostname) → stock `socksDispatcher` from `fetch-socks`.
-- `family === 4 | 6` → `createSocksDispatcherWithFamily`, which threads `socket_options` into `SocksClient.createConnection` so Happy Eyeballs cannot pick IPv4 for an IPv6-only egress policy.
+All SOCKS5 dispatches go through `createSocksDispatcherWithFamily` regardless of `family` (including `null` / `auto` over a hostname): `buildSocksFamilySocketOptions(null)` yields `{}`, and the same `SocksClient.createConnection` + TLS `buildConnector` path is used with `socket_options` pinning so Happy Eyeballs cannot pick IPv4 for an IPv6-only egress policy.
 
 SOCKS5 support itself is on by default (opt-out via `ENABLE_SOCKS5_PROXY=false`); see [PROXY_GUIDE.md → Environment Variables](../ops/PROXY_GUIDE.md#environment-variables).
 

open-sse/utils/proxyDispatcher.ts

@@ -1,6 +1,5 @@
 import "./setupPolyfill.ts";
 import { Agent, ProxyAgent, type Dispatcher } from "undici";
-import { socksDispatcher } from "fetch-socks";
 import { getUpstreamTimeoutConfig } from "@/shared/utils/runtimeTimeouts";
 import { stripIpv6Brackets, detectIpLiteralFamily, parseProxyFamily } from "./proxyFamily.ts";
 import { createSocksDispatcherWithFamily } from "./socksConnectorWithFamily.ts";
@@ -461,16 +460,11 @@ function buildProxyDispatcher(
     };
     if (parsed.username) socksOptions.userId = decodeURIComponent(parsed.username);
     if (parsed.password) socksOptions.password = decodeURIComponent(parsed.password);
-    return family === null
-      ? (socksDispatcher(
-          socksOptions as Parameters<typeof socksDispatcher>[0],
-          options
-        ) as Dispatcher)
-      : createSocksDispatcherWithFamily(
-          socksOptions as unknown as Parameters<typeof createSocksDispatcherWithFamily>[0],
-          family,
-          options
-        );
+    return createSocksDispatcherWithFamily(
+      socksOptions as unknown as Parameters<typeof createSocksDispatcherWithFamily>[0],
+      family,
+      options
+    );
   }
 
   // ProxyAgent omits `connect`; the client->proxy socket is built from `proxyTls`.

open-sse/utils/socksConnectorWithFamily.ts

@@ -39,12 +39,24 @@ function resolvePort(protocol: string, port: string): number {
  * socksConnector but threads `socket_options` (which fetch-socks does not expose)
  * into SocksClient so Happy Eyeballs cannot pick IPv4 for an IPv6-only egress policy.
  */
-function socksConnectorWithFamily(
+export function socksConnectorWithFamily(
   proxy: SocksProxy,
   family: 4 | 6 | null,
-  tlsOpts: buildConnector.BuildOptions = {}
+  tlsOpts: buildConnector.BuildOptions = {},
+  connectTimeout?: number,
+  _buildConnectorForTest?: typeof buildConnector
 ): buildConnector.connector {
-  const undiciConnect = buildConnector(tlsOpts);
+  const isDisabled = connectTimeout === 0;
+  // SOCKS lib: 0 throws (isValidTimeoutValue: value>0) and undefined → DEFAULT_TIMEOUT 30s;
+  // undici: 0 disables (core/util.js: if (!opts.timeout) return noop), undefined → 10s. Divergence intentional.
+  const handshakeTimeout = isDisabled ? undefined : (connectTimeout ?? resolveSocksHandshakeTimeoutMs());
+  const tlsTimeout = connectTimeout;
+  // Sequential budget: both phases bounded by the same connectTimeout → wall-time up to 60s for https
+  // (vs 30s direct). Shared-deadline alternative rejected as unjustified complexity.
+  const build = _buildConnectorForTest ?? buildConnector;
+  const undiciConnect = build(
+    tlsTimeout !== undefined ? ({ ...tlsOpts, timeout: tlsTimeout } as any) : tlsOpts
+  );
   const socketOptions = buildSocksFamilySocketOptions(family);
   return async (options, callback) => {
     const { protocol, hostname, port, httpSocket } = options as unknown as {
@@ -57,7 +69,7 @@ function socksConnectorWithFamily(
       const r = await SocksClient.createConnection({
         command: "connect",
         proxy,
-        timeout: resolveSocksHandshakeTimeoutMs(),
+        timeout: handshakeTimeout as any,
         destination: { host: hostname, port: resolvePort(protocol, port) },
         existing_socket: httpSocket as never,
         socket_options: socketOptions as never,
@@ -79,11 +91,12 @@ export function createSocksDispatcherWithFamily(
   family: 4 | 6 | null,
   agentOptions: Agent.Options = {}
 ): Dispatcher {
-  const { connect, ...rest } = agentOptions as Agent.Options & {
+  const { connect, connectTimeout, ...rest } = agentOptions as Agent.Options & {
+    connectTimeout?: number;
     connect?: buildConnector.BuildOptions;
   };
   return new Agent({
     ...rest,
-    connect: socksConnectorWithFamily(proxy, family, connect),
+    connect: socksConnectorWithFamily(proxy, family, connect as any, connectTimeout as any),
   });
 }

tests/unit/socks-connect-timeout-e2e.test.ts

@@ -0,0 +1,65 @@
+import { describe, it, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import net from "node:net";
+import { fetch } from "undici";
+import { createSocksDispatcherWithFamily } from "../../open-sse/utils/socksConnectorWithFamily.ts";
+import { clearDispatcherCache } from "../../open-sse/utils/proxyDispatcher.ts";
+
+async function startFakeSocks(opts: { stallAfterGrant: boolean }): Promise<{ port: number; close: () => Promise<void> }> {
+  return new Promise((resolve) => {
+    const serverSockets = new Set<net.Socket>();
+    const server = net.createServer((socket) => {
+      let step = 0;
+      serverSockets.add(socket);
+      socket.on("close", () => serverSockets.delete(socket));
+      socket.on("data", (_data: Buffer) => {
+        if (step === 0) {
+          socket.write(Buffer.from([0x05, 0x00]));
+          step = 1;
+          return;
+        }
+        if (step === 1) {
+          if (!opts.stallAfterGrant) return;
+          socket.write(Buffer.from([0x05, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]));
+          step = 2;
+        }
+      });
+    });
+    server.listen(0, "127.0.0.1", () => {
+      const addr = server.address() as net.AddressInfo;
+      resolve({
+        port: addr.port,
+        close: () =>
+          new Promise<void>((r) => {
+            for (const s of serverSockets) s.destroy();
+            server.close(() => r());
+          }),
+      });
+    });
+  });
+}
+
+describe("stub SOCKS e2e", () => {
+  afterEach(() => clearDispatcherCache());
+
+  it("pre-grant stall (SOCKS timer) \u2192 error < 1000ms", async () => {
+    const { port, close } = await startFakeSocks({ stallAfterGrant: false });
+    const dispatcher = createSocksDispatcherWithFamily({ host: "127.0.0.1", port, type: 5 } as any, 4 as any, { connectTimeout: 300, connect: {} } as any);
+    const t0 = Date.now();
+    await assert.rejects(() => fetch("https://example.invalid/", { dispatcher } as any));
+    assert.ok(Date.now() - t0 < 1000, `pre-grant stall must error < 1000ms, took ${Date.now() - t0}ms`);
+    await close();
+  });
+
+  it("post-grant stall (TLS timer, https:// only) \u2192 error < 1500ms", async () => {
+    const { port, close } = await startFakeSocks({ stallAfterGrant: true });
+    const dispatcher = createSocksDispatcherWithFamily({ host: "127.0.0.1", port, type: 5 } as any, 4 as any, { connectTimeout: 300, connect: {} } as any);
+    const t0 = Date.now();
+    let caught: any = null;
+    await assert.rejects(async () => { try { await fetch("https://example.invalid/", { dispatcher } as any); } catch (e) { caught = e; throw e; } });
+    const err: any = caught;
+    // The ~1000ms wall time is connectTimeout 300ms + undici immediate/queue overhead, not the 10000ms default.
+    assert.ok(Date.now() - t0 < 1500, `post-grant stall must error < 1500ms, took ${Date.now() - t0}ms (err: ${String((err as any)?.message ?? err).slice(0, 120)})`);
+    await close();
+  });
+});

tests/unit/socks-connect-timeout.test.ts

@@ -0,0 +1,91 @@
+import { describe, it, afterEach, beforeEach } from "node:test";
+import assert from "node:assert/strict";
+import { SocksClient } from "socks";
+
+// Lightweight oracle — node:test, no vi.mock.
+// We patch SocksClient.createConnection (writable) and inject a fake
+// buildConnector via the 5th param to capture the TLS timeout without
+// mutating the read-only undici module.
+
+describe("socks connectTimeout forwarder", () => {
+  let capturedTimeout: any = undefined;
+  let capturedTlsTimeout: any = undefined;
+  let capturedTlsUndefined = false;
+  let origCreateConnection: any;
+
+  beforeEach(() => {
+    origCreateConnection = SocksClient.createConnection;
+    capturedTimeout = undefined;
+    capturedTlsTimeout = undefined;
+    capturedTlsUndefined = false;
+    (SocksClient as any).createConnection = async (opts: any) => {
+      capturedTimeout = opts?.timeout;
+      return { socket: { setNoDelay: () => ({ setNoDelay: () => {} }) } } as any;
+    };
+  });
+
+  afterEach(() => {
+    (SocksClient as any).createConnection = origCreateConnection;
+    capturedTimeout = undefined;
+    capturedTlsTimeout = undefined;
+    capturedTlsUndefined = false;
+  });
+
+  function fakeBuildConnector(opts: any = {}) {
+    if (opts && typeof opts.timeout !== "undefined") capturedTlsTimeout = opts.timeout;
+    else capturedTlsUndefined = true;
+    return (_options: any, cb: any) => cb(null, { setNoDelay: () => ({}) } as any);
+  }
+
+  async function driveConnector(args: {
+    family: 4 | 6 | null;
+    tlsOpts?: any;
+    connectTimeout?: number;
+    protocol?: string;
+    hostname?: string;
+    port?: string;
+  }) {
+    const mod: any = await import(`../../open-sse/utils/socksConnectorWithFamily.ts?t=${Date.now()}-${Math.random()}`);
+    const proxy = { host: "1.2.3.4", port: 1080, type: 5 } as any;
+    const tlsOpts = args.tlsOpts ?? {};
+    const connectTimeout = args.connectTimeout;
+    const connector: any = mod.socksConnectorWithFamily(proxy, args.family, tlsOpts, connectTimeout, fakeBuildConnector as any);
+    await new Promise<void>((resolve, reject) =>
+      connector(
+        { protocol: args.protocol ?? "https:", hostname: args.hostname ?? "example.com", port: args.port ?? "443" } as any,
+        (err: any) => (err ? reject(err) : resolve())
+      )
+    );
+    return { capturedTimeout, capturedTlsTimeout, capturedTlsUndefined, mod, connector };
+  }
+
+  it("U1: Agent.connectTimeout → SocksClient.timeout + TLS timeout", async () => {
+    const { capturedTimeout: t, capturedTlsTimeout: tls } = await driveConnector({ family: 4, tlsOpts: {}, connectTimeout: 5000, protocol: "https:", port: "443" });
+    assert.equal(t, 5000);
+    assert.equal(tls, 5000);
+  });
+
+  it("U2: fallback sans connectTimeout → SOCKS_HANDSHAKE (TLS no timeout)", async () => {
+    const prev = process.env.SOCKS_HANDSHAKE_TIMEOUT_MS;
+    process.env.SOCKS_HANDSHAKE_TIMEOUT_MS = "7777";
+    try {
+      const { capturedTimeout: t, capturedTlsUndefined: tlsUndef } = await driveConnector({ family: 6, tlsOpts: {}, connectTimeout: undefined, protocol: "https:", port: "443" });
+      assert.equal(t, 7777);
+      assert.equal(tlsUndef, true);
+    } finally {
+      if (prev === undefined) delete process.env.SOCKS_HANDSHAKE_TIMEOUT_MS;
+      else process.env.SOCKS_HANDSHAKE_TIMEOUT_MS = prev;
+    }
+  });
+
+  it("U3: http (no TLS) still bounds SocksClient", async () => {
+    const { capturedTimeout: t } = await driveConnector({ family: null, tlsOpts: {}, connectTimeout: 5000, protocol: "http:", port: "80" });
+    assert.equal(t, 5000);
+  });
+
+  it("U4: connectTimeout=0 → SocksClient undefined (SOCKS defaults to 30s) + TLS timeout 0 (disabled)", async () => {
+    const { capturedTimeout: t, capturedTlsTimeout: tls } = await driveConnector({ family: 4, tlsOpts: {}, connectTimeout: 0, protocol: "https:", port: "443" });
+    assert.equal(t, undefined);
+    assert.equal(tls, 0);
+  });
+});