snyk / nodejs-lockfile-parser

Generate a Snyk dependency tree from package-lock.json or yarn.lock file
Other
56 stars 28 forks source link

Add pnpm support? #111

Open GermainBergeron opened 3 years ago

GermainBergeron commented 3 years ago

We recently switched our package manager from npm to pnpm since it reduce our install time by multiple minutes in our monorepo. Since then our Snyk scans are failing, as we should have expected. We hacked something to generate a package-lock from the pnpm-lock.yaml but we'd like to have a more robust solution.

Can we start working on a PR to add pnpm support? I saw that you already have a parser for yarn that uses yaml and I think we could reuse some logic.

Thanks!

sanderkooger commented 1 year ago

@milahu,

That or the company that gets paid for this functionality ( Snyk ) does their job and parses it themselves. Its either that or im moving Security scanning over to a company that does want to support their clients.

We use Snyks coupling with our repo to do security scanning to minimize any code implementation. Also, i would not use Upwork... probably a developer from my own team if i needed to do this.

We are just not in the habit of building hacky workarounds for commercial enterprises. We only do that for open source initiatives... with a nice sum of money to keep such projects going.

@weyert is gymnasium a good product for monorepo (NODE, TS, GO, Python) vulnerability scanning? if so we might be inclined to switch

weyert commented 1 year ago

@sanderkooger Yeah, I am happy with Gemnasium for vulnerability scanning. Happy enough to learn Golang for it and added PNPM support which I will keep maintaining as long I am using PNPM :) I already added draft support for PNPM v8 lock files

Yeah, we are using those languages too and I am happy with it’s support.

akinnee commented 1 year ago

We'd also like to see support for pnpm-lock.yml files added.

weyert commented 1 year ago

@akinnee Looks like Snyk / @lili2311 is blocking the adding of pnpm support. As they don’t want to accept a PR for adding parsing support until backend work is done which doesn’t to happen.

akinnee commented 1 year ago

What backend work does this need? Can't we just extract a list of dependencies and send it to snyk?

weyert commented 1 year ago

@akinnee You would think so but @lili2311 said the following few years ago: https://github.com/snyk/nodejs-lockfile-parser/issues/111#issuecomment-867736291

Maybe it’s not valid anymore?

jorenbroekema commented 1 year ago

Just my 2 cents here but I think package managers like NPM, PNPM, Yarn, should publish and maintain their own lockfile parsers. They create the lockfiles, they should be able to provide and keep in sync (lockfiles formats tend to change in major updates) a parser for such files.. Are there any open issues for the 3 package managers regarding this, and if so, what has generally been the response to that?

I get that responsibility is put on a service (like Snyk) that among other things, scans dependency installations, and is therefore directly reliant on lockfiles, but are they really the correct place to put these parsers?

weyert commented 1 year ago

