workflows: pin to full git SHAs#12
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughSix GitHub Action ChangesPin GitHub Action dependencies to commit SHAs
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
do you know if we pin with full hash, dependabot would still be able to bump them? about pinning with master in readme, yeah i should change it to pin to a tag |
It can. It should also update the tag comment. |
| repository: | ||
| description: Atelier repository | ||
| type: string | ||
| default: stepbrobd/atelier | ||
| ref: | ||
| description: Atelier repository Git ref | ||
| type: string | ||
| default: master |
There was a problem hiding this comment.
let's not change this for now (also below related changes that's not repinning to full hash)
imo users should fork directly and call their forked atelier instead of providing inputs for this
There was a problem hiding this comment.
Without this or some other solution, it's impossible to pin Atalier to a commit. People will think they're pinning to a commit, but in reality, it's just fetching master in multiple places behind the scenes.
People forking would also have to edit the code in multiple places to have their fork actually use itself.
There was a problem hiding this comment.
i see its mostly bc of uses: stepbrobd/atelier/.github/actions/atelier@master and some other forced master pin below
could you split this PR so that other dependent actions repin can be merged first
There was a problem hiding this comment.
Done. See #15 for the Atelier pinning part.
Tags are not sufficient to pin an action. What they reference can be later changed. This commit doesn't allow for pinning Atelier itself.
5841668 to
c76fcc5
Compare
Tags are not sufficient to pin an action. What they reference can be later changed. This supply chain attack has wreaked havoc multiple times recently.
This commit also provides a solution to usining the correct repo in forks.
In the future the proposed
$/syntax may improve this. See https://github.com/orgs/community/discussions/26245#discussioncomment-15601440.I think it would also be good to update the documentation, so that people can benefit from pinning atelier itself.
Another observation is that currently the recommendation is for users to use
@master. Besides security concerns, this doesn't allow for breaking changes to be made to atelier.