Skip to content

Pin GitHub Actions to specific commits for security#28

Merged
karlb merged 1 commit intokarlb:masterfrom
hartwork:pin-github-actions-at-commit-level
Apr 22, 2023
Merged

Pin GitHub Actions to specific commits for security#28
karlb merged 1 commit intokarlb:masterfrom
hartwork:pin-github-actions-at-commit-level

Conversation

@hartwork
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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