They could use @pnpm/lockfile-walker (https://www.npmjs.com/package/@pnpm/lockfile-walker)?

holgergp commented 7 months ago

Hi! Anything new on this? We would also love to see pnpm support. The manual rework of a javascript-snyk PR is a little tedious at times. Thank you! 😀

karlhorky commented 7 months ago

@JamesPatrickGill since you look like an active maintainer here, what do you think about the suggestions above?

Eg. the lowest-effort seems to be to use an existing package to parse the pnpm lockfile format - @pnpm/lockfile-walker was suggested by @weyert here:

karlhorky commented 7 months ago

To be clear, currently the Snyk "Automatic Fix PRs" feature leads to PRs that fail CI / deployment when used with pnpm:

Example PR: https://github.com/upleveled/mdx-local-link-checker/pull/220

Screenshot 2023-11-27 at 16 36 57

Screenshot 2023-11-27 at 16 37 43

Screenshot 2023-11-27 at 16 40 34

Screenshot 2023-11-27 at 16 41 43

Error as text:

pnpm install
 ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with package.json

Note that in CI environments this setting is true by default. If you still need to run install in such cases, use "pnpm install --no-frozen-lockfile"

    Failure reason:
    specifiers in the lockfile ({"@mdx-js/mdx":"1.[6](https://github.com/upleveled/mdx-local-link-checker/actions/runs/7006519124/job/19058529116?pr=220#step:5:7).22","micromatch":"4.0.5","remark-slug":"[7](https://github.com/upleveled/mdx-local-link-checker/actions/runs/7006519124/job/19058529116?pr=220#step:5:8).0.1","unified":"11.0.4","unist-util-remove":"4.0.0","walk-sync":"3.0.0","@types/micromatch":"4.0.6","@types/node":"20.[9](https://github.com/upleveled/mdx-local-link-checker/actions/runs/7006519124/job/19058529116?pr=220#step:5:10).5","@types/unist":"3.0.2","eslint-config-upleveled":"7.3.3","typescript":"5.3.2"}) don't match specs in package.json ({"@types/micromatch":"4.0.6","@types/node":"20.9.5","@types/unist":"3.0.2","eslint-config-upleveled":"7.3.3","typescript":"5.3.2","@mdx-js/mdx":"2.0.0","micromatch":"4.0.5","remark-slug":"7.0.1","unified":"[11](https://github.com/upleveled/mdx-local-link-checker/actions/runs/7006519124/job/19058529116?pr=220#step:5:12).0.4","unist-util-remove":"4.0.0","walk-sync":"3.0.0"})
Error: Process completed with exit code 1.
karlhorky commented 7 months ago

Workaround Idea

Set up a GitHub personal access token (eg. with the name SNYK_UPDATE_PNPM_LOCKFILE_GITHUB_TOKEN) with Read and Write access to Content and add a GitHub Actions workflow that runs pnpm install and commits the result, if the last commit message matches a pattern:

For example:

.github/workflows/snyk-update-pnpm-lockfile.yml

name: Update pnpm lockfile after incomplete Snyk commits
on: push

jobs:
  snyk-update-pnpm-lockfile:
    name: Update pnpm lockfile after incomplete Snyk commits
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
        with:
          # Disable configuring $GITHUB_TOKEN in local git config
          persist-credentials: false
      - uses: pnpm/action-setup@v3
        with:
          version: 'latest'
      - uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'
          cache: 'pnpm'

      # Fix Snyk security commits not updating pnpm lockfile
      # https://github.com/snyk/nodejs-lockfile-parser/issues/111#issuecomment-1828103685
      - name: Update pnpm lockfile after incomplete Snyk commits
        if: github.event.commits[0].author.name == 'snyk-bot' && github.event.commits[0].author.email == 'snyk-bot@snyk.io'
        run: |
          pnpm install --no-frozen-lockfile

          git add pnpm-lock.yaml
          if [ -z "$(git status --porcelain)" ]; then
            echo "No changes to commit, exiting"
            exit 0
          fi

          git config user.email github-actions[bot]@users.noreply.github.com
          git config user.name github-actions[bot]

          git commit -m "Update pnpm lockfile after Snyk commit"

          # Credit for oauth2 syntax is the ad-m/github-push-action GitHub Action:
          # https://github.com/ad-m/github-push-action/blob/d91a481090679876dfc4178fef17f286781251df/start.sh#L43-L55
          git push https://oauth2:${{ secrets.SNYK_UPDATE_PNPM_LOCKFILE_GITHUB_TOKEN }}@github.com/${{ github.repository }}.git HEAD:${{ github.ref }}
maxonary commented 7 months ago

Thank you for your implementation @karlhorky! I had to make sure to include --no-frozen-lockfile to pnpm install to avoid the default of breaking when the pnpm-lock.yaml is not up to date with the package.json.

karlhorky commented 7 months ago

@maxonary oh right, forgot about that. Updated my post above, thanks!

ahernd2 commented 5 months ago

Any update on pnpm support in snyk? This is a blocker for us

vrali commented 5 months ago

Any update on pnpm support in snyk? This is a blocker for us

Same here

heath-freenome commented 4 months ago

Any update on pnpm support in snyk? This is a blocker for us

same here

vrali commented 4 months ago

Any updates?? my workaround is to push a yarn lock but now dependabot shows 2 sets of alerts for yarn and pnpm lock files. pnpm is quite popular now, why is it taking over 2 year to add this.

akinnee commented 4 months ago

👋 hi @GermainBergeron @abdulhannanali while we do welcome contributions, in this particular case adding pnpn support would require setting up a new project type in the backend and lots of associated work that comes with it: settings, icons, project type, vulnerabilities, filters. This is why this request would need to be addressed by the team before the any work in the parser can be utilised.

So even if you were to raise the changes needed and they are merged the team would need to complete all the backend work first before the parser is even called for a new project type pnpm.

Could you help us gather some more requirements on this as well while we have you here:

  • are you using pnpm workspaces at all?
  • do you need the support in CLI or are scanning it in a different way?

I also wanted to share a current Github action we have that was a great contribution snyk-tech-services/github-actions-pnpm-snyk it can convert the pnpm lockfile to an npm one and then run snyk cli. It might help you in the mean time.

I don't understand why anything needs to change on the backend for this. Do we not just send a list of dependencies? It seems like we just need this parser to support pnpm-lock.yml.

My current workaround is a script I wrote that creates a new temporary package and installs all the same dependency versions using npm before running snyk. I guess I am missing some of the features, like auto-upgrade PRs from the bot, but I'm not sure how that's better than running pnpm audit --fix.

Aghassi commented 4 months ago

The issue with generating a lock file from another package manager is you aren't guaranteed all the right transitive dependencies. Since it's not 1:1 it's hard for that to be considered a work around at scale.

To the Snyk team:

My company uses PNPM workspaces in a monorepo with 70 first party libraries. We have a single lock file with all dependencies and their versions.

lmajowka-r7 commented 3 months ago

What do you guys use as an alternative to snyk? Also having the same problem here, and it seems it will not be solved soon

weyert commented 3 months ago

What do you guys use as an alternative to snyk? Also having the same problem here, and it seems it will not be solved soon

I am using Gymnasium [1] to generate a SBOM file and then upload the SBOM file to the service. So any Snyk alternative that supports uploading a SBOM file (e.g. [2]) would work. For example [3] provided by Snyk

[1] https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium [2] https://docs.github.com/en/code-security/supply-chain-security/understanding-your-software-supply-chain/using-the-dependency-submission-api [3] https://snyk.io/code-checker/sbom-security/

ahernd2 commented 3 months ago

We currently use blackduck as that supports pnpm. We were hoping to move to snyk, however this is blocking us

weyert commented 3 months ago

You could use https://snyk.io/code-checker/sbom-security/

karlhorky commented 3 months ago

Probably at some point I'm going to get annoyed enough with the broken PRs to create the GitHub Actions workaround I described above

(if someone else doesn't do it first)

In my experience, it's probably about 1 hour of work to get this set up, having done similar things before for Renovate bot running pnpm with --recursive flag and pnpm patch making it hard to automatically upgrade dependencies with bots such as Renovate

To be clear, this GitHub Actions workflow would enable Snyk with pnpm, by performing the following steps (only on Snyk PRs):

  1. Run pnpm install --no-frozen-lockfile
  2. Commit the lockfile
  3. Re-run the CI checks
weyert commented 3 months ago

Personally or at work I don't use Snyk but I have made a draft PR for someone to finish off, for parsing pnpm lock files, the problem I don't understand how to create the dep graph. The lock file is parsed and normalised so if someone has time to finish off this part: https://github.com/snyk/nodejs-lockfile-parser/pull/217/files#diff-2b8fbea4e000278df793e423c85702460e4a17ed9b161d84d05a95f30de13400R39

I think we would be good to go.... Or someone can sponsor (paid in cash or free books) me to do the rest of the development .

akinnee commented 3 months ago

The issue with generating a lock file from another package manager is you aren't guaranteed all the right transitive dependencies. Since it's not 1:1 it's hard for that to be considered a work around at scale.

To the Snyk team:

My company uses PNPM workspaces in a monorepo with 70 first party libraries. We have a single lock file with all dependencies and their versions.

100%. I don't like doing it this way. But our company forced us to hook snyk scanning up in every repo. I'm honestly not sure what value it provides that we don't get for free from npm audit/pnpm audit.

Aghassi commented 2 months ago

The issue with generating a lock file from another package manager is you aren't guaranteed all the right transitive dependencies. Since it's not 1:1 it's hard for that to be considered a work around at scale. To the Snyk team: My company uses PNPM workspaces in a monorepo with 70 first party libraries. We have a single lock file with all dependencies and their versions.

100%. I don't like doing it this way. But our company forced us to hook snyk scanning up in every repo. I'm honestly not sure what value it provides that we don't get for free from npm audit/pnpm audit.

Thee main value is diff blocking of bad dependencies that are unsafe. Running pnpm audit at scale doesn't work too well. If we were on github it would also provide value with the auto fix prs. For our security team, it gives them some reasonable confidence of where the issues in the supply chain are and what projects are unsafe.

All of this is definitely best effort

weyert commented 2 months ago

Try the new package with pnpm support

Aghassi commented 2 months ago

Which one are you referring to? On Apr 11, 2024 at 15:50 -0700, Weyert de Boer @.***>, wrote:

Try the new package with pnpm support — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

weyert commented 2 months ago

https://github.com/snyk/nodejs-lockfile-parser/releases/tag/v1.53.0

ahernd2 commented 2 months ago

@weyert Does this mean snyk now supports pnpm? Or is this some separate tool?

weyert commented 2 months ago

@gemaxim I think we can close this ticket as the pnpm lock file reading/parsing has been added with your PR

karlhorky commented 2 months ago

@gemaxim has snyk-nodejs-lockfile-parser@1.53.0 been integrated in the public Snyk "Automatic Fix PRs" feature already?

Has anyone tested this to be working in a demo repo successfully? Would be good to have some proof in this issue before it is closed.

gemaxim commented 2 months ago

Hello Thanks @weyert for the updates here. I believe this issue can be closed as this specific lockfile parsing repo now has functionality for pnpm lockfiles.

@karlhorky this hasn't been integrated in fix prs yet. For the moment, the newly added pnpm functionality will be shortly integrated in the Snyk CLI through a newly open sourced plugin - see https://github.com/snyk/snyk-nodejs-plugin/pull/2. This will include testing and monitoring, but we'll use a rolling out strategy for pnpm.

The Snyk SCM pnpm functionality in the Snyk UI is still part of a bigger effort so in the near future importing and scanning pnpm projects in the UI won't be possible.

danoc commented 2 months ago

Thank you!

The Snyk SCM pnpm functionality in the Snyk UI is still part of a bigger effort so in the near future importing and scanning pnpm projects in the UI won't be possible.

Is there a place that we can subscribe to updates on this?

lmajowka-r7 commented 2 months ago

Thank you very much, but we decided to stop using snyk because of this issue. Maybe we will evaluate using it again in the future. Thanks to everyone who tried to help

karlhorky commented 1 month ago

Received a PR just now from Snyk in one of our projects that does not update the pnpm lockfile (pnpm-lock.yaml):

So maybe as per @gemaxim's comments above, this fix is still pending some additional work:

@karlhorky this hasn't been integrated in fix prs yet.

The Snyk SCM pnpm functionality in the Snyk UI is still part of a bigger effort so in the near future importing and scanning pnpm projects in the UI won't be possible.

gilest commented 2 weeks ago

🤔 I'm trying to use this functionality via Snyk CLI (1.1292.0-preview.e7bfb1b0fa95ed6b1be0f4b3e6dce19f36b4628b). My org has the pnpm feature flag enabled and the package manager is detected correctly.

This is in a conventional repo with a single pnpm-lock.yaml file and not a monorepo/workspace.

However the dependency graph does not seem to be read, no count of checked dependencies in shown, and the web interface has no dependency data.

snyk test --org=PRIVATE output:

Testing /src/project...

Organization:      private
Package manager:   pnpm
Target file:       pnpm-lock.yaml
Project name:      project
Open source:       no
Project path:      /src/project
Licenses:          enabled

✔ Tested /src/project for known issues, no vulnerable paths found.

Next steps:
- Run `snyk monitor` to be notified about new related vulnerabilities.
- Run `snyk test` as part of your CI/test.

I know this isn't a supported feature yet, but based on what I'm reading and release notes, the supporting plugin has been added to Snyk CLI