diff --git a/bin/ocx.mjs b/bin/ocx.mjs index 0218832de..f0e1d2daa 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -14,9 +14,15 @@ import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; +import { checkNpmCacheOwnership, formatNpmCacheOwnershipFailure } from "../src/update/npm-cache-preflight.mjs"; +import { + INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE, + runProcessTreeCommand, +} from "../src/update/install-process.mjs"; import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "../src/update/tray-update-plan.mjs"; const PKG = "@bitkyc08/opencodex"; +const NPM_INSTALL_TIMEOUT_MS = 180_000; const require = createRequire(import.meta.url); const here = dirname(fileURLToPath(import.meta.url)); const cliPath = join(here, "..", "src", "cli", "index.ts"); @@ -112,7 +118,7 @@ function runTrayLifecycle(launcher, action) { }); } -function runNpmSelfUpdate() { +async function runNpmSelfUpdate() { const current = currentPackageVersion(); const tag = updateTag(current); const npm = npmBin(); @@ -133,6 +139,15 @@ function runNpmSelfUpdate() { process.exit(0); } + const cacheOwnership = checkNpmCacheOwnership({ npmBin: npm, shell: winShell }); + if (cacheOwnership.ok === false) { + console.error(`opencodex: ${formatNpmCacheOwnershipFailure(cacheOwnership)}`); + process.exit(1); + } + if (cacheOwnership.ok === "skipped") { + console.warn(`opencodex: npm cache ownership pre-flight skipped: ${cacheOwnership.reason}. Proceeding best-effort.`); + } + // Remember whether a background service manages the proxy BEFORE stopping — `ocx stop` // unloads it permanently, so a successful update must reinstall it afterwards. const serviceStatePath = join(configDir(), "service-state.json"); @@ -222,12 +237,32 @@ function runNpmSelfUpdate() { } console.log(`Updating${latest ? ` to v${latest}` : ""}...\n$ ${npm} install -g ${PKG}@${tag}`); - const res = spawnSync(npm, ["install", "-g", `${PKG}@${tag}`], { + const res = await runProcessTreeCommand(npm, ["install", "-g", `${PKG}@${tag}`], { stdio: "inherit", - timeout: 180000, + timeoutMs: NPM_INSTALL_TIMEOUT_MS, windowsHide: true, shell: winShell, }); + if (!res.treeExited) { + console.error("\nopencodex: installer process-tree cleanup could not be confirmed; automatic recovery is unsafe until those processes exit."); + console.error(` The proxy is stopped. Once no '${npm}' installer processes remain, run 'ocx start' or re-run 'ocx update'.${trayBeforeUpdate.restoreOnFailure ? " The Windows tray also remains stopped; run 'ocx tray start' after the installer processes exit." : ""}`); + process.exit(INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE); + } + if (res.interruptedSignal) { + if (trayBeforeUpdate.restoreOnFailure) runTrayLifecycle(launcher, "start"); + console.error(`\nopencodex: update interrupted (${res.interruptedSignal}); the proxy is still stopped. Run 'ocx start' or re-run 'ocx update'.`); + const exitCode = res.interruptedSignal === "SIGINT" + ? 130 + : res.interruptedSignal === "SIGHUP" + ? 129 + : 143; + if (process.platform === "win32") { + process.exit(exitCode); + } + process.kill(process.pid, res.interruptedSignal); + // Do not return to the update call site while fatal signal delivery is pending. + process.exit(exitCode); + } if (res.status === 0) { console.log(`\nUpdated${latest ? ` to v${latest}` : ""}.`); repairCodexShimIfNeeded(); @@ -278,7 +313,12 @@ function runNpmSelfUpdate() { process.exit(0); } if (trayBeforeUpdate.restoreOnFailure) runTrayLifecycle(launcher, "start"); - console.error(`\nUpdate failed (${npm} exit ${res.status ?? "?"}). Try manually: ${npm} install -g ${PKG}@${tag}`); + const failure = res.timedOut + ? `timed out after ${Math.trunc(NPM_INSTALL_TIMEOUT_MS / 1000)}s` + : res.error + ? `could not run: ${res.error.message}` + : `exit ${res.status ?? "?"}`; + console.error(`\nUpdate failed (${npm} ${failure}). Try manually: ${npm} install -g ${PKG}@${tag}`); process.exit(1); } @@ -351,7 +391,7 @@ if (updateHelpRequested) { } if (process.argv[2] === "update" && isNodeModulesInstall() && !isBunGlobalInstall()) { - runNpmSelfUpdate(); + await runNpmSelfUpdate(); } const bun = resolveBun(); diff --git a/docs-site/src/content/docs/ja/reference/cli.md b/docs-site/src/content/docs/ja/reference/cli.md index 442e21d53..a0e8d67af 100644 --- a/docs-site/src/content/docs/ja/reference/cli.md +++ b/docs-site/src/content/docs/ja/reference/cli.md @@ -423,6 +423,8 @@ npm から opencodex を自己更新します。安定版インストールは ` `git pull && bun install` を案内し、該当タグの最新版なら何もしません。ファイルを 差し替える前に実行中のプロキシを停止します。インストール済みのサービスは再ビルドして自動起動し、 フォアグラウンドインストールには次のステップとして `ocx start` を案内します。 +Unix では、更新前に設定済み npm キャッシュが現在のユーザー所有か確認します。所有者の異なる +エントリが見つかった場合やキャッシュを検査できない場合は、実行中のプロキシを停止する前に更新を中止します。 ```bash ocx update diff --git a/docs-site/src/content/docs/ko/reference/cli.md b/docs-site/src/content/docs/ko/reference/cli.md index 7ab091ef0..8708dfab7 100644 --- a/docs-site/src/content/docs/ko/reference/cli.md +++ b/docs-site/src/content/docs/ko/reference/cli.md @@ -442,6 +442,8 @@ npm에서 opencodex를 자체 업데이트합니다. 안정판 설치는 `@lates `git pull && bun install`을 안내하고, 해당 태그의 최신 버전이면 아무 작업도 하지 않습니다. 파일을 교체하기 전에 실행 중인 프록시를 중지합니다. 설치된 서비스는 다시 빌드해 자동으로 시작하고, 포그라운드 설치에는 다음 단계로 `ocx start`를 안내합니다. +Unix에서는 업데이트 전에 설정된 npm 캐시가 현재 사용자 소유인지 확인합니다. 다른 소유자의 항목이 +있거나 캐시를 검사할 수 없으면 실행 중인 프록시를 중지하기 전에 업데이트를 중단합니다. ```bash ocx update diff --git a/docs-site/src/content/docs/reference/cli.md b/docs-site/src/content/docs/reference/cli.md index 010f4b6bf..0c01b7693 100644 --- a/docs-site/src/content/docs/reference/cli.md +++ b/docs-site/src/content/docs/reference/cli.md @@ -499,7 +499,11 @@ Self-update opencodex from npm. Stable installs use `@latest`; preview installs unless you pass `--tag latest|preview`. It detects a source checkout and tells you to `git pull && bun install` instead, and is a no-op if you're already on the newest version for that tag. A running proxy is stopped before files are replaced; an installed service is rebuilt and -started automatically, while a foreground installation prints `ocx start` as the next step. +started automatically, while a foreground installation prints `ocx start` as the next step. On +Unix, the updater first checks that the configured npm cache is owned by the current user. It aborts +before stopping the proxy when it finds a foreign-owned cache entry or cannot inspect the cache, so +you can correct the cache ownership or configure a user-owned cache and retry without losing the +running service. ```bash ocx update diff --git a/docs-site/src/content/docs/ru/reference/cli.md b/docs-site/src/content/docs/ru/reference/cli.md index 15a460279..655a8bbed 100644 --- a/docs-site/src/content/docs/ru/reference/cli.md +++ b/docs-site/src/content/docs/ru/reference/cli.md @@ -470,6 +470,8 @@ ocx debug usage logs [-f|--follow] если у вас уже новейшая версия для этого тега. Работающий прокси останавливается перед заменой файлов; установленный сервис пересобирается и запускается автоматически, а при установке, работающей на переднем плане, команда печатает `ocx start` как следующий шаг. +В Unix перед обновлением проверяется, что настроенный кеш npm принадлежит текущему пользователю. +Если найден элемент с другим владельцем или кеш невозможно проверить, обновление прерывается до остановки прокси. ```bash ocx update diff --git a/docs-site/src/content/docs/zh-cn/reference/cli.md b/docs-site/src/content/docs/zh-cn/reference/cli.md index 9b744afcb..999539854 100644 --- a/docs-site/src/content/docs/zh-cn/reference/cli.md +++ b/docs-site/src/content/docs/zh-cn/reference/cli.md @@ -424,6 +424,8 @@ provider debug 默认读取 `OCX_DEBUG=1`（旧的 `OCX_DEBUG_FRAMES=1` 仍可 `--tag latest|preview`。在源码 checkout 中，它会改为提示 `git pull && bun install`；如果已经是 相应 tag 的最新版，则不执行任何操作。替换文件前会停止正在运行的代理；已安装的服务会自动重建 并启动，而前台安装会把 `ocx start` 显示为下一步。 +在 Unix 上，更新器会先检查配置的 npm 缓存是否全部归当前用户所有。如果发现其他用户拥有的条目， +或无法检查缓存，更新会在停止代理前中止。 ```bash ocx update diff --git a/docs/adr/0001-gui-update-worker.md b/docs/adr/0001-gui-update-worker.md index 056db8e8a..4b9d8e299 100644 --- a/docs/adr/0001-gui-update-worker.md +++ b/docs/adr/0001-gui-update-worker.md @@ -19,8 +19,59 @@ detached hidden CLI worker, and the worker performs the install command and opti For npm installs, the worker runs the Node launcher path (`node bin/ocx.mjs update --tag `) so the existing npm self-update guard is reused. For Bun global installs, it runs the existing Bun -global update command. Source checkouts remain manual-only and show `git pull && bun install && -bun run build:gui`. +global update command. The outer GUI installer command also runs through the shared process-tree +runner; a failed npm or Bun update is recoverable only when that runner confirms the installer tree +has exited. Source checkouts remain manual-only and show `git pull && bun install && bun run build:gui`. + +Before an npm updater stops the proxy, it resolves the configured npm cache and checks that every +entry is owned by the current Unix user. A foreign-owned entry (commonly left by an older `sudo npm` +invocation) aborts the update with an actionable error while the existing proxy and service remain +running. The scan fails closed when the cache root is absent, an entry cannot be inspected, or its +50,000-entry / 10-second traversal budget is exhausted. Only the configured cache-root symlink is +canonicalized; nested cache symlinks are rejected fail-closed so a foreign-owned target cannot be +hidden behind one. The blocking filesystem walk runs in a child process so the parent can enforce +the deadline even when a filesystem call itself stalls. +Failure logs omit both cache and entry paths; users can locate the cache explicitly with +`npm config get cache`. Platforms without Unix uid ownership skip this gate. + +The GUI worker retries its identity-checked pre-update liveness capture before deciding a service +was inactive. If health remains unavailable, a matching runtime-port record plus an identity-checked +live PID still preserves the pre-update activity evidence. If the installer exits nonzero, the worker +probes again immediately before recovery and leaves any old or concurrently replaced process alone +when health or PID identity proves it is OpenCodex. When npm already stopped the proxy and retired +the old package, the worker validates each candidate's trusted ownership, name, version, path +containment, and complete launcher runtime with a side-effect-free `--version` probe. On UID-capable +platforms, the scope and complete package tree must also reject foreign owners, group/world-writable +entries, and symlinks that leave the candidate tree before any code executes. Trusted npm-generated +links whose immediate and final targets remain inside the candidate tree are allowed. The tree walk +is bounded by entry count and elapsed time. Candidate inspection and restart are bounded to at most +two candidates, preferring the current package and then the newest retired copies, and it tries those runnable +candidates in order until one restores health. The recovery-tree walk runs in its own short-lived +worker, and the GUI worker force-kills that child at the wall-clock deadline even if one filesystem +call blocks. A recovery launcher is always started directly: it +may restore availability, but its potentially temporary path is never persisted into launchd, +systemd, Task Scheduler, or the update job log; the log stores only a path-free candidate label. +If a candidate starts but does not become healthy, the worker retains the detached child handle, +then requires both the same process generation and a fresh OpenCodex identity before stopping that +PID and trying the next candidate. An exited child or missing generation proof fails closed so PID +reuse can never terminate a concurrent replacement proxy. +Service and direct restart paths also stop when the pidfile has changed to a different identity-checked +proxy, and every such identity decision re-reads the process command line instead of trusting a +PID-only cache across the update boundary. The update job remains failed either way because restoring +availability is not the same as installing the new version. + +The npm launcher and the Bun/source CLI installer paths run in an isolated process tree. On timeout, +the updater terminates and awaits the known POSIX process group or a Windows `taskkill /T /F` tree; +piped CLI output is drained and bounded before it is replayed. A POSIX process group is not an OS +containment boundary because a lifecycle child can leave it with `setsid` or `setpgid`. Therefore a +timeout, interruption, or nonzero POSIX root exit is always reported as unconfirmed even when the +known group was successfully stopped, and automatic recovery stays disabled. Once a POSIX root has +exited, the updater also refuses to signal its leaderless group by a reusable numeric PGID. While the +root is live, cleanup treats zombie-only groups as stopped and revalidates group membership and the +original leader immediately before each signal; uninspectable or replacement-led groups are refused. +Windows has no retained job-object handle after a normally failed installer root exits, so that case +likewise remains explicitly unconfirmed. The outer worker timeout remains longer than the nested deadline, and +recovery is skipped whenever either layer cannot prove installer-tree shutdown. After an update requests a restart, the worker now waits for an identity-checked `/healthz` to return and remain healthy for a short stability window before marking the job successful. This @@ -45,6 +96,10 @@ restart path because `bun add -g` does not restart the proxy. - The GUI request handler stays responsive and does not overwrite its own running module graph. - Update status survives a proxy restart because it is stored in the opencodex config directory. - Restart handling can branch between service-managed installs and direct detached proxy starts. +- npm cache ownership failures are detected before service or proxy shutdown. +- Automatic recovery starts only when installer shutdown is confirmed; ambiguous POSIX failures and + escaped descendants remain fail-closed with manual remediation. +- A failed install best-effort restores a previously-active proxy without claiming update success. - A completed install can still finish with `status: "failed"` when the replacement proxy never becomes healthy or flaps during the stability window; the job log then points the user at `ocx start` and the Bun `--allow-scripts` reinstall path. diff --git a/src/config.ts b/src/config.ts index c056e1954..73d8103df 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1225,19 +1225,41 @@ export function isOcxStartCommandLine(commandLine: string): boolean { return hasOcxEntrypoint && /(?:^|[\s"'])start(?:$|[\s"'])/.test(normalized); } +function isLikelyOcxStartProcessUncached(pid: number): boolean | undefined { + const commandLine = readProcessCommandLine(pid); + if (commandLine === undefined) return undefined; + return isOcxStartCommandLine(commandLine); +} + /** Per-process memo: waitForProxy/findLiveProxy used to spawn powershell on every 150ms poll. */ const ocxStartProcessCache = new Map(); -function isLikelyOcxStartProcess(pid: number): boolean { - const cached = ocxStartProcessCache.get(pid); +function lookupOcxStartProcess( + pid: number, + readCommandLine: (pid: number) => string | undefined, + cache: Map, +): boolean | undefined { + const cached = cache.get(pid); if (cached !== undefined) return cached; - const commandLine = readProcessCommandLine(pid); - if (commandLine === undefined) return false; + const commandLine = readCommandLine(pid); + if (commandLine === undefined) return undefined; const ok = isOcxStartCommandLine(commandLine); - ocxStartProcessCache.set(pid, ok); + cache.set(pid, ok); return ok; } +function isLikelyOcxStartProcess(pid: number): boolean | undefined { + return lookupOcxStartProcess(pid, readProcessCommandLine, ocxStartProcessCache); +} + +export function lookupOcxStartProcessForTests( + pid: number, + readCommandLine: (pid: number) => string | undefined, + cache = new Map(), +): boolean | undefined { + return lookupOcxStartProcess(pid, readCommandLine, cache); +} + /** * Alive pid from the pid file without the expensive Windows command-line probe. * Safe for liveness polls: callers still identity-check /healthz before trusting the proxy. @@ -1268,7 +1290,31 @@ export function verifyPidIdentity(candidatePid: number): number | null { } catch (e: unknown) { if ((e as NodeJS.ErrnoException).code !== "EPERM") return null; } - return isLikelyOcxStartProcess(candidatePid) ? candidatePid : null; + return isLikelyOcxStartProcess(candidatePid) === true ? candidatePid : null; +} + +/** + * Re-read the process command line for update/recovery decisions that span enough + * time for the OS to reuse a PID. Never trust the per-process polling cache here. + */ +export function verifyPidIdentityFresh(candidatePid: number): number | null { + try { + process.kill(candidatePid, 0); + } catch (e: unknown) { + if ((e as NodeJS.ErrnoException).code !== "EPERM") { + ocxStartProcessCache.delete(candidatePid); + return null; + } + } + const isOcx = isLikelyOcxStartProcessUncached(candidatePid); + if (isOcx === undefined) { + ocxStartProcessCache.delete(candidatePid); + return null; + } + // This read is authoritative across a PID-reuse boundary. Reconcile the + // polling memo so later short-lived checks cannot serve the old identity. + ocxStartProcessCache.set(candidatePid, isOcx); + return isOcx ? candidatePid : null; } function readProcessCommandLine(pid: number): string | undefined { diff --git a/src/update/index.ts b/src/update/index.ts index 7cd453b64..9ac85db08 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -3,6 +3,11 @@ import { readFileSync, readdirSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; import { getConfigDir, loadConfig, readPid, readRuntimePort } from "../config"; +import { checkNpmCacheOwnership, formatNpmCacheOwnershipFailure } from "./npm-cache-preflight.mjs"; +import { + INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE, + runProcessTreeCommand, +} from "./install-process.mjs"; import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "./tray-update-plan.mjs"; /** @@ -164,6 +169,17 @@ export async function runUpdate(): Promise { console.log(`Verified ${PKG}@${latest} integrity metadata ${integrity.integrity.slice(0, 24)}…`); } + if (installer === "npm") { + const cacheOwnership = checkNpmCacheOwnership(); + if (cacheOwnership.ok === false) { + console.error(`⚠️ ${formatNpmCacheOwnershipFailure(cacheOwnership)}`); + process.exit(1); + } + if (cacheOwnership.ok === "skipped") { + console.warn(`⚠️ npm cache ownership pre-flight skipped: ${cacheOwnership.reason}. Proceeding best-effort.`); + } + } + // Remember whether a background service manages the proxy BEFORE stopping — `ocx stop` // unloads it permanently, so a successful update must reinstall/restart it afterwards. let serviceWasInstalled = false; @@ -245,14 +261,29 @@ export async function runUpdate(): Promise { const target = npmSpawnTarget(bin); const installStdio = updateChildStdio(); - const r = spawnSync(target.bin, cmdArgs, { + const r = await runProcessTreeCommand(target.bin, cmdArgs, { stdio: installStdio, - encoding: installStdio === "pipe" ? "utf8" : undefined, - timeout: 180000, + timeoutMs: 180000, windowsHide: true, shell: target.shell, }); if (installStdio === "pipe") logSpawnOutput("", r); + if (!r.treeExited) { + console.error("\nopencodex: installer process-tree cleanup could not be confirmed; automatic recovery is unsafe until those processes exit."); + console.error(` The proxy is stopped. Once no '${target.bin}' installer processes remain, run 'ocx start' or re-run 'ocx update'.${trayWasRunning ? " The Windows tray also remains stopped; run 'ocx tray start' after the installer processes exit." : ""}`); + process.exit(INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE); + } + if (r.interruptedSignal) { + if (trayWasRunning) { + try { + const { startWindowsTray } = await import("../tray/windows"); + startWindowsTray(); + } catch { /* preserve the primary interruption */ } + } + const signalExitCode = r.interruptedSignal === "SIGINT" ? 130 : r.interruptedSignal === "SIGHUP" ? 129 : 143; + console.error(`\nopencodex: update interrupted (${r.interruptedSignal}); the proxy is still stopped. Run 'ocx start' or re-run 'ocx update'.`); + process.exit(signalExitCode); + } if (r.status === 0) { console.log(`\n✅ Updated${latest ? ` to v${latest}` : ""}.`); // Re-bake the bundled Bun path into the Codex autostart shim on every @@ -340,7 +371,12 @@ export async function runUpdate(): Promise { startWindowsTray(); } catch { /* keep the primary update failure */ } } - console.error(`\n⚠️ Update failed (${bin} exit ${r.status ?? "?"}). Try manually: ${bin} ${cmdArgs.join(" ")}`); + const failure = r.timedOut + ? "timed out after 180s" + : r.error + ? `could not run: ${r.error.message}` + : `exit ${r.status ?? "?"}`; + console.error(`\n⚠️ Update failed (${bin} ${failure}). Try manually: ${bin} ${cmdArgs.join(" ")}`); process.exit(1); } } diff --git a/src/update/install-process.d.mts b/src/update/install-process.d.mts new file mode 100644 index 000000000..0408c5e64 --- /dev/null +++ b/src/update/install-process.d.mts @@ -0,0 +1,52 @@ +import type { StdioOptions } from "node:child_process"; + +export const INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE: number; +export const MAX_CAPTURED_OUTPUT_CHARS: number; + +export interface ProcessTreeCommandOptions { + timeoutMs?: number; + stdio?: StdioOptions; + windowsHide?: boolean; + shell?: boolean | string; + env?: NodeJS.ProcessEnv; + terminationGraceMs?: number; + forceWaitMs?: number; + inspectProcessGroup?: (groupId: number) => ProcessGroupInspection | null; +} + +export interface ProcessTreeCommandResult { + status: number | null; + signal: NodeJS.Signals | null; + error?: Error; + interruptedSignal: NodeJS.Signals | null; + timedOut: boolean; + treeExited: boolean; + stdout?: string; + stderr?: string; +} + +export interface ProcessGroupInspection { + hasRunningMember: boolean; + hasRunningLeader: boolean; +} + +export type ProcessGroupForceDecision = "exited" | "signal" | "refuse"; + +export function processGroupForceDecision( + inspection: ProcessGroupInspection | null, + originalLeaderConfirmed: boolean, +): ProcessGroupForceDecision; + +export function terminateInstallerProcessTree( + pid: number | undefined, + options?: Pick & { + isOriginalLeader?: () => boolean; + inspectProcessGroup?: (groupId: number) => ProcessGroupInspection | null; + }, +): Promise; + +export function runProcessTreeCommand( + bin: string, + args: string[], + options?: ProcessTreeCommandOptions, +): Promise; diff --git a/src/update/install-process.mjs b/src/update/install-process.mjs new file mode 100644 index 000000000..bb533cdc4 --- /dev/null +++ b/src/update/install-process.mjs @@ -0,0 +1,349 @@ +import { spawn, spawnSync } from "node:child_process"; +import { readdirSync, readFileSync } from "node:fs"; + +const TREE_POLL_INTERVAL_MS = 100; +const DEFAULT_TERMINATION_GRACE_MS = 5_000; +const DEFAULT_FORCE_WAIT_MS = 5_000; +export const MAX_CAPTURED_OUTPUT_CHARS = 4_000; + +function appendBoundedOutput(current, chunk) { + const text = typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"); + const next = current + text; + return next.length > MAX_CAPTURED_OUTPUT_CHARS + ? next.slice(-MAX_CAPTURED_OUTPUT_CHARS) + : next; +} + +/** Exit code used when the updater cannot prove that its installer tree is gone. */ +export const INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE = 75; + +function isNoSuchProcess(error) { + return error && typeof error === "object" && "code" in error && error.code === "ESRCH"; +} + +function inspectLinuxProcessGroup(groupId) { + try { + let hasRunningMember = false; + let hasRunningLeader = false; + for (const entry of readdirSync("/proc", { withFileTypes: true })) { + if (!entry.isDirectory() || !/^\d+$/.test(entry.name)) continue; + let stat; + try { + stat = readFileSync(`/proc/${entry.name}/stat`, "utf8"); + } catch { + continue; + } + const commandEnd = stat.lastIndexOf(")"); + if (commandEnd === -1) continue; + const fields = stat.slice(commandEnd + 2).trim().split(/\s+/); + const state = fields[0]; + const processGroup = Number.parseInt(fields[2] ?? "", 10); + if (processGroup !== groupId || state === "Z" || state === "X") continue; + hasRunningMember = true; + if (Number.parseInt(entry.name, 10) === groupId) hasRunningLeader = true; + } + return { hasRunningMember, hasRunningLeader }; + } catch { + return null; + } +} + +function inspectPsProcessGroup(groupId) { + const result = spawnSync("/bin/ps", ["-axo", "pid=,pgid=,state="], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + timeout: 2_000, + windowsHide: true, + }); + if (result.status !== 0 || typeof result.stdout !== "string") return null; + let hasRunningMember = false; + let hasRunningLeader = false; + for (const line of result.stdout.split(/\r?\n/)) { + const match = /^\s*(\d+)\s+(\d+)\s+(\S+)/.exec(line); + if (!match || Number.parseInt(match[2], 10) !== groupId) continue; + if (match[3].startsWith("Z") || match[3].startsWith("X")) continue; + hasRunningMember = true; + if (Number.parseInt(match[1], 10) === groupId) hasRunningLeader = true; + } + return { hasRunningMember, hasRunningLeader }; +} + +function inspectProcessGroup(groupId) { + if (process.platform === "linux") return inspectLinuxProcessGroup(groupId); + if (process.platform === "darwin" || process.platform === "freebsd" || process.platform === "openbsd") { + return inspectPsProcessGroup(groupId); + } + return null; +} + +export function processGroupForceDecision(inspection, originalLeaderConfirmed) { + if (!inspection) return "refuse"; + if (!inspection.hasRunningMember) return "exited"; + if (inspection.hasRunningLeader && !originalLeaderConfirmed) return "refuse"; + return "signal"; +} + +function processGroupTerminationDecision(inspection, originalLeaderConfirmed) { + if (!inspection) return "refuse"; + if (!inspection.hasRunningMember) return "exited"; + // SIGTERM is sent only while the original root still anchors the group ID. + // Leaderless groups are handled fail-closed after root exit. + if (!inspection.hasRunningLeader || !originalLeaderConfirmed) return "refuse"; + return "signal"; +} + +function processGroupHasRunningMember(groupId, inspect = inspectProcessGroup) { + return inspect(groupId)?.hasRunningMember ?? null; +} + +function inspectProcessGroupAfterLeaderExit(groupId, inspect = inspectProcessGroup) { + const inspection = inspect(groupId); + if (!inspection) return "unknown"; + // A new group cannot reuse this ID without a live leader whose PID equals the + // group ID. Never signal that group; it is unrelated to the completed child. + if (inspection.hasRunningLeader) return "reused"; + return inspection.hasRunningMember ? "running" : "exited"; +} + +function isProcessTreeAlive(pid, inspect = inspectProcessGroup) { + try { + process.kill(process.platform === "win32" ? pid : -pid, 0); + if (process.platform === "win32") return true; + // Zombies keep a process group addressable by signal 0 but cannot mutate the + // package tree. Treat a zombie-only group as fully stopped. + return processGroupHasRunningMember(pid, inspect) ?? true; + } catch (error) { + return !isNoSuchProcess(error); + } +} + +function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +async function waitForProcessTreeExit(pid, timeoutMs, inspect = inspectProcessGroup) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!isProcessTreeAlive(pid, inspect)) return true; + await sleep(TREE_POLL_INTERVAL_MS); + } + return !isProcessTreeAlive(pid, inspect); +} + +/** + * Terminate a detached installer and every descendant before package recovery starts. + * POSIX uses the installer's process group; Windows snapshots and kills the tree while + * the root process is still alive via taskkill /T /F. + */ +export async function terminateInstallerProcessTree( + pid, + { + terminationGraceMs = DEFAULT_TERMINATION_GRACE_MS, + forceWaitMs = DEFAULT_FORCE_WAIT_MS, + isOriginalLeader, + inspectProcessGroup: inspectOverride, + } = {}, +) { + if (!Number.isSafeInteger(pid) || pid <= 0) return true; + const inspect = inspectOverride ?? inspectProcessGroup; + + if (process.platform === "win32") { + // Once the root has exited, Node has no job-object handle with which to prove + // that background descendants also exited. Fail closed instead of treating a + // missing root PID as proof that the complete installer tree is gone. + if (!isProcessTreeAlive(pid, inspect)) return false; + const taskkill = `${process.env.SystemRoot ?? "C:\\Windows"}\\System32\\taskkill.exe`; + const result = spawnSync(taskkill, ["/PID", String(pid), "/T", "/F"], { + stdio: "ignore", + timeout: terminationGraceMs + forceWaitMs, + windowsHide: true, + }); + // A non-zero taskkill result cannot distinguish an already-gone root from a + // partially-killed tree. Recovery must remain disabled in either case. + if (result.status !== 0) return false; + return waitForProcessTreeExit(pid, forceWaitMs, inspect); + } + + // An unreaped child retains its PID. Revalidate that leader identity at the + // signal boundary so a released or replacement-led group is never signaled. + const signalInspection = inspect(pid); + const signalLeaderConfirmed = signalInspection?.hasRunningLeader === true + && isOriginalLeader?.() === true; + const signalDecision = processGroupTerminationDecision(signalInspection, signalLeaderConfirmed); + if (signalDecision === "exited") return true; + if (signalDecision === "refuse") return false; + try { + process.kill(-pid, "SIGTERM"); + } catch (error) { + if (isNoSuchProcess(error)) return true; + return false; + } + if (await waitForProcessTreeExit(pid, terminationGraceMs, inspect)) return true; + + const forceInspection = inspect(pid); + const originalLeaderConfirmed = forceInspection?.hasRunningLeader === true + && isOriginalLeader?.() === true; + const forceDecision = processGroupForceDecision(forceInspection, originalLeaderConfirmed); + if (forceDecision === "exited") return true; + if (forceDecision === "refuse") return false; + + try { + process.kill(-pid, "SIGKILL"); + } catch (error) { + if (isNoSuchProcess(error)) return true; + return false; + } + return waitForProcessTreeExit(pid, forceWaitMs, inspect); +} + +/** + * Run a command in an isolated process tree. A timeout is not reported until the + * entire tree has been terminated or cleanup failure has been made explicit. + */ +export async function runProcessTreeCommand( + bin, + args, + { + timeoutMs, + stdio = "inherit", + windowsHide = true, + shell = false, + env = process.env, + terminationGraceMs = DEFAULT_TERMINATION_GRACE_MS, + forceWaitMs = DEFAULT_FORCE_WAIT_MS, + inspectProcessGroup: inspectOverride, + } = {}, +) { + const inspect = inspectOverride ?? inspectProcessGroup; + let child; + try { + child = spawn(bin, args, { + detached: process.platform !== "win32", + env, + shell, + stdio, + windowsHide, + }); + } catch (error) { + return { + status: null, + signal: null, + error, + interruptedSignal: null, + timedOut: false, + treeExited: true, + }; + } + + let stdout = ""; + let stderr = ""; + child.stdout?.setEncoding?.("utf8"); + child.stderr?.setEncoding?.("utf8"); + child.stdout?.on?.("data", chunk => { stdout = appendBoundedOutput(stdout, chunk); }); + child.stderr?.on?.("data", chunk => { stderr = appendBoundedOutput(stderr, chunk); }); + + let posixTreeAfterRootExit = "unknown"; + let windowsFailedExitTreeUnknown = false; + let spawnFailed = false; + const outcome = new Promise(resolve => { + let settled = false; + child.once("error", error => { + if (settled) return; + settled = true; + spawnFailed = true; + resolve({ status: null, signal: null, error }); + }); + child.once("exit", (status, signal) => { + if (settled) return; + settled = true; + if (process.platform === "win32") { + // A successful package-manager exit is its completion contract. On a + // failed exit, however, a background descendant cannot be ruled out once + // the root PID is gone, so failed-install recovery stays fail-closed. + windowsFailedExitTreeUnknown = status !== 0 || signal !== null; + } else if (child.pid) { + // Inspect group membership directly. A live process whose PID equals the + // old group ID proves reuse, in which case cleanup must not signal it. + posixTreeAfterRootExit = inspectProcessGroupAfterLeaderExit(child.pid, inspect); + } + resolve({ status, signal }); + }); + }); + + let timedOut = false; + let interruptedSignal = null; + let cleanupPromise = null; + let reportCleanupFailure; + const cleanupFailure = new Promise(resolve => { + reportCleanupFailure = resolve; + }); + const startCleanup = () => { + if (cleanupPromise) return; + // If libuv already observed root exit, let the exit handler inspect the old + // group with PID-reuse protection instead of signaling by a freed PID here. + if (child.exitCode !== null || child.signalCode !== null) return; + cleanupPromise = terminateInstallerProcessTree(child.pid, { + terminationGraceMs, + forceWaitMs, + isOriginalLeader: () => child.exitCode === null && child.signalCode === null, + inspectProcessGroup: inspect, + }); + void cleanupPromise.then(treeExited => { + if (treeExited) return; + try { child.kill("SIGKILL"); } catch { /* best-effort root cleanup */ } + reportCleanupFailure({ status: null, signal: null }); + }); + }; + const forwardedSignals = process.platform === "win32" + ? ["SIGINT", "SIGTERM"] + : ["SIGINT", "SIGTERM", "SIGHUP"]; + const signalHandlers = forwardedSignals.map(signal => { + const handler = () => { + interruptedSignal ??= signal; + startCleanup(); + }; + process.on(signal, handler); + return [signal, handler]; + }); + const timer = Number.isFinite(timeoutMs) && timeoutMs > 0 + ? setTimeout(() => { + timedOut = true; + startCleanup(); + }, timeoutMs) + : null; + + const result = await Promise.race([outcome, cleanupFailure]); + if (timer !== null) clearTimeout(timer); + + let treeExited = true; + if (cleanupPromise) { + const knownGroupExited = await cleanupPromise; + // taskkill /T is a retained Windows tree operation. A POSIX process group is + // not containment: a lifecycle child can leave it with setsid/setpgid, so a + // timeout/interruption can never prove complete descendant shutdown. + treeExited = process.platform === "win32" && knownGroupExited; + } else if (spawnFailed) { + treeExited = true; + } else if (process.platform === "win32") { + treeExited = !windowsFailedExitTreeUnknown; + } else { + // A successful package-manager root exit is its completion contract unless + // inspection positively finds a surviving member of the original group. + // Failed/signaled roots remain unconfirmed even when the group looks empty. + treeExited = result.status === 0 + && result.signal === null + && posixTreeAfterRootExit !== "running"; + } + for (const [signal, handler] of signalHandlers) process.removeListener(signal, handler); + + return { + ...result, + signal: interruptedSignal ?? result.signal, + status: timedOut || interruptedSignal !== null ? null : result.status, + interruptedSignal, + timedOut, + treeExited, + stdout, + stderr, + }; +} diff --git a/src/update/job.ts b/src/update/job.ts index 62a01716e..23f79adf1 100644 --- a/src/update/job.ts +++ b/src/update/job.ts @@ -1,11 +1,26 @@ -import { spawn, spawnSync } from "node:child_process"; -import { existsSync, mkdirSync, readFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { spawn, spawnSync, type ChildProcess } from "node:child_process"; +import { existsSync, lstatSync, mkdirSync, readFileSync, readdirSync, realpathSync } from "node:fs"; +import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; -import { atomicWriteFile, getConfigDir, loadConfig, readPid, readRuntimePort } from "../config"; +import { + atomicWriteFile, + getConfigDir, + loadConfig, + readAlivePid, + readPid, + readRuntimePort, + verifyPidIdentityFresh, +} from "../config"; import { killProxy } from "../lib/process-control"; import { reclaimListenPort } from "../server/port-reclaim"; -import { isOpencodexHealthz, probeHostname, proxyIdentityAt, type HealthzIdentity } from "../server/proxy-liveness"; +import { + findLiveProxy, + isOpencodexHealthz, + probeHostname, + proxyIdentityAt, + type HealthzIdentity, + type LiveProxy, +} from "../server/proxy-liveness"; import { isServiceInstalled } from "../service"; import { type Channel, @@ -20,14 +35,24 @@ import { updateCommandStr, } from "./index"; import { isNewer } from "./notify"; +import { + INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE, + runProcessTreeCommand, +} from "./install-process.mjs"; +import { RECOVERY_TREE_SCAN_WORKER_ARG } from "./recovery-tree-scan.mjs"; import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "./tray-update-plan.mjs"; const RELEASE_NOTES_URL = "https://github.com/lidge-jun/opencodex/releases/latest"; const UPDATE_JOB_FILENAME = "update-job.json"; -const UPDATE_TIMEOUT_MS = 180_000; +const UPDATE_TIMEOUT_MS = 360_000; const RESTART_TIMEOUT_MS = 60_000; const RESTART_HEALTH_TIMEOUT_MS = 15_000; const RESTART_STABILITY_WINDOW_MS = 15_000; +const UPDATE_LIVENESS_PROBE_ATTEMPTS = 3; +const UPDATE_LIVENESS_PROBE_DELAY_MS = 250; +const MAX_NPM_RECOVERY_CANDIDATES = 2; +const MAX_NPM_RECOVERY_TREE_ENTRIES = 50_000; +const MAX_NPM_RECOVERY_TREE_SCAN_MS = 5_000; /** How long update restart waits for the captured port to become bindable after stop. */ export const RESTART_PORT_RECLAIM_MS = 30_000; @@ -92,6 +117,173 @@ function packageLauncherPath(): string { return join(dirname(fileURLToPath(import.meta.url)), "..", "..", "bin", "ocx.mjs"); } +interface ValidatedNpmLauncher { + launcher: string; + mtimeMs: number; +} + +type RecoveryLauncherProbe = (launcher: string) => Promise; + +function isPathInside(parent: string, child: string): boolean { + const fromParent = relative(parent, child); + return fromParent !== "" + && fromParent !== ".." + && !fromParent.startsWith(`..${sep}`) + && !isAbsolute(fromParent); +} + +function hasTrustedRecoveryOwner(uid: number): boolean { + const currentUid = process.getuid?.(); + // Root-owned global installations are trusted and otherwise unrecoverable by + // an unprivileged GUI process; reject packages planted by every other account. + return currentUid === undefined || uid === currentUid || uid === 0; +} + +function hasTrustedRecoveryPermissions(stat: { uid: number; mode: number }): boolean { + if (!hasTrustedRecoveryOwner(stat.uid)) return false; + // POSIX group/other write bits make even a trusted-owner entry mutable by a + // different local account. Windows ACLs are not represented by these bits; + // npm failed-root recovery is already disabled there when tree exit is unknown. + return process.getuid === undefined || (stat.mode & 0o022) === 0; +} + +interface RecoveryTreeScanOptions { + scanSpawn?: typeof spawnSync; + scanBin?: string; + scanScript?: string; + timeoutMs?: number; +} + +function hasTrustedRecoveryTree(packageRoot: string, options: RecoveryTreeScanOptions = {}): boolean { + const timeoutMs = Number.isFinite(options.timeoutMs) && options.timeoutMs! > 0 + ? Math.trunc(options.timeoutMs!) + : MAX_NPM_RECOVERY_TREE_SCAN_MS; + const scanSpawn = options.scanSpawn ?? spawnSync; + try { + const result = scanSpawn( + options.scanBin ?? process.execPath, + [ + options.scanScript ?? fileURLToPath(new URL("./recovery-tree-scan.mjs", import.meta.url)), + RECOVERY_TREE_SCAN_WORKER_ARG, + ], + { + encoding: "utf8", + input: JSON.stringify({ + packageRoot, + maxEntries: MAX_NPM_RECOVERY_TREE_ENTRIES, + maxDurationMs: timeoutMs, + }), + timeout: timeoutMs, + killSignal: "SIGKILL", + windowsHide: true, + shell: false, + }, + ); + return result.status === 0 && String(result.stdout ?? "").trim() === "1"; + } catch { + return false; + } +} + +export function scanTrustedRecoveryTreeForTests( + packageRoot: string, + options: RecoveryTreeScanOptions = {}, +): boolean { + return hasTrustedRecoveryTree(packageRoot, options); +} + +/** Validate the on-disk identity before any candidate code is executed. */ +function inspectNpmRecoveryPackage(packageRoot: string, expectedVersion: string): ValidatedNpmLauncher | null { + try { + const rootStat = lstatSync(packageRoot); + if ( + !rootStat.isDirectory() + || rootStat.isSymbolicLink() + || !hasTrustedRecoveryPermissions(rootStat) + ) return null; + if (!hasTrustedRecoveryTree(packageRoot)) return null; + + const canonicalRoot = realpathSync(packageRoot); + const manifestPath = join(canonicalRoot, "package.json"); + const manifestStat = lstatSync(manifestPath); + if ( + !manifestStat.isFile() + || manifestStat.isSymbolicLink() + || !hasTrustedRecoveryPermissions(manifestStat) + ) return null; + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")) as { name?: unknown; version?: unknown }; + if (manifest.name !== PKG || manifest.version !== expectedVersion) return null; + + const launcherPath = join(canonicalRoot, "bin", "ocx.mjs"); + const launcherStat = lstatSync(launcherPath); + if ( + !launcherStat.isFile() + || launcherStat.isSymbolicLink() + || !hasTrustedRecoveryPermissions(launcherStat) + ) return null; + const canonicalLauncher = realpathSync(launcherPath); + if (!isPathInside(canonicalRoot, canonicalLauncher)) return null; + return { launcher: canonicalLauncher, mtimeMs: rootStat.mtimeMs }; + } catch { + return null; + } +} + +async function probeNpmRecoveryLauncher(launcher: string): Promise { + const result = await runProcessTreeCommand(nodeBin(), [launcher, "--version"], { + stdio: "ignore", + timeoutMs: 10_000, + }); + return result.status === 0 && result.treeExited; +} + +/** + * npm retires a package to a hidden sibling such as `.opencodex-Ab12Cd34` + * before replacing it. On a late install failure, prefer an intact current root; + * otherwise recover through the validated retired copy of the pre-update version. + */ +export async function findNpmRecoveryLaunchers( + currentLauncher: string, + expectedVersion: string, + probeLauncher: RecoveryLauncherProbe = probeNpmRecoveryLauncher, +): Promise { + const packageRoot = dirname(dirname(resolve(currentLauncher))); + const scopeRoot = dirname(packageRoot); + const prefix = `.${basename(packageRoot)}-`; + const candidates: ValidatedNpmLauncher[] = []; + const current = inspectNpmRecoveryPackage(packageRoot, expectedVersion); + if (current) candidates.push(current); + try { + const retired = readdirSync(scopeRoot, { encoding: "utf8" }) + .filter(name => name.startsWith(prefix) && /^[A-Za-z0-9]{8}$/.test(name.slice(prefix.length))) + .map(name => inspectNpmRecoveryPackage(join(scopeRoot, name), expectedVersion)) + .filter((candidate): candidate is ValidatedNpmLauncher => candidate !== null) + .sort((a, b) => b.mtimeMs - a.mtimeMs); + candidates.push(...retired); + } catch { + // The current package can still be a valid recovery candidate when the scope + // directory itself cannot be enumerated. + } + + const launchers: string[] = []; + for (const candidate of candidates.slice(0, MAX_NPM_RECOVERY_CANDIDATES)) { + try { + // `--version` loads the launcher's static imports, resolves the bundled Bun + // binary, and imports the complete CLI graph without starting or stopping a proxy. + if (await probeLauncher(candidate.launcher)) launchers.push(candidate.launcher); + } catch { /* an un-runnable candidate is equivalent to a partial package */ } + } + return launchers; +} + +export async function findNpmRecoveryLauncher( + currentLauncher: string, + expectedVersion: string, + probeLauncher?: RecoveryLauncherProbe, +): Promise { + return (await findNpmRecoveryLaunchers(currentLauncher, expectedVersion, probeLauncher))[0] ?? null; +} + function formatCommand(bin: string, args: string[]): string { return `${bin} ${args.join(" ")}`; } @@ -115,7 +307,7 @@ function ensureJobDir(): void { function writeJob(job: UpdateJobState): void { ensureJobDir(); - atomicWriteFile(updateJobPath(), `${JSON.stringify(job, null, 2)}\n`); + atomicWriteFile(updateJobPath(), `${JSON.stringify(sanitizeUpdateJobState(job), null, 2)}\n`); } export function readUpdateJob(jobId?: string | null): UpdateJobState | null { @@ -129,14 +321,32 @@ export function readUpdateJob(jobId?: string | null): UpdateJobState | null { } } +function sanitizeUpdateJobText(value: string): string { + return value + .replace(/(?:[A-Za-z]:\\|\/)[^\s"'<>]*ocx\.mjs\b/g, "ocx.mjs") + .replace(/(?:[A-Za-z]:\\|\/)[^\s"'<>]*(?:\.npm|_cacache|_npx|\.opencodex-)[^\s"'<>]*/g, "[redacted npm path]") + .replace(/\/(?:Users|home)\/[^/\s"'<>]+(?:\/[^\s"'<>]*)*/g, "[redacted user path]") + .replace(/\b(uid|gid)\s*[:=]\s*\d+\b/gi, "$1=[redacted]") + .replace(/\b(UID|GID)\s+\d+\b/g, "$1 [redacted]"); +} + +function sanitizeUpdateJobState(job: UpdateJobState): UpdateJobState { + return { + ...job, + command: sanitizeUpdateJobText(job.command), + log: job.log.map(sanitizeUpdateJobText), + ...(job.error ? { error: sanitizeUpdateJobText(job.error) } : {}), + }; +} + function updateJob(job: UpdateJobState, patch: Partial, logLine?: string): UpdateJobState { const current = readUpdateJob(job.id) ?? job; - const next = { + const next = sanitizeUpdateJobState({ ...current, ...patch, updatedAt: new Date().toISOString(), - log: logLine ? [...current.log, logLine] : current.log, - }; + log: logLine ? [...current.log, sanitizeUpdateJobText(logLine)] : current.log, + }); writeJob(next); return next; } @@ -215,7 +425,7 @@ export function checkForUpdate( installer, updateAvailable, canUpdate: installer !== "source" && updateAvailable, - command, + command: sanitizeUpdateJobText(command), releaseNotesUrl: RELEASE_NOTES_URL, ...(reason ? { reason } : {}), }; @@ -264,7 +474,17 @@ export function startUpdateJob(channel: Channel, restart: boolean): UpdateJobSta return { ...job, pid: child.pid }; } -function runLoggedCommand(job: UpdateJobState, bin: string, args: string[], timeout: number): { status: number | null; signal: NodeJS.Signals | null } { +interface LoggedCommandResult { + status: number | null; + signal: NodeJS.Signals | null; + timedOut: boolean; +} + +interface InstallerCommandResult extends LoggedCommandResult { + treeExited: boolean; +} + +function runLoggedCommand(job: UpdateJobState, bin: string, args: string[], timeout: number): LoggedCommandResult { job = updateJob(job, {}, `$ ${formatCommand(bin, args)}`); const result = spawnSync(bin, args, { encoding: "utf8", @@ -275,21 +495,100 @@ function runLoggedCommand(job: UpdateJobState, bin: string, args: string[], time const stderr = typeof result.stderr === "string" ? result.stderr.trim() : ""; if (stdout) job = updateJob(job, {}, stdout.slice(-4000)); if (stderr) updateJob(job, {}, stderr.slice(-4000)); - return { status: result.status, signal: result.signal }; + return { + status: result.status, + signal: result.signal, + timedOut: (result.error as NodeJS.ErrnoException | undefined)?.code === "ETIMEDOUT", + }; +} + +async function runLoggedProcessTreeCommand( + job: UpdateJobState, + bin: string, + args: string[], + timeout: number, +): Promise { + job = updateJob(job, {}, `$ ${formatCommand(bin, args)}`); + const result = await runProcessTreeCommand(bin, args, { + stdio: "pipe", + timeoutMs: timeout, + windowsHide: true, + shell: false, + }); + const stdout = typeof result.stdout === "string" ? result.stdout.trim() : ""; + const stderr = typeof result.stderr === "string" ? result.stderr.trim() : ""; + if (stdout) job = updateJob(job, {}, stdout.slice(-4000)); + if (stderr) job = updateJob(job, {}, stderr.slice(-4000)); + return { + status: result.status, + signal: result.signal, + timedOut: result.timedOut, + treeExited: result.treeExited, + }; +} + +export function installerFailureAllowsRecovery(installer: Installer, result: InstallerCommandResult): boolean { + return result.treeExited + && !result.timedOut + && result.signal === null + && (installer !== "npm" || result.status !== INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE); } -function spawnDetachedStart(job: UpdateJobState, installer: Installer, port?: number): void { - const cmd = restartCommand(false, installer, packageLauncherPath(), port); +interface StartedRecoveryProcess { + pid: number; + /** True only while the detached ChildProcess handle still represents this start. */ + sameGeneration: () => boolean; +} + +type RestartStartResult = StartedRecoveryProcess | number | null | void; + +function normalizeStartedRecoveryProcess(result: RestartStartResult): StartedRecoveryProcess | null { + if (typeof result === "number") { + if (!Number.isSafeInteger(result) || result <= 0) return null; + // Numeric results are retained for test seams and older callers, but carry no + // generation proof. Recovery therefore fails closed before attempting a kill. + return { pid: result, sameGeneration: () => false }; + } + if (!result || !Number.isSafeInteger(result.pid) || result.pid <= 0) return null; + return typeof result.sameGeneration === "function" ? result : null; +} + +export function formatProxyStartLog(installer: Installer, launcher: string, port?: number): string { + const candidateLabel = launcher === packageLauncherPath() ? "current package" : "validated recovery candidate"; + const portLabel = typeof port === "number" && Number.isFinite(port) && port > 0 + ? ` on port ${Math.trunc(port)}` + : ""; + return `Starting ${installer} proxy from ${candidateLabel}${portLabel}.`; +} + +function spawnDetachedStart( + job: UpdateJobState, + installer: Installer, + port?: number, + launcher = packageLauncherPath(), +): StartedRecoveryProcess | null { + const cmd = restartCommand(false, installer, launcher, port); const env = { ...process.env }; delete env.OCX_SERVICE; - updateJob(job, {}, `$ ${cmd.display}`); - const child = spawn(cmd.bin, cmd.args, { + // Do not persist cmd.display here: a retired npm launcher contains the absolute + // user home path and update-job.json is returned by the management API. + updateJob(job, {}, formatProxyStartLog(installer, launcher, port)); + const child: ChildProcess = spawn(cmd.bin, cmd.args, { detached: true, stdio: "ignore", windowsHide: true, env, }); + const pid = child.pid; + if (typeof pid !== "number" || !Number.isSafeInteger(pid) || pid <= 0) return null; + let exited = false; + child.once("exit", () => { exited = true; }); + child.once("error", () => { exited = true; }); child.unref(); + return { + pid, + sameGeneration: () => !exited && child.exitCode === null && child.signalCode === null, + }; } /** Identity snapshot used to prove an npm self-update actually replaced the pre-update process. */ @@ -298,14 +597,20 @@ export interface RestartProxyIdentity { version?: string; } +export type FailedUpdateRecovery = "not-needed" | "still-running" | "restarted" | "failed"; + /** Test seam: the wait/spawn pair is injectable so the restart path is verifiable. */ export interface RestartIo { waitForPort?: typeof reclaimListenPort; - spawnStart?: (job: UpdateJobState, installer: Installer, port?: number) => void; + spawnStart?: (job: UpdateJobState, installer: Installer, port?: number, launcher?: string) => RestartStartResult; serviceInstalledFn?: () => boolean; + readPidFn?: () => number | null; probeProxy?: (port: number, hostname?: string) => Promise; /** Richer /healthz read for update-correlated restart evidence (pid + version). */ probeProxyIdentity?: (port: number, hostname?: string) => Promise; + verifyPidIdentityFn?: (pid: number) => number | null; + recoveryLaunchersFn?: (currentLauncher: string, expectedVersion: string) => Promise | string[]; + killProxyFn?: (pid: number) => void; sleepMs?: (ms: number) => Promise; now?: () => number; /** Service-mode install/reinstall command (defaults to spawnSync via runLoggedCommand). */ @@ -317,17 +622,23 @@ export interface RestartIo { /** Override the explicit restart path (used by finishGuiUpdateRestart tests). */ restartAfterUpdateFn?: ( job: UpdateJobState, - captured?: { port: number; hostname: string; oldPid?: number }, + captured?: { port: number; hostname: string; oldPid?: number; recoveryLauncher?: string }, io?: RestartIo, - ) => Promise; + ) => Promise; } async function restartAfterUpdate( job: UpdateJobState, - captured?: { port: number; hostname: string; oldPid?: number }, + captured?: { port: number; hostname: string; oldPid?: number; recoveryLauncher?: string }, io: RestartIo = {}, -): Promise { +): Promise { const serviceInstalled = (io.serviceInstalledFn ?? isServiceInstalled)(); + // A failed npm install may leave only a hidden, npm-retired package runnable. + // It can restore availability, but must never be baked into launchd/systemd. + const persistentServiceRestart = serviceInstalled && captured?.recoveryLauncher === undefined; + if (serviceInstalled && !persistentServiceRestart) { + updateJob(job, {}, "Recovery launcher is temporary; starting the proxy directly and leaving the installed service stopped until the package is repaired."); + } const config = loadConfig(); // The stop-first update flow has already cleared pid/runtime state by the time we run, // so the pre-update capture (taken before the update command) is the authoritative @@ -337,14 +648,27 @@ async function restartAfterUpdate( const oldPid = typeof captured?.oldPid === "number" && captured.oldPid > 0 ? captured.oldPid : undefined; + const readCurrentPid = io.readPidFn ?? readPid; + const verifyCurrentPid = io.verifyPidIdentityFn ?? verifyPidIdentityFresh; + const readPidForRestart = (context: string): { pid: number | null; refused: boolean } => { + const rawPid = readCurrentPid(); + const currentPid = rawPid !== null && verifyCurrentPid(rawPid) === rawPid + ? rawPid + : null; + if (currentPid === null || currentPid === oldPid) return { pid: currentPid, refused: false }; + updateJob(job, {}, `A different identity-checked proxy PID appeared ${context}; leaving it untouched.`); + return { pid: null, refused: true }; + }; + if (readPidForRestart("during restart handoff").refused) return null; let svcArgs: string[] | undefined; - if (serviceInstalled) { + if (persistentServiceRestart) { try { const { serviceReinstallArgs } = await import("../service"); svcArgs = serviceReinstallArgs(); } catch { /* fallback to default service install */ } } - const cmd = restartCommand(serviceInstalled, job.installer, packageLauncherPath(), port, svcArgs); + const launcher = captured?.recoveryLauncher ?? packageLauncherPath(); + const cmd = restartCommand(persistentServiceRestart, job.installer, launcher, port, svcArgs); const waitFn = io.waitForPort ?? reclaimListenPort; const reclaimOpts = { timeoutMs: RESTART_PORT_RECLAIM_MS, @@ -354,13 +678,14 @@ async function restartAfterUpdate( onlyKillPids: oldPid != null ? [oldPid] : [], }; - if (serviceInstalled) { + if (persistentServiceRestart) { // Stop-first update already unloaded the service; reclaim the socket (only the // captured old PID when trusted), then reinstall wrappers that bake `--port`. const freed = await waitFn(port, hostname, reclaimOpts); if (!freed) { updateJob(job, {}, `Port ${port} still busy after ${Math.trunc(RESTART_PORT_RECLAIM_MS / 1000)}s; refusing to hop — reinstall may fail until the port is free.`); } + if (readPidForRestart("after service port reclaim").refused) return null; const prevBake = process.env.OCX_BAKE_PORT; process.env.OCX_BAKE_PORT = String(Math.trunc(port)); let serviceOk = false; @@ -381,13 +706,15 @@ async function restartAfterUpdate( if (prevBake === undefined) delete process.env.OCX_BAKE_PORT; else process.env.OCX_BAKE_PORT = prevBake; } - if (serviceOk) return; + if (serviceOk) return null; // Fall through to the direct proxy start below so the update never leaves the // proxy stopped when the service reinstall could not run. } - const pid = readPid(); - if (pid) { + const directPid = readPidForRestart("before direct restart"); + if (directPid.refused) return null; + const pid = directPid.pid; + if (pid !== null) { updateJob(job, {}, `Stopping current proxy PID ${pid}.`); killProxy(pid); } @@ -397,18 +724,21 @@ async function restartAfterUpdate( const freed = await waitFn(port, hostname, reclaimOpts); if (!freed) { updateJob(job, {}, `Port ${port} still busy after ${Math.trunc(RESTART_PORT_RECLAIM_MS / 1000)}s (reclaim could not free the socket); not starting on another port. Retry 'ocx start --port ${port}'.`); - return; + return null; } - (io.spawnStart ?? spawnDetachedStart)(job, job.installer, port); + if (readPidForRestart("after direct port reclaim").refused) return null; + return normalizeStartedRecoveryProcess( + (io.spawnStart ?? spawnDetachedStart)(job, job.installer, port, launcher), + ); } /** Exposed for tests: drives the non-service restart path with injected io. */ export function restartAfterUpdateForTests( job: UpdateJobState, - captured: { port: number; hostname: string; oldPid?: number }, + captured: { port: number; hostname: string; oldPid?: number; recoveryLauncher?: string }, io: RestartIo, -): Promise { - return restartAfterUpdate(job, captured, io); +): Promise { + return restartAfterUpdate(job, captured, io).then(started => started?.pid ?? null); } function restartFailureHint(port: number): string { @@ -524,6 +854,153 @@ async function defaultProbeProxyIdentity( } } +/** Retry identity-aware health checks before considering a recovery restart. */ +async function probeFailedUpdateProxy( + captured: { port: number; hostname: string }, + io: RestartIo, +): Promise { + const probeIdentity = io.probeProxyIdentity ?? defaultProbeProxyIdentity; + const sleep = io.sleepMs ?? (async (ms: number) => { + await new Promise(resolve => setTimeout(resolve, ms)); + }); + for (let attempt = 0; attempt < UPDATE_LIVENESS_PROBE_ATTEMPTS; attempt += 1) { + try { + const identity = await probeIdentity(captured.port, captured.hostname); + if (identity) return identity; + } catch { /* a custom probe failure is equivalent to no health response */ } + if (attempt + 1 < UPDATE_LIVENESS_PROBE_ATTEMPTS) await sleep(UPDATE_LIVENESS_PROBE_DELAY_MS); + } + return null; +} + +export interface UpdateLivenessIo { + findLiveProxyFn?: () => Promise; + sleepMs?: (ms: number) => Promise; + readAlivePidFn?: () => number | null; + verifyPidIdentityFn?: (pid: number) => number | null; + readRuntimePortFn?: (expectedPid?: number) => { pid?: number; port: number; hostname?: string } | null; +} + +/** Retry health first, then retain a PID-verified runtime target as active evidence. */ +export async function findLiveProxyForUpdate(io: UpdateLivenessIo = {}): Promise { + const probe = io.findLiveProxyFn ?? findLiveProxy; + const sleep = io.sleepMs ?? (async (ms: number) => { + await new Promise(resolve => setTimeout(resolve, ms)); + }); + for (let attempt = 0; attempt < UPDATE_LIVENESS_PROBE_ATTEMPTS; attempt += 1) { + try { + const live = await probe(); + if (live) return live; + } catch { /* retry a transient liveness lookup failure */ } + if (attempt + 1 < UPDATE_LIVENESS_PROBE_ATTEMPTS) await sleep(UPDATE_LIVENESS_PROBE_DELAY_MS); + } + + const candidatePid = (io.readAlivePidFn ?? readAlivePid)(); + if (candidatePid === null) return null; + const verifiedPid = (io.verifyPidIdentityFn ?? verifyPidIdentityFresh)(candidatePid); + if (verifiedPid !== candidatePid) return null; + const runtime = (io.readRuntimePortFn ?? readRuntimePort)(candidatePid); + if (!runtime) return null; + return { pid: candidatePid, port: runtime.port, hostname: runtime.hostname }; +} + +/** Keep a failed GUI install from also leaving a previously-active proxy offline. */ +async function recoverFailedGuiUpdate( + job: UpdateJobState, + captured: { port: number; hostname: string; oldPid?: number }, + proxyWasActive: boolean, + io: RestartIo = {}, +): Promise { + if (!proxyWasActive) return "not-needed"; + + if (await probeFailedUpdateProxy(captured, io)) { + updateJob(job, {}, `Update command failed, but the existing proxy remains healthy on ${captured.hostname}:${captured.port}.`); + return "still-running"; + } + + const oldPidIdentityMatches = captured.oldPid != null + && (io.verifyPidIdentityFn ?? verifyPidIdentityFresh)(captured.oldPid) === captured.oldPid; + if (oldPidIdentityMatches) { + updateJob(job, {}, `Update command failed and health probes timed out, but pre-update PID ${captured.oldPid} still identifies as OpenCodex; refusing an automatic restart.`); + return "still-running"; + } + + if (await probeFailedUpdateProxy(captured, io)) { + updateJob(job, {}, `A replacement proxy became healthy on ${captured.hostname}:${captured.port} before recovery; leaving it untouched.`); + return "still-running"; + } + + updateJob(job, {}, "Update command failed after the proxy stopped; attempting to restore the proxy..."); + try { + let recoveryLaunchers: Array = [undefined]; + if (job.installer === "npm") { + const currentLauncher = packageLauncherPath(); + const resolveRecoveryLaunchers = io.recoveryLaunchersFn ?? findNpmRecoveryLaunchers; + recoveryLaunchers = await resolveRecoveryLaunchers(currentLauncher, job.currentVersion); + if (recoveryLaunchers.length === 0) { + updateJob(job, {}, "Could not find a runnable current or npm-retired launcher for the pre-update package."); + updateJob(job, {}, `Automatic proxy recovery failed. Restore the package, then run 'ocx service install' or 'ocx start --port ${captured.port}'.`); + return "failed"; + } + } + + const restartFn = io.restartAfterUpdateFn ?? restartAfterUpdate; + for (let index = 0; index < recoveryLaunchers.length; index += 1) { + const recoveryLauncher = recoveryLaunchers[index]; + const recoveryCaptured = recoveryLauncher === undefined + ? captured + : { ...captured, recoveryLauncher }; + if (index > 0) { + updateJob(job, {}, `Recovery candidate ${index} did not restore the proxy; trying candidate ${index + 1} of ${recoveryLaunchers.length}.`); + } + try { + const started = normalizeStartedRecoveryProcess(await restartFn(job, recoveryCaptured, io)); + const healthy = await awaitRestartedProxyHealthy(job, recoveryCaptured, io); + if (healthy.ok) { + updateJob(job, {}, `Previous proxy availability restored on ${captured.hostname}:${captured.port}; the update itself still failed.`); + return "restarted"; + } + if (started) { + let sameGeneration = false; + try { sameGeneration = started.sameGeneration(); } catch { /* fail closed */ } + if (!sameGeneration) { + updateJob(job, {}, `Recovery candidate ${index + 1} PID ${started.pid} no longer matches the spawned process generation; refusing to kill or try another candidate.`); + break; + } + const verifyStartedPid = io.verifyPidIdentityFn ?? verifyPidIdentityFresh; + if (verifyStartedPid(started.pid) !== started.pid) { + updateJob(job, {}, `Recovery candidate ${index + 1} left PID ${started.pid} without a matching OpenCodex identity; refusing to kill or try another candidate.`); + break; + } + try { + (io.killProxyFn ?? killProxy)(started.pid); + updateJob(job, {}, `Stopped unhealthy recovery candidate ${index + 1} PID ${started.pid}.`); + } catch (error) { + updateJob(job, {}, `Could not stop unhealthy recovery candidate ${index + 1} PID ${started.pid}: ${error instanceof Error ? error.message : String(error)}`); + break; + } + } + } catch { + updateJob(job, {}, `Recovery candidate ${index + 1} failed before health confirmation.`); + } + } + } catch (error) { + updateJob(job, {}, `Automatic proxy recovery threw: ${error instanceof Error ? error.message : String(error)}`); + } + + updateJob(job, {}, `Automatic proxy recovery failed. Restore the package, then run 'ocx service install' or 'ocx start --port ${captured.port}'.`); + return "failed"; +} + +export function recoverFailedGuiUpdateForTests( + job: UpdateJobState, + captured: { port: number; hostname: string; oldPid?: number }, + proxyWasActive: boolean, + io: RestartIo, +): Promise { + return recoverFailedGuiUpdate(job, captured, proxyWasActive, io); +} + /** * Health alone is not enough to skip the GUI worker restart: a surviving pre-update * process is still identity-healthy. Require update-correlated evidence — a new PID @@ -680,20 +1157,18 @@ export async function runGuiUpdateWorker(jobId: string, channel: Channel, restar let job = readUpdateJob(jobId); const check = checkForUpdate(channel); const now = new Date().toISOString(); - // Capture the live listen target BEFORE the update command runs: the stop-first update - // flow clears pid/runtime state, so this is the last moment the real port is knowable. - // Only trust runtime-port.json when its pid matches the live pidfile process. - const rt = readRuntimePort(); - const livePid = readPid(); + // Capture identity-checked liveness BEFORE the update command runs. Installation or + // runtime metadata alone must not turn an intentionally stopped service back on. const preUpdateConfig = loadConfig(); - const runtimeTrusted = !!(rt && livePid && rt.pid === livePid); + const liveBeforeUpdate = await findLiveProxyForUpdate(); + const proxyWasActive = liveBeforeUpdate !== null; const configPort = typeof preUpdateConfig.port === "number" && preUpdateConfig.port > 0 ? preUpdateConfig.port : 10100; const captured = { - port: runtimeTrusted ? rt.port : configPort, - hostname: (runtimeTrusted ? rt.hostname : undefined) ?? preUpdateConfig.hostname ?? "127.0.0.1", - ...(runtimeTrusted && livePid ? { oldPid: livePid } : {}), + port: liveBeforeUpdate?.port ?? configPort, + hostname: liveBeforeUpdate?.hostname ?? preUpdateConfig.hostname ?? "127.0.0.1", + ...(liveBeforeUpdate?.pid ? { oldPid: liveBeforeUpdate.pid } : {}), }; let trayWasInstalled = false; let trayWasRunning = false; @@ -767,19 +1242,27 @@ export async function runGuiUpdateWorker(jobId: string, channel: Channel, restar - 대안 분석: (1) 서버에서 runUpdate 직접 호출: process.exit/stdio/실행 파일 교체 위험. (2) GUI에서 CLI 명령 안내만 제공: 자동 업데이트 UX 부족. (3) 숨은 worker가 Node launcher/Bun 전역 명령을 실행: 상태 추적과 안전한 재시작이 가능. - 선택 근거: 현재 CLI의 npm self-update 우회를 재사용하면서도 GUI 서버 요청 생명주기와 설치 작업을 분리할 수 있어 가장 안정적이다. */ - const result = runLoggedCommand(job, cmd.bin, cmd.args, UPDATE_TIMEOUT_MS); - if (result.status !== 0) { - if (trayWasRunning) { + const result = await runLoggedProcessTreeCommand(job, cmd.bin, cmd.args, UPDATE_TIMEOUT_MS); + if (result.status !== 0 || !result.treeExited) { + const mayRecover = installerFailureAllowsRecovery(check.installer, result); + if (trayWasRunning && mayRecover) { try { const { startWindowsTray } = await import("../tray/windows"); startWindowsTray(); } catch { /* retain the primary update failure */ } } + let recovery: FailedUpdateRecovery | null = null; + if (!mayRecover) { + updateJob(job, {}, `Automatic proxy recovery was skipped because installer process-tree shutdown was not confirmed. Wait for installer processes to exit before restoring the package or proxy.${trayWasRunning ? " The Windows tray also remains stopped; run 'ocx tray start' after the installer processes exit." : ""}`); + } else { + recovery = await recoverFailedGuiUpdate(job, captured, proxyWasActive); + } updateJob(job, { status: "failed", exitCode: result.status, signal: result.signal, error: `update command failed (${result.status ?? "?"})`, + ...(recovery === "restarted" ? { restarted: true } : {}), }); return; } diff --git a/src/update/npm-cache-preflight.d.mts b/src/update/npm-cache-preflight.d.mts new file mode 100644 index 000000000..158624860 --- /dev/null +++ b/src/update/npm-cache-preflight.d.mts @@ -0,0 +1,57 @@ +export interface NpmCacheEntryStat { + uid?: number; + isDirectory(): boolean; + isSymbolicLink(): boolean; +} + +export interface NpmCachePreflightOptions { + platform?: string; + getuid?: () => number; + npmBin?: string; + shell?: boolean; + lookupTimeoutMs?: number; + spawn?: typeof import("node:child_process").spawnSync; + scanSpawn?: typeof import("node:child_process").spawnSync; + scanBin?: string; + scanScript?: string; + access?: (path: string, mode: number) => void; + lstat?: (path: string) => NpmCacheEntryStat; + readdir?: (path: string) => string[]; + realpath?: (path: string) => string; + now?: () => number; + maxEntries?: number; + maxDurationMs?: number; +} + +export type NpmCacheOwnershipIssue = + | { kind: "foreign-owner"; path: string; actualUid: number } + | { kind: "error"; path: string; reason: string }; + +export type NpmCacheOwnershipResult = + | { ok: true; cachePath: string } + | { ok: "skipped"; reason: string } + | { + ok: false; + cachePath?: string; + entryPath?: string; + expectedUid: number; + actualUid?: number; + reason: string; + }; + +export declare function findForeignOwnedNpmCacheEntry( + cachePath: string, + expectedUid: number, + io?: Pick< + NpmCachePreflightOptions, + "access" | "lstat" | "readdir" | "realpath" | "now" | "maxEntries" | "maxDurationMs" + >, +): NpmCacheOwnershipIssue | null; + +export declare function checkNpmCacheOwnership( + options?: NpmCachePreflightOptions, +): NpmCacheOwnershipResult; + +export declare function formatNpmCacheOwnershipFailure( + result: Extract, +): string; diff --git a/src/update/npm-cache-preflight.mjs b/src/update/npm-cache-preflight.mjs new file mode 100644 index 000000000..bfc63fd1b --- /dev/null +++ b/src/update/npm-cache-preflight.mjs @@ -0,0 +1,298 @@ +import { spawnSync } from "node:child_process"; +import { accessSync, constants, lstatSync, readFileSync, readdirSync, realpathSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const DEFAULT_MAX_CACHE_ENTRIES = 50_000; +const DEFAULT_CACHE_SCAN_TIMEOUT_MS = 10_000; +const DEFAULT_CACHE_LOOKUP_TIMEOUT_MS = 12_000; +const CACHE_SCAN_WORKER_ARG = "__scan-npm-cache-ownership"; + +function positiveInteger(value, fallback) { + return Number.isFinite(value) && value > 0 ? Math.trunc(value) : fallback; +} + +/** Extract a stable filesystem error code without serializing the full error. */ +function errorCode(error) { + return error && typeof error === "object" && "code" in error + ? String(error.code) + : "unknown error"; +} + +/** Find the first cache entry whose uid differs from the current user. */ +export function findForeignOwnedNpmCacheEntry(cachePath, expectedUid, io = {}) { + const lstat = io.lstat ?? lstatSync; + const access = io.access ?? accessSync; + const readdir = io.readdir ?? (path => readdirSync(path, { encoding: "utf8" })); + const realpath = io.realpath ?? realpathSync; + const now = io.now ?? (() => Date.now()); + const maxEntries = positiveInteger(io.maxEntries, DEFAULT_MAX_CACHE_ENTRIES); + const maxDurationMs = positiveInteger(io.maxDurationMs, DEFAULT_CACHE_SCAN_TIMEOUT_MS); + const startedAt = now(); + let cacheRoot; + try { + // npm follows a configured cache-root symlink. Resolve only that root; + // nested symlinks are rejected below and never traversed. + cacheRoot = realpath(resolve(cachePath)); + } catch (error) { + if (errorCode(error) === "ENOENT") { + return { + kind: "error", + path: resolve(cachePath), + reason: "npm cache root does not exist", + }; + } + return { + kind: "error", + path: resolve(cachePath), + reason: `could not resolve npm cache root (${errorCode(error)})`, + }; + } + const stack = [cacheRoot]; + let discoveredEntries = 1; + + const elapsedBudgetIssue = path => ( + now() - startedAt > maxDurationMs + ? { + kind: "error", + path, + reason: `npm cache inspection exceeded its ${maxDurationMs}ms time budget`, + } + : null + ); + + while (stack.length > 0) { + const path = stack.pop(); + const beforeStatBudget = elapsedBudgetIssue(path); + if (beforeStatBudget) return beforeStatBudget; + let stat; + try { + stat = lstat(path); + } catch (error) { + if (errorCode(error) === "ENOENT") continue; + return { + kind: "error", + path, + reason: `could not inspect npm cache entry (${errorCode(error)})`, + }; + } + const afterStatBudget = elapsedBudgetIssue(path); + if (afterStatBudget) return afterStatBudget; + + if (Number.isInteger(stat.uid) && stat.uid !== expectedUid) { + return { kind: "foreign-owner", path, actualUid: stat.uid }; + } + if (stat.isSymbolicLink()) { + // Only the configured cache root is canonicalized. Traversing a nested + // link could hide a foreign-owned target, so fail closed instead. + return { + kind: "error", + path, + reason: "npm cache contains a nested symbolic link", + }; + } + if (path === cacheRoot && !stat.isDirectory()) { + return { kind: "error", path, reason: "npm cache root is not a directory" }; + } + try { + const accessMode = stat.isDirectory() + ? constants.R_OK | constants.W_OK | constants.X_OK + : constants.R_OK | constants.W_OK; + access(path, accessMode); + } catch (error) { + return { + kind: "error", + path, + reason: stat.isDirectory() + ? `npm cache directory is not readable, writable, and searchable (${errorCode(error)})` + : `npm cache entry is not readable and writable (${errorCode(error)})`, + }; + } + if (!stat.isDirectory()) continue; + + const beforeReadBudget = elapsedBudgetIssue(path); + if (beforeReadBudget) return beforeReadBudget; + let entries; + try { + entries = readdir(path); + } catch (error) { + return { + kind: "error", + path, + reason: `could not read npm cache directory (${errorCode(error)})`, + }; + } + const afterReadBudget = elapsedBudgetIssue(path); + if (afterReadBudget) return afterReadBudget; + for (const entry of entries) { + const entryPath = resolve(path, entry); + const iterationBudget = elapsedBudgetIssue(entryPath); + if (iterationBudget) return iterationBudget; + if (discoveredEntries >= maxEntries) { + return { + kind: "error", + path: entryPath, + reason: `npm cache inspection exceeded its ${maxEntries}-entry budget`, + }; + } + discoveredEntries += 1; + stack.push(entryPath); + } + } + + return null; +} + +function parseOwnershipIssue(output, cachePath) { + try { + const parsed = JSON.parse(output); + if (parsed === null) return null; + if (!parsed || typeof parsed !== "object" || typeof parsed.path !== "string") throw new Error("invalid issue"); + if (parsed.kind === "foreign-owner" && Number.isInteger(parsed.actualUid)) return parsed; + if (parsed.kind === "error" && typeof parsed.reason === "string") return parsed; + } catch { /* convert malformed worker output into a fail-closed result below */ } + return { + kind: "error", + path: resolve(cachePath), + reason: "npm cache inspection returned invalid output", + }; +} + +/** Run the blocking filesystem walk out-of-process so its wall-clock deadline is enforceable. */ +function scanNpmCacheOwnership(cachePath, expectedUid, options) { + const maxEntries = positiveInteger(options.maxEntries, DEFAULT_MAX_CACHE_ENTRIES); + const maxDurationMs = positiveInteger(options.maxDurationMs, DEFAULT_CACHE_SCAN_TIMEOUT_MS); + const scanSpawn = options.scanSpawn ?? spawnSync; + let result; + try { + result = scanSpawn( + options.scanBin ?? process.execPath, + [options.scanScript ?? fileURLToPath(import.meta.url), CACHE_SCAN_WORKER_ARG], + { + encoding: "utf8", + input: JSON.stringify({ cachePath, expectedUid, maxEntries, maxDurationMs }), + timeout: maxDurationMs, + killSignal: "SIGKILL", + windowsHide: true, + shell: false, + }, + ); + } catch (error) { + return { + kind: "error", + path: resolve(cachePath), + reason: `could not inspect npm cache (${errorCode(error)})`, + }; + } + if (result.status !== 0) { + const timedOut = result.status === null || errorCode(result.error) === "ETIMEDOUT"; + return { + kind: "error", + path: resolve(cachePath), + reason: timedOut + ? `npm cache inspection exceeded its ${maxDurationMs}ms time budget` + : `could not inspect npm cache (${result.error ? errorCode(result.error) : `status ${result.status}`})`, + }; + } + const output = Buffer.isBuffer(result.stdout) ? result.stdout.toString("utf8") : String(result.stdout ?? ""); + return parseOwnershipIssue(output, cachePath); +} + +/** + * npm can abort after it has retired the installed package when its cache contains + * root-owned entries. Detect that state before an updater stops the running proxy. + */ +export function checkNpmCacheOwnership(options = {}) { + const platform = options.platform ?? process.platform; + const getuid = options.getuid ?? process.getuid; + if (platform === "win32" || typeof getuid !== "function") { + return { ok: "skipped", reason: "uid ownership is unavailable on this platform" }; + } + + const expectedUid = getuid(); + const npmBin = options.npmBin ?? "npm"; + const spawn = options.spawn ?? spawnSync; + const lookupTimeoutMs = positiveInteger(options.lookupTimeoutMs, DEFAULT_CACHE_LOOKUP_TIMEOUT_MS); + let result; + try { + result = spawn(npmBin, ["config", "get", "cache"], { + encoding: "utf8", + timeout: lookupTimeoutMs, + killSignal: "SIGKILL", + windowsHide: true, + shell: options.shell ?? false, + }); + } catch (error) { + return { + ok: false, + expectedUid, + reason: `could not resolve the npm cache (${errorCode(error)})`, + }; + } + if (result.status !== 0) { + const detail = result.error ? errorCode(result.error) : `status ${result.status ?? "timeout"}`; + return { + ok: false, + expectedUid, + reason: `could not resolve the npm cache (${detail})`, + }; + } + + const output = Buffer.isBuffer(result.stdout) ? result.stdout.toString("utf8") : String(result.stdout ?? ""); + const cachePath = output.split(/\r?\n/).map(line => line.trim()).filter(Boolean).at(-1); + if (!cachePath || cachePath === "undefined" || cachePath === "null") { + return { ok: false, expectedUid, reason: "npm did not report a cache path" }; + } + + const issue = scanNpmCacheOwnership(cachePath, expectedUid, options); + if (!issue) return { ok: true, cachePath: resolve(cachePath) }; + if (issue.kind === "foreign-owner") { + return { + ok: false, + cachePath: resolve(cachePath), + entryPath: issue.path, + expectedUid, + actualUid: issue.actualUid, + reason: "npm cache entry ownership does not match the current user", + }; + } + return { + ok: false, + cachePath: resolve(cachePath), + entryPath: issue.path, + expectedUid, + reason: issue.reason, + }; +} + +/** Format an actionable update error without persisting account ids or arbitrary local paths. */ +export function formatNpmCacheOwnershipFailure(result) { + return [ + "npm cache ownership pre-flight failed before stopping the proxy.", + result.reason, + "Run 'npm config get cache' to locate the configured cache.", + "Correct the cache ownership or configure a user-owned npm cache, then retry.", + ].join("\n"); +} + +function runCacheScanWorker() { + try { + const payload = JSON.parse(readFileSync(0, "utf8")); + if (typeof payload?.cachePath !== "string" || !Number.isInteger(payload?.expectedUid)) { + process.exitCode = 2; + return; + } + const issue = findForeignOwnedNpmCacheEntry(payload.cachePath, payload.expectedUid, { + maxEntries: payload.maxEntries, + maxDurationMs: payload.maxDurationMs, + }); + process.stdout.write(JSON.stringify(issue)); + } catch { + process.exitCode = 2; + } +} + +const thisModulePath = fileURLToPath(import.meta.url); +if (process.argv[2] === CACHE_SCAN_WORKER_ARG && resolve(process.argv[1] ?? "") === resolve(thisModulePath)) { + runCacheScanWorker(); +} diff --git a/src/update/recovery-tree-scan.d.mts b/src/update/recovery-tree-scan.d.mts new file mode 100644 index 000000000..db784da8b --- /dev/null +++ b/src/update/recovery-tree-scan.d.mts @@ -0,0 +1,13 @@ +export const DEFAULT_MAX_RECOVERY_TREE_ENTRIES: number; +export const DEFAULT_MAX_RECOVERY_TREE_SCAN_MS: number; +export const RECOVERY_TREE_SCAN_WORKER_ARG: string; + +export interface RecoveryTreeScanOptions { + maxEntries?: number; + maxDurationMs?: number; +} + +export function scanTrustedRecoveryTree( + packageRoot: string, + options?: RecoveryTreeScanOptions, +): boolean; diff --git a/src/update/recovery-tree-scan.mjs b/src/update/recovery-tree-scan.mjs new file mode 100644 index 000000000..c8e7659b4 --- /dev/null +++ b/src/update/recovery-tree-scan.mjs @@ -0,0 +1,124 @@ +import { lstatSync, readFileSync, readdirSync, readlinkSync, realpathSync } from "node:fs"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; + +export const DEFAULT_MAX_RECOVERY_TREE_ENTRIES = 50_000; +export const DEFAULT_MAX_RECOVERY_TREE_SCAN_MS = 5_000; +export const RECOVERY_TREE_SCAN_WORKER_ARG = "__scan-recovery-tree"; + +function isPathInside(parent, child) { + const fromParent = relative(parent, child); + return fromParent !== "" + && fromParent !== ".." + && !fromParent.startsWith(`..${sep}`) + && !isAbsolute(fromParent); +} + +function hasTrustedRecoveryOwner(uid) { + const currentUid = process.getuid?.(); + return currentUid === undefined || uid === currentUid || uid === 0; +} + +function hasTrustedRecoveryPermissions(stat) { + if (!hasTrustedRecoveryOwner(stat.uid)) return false; + return process.getuid === undefined || (stat.mode & 0o022) === 0; +} + +function hasTrustedRecoveryPath(packageRoot) { + let path = packageRoot; + while (true) { + const stat = lstatSync(path); + if ( + !stat.isDirectory() + || stat.isSymbolicLink() + || !hasTrustedRecoveryOwner(stat.uid) + || ( + process.getuid !== undefined + && (stat.mode & 0o022) !== 0 + && (stat.mode & 0o1000) === 0 + ) + ) return false; + const parent = dirname(path); + if (parent === path) return true; + path = parent; + } +} + +/** + * Scan a candidate package tree without executing any package code. The caller + * runs this in a short-lived worker so a stalled filesystem syscall is bounded + * by the parent process as well as by the between-entry budget below. + */ +export function scanTrustedRecoveryTree( + packageRoot, + { + maxEntries = DEFAULT_MAX_RECOVERY_TREE_ENTRIES, + maxDurationMs = DEFAULT_MAX_RECOVERY_TREE_SCAN_MS, + } = {}, +) { + try { + const canonicalRoot = realpathSync(packageRoot); + const scopeRoot = dirname(canonicalRoot); + const scopeStat = lstatSync(scopeRoot); + if ( + !scopeStat.isDirectory() + || scopeStat.isSymbolicLink() + || !hasTrustedRecoveryPermissions(scopeStat) + || !hasTrustedRecoveryPath(canonicalRoot) + ) return false; + + const entryLimit = Number.isFinite(maxEntries) && maxEntries > 0 + ? Math.trunc(maxEntries) + : DEFAULT_MAX_RECOVERY_TREE_ENTRIES; + const durationLimit = Number.isFinite(maxDurationMs) && maxDurationMs > 0 + ? Math.trunc(maxDurationMs) + : DEFAULT_MAX_RECOVERY_TREE_SCAN_MS; + const deadline = Date.now() + durationLimit; + const pending = [canonicalRoot]; + const visited = new Set(); + let inspected = 0; + while (pending.length > 0) { + if (Date.now() > deadline || inspected >= entryLimit) return false; + const path = pending.pop(); + inspected += 1; + const entryStat = lstatSync(path); + let canonicalPath = path; + let canonicalStat = entryStat; + if (entryStat.isSymbolicLink()) { + // npm creates node_modules/.bin links. Permit only trusted-owner links + // whose immediate and final targets remain inside this package. + if (!hasTrustedRecoveryOwner(entryStat.uid)) return false; + const directTarget = resolve(dirname(path), readlinkSync(path)); + if (!isPathInside(canonicalRoot, directTarget)) return false; + canonicalPath = realpathSync(path); + if (!isPathInside(canonicalRoot, canonicalPath)) return false; + canonicalStat = lstatSync(canonicalPath); + } + if (!hasTrustedRecoveryPermissions(canonicalStat)) return false; + if (visited.has(canonicalPath)) continue; + visited.add(canonicalPath); + if (canonicalStat.isFile()) continue; + if (!canonicalStat.isDirectory()) return false; + for (const name of readdirSync(canonicalPath, { encoding: "utf8" })) { + pending.push(join(canonicalPath, name)); + } + } + return true; + } catch { + return false; + } +} + +function runWorker() { + try { + const payload = JSON.parse(readFileSync(0, "utf8")); + if (typeof payload?.packageRoot !== "string") { + process.stdout.write("0\n"); + return; + } + process.stdout.write(scanTrustedRecoveryTree(payload.packageRoot, payload) ? "1\n" : "0\n"); + } catch { + process.stdout.write("0\n"); + } +} + +if (process.argv[2] === RECOVERY_TREE_SCAN_WORKER_ARG) runWorker(); diff --git a/tests/config.test.ts b/tests/config.test.ts index 1224c15f4..bbbd0b656 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -13,6 +13,7 @@ import { isValidProviderName, isOcxStartCommandLine, loadConfig, + lookupOcxStartProcessForTests, multiAgentGuidanceEnabled, parsePidFile, positiveIntegerConfigError, @@ -885,6 +886,27 @@ describe("opencodex config defaults", () => { expect(isOcxStartCommandLine("notepad.exe")).toBe(false); }); + test("exposes a fresh process identity path for lifecycle boundaries", () => { + const source = readFileSync(join(import.meta.dir, "..", "src", "config.ts"), "utf8"); + const freshCheck = /export function verifyPidIdentityFresh[\s\S]*?\n}/.exec(source)?.[0] ?? ""; + expect(freshCheck).toContain("isLikelyOcxStartProcessUncached(candidatePid)"); + expect(freshCheck).not.toContain("isLikelyOcxStartProcess(candidatePid)"); + expect(freshCheck).toContain("ocxStartProcessCache.set(candidatePid, isOcx)"); + expect(freshCheck).toContain("ocxStartProcessCache.delete(candidatePid)"); + }); + + test("retries a transient command-line lookup instead of caching unknown as false", () => { + const cache = new Map(); + let reads = 0; + const readCommandLine = () => (++reads === 1 ? undefined : "opencodex start"); + + expect(lookupOcxStartProcessForTests(4242, readCommandLine, cache)).toBeUndefined(); + expect(cache.has(4242)).toBe(false); + expect(lookupOcxStartProcessForTests(4242, readCommandLine, cache)).toBe(true); + expect(reads).toBe(2); + expect(cache.get(4242)).toBe(true); + }); + test("writes pid file as a numeric pid", () => { writePid(process.pid); diff --git a/tests/ocx-launcher-source.test.ts b/tests/ocx-launcher-source.test.ts index 5ecdcccb6..9954d9069 100644 --- a/tests/ocx-launcher-source.test.ts +++ b/tests/ocx-launcher-source.test.ts @@ -10,14 +10,32 @@ const source = readFileSync(join(import.meta.dir, "..", "bin", "ocx.mjs"), "utf8 describe("ocx.mjs npm launcher (source invariants)", () => { test("npm spawns go through a shell on Windows (Node ≥18.20 EINVALs shell-less .cmd spawns)", () => { - const spawnSites = source.match(/spawnSync\(npm,[\s\S]*?\}\)/g) ?? []; - expect(spawnSites.length).toBe(2); - for (const site of spawnSites) { - expect(site).toContain("shell: winShell"); - } + const npmCallSites = [ + ...source.matchAll(/spawnSync\(npm,[\s\S]*?\}\)/g), + ...source.matchAll(/runProcessTreeCommand\(npm,[\s\S]*?\}\)/g), + ].map(match => match[0]); + expect(npmCallSites).toHaveLength(2); + for (const callSite of npmCallSites) expect(callSite).toContain("shell: winShell"); expect(source).toContain('const winShell = process.platform === "win32";'); }); + test("unsafe installer cleanup never restarts the tray, while confirmed interruption does", () => { + const cleanupAt = source.indexOf("if (!res.treeExited)"); + const interruptAt = source.indexOf("if (res.interruptedSignal)"); + const successAt = source.indexOf("if (res.status === 0)"); + expect(cleanupAt).toBeGreaterThan(-1); + expect(interruptAt).toBeGreaterThan(cleanupAt); + expect(successAt).toBeGreaterThan(interruptAt); + const cleanupFailure = source.slice(cleanupAt, interruptAt); + const interruption = source.slice(interruptAt, successAt); + expect(cleanupFailure).not.toContain('runTrayLifecycle(launcher, "start")'); + expect(cleanupFailure).toContain("The proxy is stopped"); + expect(cleanupFailure).toContain("ocx tray start"); + expect(interruption).toContain('runTrayLifecycle(launcher, "start")'); + expect(interruption).toContain("process.exit(exitCode)"); + expect(source).toContain("res.error.message"); + }); + test("--tag is allowlisted before reaching shell-joined spawn args", () => { expect(source).toContain('if (explicit === "preview" || explicit === "latest") return explicit;'); expect(source).not.toMatch(/if \(tagIndex !== -1 && process\.argv\[tagIndex \+ 1\]\) return process\.argv/); diff --git a/tests/update-install-process.test.ts b/tests/update-install-process.test.ts new file mode 100644 index 000000000..8fafa3684 --- /dev/null +++ b/tests/update-install-process.test.ts @@ -0,0 +1,231 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { spawn } from "node:child_process"; +import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + MAX_CAPTURED_OUTPUT_CHARS, + processGroupForceDecision, + runProcessTreeCommand, + terminateInstallerProcessTree, +} from "../src/update/install-process.mjs"; + +const cleanupPids = new Set(); +const cleanupDirs = new Set(); + +function isRunning(pid: number): boolean { + try { + if (process.platform === "linux") { + // Linux keeps a killed orphan as a zombie until its parent (often PID 1) + // reaps it. `kill(pid, 0)` still succeeds for that interval, so inspect + // the proc state before treating the descendant as live. + const stat = readFileSync(`/proc/${pid}/stat`, "utf8"); + const closingParen = stat.lastIndexOf(")"); + if (closingParen >= 0 && stat.slice(closingParen + 2, closingParen + 3) === "Z") { + return false; + } + } + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function processGroupInspector(descendantPidPath: string) { + return (groupId: number) => { + const hasRunningLeader = isRunning(groupId); + let hasRunningDescendant = false; + try { + const descendantPid = Number.parseInt(readFileSync(descendantPidPath, "utf8"), 10); + hasRunningDescendant = Number.isSafeInteger(descendantPid) && isRunning(descendantPid); + } catch { + // The child may not have written its pid before the root exits. + } + return { + hasRunningMember: hasRunningLeader || hasRunningDescendant, + hasRunningLeader, + }; + }; +} + +afterEach(() => { + for (const pid of cleanupPids) { + try { process.kill(pid, "SIGKILL"); } catch { /* already gone */ } + } + cleanupPids.clear(); + for (const dir of cleanupDirs) rmSync(dir, { recursive: true, force: true }); + cleanupDirs.clear(); +}); + +describe("update installer process isolation", () => { + test("force cleanup refuses a reused or uninspectable process group", () => { + expect(processGroupForceDecision(null, false)).toBe("refuse"); + expect(processGroupForceDecision({ hasRunningMember: false, hasRunningLeader: false }, false)).toBe("exited"); + expect(processGroupForceDecision({ hasRunningMember: true, hasRunningLeader: false }, false)).toBe("signal"); + expect(processGroupForceDecision({ hasRunningMember: true, hasRunningLeader: true }, false)).toBe("refuse"); + expect(processGroupForceDecision({ hasRunningMember: true, hasRunningLeader: true }, true)).toBe("signal"); + }); + + test("checks the original leader immediately before SIGTERM", async () => { + if (process.platform === "win32") return; + const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { + detached: true, + stdio: "ignore", + }); + await new Promise((resolve, reject) => { + child.once("spawn", resolve); + child.once("error", reject); + }); + expect(child.pid).toBeDefined(); + cleanupPids.add(child.pid!); + let leaderChecks = 0; + + const treeExited = await terminateInstallerProcessTree(child.pid, { + inspectProcessGroup: () => ({ hasRunningMember: true, hasRunningLeader: true }), + isOriginalLeader: () => { + leaderChecks += 1; + return false; + }, + }); + + expect(treeExited).toBe(false); + expect(leaderChecks).toBe(1); + expect(isRunning(child.pid!)).toBe(true); + }); + + test("accepts a clean POSIX exit when process-group inspection is unavailable", async () => { + if (process.platform === "win32") return; + const result = await runProcessTreeCommand(process.execPath, ["-e", ""], { + stdio: "ignore", + timeoutMs: 1_000, + inspectProcessGroup: () => null, + }); + + expect(result.status).toBe(0); + expect(result.signal).toBeNull(); + expect(result.treeExited).toBe(true); + }); + + test("spawn failures report their cause without inventing a live process tree", async () => { + const result = await runProcessTreeCommand(join(tmpdir(), "ocx-command-that-does-not-exist"), [], { + stdio: "ignore", + timeoutMs: 1_000, + }); + + expect(result.status).toBeNull(); + expect(result.error).toBeInstanceOf(Error); + expect(result.treeExited).toBe(true); + }); + + test("drains and bounds piped installer output", async () => { + const stdoutPayload = "out-".repeat(MAX_CAPTURED_OUTPUT_CHARS); + const stderrPayload = "err-".repeat(MAX_CAPTURED_OUTPUT_CHARS); + const result = await runProcessTreeCommand( + process.execPath, + ["-e", `process.stdout.write(${JSON.stringify(stdoutPayload)}); process.stderr.write(${JSON.stringify(stderrPayload)})`], + { stdio: "pipe", timeoutMs: 1_000 }, + ); + + expect(result.status).toBe(0); + expect(result.stdout).toBe(stdoutPayload.slice(-MAX_CAPTURED_OUTPUT_CHARS)); + expect(result.stderr).toBe(stderrPayload.slice(-MAX_CAPTURED_OUTPUT_CHARS)); + }); + + test("timeout kills and awaits the installer descendant tree", async () => { + const dir = join(tmpdir(), `ocx-installer-tree-${process.pid}-${Date.now()}`); + const fixture = join(dir, "installer-parent.mjs"); + const descendantPidPath = join(dir, "descendant.pid"); + mkdirSync(dir, { recursive: true }); + cleanupDirs.add(dir); + writeFileSync(fixture, [ + 'import { spawn } from "node:child_process";', + 'import { writeFileSync } from "node:fs";', + 'const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" });', + 'writeFileSync(process.argv[2], String(child.pid));', + 'setInterval(() => {}, 1000);', + "", + ].join("\n")); + + const result = await runProcessTreeCommand(process.execPath, [fixture, descendantPidPath], { + forceWaitMs: 2_000, + stdio: "ignore", + terminationGraceMs: 500, + timeoutMs: 3_000, + inspectProcessGroup: processGroupInspector(descendantPidPath), + }); + + expect(existsSync(descendantPidPath)).toBe(true); + const descendantPid = Number.parseInt(readFileSync(descendantPidPath, "utf8"), 10); + const descendantRunning = isRunning(descendantPid); + if (descendantRunning) cleanupPids.add(descendantPid); + expect(result.timedOut).toBe(true); + expect(result.treeExited).toBe(process.platform === "win32"); + expect(descendantRunning).toBe(false); + }, 15_000); + + test("a failed root exit does not signal its remaining leaderless process group", async () => { + if (process.platform === "win32") return; + const dir = join(tmpdir(), `ocx-installer-leaderless-${process.pid}-${Date.now()}`); + const fixture = join(dir, "installer-parent.mjs"); + const descendantPidPath = join(dir, "descendant.pid"); + mkdirSync(dir, { recursive: true }); + cleanupDirs.add(dir); + writeFileSync(fixture, [ + 'import { spawn } from "node:child_process";', + 'import { writeFileSync } from "node:fs";', + 'const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore" });', + 'writeFileSync(process.argv[2], String(child.pid));', + 'setTimeout(() => process.exit(1), 50);', + "", + ].join("\n")); + + const result = await runProcessTreeCommand(process.execPath, [fixture, descendantPidPath], { + forceWaitMs: 2_000, + stdio: "ignore", + terminationGraceMs: 500, + timeoutMs: 5_000, + inspectProcessGroup: processGroupInspector(descendantPidPath), + }); + + expect(result.status).toBe(1); + expect(result.treeExited).toBe(false); + const descendantPid = Number.parseInt(readFileSync(descendantPidPath, "utf8"), 10); + const descendantRunning = isRunning(descendantPid); + if (descendantRunning) cleanupPids.add(descendantPid); + expect(descendantRunning).toBe(true); + }, 15_000); + + test("a failed root exit fails closed when a descendant leaves its process group", async () => { + const dir = join(tmpdir(), `ocx-installer-exit-tree-${process.pid}-${Date.now()}`); + const fixture = join(dir, "installer-parent.mjs"); + const descendantPidPath = join(dir, "descendant.pid"); + mkdirSync(dir, { recursive: true }); + cleanupDirs.add(dir); + writeFileSync(fixture, [ + 'import { spawn } from "node:child_process";', + 'import { writeFileSync } from "node:fs";', + 'const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { detached: true, stdio: "ignore" });', + 'child.unref();', + 'writeFileSync(process.argv[2], String(child.pid));', + 'setTimeout(() => process.exit(1), 50);', + "", + ].join("\n")); + + const result = await runProcessTreeCommand(process.execPath, [fixture, descendantPidPath], { + forceWaitMs: 2_000, + stdio: "ignore", + terminationGraceMs: 500, + timeoutMs: 5_000, + inspectProcessGroup: processGroupInspector(descendantPidPath), + }); + + expect(result.status).toBe(1); + expect(result.timedOut).toBe(false); + const descendantPid = Number.parseInt(readFileSync(descendantPidPath, "utf8"), 10); + const descendantRunning = isRunning(descendantPid); + if (descendantRunning) cleanupPids.add(descendantPid); + expect(result.treeExited).toBe(false); + expect(descendantRunning).toBe(true); + }, 15_000); +}); diff --git a/tests/update-job.test.ts b/tests/update-job.test.ts index 82c85b1b9..8d44c1ca0 100644 --- a/tests/update-job.test.ts +++ b/tests/update-job.test.ts @@ -1,15 +1,22 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { checkForUpdate, confirmRestartAfterUpdateForTests, + findLiveProxyForUpdate, + findNpmRecoveryLauncher, + findNpmRecoveryLaunchers, finishGuiUpdateRestart, + formatProxyStartLog, + installerFailureAllowsRecovery, npmSelfUpdateRestartEvidence, readUpdateJob, + recoverFailedGuiUpdateForTests, restartCommand, restartAfterUpdateForTests, + scanTrustedRecoveryTreeForTests, startUpdateJob, updateExecutionCommand, updateJobPath, @@ -103,6 +110,272 @@ describe("GUI update execution decisions", () => { }); }); + test("persists installer-derived job fields without raw cache paths or uid values", async () => { + const rawPath = "/home/alice/.npm/_cacache/tmp/entry"; + const rawUid = "uid=1001"; + const job: UpdateJobState = { + id: "sanitize-installer-output", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: `node ${rawPath}/bin/ocx.mjs update --tag latest`, + releaseNotesUrl: "", + log: [`npm failed at ${rawPath} ${rawUid}`], + error: `installer stderr ${rawPath} ${rawUid}`, + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, { + serviceInstalledFn: () => false, + readPidFn: () => null, + waitForPort: async () => false, + spawnStart: () => { throw new Error("must not spawn"); }, + }); + const persisted = JSON.stringify(readUpdateJob(job.id)); + expect(persisted).not.toContain("/home/alice"); + expect(persisted).not.toContain("_cacache"); + expect(persisted).not.toContain("uid=1001"); + expect(persisted).not.toContain("alice"); + expect(readUpdateJob(job.id)?.command).toContain("ocx.mjs update --tag latest"); + }); + + test("recovery start logs label candidates without persisting launcher paths", () => { + const retiredLauncher = "/Users/test/.npm-global/lib/node_modules/@bitkyc08/.opencodex-Ab12Cd34/bin/ocx.mjs"; + const line = formatProxyStartLog("npm", retiredLauncher, 10100); + + expect(line).toBe("Starting npm proxy from validated recovery candidate on port 10100."); + expect(line).not.toContain(retiredLauncher); + expect(line).not.toContain("/Users/test/"); + }); + + test("finds a validated npm-retired launcher when the current package is partial", async () => { + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + const retiredRoot = join(scopeRoot, ".opencodex-Ab12Cd34"); + for (const [root, name, version] of [ + [currentRoot, "@bitkyc08/opencodex", "2.7.41"], + [retiredRoot, "@bitkyc08/opencodex", "2.7.40"], + ] as const) { + mkdirSync(join(root, "bin"), { recursive: true }); + writeFileSync(join(root, "package.json"), JSON.stringify({ name, version })); + writeFileSync(join(root, "bin", "ocx.mjs"), "#!/usr/bin/env node\n"); + } + + expect(await findNpmRecoveryLauncher(join(currentRoot, "bin", "ocx.mjs"), "2.7.40")) + .toBe(realpathSync(join(retiredRoot, "bin", "ocx.mjs"))); + }); + + test("rejects an npm-retired launcher with the wrong package identity", async () => { + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + const retiredRoot = join(scopeRoot, ".opencodex-Ab12Cd34"); + mkdirSync(join(retiredRoot, "bin"), { recursive: true }); + writeFileSync(join(retiredRoot, "package.json"), JSON.stringify({ + name: "untrusted-package", + version: "2.7.40", + })); + writeFileSync(join(retiredRoot, "bin", "ocx.mjs"), "#!/usr/bin/env node\n"); + + expect(await findNpmRecoveryLauncher(join(currentRoot, "bin", "ocx.mjs"), "2.7.40")).toBeNull(); + }); + + test("rejects a recovery package with an untrusted-writable imported file", async () => { + if (process.getuid?.() === undefined) return; + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + mkdirSync(join(currentRoot, "bin"), { recursive: true }); + mkdirSync(join(currentRoot, "src"), { recursive: true }); + writeFileSync(join(currentRoot, "package.json"), JSON.stringify({ + name: "@bitkyc08/opencodex", + version: "2.7.40", + })); + writeFileSync(join(currentRoot, "bin", "ocx.mjs"), 'import "../src/runtime.mjs";\n'); + const imported = join(currentRoot, "src", "runtime.mjs"); + writeFileSync(imported, "process.exit(0);\n"); + chmodSync(imported, 0o666); + let probed = false; + + expect(await findNpmRecoveryLaunchers( + join(currentRoot, "bin", "ocx.mjs"), + "2.7.40", + async () => { + probed = true; + return true; + }, + )).toEqual([]); + expect(probed).toBe(false); + }); + + test("rejects a recovery package below an untrusted-writable path component", async () => { + if (process.getuid?.() === undefined) return; + const unsafeParent = join(dir, "world-writable-global"); + const scopeRoot = join(unsafeParent, "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + mkdirSync(join(currentRoot, "bin"), { recursive: true }); + writeFileSync(join(currentRoot, "package.json"), JSON.stringify({ + name: "@bitkyc08/opencodex", + version: "2.7.40", + })); + writeFileSync(join(currentRoot, "bin", "ocx.mjs"), "process.exit(0);\n"); + chmodSync(unsafeParent, 0o777); + let probed = false; + + expect(await findNpmRecoveryLaunchers( + join(currentRoot, "bin", "ocx.mjs"), + "2.7.40", + async () => { + probed = true; + return true; + }, + )).toEqual([]); + expect(probed).toBe(false); + }); + + test("allows npm-generated symlinks that resolve inside a trusted recovery package", async () => { + if (process.platform === "win32") return; + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + const dependencyBin = join(currentRoot, "node_modules", "bun", "bin", "bun.exe"); + const generatedBin = join(currentRoot, "node_modules", ".bin", "bun"); + mkdirSync(join(currentRoot, "bin"), { recursive: true }); + mkdirSync(join(currentRoot, "node_modules", ".bin"), { recursive: true }); + mkdirSync(join(currentRoot, "node_modules", "bun", "bin"), { recursive: true }); + writeFileSync(join(currentRoot, "package.json"), JSON.stringify({ + name: "@bitkyc08/opencodex", + version: "2.7.40", + })); + writeFileSync(join(currentRoot, "bin", "ocx.mjs"), "process.exit(0);\n"); + writeFileSync(dependencyBin, "#!/usr/bin/env node\n"); + symlinkSync("../bun/bin/bun.exe", generatedBin); + + const launcher = realpathSync(join(currentRoot, "bin", "ocx.mjs")); + expect(await findNpmRecoveryLaunchers( + launcher, + "2.7.40", + async candidate => candidate === launcher, + )).toEqual([launcher]); + }); + + test("rejects a recovery package symlink that leaves the candidate tree", async () => { + if (process.platform === "win32") return; + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + const externalBin = join(dir, "external-tool"); + mkdirSync(join(currentRoot, "bin"), { recursive: true }); + mkdirSync(join(currentRoot, "node_modules", ".bin"), { recursive: true }); + writeFileSync(join(currentRoot, "package.json"), JSON.stringify({ + name: "@bitkyc08/opencodex", + version: "2.7.40", + })); + writeFileSync(join(currentRoot, "bin", "ocx.mjs"), "process.exit(0);\n"); + writeFileSync(externalBin, "#!/usr/bin/env node\n"); + symlinkSync(externalBin, join(currentRoot, "node_modules", ".bin", "external-tool")); + let probed = false; + + expect(await findNpmRecoveryLaunchers( + join(currentRoot, "bin", "ocx.mjs"), + "2.7.40", + async () => { + probed = true; + return true; + }, + )).toEqual([]); + expect(probed).toBe(false); + }); + + test("skips a matching current package whose complete launcher runtime cannot load", async () => { + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + const retiredRoot = join(scopeRoot, ".opencodex-Ab12Cd34"); + for (const root of [currentRoot, retiredRoot]) { + mkdirSync(join(root, "bin"), { recursive: true }); + writeFileSync(join(root, "package.json"), JSON.stringify({ + name: "@bitkyc08/opencodex", + version: "2.7.40", + })); + } + writeFileSync(join(currentRoot, "bin", "ocx.mjs"), 'import "../src/cli/index.ts";\n'); + writeFileSync(join(retiredRoot, "bin", "ocx.mjs"), "process.exit(0);\n"); + + expect(await findNpmRecoveryLaunchers(join(currentRoot, "bin", "ocx.mjs"), "2.7.40")) + .toEqual([realpathSync(join(retiredRoot, "bin", "ocx.mjs"))]); + }); + + test("bounds npm recovery candidates before running launcher probes", async () => { + const scopeRoot = join(dir, "global", "@bitkyc08"); + const currentRoot = join(scopeRoot, "opencodex"); + const retiredRoots = [ + join(scopeRoot, ".opencodex-Ab12Cd34"), + join(scopeRoot, ".opencodex-Ef56Gh78"), + join(scopeRoot, ".opencodex-Ij90Kl12"), + ]; + for (const root of [currentRoot, ...retiredRoots]) { + mkdirSync(join(root, "bin"), { recursive: true }); + writeFileSync(join(root, "package.json"), JSON.stringify({ + name: "@bitkyc08/opencodex", + version: "2.7.40", + })); + writeFileSync(join(root, "bin", "ocx.mjs"), "process.exit(0);\n"); + } + const probed: string[] = []; + + const launchers = await findNpmRecoveryLaunchers( + join(currentRoot, "bin", "ocx.mjs"), + "2.7.40", + async launcher => { + probed.push(launcher); + return true; + }, + ); + + expect(probed).toHaveLength(2); + expect(launchers).toEqual(probed); + expect(launchers[0]).toBe(realpathSync(join(currentRoot, "bin", "ocx.mjs"))); + }); + + test("fails closed when the recovery-tree worker exceeds its hard deadline", () => { + const blockingScan = join(dir, "blocking-recovery-tree-scan.mjs"); + writeFileSync(blockingScan, [ + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 60_000);", + "", + ].join("\n")); + + const startedAt = Date.now(); + const result = scanTrustedRecoveryTreeForTests(join(dir, "candidate"), { + scanScript: blockingScan, + timeoutMs: 250, + }); + + expect(result).toBe(false); + expect(Date.now() - startedAt).toBeLessThan(2_000); + }, 5_000); + + test("only recovers after a clean installer exit", () => { + expect(installerFailureAllowsRecovery("npm", { + status: 1, signal: null, timedOut: false, treeExited: true, + })).toBe(true); + expect(installerFailureAllowsRecovery("npm", { + status: 75, signal: null, timedOut: false, treeExited: true, + })).toBe(false); + expect(installerFailureAllowsRecovery("bun", { + status: 75, signal: null, timedOut: false, treeExited: true, + })).toBe(true); + expect(installerFailureAllowsRecovery("bun", { + status: 1, signal: null, timedOut: false, treeExited: false, + })).toBe(false); + expect(installerFailureAllowsRecovery("npm", { + status: null, signal: "SIGTERM", timedOut: false, treeExited: true, + })).toBe(false); + expect(installerFailureAllowsRecovery("npm", { + status: 1, signal: null, timedOut: true, treeExited: true, + })).toBe(false); + }); + test("proxy restart pins --port so post-update start does not hop to an ephemeral port", () => { const proxy = restartCommand(false, "npm", "/pkg/bin/ocx.mjs", 10100); expect(proxy.mode).toBe("proxy"); @@ -118,7 +391,7 @@ describe("GUI update execution decisions", () => { // The stop-first update flow clears pid/runtime state before restartAfterUpdate runs, // so the wait must fire even with no readable pid — driven here via the io seam. const waited: Array<{ port: number; hostname: string; opts?: { killOcxHolders?: boolean; onlyKillPids?: number[] } }> = []; - const spawned: Array<{ port?: number }> = []; + const spawned: Array<{ port?: number; launcher?: string }> = []; const job: UpdateJobState = { id: "restart-io", status: "restarting", @@ -132,8 +405,12 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); - await restartAfterUpdateForTests(job, { port: 12345, hostname: "127.0.0.1" }, { + writeFileSync(updateJobPath(), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { + port: 12345, + hostname: "127.0.0.1", + recoveryLauncher: "/retired/bin/ocx.mjs", + }, { serviceInstalledFn: () => false, // drive the proxy-mode branch regardless of host state waitForPort: async (port, hostname, opts) => { waited.push({ @@ -146,8 +423,8 @@ describe("GUI update execution decisions", () => { }); return true; }, - spawnStart: (_job, _installer, port) => { - spawned.push({ port }); + spawnStart: (_job, _installer, port, launcher) => { + spawned.push({ port, launcher }); }, }); expect(waited).toEqual([{ @@ -155,7 +432,7 @@ describe("GUI update execution decisions", () => { hostname: "127.0.0.1", opts: { killOcxHolders: false, onlyKillPids: [] }, }]); - expect(spawned).toEqual([{ port: 12345 }]); + expect(spawned).toEqual([{ port: 12345, launcher: "/retired/bin/ocx.mjs" }]); }); test("restart reclaim allowlists only the trusted oldPid", async () => { @@ -173,7 +450,7 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1", oldPid: 4242 }, { serviceInstalledFn: () => false, waitForPort: async (_port, _hostname, opts) => { @@ -188,6 +465,117 @@ describe("GUI update execution decisions", () => { expect(optsSeen).toEqual([{ killOcxHolders: true, onlyKillPids: [4242] }]); }); + test("service restart leaves a replacement PID untouched when it appears during port reclaim", async () => { + let pidReads = 0; + const job: UpdateJobState = { + id: "restart-replacement-pid", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { + port: 10100, + hostname: "127.0.0.1", + oldPid: 111, + recoveryLauncher: "/retired/bin/ocx.mjs", + }, { + serviceInstalledFn: () => true, + readPidFn: () => (++pidReads === 1 ? 111 : 222), + verifyPidIdentityFn: pid => pid, + waitForPort: async () => true, + runService: () => { throw new Error("must not reinstall over a replacement PID"); }, + spawnStart: () => { throw new Error("must not start over a replacement PID"); }, + }); + expect(pidReads).toBe(2); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("different identity-checked proxy PID") && line.includes("leaving it untouched"), + )).toBe(true); + }); + + test("direct restart leaves a replacement PID untouched when it appears during port reclaim", async () => { + let pidReads = 0; + const job: UpdateJobState = { + id: "restart-direct-replacement-pid", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { + port: 10100, + hostname: "127.0.0.1", + oldPid: 111, + recoveryLauncher: "/retired/bin/ocx.mjs", + }, { + serviceInstalledFn: () => false, + readPidFn: () => (++pidReads < 3 ? null : 222), + verifyPidIdentityFn: pid => pid, + waitForPort: async () => true, + spawnStart: () => { throw new Error("must not start over a replacement PID"); }, + }); + expect(pidReads).toBe(3); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("different identity-checked proxy PID") && line.includes("leaving it untouched"), + )).toBe(true); + }); + + test("direct restart treats an unverified pidfile PID as absent", async () => { + const verified: number[] = []; + let spawned = 0; + const job: UpdateJobState = { + id: "restart-unverified-pid", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + await restartAfterUpdateForTests(job, { + port: 10100, + hostname: "127.0.0.1", + oldPid: 111, + recoveryLauncher: "/retired/bin/ocx.mjs", + }, { + serviceInstalledFn: () => false, + readPidFn: () => 222, + verifyPidIdentityFn: pid => { + verified.push(pid); + return null; + }, + waitForPort: async () => true, + spawnStart: () => { spawned += 1; }, + }); + expect(verified).toEqual([222, 222, 222]); + expect(spawned).toBe(1); + const log = readUpdateJob(job.id)?.log ?? []; + expect(log.some(line => line.includes("Stopping current proxy PID"))).toBe(false); + expect(log.some(line => line.includes("different identity-checked proxy PID"))).toBe(false); + }); + test("restart refuses to spawn when the captured port never becomes free", async () => { const spawned: Array<{ port?: number }> = []; const job: UpdateJobState = { @@ -203,7 +591,7 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); await restartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, { serviceInstalledFn: () => false, waitForPort: async () => false, @@ -219,6 +607,7 @@ describe("GUI update execution decisions", () => { test("service restart waits on the captured port and clears OCX_BAKE_PORT after install", async () => { const waited: Array<{ port: number; hostname: string }> = []; const bakeDuringInstall: string[] = []; + const launchersDuringInstall: string[] = []; const job: UpdateJobState = { id: "restart-svc", status: "restarting", @@ -232,24 +621,30 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const prev = process.env.OCX_BAKE_PORT; delete process.env.OCX_BAKE_PORT; try { - await restartAfterUpdateForTests(job, { port: 18765, hostname: "127.0.0.1" }, { + await restartAfterUpdateForTests(job, { + port: 18765, + hostname: "127.0.0.1", + }, { serviceInstalledFn: () => true, waitForPort: async (port, hostname) => { waited.push({ port, hostname: hostname ?? "" }); expect(process.env.OCX_BAKE_PORT).toBeUndefined(); return true; }, - runService: () => { + runService: (_job, _bin, args) => { bakeDuringInstall.push(process.env.OCX_BAKE_PORT ?? ""); + launchersDuringInstall.push(args[0] ?? ""); return { status: 0 }; }, }); expect(waited).toEqual([{ port: 18765, hostname: "127.0.0.1" }]); expect(bakeDuringInstall).toEqual(["18765"]); + expect(launchersDuringInstall).toHaveLength(1); + expect(launchersDuringInstall[0]?.endsWith(join("bin", "ocx.mjs"))).toBe(true); expect(process.env.OCX_BAKE_PORT).toBeUndefined(); } finally { if (prev === undefined) delete process.env.OCX_BAKE_PORT; @@ -257,6 +652,48 @@ describe("GUI update execution decisions", () => { } }); + test("retired recovery launchers never persist their path in an installed service", async () => { + const spawned: Array<{ port?: number; launcher?: string }> = []; + const job: UpdateJobState = { + id: "restart-retired-direct", + status: "restarting", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + + const startedPid = await restartAfterUpdateForTests(job, { + port: 10100, + hostname: "127.0.0.1", + recoveryLauncher: "/scope/.opencodex-Ab12Cd34/bin/ocx.mjs", + }, { + serviceInstalledFn: () => true, + waitForPort: async () => true, + runService: () => { throw new Error("must not persist a temporary recovery path"); }, + spawnStart: (_job, _installer, port, launcher) => { + spawned.push({ port, launcher }); + return 222; + }, + }); + + expect(startedPid).toBe(222); + expect(spawned).toEqual([{ + port: 10100, + launcher: "/scope/.opencodex-Ab12Cd34/bin/ocx.mjs", + }]); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("temporary") && line.includes("installed service stopped"), + )).toBe(true); + }); + test("service reinstall failure falls back to a direct proxy start", async () => { const spawned: Array<{ port: number }> = []; const job: UpdateJobState = { @@ -272,7 +709,7 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); await restartAfterUpdateForTests(job, { port: 19999, hostname: "127.0.0.1" }, { serviceInstalledFn: () => true, waitForPort: async () => true, @@ -285,6 +722,438 @@ describe("GUI update execution decisions", () => { expect(spawned).toEqual([{ port: 19999 }]); }); + test("pre-update liveness retries a transient miss before classifying the proxy inactive", async () => { + let probes = 0; + const delays: number[] = []; + const live = await findLiveProxyForUpdate({ + findLiveProxyFn: async () => ( + ++probes === 1 ? null : { pid: 111, port: 15432, hostname: "127.0.0.1" } + ), + sleepMs: async ms => { delays.push(ms); }, + }); + expect(live).toEqual({ pid: 111, port: 15432, hostname: "127.0.0.1" }); + expect(probes).toBe(2); + expect(delays).toEqual([250]); + }); + + test("pre-update liveness retains a PID-verified runtime target after health misses", async () => { + let probes = 0; + const live = await findLiveProxyForUpdate({ + findLiveProxyFn: async () => { + probes += 1; + return null; + }, + sleepMs: async () => {}, + readAlivePidFn: () => 111, + verifyPidIdentityFn: pid => pid, + readRuntimePortFn: expectedPid => ( + expectedPid === 111 ? { pid: 111, port: 16543, hostname: "127.0.0.1" } : null + ), + }); + expect(probes).toBe(3); + expect(live).toEqual({ pid: 111, port: 16543, hostname: "127.0.0.1" }); + }); + + test("failed install leaves an already-healthy proxy untouched", async () => { + let restartCalls = 0; + const job: UpdateJobState = { + id: "failed-install-still-running", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => ({ pid: 111, version: "2.7.40" }), + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }, + ); + expect(recovery).toBe("still-running"); + expect(restartCalls).toBe(0); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("existing proxy remains healthy"))).toBe(true); + }); + + test("failed install does not start a proxy that was inactive before the update", async () => { + let restartCalls = 0; + const job: UpdateJobState = { + id: "failed-install-inactive", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1" }, + false, + { + probeProxyIdentity: async () => { throw new Error("must not probe an inactive proxy"); }, + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }, + ); + expect(recovery).toBe("not-needed"); + expect(restartCalls).toBe(0); + }); + + test("failed install retries a transient health miss before considering restart", async () => { + let probes = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "failed-install-transient-probe", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => (++probes === 1 ? null : { pid: 111, version: "2.7.40" }), + sleepMs: async () => {}, + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }, + ); + expect(recovery).toBe("still-running"); + expect(probes).toBe(2); + expect(restartCalls).toBe(0); + }); + + test("failed install preserves a captured PID only while it still identifies as OpenCodex", async () => { + let restartCalls = 0; + const job: UpdateJobState = { + id: "failed-install-live-pid", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => null, + verifyPidIdentityFn: pid => pid, + sleepMs: async () => {}, + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }, + ); + expect(recovery).toBe("still-running"); + expect(restartCalls).toBe(0); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("refusing an automatic restart"))).toBe(true); + }); + + test("failed install leaves a concurrently restored replacement proxy untouched", async () => { + let probes = 0; + let restartCalls = 0; + const job: UpdateJobState = { + id: "failed-install-concurrent-replacement", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => ( + ++probes <= 3 ? null : { pid: 222, version: "2.7.40" } + ), + verifyPidIdentityFn: () => null, + sleepMs: async () => {}, + recoveryLaunchersFn: () => { throw new Error("must not resolve a launcher"); }, + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }, + ); + expect(recovery).toBe("still-running"); + expect(probes).toBe(4); + expect(restartCalls).toBe(0); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("replacement proxy became healthy"))).toBe(true); + }); + + test("failed install restores through the retired launcher after the old PID loses identity", async () => { + let now = 0; + let restarted = false; + let recoveryLauncher: string | undefined; + const job: UpdateJobState = { + id: "failed-install-recovery", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => null, + verifyPidIdentityFn: () => null, + recoveryLaunchersFn: () => ["/retired/bin/ocx.mjs"], + probeProxy: async () => restarted, + restartAfterUpdateFn: async (_job, captured) => { + recoveryLauncher = captured?.recoveryLauncher; + restarted = true; + }, + now: () => now, + sleepMs: async (ms) => { now += ms; }, + }, + ); + expect(recovery).toBe("restarted"); + expect(recoveryLauncher).toBe("/retired/bin/ocx.mjs"); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("update itself still failed"))).toBe(true); + }); + + test("failed install stops an unhealthy recovery process before trying the next package", async () => { + let now = 0; + let activePid: number | null = null; + const attempted: Array = []; + const killed: number[] = []; + const runningPids = new Set(); + const job: UpdateJobState = { + id: "failed-install-recovery-fallback", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => null, + serviceInstalledFn: () => false, + readPidFn: () => activePid, + verifyPidIdentityFn: pid => runningPids.has(pid) ? pid : null, + recoveryLaunchersFn: () => ["/current/bin/ocx.mjs", "/retired/bin/ocx.mjs"], + probeProxy: async () => activePid === 333 && runningPids.has(333), + waitForPort: async () => true, + spawnStart: (_job, _installer, _port, launcher) => { + attempted.push(launcher); + activePid = launcher === "/current/bin/ocx.mjs" ? 222 : 333; + runningPids.add(activePid); + const startedPid = activePid; + return { + pid: startedPid, + sameGeneration: () => runningPids.has(startedPid), + }; + }, + killProxyFn: pid => { + killed.push(pid); + runningPids.delete(pid); + if (activePid === pid) activePid = null; + }, + now: () => now, + sleepMs: async (ms) => { now += ms; }, + }, + ); + + expect(recovery).toBe("restarted"); + expect(attempted).toEqual(["/current/bin/ocx.mjs", "/retired/bin/ocx.mjs"]); + expect(killed).toEqual([222]); + expect(runningPids.has(222)).toBe(false); + expect(runningPids.has(333)).toBe(true); + expect(readUpdateJob(job.id)?.log.some(line => line.includes("trying candidate 2 of 2"))).toBe(true); + }); + + test("failed install stops trying candidates when a started PID has no OpenCodex identity", async () => { + let now = 0; + const attempted: Array = []; + const killed: number[] = []; + const job: UpdateJobState = { + id: "failed-install-unverified-pid", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => null, + verifyPidIdentityFn: () => null, + recoveryLaunchersFn: () => ["/current/bin/ocx.mjs", "/retired/bin/ocx.mjs"], + probeProxy: async () => false, + restartAfterUpdateFn: async (_job, captured) => { + attempted.push(captured?.recoveryLauncher); + return { pid: 222, sameGeneration: () => true }; + }, + killProxyFn: pid => { killed.push(pid); }, + now: () => now, + sleepMs: async (ms) => { now += ms; }, + }, + ); + + expect(recovery).toBe("failed"); + expect(attempted).toEqual(["/current/bin/ocx.mjs"]); + expect(killed).toEqual([]); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("without a matching OpenCodex identity"), + )).toBe(true); + }); + + test("failed install never kills a reused PID after the spawned process generation exits", async () => { + let now = 0; + const attempted: Array = []; + const killed: number[] = []; + const job: UpdateJobState = { + id: "failed-install-reused-generation", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => null, + verifyPidIdentityFn: pid => pid === 222 ? pid : null, + recoveryLaunchersFn: () => ["/current/bin/ocx.mjs", "/retired/bin/ocx.mjs"], + probeProxy: async () => false, + restartAfterUpdateFn: async (_job, captured) => { + attempted.push(captured?.recoveryLauncher); + return { pid: 222, sameGeneration: () => false }; + }, + killProxyFn: pid => { killed.push(pid); }, + now: () => now, + sleepMs: async (ms) => { now += ms; }, + }, + ); + + expect(recovery).toBe("failed"); + expect(attempted).toEqual(["/current/bin/ocx.mjs"]); + expect(killed).toEqual([]); + expect(readUpdateJob(job.id)?.log.some(line => + line.includes("no longer matches the spawned process generation"), + )).toBe(true); + }); + + test("failed install reports remediation when no runnable recovery package remains", async () => { + let restartCalls = 0; + const job: UpdateJobState = { + id: "failed-install-no-candidate", + status: "running", + startedAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + currentVersion: "2.7.40", + latestVersion: "2.7.41", + channel: "latest", + installer: "npm", + restart: true, + command: "", + releaseNotesUrl: "", + log: [], + }; + writeFileSync(updateJobPath(), JSON.stringify(job)); + + const recovery = await recoverFailedGuiUpdateForTests( + job, + { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, + true, + { + probeProxyIdentity: async () => null, + verifyPidIdentityFn: () => null, + sleepMs: async () => {}, + recoveryLaunchersFn: () => [], + restartAfterUpdateFn: async () => { restartCalls += 1; }, + }, + ); + + expect(recovery).toBe("failed"); + expect(restartCalls).toBe(0); + const log = readUpdateJob(job.id)?.log ?? []; + expect(log.some(line => line.includes("Could not find a runnable current or npm-retired launcher"))).toBe(true); + expect(log.some(line => line.includes("ocx start --port 10100"))).toBe(true); + }); + test("restart confirmation fails when the proxy never becomes healthy", async () => { let now = 0; const job: UpdateJobState = { @@ -300,7 +1169,7 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await confirmRestartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, { probeProxy: async () => false, now: () => now, @@ -329,7 +1198,7 @@ describe("GUI update execution decisions", () => { command: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await confirmRestartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, { probeProxy: async () => now < 12_000, now: () => now, @@ -359,7 +1228,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await confirmRestartAfterUpdateForTests(job, { port: 10100, hostname: "127.0.0.1" }, { probeProxy: async () => now >= 1_000, now: () => now, @@ -386,7 +1255,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart( job, { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, @@ -424,7 +1293,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart( job, { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, @@ -473,7 +1342,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart( job, { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, @@ -516,7 +1385,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart( job, { port: 10100, hostname: "127.0.0.1", oldPid: 111 }, @@ -563,7 +1432,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart(job, { port: 10100, hostname: "127.0.0.1" }, "npm", { serviceInstalledFn: () => false, probeProxy: async () => { @@ -606,7 +1475,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart(job, { port: 10100, hostname: "127.0.0.1" }, "npm", { serviceInstalledFn: () => true, // Soft probe times out (proxy down after npm update); confirm after explicit restart succeeds. @@ -645,7 +1514,7 @@ describe("GUI update execution decisions", () => { releaseNotesUrl: "", log: [], }; - writeFileSync(updateJobPath(job.id), JSON.stringify(job)); + writeFileSync(updateJobPath(), JSON.stringify(job)); const ok = await finishGuiUpdateRestart(job, { port: 10100, hostname: "127.0.0.1" }, "bun", { probeProxy: async () => true, now: () => now, @@ -751,7 +1620,7 @@ describe("immutable update target (WP160)", () => { const gateAt = source.indexOf("const integrity = checkUpdatePackageIntegrity(check.latestVersion);"); const failAt = source.indexOf('updateJob(job, { status: "failed", error: integrity.reason });'); - const spawnAt = source.indexOf("const result = runLoggedCommand(job, cmd.bin, cmd.args, UPDATE_TIMEOUT_MS);"); + const spawnAt = source.indexOf("const result = await runLoggedProcessTreeCommand(job, cmd.bin, cmd.args, UPDATE_TIMEOUT_MS);"); expect(gateAt).toBeGreaterThan(-1); expect(failAt).toBeGreaterThan(-1); expect(spawnAt).toBeGreaterThan(-1); diff --git a/tests/update-npm-cache-preflight.test.ts b/tests/update-npm-cache-preflight.test.ts new file mode 100644 index 000000000..7c2ae83e0 --- /dev/null +++ b/tests/update-npm-cache-preflight.test.ts @@ -0,0 +1,313 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { chmodSync, constants, lstatSync, mkdirSync, readdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { homedir, tmpdir } from "node:os"; +import { join } from "node:path"; +import { + checkNpmCacheOwnership, + findForeignOwnedNpmCacheEntry, + formatNpmCacheOwnershipFailure, +} from "../src/update/npm-cache-preflight.mjs"; + +let dir: string; +let extraPaths: string[]; + +beforeEach(() => { + dir = join(tmpdir(), `ocx-npm-cache-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`); + mkdirSync(join(dir, "_cacache", "index-v5"), { recursive: true }); + writeFileSync(join(dir, "_cacache", "index-v5", "entry"), "cache"); + extraPaths = []; +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + for (const path of extraPaths) rmSync(path, { recursive: true, force: true }); +}); + +function cacheLookup(path: string): typeof import("node:child_process").spawnSync { + return (() => ({ + status: 0, + stdout: `${path}\n`, + stderr: "", + pid: 1, + output: [], + signal: null, + })) as never; +} + +describe("npm cache ownership pre-flight", () => { + test("accepts a cache owned by the current uid", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + expect(checkNpmCacheOwnership({ + getuid: () => uid, + spawn: cacheLookup(dir), + })).toEqual({ ok: true, cachePath: dir }); + }); + + test("finds a foreign-owned nested entry before package replacement", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + const foreign = join(realpathSync(dir), "_cacache", "index-v5", "entry"); + const issue = findForeignOwnedNpmCacheEntry(dir, uid, { + lstat: (path) => { + const stat = lstatSync(path); + return { + uid: path === foreign ? uid + 1 : stat.uid, + isDirectory: () => stat.isDirectory(), + isSymbolicLink: () => stat.isSymbolicLink(), + }; + }, + readdir: path => readdirSync(path, { encoding: "utf8" }), + }); + expect(issue).toEqual({ kind: "foreign-owner", path: foreign, actualUid: uid + 1 }); + }); + + test("follows a configured cache-root symlink but rejects nested symlinks", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + const cacheLink = `${dir}-link`; + const outside = `${dir}-outside`; + extraPaths.push(cacheLink, outside); + mkdirSync(outside, { recursive: true }); + writeFileSync(join(outside, "foreign"), "outside"); + symlinkSync(dir, cacheLink, "dir"); + symlinkSync(outside, join(dir, "nested-link"), "dir"); + const outsideRoot = realpathSync(outside); + let inspectedOutside = false; + const issue = findForeignOwnedNpmCacheEntry(cacheLink, uid, { + lstat: (path) => { + if (path.startsWith(outsideRoot)) inspectedOutside = true; + const stat = lstatSync(path); + return { + uid: stat.uid, + isDirectory: () => stat.isDirectory(), + isSymbolicLink: () => stat.isSymbolicLink(), + }; + }, + readdir: path => readdirSync(path, { encoding: "utf8" }), + }); + expect(issue).toEqual({ + kind: "error", + path: join(realpathSync(dir), "nested-link"), + reason: "npm cache contains a nested symbolic link", + }); + expect(inspectedOutside).toBe(false); + }); + + test("returns an actionable failure without changing the cache", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + const result = checkNpmCacheOwnership({ + getuid: () => uid + 1, + spawn: cacheLookup(dir), + }); + expect(result).toMatchObject({ + ok: false, + cachePath: dir, + entryPath: realpathSync(dir), + expectedUid: uid + 1, + actualUid: uid, + reason: "npm cache entry ownership does not match the current user", + }); + if (result.ok !== false) throw new Error("expected ownership failure"); + expect(formatNpmCacheOwnershipFailure(result)).toContain("before stopping the proxy"); + expect(formatNpmCacheOwnershipFailure(result)).toContain("configure a user-owned npm cache"); + }); + + test("fails closed when the cache is owned by the user but not effectively writable", () => { + const uid = process.getuid?.() ?? 501; + const cacheRoot = realpathSync(dir); + const accessDenied = Object.assign(new Error("permission denied"), { code: "EACCES" }); + const issue = findForeignOwnedNpmCacheEntry(dir, uid, { + lstat: (path) => { + const stat = lstatSync(path); + return { + uid, + isDirectory: () => stat.isDirectory(), + isSymbolicLink: () => stat.isSymbolicLink(), + }; + }, + readdir: path => readdirSync(path, { encoding: "utf8" }), + access: (path, mode) => { + if (path === cacheRoot && (mode & constants.W_OK) !== 0) throw accessDenied; + }, + }); + expect(issue).toEqual({ + kind: "error", + path: cacheRoot, + reason: "npm cache directory is not readable, writable, and searchable (EACCES)", + }); + + const result = checkNpmCacheOwnership({ + platform: "linux", + getuid: () => uid, + spawn: cacheLookup(dir), + scanSpawn: (() => ({ + status: 0, + stdout: `${JSON.stringify(issue)}\n`, + stderr: "", + pid: 1, + output: [], + signal: null, + })) as never, + }); + expect(result).toMatchObject({ + ok: false, + cachePath: dir, + entryPath: cacheRoot, + expectedUid: uid, + reason: "npm cache directory is not readable, writable, and searchable (EACCES)", + }); + if (result.ok !== false) throw new Error("expected access failure"); + expect(formatNpmCacheOwnershipFailure(result)).toContain("before stopping the proxy"); + }); + + test("formats cache failures without persisting arbitrary path segments or account ids", () => { + const cachePath = join(homedir(), "customer-alice-cache"); + const entryPath = join(cachePath, "_npx", "node_modules", "@alice"); + const output = formatNpmCacheOwnershipFailure({ + ok: false, + cachePath, + entryPath, + expectedUid: 502, + actualUid: 0, + reason: "npm cache entry ownership does not match the current user", + }); + expect(output).toContain("npm config get cache"); + expect(output).not.toContain(homedir()); + expect(output).not.toContain("customer-alice-cache"); + expect(output).not.toContain("@alice"); + expect(output).not.toContain("_npx"); + expect(output).not.toContain("502"); + expect(output).not.toMatch(/\buid\b/i); + }); + + test("fails closed when the configured cache root does not exist", () => { + const missing = `${dir}-missing`; + const result = checkNpmCacheOwnership({ + platform: "linux", + getuid: () => 501, + spawn: cacheLookup(missing), + }); + expect(result).toMatchObject({ + ok: false, + cachePath: missing, + entryPath: missing, + reason: "npm cache root does not exist", + }); + }); + + test("fails closed when the cache exceeds the entry budget", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + const issue = findForeignOwnedNpmCacheEntry(dir, uid, { maxEntries: 2 }); + expect(issue).toMatchObject({ + kind: "error", + reason: "npm cache inspection exceeded its 2-entry budget", + }); + }); + + test("fails closed when the cache exceeds the elapsed-time budget", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + let now = 0; + const issue = findForeignOwnedNpmCacheEntry(dir, uid, { + maxDurationMs: 10, + now: () => { + now += 6; + return now; + }, + }); + expect(issue).toMatchObject({ + kind: "error", + reason: "npm cache inspection exceeded its 10ms time budget", + }); + }); + + test("terminates a scan process that ignores SIGTERM at the wall-clock deadline", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + const blockingScan = join(dir, "blocking-scan.mjs"); + writeFileSync( + blockingScan, + "process.on('SIGTERM', () => {});\nsetInterval(() => {}, 60_000);\n", + ); + const startedAt = Date.now(); + const result = checkNpmCacheOwnership({ + getuid: () => uid, + spawn: cacheLookup(dir), + scanScript: blockingScan, + maxDurationMs: 250, + }); + expect(Date.now() - startedAt).toBeLessThan(2_000); + expect(result).toMatchObject({ + ok: false, + reason: "npm cache inspection exceeded its 250ms time budget", + }); + }); + + test("force-kills an npm cache lookup that ignores SIGTERM", () => { + const uid = process.getuid?.(); + if (uid === undefined) return; + const blockingNpm = join(dir, "blocking-npm.mjs"); + writeFileSync( + blockingNpm, + "#!/usr/bin/env node\nprocess.on('SIGTERM', () => {});\nsetInterval(() => {}, 60_000);\n", + ); + chmodSync(blockingNpm, 0o755); + + const startedAt = Date.now(); + const result = checkNpmCacheOwnership({ + getuid: () => uid, + lookupTimeoutMs: 250, + npmBin: blockingNpm, + }); + expect(Date.now() - startedAt).toBeLessThan(2_000); + expect(result).toMatchObject({ + ok: false, + reason: "could not resolve the npm cache (ETIMEDOUT)", + }); + }, 5_000); + + test("fails closed before shutdown when npm cannot resolve its cache", () => { + let inspected = false; + const result = checkNpmCacheOwnership({ + platform: "linux", + getuid: () => 501, + spawn: (() => ({ + status: null, + stdout: "", + stderr: "", + error: Object.assign(new Error("timed out"), { code: "ETIMEDOUT" }), + pid: 1, + output: [], + signal: "SIGTERM", + })) as never, + lstat: () => { + inspected = true; + throw new Error("must not inspect without a resolved cache path"); + }, + }); + expect(result).toMatchObject({ + ok: false, + expectedUid: 501, + reason: "could not resolve the npm cache (ETIMEDOUT)", + }); + expect(inspected).toBe(false); + if (result.ok !== false) throw new Error("expected lookup failure"); + expect(formatNpmCacheOwnershipFailure(result)).toContain("before stopping the proxy"); + }); + + test("skips uid checks on Windows without invoking npm", () => { + let spawned = false; + const result = checkNpmCacheOwnership({ + platform: "win32", + spawn: (() => { + spawned = true; + throw new Error("must not spawn"); + }) as never, + }); + expect(result.ok).toBe("skipped"); + expect(spawned).toBe(false); + }); +}); diff --git a/tests/update-stop-first.test.ts b/tests/update-stop-first.test.ts index 32ca3196c..d40d69d87 100644 --- a/tests/update-stop-first.test.ts +++ b/tests/update-stop-first.test.ts @@ -3,6 +3,7 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; const updateSource = readFileSync(join(import.meta.dir, "..", "src", "update", "index.ts"), "utf8"); +const updateJobSource = readFileSync(join(import.meta.dir, "..", "src", "update", "job.ts"), "utf8"); const launcherSource = readFileSync(join(import.meta.dir, "..", "bin", "ocx.mjs"), "utf8"); const serverSource = readFileSync(join(import.meta.dir, "..", "src", "server", "index.ts"), "utf8"); const cliSource = readFileSync(join(import.meta.dir, "..", "src", "cli", "index.ts"), "utf8"); @@ -27,10 +28,25 @@ describe("update stops the running proxy before replacing files", () => { expect(abortAt).toBeLessThan(stopAt); }); + test("npm cache ownership pre-flight runs before either updater stops the proxy", () => { + const cliGateAt = updateSource.indexOf("const cacheOwnership = checkNpmCacheOwnership()"); + const cliStopAt = updateSource.indexOf('[process.argv[1], "stop"]'); + const launcherGateAt = launcherSource.indexOf("const cacheOwnership = checkNpmCacheOwnership("); + const launcherStopAt = launcherSource.indexOf('[launcher, "stop"]'); + expect(cliGateAt).toBeGreaterThan(-1); + expect(launcherGateAt).toBeGreaterThan(-1); + expect(cliGateAt).toBeLessThan(cliStopAt); + expect(launcherGateAt).toBeLessThan(launcherStopAt); + expect(updateSource).toContain("formatNpmCacheOwnershipFailure(cacheOwnership)"); + expect(launcherSource).toContain("formatNpmCacheOwnershipFailure(cacheOwnership)"); + expect(updateSource).toContain("npm cache ownership pre-flight skipped"); + expect(launcherSource).toContain("npm cache ownership pre-flight skipped"); + }); + test("npm launcher update path stops via its own launcher path before npm install", () => { expect(launcherSource).toContain('spawnSync(process.execPath, [launcher, "stop"]'); const stopAt = launcherSource.indexOf('[launcher, "stop"]'); - const installAt = launcherSource.indexOf('spawnSync(npm, ["install", "-g"'); + const installAt = launcherSource.indexOf('runProcessTreeCommand(npm, ["install", "-g"'); expect(stopAt).toBeGreaterThan(-1); expect(stopAt).toBeLessThan(installAt); expect(launcherSource).toContain('existsSync(join(configDir(), "ocx.pid"))'); @@ -50,7 +66,7 @@ describe("update stops the running proxy before replacing files", () => { expect(launcherSource).toContain("OCX_BAKE_PORT"); // Live runtime port 10100 must not be discarded as a missing-port sentinel. expect(launcherSource).toContain("sawRuntimePort"); - expect(updateSource).toContain("runtimeTrusted"); + expect(updateJobSource).toContain("const liveBeforeUpdate = await findLiveProxyForUpdate()"); }); test("both update paths surface a skipped history restore after the stop", () => { @@ -64,7 +80,7 @@ describe("update stops the running proxy before replacing files", () => { expect(launcherSource).toContain('name.startsWith("codex-history-backup-") && name.endsWith(".json")'); expect(launcherSource).toContain("if (historyRestoreIncomplete())"); const warnAt = launcherSource.indexOf("Codex resume history was NOT restored"); - const installAt = launcherSource.indexOf('spawnSync(npm, ["install", "-g"'); + const installAt = launcherSource.indexOf('runProcessTreeCommand(npm, ["install", "-g"'); expect(warnAt).toBeGreaterThan(-1); expect(warnAt).toBeLessThan(installAt); }); @@ -75,6 +91,56 @@ describe("update stops the running proxy before replacing files", () => { expect(launcherSource).toContain("stopRes.status !== 0 || stillHasRuntimeState"); }); + test("GUI failure recovery is gated by identity-checked pre-update liveness", () => { + expect(updateJobSource).toContain("const liveBeforeUpdate = await findLiveProxyForUpdate()"); + expect(updateJobSource).toContain("const proxyWasActive = liveBeforeUpdate !== null"); + expect(updateJobSource).toContain("(io.readAlivePidFn ?? readAlivePid)()"); + expect(updateJobSource).toContain("(io.verifyPidIdentityFn ?? verifyPidIdentityFresh)(candidatePid)"); + expect(updateJobSource).toContain("(io.readRuntimePortFn ?? readRuntimePort)(candidatePid)"); + expect(updateJobSource).not.toContain("const proxyWasActive = isServiceInstalled() || runtimeTrusted"); + }); + + test("GUI failure recovery identity-checks the old PID and threads a validated launcher", () => { + expect(updateJobSource).toContain("(io.verifyPidIdentityFn ?? verifyPidIdentityFresh)(captured.oldPid)"); + expect(updateJobSource).toContain("io.recoveryLaunchersFn ?? findNpmRecoveryLaunchers"); + expect(updateJobSource).toContain("{ ...captured, recoveryLauncher }"); + expect(updateJobSource).toContain("captured?.recoveryLauncher ?? packageLauncherPath()"); + expect(updateJobSource).toContain("const readPidForRestart = (context: string)"); + expect(updateJobSource).toContain("const verifyCurrentPid = io.verifyPidIdentityFn ?? verifyPidIdentityFresh"); + expect(updateJobSource).toContain("verifyCurrentPid(rawPid) === rawPid"); + expect(updateJobSource).toContain('if (readPidForRestart("after service port reclaim").refused) return'); + expect(updateJobSource).toContain('const directPid = readPidForRestart("before direct restart")'); + expect(updateJobSource).toContain('if (readPidForRestart("after direct port reclaim").refused) return'); + expect(updateJobSource).toContain("hasTrustedRecoveryPermissions(rootStat)"); + expect(updateJobSource).toContain("uid === currentUid || uid === 0"); + expect(updateJobSource).toContain("hasTrustedRecoveryTree(packageRoot)"); + expect(updateJobSource).toContain("(stat.mode & 0o022) === 0"); + }); + + test("GUI recovery waits beyond the nested npm install deadline", () => { + const outerRaw = /const UPDATE_TIMEOUT_MS = ([\d_]+);/.exec(updateJobSource)?.[1]; + const innerRaw = /const NPM_INSTALL_TIMEOUT_MS = ([\d_]+);/.exec(launcherSource)?.[1]; + expect(outerRaw).toBeDefined(); + expect(innerRaw).toBeDefined(); + const outerMs = Number(outerRaw?.replaceAll("_", "")); + const innerMs = Number(innerRaw?.replaceAll("_", "")); + expect(outerMs).toBeGreaterThanOrEqual(innerMs + 60_000); + expect(launcherSource).toContain("timeoutMs: NPM_INSTALL_TIMEOUT_MS"); + expect(launcherSource).toContain("await runProcessTreeCommand(npm"); + expect(updateJobSource).toContain("await runLoggedProcessTreeCommand(job, cmd.bin, cmd.args, UPDATE_TIMEOUT_MS)"); + expect(updateJobSource).toContain("if (result.status !== 0 || !result.treeExited)"); + expect(updateJobSource).toContain("return result.treeExited"); + expect(updateJobSource).toContain("installerFailureAllowsRecovery(check.installer, result)"); + expect(updateJobSource).toContain("if (trayWasRunning && mayRecover)"); + expect(updateJobSource).toContain("The Windows tray also remains stopped"); + expect(updateJobSource).toContain("candidates.slice(0, MAX_NPM_RECOVERY_CANDIDATES)"); + }); + + test("GUI recovery scan imports the worker argument contract from the worker module", () => { + expect(updateJobSource).toContain('import { RECOVERY_TREE_SCAN_WORKER_ARG } from "./recovery-tree-scan.mjs"'); + expect(updateJobSource).not.toContain('const RECOVERY_TREE_SCAN_WORKER_ARG = "__scan-recovery-tree"'); + }); + test("GUI worker update children use pipe stdio so Windows npm.cmd does not open consoles", () => { expect(updateSource).toContain("function updateChildStdio()"); expect(updateSource).toContain('process.env.OCX_SERVICE === "1"'); @@ -85,6 +151,17 @@ describe("update stops the running proxy before replacing files", () => { expect(updateSource).toContain("stdio: svcStdio"); expect(updateSource).toContain("windowsHide: true"); }); + + test("Bun/source installer cleanup is tree-aware before shim or service refresh", () => { + const installAt = updateSource.indexOf("await runProcessTreeCommand(target.bin, cmdArgs"); + const cleanupGateAt = updateSource.indexOf("if (!r.treeExited)"); + const successAt = updateSource.indexOf("if (r.status === 0)"); + expect(installAt).toBeGreaterThan(-1); + expect(cleanupGateAt).toBeGreaterThan(installAt); + expect(cleanupGateAt).toBeLessThan(successAt); + expect(updateSource).toContain("timeoutMs: 180000"); + expect(updateSource).toContain("INSTALLER_TREE_CLEANUP_FAILED_EXIT_CODE"); + }); }); describe("ocx update --help has no side effects (#168)", () => { @@ -99,7 +176,7 @@ describe("ocx update --help has no side effects (#168)", () => { test("the npm launcher intercepts update --help before the self-update path", () => { const helpAt = launcherSource.indexOf("updateHelpRequested"); - const updateAt = launcherSource.indexOf("runNpmSelfUpdate();"); + const updateAt = launcherSource.indexOf("await runNpmSelfUpdate();"); expect(helpAt).toBeGreaterThan(-1); expect(launcherSource).toContain('process.argv[2] === "update" &&'); // The guard that CALLS the self-update must come after the help exit. diff --git a/tests/windows-deploy-close-regressions.test.ts b/tests/windows-deploy-close-regressions.test.ts index 2d9a89afb..8b8d6f319 100644 --- a/tests/windows-deploy-close-regressions.test.ts +++ b/tests/windows-deploy-close-regressions.test.ts @@ -27,7 +27,7 @@ describe("update-job restart avoids the shell-less .cmd EINVAL (Windows, bun/sou expect(src).toContain("OCX_BAKE_PORT"); // Service reinstall still runs (with bake) even when reclaim warns; direct start refuses to hop. expect(src).toContain("refusing to hop"); - expect(src).toContain("runtimeTrusted"); + expect(src).toContain("const liveBeforeUpdate = await findLiveProxyForUpdate()"); expect(read("src/cli/index.ts")).toContain("allowEphemeralFallback: !hardPin"); expect(read("src/cli/index.ts")).toContain("preferRetryMs: hardPin ? 0 : 750"); expect(read("src/cli/index.ts")).toContain("Not opening the GUI");