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 tomax_redirects, re-validating each target through the same covers (cover 7) and strippingAuthorizationbefore a cross-origin hop (cover 8). A redirect the covers reject surfacesBlockedURLError{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 underlabkit/v2/httpclient(tracing + structured logging), maps the packageConfigonto anet/http.Transport(disabling transparent decompression so the client owns the gzip cap), builds the base TLS config from the sharedTLSConfig(mTLS), and wiresuse_proxy_from_envwith a startup WARN naming the one cover a proxy voids (DNS-rebinding protection).Client.Dotakes aBodyKind(BodyMetadata/BodyBlob) selecting the configured response-body cap, with aWithMaxBodyper-request override clamped tobody_size_cap_blob; it applies the total-request deadline (cover 12) and remaps the transport's response-header timeout (cover 11) toErrResponseHeaderTimeout.
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.TLSClientConfigis set so a proxied HTTPS upstream (a CONNECT tunnel, where net/http bypassesDialTLSContext) 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 throughDialTLSContext.🧹 Cross-origin redirects now stripCookie,Cookie2, andWww-AuthenticatealongsideAuthorization, matching net/http's own redirect handling.- The RoundTripper closes
req.Bodywhen a cover rejects the request (contract-honest for future non-NoBodycallers).
A second review round hardened three fail-open seams and closed two coverage gaps:
🔒 The decompression cap engages on every gzip spelling.Content-Encodingis parsed as a coding list rather than matched literally, so the registeredx-gzipalias and list values such asgzip, identityno 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.NewClientrejects a non-positiverequest_total_timeout,dns_resolution_timeout, ortls_handshake_timeout, naming the offending field.Dotherefore applies the total-request deadline unconditionally — the fail-open branch is gone, not just documented.internal/remote/config.goand the configuration reference state the constraint.🧹 A gunzipped response no longer lies about its body. When the gzip path engages,Content-EncodingandContent-Lengthare deleted,ContentLengthreads-1, andUncompressedis 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 toClient.Do(previously deleting thevalidateRequestcall 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 vectorhttp_v2closes viavalidate_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, matchinghttp_v2— a partially-hostile answer is treated as evidence of rebinding or split-horizon attack, not an availability problem.🔒 outbound_allowlistentries can be port-scoped.host:portandip:portentries 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 port0; the configuration reference andconfig.example.yamldocument 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.Containsnever matches an address carrying a zone andnetip.Addrequality compares the zone, so a literal such as2001:db8::5%1slipped past thefec0::/10prefix range and the own-interface check while the zone-blind bit predicates still held.candidateAddressesnow blocks any candidate with a non-empty zone — literal host or resolved record — before validation, failing closed with the network-detail-freeip_deniedreason.
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_timeoutjoins 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:a9fefor the metadata address).🔒 Unenforceableoutbound_allowlistentries 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.🧹 cappedReaderhandles amath.MaxInt64cap without overflowing,ContentLengthis 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 unwiredMaxConnsPerHostknob 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 ofsameOrigin, thedns_resolution_timeoutbound, 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. WithTransport.Proxyset, net/http hands the dialer the proxy's address, so the destination was never checked — underdeny_all_except_allowedwith only the proxy allowlisted, a proxied request could reach169.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_allowlistwith its port scoping, anddeny_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 ashttp_v2relaxes onlydns_rebind_protectionunder a proxy. Hosts exempted byNO_PROXYkeep 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 reportsdial proxyinstead 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 thehttp_v2reference it cites —http_v2enforces it best-effort and relaxes only DNS-rebinding protection (cover 6). This MR implements thehttp_v2behavior; the spec text is untouched here and needs amending.
A seventh review round made the operator's size caps enforced bounds:
🔒 body_size_cap_blobis a hard ceiling, and both caps are kind-selected.Client.Donow takes aBodyKind(BodyMetadata→body_size_cap_metadata,BodyBlob→body_size_cap_blob— the first production consumer of the blob knob) instead of a baremaxBodyBytes int64that nothing bounded. The spec-required per-request override (cover 13; npm'smax_remote_packument_size) survives as aWithMaxBodyfunctional option, clamped tobody_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.Dopassed the globaldecompression_size_capto 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 anddecompression_size_cap(the global ceiling): plain metadata decodes at most 16MB, metadata with the 64MB override at most 64MB, blob at most 5GB, and loweringdecompression_size_capbelow 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, withuse_proxy_from_envon anddeny_all_except_allowedoff, a hostile upstream could redirect to a host whose nameserver stalls pastdns_resolution_timeoutand have the proxy resolve the name itself — the destination covers were switchable from outside.http_v2draws the same line:get_address_inforaisesBlockedUrlErroron a lookup timeout, and only aSocketErrorreaches its defer-to-proxy rescue. The startup WARN and the configuration reference now name the deferral boundary.🔒 body_size_cap_metadatamust not exceedbody_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: theNewClient→ RoundTripper proxy selector wiring,Cookie/Cookie2/Www-Authenticatestripping on cross-origin redirects, URL re-validation past the first redirect hop, the exactmax_redirectsbound in both directions,req.Bodyclose 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.
🔗 References
- 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)