diff --git a/bin/cli/commands/oauth.mjs b/bin/cli/commands/oauth.mjs index c9f8386d2b..1cdaeb8267 100644 --- a/bin/cli/commands/oauth.mjs +++ b/bin/cli/commands/oauth.mjs @@ -258,8 +258,7 @@ async function runDeviceFlow(def, opts) { process.stdout.write(`\nAuthorization URL not available\n\n`); } - if (opts.browser !== false && verificationUri) - await openBrowser(verificationUri); + if (opts.browser !== false && verificationUri) await openBrowser(verificationUri); process.stderr.write("Waiting for device authorization...\n"); const deadline = Date.now() + (opts.timeout ?? 300000); const intervalMs = (start.intervalMs ?? start.interval ?? 5) * 1000; @@ -320,7 +319,18 @@ export async function runOAuthStatus(opts, cmd) { process.exit(1); } const data = await res.json(); - const connections = (data.connections ?? data.providers ?? data.items ?? data).filter( + const payload = data?.connections ?? data?.providers ?? data?.items ?? data; + // #11236 (bug 5 residual): a 200 whose body is out of contract (no + // connections/providers/items array — e.g. `{"status":"ok"}`) used to fall + // through to `.filter` on a non-array and crash with a bare TypeError plus a + // libuv teardown assertion on Windows. Coerce to an empty list with a + // sanitized one-line warning instead of dumping a stack trace. + if (!Array.isArray(payload)) { + process.stderr.write( + "Warning: unexpected response shape from /api/providers; showing no connections.\n" + ); + } + const connections = (Array.isArray(payload) ? payload : []).filter( (c) => c.authType === "oauth" || c.authType === "oauth2" ); emit(connections, globalOpts, connectionSchema); diff --git a/src/lib/services/installers/cliproxy.ts b/src/lib/services/installers/cliproxy.ts index 6ffb7e8196..dcb7dfc916 100644 --- a/src/lib/services/installers/cliproxy.ts +++ b/src/lib/services/installers/cliproxy.ts @@ -11,6 +11,7 @@ */ import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { DATA_DIR } from "@/lib/db/core"; import { upsertVersionManagerTool } from "@/lib/db/versionManager"; @@ -101,7 +102,12 @@ export async function update(): Promise { * async file I/O is not available here. */ export function resolveSpawnArgs(port: number): SpawnArgs { - const executableName = process.platform === "win32" ? "cliproxyapi.exe" : "cliproxyapi"; + // #11236 (bug 3 residual): runtime os.platform() read — a process.platform + // literal here is constant-folded to the Linux build machine when the + // published artifact is bundled, dropping the `.exe` suffix from the spawn + // path on Windows and failing with ENOENT even when a valid .exe exists + // (same fold class as b43a212680 / #10244/#10293). + const executableName = os.platform() === "win32" ? "cliproxyapi.exe" : "cliproxyapi"; const symlinkPath = path.join(BIN_DIR, executableName); fs.mkdirSync(CONFIG_DIR, { recursive: true }); diff --git a/src/lib/services/portProbe.ts b/src/lib/services/portProbe.ts index 9a890f541b..a4111400e5 100644 --- a/src/lib/services/portProbe.ts +++ b/src/lib/services/portProbe.ts @@ -15,6 +15,7 @@ import { createConnection } from "node:net"; import { spawn } from "node:child_process"; +import os from "node:os"; /** Result of probing the service before spawning. */ export interface PreSpawnProbe { @@ -190,6 +191,31 @@ export function parseNetstatPid(stdout: string, port: number): number | null { return null; } +/** + * Windows `netstat -ano` carries the pid in its own last column (#11236): + * + * Proto Local Address Foreign Address State PID + * TCP 0.0.0.0:20128 0.0.0.0:0 LISTENING 12345 + * TCP [::]:20128 [::]:0 LISTENING 12345 + * + * Only TCP LISTENING rows carry a pid (UDP rows have no state column at all). + * The local address is matched on `:` — the `:` anchor keeps a port that + * merely shares a suffix (128 vs 20128) or a foreign address ending in the + * same digits from being read as the listener. + */ +export function parseWindowsNetstatPid(stdout: string, port: number): number | null { + for (const line of stdout.split("\n")) { + const columns = line.trim().split(/\s+/); + // proto local-address foreign-address state pid + if (columns.length < 5) continue; + if (columns[3].toUpperCase() !== "LISTENING") continue; + if (!columns[1].endsWith(`:${port}`)) continue; + const pid = Number.parseInt(columns[columns.length - 1], 10); + if (Number.isFinite(pid)) return pid; + } + return null; +} + /** * Ways to ask the OS which process holds a port, in preference order. * @@ -198,6 +224,12 @@ export function parseNetstatPid(stdout: string, port: number): number | null { * once `spawn` has turned ENOENT into a null. `ss` ships with iproute2 and * `netstat` with net-tools, so between the three there is normally something * to ask on any host the supervisor runs on. + * + * The Windows `netstat -ano` probe runs last: on Windows the earlier probes + * fail fast (lsof/ss do not exist; the net-tools flags are rejected by the + * Windows netstat), while on Unix `netstat -ano` either errors out or prints + * the Linux/macOS row shapes the Windows parser deliberately never matches + * (LISTEN vs LISTENING), so it degrades to a no-op instead of a false pid. */ const PID_PROBES: ReadonlyArray<{ command: string; @@ -212,9 +244,17 @@ const PID_PROBES: ReadonlyArray<{ }, { command: "netstat", - args: () => (process.platform === "darwin" ? ["-anv", "-p", "tcp"] : ["-tlnp"]), + // #11236: runtime os.platform() read — a process.platform literal is + // constant-folded to the Linux build machine in the published artifact, + // pruning the darwin branch on macOS (same fold class as b43a212680). + args: () => (os.platform() === "darwin" ? ["-anv", "-p", "tcp"] : ["-tlnp"]), parse: parseNetstatPid, }, + { + command: "netstat", + args: () => ["-ano"], + parse: parseWindowsNetstatPid, + }, ]; /** Run one probe, resolving null on a missing binary, a non-match or a timeout. */ @@ -264,10 +304,11 @@ function runPidProbe( * way they trust a freshly-spawned one. Returns null if nothing is found or * the lookup fails/times out (best-effort; never blocks adoption on this). * - * Tries `lsof`, then `ss`, then `netstat`, so a host missing any one of them - * still reports a real pid instead of a silent null (#10431). The probes share - * one deadline, so the whole lookup still costs at most - * `PID_RESOLVE_TIMEOUT_MS`. + * Tries `lsof`, then `ss`, then `netstat`, then the Windows `netstat -ano` + * shape, so a host missing any one of them — including a stock Windows host + * with none of the Unix tools — still reports a real pid instead of a silent + * null (#10431, #11236). The probes share one deadline, so the whole lookup + * still costs at most `PID_RESOLVE_TIMEOUT_MS`. */ export async function resolvePortPid(port: number): Promise { const deadline = Date.now() + PID_RESOLVE_TIMEOUT_MS; diff --git a/src/lib/versionManager/binaryManager.ts b/src/lib/versionManager/binaryManager.ts index 743d13f54a..764168dba4 100644 --- a/src/lib/versionManager/binaryManager.ts +++ b/src/lib/versionManager/binaryManager.ts @@ -110,8 +110,16 @@ async function verifyChecksum(filePath: string, expectedSha256: string): Promise return hash.digest("hex").toLowerCase() === expectedSha256.toLowerCase(); } +/** + * #11236: read os.platform() at call time, never the build-foldable + * process.platform literal — the published-artifact build runs on Linux and + * constant-folds it, pruning the win32 branch so the managed binary lost its + * `.exe` suffix on Windows installs (same fold class as b43a212680 / + * #10244/#10293, which converted detectPlatform/detectArch; #10371 fixed the + * name in source but left this literal read behind). + */ function managedBinaryName(): string { - return process.platform === "win32" ? "cliproxyapi.exe" : "cliproxyapi"; + return os.platform() === "win32" ? "cliproxyapi.exe" : "cliproxyapi"; } function findBinaryInDir(dir: string): string | null { diff --git a/src/lib/versionManager/processManager.ts b/src/lib/versionManager/processManager.ts index ec538ab770..55164df864 100644 --- a/src/lib/versionManager/processManager.ts +++ b/src/lib/versionManager/processManager.ts @@ -152,14 +152,19 @@ export async function getProcessInfo(pid: number): Promise<{ } try { - if (process.platform === "linux" || process.platform === "android") { + // #11236: single runtime os.platform() read for the per-OS memory probes — + // a process.platform literal is constant-folded to the build machine's + // platform in the published artifact (same fold class as b43a212680 / + // #10244/#10293), so the darwin probe branch would be pruned on macOS. + const platform = os.platform(); + if (platform === "linux" || platform === "android") { const statusFile = `/proc/${pid}/status`; const content = await fs.readFile(statusFile, "utf-8"); const match = content.match(/VmRSS:\s+(\d+)\s+kB/); if (match) { return { pid, alive: true, memoryUsage: parseInt(match[1], 10) * 1024 }; } - } else if (process.platform === "darwin") { + } else if (platform === "darwin") { const { execFile } = await import("child_process"); const { promisify } = await import("util"); const execFileAsync = promisify(execFile); diff --git a/tests/unit/cli-oauth-commands.test.ts b/tests/unit/cli-oauth-commands.test.ts index 63301f818f..9d67c340a1 100644 --- a/tests/unit/cli-oauth-commands.test.ts +++ b/tests/unit/cli-oauth-commands.test.ts @@ -108,13 +108,46 @@ test("runOAuthStatus consumes the connections envelope", async () => { const parsed = JSON.parse(out); assert.deepEqual( parsed.map((connection: { id: string }) => connection.id), - ["conn1", "conn2"], + ["conn1", "conn2"] ); } finally { globalThis.fetch = origFetch; } }); +test("runOAuthStatus tolerates an out-of-contract 200 payload (#11236)", async () => { + // Bug 5 residual: #10491 added the `data.connections ??` envelope, but a 200 + // whose body is an object without connections/providers/items still fell + // through to `data` itself and crashed on `.filter is not a function` + // (followed by a libuv teardown assertion on Windows). The guard must coerce + // to an empty list and warn on stderr — never throw a raw TypeError. + const origFetch = globalThis.fetch; + // `as unknown as` (not `as any`): this file's no-explicit-any suppression is + // frozen at its pre-existing count, so new casts must be any-free. + globalThis.fetch = (() => + Promise.resolve(makeResp({ status: "ok" }))) as unknown as typeof globalThis.fetch; + + const stderrChunks: string[] = []; + const origStderr = process.stderr.write.bind(process.stderr); + process.stderr.write = ((chunk: string | Uint8Array) => { + if (typeof chunk === "string") stderrChunks.push(chunk); + return true; + }) as typeof process.stderr.write; + + try { + const { runOAuthStatus } = await import("../../bin/cli/commands/oauth.mjs"); + const out = await captureStdout(() => runOAuthStatus({}, makeCmd())); + assert.deepEqual(JSON.parse(out), []); + } finally { + globalThis.fetch = origFetch; + process.stderr.write = origStderr; + } + + const warning = stderrChunks.join(""); + assert.ok(warning.length > 0, "a sanitized warning must be written to stderr"); + assert.ok(!warning.includes("at /"), "warning must not leak a stack trace"); +}); + test("runOAuthRevoke com --yes chama endpoint de revogação", async () => { let capturedUrl = ""; let capturedMethod = ""; diff --git a/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts b/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts index a5e9d5ae6e..cc8b0a3102 100644 --- a/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts +++ b/tests/unit/services/installers/cliproxy-resolve-spawn-args-6877.test.ts @@ -12,7 +12,7 @@ * temp-directory filesystem. */ -import { describe, it, beforeEach, after } from "node:test"; +import { describe, it, beforeEach, after, mock } from "node:test"; import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; @@ -68,8 +68,11 @@ describe("resolveSpawnArgs (#6877 — real filesystem)", () => { }); it("uses the .exe command name on Windows", async () => { - const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); - Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + // resolveSpawnArgs reads os.platform() at call time (#11236 — a + // process.platform literal is constant-folded away by the Linux build of + // the published artifact), so the Windows host is simulated through the + // same runtime os.platform() seam binaryManager.test.ts uses for #10244. + const platformMock = mock.method(os, "platform", () => "win32"); try { const { resolveSpawnArgs } = @@ -78,9 +81,7 @@ describe("resolveSpawnArgs (#6877 — real filesystem)", () => { assert.equal(result.command, path.join(dataDir, "bin", "cliproxyapi.exe")); } finally { - if (originalPlatformDescriptor) { - Object.defineProperty(process, "platform", originalPlatformDescriptor); - } + platformMock.mock.restore(); } }); diff --git a/tests/unit/services/portProbePid.test.ts b/tests/unit/services/portProbePid.test.ts index c58bf86045..b9df600eac 100644 --- a/tests/unit/services/portProbePid.test.ts +++ b/tests/unit/services/portProbePid.test.ts @@ -17,6 +17,7 @@ import { parseLsofPid, parseNetstatPid, parseSsPid, + parseWindowsNetstatPid, resolvePortPid, } from "@/lib/services/portProbe"; @@ -65,8 +66,7 @@ test("parseNetstatPid matches on the local address, not the foreign one", () => }); test("parseNetstatPid reads macOS process:pid output", () => { - const stdout = - "tcp4 0 0 127.0.0.1.20128 *.* LISTEN 0 0 131072 131072 node:596922 00100\n"; + const stdout = "tcp4 0 0 127.0.0.1.20128 *.* LISTEN 0 0 131072 131072 node:596922 00100\n"; assert.equal(parseNetstatPid(stdout, 20128), 596922); }); @@ -77,6 +77,42 @@ test("parseNetstatPid ignores non-listening rows and unknown ports", () => { assert.equal(parseNetstatPid("", 20128), null); }); +/** + * Realistic `netstat -ano` sample from Windows 11 (#11236 bug 6): the pid is + * the last whitespace-separated column and only exists on rows whose state is + * LISTENING. This is the only pid probe available on a stock Windows host — + * neither lsof nor ss nor net-tools `netstat -tlnp` exist there, so a Windows + * service adopted by the supervisor reported `pid: null` while healthy. + */ +const WINDOWS_NETSTAT_ANO = [ + "Active Connections", + "", + " Proto Local Address Foreign Address State PID", + " TCP 0.0.0.0:135 0.0.0.0:0 LISTENING 1244", + " TCP 0.0.0.0:20128 0.0.0.0:0 LISTENING 12345", + " TCP 127.0.0.1:8317 0.0.0.0:0 LISTENING 5678", + " TCP 192.168.1.10:52413 140.82.121.4:443 ESTABLISHED 9012", + " TCP [::]:20128 [::]:0 LISTENING 12345", + " UDP 0.0.0.0:5353 *:* 3460", + "", +].join("\r\n"); + +test("parseWindowsNetstatPid reads the pid from a LISTENING row (#11236)", () => { + assert.equal(parseWindowsNetstatPid(WINDOWS_NETSTAT_ANO, 20128), 12345); + assert.equal(parseWindowsNetstatPid(WINDOWS_NETSTAT_ANO, 8317), 5678); +}); + +test("parseWindowsNetstatPid matches the local address, not the foreign one", () => { + // 443 appears only as a foreign address on an ESTABLISHED row. + assert.equal(parseWindowsNetstatPid(WINDOWS_NETSTAT_ANO, 443), null); + // 5353 appears only on a UDP row, which has no LISTENING state. + assert.equal(parseWindowsNetstatPid(WINDOWS_NETSTAT_ANO, 5353), null); + // A port that shares a suffix with a listening one must not match: 0128 vs + // 20128 — the `:` anchor on the local address prevents the partial hit. + assert.equal(parseWindowsNetstatPid(WINDOWS_NETSTAT_ANO, 128), null); + assert.equal(parseWindowsNetstatPid("", 20128), null); +}); + test("resolvePortPid finds the pid holding a port", async () => { const server = createServer(); await new Promise((resolve) => server.listen(29994, "127.0.0.1", resolve)); diff --git a/tests/unit/windows-platform-fold-guard-11236.test.ts b/tests/unit/windows-platform-fold-guard-11236.test.ts new file mode 100644 index 0000000000..df2812b88b --- /dev/null +++ b/tests/unit/windows-platform-fold-guard-11236.test.ts @@ -0,0 +1,168 @@ +/** + * Structural regression guard for #11236 (Windows cliproxy residuals, bugs 2+3). + * + * Why this guard exists: the published npm artifact is bundled on Linux, and + * the bundler constant-folds every literal `process.platform` read to the + * BUILD machine's platform ("linux"), pruning the win32 branch from the + * shipped artifact. Precedent: b43a212680 (#10244/#10293), which converted + * detectPlatform/detectArch to runtime `os.platform()`/`os.arch()` reads for + * exactly this reason. #10371 later fixed the Windows `.exe` binary name in + * the source but left literal `process.platform` reads behind in the same + * runtime paths, so the shipped artifact still: + * - named the managed binary `cliproxyapi` (no `.exe`) at install time + * (binaryManager.managedBinaryName), and + * - spawned that extension-less path at start time + * (installers/cliproxy.resolveSpawnArgs) -> ENOENT on Windows even with a + * valid `.exe` in place (issue #11236 bugs 2 and 3). + * + * The runtime-safe pattern is a call-time `os.platform()` read. This guard + * fails if `process.platform` reappears outside a comment in any file whose + * platform branch feeds the published artifact's runtime behavior (binary + * name, spawn path, per-OS probe selection). + */ + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); + +const GUARDED_FILES = [ + "src/lib/versionManager/binaryManager.ts", + "src/lib/versionManager/processManager.ts", + "src/lib/services/installers/cliproxy.ts", + "src/lib/services/portProbe.ts", +]; + +interface Offender { + line: number; + text: string; +} + +/** + * Returns the source with every `//` and `/* ... *\/` comment blanked out + * (replaced by spaces, newlines preserved so line numbers are stable). String + * literals are kept verbatim — a `process.platform` inside one is still + * flagged, which is acceptable: none of the guarded files carry the pattern + * in a string, and a false positive there is safer than a false negative in + * code. + */ +function stripComments(source: string): string { + let out = ""; + let i = 0; + let inBlock = false; + let inLine = false; + let inString: string | null = null; + while (i < source.length) { + const ch = source[i]; + const next = source[i + 1]; + if (inLine) { + if (ch === "\n") { + inLine = false; + out += ch; + } else { + out += " "; + } + i++; + continue; + } + if (inBlock) { + if (ch === "*" && next === "/") { + inBlock = false; + out += " "; + i += 2; + continue; + } + out += ch === "\n" ? "\n" : " "; + i++; + continue; + } + if (inString) { + out += ch; + if (ch === "\\") { + out += next ?? ""; + i += 2; + continue; + } + if (ch === inString) inString = null; + i++; + continue; + } + if (ch === "/" && next === "/") { + inLine = true; + out += " "; + i += 2; + continue; + } + if (ch === "/" && next === "*") { + inBlock = true; + out += " "; + i += 2; + continue; + } + if (ch === '"' || ch === "'" || ch === "`") inString = ch; + out += ch; + i++; + } + return out; +} + +/** + * Every remaining `process.platform` occurrence after comment stripping is an + * offender — the fold-explanation comments reference the pattern by name and + * must remain free to do so. + */ +function findFoldableReads(source: string): Offender[] { + const stripped = stripComments(source); + const offenders: Offender[] = []; + stripped.split("\n").forEach((line, index) => { + if (line.includes("process.platform")) { + offenders.push({ line: index + 1, text: source.split("\n")[index].trim() }); + } + }); + return offenders; +} + +for (const relPath of GUARDED_FILES) { + test(`${relPath} has no build-foldable process.platform reads (#11236)`, () => { + const source = fs.readFileSync(path.join(REPO_ROOT, relPath), "utf8"); + const offenders = findFoldableReads(source); + assert.deepEqual( + offenders, + [], + `${relPath} must read os.platform() at call time instead of the ` + + `build-foldable process.platform literal (Turbopack folds it to the ` + + `Linux build machine — b43a212680 / #10244 / #10371). Offenders: ` + + offenders.map((o) => `L${o.line}: ${o.text}`).join("; ") + ); + }); +} + +// Guard-the-guard (mutation check on synthetic input, so the real sources +// never need to be touched): a code occurrence MUST be caught, comment-only +// occurrences MUST be let through. +test("findFoldableReads catches a code occurrence (mutation self-check)", () => { + const snippet = [ + 'const name = process.platform === "win32" ? "a.exe" : "a";', + "// process.platform in a line comment is allowed", + "/**", + " * process.platform in a block comment is allowed", + " */", + "/* process.platform single-line block is allowed */", + "const ok = os.platform();", + ].join("\n"); + const offenders = findFoldableReads(snippet); + assert.equal(offenders.length, 1); + assert.equal(offenders[0].line, 1); +}); + +test("findFoldableReads reports nothing when only comments mention the pattern", () => { + const snippet = [ + "// process.platform", + "/* process.platform */", + "const p = os.platform();", + ].join("\n"); + assert.deepEqual(findFoldableReads(snippet), []); +});