Repository navigation
Hash-pin GitHub Actions to increase workflow resiliency #109110
Description
Activity
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Thanks for the suggestion, we discussed this recently in #103635.
The summary is that we can trust GitHub's own
actions/*, seeing as we trust them for the actions infra etc (#103635 (comment)). The three references at pypa/setuptools#4025 (comment) talk of pinning third-party actions. One explicitly saysapache/*,github/*andactions/*can be used without restrictions (e.g. no pinning).Looking closer at potential threat scenarios (#103635 (comment)), we have two (now three) workflows with write permissions, with actions from
actions/*andreadthedocs/*, both of which are trusted. And workflows withsecrets.*only use GitHub's ownactions/*.
Thanks for the link to pypa/setuptools#4025, I see there are other issues beyond integrating with jaraco/skeleton.
As mentioned at pypa/setuptools#4025 (comment), the planned github/roadmap#592 sounds useful.
Also noted in pypa/setuptools#4025, pinning increases the toil for without adding a lot of value. For example, we'll be pinned to a given x.y.z patch version for up to a month; when not pinning, we use the latest x.* version available. If a given version is compromised, it could be timed to do so just before the pin is bumped for maximum exposure (pypa/setuptools#4025 (comment)). The unpinned ones would mean that a compromised action would have a shorter window of use until a fix is released.
So far we've only had three dependabot PRs this year. When pinning x.y.z patch versions, I expect it's very likely there will be an update each and every month. (Although once a month isn't too spammy, and grouped updates sounds like it'll help.)
Also, there's been a series of incidents at GitHub this week, including one that needed an upgrade to
actions/checkout@v4, which would have been harder to do with pinned hashes.Reacted by Adam TurnerThe summary is that we can trust GitHub's own
actions/*, seeing as we trust them for the actions infra etc (#103635 (comment)).That's a reasonable position, but I'd say trusting GitHub's infrastructure isn't necessarily the same as trusting its public repositories. As far as I can tell, the
actionsorg is just an org like any other. I wouldn't be surprised if they have some added protections on the backend (they might not!), but these are still public repositories that accept external pull requests. PRs that might introduce (accidentally or maliciously) vulnerabilities in the Actions.The three references at pypa/setuptools#4025 (comment) talk of pinning third-party actions. One explicitly says
apache/*,github/*andactions/*can be used without restrictions (e.g. no pinning).The other references speak only of "third-party Actions", which I understand to mean "all Actions not maintained by my organization". But yes, the Apache policy allows using GitHub-managed Actions without pinning.
There is no right answer. It's a matter of how much you trust GitHub's public repositories.
Looking closer at potential threat scenarios (#103635 (comment)), we have two (now three) workflows with write permissions, with actions from
actions/*andreadthedocs/*, both of which are trusted. And workflows withsecrets.*only use GitHub's ownactions/*.Ah, I wasn't aware that
readthedocswas considered a trusted party (don't know what the relation is between them and the PSF). That was one that had caught my eye.Also noted in pypa/setuptools#4025, pinning increases the toil for without adding a lot of value. For example, we'll be pinned to a given x.y.z patch version for up to a month; when not pinning, we use the latest x.* version available. If a given version is compromised, it could be timed to do so just before the pin is bumped for maximum exposure (pypa/setuptools#4025 (comment)). The unpinned ones would mean that a compromised action would have a shorter window of use until a fix is released.
CPython seems to have dependabot security alerts set up (see this PR bumping /Doc/requirements.txt). Should an Action be compromised, you should receive an "out-of-season" PR bumping that dependency as soon as a patched version is released. So you'd be exposed until that PR is merged, not necessarily for an entire month. I didn't bring this up when discussing jaraco/skeleton because there's no way to ensure all its "children" also have security alerts active.
Meanwhile, if the attack isn't "properly timed", you likely won't be impacted at all. And some attack vectors are "randomly timed" (i.e. merging a PR with a subtle vulnerability – accidental or malicious – that only impact users after an eventual release), so I believe this isn't entirely implausible.
Also, there's been a series of incidents at GitHub this week, including one that needed an upgrade to
actions/checkout@v4, which would have been harder to do with pinned hashes.Yeah, GitHub had an unpleasant week, for sure. Though I hadn't heard of this need to bump to v4 before. It's not mentioned in the explanations given in https://www.githubstatus.com/history, and one comment in your linked issue says they had the same problem with v4.
Bumping to v4 resolved the immediate problems, from memory, though it may have been correlation-causation.
I tend to agree with Hugo here though, and we've taken the recommendations to move to read-only jobs, etc already.
A
Understood. I'll close the issue and PR now.
Thank you for your time!
Reacted by Hugo van Kemenade
Feature or enhancement
Proposal:
When developing with CI workflows, it's common to version-pin dependencies (i.e.
actions/checkout@v3). Such major-version pins can be useful in that you get immediate access to the latest version of each Action. However, this also makes the project vulnerable should a broken or malicious release be published. However, version tags are mutable, so minor version tags aren't the best solution, since a malicious attacker could overwrite a version tag to point to a malicious or vulnerable commit instead.Hash-pinning workflow dependencies ensures the dependency is immutable and its behavior is guaranteed.
These hashes (and comments indicating the respective version) can be kept up-to-date by dependabot, which cpython already uses. However, the current configuration (ignoring patch and minor version bumps, set in #92186) would need to be modified. My understanding of the motivation for #92186 was the flood of PRs generated by dependabot. However, this concern can be mostly eliminated by relying on dependabot's new grouped updates; dependabot can now be configured to send a single PR per month updating all Actions at once (see this example).
I'll send a PR pinning the Actions and changing dependabot's configuration along with this issue.
Hey, I'm Pedro. I work with Google and the OpenSSF to help critical projects improve their supply-chain security.
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
I previously suggested this change to pypa/setuptools, where I had an excellent exchange with @jaraco. I recommend taking a look at our conversation there.
The conclusion there was that the change wasn't feasible for that project. My understanding is that the main "hurdle" was the project's use of jaraco/skeleton as its template. Unfortunately, keeping the hashes updated in the skeleton and its "child" repositories would lead to frequent conflicts when merging skeleton updates into the repositories. The maintainers therefore reasonably concluded (and I couldn't really disagree) that the benefits of hash-pinning weren't worth the significant overhead they would cause in that situation.
However, I believe the situation for CPython is much more straightforward. The "overhead" here would be merging at most one dependabot PR per month.
Linked PRs