lirantal / lockfile-lint

Lint an npm or yarn lockfile to analyze and detect security issues
Apache License 2.0
781 stars 35 forks source link

Does not support NPM lockfile version 3 #168

Closed kusalk closed 1 year ago

kusalk commented 1 year ago

Lockfiles created using the NPM (9.x) that ships with the latest LTS of Node (18.x) use lockfile version 3. Unfortunately, this tool does not support these lockfiles. I suspect all the features do not work, but the options I have tested and confirmed do not work are --validate-https and --validate-integrity.

lirantal commented 1 year ago

Hi @kusalk,

Can you add more background about the problem you're experiencing? What doesn't exactly work? Any extra information and reproduction steps you can provide are helpful.

From quickly looking at it, looks like lockfile-lint does run but it executes quietly and reports no issues found regardless of lockfile issues like not using https and not having a proper integrity field. Is this what you're seeing too?

kusalk commented 1 year ago

@lirantal Yep that's correct

lirantal commented 1 year ago

Ok. I have a fix but the lockfile v3 update of npm presents the packages with their full path on disk, so instead of nodemon it would show node_modules/nodemon.

I was wondering whether to strip the prefix but I think it might be valuable for monorepos with packages spread out in different packages/ folders.

kusalk commented 1 year ago

Hmm strip the prefix for what purpose / part of which check?

lirantal commented 1 year ago

Stripping the prefix of that node_modules/nodemon the way it is reported on lockfile v3 due to the fact that in prior lockfile versions it didn't add that prefix and lockfile-lint will just print the package name (nodemon) as-is. So, to stay compatible.

kusalk commented 1 year ago

Oh gotcha, yeah for reporting purposes we should keep the prefix as it's possible to have the same package in multiple spots.

For example I've got a lockfile at the moment similar to the follow snippet:

    "node_modules/eslint-plugin-import/node_modules/ms": {
      "version": "2.0.0",
      "resolved": "https://packages.atlassian.com/api/npm/npm-remote/ms/-/ms-2.0.0.tgz",
      "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==",
      "dev": true
    },
    "node_modules/connect/node_modules/ms": {
      "version": "2.0.0",
      "resolved": "https://packages.atlassian.com/api/npm/npm-remote/ms/-/ms-2.0.0.tgz",
      "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==",
      "dev": true
    },
    "node_modules/body-parser/node_modules/ms": {
      "version": "2.0.0",
      "resolved": "https://packages.atlassian.com/api/npm/npm-remote/ms/-/ms-2.0.0.tgz",
      "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==",
      "dev": true
    },
    "node_modules/ms": {
      "version": "2.1.2",
      "resolved": "https://packages.atlassian.com/api/npm/npm-remote/ms/-/ms-2.1.2.tgz",
      "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==",
      "dev": true
    },
lirantal commented 1 year ago

@kusalk exactly, but that's not compatible with how we're reporting packages right now so I've stripped the prefix. Perhaps we add a --verbose flag or something instead if we want to keep the full path name?

lirantal commented 1 year ago

@kusalk changes are ready to release, wdyt about the package name? critical if we strip off the prefix?

kusalk commented 1 year ago

@lirantal Not critical. Any error report is better than the current false pass. We should refactor this tool to support the full path in the future given that this will be the format of lockfiles to come.

lirantal commented 1 year ago

@kusalk I'm going to add the full name for the package via the debug capabilities:

DEBUG=lockfile-lint lockfile-lint --path ...

and it would look like this as output: image

This would allow easy resolution when you want to find out more information