fix(responses): tell TLS hostname mismatches apart from an unreachabl… - #798
Conversation
…e provider A certificate that does not cover the host it was served for surfaced as `Provider unreachable: ERR_TLS_CERT_ALTNAME_INVALID fetching https://...`, which reads as if opencodex had built a wrong endpoint. Issue lidge-jun#553 was filed on that reading — the reporter concluded the adapter was mangling the Copilot URL, and a maintainer could not reproduce it because the URL was correct all along. That error almost always means TLS interception (corporate proxy, VPN, local MITM tooling) or a poisoned DNS answer, none of which the proxy can fix or is at fault for. The message now names the host, says the proxy did not rewrite it, and gives the openssl s_client command that settles which certificate is actually being served. The same three-line shape was duplicated at three catch sites in core.ts, so it moves into one helper. Sites 2 and 3 delegate entirely; site 1 keeps its own `outcome` ternary because that value also feeds recordCodexUpstreamOutcome. Verified byte-identical output against the old expression for TimeoutError, plain Error, empty-message Error, string, undefined, null, object and AbortError. While rewriting the message, redact URL userinfo. A provider base URL can carry credentials as `https://user:token@host/` and the runtime error echoes the URL it was fetching, so the old text handed that straight back to the caller. Both branches now strip it. The code match is anchored to the head of the message rather than a bare substring, so text that merely quotes the code back keeps the generic wording.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![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: eef9a86da9
ℹ️ 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 `Provider TLS certificate does not match ${target}: ${redactUrlUserinfo(detail)}. ` | ||
| + "opencodex did not rewrite this hostname — a certificate that does not cover it normally " | ||
| + "means TLS interception (corporate proxy, VPN, or local MITM tooling) or a poisoned DNS " | ||
| + `answer. Check with: openssl s_client -connect ${probe}:443 -servername ${probe} ` |
There was a problem hiding this comment.
When this TLS branch fires for a configured provider URL whose hostname contains shell metacharacters that new URL() accepts, the diagnostic emits that host unquoted inside a copy-pasteable pipeline. For example a host containing backticks or semicolons is expanded or splits the command before openssl runs, so the suggestion can execute unintended shell syntax; quote/escape the host (or avoid a ready-to-run shell pipeline) before including it here.
Useful? React with 👍 / 👎.
|
Closing. This pull request contains no work by its author. The head commit here is byte-identical to a branch that already exists in this repository and was authored by the maintainer. All six of these pull requests have the same shape: fork the repository, push the upstream branches back unchanged, and open them as incoming contributions.
Every listed SHA resolves to the same commit on the upstream branch of the same name. The commit authors inside them are This is not a rebase, a resubmission of stalled work, or a fork that drifted. It burns maintainer review time and CI minutes on a diff that is already in the tree, and it presents other people's commits under a new author's pull request. Repository access is being revoked for this account. |
2 participants
…e provider
A certificate that does not cover the host it was served for surfaced as
Provider unreachable: ERR_TLS_CERT_ALTNAME_INVALID fetching https://..., which reads as if opencodex had built a wrong endpoint. Issue #553 was filed on that reading — the reporter concluded the adapter was mangling the Copilot URL, and a maintainer could not reproduce it because the URL was correct all along. That error almost always means TLS interception (corporate proxy, VPN, local MITM tooling) or a poisoned DNS answer, none of which the proxy can fix or is at fault for.The message now names the host, says the proxy did not rewrite it, and gives the openssl s_client command that settles which certificate is actually being served.
The same three-line shape was duplicated at three catch sites in core.ts, so it moves into one helper. Sites 2 and 3 delegate entirely; site 1 keeps its own
outcometernary because that value also feeds recordCodexUpstreamOutcome. Verified byte-identical output against the old expression for TimeoutError, plain Error, empty-message Error, string, undefined, null, object and AbortError.While rewriting the message, redact URL userinfo. A provider base URL can carry credentials as
https://user:token@host/and the runtime error echoes the URL it was fetching, so the old text handed that straight back to the caller. Both branches now strip it.The code match is anchored to the head of the message rather than a bare substring, so text that merely quotes the code back keeps the generic wording.
Summary
Verification
Checklist