Repository navigation
fix(proxy): resolve upstream hostnames through trusted DNS - #18
Conversation
Windows DNS Client intermittently answered chatgpt.com with addresses belonging to an unrelated network, so the TLS handshake failed altname verification and the proxy surfaced 502 to clients. Trusted resolvers (1.1.1.1 / 8.8.8.8 / 223.5.5.5) returned correct addresses throughout, which scopes the fault to the poisoned system resolver chain. Query trusted servers directly instead of the OS resolver, reject non-routable answers, and cache results briefly. Interrogate the connection rather than the error code when deciding whether a failed request may be replayed: TLS buffers application data until the handshake completes, so a reset during the handshake proves no HTTP bytes reached the upstream and replaying is side-effect free. Resets on established or reused connections still require the normal idempotency contract. Certificate and unreachable-address failures additionally drop the cached answer for that host so the retry re-resolves. The invalidator lives on the agent to keep openai-proxy.js a composition root within its line budget.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8f72a8f08
ℹ️ 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".
| return withTimeout( | ||
| Promise.allSettled(queries).then((results) => { |
There was a problem hiding this comment.
For the normal family: 0 lookup used by the HTTPS agent, this waits for both A and AAAA promises before inspecting either result. If a resolver returns a valid A record but its AAAA query is dropped or hangs, the outer timeout rejects the entire operation and discards the usable A answer; when this happens across the trusted resolvers, the code falls back to the potentially poisoned system resolver that this change is intended to bypass. Return the successful family once the other query times out rather than rejecting the aggregate result.
Useful? React with 👍 / 👎.
| return Promise.all(attempts).then((results) => { | ||
| for (const item of results) { | ||
| if (item.result && item.result.addresses.length > 0) return item.result; | ||
| } |
There was a problem hiding this comment.
When any configured resolver is unreachable, Promise.all waits for its full timeout even if an earlier-priority resolver has already returned a usable answer. Because the default list contains three public resolvers and cache entries expire after at most 30 seconds, environments that cannot reach just one of them incur roughly the 1.5-second query timeout on every cold lookup. Resolve as soon as the highest-priority successful result is known, or query the servers sequentially.
Useful? React with 👍 / 👎.
| if (env.PROXY_ALLOW_UNTRUSTED_UPSTREAM === '1') { | ||
| return { | ||
| rejectUnauthorized: false, | ||
| dnsConfig: upstreamDnsConfigFromConfig(lastKnownConfig), | ||
| }; |
There was a problem hiding this comment.
When PROXY_ALLOW_UNTRUSTED_UPSTREAM=1, this branch never calls loadConfig, so lastKnownConfig remains null and the resolver always uses the built-in DNS servers. Consequently, an explicit upstreamDnsServers: [] cannot disable the feature and a custom resolver list is also ignored whenever the TLS environment override is enabled, which can break development or private-upstream setups that require system DNS. The environment override should affect certificate verification without bypassing the independent DNS configuration.
Useful? React with 👍 / 👎.
Problem
The proxy intermittently returned
502 Bad Gatewayto clients. The upstreamerror was:
chatgpt.comwas resolving to addresses belonging to an unrelated network, sothe TLS handshake legitimately failed certificate verification and the proxy
surfaced a gateway error.
Evidence
proxy-events.jsonlrecorded 260ERR_TLS_CERT_ALTNAME_INVALIDevents, intwo clusters.
fault was intermittent rather than an outage.
chatgpt.comvia the router (192.168.1.1) returnedwrong addresses 40/40 times.
1.1.1.1,8.8.8.8,223.5.5.5and192.168.99.1all returned correct addresses.answer, so the OS resolver chain is the failing layer.
Change
lib/proxy/upstream-dns.jsadds a resolver that queries trusted serversdirectly instead of the OS resolver chain, drops non-routable answers, and
keeps a bounded cache. Single-label and
.local/.internal/.lannames stilluse the system resolver, since public DNS cannot serve them.
lib/proxy/runtime/http-agents.jsattaches that resolver to the outbound HTTPSagent and exposes
invalidateUpstreamAddress(hostname, errorCode), which dropsthe cached answer only for failures that make the address itself suspect
(certificate mismatch,
ECONNREFUSED,EHOSTUNREACH,ENETUNREACH). Theinvalidator is attached to the agent so
openai-proxy.jsstays a compositionroot within its line budget.
lib/proxy/upstream-retry.jsclassifies a failure as replayable when ithappened before any request body was transmitted. TLS buffers application data
until the handshake completes, so a reset during the handshake proves no HTTP
bytes reached the upstream. This is detected from connection state
(
secureConnecting,reusedSocket) rather than the error code, so a reset onan established or reused connection still requires the normal idempotency
contract and
ECONNRESETis not broadly exempted.appSettings.upstreamDnsServersselects the resolver list. It defaults on,accepts an array or comma-separated string of up to 8 IPs, and an explicit
empty value disables the hardening. Invalid configuration now fails validation
instead of silently falling back to system resolution, which is the condition
that produced these 502s.
Verification
npm run verifypasses in full, including lint, syntax, CI governance,release governance, coverage, frontend, integration, and Rust tests.
test/upstream-dns.test.js,test/upstream-retry-handshake.test.js(real sockets for handshake-phaseresets, established-connection resets, and successful responses), plus
extensions to
test/upstream-retry.test.js,test/runtime-http-agents.test.js, andtest/validator.test.js.it entirely and completed a verified TLS handshake to
chatgpt.com. Afterrestarting the service, the post-restart window had 0
ERR_TLS_CERT_ALTNAME_INVALIDevents and 0 handshake-phaseECONNRESETevents, versus 260 and 93 respectively before.
Notes
The router at
192.168.1.1still returns poisoned answers forchatgpt.com.This change makes the proxy independent of it; other applications that use the
system resolver are unaffected by this fix.