fix(security): skip npm lifecycle scripts when installing skill deps

src/installer-core.ts

@@ -622,11 +622,51 @@ export async function installScriptDependencies(
 
   debug(`install: installing script dependencies in ${scriptsDir}`);
   try {
-    await runner("npm", ["install"], { cwd: scriptsDir, timeout: 120_000 });
+    // --ignore-scripts: skill content is untrusted, so installing it must not
+    // run lifecycle scripts from its package.json or its dependencies.
+    // --git=git: a skill-supplied scripts/.npmrc is read as project config and
+    // could point `git` at a script it ships; CLI flags outrank .npmrc.
+    // env: nested npm installs (pacote preparing git deps) don't get the CLI
+    // flags but do inherit env, which also outranks their project .npmrc.
+    // --allow-git=none: git deps are refused outright, because preparing one
+    // spawns a nested npm that a skill's .npmrc can hijack (node-options, git).
+    // Older npm ignores the unknown flag, so --node-options= and --git=git
+    // stay as a backstop.
+    // --global=false --location=user: a skill's .npmrc (global=true or
+    // location=global) could otherwise turn this into a global install that
+    // links its bins, e.g. a fake `git`, into the user's prefix on PATH.
+    await runner(
+      "npm",
+      [
+        "install",
+        "--ignore-scripts",
+        "--git=git",
+        "--allow-git=none",
+        "--node-options=",
+        "--global=false",
+        "--location=user",
+      ],
+      {
+        cwd: scriptsDir,
+        timeout: 120_000,
+        env: {
+          ...process.env,
+          npm_config_git: "git",
+          npm_config_ignore_scripts: "true",
+          npm_config_allow_git: "none",
+          npm_config_global: "false",
+          npm_config_location: "user",
+        },
+      },
+    );
   } catch (err: unknown) {
     const execErr = err as ExecError;
+    const detail = execErr.stderr || execErr.message;
+    const hint = detail?.includes("EALLOWGIT")
+      ? " (git dependencies in scripts/package.json are not allowed)"
+      : "";
     throw new Error(
-      `Installed skill, but failed to install dependencies in scripts/: ${execErr.stderr || execErr.message}`,
+      `Installed skill, but failed to install dependencies in scripts/${hint}: ${detail}`,
       { cause: err },
     );
   }

src/installer.test.ts

@@ -8,8 +8,10 @@ import {
   rm,
   readlink,
   lstat,
+  chmod,
 } from "fs/promises";
 import { existsSync } from "fs";
+import { execFileSync } from "child_process";
 import { join, relative, resolve, isAbsolute, basename, dirname } from "path";
 import { tmpdir } from "os";
 import {
@@ -1925,12 +1927,280 @@ describe("installScriptDependencies", () => {
     await installScriptDependencies(tempDir, runner as any);
 
     expect(runner).toHaveBeenCalledTimes(1);
-    expect(runner).toHaveBeenCalledWith("npm", ["install"], {
-      cwd: scriptsDir,
-      timeout: 120_000,
-    });
+    expect(runner).toHaveBeenCalledWith(
+      "npm",
+      [
+        "install",
+        "--ignore-scripts",
+        "--git=git",
+        "--allow-git=none",
+        "--node-options=",
+        "--global=false",
+        "--location=user",
+      ],
+      {
+        cwd: scriptsDir,
+        timeout: 120_000,
+        env: expect.objectContaining({
+          npm_config_git: "git",
+          npm_config_ignore_scripts: "true",
+          npm_config_allow_git: "none",
+          npm_config_global: "false",
+          npm_config_location: "user",
+        }),
+      },
+    );
   });
 
+  test("does not run lifecycle scripts from the skill's scripts/package.json", async () => {
+    const scriptsDir = join(tempDir, "scripts");
+    await mkdir(scriptsDir, { recursive: true });
+    const marker = join(tempDir, "postinstall-ran");
+    // Write the marker from a file so no path is interpolated into a shell
+    // string (Windows backslashes would otherwise be mangled).
+    await writeFile(
+      join(scriptsDir, "mark.js"),
+      `require("fs").writeFileSync(${JSON.stringify(marker)}, "");\n`,
+    );
+    await writeFile(
+      join(scriptsDir, "package.json"),
+      JSON.stringify({
+        name: "evil-skill-scripts",
+        version: "1.0.0",
+        scripts: {
+          preinstall: "node mark.js",
+          postinstall: "node mark.js",
+        },
+      }),
+    );
+
+    // This repo's .npmrc sets ignore-scripts=true, which npm/npx export to
+    // child processes; override it so only the flag under test can block scripts.
+    vi.stubEnv("npm_config_ignore_scripts", "false");
+    try {
+      await installScriptDependencies(tempDir);
+    } finally {
+      vi.unstubAllEnvs();
+    }
+
+    await expect(access(marker)).rejects.toThrow();
+  });
+
+  test.skipIf(process.platform === "win32")(
+    "does not let a skill-supplied scripts/.npmrc swap npm's git binary",
+    async () => {
+      const scriptsDir = join(tempDir, "scripts");
+      await mkdir(scriptsDir, { recursive: true });
+      const marker = join(tempDir, "fake-git-ran");
+      const fakeGit = join(scriptsDir, "evil.sh");
+      await writeFile(
+        fakeGit,
+        `#!/bin/sh\ntouch ${JSON.stringify(marker)}\nexit 1\n`,
+      );
+      await chmod(fakeGit, 0o755);
+      await writeFile(join(scriptsDir, ".npmrc"), `git=${fakeGit}\n`);
+      await writeFile(
+        join(scriptsDir, "package.json"),
+        JSON.stringify({
+          name: "evil-skill-scripts",
+          version: "1.0.0",
+          dependencies: { x: "git+https://example.invalid/a/b.git" },
+        }),
+      );
+
+      // The git dependency cannot resolve, so the install itself fails.
+      await expect(installScriptDependencies(tempDir)).rejects.toThrow();
+
+      await expect(access(marker)).rejects.toThrow();
+    },
+  );
+
+  test.skipIf(process.platform === "win32")(
+    "does not let a nested git dependency's .npmrc swap npm's git binary",
+    async () => {
+      const marker = join(tempDir, "nested-fake-git-ran");
+      const fakeGit = join(tempDir, "evil.sh");
+      await writeFile(
+        fakeGit,
+        `#!/bin/sh\ntouch ${JSON.stringify(marker)}\nexit 1\n`,
+      );
+      await chmod(fakeGit, 0o755);
+
+      const commitRepo = async (dir: string, files: Record<string, string>) => {
+        await mkdir(dir, { recursive: true });
+        for (const [name, content] of Object.entries(files)) {
+          await writeFile(join(dir, name), content);
+        }
+        const git = (...args: string[]) =>
+          execFileSync(
+            "git",
+            ["-c", "user.name=t", "-c", "user.email=t@t", ...args],
+            { cwd: dir, stdio: "ignore" },
+          );
+        git("init", "-q");
+        git("add", ".");
+        git("commit", "-q", "--no-gpg-sign", "-m", "init");
+      };
+
+      // inner: a plain git dependency of outer.
+      const innerDir = join(tempDir, "inner");
+      await commitRepo(innerDir, {
+        "package.json": JSON.stringify({ name: "inner", version: "1.0.0" }),
+      });
+      // outer: has a prepare script, so pacote runs a nested `npm install`
+      // inside its clone, which reads outer's .npmrc as project config.
+      const outerDir = join(tempDir, "outer");
+      await commitRepo(outerDir, {
+        "package.json": JSON.stringify({
+          name: "outer",
+          version: "1.0.0",
+          scripts: { prepare: "node -e 0" },
+          dependencies: { inner: `git+file://${innerDir}` },
+        }),
+        ".npmrc": `git=${fakeGit}\n`,
+      });
+
+      const scriptsDir = join(tempDir, "skill", "scripts");
+      await mkdir(scriptsDir, { recursive: true });
+      await writeFile(
+        join(scriptsDir, "package.json"),
+        JSON.stringify({
+          name: "evil-skill-scripts",
+          version: "1.0.0",
+          dependencies: { outer: `git+file://${outerDir}` },
+        }),
+      );
+
+      // Mirror the lifecycle test: drop the repo's inherited ignore-scripts so
+      // only the env passed by installScriptDependencies can harden children.
+      vi.stubEnv("npm_config_ignore_scripts", "false");
+      try {
+        await installScriptDependencies(join(tempDir, "skill")).catch(() => {});
+      } finally {
+        vi.unstubAllEnvs();
+      }
+
+      await expect(access(marker)).rejects.toThrow();
+    },
+    30_000,
+  );
+
+  test.skipIf(process.platform === "win32")(
+    "does not let a skill's scripts/.npmrc inject node-options into a nested git dep install",
+    async () => {
+      const marker = join(tempDir, "node-options-ran");
+
+      // outer: a git dependency with a prepare script, so preparing it would
+      // spawn a nested `npm install` in its clone, where evil.js lives.
+      const outerDir = join(tempDir, "outer");
+      await mkdir(outerDir, { recursive: true });
+      await writeFile(
+        join(outerDir, "package.json"),
+        JSON.stringify({
+          name: "outer",
+          version: "1.0.0",
+          scripts: { prepare: "node -e 0" },
+        }),
+      );
+      await writeFile(
+        join(outerDir, "evil.js"),
+        `require("fs").writeFileSync(${JSON.stringify(marker)}, "");\n`,
+      );
+      const git = (...args: string[]) =>
+        execFileSync(
+          "git",
+          ["-c", "user.name=t", "-c", "user.email=t@t", ...args],
+          { cwd: outerDir, stdio: "ignore" },
+        );
+      git("init", "-q");
+      git("add", ".");
+      git("commit", "-q", "--no-gpg-sign", "-m", "init");
+
+      const scriptsDir = join(tempDir, "skill", "scripts");
+      await mkdir(scriptsDir, { recursive: true });
+      await writeFile(
+        join(scriptsDir, ".npmrc"),
+        "node-options=--require ./evil.js\n",
+      );
+      await writeFile(
+        join(scriptsDir, "package.json"),
+        JSON.stringify({
+          name: "evil-skill-scripts",
+          version: "1.0.0",
+          dependencies: { outer: `git+file://${outerDir}` },
+        }),
+      );
+
+      // npm without allow-git (e.g. npm 10 on Node 22) ignores the flag, so the git dep
+      // installs; there --node-options= alone must keep evil.js from running.
+      const allowGitSupported =
+        "allow-git" in
+        JSON.parse(
+          execFileSync("npm", ["config", "ls", "-l", "--json"], {
+            cwd: tempDir,
+            encoding: "utf8",
+          }),
+        );
+
+      vi.stubEnv("npm_config_ignore_scripts", "false");
+      try {
+        const install = installScriptDependencies(join(tempDir, "skill"));
+        if (allowGitSupported) {
+          await expect(install).rejects.toThrow(
+            "git dependencies in scripts/package.json",
+          );
+        } else {
+          await install;
+        }
+      } finally {
+        vi.unstubAllEnvs();
+      }
+
+      await expect(access(marker)).rejects.toThrow();
+    },
+    30_000,
+  );
+
+  test
+    .skipIf(process.platform === "win32")
+    .each(["global=true", "location=global"])(
+    "does not let a skill's scripts/.npmrc (%s) make the install global",
+    async (setting) => {
+      const scriptsDir = join(tempDir, "scripts");
+      await mkdir(scriptsDir, { recursive: true });
+      const prefix = join(tempDir, "prefix");
+      await writeFile(
+        join(scriptsDir, ".npmrc"),
+        `${setting}\nforce=true\nprefix=${prefix}\n`,
+      );
+      await writeFile(join(scriptsDir, "fake-git.sh"), "#!/bin/sh\n");
+      await writeFile(
+        join(scriptsDir, "package.json"),
+        JSON.stringify({
+          name: "evil-skill-scripts",
+          version: "1.0.0",
+          bin: { git: "fake-git.sh" },
+        }),
+      );
+
+      vi.stubEnv("npm_config_ignore_scripts", "false");
+      // npm ignores prefix from project config, so a global install would land
+      // in the real global prefix (npm/npx also export it as npm_config_prefix).
+      // Pin it to tempDir so a regression can only link into the sandbox.
+      vi.stubEnv("npm_config_prefix", prefix);
+      vi.stubEnv("npm_config_global_prefix", prefix);
+      try {
+        await installScriptDependencies(tempDir).catch(() => {});
+      } finally {
+        vi.unstubAllEnvs();
+      }
+
+      expect(existsSync(join(prefix, "bin", "git"))).toBe(false);
+      expect(existsSync(join(prefix, "lib", "node_modules"))).toBe(false);
+    },
+    30_000,
+  );
+
   test("skips npm install when only unrelated package.json exists", async () => {
     await writeFile(join(tempDir, "package.json"), '{"dependencies":{}}\n');
     const runner = vi.fn().mockResolvedValue({ stdout: "", stderr: "" });