package-lock.json (missing resolved/integrity) not noticed and not repaired automatically by npm

[DavHau]

Is there an existing issue for this?

• I have searched the existing issues

This issue exists in the latest npm version

• I am using the latest npm

Current Behavior

In order to assure reproducible installations, every package listed in a package-lock.json which is fetched from a registry should contain a resolved and integrity field.

This doesn't seem to be the case looking at some existing lockfileVersion = 2 based file.
See for example: https://raw.githubusercontent.com/directus/directus/2938821be05eaf195872c34eed709ac9b4a430b4/package-lock.json

Inspecting the entries for camelcase@6.2.0 (and many others), neither resolved nor integrity exist.

Checking out the repository and executing npm install happily installs camelcase@6.2.0 while:

• not complaining about the missing integrity
• not adding the missing information to the package-lock.json file

To fix the lock file, one currently has to:

• delete all node_modules directories
• delete the package-lock.json file
• execute npm install

Expected Behavior

• When the integrity field is missing for a package, a warning/error should be shown to the user
  (the problem should be of equivalent importance than a mismatching integrity)
• The broken package-lock.json file should be repaired somehow
  (not necessarily automatically, but the user should be informed about the problem and instructed on how to fix it)

Steps To Reproduce

> git clone https://github.com/directus/directus
> cd directus
> git checkout 2938821be05eaf195872c34eed709ac9b4a430b4
> npm install

lock file is still broken (check entry camelcase@6.2.0)

Environment

• npm: 8.5.1
• Node.js: v16.14.0
• OS Name: docker node:16
• npm config:

; node bin location = /usr/local/bin/node
; cwd = /
; HOME = /root
; Run `npm config ls -l` to show all defaults.

[fritzy]

Warning due to missing integrity might be a good idea, but requires some discussion. This would be a great thing to bring up at an RFC meeting since this would be new behavior. Additionally, npm install without a package spec does not mutate package-lock.json. These behaviors would be new.

We don't currently consider a lack of integrity to be a "broken" package-lock.json, as there are several reasons that a package might not have an integrity field.

If you want to update the package-lock.json, npm install camelcase[@spec].

@DavHau Thanks for filing this! Since this is working as intended, can you please open an RFC issue with some details on how you could/would like to see this changed?

[fritzy]

I brought this up in today's RFC meeting, and the consensus was that we do want to change this behavior.

Actions:

• log a warning for missing integrity values (both from lock and package retrieval)
• if an integrity value is included (like from a registry), update the package-lock.json with that value

This means that we'll get warnings from local file, tarball, and git references, as well as packages from registries that do not include an integrity. It also means that the package-lock.json will mutate upon npm-install even if a spec is not included.

[DavHau]

It's good to see that the RFC team wants to move forward on improving this.
Just my 2 cents:

• I think a missing integrity should be treated as a mismatching integrity.
• A mismatching/missing integrity should not only trigger a warning, but raise a critical error requiring immediate action by the user.

If the reason integrity hashes are stored within the lock file, is to ensure reproducibility and security, then any mismatch must be treated as a critical error. If it's not, then what is the point of storing integrity hashes in the first place?

EDIT: OK, I just noticed that NPM actually does trigger an error on a integrity mismatch, if the package isn't cached. That's good. But as said, I think a missing integrity should be treated exactly the same (except if there is a good reason why integrity isn't needed, like with local/symlinked modules etc.)

[ljharb]

Why? If there’s a missing integrity value, you wouldn’t want every install to suddenly start failing. It’s about maximal reproducibility, not guaranteed reproducibility.

[DavHau]

If there’s a missing integrity value, you wouldn’t want every install to suddenly start failing.

If you really care about security and reproducibility, you'd want exactly that. Because it protects you from threat/breakages and it gives you the chance to fix it.

AFAIU, the lock file can reference arbitrary tarball URLs and git repos. Integrity checking not being enforced results in a lot of potential attack surface, and things that can go wrong accidentally.

In case you want to ignore the error and continue anyways, you could have a flag like --ignore-integrity.
The important point is, it forces you to do something in a situation where security/reproducibility cannot be guaranteed, which is appropriate.
If the user wants to ignore this circumstance, that's fine, but the decision should be up to the user.

It’s about maximal reproducibility, not guaranteed reproducibility.

Your idea of maximal seems to be different than mine.
If technology allows you to guarantee reproducibility, but you decide to not make use of it, then how can the reproducibility be maximal?

[ljharb]

It’s as much as is possible, without breaking the common case. If you care that much about security, how did a non-registry dep or a missing integrity value get committed in the first place?