feat(proxy): HTTP/2 and gRPC support through TLS interception

[andybons]

Summary

• Advertises h2 first in ALPN NextProtos on the client-facing TLS connection inside handleConnectWithInterception, enabling gRPC and other HTTP/2 clients to negotiate h2 through the CONNECT proxy
• When h2 is negotiated, http2.ConfigureServer handles correct h2 framing on the inner http.Server and the upstream transport switches to http2.Transport — credentials are injected identically
• HTTP/1.1 clients are unaffected: when h2 is not negotiated the original http.Transport is used unchanged
• Extracts shared upstreamTLS config to eliminate the duplicate &tls.Config{...} across both transport branches
• Uses http2.NextProtoTLS constant instead of bare "h2" string literals so the advertise/detect/route triple stays in sync

New tests

• proxy/http2_test.go — TestHTTP2_GRPCCredentialInjection: black-box acceptance test that spins up a real HTTP/2 gRPC backend, routes through the intercepting proxy with Modal-style credentials configured for api.modal.com, and asserts credential injection + correct gRPC response (written as a failing test first, then made green by the implementation)
• proxy/coverage_test.go — black-box coverage tests for previously uncovered paths: tunnel mode, network policy, LLM gateway policy (deny/allow/size-limit/no-engine), response transformers, token substitution, host pattern matching, grant host registration
• gatekeeper_test.go — TestMultiHandler_WithAttrs / TestMultiHandler_WithGroup

Test plan

• go test -race ./... passes
• TestHTTP2_GRPCCredentialInjection passes — h2 ALPN negotiated, credentials injected, gRPC trailer received
• Existing HTTP/1.1 intercept tests unchanged (WebSocket, credential injection, LLM policy, etc.)

🤖 Generated with Claude Code

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 3m 28s —— View job

---

Code Review

• Gather context
• Read changed files
• Review proxy/proxy.go changes
• Review proxy/http2_test.go
• Review proxy/coverage_test.go
• Review gatekeeper_test.go
• Post inline comments

---

Summary

The core approach is sound: advertise h2 first in ALPN, select http2.Transport upstream when h2 is negotiated, and call http2.ConfigureServer on the inner http.Server. Credential injection is protocol-agnostic (header manipulation) so it carries over correctly. The coverage tests are a good addition.

Three issues worth addressing before merge:

---

Issues

[High] http2.Transport missing connection-safety timeouts (line 2189)

The h2 upstream transport has no dial timeout, no TLS handshake timeout, and no idle-ping settings — the h1 branch has TLSHandshakeTimeout: 10s, ResponseHeaderTimeout: 5m, and a 30 s dialer. A stalled or slow upstream on an h2 connection will leak goroutines indefinitely.

[Medium] Unconditional h2 upstream may fail for h1-only backends (line 2187)

When the client negotiates h2, http2.Transport is used upstream unconditionally. http2.Transport never falls back to h1.1, so if the upstream only speaks HTTP/1.1 the connection fails — where the proxy would have worked transparently before. For strict gRPC this is intentional, but for a general-purpose TLS proxy it's a regression. At minimum document the known limitation in a comment.

[Medium] http2.ConfigureServer failure fallthrough with h2 already negotiated (lines 2563–2567)

If ConfigureServer fails but the client already negotiated h2 via ALPN, srv.Serve(ln) will speak h1 to an h2 client — resulting in a framing error. The fix is to bail out early (return without serving) in this unlikely case.

---

Minor

[Low] bufio.NewReader fragility in test (line 134–135)

In newGRPCProxySetup, the CONNECT response is read from a bufio.Reader but the underlying raw conn is then passed to tls.Client. Bytes the bufio.Reader pre-fetched would be silently dropped. It doesn't bite here (the proxy blocks waiting for the TLS ClientHello so there's nothing to pre-buffer), but the idiom is fragile. A small bufferedConn shim or a comment is worth adding.

---

What looks good

• http2.NextProtoTLS constant used consistently — no bare "h2" strings that could diverge.
• upstreamTLS extracted to a single *tls.Config eliminates the prior duplication.
• ConfigureServer(srv, nil) correctly follows the h2-first ALPN ordering (ConfigureServer appends, not prepends).
• TestHTTP2_GRPCCredentialInjection is a real black-box acceptance test (actual TLS + h2 transport + gRPC framing), not a mock. Good quality.
• coverage_test.go covers previously-untested paths (tunnel mode, LLM policy deny/allow/size-limit, response transformers, token substitution, host pattern matching).

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 2m 58s —— View job

---

Code Review

• Gather context
• Read changed files
• Review security and correctness issues
• Post inline comments and summary

---

Summary

The previous review's three issues (missing h2 transport timeouts, undocumented h1-only upstream failure mode, ConfigureServer fallthrough on h2-negotiated connections) and the bufio.Reader fragility in the test were all cleanly addressed. The implementation is sound and the acceptance test is high-quality.

Two remaining issues:

---

Issues

[Medium] DialTLSContext ignores the context (line 2201–2205)

tls.DialWithDialer has no context parameter. When a gRPC client disconnects mid-call (or a deadline fires), the upstream TLS dial continues running until it succeeds or dialer.Timeout expires (up to 30 s), leaking the goroutine for that window. The h1 path correctly uses dialer.DialContext. Fix: replace with (&tls.Dialer{NetDialer: dialer, Config: cfg}).DialContext(ctx, network, addr).

[Low] golang.org/x/net is listed as // indirect in go.mod (line 134)

proxy/proxy.go now imports golang.org/x/net/http2 directly, so the dependency should be promoted to a direct require entry. Running go mod tidy fixes this automatically.

---

What looks good

• All three medium/high issues from the prior review are addressed: h2 transport now has ReadIdleTimeout/PingTimeout/dialer timeout; the h1-only upstream limitation is documented; ConfigureServer failure with h2 already negotiated now returns early.
• bufferedConn shim correctly handles the bufio.Reader pre-read bytes before the TLS handshake — the fragile idiom is gone.
• http2.NextProtoTLS constant used consistently throughout; no bare "h2" strings.
• upstreamTLS extracted cleanly; no duplicated &tls.Config{} literals.
• TestHTTP2_GRPCCredentialInjection is a genuine black-box acceptance test — real TLS, real h2 transport, real gRPC framing, correct trailer verification after body drain.
• Coverage tests in coverage_test.go are well-structured and cover real gaps (tunnel mode, all four LLM policy branches, response transformers, token substitution, host pattern matching).

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 4m 17s —— View job

---

Code Review

• Gather context
• Read changed files
• Review proxy/proxy.go changes
• Review proxy/http2_test.go
• Review proxy/coverage_test.go
• Review gatekeeper_test.go
• Post summary

---

Summary

Three rounds of review have produced a clean implementation. All previously flagged issues are resolved:

Issue	Status
h2 transport missing timeouts	✅ Fixed — ReadIdleTimeout: 30s, PingTimeout: 15s, tls.Dialer with 30s timeout/keepalive
tls.DialWithDialer ignores context	✅ Fixed — tls.Dialer.DialContext used throughout
ConfigureServer failure fallthrough on h2-negotiated client	✅ Fixed — early return when client already negotiated h2
h1-only upstream failure mode undocumented	✅ Fixed — clear comment explaining the known limitation
bufio.Reader pre-fetch bytes lost before TLS handshake	✅ Fixed — bufferedConn shim drains buffer first
golang.org/x/net listed as indirect	✅ Fixed — promoted to direct in go.mod after go mod tidy

---

What looks good

Core implementation (proxy/proxy.go)

