-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Generate provenance statements on npm publish #18352
Conversation
As a general rule: Please note that the context provided in #18352 (comment) should also be included in the commit message itself such that it's possible to understand what the patch does and importantly why on the Git command line without having to reference the GitHub PR. |
Ah, sorry about that. It's been a while since I contributed to a repo that doesn't squash, which usually makes commits irrelevant. Fixed both of my PRs :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, with the comment addressed in the squashed commit. In the meantime I'll read up on provenance a bit more before merging to make sure all prerequisites are met (because I'd like to make a new release later today).
This PR adds [Provenance statements](https://docs.npmjs.com/generating-provenance-statements) on `npm publish`, increasing supply-chain security.
Thanks! |
Uh. So I guess repository.url in package.json must match the URL of the repository GitHub Actions are ran from. In other words, it cannot point to https://github.com/mozilla/pdfjs-dist, but to https://github.com/mozilla/pdf.js for provenance to work. I don't think the format is relevant, but the repository it points to is. |
@wojtekmaj Sadly this failed in GitHub Actions for the new release; please see https://github.com/mozilla/pdf.js/actions/runs/9748332722/job/26903097065#step:6:359. The issue appears to be that in dddb74d we normalized the repository URL according to NPM recommendation, and I noticed you have the same in |
This comment was marked as outdated.
This comment was marked as outdated.
Ha, thanks. I guess the best way forward is to unpublish this release, fix the URL and try again? Fortunately the error caused nothing to be published to NPM yet AFAICT, so I think unpublishing should be safe. |
Alternatively we could disable provenance for now until the fate of #18357 is decided because I believe that was why the repository URL was the way it is now, but that'd not be my preferred option. |
Both options should work! |
For provenance, enabled in PR mozilla#18352, to work the repository URL in `package.json` is required to match the repository URL of the GitHub Actions invocation. This should fix the following error we encountered publishing a new release today: ``` npm error 422 Unprocessable Entity - PUT https://registry.npmjs.org/pdfjs-dist - Error verifying sigstore provenance bundle: Failed to validate repository information: package.json: "repository.url" is "git+https://github.com/mozilla/pdfjs-dist.git", expected to match "https://github.com/mozilla/pdf.js" from provenance ```
Thanks; the issue should be fixed by #18358. |
It worked, and the provenance badge is now shown on https://www.npmjs.com/package/pdfjs-dist/v/4.4.168. Nice! |
This PR adds Provenance statements on
npm publish
, increasing supply-chain security.