Skip to content

bug(runtime): buildProxyDispatcher silently dropped to undici 8's forward-by-default HTTP proxy semantics (pin proxyTunnel: true) #6005

Description

@ggbdpq

What happened

buildProxyDispatcher (packages/runtime/src/network/proxy-dispatcher.ts:47) creates its ProxyAgent without proxyTunnel, so since the undici 7 → 8 major jump (#1247, undici 8.7.0; now 8.11.2) plain-HTTP targets are forwarded to the proxy as absolute-form requests instead of being CONNECT-tunneled. Undici 7 tunneled every request; undici 8 only tunnels non-http: protocols unless proxyTunnel: true is passed explicitly (packages/runtime/node_modules/undici/lib/dispatcher/proxy-agent.js):

function shouldProxyTunnel (requestProtocol, proxyTunnel) {
  return proxyTunnel === true || requestProtocol !== 'http:'
}

Minimal offline probe (fake proxy that records what arrives, then tunnels CONNECT to a local target), against the installed undici 8.11.2:

[default (no proxyTunnel)] body=proxy-forward-ok   proxy saw: GET http://127.0.0.1:<target>/models
[proxyTunnel:true]         body=target-ok          proxy saw: CONNECT 127.0.0.1:<target>

Why it matters

  • The dispatcher's clientFactory composition exists to keep CONNECT-tunnel sockets cancellable ("CONNECT transfers the tunnel out of the proxy pool before target TLS settles"). Under forward semantics that upgrade hook never engages for http: targets, so the abort/cleanup contract silently does not apply to plain-HTTP proxying.
  • CONNECT-only proxies (or proxies whose forward path misbehaves) break for http: targets. hqhq1025's P2 on ci(perf): retire the broken transcript data-plane benchmark #5805 (2026-09-29) verified a hang of the existing closed successful connections ... (http) suite shape against real undici majors and asked to "preserve the previous HTTP behavior and add a focused proxy regression test".
  • Current success-path coverage passes by accident of the forward semantics: the fake proxy in scoped-fetch-transport.test.ts answers any request with 200 proxy-ok, so an absolute-form forward never proves the proxy chain reached a real target.

History / intent

  • #5043 (2026-09-27) already asserts the tunnel wire format elsewhere: startStalledProxy requires CONNECT provider.invalid:443 for its http stages, and startConnectProxy accepts both CONNECT-then-origin-form and absolute-form. Tunneling is the codebase's working assumption; I found no decision adopting forward semantics.
  • This came out of the dependency-graph review of ci(perf): retire the broken transcript data-plane benchmark #5805; the finding was confirmed there to belong on main, not that PR.

Proposal

Pass proxyTunnel: true explicitly in buildProxyDispatcher (restoring the undici 7 behavior for all targets) and add a focused regression test asserting that an http:// target through buildProxyDispatcher reaches the proxy as CONNECT and gets its response from the tunneled target.

Activity

  1. ggbdpq commented on Oct 8, 2026

    @ggbdpq
    ContributorAuthor

    take

  2. ggbdpq commented on Oct 8, 2026

    @ggbdpq
    ContributorAuthor

    Closing with the adjudication recorded, since the report's premise did not survive verification.

    What was re-checked, claim by claim:

    1. The behavior is not a silent regression — it is the tested contract. Undici 8 (#1247) forwards plain-HTTP targets in absolute form unless proxyTunnel: true. That behavior has been on main for ~3.5 months with green CI, and the eval fixture (packages/eval/src/__tests__/maka-initialization.test.ts) explicitly asserts it: it serves absolute-form URLs, requires proxy-authorization on the forward request, and answers CONNECT with 403 ("Unexpected CONNECT"). Pinning proxyTunnel: true turns that suite red (test(runtime): prove the proxied HTTP forward path end-to-end #6006, run 37790469660).
    2. The hang motive does not reproduce. An ~8s stall on plain-HTTP targets through the proxy was reported in ci(perf): retire the broken transcript data-plane benchmark #5805. On undici 8.11.2 it does not reproduce: local scoped-fetch-transport baseline is 42/42 green, recent main CI runs green.
    3. The cancellation-contract argument was wrong for the forward path. Forwarded requests still go through the dispatcher factory's connect wrapper (withSocketCancellation / buildAbortableConnector); onRequestUpgrade only fires for TLS tunnel upgrades, so no cancellation coverage was lost by forwarding.
    4. Forcing tunnels would be a real-world regression. Squid's stock config (http_access deny CONNECT !SSL_ports) and many corporate proxies reject CONNECT to port 80, so http:// providers behind a LAN proxy (Ollama, vLLM, ...) would go from working to 403.

    What was done instead: the one legitimate finding — the runtime success-path test used a fake proxy that answered a fixed 200 to anything, so the proxied HTTP path was never actually proven — is fixed in #6006 (test-only, head 433b64dbe): a real forwarding proxy now asserts the absolute-form request line and proxy-authorization, relays the raw connection to a live target, and the test fails if the proxy self-answers instead of relaying.

    If tunnel semantics for plain-HTTP targets is ever wanted, it needs a stated reason plus fixture updates (per the #6006 review) — not a silent pin.

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

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions