Repository navigation
fix(security): skip npm lifecycle scripts when installing skill deps - #752
Merged
Merged
Conversation
…751) installScriptDependencies ran a plain `npm install` in an installed skill's scripts/ directory, so preinstall/postinstall scripts from an untrusted skill (or its dependencies) executed at install time. Pass --ignore-scripts so dependencies are fetched without running code. The regression test overrides npm_config_ignore_scripts because this repo's .npmrc sets it and npm exports it to child processes, which would otherwise mask the bug under `npm test`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--ignore-scripts alone does not stop a skill-shipped scripts/.npmrc from setting `git=<path>`; with a git dependency, npm would then execute the skill's script as its git binary. Pass --git=git on the CLI, which outranks project .npmrc, and add a regression test for it. Also make the lifecycle regression test write its marker from a mark.js file instead of interpolating the path into a shell string, so it stays meaningful on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a git dependency has a prepare script, pacote runs a nested `npm install` in its clone. That child does not get the parent's CLI flags and reads the dependency's own .npmrc, so `git=<path>` there was still honoured. Pass npm_config_git=git and npm_config_ignore_scripts=true in the runner env: children inherit it, and env outranks project .npmrc. Adds an offline regression test using local git+file:// repos. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A skill's scripts/.npmrc can set node-options=--require ./evil.js, which npm exports as NODE_OPTIONS to the nested npm that pacote spawns to prepare a git dependency, so code from the dep runs despite --ignore-scripts. Refuse git deps with --allow-git=none (argv and env), and keep --node-options= and --git=git as a backstop for npm versions that ignore allow-git. An EALLOWGIT failure now says git dependencies in scripts/package.json are not allowed. Adds an offline git+file:// regression test and applies prettier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A skill's scripts/.npmrc with global=true or location=global turned the hardened install into a global one, linking a skill-chosen bin (e.g. a fake `git`) into the user's global prefix on PATH. npm ignores prefix from project config, so that prefix is the user's real one. Pass --global=false --location=user (argv and env); each one alone leaves one of the two settings open. The node-options test now asserts the EALLOWGIT rejection only when the local npm defines allow-git (npm 10 on Node 22 ignores the flag) and always asserts the marker is never written, which --node-options= holds on npm 10. The new global-install tests pin npm_config_prefix to the temp dir so a regression can never link into the real prefix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #751
Problem
installScriptDependencies(src/installer-core.ts) runsnpm installin<skill>/scripts/whenever the installed skill shipsscripts/package.json, after everyasm install. npm then runs thepreinstall/install/postinstall/preparescripts of that package and its dependencies. As a result, installing an untrusted skill executed author-controlled code before the user had reviewed or run anything.Review found that
--ignore-scriptsalone is not enough. A skill can ship its ownscripts/.npmrc, which npm reads as project config, and use other npm settings to run code or plant executables.Fix
The install now runs:
The same settings are also passed as
npm_config_*environment variables. Nested npm processes, for example the one pacote spawns to prepare a git dependency, inherit the environment but not the CLI flags.--ignore-scriptsscripts/package.jsonand its dependencies--git=git.npmrcgit=./evil.shswapping npm's git binarynpm_config_git.npmrc(nested install)--allow-git=none.npmrccan hijack--node-options=.npmrcnode-options=--require ./evil.jsreaching the nested npm (backstop for npm versions withoutallow-git)--global=false --location=user.npmrcglobal=true/location=globalturning the install global and linking skill bins (e.g. a fakegit) onto PATHBehavior change: a skill whose
scripts/package.jsonhas git dependencies no longer installs.asm installfails withgit dependencies in scripts/package.json are not allowed. npm versions withoutallow-git(e.g. npm 10) ignore that flag; there the--node-options=and--git=gitbackstops still block code execution.Trade-off: dependencies that need a native build step during install (e.g.
node-gypaddons) won't compile at install time. That is the right default for untrusted content. A user who trusts a skill can runnpm rebuildin itsscripts/directory.Decision Record
npm installon untrusted skill content runs lifecycle scripts, and it also honours a skill-shippedscripts/.npmrc. Several of its keys (git,node-options,global/location) lead to code execution or planted executables even with--ignore-scripts.--ignore-scriptsonly; (b) pin each dangerous config key on the CLI and in the env; (c) refuse git dependencies (--allow-git=none) plus CLI/env pins as a backstop; (d) make thescripts/dependency install opt-in, or run npm with isolated config..npmrc; each bypass was reproduced on npm 11.19. (d) is a larger design change, deferred..npmrckeys may still redirect fetches (registry, proxy, strict-ssl), though none is known to run code. A skill can also ship symlinks inscripts/(e.g.package-lock.jsonpointing at an absolute path) that npm writes through. This predates the PR and is tracked in fix(security): skill install writes through symlinks shipped in scripts/ #753.Acceptance Criteria Verification
#751 defines no acceptance-criteria section. Its stated goal, that installing a skill must not execute code from it, is checked with real-npm regression tests:
does not run lifecycle scripts from the skill's scripts/package.json.npmrcgit swapdoes not let a skill-supplied scripts/.npmrc swap npm's git binary.npmrcdoes not let a nested git dependency's .npmrc swap npm's git binary.npmrcnode-optionsdoes not let a skill's scripts/.npmrc inject node-options into a nested git dep install.npmrcglobal / locationdoes not let a skill's scripts/.npmrc (%s) make the install global(global=true,location=global; prefix pinned to the temp dir)Tests
npm run typecheck✅, prettier ✅,CI=truefull unit suite 2989/2989 ✅. TheinstallScriptDependenciestests pass on npm 11.19 and npm 10.9.9.npm_config_ignore_scripts=falsebecause this repo's.npmrcsetsignore-scripts=true, andnpm/npxexport that to child processes, which would otherwise mask the bug undernpm test.🤖 Generated with Claude Code