Skip to content

Strip userinfo from the rendered request-target (RFC 9110 §4.2.4) - #35

Merged
hellerve merged 1 commit into
masterfrom
claude/target-strip-userinfo
Aug 21, 2026
Merged

Strip userinfo from the rendered request-target (RFC 9110 §4.2.4)#35
hellerve merged 1 commit into
masterfrom
claude/target-strip-userinfo

Conversation

@carpentry-agent

Copy link
Copy Markdown

Request.target rendered the request-target as (URI.str u), i.e. the whole URI including any userinfo. A request built from a URI that carries credentials therefore put them on the wire in the request line — one of the most routinely logged parts of a request, so they end up in server and proxy logs.

RFC 9110 §4.2.4 (http) and §4.2.5 (https):

A sender MUST NOT generate a userinfo subcomponent (and its "@" delimiter) when an "http"/"https" URI reference is generated within a message as a request target or header field value.

Before / after, for (Request.get (URI.parse "http://USER:PW@h.example/a/c") …):

GET http://USER:PW@h.example/a/c HTTP/1.1     ; before
GET http://h.example/a/c HTTP/1.1             ; after

What changed

target renders a copy of the URI with user and password set to Nothing. The stored URI is untouched, so a caller can still read the credentials off Request.uri and turn them into an Authorization header — a test pins that.

The form of the target is not touched: absolute-form stays absolute-form ("absolute-form request-target is left alone" still passes), and the empty-path rule from RFC 9112 §3.2.1 still applies. That rule now looks at the stripped copy rather than the original, which is what makes a userinfo-only URI render as / instead of the //user@ it produced before; without that, stripping would have left it with an empty target.

Fragments are still rendered into the target — RFC 9112 §3.2.1 says they should not be. Out of scope here.

Tests

Five assertions next to the three existing request-target tests in test/http.carp. Reverting only the http.carp hunk fails three of them:

Test 'request-target drops user and password' failed:
	Expected value: 'GET http://h.example/a/c HTTP/1.1…', actual value: 'GET http://USER:PW@h.example/a/c HTTP/1.1…'
Test 'request-target drops a user without a password' failed:
Test 'request-target of a userinfo-only URI is /' failed:
	Passed: 394	Failed: 3

The other two pass in both trees on purpose: request-target without userinfo is unchanged is the no-regression control, and the credentials stay on the request’s URI pins that only the rendering changed.

Checks

  • carp -x test/http.carp — 397 passed, 0 failed
  • carp -x gendocs.carp — clean, no docs diff (target is private)
  • carp-fmt --check http.carp test/http.carp — clean
  • angler http.carp test/http.carp — clean

git merge-tree --write-tree against #34's head (185af3a) merges cleanly; that PR reopens defmodule Request further down the file and does not touch target.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

RFC 9110 §4.2.4 and §4.2.5: "A sender MUST NOT generate a userinfo
subcomponent (and its "@" delimiter) when an 'http'/'https' URI
reference is generated within a message as a request target or header
field value."

`Request.target` rendered the whole URI, so a request built from a URI
carrying credentials put them on the wire — and therefore into server
and proxy logs — as `GET http://USER:PW@h.example/a/c HTTP/1.1`.

The strip happens on a copy, so the credentials stay on the request's
URI for whoever wants to turn them into an `Authorization` header. The
empty-path special case (RFC 9112 §3.2.1) now tests the stripped URI,
which also gives a userinfo-only URI a valid `/` target instead of the
`//user@` it rendered before.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Build & Tests

carp -x test/http.carp on 9dbdb86397 passed, 0 failed. CI green on both runners, both check runs at the current head sha. angler, carp-fmt --check and carp -x gendocs.carp (no docs/ diff) all clean. No CHANGELOG in this repo.

