Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 603f27da7e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func (h *Handler) apiCallClientTransport(auth *coreauth.Auth, requestProxyURL string) http.RoundTripper { | ||
| proxyURL := h.apiCallProxyURL(auth, requestProxyURL) | ||
| fallback := h.apiCallTransport(auth, requestProxyURL) | ||
| return newAPICallFingerprintTransport(proxyURL, fallback) |
There was a problem hiding this comment.
Preserve SOCKS5H proxy routing for fingerprinted calls
When the selected request, credential, or global proxy is socks5h://..., this newly routes ChatGPT requests into newUtlsRoundTripper, which passes that raw URL to proxyutil.BuildDialer. Its proxy.FromURL path does not recognize the socks5h scheme, logs the setup error, and retains proxy.Direct; consequently these ChatGPT management calls bypass the configured proxy (whereas the existing apiCallTransport explicitly supports SOCKS5H). Normalize or handle socks5h before constructing the fingerprint transport.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in proxyutil.BuildDialer (socks5/socks5h via SOCKS5 dialer, same as BuildHTTPTransport) + live proof on this branch.
TestFingerprintRoundTripperSOCKS5HUsesProxy: NewFingerprintRoundTripper("socks5h://127.0.0.1:<port>", fallback) GET https://chatgpt.com/ — local SOCKS5 listener logged domain CONNECT chatgpt.com:443 (remote DNS), not Direct.
Contrast TestFingerprintRoundTripperDirectMissesSOCKS: no CONNECT on the proxy.
Harness: internal/runtime/executor/helps/fingerprint_socks5h_live_test.go
Commit: 525f518
(403 without credentials is expected; the assert is proxy routing.)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 525f51874a
ℹ️ 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".
| Timeout: 12 * time.Second, | ||
| } | ||
|
|
||
| resp, errGet := client.Get("https://chatgpt.com/") |
There was a problem hiding this comment.
Move live network probes out of the unit test suite
Because this file has no build tag or environment guard, ordinary go test ./... runs both this request and the direct request in TestFingerprintRoundTripperDirectMissesSOCKS against public chatgpt.com. In offline or firewalled environments, these calls can consume their 12- and 8-second client timeouts even though the assertions only need to verify local dial routing; replace the upstream with a local TLS endpoint or gate these as opt-in integration tests. The repository specifically limits internal/runtime/executor/ to executors and unit tests.
AGENTS.md reference: AGENTS.md:L51-L51
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f9ad866.
Default go test no longer hits public chatgpt.com. SOCKS5H CONNECT proof now uses a local SOCKS5 listener (+ local control dial); BuildDialer("socks5h://…") asserts non-Direct. File renamed off the _live_test pattern.
CI green on the new tip.
|
Friendly ping @luispater @sususu98 — looking for a maintainer review when you have a moment. Status
Why it matters Happy to address any review feedback. Thanks! |
Route https://chatgpt.com through the existing Codex Chrome uTLS transport so management APICall can fetch backend-api JSON instead of Cloudflare bot-challenge HTML. Other hosts keep the standard transport and proxy priority is unchanged.
Build SOCKS5/SOCKS5H dialers via proxy.SOCKS5 instead of FromURL so fingerprinted utls clients keep remote-DNS proxy routing. Co-authored-by: 程辉 <HuiCheng@users.noreply.github.com>
Drive NewFingerprintRoundTripper through a local SOCKS5 listener and assert chatgpt.com is CONNECT'd as a domain name (remote DNS). Co-authored-by: 程辉 <HuiCheng@users.noreply.github.com>
Replace public chatgpt.com probes with a local SOCKS5 listener so default go test proves remote-DNS CONNECT without leaving the machine. Direct control dials a closed local port; BuildDialer asserts socks5h is not Direct. Co-authored-by: 程辉 <HuiCheng@users.noreply.github.com>
f9ad866 to
d8bf0be
Compare
Summary
Management
POST /v0/management/api-callwas getting Cloudflare HTML challenges when callinghttps://chatgpt.com/backend-api/*(e.g. subscriptions). Stock Go TLS fingerprint is blocked; Codex/Claude already use the shared utls fingerprint path.Changes
helps.NewFingerprintRoundTripperhelps.IsChatGPTUpstreamURL(same pattern as Anthropic)No
AGENTS.mdchanges.Base:
dev/v7.2.159.Related: seakee/CPA-Manager-Plus#743, CPA-Manager-Plus#745.
Test plan
go test ./internal/api/handlers/management/ ./internal/runtime/executor/helps/go build ./cmd/server/backend-api/subscriptions?account_id=…returns 200 JSON (not Cloudflare HTML)