feat(proxy): Windows system-proxy discovery behind proxy "auto" (#1525) - #3209
Conversation
…o" (#1525) Slice 1 of #1525. When config.proxy is the literal "auto", applyProxyEnv reads the WinINET static proxy (HKCU Internet Settings ProxyEnable / ProxyServer, https= then http= entry) once at process start and mirrors it into HTTP_PROXY/HTTPS_PROXY when those are unset, logging only the proxy origin. Non-Windows, disabled, SOCKS-only, or unreadable settings degrade to direct egress with one log line; the literal is never copied into the environment. Static URLs, ${ENV} references, user env precedence, and loopback NO_PROXY are unchanged. PAC/WPAD, live refresh, and direct fallback are deferred per the reviewer scope.
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. |
|
✅ Deterministic PR hygiene checks passed. |
![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: c98e383140
ℹ️ 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".
| console.log("[opencodex] proxy \"auto\": existing HTTP_PROXY/HTTPS_PROXY environment wins; system proxy not consulted"); | ||
| proxy = undefined; | ||
| } else { | ||
| const found = readWindowsSystemProxy(auto.reader, auto.platform); |
There was a problem hiding this comment.
During an ordinary ocx start with Codex sync enabled, startServer calls applyProxyEnv at src/server/index.ts:641, and the later startup sync reaches src/codex/sync.ts:199 and calls it again. Because this line performs discovery afresh on every invocation, a disabled or unreadable system proxy can spawn reg.exe twice and add another timeout-sized delay and duplicate log; successful discovery instead emits a second misleading “existing environment wins” message because the first invocation created those variables. Cache the automatic-discovery result/log per process or give startup a single owner for this operation.
Useful? React with 👍 / 👎.
| | `port` | `number` | `10100` | Proxy listen port. | | ||
| | `hostname?` | `string` | `"127.0.0.1"` | Bind address. Non-loopback binds require `OPENCODEX_API_AUTH_TOKEN`. | | ||
| | `proxy?` | `string` | — | Outbound HTTP(S) proxy URL or `${ENV_VAR}`. Applied to `HTTP_PROXY` / `HTTPS_PROXY` only when those variables are unset; loopback remains in `NO_PROXY`. | | ||
| | `proxy?` | `string` | — | Outbound HTTP(S) proxy URL, `${ENV_VAR}`, or `"auto"`. Applied to `HTTP_PROXY` / `HTTPS_PROXY` only when those variables are unset; loopback remains in `NO_PROXY`. `"auto"` reads the Windows system proxy (WinINET `ProxyEnable`/`ProxyServer`, `https=` then `http=` entry) once at process start and logs the host it chose. On other platforms, or when the system proxy is off, SOCKS-only, or unreadable, it uses direct egress and says so. PAC/WPAD and live proxy changes are not followed; restart the service after changing the system proxy. | |
There was a problem hiding this comment.
The English row now documents the valid "auto" value, but every translated counterpart (fr, ja, ko, ru, tr, zh-cn, and zh-tw) still defines the accepted forms as only a proxy URL or ${ENV_VAR}. This leaves localized configuration references contradicting the canonical page and omits the Windows-only behavior, direct-egress cases, and restart requirement; update those rows alongside the English source.
AGENTS.md reference: docs-site/AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
리뷰 · 우선순위 42 / 80이 PR은 이미 지금 슬라이스 1이 하는 일은 짧고 분명합니다. 테스트는 라인 3646-3649 ( 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
1 participant
Summary
"proxy": "auto", with clear logs and no live mutation or direct fallback.src/lib/windows-system-proxy.tsreadsHKCU\Software\Microsoft\Windows\CurrentVersion\Internet Settings(ProxyEnable,ProxyServer) through argvreg.exewithwindowsHide, a 2 s timeout, and the shared ANSI decoder; parses barehost:portor per-scheme lists (https=thenhttp=), ignores SOCKS-only, and normalizes to anhttp://URL (userinfo kept for the environment, stripped from the log).applyProxyEnvtreats the literalauto(trimmed, case-insensitive) as discovery: existingHTTP_PROXY/HTTPS_PROXYstill win without consulting the registry; a discovered proxy is mirrored exactly like a static URL; non-Windows, disabled, SOCKS-only, or unreadable settings log one privacy-safe line and use direct egress. The literal is never copied into the environment (today it would be).${ENV}references,noProxy, and loopbackNO_PROXYare byte-for-byte unchanged; the top-level schema is passthrough so no zod change and no config backup churn.server.mdproxy row and theproxyJSDoc. Deferred, per review: PAC/WPAD,ProxyOverride→NO_PROXY, periodic re-check, reachability probes, direct fallback.Closes #1525
Verification
bun x tsc --noEmitclean;bun run privacy:scanpassed.bun test tests/proxy-env.test.ts→ 21 pass / 0 fail (5 new: parser forms, ProxyEnable/platform gating, Windows mirror with credential-free log, four non-proxy outcomes leave env untouched with one line, env precedence skips the registry). Tests inject the registry reader; CI never spawnsreg.exe.Checklist