Skip to content
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

Change the default save-prefix from ^ to none #4227

Closed
wants to merge 2 commits into from
Closed

Conversation

zkldi
Copy link

@zkldi zkldi commented Jan 10, 2022

TLDR

This PR changes the default behaviour of NPM to not save package versions with the ^ prefix.
Instead, packages would be saved with no prefix.

As a TL;DR, the implications of this is that all dependencies will be pinned to the version you installed by default.

Changing This Yourself

If you're reading this PR and want to apply it to your own environments, invoke npm config set save-prefix=''.

Existing Behaviour

The ^1.0.0 range means that NPM will accept any package that is 1.0.0 or greater in the MINOR or PATCH field. This means 1.0.2 and 1.2.0 are legal matches, but 2.0.0 is not.

Since packages on NPM are expected to follow semver, this has the presumed intention of automatically opting users into security patches. For details I'll go into in a minute, there are no actual security benefits to this design decision.

NPMs design has changed a lot since its original iteration, and I feel it is important to note that this decision was made back in npmv1 (or older). This means this design decision predates package-lock.json, --save being the default, and more.

Why change it?

There are massive security implications for this default. Semver is not enforceable, and what constitutes a PATCH/MINOR to one person might be a breaking change to another.

As an example, mongodb's 3.6.4 update caused monk to endlessly spit out warnings.

This can have pretty serious implications for people using NPM in production. Blindly running npm install on a different machine can result in different dependencies being installed to your local dev version! Sure, you installed 3.6.3, but ^3.6.3 means that npm is perfectly within its right to get 3.6.4. (n.b. npm ci should be used to avoid this behaviour, but why is the default dangerous?)

Although package-lock.json exists to solve this, package-lock.json is regenerated whenever an option touches package.json or node_modules. This means that npm installing another module will result in all of your ^x.x.x packages being bumped aswell.

Security Implications

If a library is compromised somehow, the author can publish a PATCH/MINOR version that performs malicious actions. Given that semver is not enforceable (NPM cannot possibly say -- hey, this change is too major for a patch release), this means that the compromised library can essentially do whatever it wants to almost every single one of its dependents.

Since this happens automatically as the result of doing other things (i.e. anything that interacts with package.json), this means even installing a new, entirely separate package can result in the compromised library getting exec access.

Scope for attack

NPM is based around micropackages. For any given package, there are likely to be hundreds or even thousands of tiny packages holding it up. Every single one of those libraries is a target for attackers, as compromising any of the ones that make up a larger package can result in full code execution on millions of machines or applications.

Furthermore, NPM does not enforce 2FA on accounts of package authors. If a package authors password is leaked online, they could have their account hijacked. Although this would be bad on its own, the fact that they can then upload malware that near-automatically propogates across the entire ecosystem moves the scope from bad to incredibly bad.

While changing the default from ^ to nothing does not fix all of these issues, it would stunt the spread of malicious patch updates, as it would only affect users newly directly installing the package, rather than installing any package that depends on it. This moves the difficulty up (in theory) from compromising any micropackage to compromising a larger package (say, react) in order to get such an impact.

Examples

While colors.js is the example everyone is currently thinking about, this is not even close to the only time this has happened.

Here is a list of just some packages in recent memory that have leveraged this default behaviour to perform large scale attacks.

Arguing against 'automatic patching'

A potential retort to this would be that the current behaviour causes users to get potential security patches automatically. However, this is not really the case.

Consider the case where a user has a bunch of packages, and foobar@1.0.1 has a security vulnerability.
The user's package json has "foobar": "^1.0.0".
A new version of foobar is released to deal with this security vulnerabilty.

The user is not safe in any way against their vulnerability until they do anything that interacts with their package.json. When they do, it is possible that npm will update the package to the latest patch version available, but it is not required to do so. If they have another dependency that perhaps isn't as lenient with its version of foobar, npm will not automatically apply the security patch.

Furthermore, this automatic application of the latest version does not update package.json. It would be perfectly legal for NPM to install 1.0.1 again, there's absolutely no guarantee that that would happen.

The amount of edge cases and scenarios involved with hoping npm pulls the right version out of a set of versions that do contain significant security vulnerabilities is not worth it. You should explicitly state that you want the security patch and explicitly have it in your package.json that you do not ever want version v1.0.1.

A tool like npm audit is already invoked on almost every operation. Even if it is critically flawed, it is still a useful way of alerting users to security issues that affect them, and encourage them to update their version of a package.

Conclusion

In my opinion, the default behaviour should always be pinning dependencies to the exact version. Updating packages should be opt-in, not opt-out.

This patch changes the default save-prefix from '^' to ''.

At the moment, there are some massive security implications of using '^'
as the default save directive. Most people will not change this default,
and that means that any dependency of yours can push a malicious PATCH
version, and you will automatically update to it when you next call npm
install.

There are no benefits to having ^ be the default, only security issues.
Contrary to popular belief, it does not mean that you automatically opt
into security patches.
@zkldi zkldi requested a review from a team as a code owner January 10, 2022 07:50
@zkldi
Copy link
Author

zkldi commented Jan 10, 2022

Note: I think an entirely unrelated test is failing in this PR. It might just be on my local machine. Not sure what the test is about but it's this one inside test/lib/cli.js:

 t.ok(process.title.startsWith('npm version https://username:***@npmjs.org'))

@ljharb
Copy link
Collaborator

ljharb commented Jan 10, 2022

This would need to go through an RFC first, not as a PR to the cli.

Separately, it would be a very harmful change. The solution to the exceedingly rare case of bugs or malice coming in from an in-range update is “lockfiles”.

@wraithgar
Copy link
Member

This would be considered a major breaking change if implemented, so this can't land in the current version of npm. A decision like this would also need to be made through the rfc process.

@wraithgar wraithgar added the Agenda will be discussed at the Open RFC call label Jan 10, 2022
@zkldi
Copy link
Author

zkldi commented Jan 10, 2022

This would need to go through an RFC first, not as a PR to the cli.

Had no idea there was a separate RFC process. I'll move this post over there.

Separately, it would be a very harmful change. The solution to the exceedingly rare case of bugs or malice coming in from an in-range update is “lockfiles”.

This leads me to believe you just skimmed the post. This case clearly isn't exceedingly rare - bugs or malice being uploaded to npm in the form of patch updates is happening very frequently. I linked almost 7 examples that have happened just in recent memory.

Lockfiles do not solve the problem when your lockfile gets changed. If you have the following package.json and a generated package-lock file:

{
	"foobar": "^1.0.0",
	"react": "^17.5.4",
	"something-else": "^3.0.0"
}

Consider the case where foobar@1.0.1 is compromised. Your lockfile ensures that you get v1.0.0 right now, but what happens when you make any unrelated change to your dependencies? Say, how would you install a new dependency, baz in this scenario, without automatically being given foobar@1.0.1? The answer is pinning your dependencies. Not lockfiles.


Calling this "exceedingly rare" is missing the mark completely. The amount of people I've seen bitten by the colors.js and ua-parser-js thing is absurd.

@ljharb
Copy link
Collaborator

ljharb commented Jan 10, 2022

Closing in favor of npm/rfcs#509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants