feat(proxy): WebSocket support via ReverseProxy refactor

[andybons]

Summary

Replaces the manual HTTP request loop in handleConnectWithInterception with httputil.ReverseProxy, enabling WebSocket upgrade support through TLS-intercepted connections.

Supersedes #21 (interception test coverage) — includes those tests plus the refactor and WebSocket test.

What changed

The core change is in proxy/proxy.go:handleConnectWithInterception. The manual for { http.ReadRequest → transport.RoundTrip → resp.Write }` loop is replaced with:

Client ←TLS→ http.Server(tlsClientConn) → ReverseProxy → upstream

• Wrapping handler: Network policy + Keep policy checks before forwarding
• Rewrite: Credential injection, extra/remove headers, token substitution, X-Request-Id, host gateway rewrite
• ModifyResponse: LLM gateway policy, response transformers, canonical log lines
• ErrorHandler: Transport errors → 502, credential resolution errors
• WebSocket: ReverseProxy natively handles 101 Switching Protocols — hijacks both sides, bidirectional io.Copy

New supporting code

• singleConnListener — wraps a single net.Conn as a net.Listener for http.Server.Serve()
• evaluateAndReplaceLLMResponse — extracted LLM policy evaluation into a method (was inline)
• Context keys for passing credential results between Rewrite/ModifyResponse/ErrorHandler

Tests (16 new)

Test	Coverage
CredentialInjection	Auth header injected on upstream request
CredentialInjectionCanonicalLog	AuthInjected, Grants, RequestType in log
MultiRequestKeepalive	5 requests over single CONNECT tunnel
NetworkPolicyDenial	Strict policy → 407 error, Denied=true
TransportError502	Unreachable upstream → 502, error in log
CanonicalLogFields	Method, Host, Path, StatusCode, RequestType
ExtraHeaders	Custom header injected via AddExtraHeader
RemoveHeaders	Client header stripped via RemoveRequestHeader
RequestBodyForwarded	POST body arrives intact
LargeResponseBody	1MB response streams correctly
ResponseStatusCodes	7 status codes forwarded correctly
ResponseHeaders	Backend response headers preserved
XRequestIdInjected	req_ prefixed ID injected
ProxyAuthorizationStripped	Proxy-Authorization not forwarded
HTTPMethods	GET, POST, PUT, DELETE, PATCH
WebSocketUpgrade	101 upgrade, bidirectional byte echo, credential injection on upgrade request

Closes

The original problem: gatekeeper's MITM proxy couldn't handle WebSocket frames after upgrade (malformed HTTP request "\x82\xe1..."). Now it works because ReverseProxy switches to raw bidirectional copy after 101.

[claude]

📦 Previous Review (superseded by newer review)

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

---

Code Review

• Gather context
• Read proxy.go changes
• Read intercept_test.go
• Analyze for security issues, race conditions, bugs, error handling gaps
• Post review findings

---

Good refactor overall — the ReverseProxy approach cleanly solves the WebSocket problem and the test coverage is substantial. Two critical bugs need fixing before merge.

---

Critical

1. Goroutine leak per connection (proxy.go:2161–2166)

srv.Serve(newSingleConnListener(tlsClientConn)) blocks forever after the one connection finishes. Here's why: singleConnListener.Accept() returns the connection once from a buffered channel of size 1, then blocks on the now-empty, non-closed channel. http.Server.Serve never auto-closes the listener when active connections drop to zero, so the goroutine running handleConnectWithInterception is permanently stuck. Every intercepted HTTPS connection leaks one goroutine.

Fix — use ConnState to close the listener after both the normal and WebSocket-hijacked paths:

l := newSingleConnListener(tlsClientConn)
srv := &http.Server{
    Handler:     handler,
    IdleTimeout: 120 * time.Second,
    ErrorLog:    log.New(io.Discard, "", 0),
    ConnState: func(conn net.Conn, state http.ConnState) {
        if state == http.StateClosed || state == http.StateHijacked {
            l.Close()
        }
    },
}
_ = srv.Serve(l)

StateClosed covers regular request/response connections; StateHijacked covers WebSocket upgrades (once ReverseProxy hijacks for the bidirectional copy, http.Server never calls StateClosed).

Fix this →

---

2. Credential resolution error silently forwards the request (proxy.go:1945–1948)

When getCredentialsForRequest returns an error, Rewrite stores it in context and returns — but the request already has a valid URL.Scheme and URL.Host set, so ReverseProxy still sends it upstream. ErrorHandler is only called when transport.RoundTrip fails. If the upstream is reachable and returns any response, ModifyResponse runs, the credential error is never surfaced, and the unauthenticated request succeeds silently.

// Rewrite, ~line 1945
creds, credErr := p.getCredentialsForRequest(r, pr.Out, host)
if credErr != nil {
    *pr.Out = *pr.Out.WithContext(context.WithValue(pr.Out.Context(), interceptCredErrKey{}, credErr))
    return  // ← request is still forwarded to upstream with a valid URL
}

Fix — cancel the context so transport.RoundTrip fails and ErrorHandler is guaranteed to run:

if credErr != nil {
    ctx, cancel := context.WithCancel(pr.Out.Context())
    cancel()
    *pr.Out = *pr.Out.WithContext(context.WithValue(ctx, interceptCredErrKey{}, credErr))
    return
}

Fix this →

---

Moderate

3. Injected credential values appear in canonical log (proxy.go:2020)

req.Header.Clone() in ModifyResponse is called after Rewrite has already called injectCredentials, so Authorization: Bearer <token> appears in RequestHeaders in the log. The original loop captured headers before injection (originalReqHeaders := req.Header.Clone() at the top of the loop, before injectCredentials). CLAUDE.md says "Never log credential values — log host/grant names only."

Fix: in Rewrite, snapshot pre-injection headers and store them in context (alongside credResult), then use that snapshot in ModifyResponse.

Fix this →

4. Request body no longer logged (proxy.go:2011–2028)

The original loop called captureBody(req.Body, ...) and set RequestBody: reqBody in the log data. The new ModifyResponse only captures the response body. RequestBody is always nil in logs now. Whether this matters depends on downstream consumers of log data, but it's a regression worth noting.

---

Minor

5. Planning artifacts in the tree

docs/plans/2026-04-22-websocket-reverseproxy.md (627 lines) is an implementation plan, not product documentation. docs/2026-04-22-websocket-reverseproxy-design.md looks like a design note generated during development. Neither belongs in the shipped repo.

---

Looks Good

• singleConnListener.once correctly guards against double-close.
• pr.Out.Host = pr.In.Host preserves the original Host header as required when using Rewrite (not Director).
• Proxy-Authorization restore-before-strip pattern for token-exchange is carried over correctly.
• ModifyResponse is called for 101 responses by the stdlib (verified), so credential injection IS logged for WebSocket upgrades — the test assertion on logged.AuthInjected is valid.
• reqStartFromContext propagates through the context chain correctly (values from the wrapping handler's context are visible in pr.Out.Context() since ReverseProxy derives pr.Out from the incoming request context).
• WebSocket test exercises raw bidirectional byte flow through the tunnel — sufficient to verify the upgrade path works end-to-end.

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 5m 50s —— View job

---

Code Review

• Gather context
• Read proxy.go changes
• Read intercept_test.go
• Analyze for security issues, race conditions, bugs, error handling gaps
• Post review findings

---

The ReverseProxy refactor is a clean architecture improvement and the WebSocket support works correctly. Four issues need attention before merging.

Bugs

1. Goroutine / FD leak (line 2166) — After Accept() hands out the single connection, ch is empty but open. When the connection closes, Serve calls Accept() again and blocks forever. handleConnectWithInterception never returns, its defer clientConn.Close() never fires. Every intercepted CONNECT tunnel leaks a goroutine and a file descriptor. Fix: in Accept(), after receiving the conn, call l.once.Do(func() { close(l.ch) }) before returning it so the next Accept() call returns net.ErrClosed and Serve exits cleanly.

2. Credential error silently dropped when upstream is reachable (line 1948) — When getCredentialsForRequest returns an error in Rewrite, the code stores the error in context and returns — but pr.Out.URL is already set, so ReverseProxy still forwards the request. ErrorHandler is only called on transport errors; if the upstream responds successfully, ModifyResponse is called instead and the cred error is never surfaced. The unauthenticated request goes through and is logged as success. The fix is to call getCredentialsForRequest in the wrapping handler (before reverseProxy.ServeHTTP) where an early return is straightforward, and pass resolved credentials to Rewrite via context.

Security regression

3. Token-substituted URLs logged (line 2014) — The old loop captured logURL = req.URL.String() before applyTokenSubstitution so canonical logs showed the placeholder (e.g., /bot{TOKEN}/sendMessage). In ModifyResponse, req is resp.Request which already has the substituted path, so real tokens embedded in URL paths (Telegram Bot API pattern) now appear in logs. Fix: capture the pre-substitution URL in Rewrite and store it in context (like credResult is stored), then use it for logging in ModifyResponse and ErrorHandler.

Minor

4. Zero Duration in policy denial logs (line 2156) — interceptReqStartKey{} is stored in context after the policy checks, so any request denied by network or Keep policy hits reqStartFromContext's time.Now() fallback and logs zero duration. Old code set reqStart := time.Now() at the top of each iteration before the checks. Fix: move the time.Now() call to the top of the handler, before any policy check.

---

The test coverage in intercept_test.go is solid — the WebSocket test in particular validates the core new behavior end-to-end. The goroutine leak and silent-credErr issues won't show up in these tests (short-lived processes / unreachable upstreams).

[claude]

📦 Previous Review (superseded by newer review)

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

---

Code Review

• Gather context and read changed files
• Analyze for security issues, race conditions, bugs, error handling gaps
• Post review findings

---

The prior feedback has largely been addressed well. The goroutine leak is fixed (closeCh+ConnState), credential errors are caught in the handler before ServeHTTP, token-substituted URLs are captured pre-substitution, and policy denial durations are correct. Three issues remain before this is clean.

---

Security

Injected credential values appear in RequestHeaders in canonical logs (proxy.go:2033)

req.Header.Clone() is called in ModifyResponse after Rewrite has already called injectCredentials, which mutates pr.Out.Header in-place. resp.Request is that same mutated request, so Authorization: Bearer <token> (and any other injected headers) end up in the logged RequestHeaders. CLAUDE.md is explicit: "Never log credential values — log host/grant names only."

The old loop snapshotted headers before injection (originalReqHeaders := req.Header.Clone() before injectCredentials). The fix is to snapshot pre-injection headers in Rewrite and pass them through context:

// In Rewrite, before calling injectCredentials:
preInjectionHeaders := pr.Out.Header.Clone()
ctx = context.WithValue(pr.Out.Context(), interceptPreInjectionHeadersKey{}, preInjectionHeaders)
*pr.Out = *pr.Out.WithContext(ctx)

// In ModifyResponse, replace line 2033:
preHeaders, _ := req.Context().Value(interceptPreInjectionHeadersKey{}).(http.Header)
// use preHeaders instead of req.Header.Clone()

Fix this →

---

Bugs

LLM policy denials no longer set Denied/DenyReason in the log (proxy.go:1997, 2024–2041)

evaluateAndReplaceLLMResponse modifies resp in place but returns nothing about whether a denial occurred. ModifyResponse then logs with Denied unset (defaults to false). The old code tracked llmDenied/llmDenyReason and passed them to logRequest. Any downstream consumer relying on RequestLogData.Denied (including the OTel RecordPolicyDenial call in gatekeeper.go) will miss all LLM gateway denials.

Options:

1. Have evaluateAndReplaceLLMResponse return a (denied bool, reason string) — minimal change.
2. Detect denial in ModifyResponse from the response (resp.Header.Get("X-Moat-Blocked") == "llm-policy"), though this is more fragile.

Fix this →

---

RequestBody is never logged for intercepted connections (proxy.go:2024–2041)

ModifyResponse sets ResponseBody but has no RequestBody field. The old loop called captureBody(req.Body, ...) and logged it. For any consumer using RequestLogData.RequestBody, this is a silent regression.

The body needs to be captured in the wrapping handler (after credential resolution) before reverseProxy.ServeHTTP. Since ReverseProxy clones the request, the body should be read and re-wrapped before passing:

// In handler, after credential resolution:
var reqBody []byte
reqBody, req.Body = captureBody(req.Body, req.Header.Get("Content-Type"))
ctx = context.WithValue(ctx, interceptReqBodyKey{}, reqBody)

Then read interceptReqBodyKey{} in ModifyResponse.

Fix this →

---

Minor

Query params stripped from URLs in denial/error logs (proxy.go:2083, 2112, 2135, 2172)

All four early-return log entries use "https://" + r.Host + req.URL.Path. req.URL.Path drops query strings. A request to /api?key=value would log as /api. Use req.URL.RequestURI() to preserve the query: "https://" + r.Host + req.URL.RequestURI().

---

Looks Good

• singleConnListener goroutine/FD leak is fixed — closeCh unblocks Accept when ConnState fires StateClosed or StateHijacked, letting srv.Serve return cleanly.
• Credential errors caught in the wrapping handler before ServeHTTP — no longer silently forwarded.
• interceptLogURLKey correctly captures pre-substitution URL; token-in-URL paths (Telegram Bot API) won't leak to logs.
• Policy denial durations are correct — reqStart is set at the top of the handler before any checks.
• pr.Out.Host = pr.In.Host preserves the Host header correctly when using Rewrite.
• WebSocket test exercises raw bidirectional byte flow end-to-end, validating the core new behavior.

[andybons]

All three issues + the minor fixed in c990bc6:

1. Credential values in logs — pre-injection headers snapshotted in Rewrite via interceptPreInjHeadersKey, used in ModifyResponse instead of req.Header.Clone().
2. LLM policy Denied/DenyReason — evaluateAndReplaceLLMResponse now returns (denied bool, reason string); ModifyResponse passes them to logRequest.
3. RequestBody missing — captureBody called in the wrapping handler, passed via interceptReqBodyKey context.
4. Query params in log URLs — all four early-return paths now use req.URL.RequestURI().

[claude]

📦 Previous Review (superseded by newer review)

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

---

Code Review

• Gather context and read changed files
• Analyze security, race conditions, bugs, and error handling
• Post review findings

---

All seven issues flagged in the prior rounds have been correctly addressed. The fixes are clean and the approach is sound. Two minor items remain.

---

Security regression test missing

TestIntercept_CredentialInjectionCanonicalLog verifies AuthInjected=true and Grants, but doesn't assert that the injected token value is absent from the logged headers. The fix (preInjectionHeaders snapshot in Rewrite, stored via interceptPreInjHeadersKey) is correctly implemented, but there is no test to guard against future regression. Consider adding:

// Verify credential value is NOT present in logged request headers.
if v := logged.RequestHeaders.Get("Authorization"); v != "" {
    t.Errorf("logged RequestHeaders contains injected Authorization value %q; credential values must not appear in logs", v)
}

Fix this →

---

Minor

RequestSize/ResponseSize dropped from credential error log (proxy.go:2194–2204)

The network policy and Keep policy denial paths log RequestSize: req.ContentLength, ResponseSize: -1. The credential error path omits both fields, leaving them at their zero values. Not a security issue, but inconsistent with the other early-return paths in the same handler.

---

Looks Good

• singleConnListener goroutine/FD leak: fixed correctly — closeCh unblocks Accept() when ConnState fires StateClosed or StateHijacked, letting srv.Serve return cleanly. The hijacked atomic.Bool guard prevents double-close of clientConn/tlsClientConn on the WebSocket path.
• Credential error caught in the wrapping handler before ServeHTTP — no longer silently forwarded.
• Pre-injection headers snapshot in Rewrite via interceptPreInjHeadersKey — credential values won't appear in RequestHeaders in logs.
• evaluateAndReplaceLLMResponse now returns (denied bool, reason string) — ModifyResponse correctly sets Denied/DenyReason in the canonical log.
• interceptReqBodyKey correctly captures the request body before ReverseProxy consumes it.
• interceptLogURLKey captures the pre-substitution URL in Rewrite before applyTokenSubstitution runs.
• reqStart moved to top of handler — policy denial durations are correct.
• All four early-return paths now use req.URL.RequestURI() — query parameters preserved in denial logs.
• captureBody on a 101 WebSocket response with empty Content-Type returns (nil, body) unchanged — handleUpgradeResponse can consume resp.Body for the bidirectional copy without interference.
• WebSocket test exercises raw bidirectional byte flow through the full interception path end-to-end — solid coverage of the core new behavior.

[claude]

📦 Previous Review (superseded by newer review)

Claude finished @andybons's task in 6m 30s —— View job

---

Code Review

• Gather context and read changed files
• Analyze for security issues, race conditions, bugs, and error handling gaps
• Post review findings

---

All critical and moderate issues from previous rounds are correctly fixed. Two minor regressions remain.

---

Minor

1. innerReqID is not propagated to the outgoing request (proxy.go:2098–2219)

In the old loop, req.Header.Set("X-Request-Id", innerReqID) ensured the ID logged for denied requests matched the ID the backend would have received if the request had gone through. In the wrapping handler, innerReqID is generated but never written back to req.Header, so Rewrite generates a fresh newRequestID() for the outgoing X-Request-Id. When a client doesn't provide X-Request-Id, the ID in policy/credential denial logs is different from the ID set on the outgoing request (logged by ModifyResponse/ErrorHandler).

Fix — one line in the handler before reverseProxy.ServeHTTP:

req.Header.Set("X-Request-Id", innerReqID)   // propagate so Rewrite preserves it
reverseProxy.ServeHTTP(w, req.WithContext(ctx))

Fix this →

---

2. ErrorHandler omits RequestSize and ResponseSize (proxy.go:2070–2091)

The network policy and Keep policy denial paths set RequestSize: req.ContentLength, ResponseSize: -1. The credential error path (in the wrapping handler) now does too. But ErrorHandler (for transport errors) omits both fields, leaving them at zero. Not a correctness issue, just inconsistent.

---

Looks Good

• singleConnListener goroutine/FD leak: fixed correctly — closeCh unblocks Accept() on StateClosed or StateHijacked, srv.Serve exits cleanly.
• hijacked atomic.Bool guard prevents double-close of clientConn/tlsClientConn on the WebSocket path.
• Credential errors caught in wrapping handler before ServeHTTP — no longer silently forwarded.
• interceptPreInjHeadersKey snapshots headers before injectCredentials — credential values don't appear in RequestHeaders in logs. Regression test added in TestIntercept_CredentialInjectionCanonicalLog.
• interceptLogURLKey captures pre-substitution URL before applyTokenSubstitution — real tokens don't appear in URL logs.
• evaluateAndReplaceLLMResponse now returns (denied bool, reason string) — ModifyResponse correctly sets Denied/DenyReason.
• interceptReqBodyKey captures request body before ReverseProxy consumes it.
• reqStart at top of handler — policy denial durations are correct.
• All four early-return paths use req.URL.RequestURI() — query params preserved.
• captureBody on a 101 response with no text Content-Type returns (nil, body) unchanged — handleUpgradeResponse can consume resp.Body for the bidirectional copy without interference.
• WebSocket test exercises raw bidirectional byte flow through the full MITM path.

[claude]

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

---

Code Review

• Gather context
• Read proxy.go changes
• Read intercept_test.go
• Analyze for security issues, race conditions, bugs, error handling gaps
• Post review findings

---

All issues from the previous review rounds are correctly addressed. Here's a final pass over the full diff.

Looks Good

Goroutine/FD leak — singleConnListener correctly uses closeCh to unblock Accept() when ConnState fires StateClosed or StateHijacked. srv.Serve returns cleanly, and the deferred clientConn.Close()/tlsClientConn.Close() calls are guarded by hijacked.Store(true) so ReverseProxy retains sole ownership on the WebSocket path.

Credential error handling — getCredentialsForRequest is called in the wrapping handler before reverseProxy.ServeHTTP. Errors return a 502 immediately; the URL-already-set forwarding problem from the Rewrite-only approach is gone.

Credential values in logs — interceptPreInjHeadersKey snapshots pr.Out.Header.Clone() in Rewrite before injectCredentials, and ModifyResponse reads that snapshot. TestIntercept_CredentialInjectionCanonicalLog asserts logged.RequestHeaders.Get("Authorization") == "" to guard this against regression. ✓

Token-substituted URLs — interceptLogURLKey captures the URL in Rewrite before applyTokenSubstitution. ModifyResponse and ErrorHandler both read it from context, falling back to req.URL.String() only if the key is absent. ✓

LLM policy Denied/DenyReason — evaluateAndReplaceLLMResponse now returns (denied bool, reason string) and ModifyResponse passes those into logRequest. ✓

RequestBody in logs — captureBody is called in the handler before reverseProxy.ServeHTTP, stored via interceptReqBodyKey, and read in ModifyResponse. The body reader is replaced so ReverseProxy can still consume it. ✓

Policy denial duration — reqStart := time.Now() is at the top of the wrapping handler, before policy checks. All denial paths call time.Since(reqStart) directly. ✓

Query params in denial/error logs — All four early-return paths use req.URL.RequestURI(). ✓

innerReqID propagation — req.Header.Set("X-Request-Id", innerReqID) before reverseProxy.ServeHTTP means Rewrite's pr.Out.Header.Get("X-Request-Id") is non-empty and the new-ID branch doesn't fire. Denial-path logs and success-path logs now share the same ID. ✓

ErrorHandler log fields — RequestSize: req.ContentLength and ResponseSize: -1 are now present, consistent with the other early-return paths. ✓

WebSocket test — Exercises raw bidirectional byte flow through the full MITM path (CONNECT → TLS handshake → upgrade request → 101 → raw echo). logged.AuthInjected is asserted, verifying that ModifyResponse is called for 101 responses and credential injection is tracked. ✓

101 body safety — captureBody(resp.Body, resp.Header.Get("Content-Type")) for a 101 response passes "" as the content type. isTextContentType("") returns false, so captureBody returns (nil, body) unchanged. handleUpgradeResponse can consume resp.Body for the bidirectional copy without interference. ✓

Planning artifacts — docs/plans/ removed (only docs/token-exchange-endpoint.md remains). ✓

---

Nothing blocking. This is ready to merge.