[RRFC] Change the default save-prefix from ^ to none

[zkrising]

(Ported from npm/cli#4227)

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.

• UAParser (October 22 2021)
• coa (November 4 2021)
• rc (November 4 2021)
• another-date-range-picker (September 1 2020)
• another-date-picker (September 1 2020)
• stream-combine (September 2 2020)
• angular-bmap (September 1 2020)

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.

[zkrising]

Related reply to ljharb:

---

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]

I don't think it is missing the mark. 8 packages out of hundreds of millions in a decade is statistically insignificant.

This in no way means the problem should be dismissed, but it does mean the cost of a fix must be weighed carefully.

A lockfile is the only way to pin dependencies. Pinning them in package.json achieves nothing because transitive deps can update at any time.

[zkrising]

I don't think it is missing the mark. 8 packages out of hundreds of millions in a decade is statistically insignificant.

It doesn't really matter how many packages are compromised. It only takes one compromised library depended on by a lot of others to do a ridiculous amount of damage.

This in no way means the problem should be dismissed, but it does mean the cost of a fix must be weighed carefully.

A lockfile is the only way to pin dependencies. Pinning them in package.json achieves nothing because transitive deps can update at any time.

I'm not suggesting to remove package-lock, though. You can do both of these things. The scenario I gave you is important, so could you please explain how you would add a new package without pulling in the malware?

[ljharb]

You'd use overrides, new in npm v8.3, to force the transitive dependency to be an alternative version, or alternative package.

[thescientist13]

Interestingly, from my POV, this is the more salient point to make in all this (IMO)

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.

Since as you correctly point out..

npm ci should be used to avoid this behaviour, but why is the default dangerous?

☝️

This is one of the main reasons I prefer Yarn, the npm install behavior always nails me and i have to remember to use npm ci. I wish npm upgrade did what npm install did, and npm install only installed instead (e.g. was purely deterministic by default). 🤷‍♂️

[zkrising]

You'd use overrides, new in npm v8.3, to force the transitive dependency to be an alternative version, or alternative package.

Could you specify command by command what should be wrote to do that?

At the moment it doesn't matter that your lockfile says v1.0.0 as it will be changed the second you run npm install baz, which is how everyone installs packages.

EDIT: Npm v8.3 was released under a month ago! What should people do if they're on npm7 or npm6?

[ljharb]

npm 6 and 7 are both EOL, so those people should be upgrading to npm 8 regardless.

[zkrising]

npm 6 and 7 are both EOL, so those people should be upgrading to npm 8 regardless.

Ah, I didn't know 6 and 7 were EOL. Regardless, not everyone on npm8 has updated to the latest npm version. The rest of the post still applies -- What should you run instead of npm install baz to install the dependency safely?

[ljharb]

The current process is that applications should always have a lockfile. Your CI should be running npm ci. Locally, you can run npm install and it may generate lockfile changes.

These changes are pushed up, and validated by CI against your tests. Before being merged, it would of course go through code review - which would include both the lockfile changes as well as any code changes. Ideally, most applications also run npm audit or similar to catch CVEs. Then, you merge and deploy.

This tends to work incredibly well for bugs, vulnerabilities, and malice. The ^ remains incredibly useful and helpful to allow for intentional breakage to be avoided by default, and for improvements and fixes to land by default. No solution avoids the need for through review and tooling gating changes.

[zkrising]

The current process is that applications should always have a lockfile. Your CI should be running npm ci. Locally, you can run npm install and it may generate lockfile changes.

These changes are pushed up, and validated by CI against your tests. Before being merged, it would of course go through code review - which would include both the lockfile changes as well as any code changes. Ideally, most applications also run npm audit or similar to catch CVEs. Then, you merge and deploy.

I appreciate this, but you're not replying to what I've said. How do you safely install a new package baz in npm without causing other dependencies to update IF they are specified with ^?

This tends to work incredibly well for bugs, vulnerabilities, and malice.

This is provably untrue, as both colors.js and ua-parser have affected thousands if not millions of machines. This system clearly does not work well.

[ljharb]

How do you safely install a new package baz in npm without causing other dependencies to update IF they are specified with ^?

You don't - you let them update, and you catch it in review/CI.

thousands if not millions of machines

can you cite your source for this info?

[zkrising]

You don't - you let them update, and you catch it in review/CI.

What you're saying is that there is no way to do anything involving package.json without updating the lockfile. As clearly shown by how this package spread, anyone who did anything with their package.json had their colors version bumped.

