Skip to content

fix: unify Anthropic TLS fingerprint to Bun BoringSSL#2197

Open
lichengzhe wants to merge 7 commits intorouter-for-me:mainfrom
lichengzhe:fix/bun-tls-fingerprint
Open

fix: unify Anthropic TLS fingerprint to Bun BoringSSL#2197
lichengzhe wants to merge 7 commits intorouter-for-me:mainfrom
lichengzhe:fix/bun-tls-fingerprint

Conversation

@lichengzhe
Copy link

@lichengzhe lichengzhe commented Mar 18, 2026

Summary

  • Unify all Anthropic connections (API requests + OAuth token refresh) to use Bun BoringSSL TLS fingerprint (JA3: 50027c67d7d68e24c00d233bca146d88), matching real Claude Code CLI
  • Replace Chrome_Auto fingerprint with custom BunBoringSSLSpec() captured from Bun v1.3.8
  • Switch from HTTP/2 (manual h2.ClientConn pool) to HTTP/1.1 (http.Transport + DialTLSContext) — Bun's ALPN only offers http/1.1
  • Route all Claude executor API calls through newClaudeHTTPClient() using the utls transport, eliminating the independent bare Go TLS path
  • Extract ResolveProxyURL() as single source of truth for proxy priority (auth.ProxyURL > cfg.ProxyURL > env), used by both request and refresh paths

Context

CLIProxyAPI currently has two independent HTTP client paths for Anthropic connections: API requests use standard Go http.Transport (native Go TLS fingerprint), while token refresh uses utls with Chrome fingerprint. This creates a TLS fingerprint inconsistency — the User-Agent header claims Claude Code CLI (Bun/Node.js runtime), but the TLS ClientHello doesn't match what Bun actually produces.

This PR unifies both paths to use a single Bun BoringSSL fingerprint, ensuring full consistency between the declared client identity and the observed TLS behavior. This improves transparency and reduces the risk of being flagged by upstream bot detection systems that cross-reference User-Agent with TLS fingerprints.

Files Changed

File Change
internal/auth/claude/bun_tls_spec.go New — Bun BoringSSL ClientHello spec
internal/auth/claude/utls_transport.go Chrome→Bun, h2→h1.1, simplified architecture
internal/auth/claude/anthropic_auth.go Optional proxyURL param on NewClaudeAuth
internal/runtime/executor/proxy_helpers.go ResolveProxyURL() + newClaudeHTTPClient()
internal/runtime/executor/claude_executor.go Use newClaudeHTTPClient for all API calls
sdk/cliproxy/auth/conductor.go Elevate refresh failure logs DEBUG→WARN

Test plan

  • CI build passes (Go compilation + existing tests)
  • Deploy to test instance, verify TLS fingerprint with tls.peet.ws matches JA3 50027c67d7d68e24c00d233bca146d88
  • Verify API requests succeed through proxy
  • Verify OAuth token refresh works through per-account proxy

Supersedes #1947.

🤖 Generated with Claude Code

Both Claude accounts (cc/ai) were disabled because CLIProxyAPI had two
independent HTTP client paths with mismatched TLS fingerprints:
- API requests used standard Go http.Transport (bare Go TLS fingerprint)
- Token refresh used utls with Chrome fingerprint

Anthropic detected User-Agent claiming Claude Code CLI (Bun/Node.js) but
TLS fingerprint was Go, flagging it as a forged tool and disabling both
accounts.

Changes:
- Add BunBoringSSLSpec() matching real Bun v1.3.8 BoringSSL ClientHello
  (JA3: 50027c67d7d68e24c00d233bca146d88)
- Replace Chrome_Auto with HelloCustom + BunBoringSSLSpec in utls transport
- Switch from HTTP/2 (manual h2 connection pool) to HTTP/1.1 (standard
  http.Transport with DialTLSContext) — Bun's ALPN only offers http/1.1
- Route all Claude executor API calls through newClaudeHTTPClient() which
  uses the utls transport, eliminating the bare Go TLS path
