Skip to content

feat(http): a byte transport for signed GET and HEAD on the shared HTTP layer - #1632

Merged
cevheri merged 37 commits into
mainfrom
feat/s3-transport
Oct 9, 2026
Merged

cevheri merged 37 commits into
mainfrom
feat/s3-transport

Conversation

@cevheri

@cevheri cevheri commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Adds a byte transport beside the shared REST transport, so a provider that reads stored objects can send signed GET and HEAD requests and read bytes, without changing anything the shipped HTTP providers send or receive.

What changed:

  • createNodeByteTransport in src/lib/db/http/node-transport.ts, on one core shared with createNodeTransport: GET and HEAD only, an exact request target that is never parsed (so . and .. segments reach the server as given), a Host header set by the transport, a synchronous per-request signer that may add only the header names it lists and is the only way to set authorization, connection headers held to the same rule as per-request ones, every field of a request read once, opt-in response headers capped at 64 and never Location or Set-Cookie, truncateAt, stored content-encodings passed through undecoded, and a refused redirect that carries its status and selected headers.
  • The byte transport never reaches a link-local address or AWS's IPv6 instance metadata address, whatever DB_HTTP_BLOCK_PRIVATE_HOSTS says: a literal is refused when the transport is built, every socket is opened through a lookup that checks its answer, and a zoned IPv6 address counts as link-local (src/lib/db/http/egress-policy.ts).
  • The socket queue drains in one loop, a request cancelled while queued is never signed or sent, and a 101 answer is refused at once, with or without an Upgrade header; repeated content-encoding values are returned joined, as node:http joins them.
  • Strict RFC 3986 encoders and originHost in src/lib/db/http/endpoint.ts; isLoopbackHost is now exported, unchanged.
  • TransportError gains an optional redirect field, present only on a refused redirect; no new kind.
  • CI runs the transport's runtime cases once more on Node 26.10.0, the image's runtime, inside the existing Unit & Integration Tests job.
  • Docs: ADDING_A_PROVIDER, SECURITY row 0.6 and its note and known limits, ARCHITECTURE, and backlog entries D254 to D260, DOC19 and DOC20; D255 to D260 record text-transport defects the review found that main has as well.

Why: the S3-compatible object storage provider that follows needs SigV4-signed GET and HEAD, binary bodies, response headers and byte-exact keys, none of which the text transport offers; doing it as a second factory keeps every existing caller and test fake unchanged.

Tests: five new test files and appended cases in two existing ones; the only existing test line changed is the runtimes file's spawn line, which now starts an unbundled entry that patches node:dns before the bundle loads. Red-teamed in three lenses, with a check of the fixes, then reviewed by OpenCode, Antigravity and a fresh Opus reviewer (Cursor and Kiro were out of quota); every finding is fixed with a test or filed.

Verification:

  • format, lint, typecheck, knip, the four drift guards, test (1,133 files, 44,002 tests: 43,999 pass, 3 skip), build, build:lib and attw all pass on the head with main merged; coverage 121,823 of 121,823 lines.
  • Runtime cases: 192 pass, with children on Bun 1.4.2, Node v24.14.0 and Node v26.10.0, among them the 101 refusal in both forms, .. and %2E%2E targets byte for byte, one Host line equal to the signed one, and a latin1 response header.
  • Live, on a production build: Databend and Qdrant browse, query, cancel and overview as before, with integers above 2^53 exact; agent plan mode on both drafted reads and executed nothing; the SQLite, PostgreSQL and SQL Server regression passed in the editor, plan mode and agent mode.

cevheri added 30 commits October 9, 2026 23:12
…ral and by DNS answer

The pinned lookup and its answer check move into one private helper, checkedLookup, which publicAddressLookup and the new linkLocalRefusingLookup both build on.
createNodeByteTransport sends GET and HEAD on the connection's own Agent, writes the checked request target byte for byte, sets Host from the origin on every request and returns the body as bytes, never decoded.
Both factories now build through one private connectionOf, admit and dispatch, so the build-time checks, the closed and aborted refusals and the request write exist once; the text transport's behaviour is unchanged.
…ith a query

A path-only target of exactly 16384 bytes was refused because the cap counted a "?" that is never sent.
The boundary tests now send 16384 bytes and refuse 16385, path-only and with a query.
…us and selected headers

Both transports now take the 3xx refusal from one private helper, so the
byte path's message is the text path's, and the redirect detail's shape is
one named type.
request() reads method, signal, maxResponseBytes and truncateAt once, checks those reads and hands them to the exchange, so a getter that answers differently on a later read can no longer change what is sent after the check, as the target and the headers already were.
Bun's BlockList does not match fe80::1%eth0 against fe80::/10 where Node's does, so a zoned link-local literal or DNS answer passed both link-local checks on Bun. Any IPv6 literal or answer with a zone index is now refused with the link-local sentence on every runtime.
…ent is built

checkedSigner snapshots the signer's header list once and checks and keeps that copy. connectionOf now returns the core's settings instead of building the core, so the byte factory checks its signer before any Agent exists, and its link-local rule is a named field rather than a bare true.
…des the body, as on main

The shared core moved the settled check into resolve(), after the body was joined and decoded. The end handler now returns first when the request has settled, the order main had.
…o, no more

The rfc3986 encoders percent-encode every byte outside the unreserved set but do not make every target valid; a 304 is a refused redirect like any 3xx; the cut path's leftover responded flag is harmless because a late failure is ignored.
…p assertions that cannot fail

Adds a lone low surrogate, a valid astral pair and the empty-segment and empty-name outputs to the encoder tests, a 3xx with more selected headers than the cap, and an explicit maxResponseBytes in the truncateAt refusal. Drops the accepted-count check on literals refused when the transport is built and the runtimes check on metadata.test, neither of which could fail.
D255 records that the text transport reads maxResponseBytes again after checking it and never checks the method. D254 names the NAT64 local-use prefix as a second candidate and says CGNAT is refused only with DB_HTTP_BLOCK_PRIVATE_HOSTS on. DOC20 says the label table has no row for three labels.
…54's NAT64 prefix

The selection comment still said connectionOf builds the Agent, which
the signer change made false: the core builds it. D254 now names the
well-known NAT64 prefix its first sentence covers, beside the local-use
prefix its second sentence names.
…rts cannot overflow the stack

A request that fails as it starts, as one whose signer throws does, freed its slot and started the next waiting
request from inside its own failure. Past about 1,800 such requests on Node and 6,000 on Bun the stack overflowed: the
RangeError escaped the first request's end listener, ending the process on Node, and the rest never settled.
The queue now starts waiting requests in one re-entrancy-guarded loop.
…eady cancelled

When one signal cancels a running request and the requests queued behind it, the running request's failure frees
its slot before the next request's own abort listener has run, so that request was signed and handed to the Agent,
which dialled a socket for it. The byte transport now fails it as cancelled before it is signed.
- With DB_HTTP_BLOCK_PRIVATE_HOSTS on, the byte transport's lookup runs the link-local check after the guard's, so a
  zoned fe80::/10 answer, which Bun's BlockList misses, is refused instead of dialled.
- The byte factory holds its connection headers to the rule a request's own headers meet: a plain record of string
  values, visible ASCII, no owned name, authorization included.
- A TransportError carries redirect as an own property only when it has a redirect detail.
- A truncated answer frees its socket slot once its request has closed, so the next request is never handed to an
  Agent that still counts the cut socket.
- A byte request is admitted again once every field has been read, so a getter that cancels or closes is not sent.
- A 101 answer fails a byte request at once as a network failure and destroys the switched socket.
…rry authorization, and what the fixes guarantee
…itten

Connection header names were lower-cased before the token check, so a
name spelled with the Kelvin sign became the token "key", and two
spellings of one name collapsed into whichever came last. The name is
now checked as written and refused when another spelling of it was
given. D256 also records the queued form case, where the TypeError
escapes an earlier request's end listener.
@codecov

codecov Bot commented Oct 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

A 101 without both Connection: upgrade and an Upgrade header never reaches the upgrade event: node:http hands it to the
response callback, where it resolved as status 101 and its socket went back to the pool. The byte transport now
refuses it with the same switched-protocols failure and destroys its socket.
@cevheri
cevheri marked this pull request as draft October 9, 2026 22:50
…s, Host line and latin1 header on Node too

The runtimes byte cases now cover both forms of a 101, the .. and %2E%2E targets, one Host line equal to originHost
on every byte request, and a selected header with a UTF-8 value read as latin1, on Bun and on each Node child. The
socket-count test is renamed to the cut-answer case it exercises.
node:http joins repeated content-encoding lines with a comma where it keeps the first content-type, so the field
docblock and the module docblock no longer say the two are read the same way.
@cevheri
cevheri marked this pull request as ready for review October 9, 2026 23:03
@sonarqubecloud

sonarqubecloud Bot commented Oct 9, 2026

Copy link
Copy Markdown

@cevheri
cevheri merged commit da95617 into main Oct 9, 2026
36 of 42 checks passed
@cevheri
cevheri deleted the feat/s3-transport branch October 9, 2026 23:45
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