[BUG] `npm ci` succeeds when `package-lock.json` doesn't match `package.json`
icatalina opened this issue · 50 comments
Current Behavior:
npm ci does not fail when package.json doesn't match package-lock.json
Expected Behavior:
npm ci refuses to install when the lock file is invalid.
Steps To Reproduce:
- Manually bump a major version of a dependency in
package.json - Run
npm ci - It should fail but performs the whole installation
npm@7
npm@6
Environment:
- OS: Mac OS
- Node: 14.15.3
- npm: 7.5.4
I've ran into this as well, is there any workarounds until this issue is resolved? Perhaps a flag or a new command could be added to verify the two files are in sync.
A consequence of this is that CI builds (which uses npm ci) now passes (as it installs an old version) even if a dependency would normally cause a build failure (such as when a major version as suggested by OP).
This is also affecting workspaces.
We are trying to switch from Yarn workspaces to NPM workspaces, and we were confused why npm ci would tolerate an outdated lockfile (whereas yarn install --immutable would error out). This bug seems to explain it.
This is more or less a blocker for switching back to NPM. The NPM CLI tools for managing workspace dependencies are still rough enough that I expect some mistakes; but without this bugfix, I don't know any other way to protect our main branch against a lockfile mismatch. 😕
I see this happen quite often when the version field in package.json is bumped without making a corresponding change to package-lock.json, resulting in the files being out of sync.
This bug contradicts description from documentation on how npm ci should work:
In short, the main differences between using npm install and npm ci are:
The project must have an existing package-lock.json or npm-shrinkwrap.json.
If dependencies in the package lock do not match those in package.json, npm ci will exit with an error, instead of updating the package lock.
npm ci can only install entire projects at a time: individual dependencies cannot be added with this command.
If a node_modules is already present, it will be automatically removed before npm ci begins its install.
It will never write to package.json or any of the package-locks: installs are essentially frozen.
@darcyclarke
This has been on P1 for a while, is there anything we can do to help move it along?
We just hit this too @darcyclarke, as we're moving more repos to npm@7. I can confirm that there are no errors in the CI.
Same issue with npm 8.1.0
Hi, This is MAJOR security issue, especially today with all the dependency attacks..
I just found that even our protections using package-lock is not validated, and this bug is open from February.
TLDR.. How can I help fix this?
@rotem-cider I don't think this is a security issue, because as I understand it just ignores what's in the
package.jsonand does exactly whatpackage-lock.jsonsays.To temporary overcome this issue you could switch to
npm iand check if file was changed afterwards (either by git or a checksum)
I do think it's a major security issue as well ...
If your package.json has a version marked as ^1.2.3 and your lockfile is set at 1.2.3 followed by a malicious release of said dependency at version 1.2.4 then running npm ci in it's current form will pick up this vulnerability. In the past running npm ci would've only installed 1.2.3 since that's the version pinned in the lock file.
My team's faced numerous issues with unexpected version bumps because of this bug. Also happy to help with this issue if i can
I've re-checked and it indeed ignores what is in the lock file and installs by semver from package.json.
For now, I think the best thing is to run npm6 in ci environments.. is there another workaround which can be recommended?
For now, I think the best thing is to run npm6 in ci environments.. is there another workaround which can be recommended?
Not ideal but you can pin version in package.json by removing ^ / ~ / etc. and use the newer versions (but it's not ideal imo)
@virkt25 What about transitive dependencies (where the version can't be pinned)? Do we already know this bug doesn't affect those as well?
To make it explicit, let's say our package.json depends on version 1.0.0 of module A, which in turn depends on ^2.0.0 of module B. Our package-lock.json contains versions 1.0.0 for A and 2.0.0 for B. Now, a new release 2.0.1 of B is published. Which version of B does npm ci install: 2.0.0 or 2.0.1?
Hi @valentjn, I checked the same scenario now, and as I tested it and understood, if there is a package-lock.json it does take the versions from it. but if there was a new dependency it downloads the latest by the semantic versioning.
So with npm ci I wasn't able to upgrade the lower dependency, but I feel like if one will add a dependency that wasn't previously it will be added. Needs more testing and understanding of how it works
Or use script to fail if lock-file doesn't match:
CKSUM_BEFORE=$(cksum package-lock.json)
npm i
CKSUM_AFTER=$(cksum package-lock.json)
if [[ $CKSUM_BEFORE != $CKSUM_AFTER ]]; then
echo "package-lock.json is outdated"
exit 1
fi@the-spyke If there is a new unwanted release of a dependency, wouldn't npm i execute post-install scripts, which have been leveraged in some of the latest attacks?
@valentjn in this case you could add --ignore-scripts and use this as a separate job to just check that package-lock.json matches package.json before actually installing and building
@virkt25 What about transitive dependencies (where the version can't be pinned)? Do we already know this bug doesn't affect those as well?
To make it explicit, let's say our
package.jsondepends on version1.0.0of moduleA, which in turn depends on^2.0.0of moduleB. Ourpackage-lock.jsoncontains versions1.0.0forAand2.0.0forB. Now, a new release2.0.1ofBis published. Which version ofBdoesnpm ciinstall:2.0.0or2.0.1?
I haven't tested this but it's a valid concern. npm v7/8 aren't production ready imo but were made the default with Node 16 which we've upgraded to for some of the newer features.
If you have the choice then @rotem-cider's suggestion is the best one -- use v6.
Tried comparing the code between v6/7 to see if it was a regression that could be quickly fixed but v7 changed the underlying engine entirely so it's likely a feature implementation (don't quote me -- need to dig into the new engine code more to understand what's happening)
More testing.
npm ci of version 6 doesn't care if you change ^4.11.2 to ^4.11.0 or 4.11 in package.json. It just installs what package-lock.json has if it matches that version specifier by semver .
But changing to a different version like ^4.12.0 raises an error.
I just skimmed over the Bug Bounty Program of GitHub (owner of npm). One example of a “medium severity issue” is explicitly given as:
- package integrity compromise, i.e., downloading a package that does not match the integrity as defined in
package-lock.json
Isn't the bug in this issue here doing exactly this? npm downloads a package of a different version than the one defined in package-lock.json. That downloaded package must have a different integrity/hash than the one stored in package-lock.json, because the stored integrity belongs to a different version than that of the downloaded package.
I think this is a major security problem. This issue needs the Security label and a fix ASAP, also in light of all the recent attacks. In the meantime, as an immediate hotfix, npm should IMO disable the npm ci subcommand (or at least print a warning that it's not doing what it's supposed to do, although not many will read that in CI contexts).
Isn't it the opposite? it just ignores package.json
@icatalina It ignores package-lock.json and follows package.json when I do the following:
- Create new module that depends on version
Xof some module, for which there is actually a newer versionYavailable. - Run
npm i. - Change the version of dependency in
package.jsonfromXtoY. - Verify that
package-lock.jsonstill contains versionXfor the dependency. - Delete
node_modules/, but notpackage-lock.json. - Run
npm ci. - Observe that no error occurred and version
Yhas been installed (look innode_modules/.../package.json).
It did install version X when I changed the dependency version in step 3 from X to ^X (even though Y is available and matches ^X).
Oh, that's worse than my experience... I thought it wouldn't even touch package.json...
I opened a CVE for proper reference of this issue
@valentjn, just gotten around to testing this. It is pretty bad 🤦
I created a repo with the issue: https://github.com/icatalina/CVE-2021-43616
There definitely seems to be a bug where npm ci sometimes (not always! I did get a failure in some of my local tests) proceeds with installation, rather than fails, when package-lock.json and package.json are in disagreement.
However, I was not able to reproduce a case where it installs something that matches the semver in package.json, which is different to what is defined in package-lock.json, i.e. I don't see how this is a security issue, since having control of package.json implies control of package-lock.json, i.e. there is no point to make them disagree when you can just modify both to install whatever malicious thing you want installed.
However, I was not able to reproduce a case where it installs something that matches the semver in
package.json, which is different to what is defined inpackage-lock.json
@dominykas Refer to these steps to have npm install the version given in package.json, ignoring the version given in package-lock.json.
Furthermore, it's not a question of maliciously modifying package.json and/or package-lock.json. It's a question of maliciously publishing updates of dependencies via compromised accounts, without modifying the files. At least, that's how the recent attacks worked:
package.jsonrefers to non-malicious version~1.0.0of dependencyA- Attacker compromises account of maintainer of
Aand publishes malicious version1.0.1 npm inow installs malicious version1.0.1and compromises the system (usually, post-install scripts are leveraged to compromise the system at this point, but there are of course more possibilities)
The question is, can a malicious dependency somehow be installed via npm ci when a lockfile exists that doesn't contain the malicious dependency (because the lockfile was created before the malicious dependency was published)? If not, why not (I'd like to understand what npm ci exactly does in this bugged state)? This bug might make this possible, but I haven't had the time to do the proper testing.
@valentjn you yourself said this:
It did install version X when I changed the dependency version in step 3 from X to ^X (even though Y is available and matches ^X).
npm ci installs the correct thing - I have not seen any examples of bypassing that when package.json and package-lock.json agree with each other.
@valentjn It might be best to give an example with an explicit package name, package version, and npm version.
At Snyk we currently have a lot of noise related to the CVE-2021-43616. We decided to have a closer look at the issue.
We believe that this issue is a software bug rather than vulnerability. We think so because we can't imagine possible attack scenarios where a malicious actor will be able to hijack package.json but not a package-lock.json. And even in this case hijacking package.json means that a malicious actor can do much more damage than just change the version of a package (i.e. change scripts section).
Although we understand that this bug can cause vulnerabilities in some exotic cases (if package.json is a user input of an application).
So, we don't think it is correct to assign a CVE and recommend revoking the CVE-2021-43616.
Hey @Kirill89, First great that Snyk is looking at this vector and researching this closer. I also researched this vector and I also didn't find a way yet for attacking as malware in an external dependency as it does respect the package-lock same as the npm install functionality.
The issue here is that when using "npm ci" I am expecting the npm CLI to verify that the package is strictly written in package-lock and that it is the same hash integrity as before. I hadn't had the time or enough resources to verify all the paths happening inside the arborist package which is in charge of verifying the tree, but sure enough the functionality that we had in npm6 is not the same as in npm7 and leaves loopholes that may be abused by attackers.
One scenario I can think of depends on the CODEOWNERS file, pipelines can be automatically run and even merged, depending on if the package or lock files are in it. The malicious actor can add a rogue package to package.json or reset the package-lock file to something like {} which will still work.
As this is clearly against what is written about the functionality of "npm ci" in the documentation and security teams are relying on it for their security audits and checks I do think this should stay as a CVE and should be addressed.
About the impact, I think that it is per usage and personally feel it should be a medium-high and not critical.
@Kirill89 There is no talk about somebody hijacking package.json or package-lock.json. It's much simpler.
People sometimes just forget to check in the updated lock file because it's machine generated and they're humans. This is exactly why we have ci command. And with current "behavior" it will install an unknown version on every CI run instead of failing. And not just every, but the newest version on every run.
It will be as always: somebody hijacks Lodash/whatever popular, publishes a patch version with a backdoor, 1 minute after it will be already sucked into some build job without devs even doing anything.
@Kirill89 thanks for chiming in. I get that it's a bug BUT I think it has massive security implications for production environments. Would love to be explained how the scenario below isn't a critical security issue:
npm@6 guaranteed package installs to be "frozen" (as per their documentation) based on the contents of package-lock.json when using npm ci. This made it ideal for CI/CD deployments since it guaranteed the same build based on package-lock.json. It is common for developers to pick up minor/patch bumps by running npm i locally -- this would normally result in changes to package-lock.json BUT not package.json.
Ex: package.json has shortid: "^2.0.0" and so does package-lock.json. Running npm iwill leave package.json untouched BUT update package-lock.json so it now hasshortid: "^2.0.4"`.
The new lock file is tested and committed to the application. CI/CD systems always use npm ci since it is supposed to guarantee a frozen install that matches exact versions as defined in package-lock.json.
Version 7 and up
Following the above scenario running npm ci in CI/CD environments can result in a NEW artifact if shortid releases versions 2.0.5 even though package-lock.json doesn't specific this version (and package.json is still at ^2.0.0).
- If
2.0.5is a compromised version then downstream apps have now automatically picked it up regardless of thelockfile - If
2.0.5contains a breaking change (let's say it didn't follow semver) then your app can fail at runtime / build as it has deviated from the tested lock file.
The whole point of npm ci is to guarantee a "frozen" install which it no longer does which in my opinion is a significant security risk and adds instability even otherwise for production applications.
Step-by-step:
Prepare a repo. Let's pretend that we installed lodash@4.11.0 some time ago:
$ mkdir npm-2701
$ cd npm-2701
$ npm -v
8.1.0
$ npm init -y
$ npm i lodash@4.11.0
$ npx json -I -f package.json -e 'this.dependencies.lodash="^4.11.0"'
$ npm iLet's see what we have as initial setup:
$ cat package.json | npx json 'dependencies.lodash'
^4.11.0
$ cat package-lock.json | npx json 'dependencies.lodash'
{
"version": "4.11.0",
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.11.0.tgz",
"integrity": "sha1-Qo9xcqXpqC6aRZtUOBZImBDsuK8="
}
$ cat ./node_modules/lodash/package.json | npx json 'version'
4.11.0Okay. It's time to update Lodash to a newer version ^4.11.0 >>> ^4.11.1 which recently came out:
$ npx json -I -f package.json -e 'this.dependencies.lodash="^4.11.1"'
$ git commit -m "Update Lodash to the latest and greatest"
$ git push originI have a feeling that I forgot something... Meanwhile on my CI build:
$ git clone
$ cat package.json | npx json 'dependencies.lodash'
^4.11.1
$ cat package-lock.json | npx json 'dependencies.lodash'
{
"version": "4.11.0",
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.11.0.tgz",
"integrity": "sha1-Qo9xcqXpqC6aRZtUOBZImBDsuK8="
}
$ ls node_modules
ls: cannot access 'node_modules': No such file or directoryNow the fun part. Output below is exact copy from my terminal:
$ npm ci
added 1 package, and audited 2 packages in 939ms
found 0 vulnerabilities
$ cat package.json | npx json 'dependencies.lodash'
^4.11.1
$ cat package-lock.json | npx json 'dependencies.lodash'
{
"version": "4.11.0",
"resolved": "https://registry.npmjs.org/lodash/-/lodash-4.11.0.tgz",
"integrity": "sha1-Qo9xcqXpqC6aRZtUOBZImBDsuK8="
}
$ cat ./node_modules/lodash/package.json | npx json 'version'
4.17.21You're welcome.
Just bitten by this too.
Precondition: A version of a dependency listed in package-lock.json is no longer a semver match for the dependency in package.json
Expected behaviour: npm ci should fail with an error.
Actual behaviour: npm ci installs the latest version matching the semver in package.json and ignores the lockfile entirely.
Regardless of whether it's a CVE or not, it's clearly a big bug and really worrying that there's no word on anyone working on a fix for it despite affected versions of npm now being the default in the current "Recommended for most users" Node.js installers.
FWIW I'm in the camp it is a security issue - it's very easy through human error to get package.json out-of-sync with a lockfile - eg just one developer manually editing the package.json file and committing without running npm install. Now any future CI builds using npm ci will pull in the latest versions for that dependency at that particular moment (and presumably its transitive deps too). All of the protections the developers thought they were getting through lockfiles and npm ci have been effectively sidestepped.
Sorry for late response,
I don't have any objections about the bug itself – it is indeed a serious security concern.
I only want to emphasize that it is not a security vulnerability because by definition a vulnerability has to be exploitable.
I still don't see an attack vector for this issue. @rotem-cider CODEOWNERS example maybe the closest possible example to count it as a vulnerability, but I would count it more like a vulnerable configuration of a repository because the same could happen for example if you simply not use package-lock.json in your project and run npm install in the CI/CD.
I know this discussion is a bit tedious and I agree with most of you that this bug has to be fixed ASAP. But at Snyk we also care to keep the vulnerability database clean and meaningful, hence all my concerns are only around CVE – not about the bug itself.
@Kirill89 I wasn't advocating for making it a CVE as I don't know how this process works.
But now I'm curious, because by your reasoning SolarBurn didn't exist as its victims had no exploitable vulnerabilities. Using npm ci and not using npm ci are two different things. It's like having a lock and not having a lock. In our case we have a lock, but it stopped to care which keys are inserted. Also I assume that from the point of view of Snyk customers (which are mitigating supply chain attacks by using npm ci) there is absolutely no need to know about this "bug" and they are not exploitable, it's just fresh packages.
I think the CVE is important and Snyk should be flagging this to others because you could just trust npm ci in the past (and most production systems I've seen rely on that) and you can no longer do that. Comments like "this bit me" are perfect examples of people expecting one thing but it doing another and hence being exposed to unexpected upgrades to versions which may be compromised / haven't been tested.
And if it helps expedite a solution to this issue, great but that doesn't seem to be the case.
Either way I think a CVE is important for awareness even if this isn't a direct vulnerability
@virkt25 that logic would make everything a CVE, and make CVEs even more useless than they already are due to false positives. CVEs are not solely for mismatched expectations.
As far as I understand CVE’s are supposed to tell about security problems in products, and to affect the versions that are vulnerable so companies can upgrade to non vulnerable versions.
in this case companies using npm7 and 8 are vulnerable In cases they have a drift between the lock file and the package.json file, same happens in other CVE’s where there is a need for a specific configuration of Apache or Nginx - context is everything in every CVE that comes out and our mission is to understand fast if this affects us
Because CVE’s are not specific to usage yet, (maybe in future there will be a way to define the use case specifically in the format and will save us lots of trouble)
Then we have no choice but to flag the whole version vulnerable without going into how is it used, are we really running it in isolated environments or giving it access to secrets for deployment to our cloud services.
This is a very valid CVE from my perspective, I know there is a lot of heat lately on npm specifically and they are handling this as much as they can, hope this is a wake up call for them to step up their game, and for us as an industry to understand that the weakest link can harm our whole environment.
I just came across this and I'm shocked. In my tests, package-lock.json is completely disregarded and npm ci seems to act just like npm i, except it doesn't update the package-lock.json, so you don't even realise that a different version has been installed. What's the point of having package-lock.json then?
npm ci now seems like at least as dangerous an operation in CI/CD as npm i. It seems like the only use case for the command is no longer fulfilled. But maybe it does indeed work sometimes, as some people have indicated.
Yeesh, this bug. Haven't tested it, but this might help for CI in the meantime: https://github.com/RocketChat/package-lock-check
It is sort of ridiculous that a Priority 1 bug has been opened for almost a year now... 😔
Can anyone provide more context on what just happened here? The ci command was introduced as
npm ci bypasses a package’s package.json to install modules from a package’s lockfile
If ci installs directly from a lockfile, why is the extra validation step in 457e0ae necessary?
@ricardobeat the lockfile is only reliable if it satisfies the package.json; it's a bug that that validation was ever omitted imo.
The docs have always been clear too:
If dependencies in the package lock do not match those in package.json, npm ci will exit with an error, instead of updating the package lock.
This was meant to be fixed in 8.4.1, if I'm following the tags correctly, but I can still npm ci and it will happily install things even if package.json disagrees? Can someone please confirm this?
I'm also wondering about the correct behavior with deep dependencies - should there be a failure if the shrinkwrap overrides what's in their parent's package.json?
Maybe of interest to others here, re: npm ci failure modes: (malformed integrity hashes ignored) #4460 (comment)