fix(install): strip npm_config_allow_scripts from the per-client installs

scripts/install-clients.mjs

@@ -19,47 +19,87 @@
  *
  * Set INSPECTOR_SKIP_CLIENT_INSTALL=1 to skip explicitly — e.g. CI that
  * installs each client itself and wants to avoid the redundant pass.
+ *
+ * The child installs run with `npm_config_allow_scripts` stripped from their
+ * environment (#2518). npm exports every non-default config value to lifecycle
+ * scripts as `npm_config_*`, whichever npmrc set it, and a child npm reads that
+ * env var as if it were `--allow-scripts` on the command line — which npm
+ * 11.19+ rejects outright in a project install (EALLOWSCRIPTS). So a user with
+ * `allow-scripts` in their ~/.npmrc could install the root and then fail on
+ * the first client. Stripping it loses nothing: each child npm reads the same
+ * npmrc files itself, as npmrc, where the setting is allowed.
  */
 
 import { spawnSync } from "node:child_process";
-import { existsSync } from "node:fs";
+import { existsSync, realpathSync } from "node:fs";
 import { dirname, join, resolve, sep } from "node:path";
 import { fileURLToPath } from "node:url";
 
 const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
 const CLIENTS = ["web", "cli", "mcpdo", "tui", "launcher"];
 
-if (process.env.INSPECTOR_SKIP_CLIENT_INSTALL) {
-  console.log(
-    "[install-clients] INSPECTOR_SKIP_CLIENT_INSTALL set — skipping client installs.",
-  );
-  process.exit(0);
+/**
+ * The environment for a per-client `npm install`: the parent's, minus the
+ * config npm refuses to receive through the environment in a project install.
+ * npm matches `npm_config_*` case-insensitively, so the strip does too.
+ * @param {NodeJS.ProcessEnv} env
+ * @returns {NodeJS.ProcessEnv}
+ */
+export function childInstallEnv(env) {
+  const next = { ...env };
+  for (const key of Object.keys(next)) {
+    if (key.toLowerCase() === "npm_config_allow_scripts") {
+      delete next[key];
+    }
+  }
+  return next;
 }
 
-// Installed as a dependency (lives under node_modules) → nothing to do.
-if (repoRoot.split(sep).includes("node_modules")) {
-  process.exit(0);
-}
+function main() {
+  if (process.env.INSPECTOR_SKIP_CLIENT_INSTALL) {
+    console.log(
+      "[install-clients] INSPECTOR_SKIP_CLIENT_INSTALL set — skipping client installs.",
+    );
+    process.exit(0);
+  }
 
-for (const name of CLIENTS) {
-  const dir = join(repoRoot, "clients", name);
-  // The published tarball ships build/ output only, no client package.json.
-  if (!existsSync(join(dir, "package.json"))) {
-    continue;
+  // Installed as a dependency (lives under node_modules) → nothing to do.
+  if (repoRoot.split(sep).includes("node_modules")) {
+    process.exit(0);
   }
-  console.log(
-    `[install-clients] Installing dependencies for clients/${name}...`,
-  );
-  const result = spawnSync("npm", ["install"], {
-    cwd: dir,
-    stdio: "inherit",
-    // npm is npm.cmd on Windows, which needs a shell to resolve.
-    shell: process.platform === "win32",
-  });
-  if (result.status !== 0) {
-    console.error(
-      `[install-clients] npm install failed in clients/${name} (exit ${result.status ?? "unknown"}).`,
+
+  for (const name of CLIENTS) {
+    const dir = join(repoRoot, "clients", name);
+    // The published tarball ships build/ output only, no client package.json.
+    if (!existsSync(join(dir, "package.json"))) {
+      continue;
+    }
+    console.log(
+      `[install-clients] Installing dependencies for clients/${name}...`,
     );
-    process.exit(result.status ?? 1);
+    const result = spawnSync("npm", ["install"], {
+      cwd: dir,
+      stdio: "inherit",
+      env: childInstallEnv(process.env),
+      // npm is npm.cmd on Windows, which needs a shell to resolve.
+      shell: process.platform === "win32",
+    });
+    if (result.status !== 0) {
+      console.error(
+        `[install-clients] npm install failed in clients/${name} (exit ${result.status ?? "unknown"}).`,
+      );
+      process.exit(result.status ?? 1);
+    }
   }
 }
+
+// Compare real paths on both sides. By default Node resolves symlinks for
+// import.meta.url but not for argv[1], and --preserve-symlinks-main reverses
+// that, so a checkout reached through a symlink (macOS /var -> /private/var)
+// would otherwise skip the cascade silently.
+if (
+  process.argv[1] &&
+  realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url))
+) {
+  main();
+}

scripts/install-clients.test.mjs

@@ -0,0 +1,239 @@
+/**
+ * Tests for scripts/install-clients.mjs — the root postinstall cascade.
+ *
+ * The unit cases pin `childInstallEnv`. The end-to-end cases run the real
+ * script and the real npm against a throwaway repo layout, because #2518 is
+ * an interaction between two npm processes: the parent exports a user's
+ * npmrc `allow-scripts` as `npm_config_allow_scripts`, and npm 11.19+ refuses
+ * that env var in a project install. Only running npm shows the cascade
+ * surviving it.
+ */
+
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import {
+  copyFileSync,
+  mkdirSync,
+  mkdtempSync,
+  realpathSync,
+  rmSync,
+  symlinkSync,
+  writeFileSync,
+} from "node:fs";
+import { tmpdir } from "node:os";
+import { delimiter, dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+import { childInstallEnv } from "./install-clients.mjs";
+
+const SCRIPT = join(
+  dirname(fileURLToPath(import.meta.url)),
+  "install-clients.mjs",
+);
+
+test("childInstallEnv strips npm_config_allow_scripts", () => {
+  const env = childInstallEnv({
+    PATH: "/bin",
+    npm_config_allow_scripts: "esbuild",
+  });
+  assert.deepEqual(env, { PATH: "/bin" });
+});
+
+test("childInstallEnv strips the key whatever its case, as npm reads it", () => {
+  const env = childInstallEnv({
+    NPM_CONFIG_ALLOW_SCRIPTS: "a",
+    Npm_Config_Allow_Scripts: "b",
+  });
+  assert.deepEqual(env, {});
+});
+
+test("childInstallEnv keeps every other npm config and does not mutate its input", () => {
+  const input = {
+    npm_config_allow_scripts: "esbuild",
+    npm_config_registry: "https://registry.example/",
+    npm_config_allow_scripts_pin: "true",
+  };
+  const env = childInstallEnv(input);
+  assert.deepEqual(env, {
+    npm_config_registry: "https://registry.example/",
+    npm_config_allow_scripts_pin: "true",
+  });
+  assert.equal(input.npm_config_allow_scripts, "esbuild");
+});
+
+/**
+ * A repo-shaped temp dir: scripts/install-clients.mjs plus one dependency-free
+ * client, and a HOME whose .npmrc sets allow-scripts — the reporter's setup.
+ */
+function fixture() {
+  // Canonical, so a symlinked tmpdir (macOS /var) can't make the plain case
+  // exercise the symlink path by accident; that has its own case below.
+  const root = realpathSync(mkdtempSync(join(tmpdir(), "install-clients-")));
+  mkdirSync(join(root, "scripts"));
+  copyFileSync(SCRIPT, join(root, "scripts", "install-clients.mjs"));
+  const client = join(root, "clients", "web");
+  mkdirSync(client, { recursive: true });
+  writeFileSync(
+    join(client, "package.json"),
+    JSON.stringify({ name: "fixture-web", version: "1.0.0", private: true }),
+  );
+  const home = join(root, "home");
+  mkdirSync(home);
+  writeFileSync(join(home, ".npmrc"), "allow-scripts=esbuild\n");
+  return { root, client, home };
+}
+
+/** The env a parent npm hands its postinstall when ~/.npmrc sets allow-scripts. */
+function parentNpmEnv(home) {
+  const env = childInstallEnv(process.env);
+  for (const key of Object.keys(env)) {
+    if (key.toLowerCase().startsWith("npm_config_")) delete env[key];
+  }
+  return {
+    ...env,
+    HOME: home,
+    USERPROFILE: home,
+    npm_config_allow_scripts: "esbuild",
+    npm_config_offline: "true",
+    npm_config_audit: "false",
+    npm_config_fund: "false",
+    npm_config_update_notifier: "false",
+  };
+}
+
+const npmCmd = { shell: process.platform === "win32", encoding: "utf8" };
+
+const npmVersion = spawnSync("npm", ["--version"], npmCmd).stdout.trim();
+const [major, minor] = npmVersion.split(".").map(Number);
+const npmGatesAllowScripts = major > 11 || (major === 11 && minor >= 19);
+
+test(
+  "premise: npm rejects allow-scripts arriving through the environment",
+  { skip: !npmGatesAllowScripts && `npm ${npmVersion} predates the gate` },
+  () => {
+    const { root, client, home } = fixture();
+    try {
+      const result = spawnSync("npm", ["install"], {
+        ...npmCmd,
+        cwd: client,
+        env: parentNpmEnv(home),
+      });
+      assert.notEqual(result.status, 0);
+      assert.match(result.stderr, /EALLOWSCRIPTS/);
+    } finally {
+      rmSync(root, { recursive: true, force: true });
+    }
+  },
+);
+
+/** Run the fixture's copy of the script the way npm runs the postinstall. */
+function runCascade(script, cwd, home, nodeArgs = []) {
+  // process.execPath is directly executable everywhere, so no shell: on
+  // Windows a shell would re-split a temp path containing a space.
+  return spawnSync(process.execPath, [...nodeArgs, script], {
+    encoding: "utf8",
+    cwd,
+    env: parentNpmEnv(home),
+  });
+}
+
+test("the cascade installs a client when the parent npm exported allow-scripts", () => {
+  const { root, home } = fixture();
+  try {
+    const result = runCascade(
+      join(root, "scripts", "install-clients.mjs"),
+      root,
+      home,
+    );
+    assert.equal(result.status, 0, result.stderr);
+    assert.match(result.stdout, /Installing dependencies for clients\/web/);
+    assert.doesNotMatch(result.stderr, /EALLOWSCRIPTS/);
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});
+
+for (const nodeArgs of [[], ["--preserve-symlinks-main"]]) {
+  const how = nodeArgs.length ? ` (${nodeArgs.join(" ")})` : "";
+  test(`the cascade still runs when the checkout is reached through a symlink${how}`, () => {
+    const { root, home } = fixture();
+    const linkParent = mkdtempSync(join(tmpdir(), "install-clients-link-"));
+    const link = join(linkParent, "checkout");
+    try {
+      // A junction needs no privilege on Windows; the type is ignored elsewhere.
+      symlinkSync(root, link, "junction");
+      const result = runCascade(
+        join(link, "scripts", "install-clients.mjs"),
+        link,
+        home,
+        nodeArgs,
+      );
+      assert.equal(result.status, 0, result.stderr);
+      assert.match(result.stdout, /Installing dependencies for clients\/web/);
+    } finally {
+      rmSync(linkParent, { recursive: true, force: true });
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+}
+
+/**
+ * A stand-in `npm` on PATH that fails, as npm 11.19+ does, when
+ * allow-scripts reaches it through the environment. It pins the spawn wiring
+ * whatever npm the runner has: CI's Node 22 bundles npm 10, which predates
+ * the gate, so against the real npm there the cascade passes with or without
+ * the strip.
+ */
+function stubNpm() {
+  const dir = mkdtempSync(join(tmpdir(), "install-clients-npm-"));
+  const js = join(dir, "npm-stub.cjs");
+  writeFileSync(
+    js,
+    `const hit = Object.keys(process.env).some(
+  (k) => k.toLowerCase() === "npm_config_allow_scripts",
+);
+if (hit) {
+  console.error("npm error code EALLOWSCRIPTS");
+  process.exit(1);
+}
+`,
+  );
+  writeFileSync(
+    join(dir, "npm"),
+    `#!/bin/sh\nexec "${process.execPath}" "${js}" "$@"\n`,
+    { mode: 0o755 },
+  );
+  writeFileSync(join(dir, "npm.cmd"), `@"${process.execPath}" "${js}" %*\r\n`);
+  return dir;
+}
+
+test("the cascade hands npm an environment without allow-scripts (stub npm)", () => {
+  const { root, home } = fixture();
+  const npmDir = stubNpm();
+  try {
+    const env = {
+      ...parentNpmEnv(home),
+      PATH: `${npmDir}${delimiter}${process.env.PATH ?? ""}`,
+    };
+    // The stub itself must reject the variable, or this case proves nothing.
+    const probe = spawnSync("npm", ["install"], {
+      ...npmCmd,
+      cwd: root,
+      env,
+    });
+    assert.equal(probe.status, 1);
+    assert.match(probe.stderr, /EALLOWSCRIPTS/);
+
+    const result = spawnSync(
+      process.execPath,
+      [join(root, "scripts", "install-clients.mjs")],
+      { encoding: "utf8", cwd: root, env },
+    );
+    assert.equal(result.status, 0, result.stderr);
+    assert.match(result.stdout, /Installing dependencies for clients\/web/);
+  } finally {
+    rmSync(npmDir, { recursive: true, force: true });
+    rmSync(root, { recursive: true, force: true });
+  }
+});