Skip to content

undici transport parity: header rejections degrade, fileBody short-write detection, SOCKS refusal (#81) - #96

Merged
Wahbeh-Mohammad merged 7 commits into
audit/remediation-67from
audit/67/81-undici-parity
Sep 5, 2026
Merged

undici transport parity: header rejections degrade, fileBody short-write detection, SOCKS refusal (#81)#96
Wahbeh-Mohammad merged 7 commits into
audit/remediation-67from
audit/67/81-undici-parity

Conversation

@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor

Closes #81. Part of #67, milestone 5. Wave 6a; #82 lands on top of this.

Decision followed: D19, with two documented departures from its letter (below).

What changed

1. Headers the native client refuses degrade to a logged drop (TRANSPORT-11/12/13)

undiciTransport().send() failed outright for four model-valid headers. undici 6.28.0 rejects
expect (NotSupportedError), keep-alive and upgrade (InvalidArgumentError) unconditionally
(lib/core/request.js:398,409), connection with any value but close/keep-alive (:400-404),
and any name outside RFC 9110 token (lib/core/util.js:547-587) — while @dexpace/core admits
every printable ASCII byte in a name (http/ascii-validation.ts:29-33), so X Custom is
model-valid and unsendable. All five reached toDispatchError as a bare non-retryable TypeError;
nothing was sent.

  • UNDICI_FORBIDDEN_HEADERS gains expect, keep-alive, upgrade.
  • toUndiciHeaders validates each name against RFC 9110 token and each connection value against
    undici's own two (case-insensitively, as undici compares), dropping and logging what fails. Values
    are deliberately not re-checked: undici's value grammar admits obs-text 0x80–0xFF, strictly
    wider than the outbound rule mapOutboundHeaders already applied (HTTP-18).
  • connection stays out of the by-name drop set — §17's own note — so close/keep-alive still
    reach the wire and dropsConnectionHeader: false is still true.

FETCH_FORBIDDEN_HEADERS widened too, and a row proves it. D19 made that conditional. Measured
2026-09-05: on Node, fetchTransport() rejects all three with TransportFailureError
retryable, so a permanent misconfiguration spends the caller's whole retry budget re-proving
itself — because Node's global fetch is undici-backed and undici's Headers deliberately does not
implement the WHATWG forbidden-name list. On Bun 1.3.14 it diverges a third way: expect and
keep-alive go to the wire, and upgrade hangs indefinitely. Neither is TRANSPORT-12's outcome.

2. A file body goes through its own writeTo (BODY-13, TRANSPORT-28)

The file branch of prepareBody handed createReadStream(path, {start, end}) to undici, skipping
the descriptor's writeTo and so @dexpace/body-file's transferred === count invariant.
content-length is dropped outbound, so undici framed the body chunked and the wire could not see a
short write either: a 1 MB fileBody truncated to 10 bytes before send() POSTed 10 bytes and
resolved 200, where @dexpace/transport-fetch raised TransportFailureError.

Departure from D19's letter, accepted: D19 said the file branch "takes the same pumpBody path
the streamed case uses". The branch is deleted instead, so prepareBody is now the same two
decisions on both transports — buffered at or below 1,000,000 declared bytes, streamed above. That
is pumpBody for a large file and the buffered sibling for a small one, and it is the only version
in which the two adapters cannot drift again. The zero-count branch goes with it:
isMaterializable admits contentLength === 0 and materializeBody never opens a read stream.

Framing changed, as D19 asked to be told. A file body at or below 1,000,000 bytes is now sent
with content-length where it used to be chunked. Above that it stays chunked. Both match the fetch
twin exactly.

TRANSPORT-28's zero-copy clause is a SHOULD no user-space path in either client can honour
(docs/deviations.md item 13, recorded since Phase 8a — the createReadStream path was never
sendfile(2) either). Its MUSTs — replayable, and exactly the declared range on the wire — are the
descriptor's, honoured on both transports by one code path, and asserted by a row.

3. A SOCKS proxy is refused at the factory (TRANSPORT-30)

toProxyAgentOptions used proxy.type as the URI scheme unchecked, so
undiciTransport({proxy: createProxyOptions({type: 'socks5', …})}) threw undici's
InvalidArgumentError('Invalid URL protocol: socks5:') straight out of a public factory. Core
resolves ALL_PROXY's socks:, socks4:, socks4a:, socks5: and socks5h: onto ProxyType
quite legitimately (CFG-22, config/proxy.ts:372-380).

selectDispatchers now refuses anything but http with a TypeError naming the type, matching its
existing dispatcher-plus-proxy refusal and deliberately outside the IoError tree so
retry/classify.ts's allow-list makes it non-retryable (RETRY-2). The check runs before
new undici.Agent(...), so a refused construction allocates nothing. @throws on undiciTransport
and on UndiciTransportOptions.proxy. ProxyType keeps socks4/socks5 (D19; narrowing a
@public union is a release-pass call). fetchTransport() needs no equivalent — it has no proxy
option at all.

Test rows added

Shared suite (packages/transport-conformance/src/{run-suite,fixtures}.ts), both transports run
every one
:

Row Before
Expect: 100-continue dropped, logged, request still dispatches red on undici, red on fetch
Keep-Alive: timeout=5 — same red on undici, red on fetch
Upgrade: websocket — same red on undici, red on fetch (hung to the 5 s deadline)
X Custom (non-token name) — same red on undici, green on fetch
Connection: upgrade is dropped on both red on undici
an intact ranged file body puts exactly its declared bytes on the wire, through writeTo red on undici (writes.count === 0)
a file body truncated after its length was captured fails the send, naming transferred-of-total red on undici (resolved 200)
an unsupported proxy type fails at construction, typed and naming the type red on undici (raw InvalidArgumentError)

TransportCapabilities gains an optional unsupportedProxy: {type, build()} — only the adapter
knows which ProxyType values its client refuses. Where it is absent (fetch) the row asserts
supportsProxy === false rather than skipping, so "no proxy surface" and "an unasserted gap" cannot
look alike.

New fixture: fileBodyFixture(path, {start?, count, writes?}) in fixtures.ts — a kind: 'file'
descriptor carrying BODY-13's check itself. Deliberately a stand-in, not the real fileBody():
@dexpace/transport-conformance is private, resolves unbuilt and depends on @dexpace/core
alone, and taking @dexpace/body-file would add a fifth entry to the root build:deps chain plus a
lockfile change for one row — all three files outside this task's partition.

Unit rows in packages/transport-undici/src/undici-transport.test.ts: every refused header absent
from the dispatched argument array with a sibling surviving; Connection carried for
close/Keep-Alive and dropped for upgrade; both SOCKS values refused with no Agent
constructed. The two existing file-body rows now assert writeTo was called (they asserted it
must not be).

Node conformance (tests/node-conformance/transport.test.mjs, both transports):

  • the four refused names on Node, where an undropped one rejects the send and the Bun rows prove a
    weaker claim;
  • the streamed-path truncate-after-stat, with a real fileBody() over a 1.1 MB file.

Findings

Bun 1.3.14's Readable.fromWeb leaks unhandled rejections when the web readable behind it is
aborted mid-pull — two or three per abort. That is exactly what a producer failure on the streamed
request-body path does, and bun:test scores them against whichever row is running. Isolated to
sixteen lines with no SDK code in them; node --test is clean on both transports. It is why the
streamed leg of the truncate row lives in the Node tree, and TRUNCATED_FILE_BYTES's TSDoc says so
in case anyone raises it past 1,000,000. Reachable in production only under Bun, only on a >1 MB
streaming request body whose producer fails mid-flight — not this task's to fix, and not this SDK's
bug, but #82 should know, because D20's producer-failure-race item is in the same code.

packages/transport-fetch/README.md:53 is now stale — it lists the drop set as
"Content-Length, Host, Transfer-Encoding, and Connection". Outside this task's partition
(which names src/fetch-transport.ts only), so left alone per contract item 4. One-line fix for
#82 or the supervisor: add Expect, Keep-Alive and Upgrade to that list.

tests/node-conformance/transport.test.mjs was edited outside the partition, as #72/#76 did
before, for the reason CLAUDE.md's membership rule gives: the divergence is real and measured (Bun
forwards expect/keep-alive and hangs on upgrade; Node rejects all three), and that file is the
only layer where a real fileBody() meets a real transport — its own header comment says so.

For #82

Gates

node .claude/skills/ci-preflight/run-ci.mjs --clean — all 20 steps passed (install,
verify:knowledge-structure, typecheck, lint, build, test, test:scripts, api,
lint:publish, verify:dual-consumption, verify:consumer-types, verify:seam-1,
verify:sse-37, verify:runtime-floor, verify:test-partition, test:examples,
verify:import-cycles, verify:reproducible-build, audit, test:node). 2466 Bun tests across
166 files; 178 Node tests. Run without --node-floor, so CI's Node 20.3.0 leg is untested locally.
housekeeping/probe.mjs: no drift. check-fences.mjs: pass.

api:local regenerated for transport-undici and transport-fetch; both reports are
byte-identical — the changed TSDoc is prose on existing signatures, which api-extractor's report
does not carry. Nothing to commit. bun run api verifies all nine.

Deviations recorded

One row appended at the end of docs/deviations.md's "Deviations recorded outside a phase" (D0):
CFG-22's SOCKS proxy types are resolved by the configuration layer and supported by neither
shipped transport; the refusal is at the transport factory, and ProxyType keeps them.

No phase-ledger edits, no §10 edits. Nothing else is a deviation: TRANSPORT-12 and BODY-13 are now
satisfied rather than departed from, and TRANSPORT-11's drop set is transport-specific by its own
text.

Deferred — release machinery

Suspended under D1; nothing below was done.

  • Patch changeset for @dexpace/transport-undici: send() no longer fails for Expect,
    Keep-Alive, Upgrade, a non-token header name, or a Connection value other than
    close/keep-alive — each is dropped and logged instead (behaviour change: a send that used to
    throw now succeeds without that header). A file body is written through its own writeTo, so a
    short write now fails the send, and a file body at or below 1 MB is framed with content-length
    rather than chunked. undiciTransport() throws TypeError for a non-http proxy.type that
    previously escaped as undici's InvalidArgumentError. Shipped .d.ts prose changed for
    undiciTransport (@throws) and UndiciTransportOptions.proxy.
  • Patch changeset for @dexpace/transport-fetch: Expect, Keep-Alive and Upgrade are now
    dropped outbound. On Node these previously failed the send with a retryable
    TransportFailureError; on Bun Expect/Keep-Alive reached the wire and Upgrade hung. No
    .d.ts change.
  • ProxyType narrowing, and the question behind it. @dexpace/core's ProxyType still admits
    socks4 and socks5, and resolveProxyOptions still resolves them from the environment, while
    no shipped transport can send over one. Dropping them from the union is free only before the first
    version bump — but it would also put @dexpace/core in breach of CFG-22, whose MUST enumerates
    "HTTP, SOCKS4, SOCKS5" for the proxy model. Recommendation: keep the union and leave the
    docs/deviations.md row standing.
    The model is complete and the transport layer is not, which
    is an honest state and one a future SOCKS transport closes without a public-shape change. If the
    release pass disagrees, it is a minor changeset for @dexpace/core (breaking for anyone
    naming the type) plus a matching one for @dexpace/transport-undici.
  • No docs/first-release.md edits.

…adapters

`undiciTransport().send()` failed outright for four model-valid headers, where
TRANSPORT-12 requires the header to give way and the request to dispatch. undici
6.28.0 rejects `expect` (`NotSupportedError`), `keep-alive` and `upgrade`
(`InvalidArgumentError`) unconditionally, `connection` with any value but
`close`/`keep-alive`, and any name outside RFC 9110 `token` -- while
`@dexpace/core` admits every printable ASCII byte in a name, so `X Custom` is
model-valid and unsendable. `toDispatchError` mapped all five to a bare
non-retryable `TypeError`; nothing reached the wire.

The fetch twin was no better on the runtime that ships. Node's global `fetch`
is undici-backed and undici's `Headers` deliberately does not implement the
WHATWG forbidden-name list, so `expect`/`keep-alive`/`upgrade` reach the same
validation and `fetch()` rejects with a bare `TypeError: fetch failed` -- which
this transport can only classify as the RETRYABLE `TransportFailureError`, so a
permanent misconfiguration spends the caller's whole retry budget re-proving
itself. Bun 1.3.14 diverges a third way: it forwards `expect` and `keep-alive`
to the wire and hangs indefinitely on `upgrade`. Measured on both, 2026-09-05.

- `UNDICI_FORBIDDEN_HEADERS` gains `expect`, `keep-alive`, `upgrade`.
- `FETCH_FORBIDDEN_HEADERS` gains the same three (`connection` was already in
  it), which is the widening D19 left conditional on a row proving it.
- `toUndiciHeaders` validates each name against RFC 9110 `token` and each
  `connection` value against undici's own two, dropping and logging what fails
  -- the degrade transport-fetch gets for free from `try`/`catch` around
  `Headers.append`. Value grammar is not re-checked: undici's admits obs-text,
  which is strictly wider than the outbound rule `mapOutboundHeaders` applied.

Five conformance rows both transports run (`Expect`, `Keep-Alive`, `Upgrade`, a
non-token name, `Connection: upgrade`); all five were red on undici and three on
fetch beforehand. Plus two unit rows against a recording dispatcher, and one
Node-conformance case -- the Bun rows prove a weaker claim than Node's, because
only on Node does an undropped name reject the send.

Refs #81, #67. Deviation ledger: none -- TRANSPORT-11's drop set is
transport-specific by its own text, and TRANSPORT-12 is now satisfied rather
than departed from.
…Y-13)

