Skip to content

Never rewrite a request target silently - #698

Merged
ronleizrowice-ant merged 9 commits into
mainfrom
fix/never-rewrite-a-request-target-silently
Oct 16, 2026
Merged

ronleizrowice-ant merged 9 commits into
mainfrom
fix/never-rewrite-a-request-target-silently

Conversation

@ronleizrowice-ant

@ronleizrowice-ant ronleizrowice-ant commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Waiting for a second reviewer. It is approved and every check passes, but the branch rules want the most recent reviewable push approved by someone other than whoever pushed it. Here that push is the merge of #702 into this branch, so an approval from whoever made that merge does not count. An approval from anyone else with write access is enough: auto-merge is on and will take it. Nothing has changed since but a merge of main.


This branch now holds #702 too, which was stacked on it and has been merged into it (9ce832f: five files, +150/-16). So merging this PR lands both. See "Three small ends (#702)" at the foot, and #702 for its own description and figures.

While a filterRequest is set, main forwards the request target normalised (#663; in no release). So:

  • A request signed over its path no longer verifies. 33 of 66 SigV4 operations on object keys with a dot segment or a leading / fail.
  • With a masked AWS credential it is re-signed, validly, for another path. On an object store that takes the path literally, PUT /a/../b.txt overwrites b.txt, and /bucket/../up.txt leaves the bucket. Each reports success: 22 of 44.

0.0.79 forwards the spelling, and shows the callback only a parsed URL: /denied/../allowed/x is judged as /allowed/x, while a literal upstream acts under denied/. That is what #663 set out to close.

What changes

New network.ambiguousRequestTarget, for a target that names another path once normalised: a dot segment (%2e included), leading //, or \ in the path.

on the wire
"refuse" (default) nothing 400 with X-Proxy-Error: ambiguous-request-target before the callback is asked. The body names the option. Recorded as a violation. The connection ends, as after a denial.
"asSpelled" the spelling The callback has to hold for every reading an upstream may make: /a\b/../admin, normalised /a/admin, is /admin to one that keeps the \.
"normalize" the normalised form main's behaviour.
  • Every other target goes out byte for byte. A byte outside ASCII, which only Bun's plain-HTTP listener lets through, goes percent-encoded, as in 0.0.79.
  • A # and what follows are cut off first, in every mode. Python's urllib, Perl's LWP and Java's HttpClient leave the fragment in the absolute form they send a proxy, and 0.0.79 dropped it there. The callback, a signature, the upstream and a violation line get the target without it. %23 is data.
  • 400 in every mode: bytes that are not UTF-8 (the runtime has replaced them; 0.0.79 forwards %EF%BF%BD, another resource).
  • In a terminated tunnel, a request the runtime's HTTP parser refuses is answered and recorded, with or without a filterRequest: 400 with what to do (curl 'https://host/x?q=café': percent-encode the bytes) and Connection: close. It used to be a closed connection with no answer.
  • Every refusal of a target is recorded as a violation, and the line shows the URL as spelled, its query or fragment cut.
  • The callback gets requestTarget (as spelled, without a fragment), normalizedRequestTarget and requestTargetIsAmbiguous. request.url means what it did in 0.0.79.
  • Re-signing covers exactly the bytes sent.
  • Without a filterRequest, and in srt proxy, nothing changes on the wire to the upstream.

The trade-off, for whoever decides

Neither form is safe to forward silently. The spelling lets a literal upstream act on what the policy did not judge. The normalised form changes which resource the request names. So the default forwards neither, and says so.

Its cost: a caller who upgrades from 0.0.79 with a filterRequest gets 400s for requests that worked, on every host. Of ten common clients, eight send a leading // as typed (curl, wget, git, Node's fetch among them). Only curl --path-as-is and urllib send a raw ... None sends # to an origin. "asSpelled" keeps all they send. ..%2f, ..; and an inner // are the same in both forms, and stay the policy's own matter.

Testing

  • 104 targets, TLS and plain, on Node 22, Bun 1.4.2 and 1.4.3, against 0.0.79 and main: main rewrites 34. Here nothing is rewritten outside "normalize", and the default refuses 23.
  • The SigV4 pair through the public config: default 33 + 22 answered 400, none on another object; "asSpelled" 66 of 66 and 44 of 44, as on 0.0.79.
  • Real clients through the proxy: urllib with a fragment, with and without a path, gets 200 and 0.0.79's wire. curl with café in the query over TLS gets the 400 and its advice.
  • 504 tests in request-target-forwarding, of which about 475 fail on main's source. 173 mutations: 171 fail a test, and two change no answer.
  • With user:secret@ in the authority, a token in the query and one in the fragment, on both legs and in an absolute-form target inside a tunnel: none reaches a violation line. A request line with an injected header line is recorded as the authority alone.
  • Tests changed: the 14 cases of "the target it judges is the target forwarded" become per-mode tests. 20 tests in request-rewriting now run under "normalize".

Not covered

  • Plain HTTP without a filterRequest forwards the parsed path, as in 0.0.79.
  • A raw byte outside ASCII in a tunnel is refused, not served. The parser gives up before any handler sees the request, its lenient switch also loosens header checks, and rewriting bytes ahead of it needs a parser of our own.
  • Under Node, on plain HTTP, the parser's own refusals (a raw byte outside ASCII, http://host#frag) stay a bare 400, unrecorded, as in 0.0.79: nothing has authenticated the sender at that point.
  • srt proxy's strict spelling still answers # with 400 on its plain leg.
  • Not recorded either: a request without Host, requireAsciiHost, the SigV4 refusals, 407.
  • The parent-proxy route.

Merge order

Meets #700 in one list of options (sandbox-manager.ts). Whichever merges second needs main merged in.

Three small ends (#702)

  1. A refused upload keeps its answer. The 400 for a request the parser refuses, above, had its socket destroyed as soon as it was flushed. With a body still on its way, the close became a reset that discarded the answer (curl: (56); the 400 arrived in 2 to 5 of 40). The socket is now ended, and destroyed a second later, as after a denial: 480 of 480.
  2. The mux's 502 is an answer, for a client that has had none. Its handler for a failed dial ran on any error of the backend connection, so a clear-text 502 … dial failed was written into established connections, TLS tunnels included (18 to 29 of 30; as in 0.0.79). It is now written only where nothing has gone back to the client yet.
  3. OPTIONS * is judged under its own host. The URL for filterRequest was the authority and the target put together: https://host*. The target URI of the asterisk form has an empty path (RFC 9112 3.3): https://host/, with requestTarget *.

While a filterRequest is set the request target was forwarded normalized. A
request signed over its path then no longer verifies, and with a masked AWS
credential it is re-signed, validly, for another path.

network.ambiguousRequestTarget says what becomes of a target that names
another path once normalized: "refuse" (default) answers 400 before the
callback is asked, "asSpelled" forwards the spelling, "normalize" forwards
the normalized form. Every other target goes out byte for byte. The callback
gets both forms, and re-signing covers the bytes sent.
The rule against direct realpath calls and the one call that is made on
purpose met on main.
…y refusal is recorded

A plain-HTTP target with raw UTF-8 bytes, which only Bun's listener lets
through, was answered 400. It is forwarded percent-encoded, as in 0.0.79.
Bytes that are not UTF-8 are refused: the runtime has replaced them, so what
the client named is lost.

A raw "#" is refused in every mode: a request-target has no fragment, no
client sends one, and upstreams differ in what they make of it.

Every refusal of a target is recorded as a violation, and the line shows the
URL as spelled, its query or fragment cut.
…s answered and recorded

A raw "#" in a request-target was answered 400. Python's urllib, Perl's LWP
and Java's HttpClient leave the fragment in the absolute form they send a
proxy, and 0.0.79 dropped it there. It is cut before anything is read, judged,
signed, reported or sent, in every mode and on both legs. A fragment right
after the authority leaves the root.

In a terminated tunnel, a request the runtime's HTTP parser refuses (raw bytes
outside ASCII in the target, as curl sends a query) closed the connection with
no answer and no record. It is answered 400 with what to do, and recorded.
Nothing is written while an earlier response is under way.
… OPTIONS * keeps its host (#702)

* A refused upload keeps its answer, the mux's 502 is for a failed dial, and OPTIONS * keeps its host

A request the parser refuses in a tunnel had its socket destroyed as soon as
the 400 was flushed. A body still on its way, unread, turned the close into a
reset, which discarded the answer. The socket is ended, and destroyed a second
later, as after a denial.

The mux's handler for a failed dial ran on any error of the backend
connection, so a backend that went away under an upload had a clear-text 502
written into an established connection, a TLS tunnel included. Past the dial
the error is logged, and the close that follows ends the client.

The URL given to filterRequest for the asterisk form was the authority and the
"*" put together, which reads the "*" into the host. Its path is empty
(RFC 9112 3.3).

* The mux's 502 is an answer, for a client that has had none

The condition was that the dial had failed. But the mux is in front of plain
HTTP too, and a backend that fails after the dial and before any reply left
the client a bare close where it used to get a readable 502. What makes the
clear text harmful is that something has already gone back.

* test: the mux's 502 for a backend that fails unanswered, on Linux alone

Whether a backend that goes away shows at the mux as an error is the
platform's. On macOS and Windows the client in this case is just closed, so
there is no 502 to tell apart.
@dylan-conway
dylan-conway enabled auto-merge (squash) October 14, 2026 02:12
@dylan-conway
dylan-conway disabled auto-merge October 14, 2026 16:27
@dylan-conway
dylan-conway enabled auto-merge (squash) October 14, 2026 16:27

@antdres antdres left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/stamp

@ronleizrowice-ant
ronleizrowice-ant merged commit 0ded0c8 into main Oct 16, 2026
16 checks passed
@ronleizrowice-ant
ronleizrowice-ant deleted the fix/never-rewrite-a-request-target-silently branch October 16, 2026 01:37
@antdres antdres mentioned this pull request Oct 16, 2026
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.

3 participants