feat(proxy): Windows system-proxy discovery behind proxy "auto" (#1525) #3209
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # wp8 — #1525 Windows `proxy: "auto"` (slice 1: startup WinINET static-proxy discovery) | ||
|
|
||
| Issue #1525 (score 60, enhancement/proxy/platform). Reviewer scoped the mergeable first slice: | ||
| startup-time WinINET static-proxy discovery behind `proxy: "auto"`, clear logs, no live mutation, | ||
| no direct fallback, PAC/WPAD deferred. Investigation by grok subagent (Poincare); see 081. | ||
|
|
||
| ## Design | ||
|
|
||
| - `src/lib/windows-system-proxy.ts` (new): `readWindowsSystemProxy(reader?)` returns | ||
| `{ kind: "proxy", url } | { kind: "disabled" } | { kind: "unsupported" } | { kind: "unreadable" } | { kind: "socks-only" }`. | ||
| Reader spawns `%SystemRoot%\System32\reg.exe query HKCU\Software\Microsoft\Windows\CurrentVersion\Internet Settings /v ProxyEnable` and `/v ProxyServer` with argv `execFileSync`, `windowsHide`, 2s timeout, never throws. Parsing: `https=` entry → `http=` entry → bare `host:port`; SOCKS-only ignored; normalized to `http://host:port`. The reader is injectable so tests never spawn `reg.exe`. | ||
| - `src/config.ts` `applyProxyEnv`: when the resolved string is exactly `auto` (case-insensitive, trimmed), call the discovery; on `proxy` continue with the resolved URL; every other outcome logs one privacy-safe line (no URL userinfo, and only host:port on success) and returns without setting `HTTP_PROXY` (today `"auto"` would be copied verbatim into `HTTP_PROXY`). Env vars still win; loopback `NO_PROXY` unchanged. | ||
| - `src/types/config.ts` JSDoc for `proxy`. No zod change (schema is passthrough and does not declare `proxy`). | ||
| - Docs: `reference/configuration/server.md` proxy row (English). | ||
| - Doctor: untouched this slice (it already hides values; `auto` shows as configured). | ||
|
|
||
| ## Out of slice | ||
| PAC/WPAD, ProxyOverride → NO_PROXY, periodic re-check, direct fallback, live mutation. | ||
|
|
||
| ## Acceptance | ||
| - Static URL / `${ENV}` / user env precedence: existing `tests/proxy-env.test.ts` unchanged and green. | ||
| - `auto` + injected reader returning proxy → `HTTP_PROXY`/`HTTPS_PROXY` set to normalized URL, log line without userinfo. | ||
| - `auto` + disabled/unsupported/unreadable/socks-only → env untouched, one log line. | ||
| - `auto` + user env set → env untouched. | ||
| - Parser unit cases: bare, `http=;https=`, `https=` only, `socks=` only, credentials stripped from log. | ||
| - tsc, privacy, focused test file. | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # wp8 audit r1 — synthesis | ||
|
|
||
| Reviewer: grok-4.6 subagent (Poincare). Verdict near-pass; all findings adopted: | ||
| - All `applyProxyEnv` callers are synchronous (`src/server/index.ts:641`, `src/codex/sync.ts:126/146/199`); a sync `reg.exe` read with argv `execFileSync`, `windowsHide`, 2s timeout mirrors `src/tray/windows.ts:361`. No await in `startServer`. | ||
| - Defer ProxyOverride: separators and `<local>` semantics differ from NO_PROXY; second policy. | ||
| - Logs: host:port only, userinfo stripped; doctor already never prints values. | ||
| - Tests inject the reader; CI never spawns `reg.exe`. | ||
| - Schema: passthrough, JSDoc only; an enum would start backing up configs. | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { execFileSync } from "node:child_process"; | ||
| import { existsSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { decodeWindowsTextBytes } from "./windows-text"; | ||
|
|
||
| /** | ||
| * Startup-time discovery of the Windows WinINET static proxy (#1525, slice 1). | ||
| * | ||
| * Reads `HKCU\Software\Microsoft\Windows\CurrentVersion\Internet Settings` once and returns a | ||
| * normalized `http://host:port` URL when a static proxy is enabled. PAC/WPAD, per-request | ||
| * resolution, ProxyOverride, live refresh, and direct fallback are deliberately out of scope: | ||
| * this is the piece an operator can audit from one log line, and everything else needs the | ||
| * transport boundary the reviewer asked for first. | ||
| */ | ||
|
|
||
| const INTERNET_SETTINGS_KEY = "HKCU\\Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings"; | ||
|
|
||
| export type WindowsSystemProxyResult = | ||
| | { kind: "proxy"; url: string } | ||
| | { kind: "disabled" } | ||
| | { kind: "socks-only" } | ||
| | { kind: "unsupported" } | ||
| | { kind: "unreadable" }; | ||
|
|
||
| /** Raw registry values; `null` when the value is absent or the read failed. */ | ||
| export interface WindowsProxyRegistryValues { | ||
| proxyEnable: string | null; | ||
| proxyServer: string | null; | ||
| } | ||
|
|
||
| export type WindowsProxyRegistryReader = () => WindowsProxyRegistryValues | null; | ||
|
|
||
| function registryExe(): string { | ||
| const candidate = join(process.env.SystemRoot ?? "C:\\Windows", "System32", "reg.exe"); | ||
| return existsSync(candidate) ? candidate : "reg.exe"; | ||
| } | ||
|
|
||
| function queryValue(name: string): string | null { | ||
| try { | ||
| const stdout = execFileSync(registryExe(), ["query", INTERNET_SETTINGS_KEY, "/v", name], { | ||
| encoding: "buffer", | ||
| stdio: ["ignore", "pipe", "ignore"], | ||
| timeout: 2000, | ||
| maxBuffer: 64 * 1024, | ||
| windowsHide: true, | ||
| }); | ||
| const text = decodeWindowsTextBytes(stdout); | ||
| // " ProxyServer REG_SZ host:port" | ||
| const line = text.split(/\r?\n/).find(row => row.trim().startsWith(name)); | ||
| if (!line) return null; | ||
| const match = line.match(/REG_(?:SZ|DWORD|EXPAND_SZ)\s+(.*)$/); | ||
| return match ? match[1]!.trim() : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| export function readWindowsProxyRegistry(): WindowsProxyRegistryValues | null { | ||
| const proxyEnable = queryValue("ProxyEnable"); | ||
| if (proxyEnable === null) return null; | ||
| return { proxyEnable, proxyServer: queryValue("ProxyServer") }; | ||
| } | ||
|
|
||
| /** | ||
| * `ProxyServer` is either a bare `host:port` (applies to every scheme) or a semicolon list of | ||
| * `scheme=host:port` entries. Prefer the https entry, then http; a SOCKS-only value cannot be | ||
| * mirrored into HTTP_PROXY/HTTPS_PROXY. | ||
| */ | ||
| export function parseWindowsProxyServer(value: string): { kind: "proxy"; url: string } | { kind: "socks-only" } | { kind: "disabled" } { | ||
| const trimmed = value.trim(); | ||
| if (!trimmed) return { kind: "disabled" }; | ||
| if (!trimmed.includes("=")) return normalize(trimmed); | ||
| const entries = new Map<string, string>(); | ||
| for (const part of trimmed.split(";")) { | ||
| const eq = part.indexOf("="); | ||
| if (eq <= 0) continue; | ||
| entries.set(part.slice(0, eq).trim().toLowerCase(), part.slice(eq + 1).trim()); | ||
| } | ||
| const candidate = entries.get("https") || entries.get("http"); | ||
| if (candidate) return normalize(candidate); | ||
| if (entries.has("socks")) return { kind: "socks-only" }; | ||
| return { kind: "disabled" }; | ||
| } | ||
|
|
||
| function normalize(hostPort: string): { kind: "proxy"; url: string } | { kind: "disabled" } { | ||
| const withScheme = /^[a-z][a-z0-9+.-]*:\/\//i.test(hostPort) ? hostPort : `http://${hostPort}`; | ||
| try { | ||
| const url = new URL(withScheme); | ||
| if (!url.hostname || (url.protocol !== "http:" && url.protocol !== "https:")) return { kind: "disabled" }; | ||
| // Keep userinfo: a credentialed proxy is valid in HTTP_PROXY. Only the log strips it. | ||
| const auth = url.username ? `${url.username}${url.password ? `:${url.password}` : ""}@` : ""; | ||
| return { kind: "proxy", url: `${url.protocol}//${auth}${url.host}` }; | ||
| } catch { | ||
| return { kind: "disabled" }; | ||
| } | ||
| } | ||
|
|
||
| export function readWindowsSystemProxy( | ||
| reader: WindowsProxyRegistryReader = readWindowsProxyRegistry, | ||
| platform: NodeJS.Platform = process.platform, | ||
| ): WindowsSystemProxyResult { | ||
| if (platform !== "win32") return { kind: "unsupported" }; | ||
| const values = reader(); | ||
| if (!values) return { kind: "unreadable" }; | ||
| // REG_DWORD prints as 0x1 / 0x0. | ||
| const enabled = /^(0x)?0*1$/i.test((values.proxyEnable ?? "").trim()); | ||
| if (!enabled) return { kind: "disabled" }; | ||
| if (!values.proxyServer) return { kind: "disabled" }; | ||
| return parseWindowsProxyServer(values.proxyServer); | ||
| } | ||
|
|
||
| /** Log-safe form: origin only, so a credentialed value can never reach the console. */ | ||
| export function describeProxyForLog(url: string): string { | ||
| try { return new URL(url).origin; } catch { return "<unparseable>"; } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The English row now documents the valid
"auto"value, but every translated counterpart (fr,ja,ko,ru,tr,zh-cn, andzh-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 👍 / 👎.