The file branch of `prepareBody` handed `createReadStream(path, {start, end})`
straight to undici. That is one fewer userspace copy, and it skipped the
descriptor's `writeTo` entirely, so `@dexpace/body-file`'s
`transferred === count` invariant never ran. `content-length` is dropped
outbound, so undici framed the body chunked and the wire could not detect a
short write either: a file truncated between `stat` and `send` POSTed its
surviving bytes and resolved 200, where `@dexpace/transport-fetch` raised
`TransportFailureError`. The Phase 8a checklist marked BODY-13 done while
recording the bypass.

The branch is gone rather than rerouted, so `prepareBody` is now the same
function on both transports: buffered at or below 1,000,000 declared bytes,
streamed above. D19 said "takes the same `pumpBody` path the streamed case
uses"; deleting the branch is that for a large file and the buffered sibling
for a small one, and it is the only version in which the two adapters cannot
drift again. Framing changes for a small file body -- `content-length` where it
used to be chunked, matching the fetch twin.

TRANSPORT-28's zero-copy clause is a SHOULD no user-space path in either client
can honour (`docs/deviations.md` item 13, recorded since Phase 8a). Its MUSTs --
a file body is replayable, and exactly its declared byte range reaches the wire
-- are honoured by the descriptor, on both transports, by one code path. The
zero-count case needs no branch of its own: `isMaterializable` admits
`contentLength === 0` and `materializeBody` never opens a read stream.

Rows: a shared conformance row on the buffered path (intact ranged body through
`writeTo`, and truncate-after-stat), red on undici and green on fetch before
this; the streamed leg in `tests/node-conformance/transport.test.mjs` with a
real `fileBody()`, likewise red on undici only. The streamed leg cannot live in
the Bun suite -- Bun 1.3.14's `Readable.fromWeb` leaks the abort reason as
unhandled rejections when the web readable behind it is aborted mid-pull, on
both transports and with no SDK code involved (isolated; `node --test` is
clean). `run-suite.ts`'s constant says so, so nobody raises it back.

Refs #81, #67. Deviation ledger: none -- BODY-13 is now satisfied rather than
departed from, and TRANSPORT-28's SHOULD already has item 13.
…the factory

`toProxyAgentOptions` used `proxy.type` as the URI scheme with no check, so
`undiciTransport({proxy: createProxyOptions({type: 'socks5', …})})` reached
`new ProxyAgent({uri: 'socks5://…'})` and threw undici's
`InvalidArgumentError('Invalid URL protocol: socks5:')` straight out of a public
factory -- untyped, undocumented, and not in the SDK's error vocabulary. The
configuration can express it perfectly legitimately: core maps `ALL_PROXY`'s
`socks:`, `socks4:`, `socks4a:`, `socks5:` and `socks5h:` schemes onto
`ProxyType` (CFG-22, `config/proxy.ts:372-380`).

`selectDispatchers` now refuses anything but `http` with a `TypeError` naming
the type, matching its existing dispatcher-plus-proxy refusal and deliberately
outside the `IoError` tree, so `retry/classify.ts`'s allow-list makes it
non-retryable for free (RETRY-2). The check runs before `new undici.Agent(...)`,
so a refused construction allocates nothing -- there would be no transport left
to close it through. `@throws` on `undiciTransport` and on
`UndiciTransportOptions.proxy` say so.

`ProxyType` keeps `socks4`/`socks5`: narrowing a `@public` union is a
release-pass decision (D1/D19), and the gap is recorded in `docs/deviations.md`
instead. `fetchTransport()` needs no equivalent -- it ships no `proxy` option at
all, deliberately (design doc §6).

The conformance row runs on both transports. `TransportCapabilities` gains an
optional `unsupportedProxy: {type, build()}`, because only the adapter knows
which of `ProxyType`'s values its client refuses; where it is absent the row
asserts `supportsProxy === false` rather than skipping, so "no proxy surface"
and "an unasserted gap" cannot look the same. Red before this change: undici's
raw error is neither a `TypeError` nor names `socks5`. Plus a unit row covering
both SOCKS values and asserting no `Agent` was constructed on the way out.

Refs #81, #67.
`docs/deviations.md` gains one row at the end of "Deviations recorded outside a
phase" (D0): `CFG-22`'s proxy model carries SOCKS4/SOCKS5 in full, core resolves
both from `ALL_PROXY`'s five schemes, and neither shipped transport can send
over one -- undici's `ProxyAgent` is an HTTP CONNECT tunnel and
`@dexpace/transport-fetch` has no proxy option at all. The row records why
`ProxyType` keeps the two values (narrowing a `@public` union is a release-pass
decision, and `CFG-22`'s MUST is about the model, which would then stop
satisfying it) and where the refusal now happens instead.

`write-a-transport.md` grows from nine rules to eleven, both of them things #81
found the shipped transports getting wrong:

- Rule 3, split out of rule 2: whatever your native client refuses, drop that
  header, never the request -- and find out *where* it decides, because WHATWG
  `Headers.append` throws at construction while undici validates inside
  `dispatch`. Getting it wrong does not look like a transport bug; it looks like
  a retryable network failure.
- Rule 9, rewritten: recognise a file body structurally and still write it
  through `writeTo`, because reading `path` yourself skips BODY-13's
  `transferred === count` and `Content-Length` is dropped by rule 2, so nothing
  else can see a short write.
- Rule 10, new: refuse a proxy you cannot honour at construction, typed, named,
  and outside the `IoError` tree.

Plus the new optional `unsupportedProxy` capability in the "Prove it" example.
Fence check passes.

Refs #81, #67.
`fetch-transport.ts:146-152` for the `Headers.append` degrade became `:159-166`
when this branch widened the fetch drop set above it. And `prepareBody` is not
"identical in shape" to the fetch twin's -- the two return different structures;
what is identical is the two decisions it makes and their order, which is what
the comment meant and now says. Plus the undici version behind the
`lib/core/request.js` line numbers, in the one of the three places that omitted
it.

Refs #81, #67.
Two bare `64`s and a `.slice(10, 30)` that had to be read together to see they
agreed. Also re-anchors the header comment's TRANSPORT-22 pointer, which this
branch moved from `undici-transport.test.ts:503` to `:614`.

Refs #81, #67.
`README.md:53` still listed `Content-Length`, `Host`, `Transfer-Encoding` and
`Connection`. `FETCH_FORBIDDEN_HEADERS` gained `Expect`, `Keep-Alive` and
`Upgrade` earlier on this branch, so the sentence has been false since f02dd8e.

Rewritten as three bullets rather than one: which names the client computes and
which the layer underneath refuses; why the refused three are dropped rather
than forwarded (WHATWG names all four forbidden, the implementations enforce
none of it, and they disagree about what happens instead -- Node's undici-backed
`fetch` fails the send with the RETRYABLE `TransportFailureError`, Bun 1.3.14
forwards two to the wire and hangs on `Upgrade`); and that a non-token header
name degrades to the same drop, which the README had never said at all.

`packages/transport-undici/README.md` needed nothing: 6df9645 already widened
its enumeration, including the `Connection`-value split undici alone has.

Refs #81, #67.
@Wahbeh-Mohammad
Wahbeh-Mohammad merged commit 808f6b0 into audit/remediation-67 Sep 5, 2026
1 check passed
@Wahbeh-Mohammad
Wahbeh-Mohammad deleted the audit/67/81-undici-parity branch September 7, 2026 18:42
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.

1 participant