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

Configure npm provenance statements #132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tjenkinson
Copy link

See https://docs.npmjs.com/generating-provenance-statements

It signs the release linking it back to the github action run that built it.

Might be worth adding a new option to bcomnes/npm-bump but went with this for now.

@bcomnes
Copy link
Owner

bcomnes commented Feb 5, 2024

Open to adding it but last I checked this didn't work:

Unfortunately my current automation creates the release commit and tag off of the preceding commit (because thats a tedious process and I want to make sure its done consistently) and npm provenance statements don't work unless the action runner was triggered directly off of the commit hash that is requesting the provenance statement.

I was thinking I could add a two-step action: create the release commit and tags with the changelog, and then trigger a subsequent action run on that specific commit which performs the npm release, then provenance would work. But it has drawbacks:

  • More fragile/complex
  • Slower
  • No offline workflow
  • Provenance can just as easily be added to malware and doesn't really solve real world problems afaict.
  • Actions makes this difficult (to prevent action loops iirc)
  • Not 100% sure this would work with provenance (but I think it might).

I'll try to revisit this, but in the meantime here is what this module does implement security wise:

  • Low npm maintainer count. @voxpelli and I are in frequent communication regarding any security issues.
  • Revokable single use automation token stored in GitHub actions secret store. I don't have a copy of this on my local machine and audit all PRs coming in.
  • Security scanning with @SocketDev
  • Signed git commits and release tags when generated on my computer

Thats probably more words that you need but thats my current brain dump on the issue.

@tjenkinson
Copy link
Author

Ah interesting. Are you sure the provenance check won't work with the manual dispatch trigger and still sign it with a link back to the job that ran and some
of the job metadata? That still seems valuable to me as it's connecting the published package back to the GitHub job run.

All sounds good though. I thought this would be a quick win.

Feel free to close this 🙂

@bcomnes
Copy link
Owner

bcomnes commented Feb 5, 2024

We can leave open for a bit to provide motivation to look into it again. I convinced myself Provenance was another DOA npm feature launch back when it first came out, but I should revisit to make sure my assumptions are still correct.

@voxpelli
Copy link
Collaborator

@bcomnes I think we can merge this 👍 Provenance would be good

@bcomnes
Copy link
Owner

bcomnes commented May 14, 2024

@voxpelli I don't think it will work as written though. Need to test it out and or change the automation flow to accommodate provenance. I don't think it likes it when providence is signed in an action run thats different than the release commit.

@bcomnes
Copy link
Owner

bcomnes commented Aug 9, 2024

Thinking of revisiting this given the uptick in usage.

@travi
Copy link

travi commented Aug 9, 2024

my current automation creates the release commit and tag off of the preceding commit (because thats a tedious process and I want to make sure its done consistently)

i maintain semantic-release, which supports npm provenance. whether you'd want to adopt semantic-release completely falls to your preferences, but if that is a route you'd be interested in, i'd be happy to help with any questions you might have

@bcomnes
Copy link
Owner

bcomnes commented Aug 13, 2024

Semantic release is great, been meaning to revisit sometime soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants