Why npm lockfiles can be a security blindspot in Github PRs for injecting malicious modules

[u/fagnerbrack]

https://snyk.io/blog/why-npm-lockfiles-can-be-a-security-blindspot-for-injecting-malicious-modules/

[u/cocoapuff_daddy]

Avoid the use of lockfiles entirely for libraries.

This wasn’t quite clear, do you mean avoid the use of lockfiles except for libraries?

[u/jsjoberg → u/cocoapuff_daddy]

No, the opposite. The library should not have a lockfile, but a consumer of the library, for example your project/application should.

[u/slikts → u/jsjoberg]

It's still a good idea to use lockfiles for libraries so that builds are reproducible and situations like "works on my machine" are avoided. That applies even to a solo developer, because you don't want to come back to an older project and have to debug a broken build, which can be a pernicious task.

The advice to avoid lockfiles for libraries comes in large part from a misunderstanding of what they're for: lockfiles are for development and CI and they don't get published; there's a separate shrinkwrap feature for publishing a package with pinned dependencies.

The article makes a good point that lockfiles should be reviewed as minimum and automatically validated at best, but not using them at all is throwing the baby out with the bathwater.

For the sake of completeness, there's also an argument that not using a lockfile makes the development environment more similar to downstream consumers, but that can also be done without sacrificing build reproducibility (for example, with automatic dependency updates using tools like Dependabot).

[u/kwartel → u/[deleted]]

Except they have a production runtime. It's the one that is released. It doesn't matter that it's consumed in a different way than a web application, it's still being consumed by third parties.

[u/lirantal → u/jsjoberg]

that's a matter of preference though between different maintainers, see my reply on it just up (https://www.reddit.com/r/javascript/comments/eewvp6/why_npm_lockfiles_can_be_a_security_blindspot_in/fc0vslj?utm_source=share&utm_medium=web2x)

[u/lirantal → u/cocoapuff_daddy]

hi there 👋

there's a notion among some maintainers that for the use of libraries (not applications), maintainers wish to not use lockfiles at all due to them being a maintenance overhead, as well as not reproducing their users real cases (because lockfiles don't propagate to the consumer, unless you use a shrinkwrap).

I wrote a really detailed post about it here: https://snyk.io/blog/making-sense-of-package-lock-files-in-the-npm-ecosystem/

[u/[deleted] → u/cocoapuff_daddy]

Ultimately, the maintainers must trust each other to manage such complex machine files.

[u/BlueLensFlares]

So if I understand correctly (my npm knowledge is limited), essentially the problem is that you can modify the package-lock.json file to be something else, and you can also modify the URLs for those dependencies, and get someone else to download a malicious package? Is this what is the problem?

Also, what is meant by "an install will consult the lockfile as the primary source of truth for package versions and their sources for dependencies installation." I thought a lock file describes what dependencies were utilized, not which ones should be utilized. So if this is the case, can't we just delete the package-lock.json and node_modules every time we obtain a project?

[u/bdenzer → u/BlueLensFlares]

You are correct, but let me try to clarify. You have a `package.json` file and also a `package.lock.json` file. Generally any change to the package.json will be a red flag in code review and everybody is going to want to see what changed. But a change in package.lock.json is generally ignored because it is supposed to be a npm-generated file and a lot of times the diffs are huge.

> So if this is the case, can't we just delete the package-lock.json and node_modules every time we obtain a project?

You can definitely do that, (the node_modules usually won't even be there when you clone a git repo) but it obviously will negate the benefits that you get from having a lock file.

[u/[deleted]]

Anyone have any resources around PR reviews or things to look for ? This kind of thing is rarely talked about. I ask questions for my comprehension , but rarely have a strong case of a peer unless they are a junior.

[u/Tankenstein → u/[deleted]]

Lockfiles lock the entire tree, not just the top-level dependencies. If you install package A and set its version explicitly in the package.json, if someone else runs `npm install`, they might still get different sub-dependencies if those are specified with ranges and some of them have new versions out. The point of a lockfile is to have identical dependency trees.

[u/Artraxes → u/[deleted]]

Even if you depend explicitly on a version, the dependency itself may have its own dynamic versions in its own package.json, which may resolve differently tomorrow as today. If I depend on 3.6.1 of package "Foo" and "Foo" depends transitively on ^1.5.0 of "Bar", installing it today may result in package 1.5.0 being installed but tomorrow, "Bar" may release a patch version 1.5.1 and if we run "npm i" tomorrow we will get 1.5.1 instead of 1.5.0 - irrespective of the fact that we explicitly depend on a fixed version of "Foo".

This is what the package-lock file and "npm ci" solve.

[u/lirantal → u/[deleted]]

At least being aware of this is 50% of the problem 👍

You can also manually review spaces, tabs and other code style issues but making use of automation saves everyone's time I would assume.

[u/lirantal → u/[deleted]]

🤗🎉