- Extract ResolveProxyURL() as single source of truth for proxy priority
  (auth.ProxyURL > cfg.ProxyURL > env), used by both request and refresh
- Add optional proxyURL param to NewClaudeAuth for refresh path alignment
- Elevate auth refresh failure logs from DEBUG to WARN

Supersedes router-for-me#1947.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue where Anthropic accounts were being disabled due to a TLS fingerprint mismatch. Previously, API requests used a standard Go TLS fingerprint, while token refreshes used a Chrome fingerprint, leading Anthropic to flag the tool as forged. The changes unify the TLS fingerprint across all Anthropic connections to precisely mimic the Bun BoringSSL fingerprint used by the official Claude Code CLI. This ensures consistent TLS characteristics, preventing detection as a non-official client and improving the reliability of Anthropic API interactions.

Highlights

  • TLS Fingerprint Unification: Unified all Anthropic connections (API requests and OAuth token refresh) to use the Bun BoringSSL TLS fingerprint (JA3: 50027c67d7d68e24c00d233bca146d88), matching the real Claude Code CLI.
  • Custom TLS Spec: Replaced the generic Chrome_Auto fingerprint with a custom BunBoringSSLSpec() derived from Bun v1.3.8.
  • Protocol Downgrade: Switched from HTTP/2 (manual h2.ClientConn pool) to HTTP/1.1 (http.Transport + DialTLSContext) as Bun's ALPN only offers http/1.1.
  • Centralized HTTP Client: All Claude executor API calls now route through a new newClaudeHTTPClient() using the utls transport, eliminating a separate bare Go TLS path.
  • Proxy Resolution: Extracted ResolveProxyURL() to provide a single source of truth for proxy priority (auth.ProxyURL > cfg.ProxyURL > env), ensuring consistent proxy usage for both request and refresh paths.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request provides a crucial fix by unifying the TLS fingerprint for all Anthropic connections to match the Claude Code CLI, addressing the account suspension issue. The refactoring is excellent: replacing the manual HTTP/2 connection pool with a standard http.Transport and a custom DialTLSContext greatly simplifies the code and improves maintainability. Centralizing the proxy resolution logic into ResolveProxyURL is also a significant improvement. I've found one issue regarding proxy environment variable handling in the new utls transport, for which I've left a specific comment.

// The proxyURL parameter is the pre-resolved proxy URL string; an empty string means
// no proxy (direct connection).
func newUtlsRoundTripper(proxyURL string) *utlsRoundTripper {
var dialer proxy.Dialer = proxy.Direct
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The default dialer is set to proxy.Direct, which explicitly bypasses any proxy settings from environment variables like HTTPS_PROXY. The PR description states the intended proxy priority is auth.ProxyURL > cfg.ProxyURL > env. To align with this and respect environment variables when no other proxy is configured, the default dialer should be proxy.FromEnvironment(). This function correctly falls back to a direct connection if no proxy environment variables are set.

Suggested change
var dialer proxy.Dialer = proxy.Direct
var dialer proxy.Dialer = proxy.FromEnvironment()

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ac764953b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +27 to +31
if u := strings.TrimSpace(cfg.ProxyURL); u != "" {
return u
}
}
return ""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve environment proxy fallback for empty proxy-url

When neither auth.ProxyURL nor cfg.ProxyURL is set, ResolveProxyURL() now returns "" here, and the new Anthropic transport treats that as proxy.Direct instead of “inherit”. Before this change, Claude API calls went through the default http.Client path, so deployments that rely on HTTP_PROXY/HTTPS_PROXY with proxy-url unset (the mode documented in config.example.yaml:65-67) will now bypass the proxy and fail to reach Anthropic.

Useful? React with 👍 / 👎.