Catching security bugs by just letting arbitrary code run on a CI server is a terrible idea (exploit the CI server, mine bitcoin on it, run up lambda bills... Infact, I have had people personally mention to me that it caused their CI to log an obscene amount of information. That can be costly in the cloud!).
Reviewing all dependencies is also impossible because of the sheer scope of it all.

What this means is that you cannot npm install baz without getting the new version of a transient dependency. This completely violates the principle of least surprise.

When I tell NPM to install a new package baz, I did NOT tell it to update every transient dependency in my tree. Hell, I think most developers aren't even aware that that is the behaviour, and likely just see their package-lock.json file as a bundle of noise to blindly commit.

can you cite your source for this info?

And this definitely did not spread by 25m weekly users all individually choosing to update to 1.44-liberty or whatever it was - This was an attack that leverages this loose "update everything whenever possible" default behaviour.

[azu]

https://web.archive.org/web/20220110130906/https://www.npmjs.com/package/colors
https://docs.google.com/spreadsheets/d/1GRtns2mhHCaSiY29HmlHUPrl1h4_Fb0xqScJU_kpcr8/edit#gid=0
FYI: Internet Archive show concrete downloads count.

[ljharb]

Thanks! I couldn't find the download counts now that the malicious versions were removed. so, 190,000 downloads out of 23 million, which is about 0.08%?

[zkrising]

Thanks! I couldn't find the download counts now that the malicious versions were removed. so, 190,000 downloads out of 23 million, which is about 0.08%?

1.4.1 and 1.4.44 were also compromised, pushing it closer to 650,000 out of 23m (2% ish).

Please keep in mind that all those downloads came in in under 24 hours. The 23 million statistic is weekly downloads. If you were to naively multiply that 650,000 by 7, you'd see a far worse statistic of 4.55 million downloads.

That said, the percentage of downloads that are bad has nothing to do with it - This still spread through the ecosystem at a rapid rate. For a more insightful look into the impact of this into organisations, look at the amount of linked PRs and Issues in the colors.js issue.

This is clearly affecting the ecosystem in a significant way. Infact, the only reason this has such a small amount of downloads is because NPMs security team worked on it pretty fast.

[pfych]

You don't - you let them update, and you catch it in review/CI.

The colors issue caused temporary downtime in at least two development environments I was actively working in. A companies Review/CI team should not be expected to comb through every dependencies dependencies to find an issue, especially if the issue only affects fresh npm install's.

I agree this is on the package maintainers for not pinning dependencies, not the npm team, however this RFCs proposal will reduce the chances of this happening in the future.

---

You'd use overrides, new in npm v8.3, to force the transitive dependency to be an alternative version, or alternative package.

It's a bit unreasonable to expect a developer to pin all shared dependencies in overrides to avoid issues with rogue packages.

[thescientist13]

so, 190,000 downloads out of 23 million, which is about 0.08%?

One could argue that this malicious action with colors / faker was specifically intended to get notoriety and draw attention to itself. How many of these go under the radar though? Miners and other data harvesting type attacks from scammers typically do not go out of their way to make their presence known, and when they are revealed, often it is too late or the damage is done.

Also, isn't it a common strategy of scammers, to spam out scams to millions, and bet on the odds that if just a small percentage fall for it, then they've got what they need?

I'm not saying we can protect everyone 100 percent of the time, but I think there is a lot of merit to discussing the point of could npm install be a bit more cautious? I think we're unfortunately seeing more and more people taking advantage of this "feature" of npm and maybe it's time to review the appropriateness of this default behavior. I'm sure it could be preserved with a flag, e.g. npm install --upgrade or whatever.

[ljharb]

@pfych any environment in which it caused downtime failed to use a lockfile. Any environment which used one didn’t have any downtime.

[zkrising]

@pfych any environment in which it caused downtime failed to use a lockfile. Any environment which used one didn’t have any downtime.

This isn't the point though. Almost every environment use lockfiles at this point (they've been included for years!). The default behaviour of npm install not honoring it is a footgun in and of itself, but what's also bad is that the lockfile doesn't matter if any touching of your package.json causes the lockfile to bump versions anyway. It's not much help package-locking dependencies if the package-lock going to change during unrelated use of NPM.

[chocolateboy]

I understand the impulse, but this feels more like a way of mitigating headlines than exploits. Looking through the list of critical and high severity vulnerabilities, it seems to me that:

1. Most are fixed by updating dependencies to patched versions rather than pinning them.
2. Most are due to oversight rather than malice.
3. There are a ton of security holes in our dependencies right now (in all ecosystems) that are being overlooked because we're all focused on ensuring something annoying but atypical doesn't happen again.

Examples

Bugs

vulnerability	severity	remedy	downloads per week
Command Injection in lodash	high	patch	42,708,666
Incorrect Comparison in axios	high	patch	20,106,154
Denial of service in css-what	high	patch	19,068,272
Remote code execution in handlebars when compiling templates	critical	patch	9,131,162
Arbitrary Code Execution in underscore	high	patch	7,820,344
Prototype Pollution in immer	critical	patch	7,428,209
Improper Input Validation in klona	high	patch	7,038,357
Prototype Pollution in vm2	critical	patch	5,096,738
Unexpected server crash in Next.js.	critical	patch	1,819,663
Command injection in nodemailer	critical	patch	1,716,403
Prototype pollution in chart.js	high	patch	1,571,535

Malware

vulnerability	severity	remedy	downloads per week
Infinite loop causing Denial of Service in colors	high	pin	25,381,569
Embedded malware in ua-parser-js	critical	patch	8,735,137
Embedded malware in coa	critical	pin	7,921,768
Malicious Package in stream-combine	critical	pin	32
Malicious Package in angular-bmap	critical	pin	2
Malicious Package in another-date-range-picker	critical	pin	2

[ianldgs]

I know I'm a bit late to the discussion, sorry if I'm repeating something, haven't read all the comments.

But when one of my colleagues joined my team, this was one of his first suggestions: to pin all versions of our packages.

My reply was that the package-lock.json would do it for us, and he agreed.
Also, that one of the downsides would be that constraining packages to a specific version causes multiple versions of the same packages to be installed on our node_modules if everyone did that. It's the case with this library. We have 2 installations of every of their packages, even though the same major versions match.

Lastly, I wanted to add that when I install something new, not all of the package-lock is affected, as stated above, only stuff related to the library I installed or updated. We have pretty much gotten used to using npm ci instead of npm install by now.
Here are some logs to demonstrate:

npm --version

7.24.2

ncu

 @ant-design/charts                ^1.2.14  →   ^1.3.4     
 @mui/material                      ^5.2.6  →   ^5.2.8     
 @okta/okta-auth-js                 ^5.9.1  →  ^5.10.0     
 history                           ^4.10.1  →   ^5.2.0     
 luxon                              ^2.2.0  →   ^2.3.0     
 react-router-dom                   ^5.3.0  →   ^6.2.1     
 react-virtuoso                     ^2.3.4  →   ^2.4.0     
 @types/luxon                       ^2.0.8  →   ^2.0.9     
 @types/node                       ^17.0.5  →  ^17.0.8     
 @typescript-eslint/eslint-plugin   ^5.8.1  →   ^5.9.1     
 @typescript-eslint/parser          ^5.8.1  →   ^5.9.1     
 eslint                             ^8.5.0  →   ^8.6.0     
 eslint-plugin-import              ^2.25.3  →  ^2.25.4     
 jest                               27.4.5  →   27.4.7     
 msw                               ^0.36.3  →  ^0.36.4

npm install axios

added 1 package, and audited 2071 packages in 5s

git diff package-lock.json | cat

diff --git a/frontend/ui/package-lock.json b/frontend/ui/package-lock.json
index 11831e8a..ac4e2fca 100644
--- a/frontend/ui/package-lock.json
+++ b/frontend/ui/package-lock.json
@@ -18,6 +18,7 @@
         "@okta/okta-auth-js": "^5.9.1",
         "@okta/okta-react": "^6.3.0",
         "@reduxjs/toolkit": "^1.7.1",
+        "axios": "^0.24.0",
         "chroma-js": "^2.1.2",
         "history": "^4.10.1",
         "lodash": "^4.17.21",
@@ -6297,6 +6298,14 @@
         "node": ">=4"
       }
     },
