feat(remote): S13 upstream HTTP secure RoundTripper + Client (step 5)

🛡️ What this adds

S13 Step 5 lands the security-hardened upstream HTTP client that every remote and virtual repository will use for its outbound calls (Probe, Fetch, and health probes). It is the piece that lets AR reach an untrusted upstream registry without becoming an SSRF pivot: it re-runs the security covers on every hop, streams responses under hard caps, and never lets a block reason leak network detail back to a caller.

It assembles the already-merged building blocks — the URL/header validators (Step 3) and the resolved-IP secure dialer (Step 4) — under a labkit-instrumented transport, rather than reimplementing them.

📦 The pieces

  • roundtripper.go — the inner security RoundTripper. Runs the URL and header covers on every request, then follows redirects manually up to max_redirects, re-validating each target through the same covers (cover 7) and stripping Authorization before a cross-origin hop (cover 8). A redirect the covers reject surfaces BlockedURLError{redirect_blocked} so the reason leaks no network detail. Response bodies stream through a size cap (cover 13) and, for gzip bodies, a decompression cap (cover 14), both enforced on read.
  • client.go — the outer client. Wraps the security RoundTripper and the secure dialer under labkit/v2/httpclient (tracing + structured logging), maps the package Config onto a net/http.Transport (disabling transparent decompression so the client owns the gzip cap), builds the base TLS config from the shared TLSConfig (mTLS), and wires use_proxy_from_env with a startup WARN naming the one cover a proxy voids (DNS-rebinding protection). Client.Do takes a BodyKind (BodyMetadata/BodyBlob) selecting the configured response-body cap, with a WithMaxBody per-request override clamped to body_size_cap_blob; it applies the total-request deadline (cover 12) and remaps the transport's response-header timeout (cover 11) to ErrResponseHeaderTimeout.

This step owns covers 7–8 and 11–14 plus the redirect re-validation of covers 1–6.

Review follow-ups folded in

This branch went through a full pre-MR review; the fixes are folded into the fix/test commits on top:

  • 🔒 mTLS now survives an outbound proxy. Transport.TLSClientConfig is set so a proxied HTTPS upstream (a CONNECT tunnel, where net/http bypasses DialTLSContext) still presents the configured client certificate and trusts the configured custom CA instead of silently falling back to the system roots. Direct HTTPS keeps dialing through DialTLSContext.
  • 🧹 Cross-origin redirects now strip Cookie, Cookie2, and Www-Authenticate alongside Authorization, matching net/http's own redirect handling.
  • The RoundTripper closes req.Body when a cover rejects the request (contract-honest for future non-NoBody callers).

A second review round hardened three fail-open seams and closed two coverage gaps:

  • 🔒 The decompression cap engages on every gzip spelling. Content-Encoding is parsed as a coding list rather than matched literally, so the registered x-gzip alias and list values such as gzip, identity no longer skip the cover and stream through bounded only by the raw body cap. A coding this seam cannot unwrap still streams undecoded under the raw cap.
  • 🔒 No timeout cover can be switched off by a zero value. NewClient rejects a non-positive request_total_timeout, dns_resolution_timeout, or tls_handshake_timeout, naming the offending field. Do therefore applies the total-request deadline unconditionally — the fail-open branch is gone, not just documented. internal/remote/config.go and the configuration reference state the constraint.
  • 🧹 A gunzipped response no longer lies about its body. When the gzip path engages, Content-Encoding and Content-Length are deleted, ContentLength reads -1, and Uncompressed is set, matching net/http's own transparent decompression — so a downstream caller cannot gunzip already-decompressed bytes a second time and persist a corrupted artifact.
  • 🧪 The request-time covers are now pinned to Client.Do (previously deleting the validateRequest call left the suite green), and the pure-URL half of redirect re-validation is covered (previously only the dialer-surfaced IP block was).

A third review round hardened the step-4 secure dialer this client exercises:

  • 🔒 Own-interface addresses are denied. An upstream resolving to one of the host's own interface addresses (net.InterfaceAddrs, cached with a one-minute TTL) is blocked before dial even when that address is public — the self-request vector http_v2 closes via validate_localhost. A failed interface listing fails the dial closed; an allowlisted destination keeps its bypass.
  • 🔒 A mixed DNS answer fails whole. A resolution containing any denied record (for example [127.0.0.1, 203.0.113.5]) now blocks the request instead of pinning the surviving record, matching http_v2 — a partially-hostile answer is treated as evidence of rebinding or split-horizon attack, not an availability problem.
  • 🔒 outbound_allowlist entries can be port-scoped. host:port and ip:port entries permit only that port; bare host and IP entries keep permitting every port, and CIDR entries stay port-blind. Config load validates the new forms and rejects port 0; the configuration reference and config.example.yaml document the syntax.

