Repository navigation
fix(import): support opt-in loopback proxy for OpenAI host-file downloads - #1418
Conversation
Codex Review Summary: ✅ CompletedThis 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: 3525086f10
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
swjnxyf
left a comment
swjnxyf
left a comment
There was a problem hiding this comment.
Reviewed PR #1418 on the special primary checkout at c691475d3b7ba1b2453f302bc7b09067ece77ebb (no worktree). No remaining blocking issues; LGTM.
The change is opt-in and defaults off. The explicit HTTP proxy is constrained to IPv4/IPv6 loopback endpoints with a written port and no credentials/path/query/fragment, and it is selected only for LoopbackTrustedMcpHostFile with an allowlisted OpenAI download hostname. Unrelated trusted URLs and authenticated/remote MCP imports retain the existing direct, DNS-pinned path. TLS verification and redirect rejection remain enabled. The documentation accurately calls out the unavoidable DNS-pinning tradeoff when a trusted local CONNECT proxy selects the upstream address.
Both earlier Codex findings are addressed by c691475: explicitly configured :80 survives URL normalization while omitted ports remain rejected, and the real loopback CONNECT socket fixture has moved to src/mcp_tests/file_import.rs in accordance with AGENTS.md. I confirmed and resolved those two review threads.
Validation on special: the previous revision's conversation-import tests passed (15/15); on the latest revision, the conversation-import helper suite passed (14/14), the MCP file-import domain tests including the local CONNECT fixture passed, and cargo fmt --all -- --check plus git diff --check passed. The original full GitHub CI completed successfully; the new full matrix triggered by c691475 is still in progress with no failed jobs at review time, so final integration remains subject to its result.
No follow-up code changes are necessary from this review.
Problem
On Windows setups using a local HTTP proxy with fake-IP DNS (RFC 2544
198.18.0.0/15), an authenticated ChatGPT host-file import may pass the trusted loopback/hostname admission checks but still fail during download.The existing downloader uses
reqwest::Client::builder().no_proxy()and pins the DNS result. In the reproduced environment, the local resolver returned a synthetic IP for an OpenAI attachment host. Direct HTTPS failed TLS verification, whereas HTTPS via the trusted local HTTP proxy could reach the actual upstream endpoint. The user-facingimport_host_filesfailure wasfailed to download.This is separate from #1138, which addresses fake-IP admission, rather than the actual HTTPS download transport.
Change
WEBCODEX_OPENAI_FILE_IMPORT_PROXY=http://127.0.0.1:PORTsetting (also acceptshttp://[::1]:PORT) forLoopbackTrustedMcpHostFileon allowlisted OpenAI attachment hosts only.Verification
On the updated upstream
mainbaseline (f8228b6):cargo fmt --allandgit diff --check: passed.cargo +stable-x86_64-pc-windows-gnu check --target x86_64-pc-windows-gnu -p webcodex --tests --offline: passed.cargo +stable-x86_64-pc-windows-gnu test --target x86_64-pc-windows-gnu -p webcodex --lib tool_runtime::conversation_import::tests:: --offline: 15 passed, 0 failed.import_host_filescall imported an approximately 24.8 MB ZIP; contents were verified by SHA-256 on the destination. The latest rebase additionally preserves non-OpenAI trusted download behavior, which is covered by unit tests. No broad cross-platform E2E claim is made.Security / tradeoff
An HTTP CONNECT proxy chooses the upstream address itself, so the proxy path cannot enforce WebCodex's direct-mode DNS pin on the eventual connection. The option therefore requires an operator-controlled, explicitly configured loopback proxy, and only applies to the already-authorized local OpenAI attachment import flow. TLS verification remains enabled. This behavior is not enabled automatically.
No authentication credentials, signed attachment URLs, private file contents, or local installation paths are included in this PR.