sixfold-space/madtea

SHA256

Fork 0

security: route every HTTP client through one hardened builder (closes the config redirect leak) #709

Merged

hexajon merged 6 commits from feat/http-transport-chokepoint into main

2026-10-04 18:02:53 +00:00

hexajon commented

2026-10-04 18:02:51 +00:00

Owner

Summary

Closes #708.

madtea built three HTTP clients independently, and only the gitea API client carried the redirect credential guard. The config bootstrap client kept its Authorization header across a same-host https -> http downgrade redirect, so the API token (on whoami and the pre-trust probes) and the OAuth token (on mint and the code-exchange POST) could leak in cleartext.

This routes every client through one base-layer builder, internal/httpx, which owns the redirect credential strip, the mutating-redirect stop, and the response body cap. Credential resolution stays in internal/config. A parity gate test fails if any production file outside internal/httpx builds a raw http.Client, so the chokepoint cannot be bypassed.

feat(httpx): the hardened client builder, with unit tests for the redirect policy (strip off-origin, stop a mutating redirect, cap hops) and the body cap.
refactor(gitea): build through internal/httpx; its local copies deleted. The existing redirect, token-pin, and body-cap tests pass unchanged.
fix(config): route the bootstrap client through internal/httpx, closing the leak across its three call sites. A test proves the token is stripped across an off-origin redirect.
refactor(setup): route the connectivity probe through internal/httpx.
test(parity): fail the gate on a raw HTTP client outside internal/httpx.
docs(adr): proposed ADR 0049, grounded in ADR 0041 and ADR 0025 section 5.

The only behavior change for a legitimate user is that a credential is stripped on a malicious downgrade or cross-origin redirect. The full gate passes.

ADR 0049 is proposed, not accepted: acceptance needs a second contributor's assent per ADR 0047.

## Summary Closes #708. madtea built three HTTP clients independently, and only the gitea API client carried the redirect credential guard. The config bootstrap client kept its `Authorization` header across a same-host `https -> http` downgrade redirect, so the API token (on `whoami` and the pre-trust probes) and the OAuth token (on mint and the code-exchange POST) could leak in cleartext. This routes every client through one base-layer builder, `internal/httpx`, which owns the redirect credential strip, the mutating-redirect stop, and the response body cap. Credential resolution stays in `internal/config`. A parity gate test fails if any production file outside `internal/httpx` builds a raw `http.Client`, so the chokepoint cannot be bypassed. - `feat(httpx)`: the hardened client builder, with unit tests for the redirect policy (strip off-origin, stop a mutating redirect, cap hops) and the body cap. - `refactor(gitea)`: build through `internal/httpx`; its local copies deleted. The existing redirect, token-pin, and body-cap tests pass unchanged. - `fix(config)`: route the bootstrap client through `internal/httpx`, closing the leak across its three call sites. A test proves the token is stripped across an off-origin redirect. - `refactor(setup)`: route the connectivity probe through `internal/httpx`. - `test(parity)`: fail the gate on a raw HTTP client outside `internal/httpx`. - `docs(adr)`: proposed ADR 0049, grounded in ADR 0041 and ADR 0025 section 5. The only behavior change for a legitimate user is that a credential is stripped on a malicious downgrade or cross-origin redirect. The full gate passes. ADR 0049 is proposed, not accepted: acceptance needs a second contributor's assent per ADR 0047.

hexajon added 6 commits

2026-10-04 18:02:51 +00:00

feat(httpx): add the hardened HTTP client builder 9f9ad885df

Add internal/httpx, the base-layer HTTP client builder. It wires one
CheckRedirect policy that strips Authorization on any non-same-origin or
scheme-downgrade redirect hop, stops a method-changing redirect, and caps
the response body at 64 MiB. The config, gitea, and setup clients will all
route through it, so the transport security policy lives in one place and
cannot drift between callers.

Refs #708.

refactor(gitea): build HTTP clients through internal/httpx 69c965f37d

Route the gitea client through internal/httpx and delete its local copies of
the hardened primitives, so there is one implementation of the redirect
credential guard, the same-origin check, and the response body cap. Export
httpx.RedirectPolicy so the gitea redirect tests exercise the one policy by
name. Behavior is unchanged: the gitea client's redirect, token-pin, and
body-cap tests pass as before. Also update stale comments that named the
moved symbols.

Refs #708.

fix(config): route the bootstrap HTTP client through internal/httpx 571d05fab1

The config bootstrap client was a bare http.Client with no redirect policy,
so it kept the Authorization header across an off-origin redirect (an
https->http downgrade, a port change, or a subdomain) - net/http strips the
header only on a host change. That client carries a credential on three call
sites: the whoami and pre-trust probes, the OAuth token mint, and the OAuth
code-exchange POST. Building it through httpx.Build gives it the one hardened
policy, which strips the credential off-origin and stops a mutating redirect,
so all three call sites are covered at once. Also dedupe DefaultHTTPTimeout to
the httpx constant so the value has one home.

Refs #708.

refactor(setup): route the connectivity probe through internal/httpx 4de3b7c98f

The setup connectivity probe built a bare http.Client. It carries no token, so
this is consistency, not a leak fix: routed through httpx.Build, all three of
madtea's HTTP clients now share the one hardened policy, and a guard test can
assert no client is built outside internal/httpx.

Refs #708.

test(parity): fail the gate on a raw HTTP client outside internal/httpx 0eec71a8bb

Add a source-scanning gate test that fails if any production Go file outside
internal/httpx builds an http.Client, references http.DefaultClient, or uses
http.Get/Head/Post/PostForm. It parses the AST, so comments and strings that
only mention these never match, and it resolves the net/http import's local
name so an aliased import still matches. This makes the transport chokepoint
un-bypassable: a new bare client cannot slip back in unnoticed, which is the
drift the consolidation exists to prevent.

Refs #708.

docs(adr): add proposed ADR 0049 for the HTTP transport chokepoint

drift / govulncheck (pull_request) Has been skipped

Details

drift / surface-diff (pull_request) Successful in 50s

Details

ci / mcp-conformance (pull_request) Successful in 1m41s

Details

drift / govulncheck (push) Has been skipped

Details

drift / forgejo-smoke (push) Has been skipped

Details

drift / forgejo-smoke (pull_request) Has been skipped

Details

ci / mcp-conformance (push) Successful in -6s

Details

drift / surface-diff (push) Successful in -16s

Details

ci / gate (push) Successful in 12m15s

Details

ci / gate (pull_request) Successful in 20m54s

Details

9029a95297

Record the decision behind #708 as a proposed ADR: all outbound HTTP goes
through one hardened client layer (internal/httpx), which extends ADR 0041's
one-chokepoint rule across packages and realizes the per-hop half of ADR 0025
section 5. Status is proposed; acceptance needs second-contributor assent per
ADR 0047.

Refs #708.

hexajon merged commit 9029a95297 into main

2026-10-04 18:02:53 +00:00

hexajon referenced this pull request

2026-10-04 18:37:19 +00:00

security: consolidate the HTTP clients through one hardened builder (the config client leaks a credential on redirect) #708

hexajon referenced this pull request

2026-10-04 18:44:43 +00:00

docs(adr): accept ADR 0049 #713