diff --git a/devlog/_plan/260902_nonbug_adoption_backlog/080_wp8_windows_proxy_auto.md b/devlog/_plan/260902_nonbug_adoption_backlog/080_wp8_windows_proxy_auto.md new file mode 100644 index 0000000000..1af7d34fd5 --- /dev/null +++ b/devlog/_plan/260902_nonbug_adoption_backlog/080_wp8_windows_proxy_auto.md @@ -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. + diff --git a/devlog/_plan/260902_nonbug_adoption_backlog/081_wp8_audit_r1_synthesis.md b/devlog/_plan/260902_nonbug_adoption_backlog/081_wp8_audit_r1_synthesis.md new file mode 100644 index 0000000000..92e9e4af84 --- /dev/null +++ b/devlog/_plan/260902_nonbug_adoption_backlog/081_wp8_audit_r1_synthesis.md @@ -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 `` 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. + diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index 5b79729660..9d9803b7a1 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -12,7 +12,7 @@ runs helper features around provider requests. | --- | --- | --- | --- | | `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. | | `noProxy?` | `string \| string[]` | — | Hosts that bypass `proxy`, merged with inherited `NO_PROXY` and loopback entries. A string may use comma-separated `NO_PROXY` syntax or `${ENV_VAR}`. | | `emptyCompletionRetry?` | `boolean` | `false` | Opt in to one identical Responses retry when a turn has no text or tool call, including a stream that ends before a terminal event. The retry may be billable. `OCX_EMPTY_COMPLETION_RETRY=0` disables it without changing config; combo and routed-compaction turns remain excluded. | | `stallTimeoutSec?` | `number` | `300` | Seconds without upstream data before `response.incomplete`. Minimum 1. | diff --git a/src/config.ts b/src/config.ts index 4a2b0199d5..77f8caade8 100644 --- a/src/config.ts +++ b/src/config.ts @@ -119,6 +119,11 @@ export { type AtomicWriteIO, } from "./config/atomic-write"; import { getConfigDir, getConfigPath, hardenConfigDir } from "./config/paths"; +import { + describeProxyForLog, + readWindowsSystemProxy, + type WindowsProxyRegistryReader, +} from "./lib/windows-system-proxy"; export { expandUserPath, getConfigDir, getConfigPath, hardenConfigDir } from "./config/paths"; export { getPidPath, @@ -3526,6 +3531,14 @@ function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements * that makes outbound provider requests (server start, catalog sync). */ export function applyProxyEnv(config: OcxConfig): void { + applyProxyEnvWith(config); +} + +/** Test seam for `proxy: "auto"`: the registry reader and platform are injectable. */ +export function applyProxyEnvWith( + config: OcxConfig, + auto: { reader?: WindowsProxyRegistryReader; platform?: NodeJS.Platform } = {}, +): void { // `proxy` and `noProxy` are not declared in the top-level schema, which ends in // `.passthrough()`, so whatever is on disk arrives here verbatim. A non-string value // reached string-only methods and threw out of this function, and it runs once per @@ -3533,13 +3546,40 @@ export function applyProxyEnv(config: OcxConfig): void { // malformed values with a privacy-safe warning instead: they cannot express a routing // intent, and refusing to start is a worse answer than starting without them. const rawProxy = config.proxy; - const proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; + let proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; if (!proxy) { if (rawProxy !== undefined) warnProxyConfigDiscardOnce("proxy"); return; } + if (proxy.trim().toLowerCase() === "auto") { + // #1525 slice 1: one startup read of the Windows static proxy. Never copy the literal + // "auto" into HTTP_PROXY; every non-proxy outcome leaves outbound routing as it was. + if (process.env.HTTP_PROXY?.trim() || process.env.http_proxy?.trim() + || process.env.HTTPS_PROXY?.trim() || process.env.https_proxy?.trim()) { + 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); + if (found.kind === "proxy") { + console.log(`[opencodex] proxy "auto": using Windows system proxy ${describeProxyForLog(found.url)}`); + proxy = found.url; + } else { + const reason = found.kind === "unsupported" + ? "only Windows system proxy discovery is supported; using direct egress on this OS" + : found.kind === "disabled" + ? "Windows system proxy is disabled; using direct egress" + : found.kind === "socks-only" + ? "Windows system proxy is SOCKS-only, which HTTP_PROXY cannot express; using direct egress" + : "Windows proxy settings could not be read; using direct egress"; + console.log(`[opencodex] proxy "auto": ${reason}`); + proxy = undefined; + } + } + } + if (proxy) { if (!process.env.HTTP_PROXY?.trim() && !process.env.http_proxy?.trim()) process.env.HTTP_PROXY = proxy; if (!process.env.HTTPS_PROXY?.trim() && !process.env.https_proxy?.trim()) process.env.HTTPS_PROXY = proxy; + } const existing = process.env.NO_PROXY ?? process.env.no_proxy ?? ""; const entries = existing.split(",").map(s => s.trim()).filter(Boolean); const seen = new Set(entries.map(e => e.toLowerCase())); diff --git a/src/lib/windows-system-proxy.ts b/src/lib/windows-system-proxy.ts new file mode 100644 index 0000000000..1316698b23 --- /dev/null +++ b/src/lib/windows-system-proxy.ts @@ -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(); + 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 ""; } +} diff --git a/src/types/config.ts b/src/types/config.ts index 4a2f016f45..b59a366a5b 100644 --- a/src/types/config.ts +++ b/src/types/config.ts @@ -598,6 +598,10 @@ export interface OcxConfig { * Outbound HTTP(S) proxy URL for provider requests (e.g. "http://user:pass@proxy:8080", or * "${HTTPS_PROXY}"-style env reference). Mirrored into HTTP_PROXY/HTTPS_PROXY at startup when * those are unset — Bun's fetch honors them for all outbound calls; localhost is excluded. + * The literal `"auto"` reads the Windows WinINET static proxy (`ProxyEnable`/`ProxyServer`) + * once at process start; on other platforms, or when the system proxy is off, SOCKS-only, + * or unreadable, it degrades to direct egress with one log line (#1525). PAC/WPAD and live + * changes are not followed. */ proxy?: string; /** diff --git a/tests/proxy-env.test.ts b/tests/proxy-env.test.ts index bf7ccaa467..1439013162 100644 --- a/tests/proxy-env.test.ts +++ b/tests/proxy-env.test.ts @@ -151,3 +151,70 @@ describe("applyProxyEnv", () => { expect(process.env.HTTP_PROXY).toBe("http://ref-proxy:9999"); }); }); + +describe("applyProxyEnv with proxy: \"auto\" (#1525)", () => { + const { applyProxyEnvWith } = require("../src/config") as typeof import("../src/config"); + const { parseWindowsProxyServer, readWindowsSystemProxy } = require("../src/lib/windows-system-proxy") as typeof import("../src/lib/windows-system-proxy"); + + function capture(run: () => void): string[] { + const lines: string[] = []; + const original = console.log; + console.log = (...args: unknown[]) => { lines.push(args.map(String).join(" ")); }; + try { run(); } finally { console.log = original; } + return lines; + } + + test("parses bare, per-scheme, and socks-only ProxyServer values", () => { + expect(parseWindowsProxyServer("127.0.0.1:7890")).toEqual({ kind: "proxy", url: "http://127.0.0.1:7890" }); + expect(parseWindowsProxyServer("http=10.0.0.5:3128;https=10.0.0.6:3129;ftp=x:1")).toEqual({ kind: "proxy", url: "http://10.0.0.6:3129" }); + expect(parseWindowsProxyServer("http=10.0.0.5:3128")).toEqual({ kind: "proxy", url: "http://10.0.0.5:3128" }); + expect(parseWindowsProxyServer("socks=127.0.0.1:1080")).toEqual({ kind: "socks-only" }); + expect(parseWindowsProxyServer("")).toEqual({ kind: "disabled" }); + }); + + test("readWindowsSystemProxy honors ProxyEnable and platform", () => { + const on = () => ({ proxyEnable: "0x1", proxyServer: "127.0.0.1:7893" }); + expect(readWindowsSystemProxy(on, "win32")).toEqual({ kind: "proxy", url: "http://127.0.0.1:7893" }); + expect(readWindowsSystemProxy(() => ({ proxyEnable: "0x0", proxyServer: "127.0.0.1:7893" }), "win32")).toEqual({ kind: "disabled" }); + expect(readWindowsSystemProxy(() => null, "win32")).toEqual({ kind: "unreadable" }); + expect(readWindowsSystemProxy(on, "darwin")).toEqual({ kind: "unsupported" }); + }); + + test("auto on Windows mirrors the discovered proxy and logs only the origin", () => { + const lines = capture(() => applyProxyEnvWith(configWithProxy("auto"), { + platform: "win32", + reader: () => ({ proxyEnable: "0x1", proxyServer: "user:secret-pass-91@127.0.0.1:7893" }), + })); + expect(process.env.HTTP_PROXY).toBe("http://user:secret-pass-91@127.0.0.1:7893"); + expect(process.env.HTTPS_PROXY).toBe("http://user:secret-pass-91@127.0.0.1:7893"); + expect(process.env.NO_PROXY).toBe("localhost,127.0.0.1,::1,[::1]"); + expect(lines.join("\n")).toContain("http://127.0.0.1:7893"); + expect(lines.join("\n")).not.toContain("secret-pass-91"); + }); + + test("auto never leaks the literal into HTTP_PROXY when discovery yields nothing", () => { + for (const [platform, reader] of [ + ["darwin", () => ({ proxyEnable: "0x1", proxyServer: "127.0.0.1:1" })], + ["win32", () => ({ proxyEnable: "0x0", proxyServer: "127.0.0.1:1" })], + ["win32", () => ({ proxyEnable: "0x1", proxyServer: "socks=127.0.0.1:1080" })], + ["win32", () => null], + ] as const) { + delete process.env.HTTP_PROXY; delete process.env.HTTPS_PROXY; + const lines = capture(() => applyProxyEnvWith(configWithProxy("auto"), { platform, reader })); + expect(process.env.HTTP_PROXY).toBeUndefined(); + expect(process.env.HTTPS_PROXY).toBeUndefined(); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('proxy "auto"'); + } + }); + + test("auto defers to an existing proxy environment without consulting the registry", () => { + process.env.HTTPS_PROXY = "http://from-env:9"; + let consulted = false; + applyProxyEnvWith(configWithProxy("auto"), { platform: "win32", reader: () => { consulted = true; return null; } }); + expect(consulted).toBe(false); + expect(process.env.HTTPS_PROXY).toBe("http://from-env:9"); + expect(process.env.HTTP_PROXY).toBeUndefined(); + }); +}); +