fix(plugins): run npm update checks without the npm.cmd shim on Windows (#837)

On Windows `npm` on PATH is actually `npm.cmd`, which `execFile` refuses to
spawn (CVE-2024-27980), so every npm plugin update check failed with
`spawn npm ENOENT` while Git sources kept working. The plugin page could
therefore never offer an update for npm-installed plugins.

Resolve `npm` and `npx` through the CLI script bundled with the running
Node.js (`npm-cli.js` / `npx-cli.js`) and spawn it with `node` directly: no
shell is involved and the behavior is identical on every platform. The
lookup the skills routes already used for npx now lives in
`lib/node-cli.ts` and is reused for npm.

Co-authored-by: userliwq <22310579+userliwq@users.noreply.github.com>

Author: userliwq

lib/node-cli.test.mjs

@@ -0,0 +1,37 @@
+import assert from "node:assert/strict";
+import { join } from "node:path";
+import { execPath } from "node:process";
+import test from "node:test";
+import { createJiti } from "jiti";
+
+const jiti = createJiti(import.meta.url);
+const { findNodeCliScript, nodeCliInvocation } = await jiti.import("./node-cli.ts");
+
+const nodeDir = join("opt", "node", "bin");
+const windowsScript = join(nodeDir, "node_modules", "npm", "bin", "npm-cli.js");
+const unixScript = join(nodeDir, "..", "lib", "node_modules", "npm", "bin", "npm-cli.js");
+
+test("both bundled install layouts are probed", () => {
+  assert.equal(findNodeCliScript("npm", { nodeDir, fileExists: (p) => p === windowsScript }), windowsScript);
+  assert.equal(findNodeCliScript("npm", { nodeDir, fileExists: (p) => p === unixScript }), unixScript);
+  assert.equal(findNodeCliScript("npx", { nodeDir, fileExists: () => false }), null);
+});
+
+test("the Windows layout wins when both candidates exist", () => {
+  assert.equal(findNodeCliScript("npx", { nodeDir, fileExists: () => true }), join(nodeDir, "node_modules", "npm", "bin", "npx-cli.js"));
+});
+
+test("npm and npx run through node instead of a .cmd shim", () => {
+  const options = { nodeDir, fileExists: (p) => p === windowsScript };
+  assert.deepEqual(nodeCliInvocation("npm", ["view", "some-pkg", "version", "--json"], options), {
+    command: execPath,
+    args: [windowsScript, "view", "some-pkg", "version", "--json"],
+  });
+});
+
+test("a missing CLI script keeps the bare command and its args", () => {
+  assert.deepEqual(
+    nodeCliInvocation("npx", ["skills", "find", "pdf"], { nodeDir, fileExists: () => false }),
+    { command: "npx", args: ["skills", "find", "pdf"] },
+  );
+});

lib/node-cli.ts

@@ -0,0 +1,66 @@
+import { existsSync } from "fs";
+import { dirname, join } from "path";
+import { execPath } from "process";
+
+export type NodeCliName = "npm" | "npx";
+
+export interface NodeCliLookupOptions {
+  /** Directory holding the running `node` binary. Defaults to the real one. */
+  nodeDir?: string;
+  /** Probe used to test a candidate path, injectable for tests. */
+  fileExists?: (path: string) => boolean;
+}
+
+/**
+ * Locate the `<name>-cli.js` shipped with the running Node.js installation.
+ *
+ * On Windows the `npm`/`npx` on PATH are actually `npm.cmd`/`npx.cmd`, which
+ * Node.js (since 20.12, due to CVE-2024-27980) refuses to spawn from
+ * `execFile`/`spawn` without `shell: true` — the failure mode is a bare
+ * `spawn npm ENOENT` on every call. Going through a shell reintroduces quoting
+ * bugs for user-supplied args. Instead we find the real CLI script and invoke
+ * it directly through the current `node` binary, which works identically on
+ * every platform and needs no shell.
+ */
+export function findNodeCliScript(
+  name: NodeCliName,
+  options: NodeCliLookupOptions = {},
+): string | null {
+  const nodeDir = options.nodeDir ?? dirname(execPath);
+  const fileExists = options.fileExists ?? existsSync;
+  const candidates = [
+    // Windows MSI installer layout: node.exe and node_modules share a dir
+    join(nodeDir, "node_modules", "npm", "bin", `${name}-cli.js`),
+    // Unix layout: .../bin/node + .../lib/node_modules/npm/bin/<name>-cli.js
+    join(nodeDir, "..", "lib", "node_modules", "npm", "bin", `${name}-cli.js`),
+  ];
+  for (const candidate of candidates) {
+    try {
+      if (fileExists(candidate)) return candidate;
+    } catch {
+      // ignore
+    }
+  }
+  return null;
+}
+
+export interface NodeCliInvocation {
+  command: string;
+  args: string[];
+}
+
+/**
+ * `execFile`-spawnable invocation of a Node.js CLI, never routed through a
+ * shell. Falls back to the bare command name when no bundled CLI script is
+ * found, so behavior is unchanged on installs that ship none.
+ */
+export function nodeCliInvocation(
+  name: NodeCliName,
+  args: string[],
+  options: NodeCliLookupOptions = {},
+): NodeCliInvocation {
+  const script = findNodeCliScript(name, options);
+  return script
+    ? { command: execPath, args: [script, ...args] }
+    : { command: name, args };
+}

lib/npx.ts

@@ -1,39 +1,9 @@
 import { execFile } from "child_process";
 import { promisify } from "util";
-import { existsSync } from "fs";
-import { dirname, join } from "path";
-import { execPath } from "process";
+import { nodeCliInvocation } from "./node-cli";
 
 const execFileAsync = promisify(execFile);
 
-/**
- * Locate `npx-cli.js` shipped with the running Node.js installation.
- *
- * On Windows the `npx` on PATH is actually `npx.cmd`, which Node.js (since
- * 20.12 due to CVE-2024-27980) refuses to spawn from `execFile`/`spawn`
- * without `shell: true`. Going through a shell reintroduces quoting bugs for
- * user-supplied args. Instead we find the real `npx-cli.js` and invoke it
- * directly via the current `node` binary, which works identically on every
- * platform and needs no shell.
- */
-function findNpxCli(): string | null {
-  const nodeDir = dirname(execPath);
-  const candidates = [
-    // Windows MSI installer layout: node.exe and node_modules share a dir
-    join(nodeDir, "node_modules", "npm", "bin", "npx-cli.js"),
-    // Unix layout: .../bin/node + .../lib/node_modules/npm/bin/npx-cli.js
-    join(nodeDir, "..", "lib", "node_modules", "npm", "bin", "npx-cli.js"),
-  ];
-  for (const p of candidates) {
-    try {
-      if (existsSync(p)) return p;
-    } catch {
-      // ignore
-    }
-  }
-  return null;
-}
-
 export interface RunNpxOptions {
   timeout?: number;
   cwd?: string;
@@ -48,12 +18,11 @@ export interface RunNpxResult {
 /**
  * Cross-platform wrapper for invoking `npx <args>` without ever using a
  * shell, so user-controlled arguments are never interpreted as shell syntax.
+ * See `lib/node-cli.ts` for why the bundled `npx-cli.js` is preferred over the
+ * `npx` (or `npx.cmd`) on PATH.
  */
 export async function runNpx(args: string[], opts: RunNpxOptions = {}): Promise<RunNpxResult> {
-  const npxCli = findNpxCli();
-  const { command, commandArgs } = npxCli
-    ? { command: execPath, commandArgs: [npxCli, ...args] }
-    : { command: "npx", commandArgs: args };
+  const { command, args: commandArgs } = nodeCliInvocation("npx", args);
   return execFileAsync(command, commandArgs, {
     timeout: opts.timeout,
     cwd: opts.cwd,

lib/plugin-updates.ts

@@ -9,6 +9,7 @@ import {
 } from "@earendil-works/pi-coding-agent";
 import { gt, maxSatisfying, rcompare, valid, validRange } from "semver";
 import type { PluginScope, PluginUpdateResult } from "@/lib/api-types";
+import { nodeCliInvocation } from "./node-cli";
 import { getProjectTrustStatus } from "./project-trust";
 
 const execFileAsync = promisify(execFile);
@@ -97,7 +98,10 @@ async function runCommand(
   args: string[],
   options: { cwd: string; env?: NodeJS.ProcessEnv },
 ): Promise<string> {
-  const { stdout } = await execFileAsync(command, args, {
+  // A bare `npm` resolves to `npm.cmd` on Windows, which `execFile` cannot
+  // spawn (CVE-2024-27980); run the bundled `npm-cli.js` through `node`.
+  const invocation = command === "npm" ? nodeCliInvocation("npm", args) : { command, args };
+  const { stdout } = await execFileAsync(invocation.command, invocation.args, {
     cwd: options.cwd,
     env: options.env ? { ...process.env, ...options.env } : process.env,
     encoding: "utf8",