From 3dba269b16a488f93804a7d43ff49355dd7a0f54 Mon Sep 17 00:00:00 2001 From: Ingwannu <186453546+Ingwannu@users.noreply.github.com> Date: Fri, 3 Jul 2026 11:34:40 +0900 Subject: [PATCH] clarify doctor proxy env diagnostics --- docs/adr/0002-doctor-proxy-env-diagnostics.md | 38 +++++ src/doctor.ts | 160 +++++++++++++++++- tests/doctor.test.ts | 64 +++++++ 3 files changed, 255 insertions(+), 7 deletions(-) create mode 100644 docs/adr/0002-doctor-proxy-env-diagnostics.md diff --git a/docs/adr/0002-doctor-proxy-env-diagnostics.md b/docs/adr/0002-doctor-proxy-env-diagnostics.md new file mode 100644 index 00000000000..b76b3bd3701 --- /dev/null +++ b/docs/adr/0002-doctor-proxy-env-diagnostics.md @@ -0,0 +1,38 @@ +# ADR 0002: Doctor separates shell proxy env from running proxy process env + +## Status + +Accepted + +## Context + +`ocx doctor` used to show only the proxy variables visible to the `ocx doctor` +process. That is accurate for the current shell, but misleading when a user +started `ocx start` or a service from a different environment and then ran +`ocx doctor` in a new terminal. + +Environment variables are inherited by child processes, not shared globally +between existing shells. A new terminal can therefore show `HTTP_PROXY` as unset +while the already-running opencodex proxy process still has it set. + +## Decision + +`ocx doctor` reports three separate proxy surfaces: + +- the current doctor process environment +- the effective `config.proxy` state, with the value hidden +- the running opencodex proxy process environment when a recorded PID is + available and Linux `/proc//environ` can be read + +The process environment diagnostic reports only presence/absence of known proxy +keys. It never prints or stores proxy values because proxy URLs can contain +credentials. + +## Consequences + +- Users can distinguish "this new terminal has no proxy env" from "the running + proxy process has no proxy env". +- Linux and WSL get an accurate runtime-process check without service-manager + parsing. +- Non-Linux platforms show the running-process env check as unavailable instead + of pretending service environment inspection is portable. diff --git a/src/doctor.ts b/src/doctor.ts index ed9d74c433b..73f8bcdce27 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -10,7 +10,7 @@ import { existsSync, readFileSync } from "node:fs"; import { homedir } from "node:os"; import { join, resolve } from "node:path"; -import { expandUserPath, getConfigDir, getConfigPath } from "./config"; +import { expandUserPath, getConfigDir, getConfigPath, readConfigDiagnostics, readPid, resolveEnvValue } from "./config"; import { readCodexTokens } from "./codex-auth-collision"; const WHAM_USAGE_URL = "https://chatgpt.com/backend-api/wham/usage"; @@ -78,16 +78,143 @@ function readMounts(): string | null { const PROXY_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY"] as const; export type ProxyEnvRow = { key: string; present: boolean }; +export type EnvMap = Record; /** Report only presence/absence of proxy env vars - never the value (it may * embed credentials). Checks both upper- and lower-case forms. */ -export function collectProxyEnv(): ProxyEnvRow[] { +export function collectProxyEnv(env: EnvMap = process.env): ProxyEnvRow[] { return PROXY_KEYS.map(key => ({ key, - present: !!(process.env[key]?.trim() || process.env[key.toLowerCase()]?.trim()), + present: !!(env[key]?.trim() || env[key.toLowerCase()]?.trim()), })); } +export type ConfiguredProxyDiagnostic = { + key: "config.proxy"; + present: boolean; + configured: boolean; + source: "default" | "file" | "fallback"; + detail: string; +}; + +function envReferenceName(value: string): string | null { + const braced = value.match(/^\$\{(\w+)\}$/); + if (braced) return braced[1]!; + const bare = value.match(/^\$(\w+)$/); + return bare ? bare[1]! : null; +} + +export function collectConfiguredProxy(): ConfiguredProxyDiagnostic { + const diagnostics = readConfigDiagnostics(); + const rawProxy = typeof diagnostics.config.proxy === "string" ? diagnostics.config.proxy.trim() : ""; + if (diagnostics.error) { + return { + key: "config.proxy", + present: false, + configured: false, + source: diagnostics.source, + detail: `config unreadable (${diagnostics.error})`, + }; + } + if (!rawProxy) { + return { + key: "config.proxy", + present: false, + configured: false, + source: diagnostics.source, + detail: "not configured", + }; + } + + const envName = envReferenceName(rawProxy); + const resolved = resolveEnvValue(rawProxy); + if (resolved?.trim()) { + return { + key: "config.proxy", + present: true, + configured: true, + source: diagnostics.source, + detail: envName ? `env reference ${envName} resolved` : "value hidden", + }; + } + + return { + key: "config.proxy", + present: false, + configured: true, + source: diagnostics.source, + detail: envName ? `env reference ${envName} is unset` : "empty after resolution", + }; +} + +export function parseProcessEnvBlock(content: string): EnvMap { + const env: EnvMap = {}; + for (const entry of content.split("\0")) { + if (!entry) continue; + const separator = entry.indexOf("="); + if (separator <= 0) continue; + env[entry.slice(0, separator)] = entry.slice(separator + 1); + } + return env; +} + +export type RunningProxyEnvDiagnostic = + | { status: "not_running"; rows: ProxyEnvRow[] } + | { status: "ok"; pid: number; rows: ProxyEnvRow[] } + | { status: "unavailable"; pid: number; reason: string; rows: ProxyEnvRow[] }; + +type RunningProxyEnvDeps = { + readPidFn?: () => number | null; + readEnvironFn?: (pid: number) => string | null; + platform?: NodeJS.Platform | string; +}; + +function readProcessEnviron(pid: number): string | null { + try { + return readFileSync(`/proc/${pid}/environ`, "utf-8"); + } catch { + return null; + } +} + +/* + * [Decision Log] + * - Purpose: Make `ocx doctor` distinguish the current shell env from the already-running proxy process env. + * - Alternatives: Rename the old section only; parse service-manager env for each OS; read the recorded proxy PID's env presence. + * - Rationale: PID env presence is the narrowest useful diagnostic on Linux/WSL, avoids secret value output, and keeps unsupported platforms explicit. + */ +export function collectRunningProxyEnv(deps: RunningProxyEnvDeps = {}): RunningProxyEnvDiagnostic { + const rowsWhenEmpty = () => collectProxyEnv({}); + const pid = (deps.readPidFn ?? readPid)(); + if (!pid) return { status: "not_running", rows: rowsWhenEmpty() }; + + const platform = deps.platform ?? process.platform; + if (platform !== "linux" && !deps.readEnvironFn) { + return { + status: "unavailable", + pid, + reason: "process env inspection is only supported on Linux", + rows: rowsWhenEmpty(), + }; + } + + const content = (deps.readEnvironFn ?? readProcessEnviron)(pid); + if (content === null) { + return { + status: "unavailable", + pid, + reason: "could not read process environment", + rows: rowsWhenEmpty(), + }; + } + + return { + status: "ok", + pid, + rows: collectProxyEnv(parseProcessEnvBlock(content)), + }; +} + export type WhamProbeResult = { ok: boolean; status: number | null; @@ -142,11 +269,30 @@ export async function runDoctor(): Promise { console.log(` ${row.exists ? "ok " : "-- "} ${row.label}: ${row.path}${flags ? ` (${flags})` : ""}`); } - console.log("\nProxy env (presence only)"); - for (const row of collectProxyEnv()) { + const currentProxyEnv = collectProxyEnv(); + const configuredProxy = collectConfiguredProxy(); + const runningProxyEnv = collectRunningProxyEnv(); + + console.log("\nCurrent doctor process proxy env (presence only)"); + for (const row of currentProxyEnv) { console.log(` ${row.present ? "set " : "unset "} ${row.key}`); } + console.log("\nConfigured proxy (value hidden)"); + console.log(` ${configuredProxy.present ? "set " : "unset "} ${configuredProxy.key} (${configuredProxy.source}; ${configuredProxy.detail})`); + + console.log("\nRunning proxy process proxy env (presence only)"); + if (runningProxyEnv.status === "not_running") { + console.log(" -- no running ocx proxy process found"); + } else if (runningProxyEnv.status === "unavailable") { + console.log(` -- pid ${runningProxyEnv.pid}: ${runningProxyEnv.reason}`); + } else { + console.log(` ok pid ${runningProxyEnv.pid}`); + for (const row of runningProxyEnv.rows) { + console.log(` ${row.present ? "set " : "unset "} ${row.key}`); + } + } + console.log("\nWHAM reachability"); const probe = await probeWham(); const detail = probe.status !== null ? `status=${probe.status}` : `error=${probe.classification}`; @@ -156,7 +302,7 @@ export async function runDoctor(): Promise { // Hints, not fixes. const hints: string[] = []; const anyDrvfs = paths.some(p => detectFsType(p.path, mounts).isDrvfs || detectFsType(p.path, mounts).isMntDrive); - const noProxy = collectProxyEnv().every(p => !p.present); + const noProxy = currentProxyEnv.every(p => !p.present) && !configuredProxy.present; if (anyDrvfs) { hints.push("State dir is on a Windows-mounted (/mnt) drive. Prefer the Linux home (~) under WSL for token/lock reliability."); } @@ -164,7 +310,7 @@ export async function runDoctor(): Promise { if (probe.classification === "timeout" || probe.classification === "connect_error") { hints.push("WHAM probe could not reach chatgpt.com. On WSL2 this is often NAT/DNS/VPN. Quota cannot prime, so auto-switch stays on unknown scores."); if (noProxy) { - hints.push("No *_PROXY env is set in this WSL process. If Windows uses a proxy/VPN, set HTTP(S)_PROXY here or enable WSL autoProxy so Bun fetch can reach the network."); + hints.push("No proxy is visible to this doctor process and config.proxy is unset or unresolved. If Windows uses a proxy/VPN, set config.proxy or start ocx from a shell with HTTP(S)_PROXY."); } } } diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index ce0c6191e8b..14f1b56eeab 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -5,7 +5,10 @@ import { homedir } from "node:os"; import { collectPaths, detectFsType, + collectConfiguredProxy, collectProxyEnv, + collectRunningProxyEnv, + parseProcessEnvBlock, probeWham, resolveCodexHomeDir, } from "../src/doctor"; @@ -16,12 +19,16 @@ const TEST_OPENCODEX_HOME = join(TEST_DIR, "opencodex"); let prevOpencodexHome: string | undefined; let prevCodexHome: string | undefined; let prevHttpsProxy: string | undefined; +let prevLowerHttpsProxy: string | undefined; +let prevProxyRef: string | undefined; describe("doctor", () => { beforeEach(() => { prevOpencodexHome = process.env.OPENCODEX_HOME; prevCodexHome = process.env.CODEX_HOME; prevHttpsProxy = process.env.HTTPS_PROXY; + prevLowerHttpsProxy = process.env.https_proxy; + prevProxyRef = process.env.OCX_TEST_PROXY_REF; if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); mkdirSync(TEST_CODEX_HOME, { recursive: true }); mkdirSync(TEST_OPENCODEX_HOME, { recursive: true }); @@ -29,6 +36,7 @@ describe("doctor", () => { process.env.CODEX_HOME = TEST_CODEX_HOME; delete process.env.HTTPS_PROXY; delete process.env.https_proxy; + delete process.env.OCX_TEST_PROXY_REF; }); afterEach(() => { @@ -38,6 +46,10 @@ describe("doctor", () => { else process.env.CODEX_HOME = prevCodexHome; if (prevHttpsProxy === undefined) delete process.env.HTTPS_PROXY; else process.env.HTTPS_PROXY = prevHttpsProxy; + if (prevLowerHttpsProxy === undefined) delete process.env.https_proxy; + else process.env.https_proxy = prevLowerHttpsProxy; + if (prevProxyRef === undefined) delete process.env.OCX_TEST_PROXY_REF; + else process.env.OCX_TEST_PROXY_REF = prevProxyRef; if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true }); }); @@ -96,6 +108,58 @@ describe("doctor", () => { expect(JSON.stringify(rows)).not.toContain("secret"); }); + test("parseProcessEnvBlock supports proxy presence without carrying secret values", () => { + const env = parseProcessEnvBlock([ + "HTTP_PROXY=http://user:secret@proxy.example.test:8080", + "NO_PROXY=localhost,127.0.0.1", + "", + ].join("\0")); + + const rows = collectProxyEnv(env); + expect(rows.find(r => r.key === "HTTP_PROXY")!.present).toBe(true); + expect(rows.find(r => r.key === "NO_PROXY")!.present).toBe(true); + expect(JSON.stringify(rows)).not.toContain("secret"); + }); + + test("collectRunningProxyEnv separates no pid, unreadable pid env, and pid env presence", () => { + const none = collectRunningProxyEnv({ readPidFn: () => null }); + expect(none.status).toBe("not_running"); + expect(none.rows.every(row => !row.present)).toBe(true); + + const unreadable = collectRunningProxyEnv({ + readPidFn: () => 4242, + readEnvironFn: () => null, + platform: "linux", + }); + expect(unreadable.status).toBe("unavailable"); + expect(unreadable.rows.every(row => !row.present)).toBe(true); + + const running = collectRunningProxyEnv({ + readPidFn: () => 4242, + readEnvironFn: () => "HTTPS_PROXY=http://user:secret@proxy.example.test:8080\0NO_PROXY=localhost\0", + platform: "linux", + }); + expect(running.status).toBe("ok"); + expect(running.rows.find(row => row.key === "HTTPS_PROXY")!.present).toBe(true); + expect(running.rows.find(row => row.key === "NO_PROXY")!.present).toBe(true); + expect(JSON.stringify(running)).not.toContain("secret"); + }); + + test("collectConfiguredProxy reports effective config proxy without leaking values", () => { + writeFileSync(join(TEST_OPENCODEX_HOME, "config.json"), JSON.stringify({ proxy: "${OCX_TEST_PROXY_REF}" })); + + let diagnostic = collectConfiguredProxy(); + expect(diagnostic.configured).toBe(true); + expect(diagnostic.present).toBe(false); + expect(diagnostic.detail).toContain("OCX_TEST_PROXY_REF"); + + process.env.OCX_TEST_PROXY_REF = "http://user:secret@proxy.example.test:8080"; + diagnostic = collectConfiguredProxy(); + expect(diagnostic.configured).toBe(true); + expect(diagnostic.present).toBe(true); + expect(JSON.stringify(diagnostic)).not.toContain("secret"); + }); + test("probeWham classifies ok, http error, timeout, and connect failures", async () => { const ok = await probeWham((async () => new Response("{}", { status: 200 })) as typeof fetch); expect(ok.ok).toBe(true);