Comment on lines 62 to 64
conn, err := t.dialer.Dial("tcp", addr)
if err != nil {
return nil, err

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor request context during TLS dialing

dialTLS receives a context but ignores it and calls t.dialer.Dial directly. Because this transport is built from a bare http.Transport, there is no default dial timeout to compensate, so Claude requests no longer respect cancellation/deadlines while the TCP/TLS connection is being established. A blackholed Anthropic endpoint or hung proxy can therefore block a worker indefinitely instead of timing out and triggering the existing retry/failover paths.

Useful? React with 👍 / 👎.

Address review feedback from Gemini and Codex:
- Default dialer to proxy.FromEnvironment() instead of proxy.Direct, so
  deployments relying on HTTPS_PROXY env vars continue to work when no
  explicit proxy-url is configured
- Use proxy.ContextDialer when available for context-aware TCP dial
- Propagate context deadline to TLS handshake via conn.SetDeadline to
  prevent indefinite hangs on unreachable endpoints

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lichengzhe
Copy link
Author

Addressed all review feedback in 10c1ef5:

  1. Env proxy fallback (Gemini + Codex P1): Changed default dialer from proxy.Direct to proxy.FromEnvironment(), so deployments relying on HTTPS_PROXY without explicit proxy-url config continue to work.

  2. Context support in dialTLS (Codex P2): Now uses proxy.ContextDialer when available for context-aware TCP dial, and propagates context deadline to TLS handshake via conn.SetDeadline() to prevent indefinite hangs on unreachable endpoints.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10c1ef5a81

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 37 to 40
proxyDialer, mode, errBuild := proxyutil.BuildDialer(proxyURL)
if errBuild != nil {
log.Errorf("failed to configure proxy dialer for %q: %v", cfg.ProxyURL, errBuild)
log.Errorf("failed to configure proxy dialer for %q: %v", proxyURL, errBuild)
} else if mode != proxyutil.ModeInherit && proxyDialer != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep HTTP/HTTPS proxy-url working for Claude traffic

If proxy-url is set to http://... or https://..., this path now calls proxyutil.BuildDialer(), but sdk/proxyutil/proxy.go:119-135 delegates to golang.org/x/net/proxy.FromURL, whose built-in schemes are only SOCKS (go.googlesource.com/net/proxy/proxy.go#73-100). In practice the error branch here will fire for the documented HTTP/HTTPS proxy modes from config.example.yaml:65-67, and newClaudeHTTPClient()/claude_executor.go now routes all Claude requests through this transport, so message/count-token calls will ignore the configured corporate proxy and go env/direct instead.

Useful? React with 👍 / 👎.

func newUtlsRoundTripper(proxyURL string) *utlsRoundTripper {
// Default to environment proxy (HTTPS_PROXY, etc.) so deployments that
// rely on env vars without explicit proxy-url config continue to work.
var dialer proxy.Dialer = proxy.FromEnvironment()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Restore HTTP(S)_PROXY inheritance for empty proxy-url

Fresh evidence: golang.org/x/net/proxy.FromEnvironment() only reads ALL_PROXY and NO_PROXY (go.googlesource.com/net/proxy/proxy.go#30-59,103-108), not HTTP_PROXY/HTTPS_PROXY. Switching the Claude client to this call therefore does not preserve the previous stdlib env-proxy behavior when proxy-url is unset, even though the comment says it does. Because internal/runtime/executor/proxy_helpers.go:39-41 now uses this transport for all Claude requests, deployments that rely on the documented env-proxy mode will silently bypass their proxy.

Useful? React with 👍 / 👎.

Copy link
Collaborator

@luispater luispater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary:
Thanks for unifying the Claude TLS fingerprint path. I found two blocking proxy regressions in the new uTLS transport, so I don't think this is safe to merge yet.

Key findings:

  • Blocking: explicit proxy-url values now go through proxyutil.BuildDialer(), which delegates to golang.org/x/net/proxy.FromURL(). That dialer only supports socks5/socks5h, so http://... and https://... proxies now fail and get ignored for Claude API / refresh traffic.
  • Blocking: the environment fallback is no longer equivalent to the previous net/http behavior. proxy.FromEnvironment() only checks ALL_PROXY / NO_PROXY, so standard HTTP_PROXY / HTTPS_PROXY deployments stop working. ALL_PROXY=http://... also fails for the same reason above, so only SOCKS via ALL_PROXY still works here.

Test plan:

  • Add regression coverage for proxy-url=http(s)://...
  • Add regression coverage for proxy-url=socks5://...
  • Add regression coverage for proxy-url=direct
  • Add regression coverage for HTTPS_PROXY
  • Add regression coverage for ALL_PROXY=socks5://...

Address luispater's blocking review:

1. HTTP/HTTPS proxy support: implement CONNECT tunneling in dialTLS so
   http:// and https:// proxy URLs work correctly. Previously these went
   through proxyutil.BuildDialer → proxy.FromURL which only supports
   SOCKS5, silently falling back to direct connections.

2. Environment variable compatibility: replace proxy.FromEnvironment()
   (which only reads ALL_PROXY) with custom proxyFromEnv() that checks
   HTTPS_PROXY, HTTP_PROXY, ALL_PROXY, and respects NO_PROXY — matching
   standard http.Transport behavior.

3. Restructure utlsRoundTripper to resolve proxy per-request based on
   explicit config or environment, then handle each proxy type in dialTLS:
   - Direct: net.Dialer.DialContext
   - SOCKS5: proxy.SOCKS5 dialer with ContextDialer support
   - HTTP/HTTPS: manual CONNECT tunnel with Proxy-Authorization

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lichengzhe
Copy link
Author

@luispater Thanks for catching the proxy regressions — both were real issues. Fixed in 8461a8a:

1. HTTP/HTTPS proxy support:
Replaced proxyutil.BuildDialer (SOCKS5-only) with explicit proxy handling in dialTLS:

  • SOCKS5: proxy.SOCKS5 dialer with ContextDialer support
  • HTTP/HTTPS: manual CONNECT tunneling with Proxy-Authorization header
  • Direct: net.Dialer.DialContext

2. Environment variable compatibility:
Replaced proxy.FromEnvironment() (only reads ALL_PROXY) with proxyFromEnv() that checks HTTPS_PROXY, HTTP_PROXY, ALL_PROXY (case-insensitive) and respects NO_PROXY — matching standard http.Transport behavior.

Architecture: resolveProxy(targetAddr) determines the effective proxy per-request (explicit config > env vars > direct), then dialTLS handles the tunnel/dial + utls handshake in a single path. This keeps the fingerprint consistent regardless of proxy type.

I don't have Go test infrastructure for proxy integration tests (would need a test SOCKS5/HTTP proxy), but verified locally:

  • go build ./... passes
  • go test ./internal/runtime/executor/... ./sdk/proxyutil/... passes
  • Deployed and running in production with SOCKS5 proxy

Self-review polish:

- Replace hand-rolled proxyFromEnv() with http.ProxyFromEnvironment for
  correct NO_PROXY matching (CIDR, port, wildcard) — removes ~40 lines
  of incomplete reimplementation
- Add default port for HTTP/HTTPS proxies (80/443) when proxy URL has
  no explicit port, preventing dial failures on bare hostnames
- Close CONNECT response body properly
- Handle bufio.Reader buffered bytes after CONNECT handshake with
  bufferedConn wrapper to prevent data loss on the TLS layer
- Fix newClaudeHTTPClient doc comment: empty proxyURL inherits from
  env vars, not direct

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lichengzhe
Copy link
Author

@luispater Pushed two more commits addressing both blocking issues:

8461a8a — HTTP/HTTPS proxy + env var fix:

  • Replaced proxyutil.BuildDialer (SOCKS5-only) with explicit handling per proxy type in dialTLS:
    • SOCKS5: proxy.SOCKS5 dialer with ContextDialer support
    • HTTP/HTTPS: manual CONNECT tunneling with Proxy-Authorization
    • Direct: net.Dialer.DialContext
  • Replaced proxy.FromEnvironment() (ALL_PROXY only) with custom proxyFromEnv() checking HTTPS_PROXY, HTTP_PROXY, ALL_PROXY and respecting NO_PROXY

c4bc2e9 — Self-review polish:

  • Replaced the hand-rolled proxyFromEnv() with http.ProxyFromEnvironment for correct NO_PROXY matching (CIDR, port, wildcard) — cleaner and more robust
  • Added default port for HTTP/HTTPS proxies (:80/:443) when the proxy URL has no explicit port
  • Properly close CONNECT resp.Body
  • Handle bufio.Reader buffered bytes after CONNECT handshake with bufferedConn wrapper

All tests pass locally (go build ./... + go test ./internal/runtime/executor/... ./sdk/proxyutil/...).

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4bc2e9e6e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +159 to +160
// Connect to the proxy.
conn, err := (&net.Dialer{}).DialContext(ctx, "tcp", proxyAddr)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Wrap https:// proxy connections in TLS before CONNECT

Fresh evidence: the new manual CONNECT path fixes plain http:// proxies, but https:// proxies are still broken. config.example.yaml:65-67 documents https proxy support, yet this helper always opens a raw TCP socket and immediately writes the CONNECT request, so any explicit proxy-url: https://... (or inherited HTTPS_PROXY=https://...) will fail during proxy negotiation because the proxy expects a TLS handshake first.

Useful? React with 👍 / 👎.

Comment on lines +180 to +188
if err := connectReq.Write(conn); err != nil {
conn.Close()
return nil, fmt.Errorf("write CONNECT: %w", err)
}

// Read CONNECT response. Use a bufio.Reader, then check for buffered
// bytes to avoid data loss if the proxy sent anything beyond the header.
br := bufio.NewReader(conn)
resp, err := http.ReadResponse(br, connectReq)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the request deadline while waiting for CONNECT

dialTLS only installs the context deadline after dialViaHTTPConnect returns, so the CONNECT write/read here are still unbounded. In the presence of an explicit or inherited HTTP(S) proxy that accepts the TCP socket but stalls before replying, Claude API calls and token refreshes can hang indefinitely instead of honoring request cancellation and falling back to the existing retry logic.

Useful? React with 👍 / 👎.

Two bugs in dialViaHTTPConnect:

1. HTTPS proxy connections were sent raw CONNECT requests over plain TCP.
   HTTPS proxies expect TLS first, then CONNECT inside the encrypted tunnel.
   Fix: wrap the TCP conn in standard crypto/tls before sending CONNECT.
   (utls fingerprinting is only for the final api.anthropic.com connection.)

2. The CONNECT request/response had no deadline — if a proxy accepted TCP
   but never replied, the goroutine would hang indefinitely.
   Fix: propagate context deadline to the conn before CONNECT.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lichengzhe
Copy link
Author

Pushed commit 5 with two bug fixes found during code review:

Fix 1 (P1): HTTPS proxy TLS wrapping
dialViaHTTPConnect() was sending raw CONNECT requests to https:// proxies over plain TCP. HTTPS proxies expect TLS first, then CONNECT inside the encrypted tunnel. Fixed by wrapping the TCP connection in standard crypto/tls before sending CONNECT. (utls fingerprinting is only for the final api.anthropic.com connection.)

Fix 2 (P2): CONNECT deadline
The CONNECT request/response had no deadline — if a proxy accepted TCP but never replied, the goroutine would hang indefinitely. Fixed by propagating the context deadline to the connection before CONNECT.

@luispater ready for re-review — these are the only remaining issues from the code review.

@lichengzhe lichengzhe requested a review from luispater March 18, 2026 05:52
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 222a684d72

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

return nil, err
}
httpClient := newProxyAwareHTTPClient(ctx, e.cfg, auth, 0)
httpClient := newClaudeHTTPClient(e.cfg, auth)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep honoring injected RoundTrippers for Claude requests

When CLIProxyAPI is embedded with a custom RoundTripperProvider (see sdk/cliproxy/auth/conductor.go:SetRoundTripperProvider), this replacement drops that transport from the Claude request path. execute*MixedOnce still injects cliproxy.roundtripper into execCtx, and the old newProxyAwareHTTPClient(ctx, ...) consumed it, but newClaudeHTTPClient has no ctx parameter, so HttpRequest/Execute/ExecuteStream/CountTokens now bypass custom transports entirely. In deployments that rely on that hook for mTLS, tracing, or a non-URL-based egress proxy, Claude becomes the only provider that stops using the configured transport.

Useful? React with 👍 / 👎.

Comment on lines +39 to +41
func newClaudeHTTPClient(cfg *config.Config, auth *cliproxyauth.Auth) *http.Client {
proxyURL := ResolveProxyURL(cfg, auth)
return claudeauth.NewAnthropicHttpClient(proxyURL)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reuse the Anthropic transport across Claude requests

For workloads that send repeated Claude requests, this helper now allocates a fresh http.Client and http.Transport for every call, and ClaudeExecutor invokes it on each HttpRequest/Execute/ExecuteStream/CountTokens. Because the idle-connection pool lives on the transport, none of those requests can reuse a warmed TCP/TLS session; before this change, the common no-proxy path fell back to http.DefaultTransport and did get keep-alive reuse. The result is an extra uTLS handshake on every request and noticeably higher latency/socket churn under bursty traffic.

Useful? React with 👍 / 👎.

Each call to newClaudeHTTPClient() previously created a fresh http.Client
and http.Transport, meaning every API request performed a full Bun BoringSSL
TLS handshake with no connection reuse. Under bursty traffic this caused
significant latency and socket churn.

Changes:
- Cache *http.Client per proxyURL in a sync.Map (package-level singleton).
  LoadOrStore ensures only one client per unique proxy URL survives races.
- Configure http.Transport pool parameters (MaxIdleConns=100,
  MaxIdleConnsPerHost=10, IdleConnTimeout=90s) matching DefaultTransport
  defaults for proper keep-alive reuse.
- Document why newClaudeHTTPClient intentionally does not honor context
  RoundTrippers: an injected RoundTripper replaces the entire TLS layer,
  which would break the Bun fingerprint this PR unifies. SDK embedders
  should use proxy-url instead.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lichengzhe
Copy link
Author

@luispater Thanks for the thorough review — both blocking issues have been addressed:

1. HTTP/HTTPS proxy support (commit 8461a8a)

  • Implemented manual CONNECT tunneling in dialViaHTTPConnect() for http:// and https:// proxies
  • HTTPS proxies are properly wrapped in standard crypto/tls before the CONNECT handshake (commit 222a684)
  • SOCKS5 continues to work via proxy.SOCKS5 dialer
  • Default port inference (80 for http, 443 for https) when proxy URL has no explicit port

2. Environment variable compatibility (commit c4bc2e9)

  • Replaced proxy.FromEnvironment() (which only reads ALL_PROXY) with http.ProxyFromEnvironment in resolveProxy()
  • This correctly handles HTTPS_PROXY, HTTP_PROXY, ALL_PROXY, and respects NO_PROXY (CIDR, port, wildcard) — matching standard http.Transport behavior

Additional improvements in latest commit (46ae697):

  • Connection reuse: NewAnthropicHttpClient now caches *http.Client per proxyURL via sync.Map, so the http.Transport connection pool is reused across requests. Previously each API call created a fresh transport, meaning a full TLS handshake every time.
  • Transport pool config: MaxIdleConns=100, MaxIdleConnsPerHost=10, IdleConnTimeout=90s, ExpectContinueTimeout=1s — matching http.DefaultTransport defaults.
  • Context RoundTripper: Documented as an intentional design decision. newClaudeHTTPClient does not honor cliproxy.roundtripper because an injected RoundTripper would replace the TLS layer and break the Bun fingerprint. SDK embedders should use proxy-url instead. newProxyAwareHTTPClient continues to honor it for non-Claude providers.

Ready for re-review when you get a chance.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 46ae697712

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

return nil, err
}
httpClient := newProxyAwareHTTPClient(ctx, e.cfg, auth, 0)
httpClient := newClaudeHTTPClient(e.cfg, auth)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep proxy handling on custom HTTP Claude base URLs

Claude credentials already support a custom base-url, and Manager.HttpRequest can hand this executor an arbitrary request URL. After switching all Claude traffic to newClaudeHTTPClient, plain http:// endpoints no longer honor proxy-url or HTTP_PROXY: the new transport in internal/auth/claude/utls_transport.go:56-63 only customizes DialTLSContext, which Go uses for HTTPS, and leaves Proxy unset. Any deployment that routes Claude through an internal HTTP gateway will now connect directly and fail anywhere that gateway is only reachable through the configured proxy.

Useful? React with 👍 / 👎.

Comment on lines +90 to 94
host, _, err := net.SplitHostPort(addr)
if err != nil {
return nil, err
host = addr
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve port-specific NO_PROXY matches for custom HTTPS ports

This strips the destination port before proxy resolution. resolveProxy then builds https://<host> and feeds that to http.ProxyFromEnvironment, so the NO_PROXY check is evaluated as <host>:443 regardless of the real upstream port. For custom Claude base-urls on alternate HTTPS ports (for example https://gateway.example.com:8443), a bypass like NO_PROXY=gateway.example.com:8443 will stop matching and the request will be sent through the proxy unexpectedly; the previous stdlib path used the real request URL and kept that port-sensitive behavior.

Useful? React with 👍 / 👎.

Separate proxy tunneling (SOCKS5, HTTP/HTTPS CONNECT, env fallback)
from TLS fingerprinting in utls_transport.go. Introduces ProxyDialer
as a self-contained proxy dialing abstraction.

- utls_transport.go: 277 → 113 lines (TLS fingerprint + client cache only)
- proxy_dial.go: 202 lines (all proxy tunneling logic)
- Public API unchanged (NewAnthropicHttpClient signature preserved)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@lichengzhe
Copy link
Author

Refactor: Extract proxy dialing into proxy_dial.go

Pushed aaed93ba — separates proxy tunneling from TLS fingerprinting for cleaner review.

What moved

proxy_dial.go (new, 202 lines) Stays in utls_transport.go (113 lines)
ProxyDialer struct + NewProxyDialer() utlsRoundTripper + newUtlsRoundTripper()
resolveProxy() (env/direct/explicit dispatch) dialTLS() (utls handshake only)
DialContext() (SOCKS5/CONNECT/direct switch) RoundTrip()
dialViaSocks5(), dialViaHTTPConnect() NewAnthropicHttpClient() + sync.Map cache
bufferedConn

What didn't change

  • bun_tls_spec.go — unchanged
  • Public API — NewAnthropicHttpClient(proxyURL string) signature preserved
  • All other files — unchanged

Build ✅ Tests ✅

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aaed93ba6a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +98 to +99
dialer, err := proxy.SOCKS5("tcp", proxyURL.Host, auth, proxy.Direct)
if err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve SOCKS5 default-port handling

For SOCKS5 proxies that omit an explicit port, this new path now passes proxyURL.Host straight to proxy.SOCKS5. The previous Claude transport went through sdk/proxyutil/proxy.goproxy.FromURL, which fills in the standard :1080 default, but proxy.SOCKS5 expects host:port and will fail on values like socks5://proxy.example.com (or inherited ALL_PROXY=socks5://proxy.example.com). This regresses any deployment that relies on the default SOCKS5 port instead of spelling out :1080 in the URL.

Useful? React with 👍 / 👎.

@lichengzhe
Copy link
Author

@luispater Thanks for the thorough review! Both blocking issues have been addressed in subsequent commits:

  1. HTTP/HTTPS proxy regression — Fixed in c4bc2e9 + 222a684. No longer uses proxyutil.BuildDialer(). proxy_dial.go handles all proxy types manually: SOCKS5 via proxy.SOCKS5, HTTP via TCP+CONNECT, HTTPS via TCP+TLS+CONNECT.

  2. Environment proxy fallback — Fixed in c4bc2e9. resolveProxy() delegates to http.ProxyFromEnvironment() (reads HTTPS_PROXY/HTTP_PROXY/ALL_PROXY, respects NO_PROXY) instead of proxy.FromEnvironment().

Latest commit aaed93b is a pure refactor (extract proxy dialing into own file) with no behavioral changes. Build + tests pass.

Could you re-review when you get a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants