Additional validations

[MikeRalphson]

From some quick testing, it appears the following issues are not currently validated:

1. Ensure every dependencies entry in package-lock.json has an integrity property with a non-empty string.
2. Ensure the resolved URL package name matches the dependency it should resolve, to prevent substitutions
3. Ensure the version number within the resolved URL matches the dependency version it should resolve, to prevent down-levelling to introduce bugs

[lirantal]

(1) is a nice idea for a flag, agree.

Can you expand what you mean by (2)?

[MikeRalphson]

Given this snippet of package-lock.json:

{
  "dependencies": {
   "jquery": {
      "version": "3.5.1",
      "resolved": "https://registry.npmjs.org/jquery/-/jquery-3.5.1.tgz",
      "integrity": "sha512-XwIBPqcMn57FxfT+Go5pzySnm4KWkT1Tv7gjrpT1srtf8Weynl6R273VJ5GjkRb51IzMp5nbaPjJXMWeju2MKg=="
    }
}

It appears that if I change this to:

{
  "dependencies": {
   "jquery": {
      "version": "3.5.1",
      "resolved": "https://registry.npmjs.org/malicious-jquery/-/malicious-jquery-3.5.1.tgz",
      "integrity": "sha512-MYNEWINTEGRITYVALUE"
    }
}

If jquery@3.5.1 is not in the npm cache, it will use the resolved URL to fetch and install the malicious package in place of the one expected.

Proposed validation (3) was a variant of this where the first example was changed to:

{
  "dependencies": {
   "jquery": {
      "version": "3.5.1",
      "resolved": "https://registry.npmjs.org/jquery/-/jquery-3.5.0.tgz",
      "integrity": "sha512-MYNEWINTEGRITYVALUE"
    }
}

where the intention is to pin the version installed to one with a known security flaw.

Log of npm fetching the wrong version:

npm i --verbose
npm info it worked if it ends with ok
npm verb cli [
npm verb cli   '/home/mike/.nvm/versions/node/v14.15.1/bin/node',
npm verb cli   '/home/mike/.nvm/versions/node/v14.15.1/bin/npm',
npm verb cli   'i',
npm verb cli   '--verbose'
npm verb cli ]
npm info using npm@6.14.8
npm info using node@v14.15.1
npm verb npm-session e0072736effa09cf
npm info lifecycle t@1.0.0~preinstall: t@1.0.0
npm timing stage:loadCurrentTree Completed in 15ms
npm timing stage:loadIdealTree:cloneCurrentTree Completed in 0ms
npm http fetch GET 200 https://registry.npmjs.org/jquery/-/jquery-3.5.0.tgz 569ms
npm timing stage:loadIdealTree:loadShrinkwrap Completed in 632ms
...
npm info lifecycle jquery@3.5.1~preinstall: jquery@3.5.1
...

[lirantal]

I see. So in (2), the check in place would actually be to ensure the dependency key jquery matches the path name jquery too? Is this how you would actually apply the test? I'm wondering if this is something that is:

1. Cross-compatible between npm and yarn's lockfile 2.An expected format that is guaranteed to have that structure (thinking about scoped packages and other strings in the package name).

What do you think?

Also, nothing prevents you from breaking this up to multiple PRs - probably ideal anyway. So you're welcome to push them in, one by one. Is that ok?