A fourth review round closed an IPv6 zone-suffix bypass in the step-4 dialer:

  • 🔒 A zoned IPv6 address is rejected outright. netip.Prefix.Contains never matches an address carrying a zone and netip.Addr equality compares the zone, so a literal such as 2001:db8::5%1 slipped past the fec0::/10 prefix range and the own-interface check while the zone-blind bit predicates still held. candidateAddresses now blocks any candidate with a non-empty zone — literal host or resolved record — before validation, failing closed with the network-detail-free ip_denied reason.

A fifth review round hardened the client assembly and its edges:

  • 🔒 The resolved-address DNS cache is bounded. A resolution is cached only after every record passes validation (a blocked redirect target leaves no entry), an explicit "0s" TTL really disables caching, every store evicts expired entries first, and the map stops growing at 1024 entries — a wildcard DNS zone answering attacker-chosen redirect hostnames can no longer grow it without bound.
  • 🔒 response_header_timeout joins the construction-time validation, closing the last cover-backing timeout a zero could switch off silently.
  • 🔒 NAT64 (64:ff9b::/96) and 6to4 (2002::/16) join the resolved-address deny-list, closing the IPv6 spellings of denied IPv4 addresses (64:ff9b::a9fe:a9fe for the metadata address).
  • 🔒 Unenforceable outbound_allowlist entries fail construction naming the entry, and an IPv4-mapped CIDR entry (::ffff:10.0.0.0/104) is normalized to its IPv4 form instead of being silently inert at dial.
  • 🧹 cappedReader handles a math.MaxInt64 cap without overflowing, ContentLength is clamped to the body cap so a caller cannot size a buffer from an untrusted claim, the scheme/low-port policy slices are cloned at construction, redirect-hop bodies are drained for connection reuse, the unwired MaxConnsPerHost knob is dropped, and the dialer is unexported.
  • 🧪 New pins: the transport's security fields (mTLS-through-proxy config, env-proxy wiring, disabled transparent decompression), the hostname axis of sameOrigin, the dns_resolution_timeout bound, and the at-cap body read.

A sixth review round made the outbound-proxy path enforce the destination covers, matching http_v2:

  • 🔒 Proxied requests validate their destination. With Transport.Proxy set, net/http hands the dialer the proxy's address, so the destination was never checked — under deny_all_except_allowed with only the proxy allowlisted, a proxied request could reach 169.254.169.254. The security RoundTripper now decides per hop whether the request travels through the proxy and, when it does, runs the destination's resolved-address covers itself (deny-list, own-interface check, outbound_allowlist with its port scoping, and deny_all_except_allowed) before the proxy is contacted — best-effort against this process's own resolution, with the resolved-IP pin as the one cover a proxied request drops, exactly as http_v2 relaxes only dns_rebind_protection under a proxy. Hosts exempted by NO_PROXY keep the full direct path, and the proxied/direct decision is remade on every redirect hop so the proxy exemption can never leak into a direct dial.
  • 🔒 The proxy itself is dialable and named. The destination deny-list no longer misapplies to the proxy's own address — a proxy on a private IP works without allowlisting it — and a failed proxy dial reports dial proxy instead of blaming the upstream. The startup WARN and the configuration reference now describe what actually holds.
  • 📝 Spec amendment flagged for work item 320: the spec's Outbound proxy section states the resolved-address deny-list (cover 5) cannot be enforced for proxied requests, which diverges from the http_v2 reference it cites — http_v2 enforces it best-effort and relaxes only DNS-rebinding protection (cover 6). This MR implements the http_v2 behavior; the spec text is untouched here and needs amending.

