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

Pnpm support #48

Closed tunnckoCore closed 1 year ago

tunnckoCore commented 4 years ago

Is your feature request related to a problem? Please describe.

Support for the @pnpm lockfile linting.

Describe the solution you'd like

I quickly looked over the source here and what I found is that it would be quite easy to add support and it could start from the lockfile-lint-api/src/ParseLockfile.js file.

We could probably use @pnpm/lockfile-file, but I don't see exposed api there for "parsing" there is only methods that accept the path to the lockfile. Here, we need the final "parsed" content of the file. /cc @zkochan

And yet, I'm not sure if that's the only place that changes are needed.

Describe alternatives you've considered

None.

lirantal commented 4 years ago

@tunnckoCore indeed I believe the change is only limited to that parsing function/file. However, notice that we might be changing the way that the parsed results are represented (right now an object, but we'll be changing it to a link based on discussion in #49)

mcmxcdev commented 2 years ago

I was wondering if pnpm is already supported or not?

Looking at the readme and the source code it doesnt look like it, but when running npx lockfile-lint --path pnpm-lock.yaml, I receive a ✔ No issues detected log statement. Is this misleading and just exiting with success because it doesn't understand the lockfile format?

lirantal commented 2 years ago

It isn't. I wonder if that might be a false positive just because the code doesn't throw an error and catches it silently, or that maybe the format is supported. I haven't looked at it recently but we definitely don't have native support for pnpm here.

If you want to take a look at it, I'm happy to merge a PR.

lirantal commented 1 year ago

Looking into a pnpm lockfile, for example:

lockfileVersion: '6.0'

settings:
  autoInstallPeers: true
  excludeLinksFromLockfile: false

dependencies:
  debug:
    specifier: ^4.3.4
    version: 4.3.4
  moment:
    specifier: ^2.29.4
    version: 2.29.4

packages:

  /debug@4.3.4:
    resolution: {integrity: sha512-PRWFHuSU3eDtQJPvnNY7Jcket1j0t5OuOsFzPPzsekD52Zl8qUfFIPEiswXqIvHWGVHOgX+7G/vCNNhehwxfkQ==}
    engines: {node: '>=6.0'}
    peerDependencies:
      supports-color: '*'
    peerDependenciesMeta:
      supports-color:
        optional: true
    dependencies:
      ms: 2.1.2
    dev: false

  /moment@2.29.4:
    resolution: {integrity: sha512-5LC9SOxjSc2HF6vO2CyuTDNivEdoz2IvyJJGj6X8DJ0eFyfszE0QiEd+iXmBvUP3WHxSjFH/vIsA0EN00cgr8w==}
    dev: false

  /ms@2.1.2:
    resolution: {integrity: sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==}
    dev: false

What is there to lint here in terms of trust policies?

The most important bit which is the tarball URL like in yarn/npm isn't included here and is fetched via pnpm's configuration for the remote registry. Integrity isn't that of an issue. I think it leaves us basically with figuring out how would pnpm behave if you specifically manipulate the dependencies in the lockfile, such as adding new entries there. Happy to receive more input on this if someone wants to share some insights.

lirantal commented 1 year ago

I just ran a small test for manipulating the lockfile and pnpm handles this well without mutating the installed dependencies at all if any changes made to the pnpm lockfile are out of sync with the package.json. Looks good to me and I think we can close, concluding that pnpm is pretty solid here in terms of safe behavior.

mcmxcdev commented 1 year ago

Happy to hear!

Looks good to me and I think we can close, concluding that pnpm is pretty solid here in terms of safe behavior.

But isn't https://github.com/pnpm/pnpm/issues/4361 still open and relevant?

lirantal commented 1 year ago

Thanks for referencing that, I chimed in there.