diff --git a/bin/ocx.mjs b/bin/ocx.mjs index dbf63b4ae2f..fe1d34a4c6a 100755 --- a/bin/ocx.mjs +++ b/bin/ocx.mjs @@ -42,6 +42,7 @@ import { resolvePnpmGlobalOwner, runPnpmGlobalUpdate, } from "../src/update/pnpm-global-install.mjs"; +import { PNPM_READ_CWD, pnpmCommandCwd, pnpmReadEnvironment } from "../src/update/pnpm-read-policy.mjs"; import { checkRegistryPackageIntegrity } from "../src/update/registry-integrity.mjs"; import { hasPendingTeardownIn } from "../src/config/pending-teardown-names.mjs"; import { @@ -208,6 +209,8 @@ function runPackageManagerSelfUpdate(manager) { encoding: "utf8", timeout: 20_000, windowsHide: true, + cwd: PNPM_READ_CWD, + env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(process.env)), ...invocation.options, }); }, @@ -221,6 +224,20 @@ function runPackageManagerSelfUpdate(manager) { const managerInvocation = args => manager === "pnpm" ? pnpmOwnerInvocation(owner, args) : npmInvocation(args); + // Read-only pnpm probes run from the installed package directory with project pnpmfiles + // disabled, so an attacker-controlled cwd cannot execute hooks during the update check. + const readProbeOptions = invocation => ({ + encoding: "utf8", + timeout: 12000, + windowsHide: true, + ...(manager === "pnpm" + ? { + cwd: PNPM_READ_CWD, + env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env)), + } + : invocation.env ? { env: invocation.env } : {}), + ...invocation.options, + }); const latestInvocation = managerInvocation(["view", `${PKG}@${tag}`, "version"]); const installArgs = manager === "pnpm" ? ["add", "-g", "--allow-build=bun", `${PKG}@${tag}`] @@ -230,13 +247,7 @@ function runPackageManagerSelfUpdate(manager) { console.error(`opencodex: could not resolve ${manager} from a trusted absolute PATH entry; aborting before stopping the proxy.`); process.exit(1); } - const latestResult = spawnSync(latestInvocation.file, latestInvocation.args, { - encoding: "utf8", - timeout: 12000, - windowsHide: true, - ...(latestInvocation.env ? { env: latestInvocation.env } : {}), - ...latestInvocation.options, - }); + const latestResult = spawnSync(latestInvocation.file, latestInvocation.args, readProbeOptions(latestInvocation)); const latest = latestResult.status === 0 && typeof latestResult.stdout === "string" ? latestResult.stdout.trim() : ""; console.log(`opencodex v${current} (installed via ${manager}, tag ${tag})`); @@ -248,13 +259,7 @@ function runPackageManagerSelfUpdate(manager) { const integrity = checkRegistryPackageIntegrity(PKG, latest || null, args => { const invocation = managerInvocation(args); if (!invocation) return { status: 1 }; - return spawnSync(invocation.file, invocation.args, { - encoding: "utf8", - timeout: 12000, - windowsHide: true, - ...(invocation.env ? { env: invocation.env } : {}), - ...invocation.options, - }); + return spawnSync(invocation.file, invocation.args, readProbeOptions(invocation)); }); if (integrity.ok === false) { console.error(`opencodex: ${integrity.reason}; aborting before stopping the proxy.`); @@ -771,7 +776,10 @@ function runPackageManagerSelfUpdate(manager) { encoding: "utf8", timeout: 180000, windowsHide: true, - env: unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env), + // Reads probe from the package dir; mutations (add -g, rollback) must not + // keep a cwd handle inside the package Windows is replacing. + cwd: pnpmCommandCwd(args), + env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env)), }); }, log: line => console.log(line), diff --git a/src/update/async-check.ts b/src/update/async-check.ts index e24a6eccc19..08ffa5fb3a2 100644 --- a/src/update/async-check.ts +++ b/src/update/async-check.ts @@ -2,6 +2,7 @@ import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; import { unprivilegedOwnershipMutationEnvironment } from "../service/ownership-mutation-lease.mjs"; import { PKG, registrySpawnTarget, type Channel, type Installer } from "./index"; import type { PnpmGlobalOwner } from "./pnpm-global-install.mjs"; +import { PNPM_READ_CWD, pnpmReadEnvironment } from "./pnpm-read-policy.mjs"; export const REGISTRY_DEADLINE_MS = 12_000; export const REGISTRY_OUTPUT_LIMIT = 4_096; @@ -64,7 +65,10 @@ export async function latestVersionAsync( child = deps.spawnFn(target.bin, target.args, { stdio: ["pipe", "pipe", "pipe"], windowsHide: true, - env: unprivilegedOwnershipMutationEnvironment(target.env ?? process.env), + cwd: installer === "pnpm" ? PNPM_READ_CWD : undefined, + env: installer === "pnpm" + ? pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(target.env ?? process.env)) + : unprivilegedOwnershipMutationEnvironment(target.env ?? process.env), ...target.options, }) as ChildProcessWithoutNullStreams; } catch { diff --git a/src/update/index.ts b/src/update/index.ts index 15ee935db62..552edf01a0b 100644 --- a/src/update/index.ts +++ b/src/update/index.ts @@ -40,6 +40,7 @@ import { handoffWindowsTrayForUpdate, planWindowsTrayUpdate } from "./tray-updat import { withProcessRuntimeProvenance } from "../lib/bun-runtime"; import { packageVersion } from "../lib/package-version"; import { selfLaunchArgv } from "../lib/self-launch-argv"; +import { PNPM_READ_CWD, pnpmCommandCwd, pnpmReadEnvironment } from "./pnpm-read-policy.mjs"; /** * A `codex-history-backup-*.json` surviving a stop means the native-history restore was @@ -97,27 +98,33 @@ function runPnpmCandidate( commandPath: string, args: readonly string[], capture = false, + spawn: typeof spawnSync = spawnSync, ): { status: number | null; stdout?: string | null; stderr?: string | null } { const invocation = pnpmInvocationForPath(commandPath, args); if (!invocation) return { status: 1 }; - return spawnSync(invocation.file, invocation.args, { + return spawn(invocation.file, invocation.args, { stdio: capture ? "pipe" : "ignore", encoding: "utf8", timeout: 20_000, windowsHide: true, - env: unprivilegedOwnershipMutationEnvironment(process.env), + cwd: PNPM_READ_CWD, + env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(process.env)), ...invocation.options, }); } /** Resolve the exact pnpm executable/group/bin that own this package. */ -export function resolveCurrentPnpmGlobalOwner(invoked = process.argv[1]): PnpmGlobalOwnerResult { +export function resolveCurrentPnpmGlobalOwner( + invoked = process.argv[1], + deps: { commandPaths?: readonly string[]; spawn?: typeof spawnSync } = {}, +): PnpmGlobalOwnerResult { + const spawn = deps.spawn ?? spawnSync; return resolvePnpmGlobalOwner({ packageName: PKG, packagePath: packageRoot(), - commandPaths: resolvePnpmCommands(), + commandPaths: deps.commandPaths ?? resolvePnpmCommands(), runningShimPath: runningPnpmShimPath(invoked), - runPnpm: runPnpmCandidate, + runPnpm: (commandPath, args, capture) => runPnpmCandidate(commandPath, args, capture, spawn), }); } @@ -148,7 +155,10 @@ function runOwnedPnpm( encoding: "utf8", timeout: 180_000, windowsHide: true, - env: unprivilegedOwnershipMutationEnvironment(target.env), + // Reads probe from the package dir; `add -g`/rollback children run from a neutral + // directory so a Windows cwd handle never pins open the package pnpm is replacing. + cwd: pnpmCommandCwd(args), + env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(target.env)), ...target.options, }); } @@ -275,16 +285,20 @@ export function latestVersion( tag: string, installer: Installer = detectInstall(), owner?: PnpmGlobalOwner, + spawn: typeof spawnSync = spawnSync, ): string | null { const resolvedOwner = installer === "pnpm" ? selectedPnpmOwner(owner) : undefined; if (installer === "pnpm" && !resolvedOwner) return null; const manager = registrySpawnTarget(installer, ["view", `${PKG}@${tag}`, "version"], resolvedOwner); if (!manager) return null; - const r = spawnSync(manager.bin, manager.args, { + const r = spawn(manager.bin, manager.args, { encoding: "utf8", timeout: 12000, windowsHide: true, - env: unprivilegedOwnershipMutationEnvironment(manager.env ?? process.env), + cwd: installer === "pnpm" ? PNPM_READ_CWD : undefined, + env: installer === "pnpm" + ? pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(manager.env ?? process.env)) + : unprivilegedOwnershipMutationEnvironment(manager.env ?? process.env), ...manager.options, }); return r.status === 0 && typeof r.stdout === "string" ? (r.stdout.trim() || null) : null; @@ -341,7 +355,10 @@ export function checkUpdatePackageIntegrity( encoding: "utf8", timeout: 12000, windowsHide: true, - env: unprivilegedOwnershipMutationEnvironment(target.env ?? process.env), + cwd: installer === "pnpm" ? PNPM_READ_CWD : undefined, + env: installer === "pnpm" + ? pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(target.env ?? process.env)) + : unprivilegedOwnershipMutationEnvironment(target.env ?? process.env), ...target.options, }); }); diff --git a/src/update/pnpm-read-policy.d.mts b/src/update/pnpm-read-policy.d.mts new file mode 100644 index 00000000000..2facc3cbb5f --- /dev/null +++ b/src/update/pnpm-read-policy.d.mts @@ -0,0 +1,9 @@ +export declare const PNPM_READ_CWD: string; + +export declare const PNPM_MUTATION_CWD: string; + +export declare function pnpmCommandCwd(args?: readonly string[]): string; + +export declare function pnpmReadEnvironment( + env?: Record, +): Record; diff --git a/src/update/pnpm-read-policy.mjs b/src/update/pnpm-read-policy.mjs new file mode 100644 index 00000000000..b6b48aa9649 --- /dev/null +++ b/src/update/pnpm-read-policy.mjs @@ -0,0 +1,27 @@ +import { tmpdir } from "node:os"; +import { dirname } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** Keep read-only pnpm probes away from the caller's project and its executable hooks. */ +export const PNPM_READ_CWD = dirname(fileURLToPath(import.meta.url)); + +const PNPM_MUTATION_COMMANDS = new Set(["add", "install", "update", "remove", "uninstall"]); + +/** + * Mutations (`add -g`, rollback) cannot run from inside the installed package: on Windows + * a child whose working directory sits in the tree being replaced pins it open and blocks + * removal. A stable directory outside the package keeps that handle neutral. + */ +export const PNPM_MUTATION_CWD = tmpdir(); + +/** Working directory for a pnpm child: read probes isolate, mutations stay outside the package. */ +export function pnpmCommandCwd(args) { + return PNPM_MUTATION_COMMANDS.has(args?.[0]) ? PNPM_MUTATION_CWD : PNPM_READ_CWD; +} + +export function pnpmReadEnvironment(env = process.env) { + const isolated = Object.fromEntries( + Object.entries(env).filter(([key]) => key.toLowerCase() !== "npm_config_ignore_pnpmfile"), + ); + return { ...isolated, npm_config_ignore_pnpmfile: "true" }; +} diff --git a/structure/ops/service-and-sidecars.md b/structure/ops/service-and-sidecars.md index 2c5b32d69b0..d0b16308c1e 100644 --- a/structure/ops/service-and-sidecars.md +++ b/structure/ops/service-and-sidecars.md @@ -272,6 +272,6 @@ constants in child processes. src/update/refresh-scheduler.ts owns the package cache timer and per-channel singleflight for the running proxy. Eligible npm, pnpm and Bun installs refresh missing or 20-hour-stale `version.json` after bind, check staleness hourly and retry failures with bounded backoff. Each server start owns one scheduler reference; the last matching stop disarms the timer. A stopped automatic lookup cannot write a late result, but an explicit check joining that lookup marks explicit interest and writes its successful result even if the last listener stops before it resolves. Source/mise installs and `OCX_DISABLE_UPDATE_CHECK=1` do not start automatic lookup; explicit requests remain available. -src/update/async-check.ts uses the existing owner-bound registry target with a bounded asynchronous child; pnpm owner discovery runs in src/update/pnpm-owner-worker.ts off the request loop. `src/update/notify.ts` writes successful results atomically and preserves a dismissal only for the same channel and version. The interactive pre-bind prompt reads the cache and does not launch a second detached refresh. `src/update/badge.ts` only reads the cache and reports unknown at 40 hours. +src/update/async-check.ts uses the existing owner-bound registry target with a bounded asynchronous child; pnpm owner discovery runs in src/update/pnpm-owner-worker.ts off the request loop. Read-only pnpm owner and registry probes — in the scheduler, the synchronous updater, and the `bin/ocx.mjs` package-manager self-update — run from the installed update module directory via `src/update/pnpm-read-policy.mjs` with project pnpmfiles disabled, never from the caller's workspace. pnpm mutations (`add -g`, rollback) instead run from a neutral directory outside the package — on Windows a cwd inside the tree being replaced pins it open and blocks removal. `src/update/notify.ts` writes successful results atomically and preserves a dismissal only for the same channel and version. The interactive pre-bind prompt reads the cache and does not launch a second detached refresh. `src/update/badge.ts` only reads the cache and reports unknown at 40 hours. The desktop badge snapshot in src/update/desktop-badge.ts is process-local display state keyed by a Tauri session id. A 60-second shell heartbeat renews receipt time; entries expire after 180 seconds and the store retains at most 32 sessions. It is separate from the package version cache and from the updater job/ownership transaction. A proxy restart reports unknown until a bound desktop shell republishes; no update installation can be authorized by this snapshot. diff --git a/tests/update/update-refresh.test.ts b/tests/update/update-refresh.test.ts index 9981ac6ff3b..534cdcc898e 100644 --- a/tests/update/update-refresh.test.ts +++ b/tests/update/update-refresh.test.ts @@ -1,10 +1,14 @@ import { describe, expect, test } from "bun:test"; import { EventEmitter } from "node:events"; +import { dirname } from "node:path"; import { PassThrough } from "node:stream"; +import { fileURLToPath } from "node:url"; import { createRefreshScheduler, RETRY_BASE_MS, STALENESS_TICK_MS, type RefreshDeps } from "../../src/update/refresh-scheduler"; import { latestVersionAsync, pnpmOwner, REGISTRY_DEADLINE_MS, REGISTRY_OUTPUT_LIMIT } from "../../src/update/async-check"; import type { VersionCache } from "../../src/update/notify"; -import type { Channel, Installer } from "../../src/update/index"; +import { checkUpdatePackageIntegrity, latestVersion, resolveCurrentPnpmGlobalOwner, type Channel, type Installer } from "../../src/update/index"; +import { PNPM_MUTATION_CWD, PNPM_READ_CWD, pnpmCommandCwd, pnpmReadEnvironment } from "../../src/update/pnpm-read-policy.mjs"; +import { runPnpmGlobalUpdate } from "../../src/update/pnpm-global-install.mjs"; function fixture(installer: Installer = "npm", disabled = false, lookupFn?: RefreshDeps["lookup"]) { let now = 1_700_000_000_000; @@ -278,6 +282,168 @@ test("pnpm owner resolution failure is unavailable, not an unowned PATH lookup", expect(spawned).toBe(false); }); +test("pnpm read probes ignore caller project hooks from a trusted directory", () => { + const input = { npm_config_ignore_pnpmfile: "false", NPM_CONFIG_IGNORE_PNPMFILE: "false", SECRET: "retained" }; + expect(pnpmReadEnvironment(input)).toEqual({ npm_config_ignore_pnpmfile: "true", SECRET: "retained" }); + expect(input.npm_config_ignore_pnpmfile).toBe("false"); + expect(PNPM_READ_CWD).toBe(dirname(fileURLToPath(new URL("../../src/update/pnpm-read-policy.mjs", import.meta.url)))); +}); + +test("pnpm registry lookup applies project isolation to its child", async () => { + const child = fakeChild(); + let options: Record | undefined; + const result = latestVersionAsync("latest", "pnpm", { + ownerFn: async () => ({ + commandPath: "/trusted/pnpm", packagePath: "/pkg", globalDir: "/global", + globalRoot: "/global", globalBinDir: "/bin", + }), + spawnFn: ((_bin: string, _args: string[], observed: Record) => { + options = observed; + queueMicrotask(() => { child.stdout.write("2.7.44\n"); child.emit("close", 0); }); + return child; + }) as never, + }); + expect(await result).toBe("2.7.44"); + expect(options?.cwd).toBe(PNPM_READ_CWD); + expect((options?.env as Record).npm_config_ignore_pnpmfile).toBe("true"); +}); + +test("non-pnpm registry lookups keep the caller's working directory", async () => { + const child = fakeChild(); + let options: Record | undefined; + const result = latestVersionAsync("latest", "npm", { + ownerFn: async () => null, + spawnFn: ((_bin: string, _args: string[], observed: Record) => { + options = observed; + queueMicrotask(() => { child.stdout.write("2.7.44\n"); child.emit("close", 0); }); + return child; + }) as never, + }); + expect(await result).toBe("2.7.44"); + expect(options?.cwd).toBeUndefined(); +}); + +const TEST_PNPM_OWNER = { + commandPath: "/trusted/pnpm", packagePath: "/pkg", globalDir: "/global", + globalRoot: "/global", globalBinDir: "/bin", +}; + +test("synchronous pnpm registry lookup applies project isolation", () => { + let options: Record | undefined; + const version = latestVersion("latest", "pnpm", TEST_PNPM_OWNER, (( + _bin: string, + _args: string[], + observed: Record, + ) => { + options = observed; + return { status: 0, stdout: "2.7.44\n", stderr: "", pid: 1, output: [], signal: null }; + }) as never); + expect(version).toBe("2.7.44"); + expect(options?.cwd).toBe(PNPM_READ_CWD); + expect((options?.env as Record).npm_config_ignore_pnpmfile).toBe("true"); +}); + +test("synchronous pnpm integrity probe applies project isolation", () => { + let options: Record | undefined; + const result = checkUpdatePackageIntegrity("2.7.44", (( + _bin: string, + _args: string[], + observed: Record, + ) => { + options = observed; + return { status: 0, stdout: "sha512-AbC123+/=\n", stderr: "", pid: 1, output: [], signal: null }; + }) as never, "pnpm", TEST_PNPM_OWNER); + expect(result.ok).toBe(true); + expect(options?.cwd).toBe(PNPM_READ_CWD); + expect((options?.env as Record).npm_config_ignore_pnpmfile).toBe("true"); +}); + +test("pnpm mutations run outside the installed package while reads stay isolated", () => { + // On Windows a cwd inside the replaced package pins it open; mutations go to a neutral dir. + for (const read of [["list", "-g"], ["root", "-g"], ["view", "pkg@1", "version"], ["config", "get", "global-dir"]]) { + expect(pnpmCommandCwd(read)).toBe(PNPM_READ_CWD); + } + for (const mutation of [["add", "-g", "pkg@1"], ["install", "-g", "pkg@1"], ["remove", "-g", "pkg"], ["update", "-g"], ["uninstall", "-g", "pkg"]]) { + expect(pnpmCommandCwd(mutation)).toBe(PNPM_MUTATION_CWD); + } + expect(PNPM_MUTATION_CWD).not.toBe(PNPM_READ_CWD); +}); + +const MUTATION_VERBS = new Set(["add", "install", "update", "remove", "uninstall"]); + +// The spawn sites apply `cwd: pnpmCommandCwd(args)` to the args the transaction issues, so +// driving the transaction and classifying every recorded command is the spawn-options check: +// a future verb or arg shape that escapes the mutation list fails here. +function drivePnpmUpdate(listVersions: string[], installStatus = 0, rollbackStatus = 0) { + const issued: string[][] = []; + let listCall = 0; + let addCall = 0; + const listJson = (version: string) => JSON.stringify([ + { path: "/global", dependencies: { ocx_test: { version, path: "/pkg" } } }, + ]); + const result = runPnpmGlobalUpdate({ + packageName: "ocx_test", + currentVersion: "2.7.44", + targetVersion: listVersions[1] ?? listVersions[0], + tag: "latest", + owner: TEST_PNPM_OWNER, + runningPackagePath: "/pkg", + runPnpm: (args: string[]) => { + issued.push([...args]); + if (args[0] === "list") { + return { status: 0, stdout: listJson(listVersions[Math.min(listCall++, listVersions.length - 1)]) }; + } + if (args[0] === "add") return { status: addCall++ === 0 ? installStatus : rollbackStatus }; + return { status: 1 }; + }, + verify: () => ({ ok: true }), + verifyShims: () => ({ ok: true }), + }); + return { issued, result }; +} + +test("the install path launches mutations from the neutral cwd and reads from the package", () => { + const { issued, result } = drivePnpmUpdate(["2.7.44", "2.7.45"]); + expect(result.ok).toBe(true); + const adds = issued.filter(args => args[0] === "add"); + expect(adds).toHaveLength(1); + expect(adds[0]).toContain("ocx_test@2.7.45"); + for (const args of issued) { + expect(pnpmCommandCwd(args)).toBe(MUTATION_VERBS.has(args[0]) ? PNPM_MUTATION_CWD : PNPM_READ_CWD); + } +}); + +test("the rollback path also launches its pnpm child from the neutral cwd", () => { + // Install fails and the active package moved, so the transaction issues a second `add -g` + // to restore the previous version — that child must also leave the replaced package. + const { issued, result } = drivePnpmUpdate(["2.7.44", "2.7.45", "2.7.44"], 1); + expect(result.ok).toBe(false); + expect(result.phase).toBe("rollback"); + const adds = issued.filter(args => args[0] === "add"); + expect(adds).toHaveLength(2); + expect(adds[1]).toContain("ocx_test@2.7.44"); + for (const args of issued) { + expect(pnpmCommandCwd(args)).toBe(MUTATION_VERBS.has(args[0]) ? PNPM_MUTATION_CWD : PNPM_READ_CWD); + } +}); + +test("synchronous pnpm owner discovery applies project isolation", () => { + const observed: Array> = []; + const result = resolveCurrentPnpmGlobalOwner("not-a-shim", { + commandPaths: ["/trusted/pnpm"], + spawn: ((_bin: string, _args: string[], options: Record) => { + observed.push(options); + return { status: 1, stdout: "", stderr: "", pid: 1, output: [], signal: null }; + }) as never, + }); + expect(result.ok).toBe(false); + expect(observed.length).toBeGreaterThan(0); + for (const options of observed) { + expect(options.cwd).toBe(PNPM_READ_CWD); + expect((options.env as Record).npm_config_ignore_pnpmfile).toBe("true"); + } +}); + const CAN_RUN_BUN_WORKER = ["darwin", "linux", "win32"].includes(process.platform) && typeof Worker === "function";