-
Notifications
You must be signed in to change notification settings - Fork 67
ci: pin github action with full-length commit hash #130
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
Conversation
|
@matiasalbarello @JoelLefkowitz Could you please review this? |
|
Hey @ostk0069 thanks for this PR, could you please explain why this is necessary since the versions numbers and hashes are aliases? |
|
Is it in case a compromised version of actions/caches gets published and tagged? The tradeoff is that you don't get improvement patches when you lock to a specific hash and it's less readable too. It's quite common in large packages to use the version numbers not hashes and I'd like to follow that convention. If you'd like to proceed with the PR I think it's a good idea to update the actions to the latest versions like actions/caches to v6. Would you like to do that? |
|
Thank you for your review.
There is a
In my commit these are already done. or is it shoulc be pinned like v6.x.x ? |
|
Thanks @ostk0069,
I don't think that's how that works. From the docs it looks like it means a consumer repository would need to use the sha when using steps:
- name: Wait for tests to succeed
uses: lewagon/wait-on-check-action@3603e826ee561ea102b58accb5ea55a1a7482343But it doesn't look like there are any constraints on the content inside In terms of the tradeoff of using explicit shas in this workflow the same docs article says:
These actions have the'"Verified creator" badges. I also checked some of the workflows in actions/checkout and react for reference and neither of them use the full shas, I'm guessing for readability and improvement patches.
Pinned as |
ae38f1d to
63ab15b
Compare
|
@JoelLefkowitz Thanks for review. I updated. |
|
Thanks @ostk0069 but it's still got the hashes could you remove them as discussed? |
Pull Request
Description
In order to enable
Require actions to be pinned to a full-length commit SHAsetting, this diff is necessary.version tags for improved security
actions/checkoutfrom v2/v4 to v6 (pinned with commithash)
ruby/setup-rubyto be pinned with commit hashCurrent Behavior
nothing changes
New Behavior
nothing changes