• ALPN NextProtos: []string{http2.NextProtoTLS, "http/1.1"} correctly advertises h2 first. The comment explaining why ordering matters (ConfigureServer appends, doesn't reorder) is exactly the right thing to document here.
• upstreamTLS extracted to a single config object — clean, no duplication.
• Transport selection via tlsClientConn.ConnectionState().NegotiatedProtocol is the right check: it reads the already-completed handshake, not a future prediction.
• DialTLSContext correctly handles the cfg == nil case: http2.Transport passes a merged config derived from TLSClientConfig, but can pass nil in some code paths.
• http2.ConfigureServer(srv, nil) on the inner *http.Server correctly hooks TLSNextProto["h2"], which is consulted when srv.Serve(ln) accepts the *tls.Conn from singleConnListener. The h2 dispatch happens correctly without re-doing the TLS handshake.

Tests (proxy/http2_test.go)

• TestHTTP2_GRPCCredentialInjection is a genuine black-box acceptance test: real TLS, real h2 transport, gRPC length-prefix framing, trailer check after body drain. This verifies the full pipeline end-to-end.
• bufferedConn shim is the correct fix — pre-fetched bytes from bufio.NewReader are drained before the TLS handshake reads the same bytes off the wire.
• Trailer verification (resp.Trailer.Get("grpc-status") after io.Copy(io.Discard, resp.Body)) is correct: HTTP/2 trailers are only available after the body is fully consumed.

Coverage tests (proxy/coverage_test.go)

• All four LLM policy branches (deny, allow, size-limit, no-engine passthrough) are covered, plus the deny-logged variant.
• Tunnel mode tests (TestTunnel_ForwardsPlainHTTPS, TestTunnel_NetworkPolicyBlocked, TestTunnel_InvalidHostFormat, TestTunnel_PathRulesWarning) exercise real gaps in the prior suite.
• newAnthropicInterceptSetup helper is clean: HostGateway + AllowedHostPorts + explicit upstreamCAs is the right setup for routing the fake backend through the interceptor.

gatekeeper_test.go

• TestMultiHandler_WithAttrs and TestMultiHandler_WithGroup are small, precise, and cover the previously untested fan-out branches.

---

No remaining issues

The implementation is correct and ready to merge.

[claude]

Claude finished @andybons's task in 3m 1s —— View job

---

Code Review

• Gather context
• Read changed files
• Review proxy/proxy.go changes
• Review proxy/http2_test.go
• Review coverage and gatekeeper tests
• Post inline comments and summary

---

Summary

Three rounds of review have produced a clean, correct implementation. All previously flagged issues are resolved. No new issues found in this sync.

Prior issue	Status
h2 transport missing timeouts	✅ ReadIdleTimeout: 30s, PingTimeout: 15s, dial via tls.Dialer with 30 s timeout
DialTLSContext ignored context (tls.DialWithDialer)	✅ Replaced with (&tls.Dialer{...}).DialContext(ctx, ...)
ConfigureServer failure fallthrough on h2-negotiated client	✅ Early return when NegotiatedProtocol == http2.NextProtoTLS
h1-only upstream failure mode undocumented	✅ Clear comment noting the known limitation
bufio.Reader pre-fetched bytes lost before TLS handshake	✅ bufferedConn shim drains buffer before tls.Client takes over
golang.org/x/net listed as // indirect	✅ Promoted to direct in go.mod after go mod tidy

---

What looks good

Core implementation (proxy/proxy.go)

• NextProtos: []string{http2.NextProtoTLS, "http/1.1"} correctly advertises h2 first. The comment explaining why ordering matters (ConfigureServer only appends, doesn't reorder) is exactly the right thing to document.
• upstreamTLS extracted to a single *tls.Config — clean, no duplication.
• Transport selection on tlsClientConn.ConnectionState().NegotiatedProtocol is the correct check: reads the already-completed handshake.
• DialTLSContext cfg == nil guard is correct defensive coding — in practice cfg is never nil given TLSClientConfig: upstreamTLS, but the guard is harmless and protects against code path surprises in the http2.Transport internals.
• http2.ConfigureServer(srv, nil) hooks TLSNextProto["h2"], which srv.Serve(ln) consults when it sees the pre-completed handshake on the singleConnListener's *tls.Conn. The dispatch is correct.
• maxLLMResponseSize = 10 << 20 = 10MB — the large-response coverage test uses 11 MB, so the limit check fires correctly.

Tests (proxy/http2_test.go)

• TestHTTP2_GRPCCredentialInjection is a genuine black-box acceptance test: real TLS, real h2 transport, gRPC length-prefix framing, trailer check after body drain. The resp.Trailer.Get("grpc-status") check after io.Copy(io.Discard, resp.Body) is the correct idiom for HTTP/2 trailers.
• bufferedConn.Read correctly drains from bufio.Reader before falling through to the raw conn.

Coverage tests

• All four LLM policy branches (deny, allow, size-limit, no-engine passthrough) plus the deny-logged variant now have integration coverage exercising real policy evaluation through the proxy.
• Tunnel mode tests (ForwardsPlainHTTPS, NetworkPolicyBlocked, InvalidHostFormat, PathRulesWarning) cover previously-untested paths.
• newAnthropicInterceptSetup helper is clean: HostGateway + AllowedHostPorts + explicit upstreamCAs is the right wiring for routing the fake backend through the interceptor.
• TestNewTokenSubstitution, TestParseAndMatchHostPattern, TestRegisterGrantHosts — targeted, focused coverage additions.

gatekeeper_test.go

• TestMultiHandler_WithAttrs / TestMultiHandler_WithGroup are minimal and precise.

---

No remaining issues. Ready to merge.