A seventh review round made the operator's size caps enforced bounds:

  • 🔒 body_size_cap_blob is a hard ceiling, and both caps are kind-selected. Client.Do now takes a BodyKind (BodyMetadatabody_size_cap_metadata, BodyBlobbody_size_cap_blob — the first production consumer of the blob knob) instead of a bare maxBodyBytes int64 that nothing bounded. The spec-required per-request override (cover 13; npm's max_remote_packument_size) survives as a WithMaxBody functional option, clamped to body_size_cap_blob — the absolute ceiling — not to the kind's own default, so the sanctioned 64MB metadata raise stays possible while no caller can read past the blob cap. A non-positive override or unknown kind keeps the fail-small metadata default. No production callers existed yet (Step 14 wires the composition root), so the signature change lands free.
  • 🔒 The decoded-size bound follows the effective per-request cap. Do passed the global decompression_size_cap to every request, so a metadata request bounded at 16MB raw could still decode a small gzip body up to the 5GB global bound. The bound is now the smaller of the request's effective body cap and decompression_size_cap (the global ceiling): plain metadata decodes at most 16MB, metadata with the 64MB override at most 64MB, blob at most 5GB, and lowering decompression_size_cap below a kind's cap tightens that kind. A 1:1 compressed-to-decoded bound was deliberately rejected — a legitimately gzipped packument outgrows its compressed bytes. No new configuration key.
  • 📝 Spec wording flagged for work item 320: this is the reading cover 14's "default matches the body cap" implies once cover 13's body cap is per-type; the spec should say so explicitly. The spec is untouched in this MR.

An eighth review round closed a proxied-path fail-open and pinned covers a mutation audit found unpinned:

  • 🔒 A failed resolution no longer defers a proxied destination to the proxy. Only a genuine NXDOMAIN reaches the defer branch; a lookup timeout or resolver failure fails closed. Before this, with use_proxy_from_env on and deny_all_except_allowed off, a hostile upstream could redirect to a host whose nameserver stalls past dns_resolution_timeout and have the proxy resolve the name itself — the destination covers were switchable from outside. http_v2 draws the same line: get_address_info raises BlockedUrlError on a lookup timeout, and only a SocketError reaches its defer-to-proxy rescue. The startup WARN and the configuration reference now name the deferral boundary.
  • 🔒 body_size_cap_metadata must not exceed body_size_cap_blob. Only per-request overrides were clamped to the blob ceiling; the metadata kind default applied unclamped, so a larger metadata cap read past the documented hard ceiling. Construction now rejects the ordering.
  • 🧪 Unpinned security properties are pinned: the NewClient → RoundTripper proxy selector wiring, Cookie/Cookie2/Www-Authenticate stripping on cross-origin redirects, URL re-validation past the first redirect hop, the exact max_redirects bound in both directions, req.Body close on cover rejection, and the proxied-path zoned-literal, port-scope, and own-interface covers. The proxied marker-leak test now reaches its named path (its Location previously failed the low-port cover before the marker mattered).

🧪 Testing

Case Test
Redirect to the cloud-metadata address is re-validated and blocked before dial TestClient_RedirectToBlockedIPRejected
A chain longer than max_redirects aborts after exactly max_redirects + 1 requests; a chain of exactly max_redirects succeeds TestClient_ExceedingMaxRedirectsAborts, TestClient_RedirectChainAtLimitSucceeds
Cross-origin redirect drops Authorization, Cookie, Cookie2, and Www-Authenticate; same-origin keeps them TestClient_RedirectSensitiveHeaderHandling
URL covers re-validated past the first redirect hop TestClient_HostileRedirectBlockedPastFirstHop
req.Body closed when a cover rejects TestClient_CoverRejectionClosesRequestBody
Metadata cap above the blob cap rejected at construction TestNewClient_RejectsMetadataCapAboveBlobCap
NewClient hands the proxy selector to the security RoundTripper TestNewClient_WiresProxySelectorIntoRoundTripper
303 See Other downgrades POST to GET and preserves HEAD TestClient_Redirect303DowngradesMethod
Slow headers cut off at response_header_timeout TestClient_SlowHeaderCutOffAtResponseHeaderTimeout
Slow body cut off at the total-request deadline TestClient_TotalRequestTimeout
Oversize body truncated at body_size_cap, never over-delivered TestClient_BodySizeCapTruncatesOversizeBody
gzip bomb rejected at decompression_size_cap TestClient_DecompressionCapRejectsGzipBomb
Empty Content-Encoding: gzip body (HEAD/304) reads as clean EOF TestClient_GzipEmptyBodyReadsCleanEOF
Non-positive WithMaxBody and unknown kinds keep the fail-small metadata default TestClient_Do_NonPositiveOverrideKeepsKindDefault
Each BodyKind selects its configured cap; a blob reads past the metadata cap TestClient_Do_BodyKindSelectsConfiguredCap
WithMaxBody raises the metadata cap (the 64MB packument case) and still bounds the read TestClient_Do_OverrideRaisesMetadataCap
An override above body_size_cap_blob is clamped to it for every kind TestClient_Do_OverrideClampedToBlobCap
Decoded-size bound follows the effective cap: gzip bomb within the raw metadata cap rejected at the metadata cap; override lifts the bound; global knob wins when lower TestClient_Do_DecompressionBoundFollowsEffectiveCap
Proxy startup WARN names only DNS-rebinding protection TestNewClient_ProxyFromEnvStartupWarning
mTLS/CA config assembly across every branch TestTLSConfigFrom_*
End-to-end assembly over a permitted upstream TestClient_Do_SuccessfulRequest
Zero-allocation capped read path BenchmarkCappedReaderRead (0 B/op, 0 allocs/op)
Decompression cap engages across gzip coding spellings (x-gzip, gzip, identity, casing, spacing) TestClient_DecompressionCapEngagesAcrossContentEncodingSpellings
An unsupported or stacked coding streams through undecoded under the raw body cap TestClient_NonGzipContentEncodingStreamsUnderBodyCap
Disallowed scheme on the initial request is blocked before dial TestClient_DisallowedSchemeRejectedBeforeDial
CRLF header value on the initial request is rejected before dial TestClient_HeaderInjectionRejectedBeforeDial
Hostile redirect Location (disallowed scheme, blocked port, unparseable) surfaces redirect_blocked TestClient_HostileRedirectLocationBlocked
Gunzipped response scrubs Content-Encoding/Content-Length and sets Uncompressed TestClient_GzipResponseMetadataScrubbed
A response the client never decompresses keeps its metadata TestClient_UncompressedResponseMetadataPreserved
Non-positive request_total_timeout/response_header_timeout/dns_resolution_timeout/tls_handshake_timeout fail construction TestNewClient_RejectsNonPositiveTimeouts
Upstream resolving to an own-interface address blocked; allowlist opt-in bypass; interface listing cached and failing closed TestDialer_RejectsOwnInterfaceAddress, TestDialer_AllowlistBypassesOwnInterfaceCheck, TestDialer_OwnAddressesCachedWithinTTL, TestDialer_InterfaceAddrsError
Mixed DNS answer fails whole (order-independent); empty resolution blocks TestDialer_MixedResolutionBlocked, TestDialer_MixedResolutionPermittedFirstBlocked, TestDialer_EmptyResolutionBlocked
Port-scoped allowlist entries permit only their port; bare entries every port; malformed port scopes fail startup TestDialer_OutboundAllowlist, TestLoad_VirtualRepositories_PortScopedOutboundAllowlist, TestLoad_VirtualRepositories_InvalidOutboundAllowlist
Zoned IPv6 address blocked before dial (literal host, resolved record, mixed answer fails whole) TestDialer_RejectsZonedIPv6Address
DNS cache stores only validated resolutions, honors "0s", evicts expired entries, and caps at 1024 entries TestDialer_BlockedResolutionNotCached, TestDialer_ZeroCacheTTLDisablesCaching, TestDialer_ExpiredCacheEntriesEvicted, TestDialer_CacheSizeCapped
Transport security fields pinned; env-proxy hook actually wired TestNewTransport_SecurityFields, TestNewTransport_ProxyFromEnvWiresProxy
Body read at exactly the cap is a clean EOF; math.MaxInt64 cap does not overflow; ContentLength clamped to the cap TestCappedReaderBoundary, TestCappedReaderMaxInt64CapReadsClean, TestNewCappedBodyClampsContentLength
Origin comparison pinned per axis (scheme, hostname, effective port) TestSameOrigin
Unenforceable allowlist entries rejected at construction; IPv4-mapped CIDR entries normalized and effective; mixed-case hostname entries match TestNewDialer_RejectsUnusableAllowlistEntry, TestDialer_MappedIPv4PrefixAllowlistNormalized, TestDialer_OutboundAllowlist
NAT64 and 6to4 spellings of denied addresses rejected TestDeniedIP_DenyListRanges
DNS resolution bounded by dns_resolution_timeout TestDialer_DNSResolutionTimeout
Proxied request to a denied destination blocked before the proxy is contacted — the deny-list case and the deny-all case with only the proxy allowlisted TestProxiedRequest_DeniedDestinationBlocked
Proxied request to a permitted destination travels through the proxy; a loopback proxy is dialable without allowlisting it TestProxiedRequest_PermittedDestinationSucceeds
NO_PROXY-exempt host keeps the direct path with the resolved-IP pin TestProxiedRequest_NoProxyExemptHostKeepsDirectPath
Proxied/direct decision remade per redirect hop; the proxy marker never exempts a direct dial TestProxiedRequest_RedirectTargetRevalidatedPerHop
Destination covers on the proxied path: deny-list, deny-all, allowlist (by name and port scope), zoned literal, own interface, NXDOMAIN deferral TestDialer_ValidateProxiedDestination
A failed resolution fails a proxied destination closed; only NXDOMAIN defers to the proxy TestDialer_ValidateProxiedDestination_ResolverFailureFailsClosed
A marked proxy dial skips the destination covers and names the proxy on failure; an unmarked dial still validates TestDialer_ProxiedDialSkipsDestinationCovers, TestDialer_ProxiedDialErrorNamesProxy, TestDialer_UnmarkedDialStillValidates

Verified clean on this branch: go build, go vet, go test -race, goimports, and golangci-lint (v2.12.0).

No e2e scenario catalog entry is added or affected: this step ships library code with no composition-root wiring, so the upstream client is not reachable from a request path until Step 14 builds it. The covers here are exercised by the httptest-backed unit fixtures above.

  • Plan: docs/plans/2026-07-16-s13-virtual-remote-foundation.md — Step 5
  • Spec: docs/specs/S13-virtual-remote-foundation.md — HTTP client security covers

Related to #326 (closed)

Edited by David Fernandez

Merge request reports

Loading
Loading