npm v3 non-determinism does actually result in different code (not just tree structure)

[tolmasky]

In the example in this doc page: https://docs.npmjs.com/how-npm-works/npm3-nondet , it shows that although differing install order changes the tree structure, every package still actually gets the same dependency. In the example on this page, this is actually the case, every package requires the same version regardless of the order, regardless of where lives in the tree.

However, I have constructed a small reduction that shows that install order can actually change the way the code functions (in other words, the ACTUAL versions of a dependency you will pick up, not the way you pick it up from the tree).

To test:

mkdir whatever;
cd whatever;
npm init;
npm install test-b --save
npm install test-a --save
node index.js

This will print:

1.0.0
1.0.1

Now let's try the opposite order:

mkdir whatever;
cd whatever;
npm init;
npm install test-a --save
npm install test-b --save
node index.js

This will print:

1.0.0

As you can see, install order actually affects the bits that will be run. The key here is that test-a has an absolute dependency on test-c 1.0.0, while test-b has a semver range dependency on test-c ^1.0.0. That means if test-b is installed first, it correctly picks up 1.0.1, then a will of course want 1.0.0. However, if a installs first, then 1.0.0 will be installed, which satisfies b, and thus both get 1.0.0.

The important thing here is that anyone ELSE that has an absolute dependency (and is alphabetically before you), effectively sabotages YOUR package. You're stuck, unless you start fiddling with your shrink-wrap, its very hard to tell all your users ("hey this unrelated dependency is influencing what dependency we pick up, please ____ ").

Edit: I'd like to quickly point out that I'm not making a claim to the require-cache. I could have had it print "good" and "bad" instead of "1.0.0" and "1.0.1". I am aware that there is no guarantee of getting two 1.0.0's or just 1 depending on the tree structure.

[ashleygwilliams]

this is super interesting. thanks for sharing. could you make a PR to this sandbox with your example code so i could play with it a bit? https://github.com/ashleygwilliams/npm-sandbox (put it in npm3/10999/)

[tolmasky]

Sure, do you want me to put test-a/test-b/test-c all in there, or just the scripts that npm install them (the tests themselves are on npm)

[ashleygwilliams]

whatever you think is best :) thanks!

[tlrobinson]

FYI here's a full script to easily run @tolmasky's reduction:

mkdir whatever
cd whatever
echo 'require("test-a"); require("test-b");' > index.js
npm init -y > /dev/null

npm install test-b --save 2> /dev/null
npm install test-a --save 2> /dev/null
node index.js

rm -rf node_modules package.json
npm init -y > /dev/null

npm install test-a --save 2> /dev/null
npm install test-b --save 2> /dev/null
node index.js

Here's what I get:

tmp/   $ mkdir whatever
tmp/   $ cd whatever
whatever/   $ echo 'require("test-a"); require("test-b");' > index.js
whatever/   $ npm init -y > /dev/null
whatever/   $ npm install test-b --save 2> /dev/null
whatever@1.0.0 /Users/tlrobinson/tmp/whatever
└─┬ test-b@1.0.0
  └── test-c@1.0.1

whatever/   $ npm install test-a --save 2> /dev/null
whatever@1.0.0 /Users/tlrobinson/tmp/whatever
└─┬ test-a@1.0.0
  └── test-c@1.0.0

whatever/   $ node index.js
1.0.0
1.0.1
whatever/   $ rm -rf node_modules package.json
whatever/   $ npm init -y > /dev/null
whatever/   $ npm install test-a --save 2> /dev/null
whatever@1.0.0 /Users/tlrobinson/tmp/whatever
└─┬ test-a@1.0.0
  └── test-c@1.0.0

whatever/   $ npm install test-b --save 2> /dev/null
whatever@1.0.0 /Users/tlrobinson/tmp/whatever
└── test-b@1.0.0

whatever/   $ node index.js
1.0.0

[mhils]

It might be sensible to actually treat ^1.0.0 as ==1.0.$latestPatchRelease. In other words, the goal should not be to install any version that fulfills all constraints, the goal should be to install the latest version(s) that fulfill all constraints. Even if 1.0.0 is present, test-b should still get the 1.0.1 release.

While this increases the total number of downloaded versions, it fixes the (IMO way worse) issue of install order dependency poisoning.

[iarna]

This is npm@2 non-determinism too, FYI.

[tolmasky]

Just hoping to understand what you mean, but if I run those identical commands with npm 2.12.0 I get the correct behavior both times (independent of install order), so I don't think its npm v2 non-determinism? Unless you mean that running dedupe in npm 2 will cause this non-determinism as well?

[felixfbecker]

I don't see the problem.

1. If test-a abosolutely depends on 1.0.0 and not ^1.0.0 like any sane package author would, NPM needs to respect that
2. If your module needs the bugfix in 1.0.1, then it can just set its dependency to ^1.0.1. If it doesn't, it gets a package that is perfectly compatible with what it specified as a dependency.

[tolmasky]

Having put further thought into this, this is actually a pretty serious security concern. @felixfbecker actually outlines the problem pretty well, which is that npm3 only works well with good actors, but is can be easily exploited by malicious actors (test-a). A small example:

1. Let's say many packages depend on package test-c (common scenario). test-c is relatively well maintained and updates to 1.0.1 from 1.0.0 for a security patch.
2. If a malicious actor can sneak in a dependency to test-c@1.0.0 anywhere in the sub-dependency tree of an early-enough-in-the-alphabet package, they will successfully sabotage your app by making ALL its dependencies use the insecure 1.0.0 package (compared to npm2 behavior where all other packages would not suffer from this).

In other words, npmv3 greatly increases the amount of maintenance required as a burden on package writers. With npm v2, not updating a package EVER and having the package JSON say ^1.0.0 is good enough to ensure your package will always receive the latest security updates. With npm v3, this is no longer the case, as your package is now open to attack from unrelated packages. As @felixfbecker explains, it is now up to everyone on npm to be incredibly diligent in updating their ^ dependencies to always point to at least the current version to protect against this (which is unlikely).

[felixfbecker]

@tolmasky This is a valid concern, but I still think that NPM behaves correctly here.

[npm-robot]

We're closing this issue as it has gone thirty days without activity. In our experience if an issue has gone thirty days without any activity then it's unlikely to be addressed. In the case of bug reports, often the underlying issue will be addressed but finding related issues is quite difficult and often incomplete.

If this was a bug report and it is still relevant then we encourage you to open it again as a new issue. If this was a feature request then you should feel free to open it again, or even better open a PR.

For more information about our new issue aging policies and why we've instituted them please see our blog post.