Regarding the general approach: wouldn't you agree that supporting a version-less dependency (raising all vulnerabilities for any version of the dependency ) would be a way better approach than skipping the dependency when the package-lock contains no version? (which I would expect to be an improper package-lock by the way, as in my reading of NPMs package-lock documentation there should always be a version field).

Regarding the general approach: wouldn't you agree that supporting a version-less dependency (raising all vulnerabilities for any version of the dependency ) would be a way better approach than skipping the dependency when the package-lock contains no version? (which I would expect to be an improper package-lock by the way, as in my reading of NPMs package-lock documentation there should always be a version field).

Hello! 👋

which I would expect to be an improper package-lock by the way, as in my reading of NPMs package-lock documentation there should always be a version field

Yes, absolutely, as stated by https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json#dependencies.

What made me make this choice was the OP package-lock.json inspection. My comment in #4293 is not detailed enough, I am sorry about that. Here are more details:

So as stated in #4293, the NPE is raised by the following dependency mention:

    "node_modules/jest-resolve": {
      "dev": true,
      "optional": true,
      "peer": true
    },

This is not the only place in the package-lock.json this dependency is mentioned. We do see it multiple times as it seems to be a transitive dependency of multiple jest modules.

For example, here it is mentioned in the jest-runtime resolution:

"node_modules/@jest/test-sequencer/node_modules/jest-runtime": {
      "version": "27.4.6",
      "resolved": "https://registry.npmjs.org/jest-runtime/-/jest-runtime-27.4.6.tgz",
      "integrity": "sha512-eXYeoR/MbIpVDrjqy5d6cGCFOYBFFDeKaNWqTp0h6E74dK0zLHzASQXJpl5a2/40euBmKnprNLJ0Kh0LCndnWQ==",
      "dev": true,
      "dependencies": {
        "@jest/environment": "^27.4.6",
        "@jest/fake-timers": "^27.4.6",
        "@jest/globals": "^27.4.6",
        "@jest/source-map": "^27.4.0",
        "@jest/test-result": "^27.4.6",
        "@jest/transform": "^27.4.6",
        "@jest/types": "^27.4.2",
        "chalk": "^4.0.0",
        "cjs-module-lexer": "^1.0.0",
        "collect-v8-coverage": "^1.0.0",
        "execa": "^5.0.0",
        "glob": "^7.1.3",
        "graceful-fs": "^4.2.4",
        "jest-haste-map": "^27.4.6",
        "jest-message-util": "^27.4.6",
        "jest-mock": "^27.4.6",
        "jest-regex-util": "^27.4.0",
        "jest-resolve": "^27.4.6",
        "jest-snapshot": "^27.4.6",
        "jest-util": "^27.4.2",
        "slash": "^3.0.0",
        "strip-bom": "^4.0.0"
      },
      "engines": {
        "node": "^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0"
      }
    },

jest-resolve ends up being resolved to the 27.4.6 version in the sample lock file. We can see it during the resolution of the transitive dependencies, like here for jasmine:

"node_modules/jest-jasmine2/node_modules/jest-resolve": {
      "version": "27.4.6",
      "resolved": "https://registry.npmjs.org/jest-resolve/-/jest-resolve-27.4.6.tgz",
      "integrity": "sha512-SFfITVApqtirbITKFAO7jOVN45UgFzcRdQanOFzjnbd+CACDoyeX7206JyU92l4cRr73+Qy/TlW51+4vHGt+zw==",
      "dev": true,
      "dependencies": {
        "@jest/types": "^27.4.2",
        "chalk": "^4.0.0",
        "graceful-fs": "^4.2.4",
        "jest-haste-map": "^27.4.6",
        "jest-pnp-resolver": "^1.2.2",
        "jest-util": "^27.4.2",
        "jest-validate": "^27.4.6",
        "resolve": "^1.20.0",
        "resolve.exports": "^1.1.0",
        "slash": "^3.0.0"
      },
      "engines": {
        "node": "^10.13.0 || ^12.13.0 || ^14.15.0 || >=15.0.0"
      }
    },

The cause of the issue is coming from jest-pnp-resolver, which reference jest-resolve as a peer dependency, without specifying any version:

    "node_modules/jest-pnp-resolver": {
      "version": "1.2.2",
      "resolved": "https://registry.npmjs.org/jest-pnp-resolver/-/jest-pnp-resolver-1.2.2.tgz",
      "integrity": "sha512-olV41bKSMm8BdnuMsewT4jqlZ8+3TCARAXjZGT9jcoSnrfUnRCqnMoF9XEeoWjbzObpqF9dRhHQj0Xb9QdF6/w==",
      "dev": true,
      "engines": {
        "node": ">=6"
      },
      "peerDependencies": {
        "jest-resolve": "*"
      },
      "peerDependenciesMeta": {
        "jest-resolve": {
          "optional": true
        }
      }
    },

I thought because it was mentioned as a peer dependency, the dependency itself must be declared somewhere else to be imported, so this very specific dependency object declared could be specified as a duplicate.

After you comments and more thought, I'd say there is at least a flaw in my implementation. I should check at least the dependency is flagged as "peer" when ignoring because of no version.

Your point looks also very valid. What if the only mention of a dependency is a wildcard as a peer? This looks kind of blurry as NPM seems to not have the same behavior given it's major version. I think it will end up being resolved by NPM as the latest version but some tests from my side would be needed.

The point I would like to clarify with you is what do you think should we do in the OP case, which is we have a wildcard defined but we end up having a specific version number resolved in the file? Should we ignore the wild card to prevent raising too much false positive?

I am putting this PR as a draft to prevent it being merged yet. Once you have the time to reply, I will work toward a solution that fits your specification (handling no version as wildcard + precision about the OP case).

Thank you for your help!

Checking for peer dependency would definitely be an improvement, however, not sure if it would be sufficient as npm appears to install the peer as needed when not installed (in versions 1.x, 2.x , removed in 3.x and re-added with improvements in 7.x) according to https://github.blog/2020-10-13-presenting-v7-0-0-of-the-npm-cli/

Not deep enough into the NPM ecosystem to be able to say for certain whether or not this would get reflected elsewhere in the package-lock.json or only take effect in the node_modules structure.

Checking for peer dependency would definitely be an improvement, however, not sure if it would be sufficient as npm appears to install the peer as needed when not installed (in versions 1.x, 2.x , removed in 3.x and re-added with improvements in 7.x) according to https://github.blog/2020-10-13-presenting-v7-0-0-of-the-npm-cli/

Not deep enough into the NPM ecosystem to be able to say for certain whether or not this would get reflected elsewhere in the package-lock.json or only take effect in the node_modules structure.

Ok, so I will go with what you suggested in your first comment. It will be enough to fix the bug. I will update this PR and ask for a new review once I am done. Thank you. 👍

@nhumblot can you email me? jeremy.long @ gmail.com.

Updated the change so Dependency Check does not ignore a dependency if it does not have a version.