feat: [OSM-1024] pnpm dep graph builder

snyk / nodejs-lockfile-parser

Generate a Snyk dependency tree from package-lock.json or yarn.lock file

Other

56 stars 28 forks source link

feat: [OSM-1024] pnpm dep graph builder #218

Closed gemaxim closed 2 months ago

gemaxim commented 3 months ago

[x] Tests written and linted
[x] Documentation written / README.md updated https://snyk.io/docs/snyk-for-node/
[x] Follows CONTRIBUTING agreement
[x] Commit history is tidy https://git-scm.com/book/en/v2/Git-Branching-Rebasing
[x] Reviewed by Snyk team

What this does

Extends functionality with dependency graph builder for pnpm lockfiles with versions 5.x and 6.x (corresponding to pnpm@7 and pnpm@8).

Notes for the reviewer

Check out the added fixtures for pnpm lockfile v5 and pnpm lockfile v6. 'package.json' inputs taken from npm and yarn fixtures and checked for the same expected dependency graph. pnpm lockfiles do not store bundled dependencies. (related to 2 test cases). Links about this: https://docs.gitlab.com/ee/user/application_security/dependency_scanning/ , https://github.com/pnpm/pnpm/issues/7576.
Ported a few related errors to error catalog (only related to new functionality, as we still need to see if cli correctly handles them), need to update @snyk/error-catalog-nodejs-public for tests to pass.

More information

OSM-1024

CLAassistant commented 3 months ago

All committers have signed the CLA.

weyert commented 3 months ago

Great job, it's looking good 👍 I hope my draft PR was a bit helpful :)

You might want to consider to also support lock file v7 is will be the basis for the upcoming pnpm v9. Currently in beta.

You could convert the v7 back into v6 format and use that for the rest of the code.

Check out the added fixtures for pnpm lockfile v5 and pnpm lockfile v6. 'package.json' inputs taken from npm and yarn fixtures and checked for the same expected dependency graph. pnpm lockfiles do not store bundled dependencies. (related to 2 test cases). Links about this: https://docs.gitlab.com/ee/user/application_security/dependency_scanning/

Yups, that's correct. I added the pnpm support for Gitlab.

gemaxim commented 2 months ago

Great job, it's looking good 👍 I hope my draft PR was a bit helpful :)

You might want to consider to also support lock file v7 is will be the basis for the upcoming pnpm v9. Currently in beta.

You could convert the v7 back into v6 format and use that for the rest of the code.

Check out the added fixtures for pnpm lockfile v5 and pnpm lockfile v6. 'package.json' inputs taken from npm and yarn fixtures and checked for the same expected dependency graph. pnpm lockfiles do not store bundled dependencies. (related to 2 test cases). Links about this: https://docs.gitlab.com/ee/user/application_security/dependency_scanning/

Yups, that's correct. I added the pnpm support for Gitlab.

Thanks @weyert ! 😄 I saw your PR and wanted to link my changes here. Really appreciate your work and taking the time to look over this. As for pnpm v9, I think we'll take it into account once it's not beta, should the new lockfile version not be backwards compatible.

weyert commented 2 months ago

@gemaxim It's definitely not backward compatible, main reason for the major version bump actually, some of the changes:

dep path changes a bit, name and id fields are dropped (see: https://github.com/pnpm/pnpm/pull/7752)
reducing duplications by splitting up packages into packages and snapshots, were the latter contains the most details (see: https://github.com/pnpm/pnpm/pull/7700)

snyksec commented 2 months ago

:tada: This PR is included in version 1.53.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

weyert commented 2 months ago

Great work 👍