-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pnpm support? #111
Comments
I would like to work on this. |
I'm sorry to ping you directly @lirantal but the issue was open 12 days ago and I'm still waiting on an answer |
Again I'm sorry and notifying random contributors but can you have a look at this issue @lili2311 ? |
hi @GermainBergeron apologies for slow response I am not longer directly involved in this plugin, I have however shared this with the relevant team. Is the ask here that you would like to collaborate on the support for the pnpm lockfile? |
Hi @GermainBergeron, I am Steph and I am a Product Manager at Snyk. Thank you for your request! Whilst we think this would be a good addition to our current offering due to other priorities we won't be able to work on this anytime soon, sorry about that. I have noted your request down and I will make sure to keep you posted If anything changes. Kind Regards, |
If I or @abdulhannanali manage to get it working in this repository, would it be something that you would consider @steph-herd-snyk-pm? Is there any other big piece missing to integrate it in the Snyk CLI? Thanks |
👋 hi @GermainBergeron @abdulhannanali while we do welcome contributions, in this particular case adding 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 Could you help us gather some more requirements on this as well while we have you here:
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 |
Hey @lili2311, thanks for your response! This is why I asked, I figured it could be a lot more complex than just adding the parser in here. To answer your questions:
I'll have a look at the Github action, it's probably better than our current workaround 🙏 |
Thanks, @lili2311! |
We have a custom script based on the previously mentioned Github action but it's not working currently since we are in a workspace and the package-lock doesn't match the package.json at the root of our workspace (0 dependencies detected). We're thinking about different improvements but they all seem hackish:
|
Hi @GermainBergeron, I am Mathilde, I am working with @lili2311 on this issue. I have a question on your project. So you have a pnpm-lock, a package.json and a pnpm-workspace.yaml at the base of the project, is that right? |
Hey Mathilde, We have a single pnpm-lock.yaml at the root folder along with a pnpm-workspace.yaml and a package.json. Our different packages all have a package.json but no pnpm-lock.yaml. It looks like this:
|
Ok thank you. |
Hi @GermainBergeron, We should be releasing a solution supporting workspaces in the next couple of days. The solution we have at the moment doesn't use the CLI but the API, we will release a tool to that use the nodejs-parser to produce a depTree convert it into a depGraph and send it to the API. Thank you, |
An update on pnpm parser: We will also release a GitHub action later on. If you wan to try the snyk-pnpm-depTree-api-tool you can install it using : npm i -g snyk-pnpm-depTree-api-tool Then to run it with npm snyk-pnpm-deptree-api-tool -—root ‘yourProjectPath’ —-orgId ‘yourOrganisation’ —-snykToken ‘yoursnykToken’ —-includeDev ‘false’ https://www.npmjs.com/package/snyk-pnpm-deptree-api-tool Thank you |
hi, Here is the github action: https://github.com/snyk-tech-services/snyk-pnpm-github-action Thank you |
Thanks a lot @mathild3r, We'll have a look as soon as possible, it seems to fit our use case quite well 🎉 |
Hello @GermainBergeron, Did you had a chance to try the tool? Thank you |
I have a few issues with the tool, I tested it in two different packages: Here's the stack trace for the first repository when I use the full path to the repository:
The repository doesn't seem that big to me, so I'm a bit confused about why it would break. If I change the
The second one is a private repository which is a bit more complex (we have packages inside a
Side notes:
|
Hi Germain, Thank you for your email. I will have a look at the issues shortly. For the second repo, is look like it's a workspace project, would you be able to share the pnpm-workspace.yaml? the tool probably haven't found one of the files. The tool is not open source at the moment, I am sorry, but I will discuss it with the team. Thank you, |
For the root option I tried multiple things, but I got the call stack size exceeded with The pnpm-workspace.yaml look like this for the second repository: packages:
- 'build/*'
- 'packages/*'
- 'core/*'
- 'packages/package-a/cypress/'
- 'packages/package-b/cypress/'
|
Hi Germain, Apologies for the late answer, I had to look at something else. I look at your issue a bit closer today and it looks like we are stuck in a loop: I will let you know when I have a fix. Thank you |
Hi Germain, Apologies for the delay. Let me know, Thank you, |
Hey Mathilde, I tried running the tool with different configurations of my repository but I always get
Since the repository is open source, can you try to run the tool on it? |
Hi, yes I will and let you know mid next week. Thank you |
Hi Germain, I would like to apologies for the delay to resolve your problems. it would find the ones linked that are under packages (ie: demo linked to vapor) but if vapor linked to helpers/enzym-redux it not capable of finding it. I am working on a solution and will update you before the end of the week. Thank you, |
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. |
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 |
We currently use blackduck as that supports pnpm. We were hoping to move to snyk, however this is blocking us |
You could use https://snyk.io/code-checker/sbom-security/ |
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 To be clear, this GitHub Actions workflow would enable Snyk with pnpm, by performing the following steps (only on Snyk PRs):
|
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 . |
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 |
Thee main value is diff blocking of bad dependencies that are unsafe. Running All of this is definitely best effort |
Try the new package with pnpm support |
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 Does this mean snyk now supports pnpm? Or is this some separate tool? |
@gemaxim I think we can close this ticket as the pnpm lock file reading/parsing has been added with your PR |
@gemaxim has 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. |
Hello @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 snyk/snyk-nodejs-plugin#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. |
Thank you!
Is there a place that we can subscribe to updates on this? |
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 |
Received a PR just now from Snyk in one of our projects that does not update the pnpm lockfile ( So maybe as per @gemaxim's comments above, this fix is still pending some additional work:
|
Update – This issue was solved – the project I was testing with had only 🤔 I'm trying to use this functionality via Snyk CLI ( This is in a conventional repo with a single 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.
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 |
Hello! I apologise for the late reply, I'm not always following this thread. @gilest The plugin has indeed been added to the cli preview under a feature flag. I notice it is working for you project, as it is detected as @karlhorky Indeed, this only includes CLI functionality, and is still under a feature flag. CLI functionality includes test and monitor - monitor displays and monitors your project in the UI, includes vulnerabilities fix advice, but not the Fix PR functionality. |
Thank you @gemaxim . I had resolved this issue but I didn't return here to update my comment. I have done so now. As penance I will share the GitHub actions I have setup organisation-wide to temporarily monitor pnpm dependencies in many repos during the private beta. Monitor Snyk dependencies with pnpm during private beta (Github Actions) |
@gemaxim I guess the GitHub bot closing this was a mistake / accident? I'm assuming that Snyk is still working on pnpm lockfile support? |
@karlhorky we have support for pnpm in the Snyk CLI. You can enable it by going to you organization's Settings -> Snyk Preview page and enable |
Yeah, this is the part that I observed recently that is not yet complete. Should this issue be reopened to track that? Or otherwise, in which public issue is it tracked? |
@gemaxim (or whomever can answer), its been a few months without an update, and I have the same question as @karlhorky. I would be happy to follow along wherever I can so I know when it might be safe to use pnpm more extensively. It looks like your answer from October is still the state of the system. |
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!
The text was updated successfully, but these errors were encountered: