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

Pin GitHub Actions to specific commits for security #28

Merged
merged 1 commit into from
Apr 22, 2023

Conversation

hartwork
Copy link
Contributor

@hartwork hartwork commented Apr 18, 2023

For proof that GitHub Dependabot can still keep things up to date for us with the new format see https://github.com/gentoo-ev/www.gentoo-ev.org/pull/5/files .

@karlb
Copy link
Owner

karlb commented Apr 20, 2023

I see that updates could break CI and pinning to commits works. But what is the actual risk, considering that the CI jobs don't have any write permissions? That they could be used as part of a botnet and waste resources?

@hartwork
Copy link
Contributor Author

@karlb the attack would be replacing the action's own code with something malicious and making the moving Git tag point to it. The tag name would be the same, the commit hash would not unless you assume SHA1 broken. CI permissions do (or at least did) include write access unless you switched defaults on the repository(, and the default can be bypassed by the yaml file content, which is... not ideal).

This is probably not the repository at most risk, I just ran through all my local clones where this vulnerability was a theoretical possibility and made pull requests. I believe it is 100% step forward and 0% backwards personally, but if I'm missing some aspect, please let me know.

PS: Related tool https://app.stepsecurity.io/secureworkflow/ could also be of interest in this very context.

Copy link
Owner

@karlb karlb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0% backwards personally

You might lose out on fixes and optimizations of newer versions, but that is a really minor downside.

Thanks for the explanation!

@karlb karlb merged commit 2a232f6 into karlb:master Apr 22, 2023
@hartwork
Copy link
Contributor Author

You might lose out on fixes and optimizations of newer versions, but that is a really minor downside.

@karlb I just re-checked, GitHub Dependabot already has us covered on that front via https://github.com/karlb/wikdict-web/blob/master/.github/dependabot.yml .

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.

2 participants