Teeth reproduce exactly: with only the http.carp hunk reverted, the suite is 394 / 3, and the three that fail are the three named in the description. The two control assertions (request-target without userinfo is unchanged, the credentials stay on the request's URI) pass in both trees, which is what makes them controls rather than padding.

I also merged this with #34 and ran the result, since merge-tree being clean is not the same as the two changes being compatible. The merged tree carries both (RFC 9110 §4.2.4 at http.carp:315, 29 ETag occurrences) and is 452 passed, 0 failed.

Findings

Nothing blocking. What I checked:

Differential over the whole request-target surface

28 parsed URIs plus 6 synthetic ones the parser cannot produce, rendered through Request.str on master and on the branch. 21 rows change, and every one of them removes userinfo and changes nothing else. Two of those rows are worth naming because the description does not:

http://:PW@h.example/a/c    master: GET http://:PW@h.example/a/c    branch: GET http://h.example/a/c
http://@h.example/a/c       master: GET http://@h.example/a/c       branch: GET http://h.example/a/c

A password with no user also reached the wire on masterURI.parse gives user = Just "" there, so URI.str's apply on the user branch fires and renders :PW@. The fix covers it because it blanks the component rather than the rendering, but it is a second leak shape and a reason the fix is in the right place.

The same property shows up as a non-regression I specifically went looking for: a USER:PW@ sequence in the query is untouched.

http://USER:PW@h.example/a%2Fb?u=USER:PW@x#z  ->  GET http://h.example/a%2Fb?u=USER:PW@x#z

A textual scrub would have eaten that. Setting user/password to Nothing and re-rendering cannot.

Everything without userinfo is byte-identical across the two trees, including //:8080 and // from degenerate synthetic URIs, mailto:a@b, path-only, query-only and fragment-only URIs, [::1]:8080, and the default-port elisions for :80/:443.

The dropped user conjunct in the empty-target guard

Removing (Maybe.nothing? (URI.user u)) is safe rather than merely tidy: v has user = Nothing by construction, so the conjunct would be vacuously true. I checked the guard cannot now produce an empty target, which is the failure mode that removing a conjunct invites — a URI reaching URI.str with everything blank but password still renders / (SYN password-only), because a password alone never renders without a user.

The behaviour change the description calls out is real and is an improvement: a userinfo-only URI went from GET //solo@ HTTP/1.1 to GET / HTTP/1.1, and //solo@?a=1 to /?a=1.

The stored URI really does survive, on both construction paths

The test pins it for Request.get. It also holds for a parsed request, which matters more — a server reading credentials off an incoming absolute-form request line still gets them, while a forwarded copy no longer carries them:

GET http://USER:PW@h.example/a/c HTTP/1.1   (parsed)
  Request.uri user     = Just(USER)
  Request.uri password = Just(PW)
  Request.str          = GET http://h.example/a/c HTTP/1.1

This is genuinely the wire form, not just a rendering

Before checking out this branch I pointed a raw socket at http-client (which pins http@0.4.2, i.e. master) and logged the actual request lines:

GET http://USER:PW@127.0.0.1:8899/a/hop1 HTTP/1.1     ; hop 1
GET http://USER:PW@127.0.0.1:8899/a/c HTTP/1.1        ; hop 2, relative Location
GET http://USER:PW@127.0.0.1:8899/ HTTP/1.1           ; hop 2, Location: /

Client.build-and-send sends (Request.str &req) verbatim, and those bytes are exactly what Request.str returns on master, so the premise holds on a real socket and not only in a test harness. Credentials were on the wire on every hop.

Two leads, both out of scope here and correctly named as such

  • Fragments are still rendered into the target, which I confirmed (GET http://h.example:8080/a?x=1#f). RFC 9112 §3.2.1 makes that a MUST NOT too, and it is now literally one more URI.set-fragment … (Maybe.Nothing) in the same let — the hard part (deciding to blank components rather than post-process the string) is done by this PR.
  • The reason userinfo could reach a request line at all is absolute-form. http does not force it — target renders whatever URI it is handed — but http-client's build-and-send hands it the whole parsed URI, so every request it makes goes out as GET http://host/path. RFC 9112 §3.2.1 says a client making a request directly to an origin server sends only the path and query; origin-form would have made both this bug and the fragment one unreachable. test/http.carp:429 deliberately pins absolute-form here, so that belongs in http-client rather than in this PR, but it is worth knowing that this fix is treating a symptom of that choice.

Verdict: merge

A MUST NOT violation that put credentials into every server and proxy log, fixed by blanking the components rather than scrubbing the string — which is why a USER:PW@ inside a query survives and a :PW@ with an empty user does not. 21 changed rows across a 34-URI differential, all of them removals of userinfo and nothing else, and the merged tree with #34 is green.

@hellerve
hellerve merged commit 5d02355 into master Aug 21, 2026
2 checks passed
@hellerve
hellerve deleted the claude/target-strip-userinfo branch August 21, 2026 02:40
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