fix(update): isolate pnpm probes and mutation workspaces

bin/ocx.mjs

@@ -42,6 +42,7 @@ import {
   resolvePnpmGlobalOwner,
   runPnpmGlobalUpdate,
 } from "../src/update/pnpm-global-install.mjs";
+import { PNPM_READ_CWD, withPnpmCommandCwd, 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.`);
@@ -768,14 +773,17 @@ function runPackageManagerSelfUpdate(manager) {
           runPnpm: (args, capture = false) => {
             const invocation = pnpmOwnerInvocation(owner, args);
             if (!invocation) return { status: 1 };
-            return spawnSync(invocation.file, invocation.args, {
+            return withPnpmCommandCwd(args, cwd => spawnSync(invocation.file, invocation.args, {
               ...invocation.options,
               stdio: capture ? "pipe" : "inherit",
               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,
+              env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(invocation.env ?? process.env)),
+            }));
           },
           log: line => console.log(line),
         });

scripts/test-layout/layout.json

@@ -168,6 +168,7 @@
     }
   },
   "explicit": {
+    "pnpm-command-isolation.test.ts": "update",
     "provider-antigravity-quota-retry.test.ts": "providers",
     "responses-compaction-recovery.test.ts": "responses", "compaction-recovery-settings.test.ts": "config", "responses-compaction-recovery-policy.test.ts": "responses", "plugin-loader.test.ts": "lib", "plugin-upstream-hooks.test.ts": "lib",
     "cli-kiro-auto-selection.test.ts": "cli", "codebuddy-live-models.test.ts": "providers", "kiro-auto-selection.test.ts": "providers/kiro", "kiro-quota-metrics.test.ts": "providers/kiro", "management-provider-request-pacing.test.ts": "server",

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 {

src/update/index.ts

@@ -41,6 +41,7 @@ import { withProcessRuntimeProvenance } from "../lib/bun-runtime";
 import { withoutSiblingMarker } from "../codex/sibling-start";
 import { packageVersion } from "../lib/package-version";
 import { selfLaunchArgv } from "../lib/self-launch-argv";
+import { PNPM_READ_CWD, withPnpmCommandCwd, pnpmReadEnvironment } from "./pnpm-read-policy.mjs";
 
 /**
  * A `codex-history-backup-*.json` surviving a stop means the native-history restore was
@@ -98,27 +99,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),
   });
 }
 
@@ -136,22 +143,26 @@ function ownerPnpmTarget(
   };
 }
 
-function runOwnedPnpm(
+export function runOwnedPnpm(
   owner: PnpmGlobalOwner,
   args: readonly string[],
   capture: boolean,
   stdio: "inherit" | "pipe" | "ignore" = capture ? "pipe" : "inherit",
+  spawn: typeof spawnSync = spawnSync,
 ): { status: number | null; stdout?: string | null; stderr?: string | null } {
   const target = ownerPnpmTarget(owner, args);
   if (!target) return { status: 1 };
-  return spawnSync(target.bin, target.args, {
+  return withPnpmCommandCwd(args, cwd => spawn(target.bin, target.args, {
     stdio,
     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,
+    env: pnpmReadEnvironment(unprivilegedOwnershipMutationEnvironment(target.env)),
     ...target.options,
-  });
+  }));
 }
 
 /** Re-read the owning group's active package and return its verified launcher. */
@@ -276,16 +287,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;
@@ -342,7 +357,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,
     });
   });

src/update/pnpm-read-policy.d.mts

@@ -0,0 +1,4 @@
+export declare const PNPM_READ_CWD: string;
+/** The callback must complete synchronously before its temporary workspace is cleaned. */
+export declare function withPnpmCommandCwd<T>(args: readonly string[], run: (cwd: string) => T): T;
+export declare function pnpmReadEnvironment(env?: Record<string, string | undefined>): Record<string, string | undefined>;

src/update/pnpm-read-policy.mjs

@@ -0,0 +1,44 @@
+import { mkdtempSync, lstatSync, writeFileSync, unlinkSync, rmdirSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+/** Read probes never inherit the caller's project as their working directory. */
+export const PNPM_READ_CWD = dirname(fileURLToPath(import.meta.url));
+const MUTATIONS = new Set(["add", "install", "update", "remove", "uninstall"]);
+
+/** Run a synchronous pnpm mutation outside the package, in a private workspace boundary. */
+export function withPnpmCommandCwd(args, run) {
+  if (!MUTATIONS.has(args?.[0])) return run(PNPM_READ_CWD);
+  const cwd = mkdtempSync(join(tmpdir(), "ocx-pnpm-command-"));
+  const created = lstatSync(cwd);
+  const files = ["pnpm-workspace.yaml", ".npmrc"];
+  try {
+    // Stop discovery at our own workspace, rather than inheriting a shared /tmp workspace.
+    writeFileSync(join(cwd, files[0]), "packages: []\nignorePnpmfile: true\n", { flag: "wx", mode: 0o600 });
+    writeFileSync(join(cwd, files[1]), "ignore-pnpmfile=true\n", { flag: "wx", mode: 0o600 });
+    return run(cwd);
+  } finally {
+    try {
+      const now = lstatSync(cwd);
+      if (!now.isDirectory() || now.isSymbolicLink() || now.dev !== created.dev || now.ino !== created.ino) {
+        throw new Error("temporary directory identity changed");
+      }
+      // Never recursively remove pnpm-created or replacement contents.
+      for (const name of files) {
+        try { unlinkSync(join(cwd, name)); }
+        catch (error) { if (error?.code !== "ENOENT") throw error; }
+      }
+      rmdirSync(cwd);
+    } catch {
+      console.warn("[opencodex] Temporary pnpm workspace cleanup was incomplete; retained for inspection.");
+    }
+  }
+}
+
+/** pnpm 11 uses pnpm_config_ while earlier versions use npm_config_. */
+export function pnpmReadEnvironment(env = process.env) {
+  const ignored = new Set(["npm_config_ignore_pnpmfile", "pnpm_config_ignore_pnpmfile"]);
+  const isolated = Object.fromEntries(Object.entries(env).filter(([key]) => !ignored.has(key.toLowerCase())));
+  return { ...isolated, npm_config_ignore_pnpmfile: "true", pnpm_config_ignore_pnpmfile: "true" };
+}

structure/ops/service-and-sidecars.md

@@ -328,7 +328,7 @@ detached replacement's environment drops the marker. Coverage:
 
 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 in unique private temporary workspaces outside the package, with an explicit empty workspace boundary to stop parent-project discovery. Both npm_config_ and pnpm_config_ ignore-pnpmfile controls are set case-insensitively for pnpm 10/11. Cleanup removes only known files and an empty unchanged directory; unexpected contents remain for inspection. On Windows, this also avoids pinning the replaced package as cwd. `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.
 

tests/fixtures/test-layout-expected.json

@@ -1,4 +1,5 @@
 {
+  "pnpm-command-isolation.test.ts": "update",
   "provider-antigravity-quota-retry.test.ts": "providers",
   "responses-compaction-recovery.test.ts": "responses",
   "compaction-recovery-settings.test.ts": "config",

tests/update/pnpm-command-isolation.test.ts

@@ -0,0 +1,57 @@
+import { expect, test } from "bun:test";
+import { existsSync, readFileSync, lstatSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { runOwnedPnpm } from "../../src/update/index";
+import { runPnpmGlobalUpdate } from "../../src/update/pnpm-global-install.mjs";
+import { PNPM_READ_CWD, pnpmReadEnvironment, withPnpmCommandCwd } from "../../src/update/pnpm-read-policy.mjs";
+
+test("both pnpm environment prefixes override every case variant without mutating the parent", () => {
+  const original = { npm_config_ignore_pnpmfile: "false", NPM_CONFIG_IGNORE_PNPMFILE: "false", PnPm_CoNfIg_IgNoRe_PnPmFiLe: "false", pnpm_config_ignore_pnpmfile: "false", KEEP: "retained" };
+  const result = pnpmReadEnvironment(original);
+  expect(result).toEqual({ npm_config_ignore_pnpmfile: "true", pnpm_config_ignore_pnpmfile: "true", KEEP: "retained" });
+  expect(original.npm_config_ignore_pnpmfile).toBe("false");
+});
+
+test("mutation workspaces are unique, bounded to known files, and cleaned after a thrown callback", () => {
+  const seen: string[] = [];
+  for (let i=0; i<2; i++) {
+    expect(() => withPnpmCommandCwd(["add", "-g", "fixture"], cwd => {
+      seen.push(cwd);
+      expect(cwd).not.toBe(tmpdir()); expect(cwd).not.toBe(PNPM_READ_CWD);
+      expect(readFileSync(cwd + "/pnpm-workspace.yaml", "utf8")).toContain("packages: []");
+      if (process.platform !== "win32") expect(lstatSync(cwd).mode & 0o077).toBe(0);
+      throw new Error("fixture failure");
+    })).toThrow("fixture failure");
+  }
+  expect(seen[0]).not.toBe(seen[1]);
+  for (const cwd of seen) expect(existsSync(cwd)).toBe(false);
+});
+
+const owner = { commandPath: "/trusted/pnpm", packagePath: "/pkg", globalDir: "/global", globalRoot: "/global", globalBinDir: "/bin" };
+for (const fail of [false,true]) {
+  test(`actual pnpm spawn options isolate ${fail ? "rollback" : "install"} and registry reads`, () => {
+    const mutations: string[] = [];
+    let listCall=0, addCall=0, spawnCalls=0;
+    const versions = fail ? ["1.0.0","1.0.1","1.0.0"] : ["1.0.0","1.0.1"];
+    const result = runPnpmGlobalUpdate({ packageName:"ocx_test", currentVersion:"1.0.0", targetVersion:"1.0.1", tag:"latest", owner, runningPackagePath:"/pkg",
+      runPnpm:(args:string[],capture=false) => runOwnedPnpm(owner,args,capture,"ignore", ((_bin:unknown,_argv:unknown,options:{cwd:string;env:Record<string,string>}) => {
+        spawnCalls++;
+        expect(options.env.npm_config_ignore_pnpmfile).toBe("true");
+        expect(options.env.pnpm_config_ignore_pnpmfile).toBe("true");
+        let status=0,stdout="";
+        if(args[0]==="add") {
+          mutations.push(options.cwd);
+          expect(options.cwd).not.toBe(tmpdir()); expect(options.cwd).not.toBe(PNPM_READ_CWD);
+          expect(existsSync(options.cwd+"/pnpm-workspace.yaml")).toBe(true);
+          status=fail && addCall++===0 ? 1 : 0;
+        } else {
+          expect(options.cwd).toBe(PNPM_READ_CWD);
+          if(args[0]==="list") stdout=JSON.stringify([{path:"/global",dependencies:{ocx_test:{version:versions[Math.min(listCall++,versions.length-1)],path:"/pkg"}}}]);
+        }
+        return {status,stdout,stderr:"",pid:1,output:[],signal:null};
+      }) as never), verify:()=>({ok:true}),verifyShims:()=>({ok:true}) });
+    expect(result.ok).toBe(!fail); expect(spawnCalls).toBeGreaterThan(1);
+    expect(mutations).toHaveLength(fail ? 2 : 1);
+    for(const cwd of mutations) expect(existsSync(cwd)).toBe(false);
+  });
+}