Clarify doctor proxy environment diagnostics

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/<pid>/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.

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<string, string | undefined>;
 
 /** 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<void> {
     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,15 +302,15 @@ export async function runDoctor(): Promise<void> {
   // 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.");
   }
   if (!probe.ok) {
     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.");
       }
     }
   }

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,19 +19,24 @@ 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 });
     process.env.OPENCODEX_HOME = TEST_OPENCODEX_HOME;
     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);