Skip to content
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

pnpm: support v9 lockfile format for dependabot alerts - and/or warn that it is unsupported #10534

Open
1 task done
jamescrowley opened this issue Sep 2, 2024 · 13 comments
Open
1 task done
Labels
T: feature-request Requests for new features

Comments

@jamescrowley
Copy link

jamescrowley commented Sep 2, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Feature description

There is already an issue for this: #9522 that has been closed, but I've confirmed with GitHub support that in fact it is still not supported for security updates.

We are not seeing any security alerts since upgrading. This seems pretty dangerous given others may also have upgraded without realising they would no longer receive updates - dependabot doesn't seem to trigger any warning for this?

@jamescrowley jamescrowley added the T: feature-request Requests for new features label Sep 2, 2024
@jamescrowley jamescrowley changed the title pnpm: support v9 lockfile format - and/or warn that it is unsupported pnpm: support v9 lockfile format for dependabot alerts - and/or warn that it is unsupported Sep 2, 2024
@jeasmith
Copy link

jeasmith commented Sep 2, 2024

We are also no longer seeing security alerts, having upgraded recently. There has been very little, if no, warning provided.

@cseas
Copy link

cseas commented Sep 11, 2024

Version updates don't seem to be working either in our case since we've migrated from yarn to pnpm v9.

@msudol
Copy link

msudol commented Sep 24, 2024

Not seeing dependabot security alerts for projects using pnpm, found this issue searching for answers... hopefully will be fixed soon.

@vluoto
Copy link

vluoto commented Oct 4, 2024

ICYMI: https://github.com/orgs/community/discussions/134391

@jeasmith
Copy link

jeasmith commented Oct 7, 2024

I can see dependabot alerts in our repo using the v9 lockfile again as of a few hours ago. I can't see any related notes in the change log. Hopefully this is working again for others as well.

@msudol
Copy link

msudol commented Oct 7, 2024

I can see dependabot alerts in our repo using the v9 lockfile again as of a few hours ago. I can't see any related notes in the change log. Hopefully this is working again for others as well.

I don't see anything yet but maybe it will take a bit to propagate. Are you doing anything with your dependabot.yml to define that you are using PNPM or did dependabot just start picking it up natively?

@vluoto
Copy link

vluoto commented Oct 8, 2024

I can see dependabot alerts in our repo using the v9 lockfile again as of a few hours ago. I can't see any related notes in the change log. Hopefully this is working again for others as well.

We're seeing some alerts too, but I’m unclear on the reason why. The confusion stems from the fact that the dependency graph still doesn’t detect transitive dependencies from v9 lockfiles:

The reason I’m bringing up the dependency graph is because of this section in the README of the repository:

Don't file issues about Security Alerts or Dependency Graph

The issue-tracker is meant solely for issues related to Dependabot's updating logic. Issues about security alerts or Dependency Graph should instead be filed as a Code Security discussion.

A good rule of thumb is that if you have questions about the diff in a PR, it belongs here.

This is also why I opened the aforementioned discussion #10534 (comment).

@jeasmith
Copy link

jeasmith commented Oct 8, 2024

Are you doing anything with your dependabot.yml to define that you are using PNPM or did dependabot just start picking it up natively?

We're not doing anything special beyond the following @msudol:

version: 2
updates:
  - package-ecosystem: "npm" # See documentation for possible values - npm is used for pnpm
    directories: # Location of package manifests
      - "/"
      - "/apps/*"
      - "/packages/*"
    schedule:
      interval: "weekly"

We're seeing some alerts too, but I’m unclear on the reason why. The confusion stems from the fact that the dependency graph still doesn’t detect transitive dependencies from v9 lockfiles

We're seeing transitive dependencies as part of our dependency graph now.

image

We've got a support ticket on this running in parallel, I've enquired to find out what change has been made for things to start working again.

Good spot on the readme @vluoto . Thanks for opening the discussion, I'll post an update there as well.

@vluoto
Copy link

vluoto commented Oct 8, 2024

We're seeing transitive dependencies as part of our dependency graph now.

Hmm, I just checked, and we’re also seeing transitive dependencies in our repository:

Screenshot 2024-10-08 at 10 10 13

Interestingly, my v9 showcase repo still only shows direct dependencies:

This could be due to a gradual rollout on GitHub's end. 🤷🏻‍♂️

I've enquired to find out what change has been made for things to start working again.

Thanks for this! I'm eager to learn what it was.

@SuperchupuDev
Copy link

It works for me, but I had to update dependencies first to trigger a re-scan

@jamescrowley
Copy link
Author

jamescrowley commented Oct 9, 2024

Also working here now.

edit: though also seeing other problems like dependencies outside of the root of our mono repo get a package.json update but no lock file, and groups are not being applied correctly. Unsure if related.

@jeasmith
Copy link

Regarding the visibility of the change, I've had the following back from support:

This is not currently available in our public changelog. The change was classified as a rudimentary support to accompany support for previous versions. So this is a workaround pending when more granular change can be done.

@ArnoldZokas
Copy link

ArnoldZokas commented Oct 22, 2024

As of 20 Oct 2024, GitHub now successfully detects and reports vulnerabilities detected in pnpm v9 lockfiles.

This appears to be working even without configuring the packageManager field (proposed in some workarounds).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: feature-request Requests for new features
Projects
Status: No status
Development

No branches or pull requests

7 participants