+    "node_modules/axios": {
+      "version": "0.24.0",
+      "resolved": "https://registry.npmjs.org/axios/-/axios-0.24.0.tgz",
+      "integrity": "sha512-Q6cWsys88HoPgAaFAVUb0WpPk0O8iTeisR9IMqy9G8AbO4NlpVknrnQS03zzF9PGAWgO3cgletO3VjV/P7VztA==",
+      "dependencies": {
+        "follow-redirects": "^1.14.4"
+      }
+    },
     "node_modules/axobject-query": {
       "version": "2.2.0",
       "resolved": "https://registry.npmjs.org/axobject-query/-/axobject-query-2.2.0.tgz",
@@ -11000,7 +11009,6 @@
       "version": "1.14.6",
       "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.14.6.tgz",
       "integrity": "sha512-fhUl5EwSJbbl8AR+uYL2KQDxLkdSjZGR36xy46AO7cOMTrCMON6Sa28FmAnC2tRTDbd/Uuzz3aJBv7EBN7JH8A==",
-      "dev": true,
       "funding": [
         {
           "type": "individual",
@@ -28876,6 +28884,14 @@
       "integrity": "sha512-WKTW1+xAzhMS5dJsxWkliixlO/PqC4VhmO9T4juNYcaTg9jzWiJsou6m5pxWYGfigWbwzJWeFY6z47a+4neRXA==",
       "dev": true
     },
+    "axios": {
+      "version": "0.24.0",
+      "resolved": "https://registry.npmjs.org/axios/-/axios-0.24.0.tgz",
+      "integrity": "sha512-Q6cWsys88HoPgAaFAVUb0WpPk0O8iTeisR9IMqy9G8AbO4NlpVknrnQS03zzF9PGAWgO3cgletO3VjV/P7VztA==",
+      "requires": {
+        "follow-redirects": "^1.14.4"
+      }
+    },
     "axobject-query": {
       "version": "2.2.0",
       "resolved": "https://registry.npmjs.org/axobject-query/-/axobject-query-2.2.0.tgz",
@@ -32635,8 +32651,7 @@
     "follow-redirects": {
       "version": "1.14.6",
       "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.14.6.tgz",
-      "integrity": "sha512-fhUl5EwSJbbl8AR+uYL2KQDxLkdSjZGR36xy46AO7cOMTrCMON6Sa28FmAnC2tRTDbd/Uuzz3aJBv7EBN7JH8A==",
-      "dev": true
+      "integrity": "sha512-fhUl5EwSJbbl8AR+uYL2KQDxLkdSjZGR36xy46AO7cOMTrCMON6Sa28FmAnC2tRTDbd/Uuzz3aJBv7EBN7JH8A=="
     },
     "font-atlas": {
       "version": "2.1.0",

[zkrising]

Just came off #510, and there's some excellent points made against this proposal. It seems like this is far too heavy handed for the issue, and I'm very much inclined to agree.

However, I am very interested by a related issue that's been brought up in parallel to this, which is that the default behaviour for npm install is counterintuitive. Infact, until this thread, I actually thought myself that if a package-lock.json file was present, npm install would honour it.

I think this is a surprising behaviour, and should probably be looked into being changed. I'm going to open a separate RRFC for that now, as I think it's far more actionable and far more interesting.

Regardless, I'm pretty convinced that this (^ -> "") is not a good solution for this problem.

[zkrising]

An RRFC for my proposal already kind of exists at #415. I'll write what I think in there, then.

[mortargrind]

I know this very late but @zkldi Can you elaborate a bit more on the counter arguments and the implications of having this as a default? Unfortunately the meeting on 12th of January is nowhere to be found and from the meeting notes it's not very clear what were the points 😞

[zkrising]

• Breaks half the ecosystem which depends on this behaviour

The issue here is that it just moves the problem one-layer down. If a dependency of yours is pinned to a faulty version of a dependency, it's difficult to actually get that dependency to update to safe version.

consider the case where foo@1.0.1 has a critical security bug, and foo@1.0.2 doesn't.

- your_project
  - foo@1.0.2
  - bar@1.0.0
    - foo@1.0.1

if bar was lenient and allowed any patch upgrades, these would be safe to meld together and you'd be using 1.0.2 throughout the stack. if it isn't, you have to wait for the bar maintainers to push their own patch updating the dependencies. Of course, in an ideal world this would be fine, but it causes massive ecosystem slides.

In retrospect I'm not entirely convinced this is a critical blocker -- a way of specifying overrides would be nice.

The real issue (which was highlighted in that call) was that lockfiles just aren't honoured by default, and a ridiculous amount of users run npm install in production -- which is very harmful as it just ignores the lockfile!

[ljharb]

@zkldi its not difficult anymore - npm now has overrides. I don’t think the problem this rfc wants to solve is actually a problem now.