diff --git a/scripts/install-clients.mjs b/scripts/install-clients.mjs index 815ef2da2..244c21dcd 100644 --- a/scripts/install-clients.mjs +++ b/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(); +} diff --git a/scripts/install-clients.test.mjs b/scripts/install-clients.test.mjs new file mode 100644 index 000000000..9c2ac28ac --- /dev/null +++ b/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 }); + } +});