CI: update actions

[dnicolodi]

Follow best practices and switch to pinning actions by commit hash rather than by major version number.

[dnicolodi]

This patch has been generated with this thing https://github.com/dnicolodi/update-github-actions

I always thought that things like dependabot are more of a liability than a resource and I wondered how difficult it would be to write a script that does the same thing locally. Turns out it is quite easy and I am very surprised that something similar does not exist yet. This implementation is very crude, but it works.

[eli-schwartz]

Follow best practices and switch to pinning actions by commit hash rather than by major version number.

The flop [sic] side of this is that in exchange for getting rid of the possibility that a GitHub.com Inc owned and operated repository has its tags maliciously retagged to point to malicious contents, you have added the possibility that the commit sha1 hash points to completely arbitrary (malicious) contents from a fork. Any and every sha1 commit pinned action has to be manually reviewed, independently, by:

• people who author a PR,
• people who merge a PR,
• people who fork the repo and want to enable actions in their fork,
• people who want to locally run the CI using tooling such as act,
• people who browse the repo and want to know how safe it and the artifacts it creates are

"Best practices" isn't simply carrying a lot of weight in that sentence, it's outright something that opinionated people disagree whether it is best practices or worst practices.

I fall into the latter camp, and have been known to accuse the OpenSSF of being an extractive corporate-motivated drain on the FOSS community precisely over this.

[dnicolodi]

The flop [sic] side of this is that in exchange for getting rid of the possibility that a GitHub.com Inc owned and operated repository has its tags maliciously retagged to point to malicious contents, you have added the possibility that the commit sha1 hash points to completely arbitrary (malicious) contents from a fork.

Thanks @eli-schwartz for the perspective. I share your opinion that the eventuality of the tag being redirected to something malicious is very low. I don't know if that is lower or higher than someone constructing a SHA1 collision. I thought I was the only one being sceptical of the effectiveness of pinning via the commit hash, and I do not have time (nor the competences, probably) to do a complete assessment of the security threat, thus I trusted what the (loud fraction of the) community seems to agree upon.

Any and every sha1 commit pinned action has to be manually reviewed, independently, by:

I think that pinning actions via the commit hash implicitly replaces the entity the users need to trust form from the one publishing the actions to the maintainers of the repository using them. There are also tools (like zizmor) to verify that the hash pins correspond to the version specified in the comment next to it.

I am happy to keep pinning the tag, instead of the commit. However, so far (in almost all cases) we just pinned the major version of the actions. This leaves the door open for picking the latest minor and patch version release, without cooldown time (another "best practice" becoming popular of later). Should we switch to pin the exact version?

[rgommers]

I don't think the commit hash pin for GitHub-owned actions is very useful either way. However for random actions from third-parties it does help I believe; there's quite a few of those we use. Switching over the actions/* ones can be seen as a consistency measure; I don't care too much either way, it might just be a little easier for tooling.

you have added the possibility that the commit sha1 hash points to completely arbitrary (malicious) contents from a fork

I'm not sure I understand - that cannot resolve to a fork unless you've already done git remote add <fork-here>.

This patch has been generated with this thing https://github.com/dnicolodi/update-github-actions

I always thought that things like dependabot are more of a liability than a resource and I wondered how difficult it would be to write a script that does the same thing locally. Turns out it is quite easy and I am very surprised that something similar does not exist ye

https://github.com/davidism/gha-update exists and we've been using that in SciPy et al. for a while; its maintainer is well-known (e.g., he's a Python Discourse moderator).

Dependabot: 100% agreed, really don't like it.

[dnicolodi]

https://github.com/davidism/gha-update exists and we've been using that in SciPy et al. for a while

Thanks. I'm surprised I didn't find it in my searches.

[dnicolodi]

I don't think the commit hash pin for GitHub-owned actions is very useful either way. However for random actions from third-parties it does help I believe; there's quite a few of those we use. Switching over the actions/* ones can be seen as a consistency measure; I don't care too much either way, it might just be a little easier for tooling.

I don't see it difficult to have a different policy for different repository owners. However, should we keep pinning the major version only, or the full version? Namely, should it be actions/checkout@v7 or actions/checkout@v7.0.1 or something in between?

However for random actions from third-parties it does help I believe; there's quite a few of those we use.

• pypa/gh-action-pypi-publish
• codecov/codecov-action
• bus1/cabuild/action/msdevshell (already pinned by commit hash)
• cygwin/cygwin-install-action

[rgommers]

I'm fine with either v7 or v7.0.1 (or a hash, anything is fine for them), I don't see a real need to change anything - basic functionality like actions/checkout almost never breaks, so very granular tags will require regular bumping as well.

[eli-schwartz]

you have added the possibility that the commit sha1 hash points to completely arbitrary (malicious) contents from a fork

I'm not sure I understand - that cannot resolve to a fork unless you've already done git remote add <fork-here>.

It is with the deepest dismay and regret that I must inform you your reasonable expectations are nonetheless entirely 100% wrong in EXTREMELY SECURITY-CRITICAL WAYS. (shouty-case because this must not be ignored or overlooked)

Mechanically, the git protocol doesn't care what it asks for over the wire. An sha1 commit hash is "just another type of ref". And frankly when cloning a repo specifically in order to detach at some pinned reference, fetching by sha1 ref is good for efficiency! You avoid downloading large amounts of content you don't actually need that occurred later on the master branch (or with --depth, earlier too). Git supporting cloning by sha1 is a public service that happens to have sharp edges.

On forges such as codeberg.org, forks are isolated. You can push to a fork and then swap out the {repo_slug}/commit/{sha1} for the originating repo's slug and the result of that url is a "404 not found" if the commit hasn't been pushed to the originating repo via e.g. a Pull Request. On GitHub it is a webpage displaying the commit contents with an added little yellow notebox saying "this commit isn't on any branch, be cautious". This happens on GitHub because all repositories in a fork network have the same .git/ directory in the internal backing store, to save space. Additionally, GitHub does not perform any garbage collection, ever, in order to avoid breaking links after a link is made "unreachable" by force pushing.

By default, git allows you to clone and checkout any sha1, regardless of reachability from a public branch or tag.

The most common scenario where users face this issue is when using git submodules, which, like GHA pinning, record a sha1 in the committed tree metadata. It has to for reproducibility, and it has to checkout a repeatable submodule even if the clone url for that submodule (operated by someone else) has chosen to force push and get rid of a commit sha1 that you rely on and need. It is unfortunately not nearly as well known as it should be, but git submodules can therefore logically contain sha1 commits that were never part of the repo, and in practice this is also a viable attack on any project with submodules:

• malicious contributor submits a PR updating to a malicious fork, and it's impossible to detect via eyeball alone, because it looks like it's just using a version you "know" comes from upstream, and hey, it's even got the right version info
• malicious maintainer tricks users into using a project. The project itself is useful and has been independently verified to be non-malicious. Then the maintainer slips in a malicious submodule update and waits for users to verify the project code but gloss over the submodule "because that's a really famous project whose developers I know and trust" and then build and install malware

For submodules present on GitHub, a GitHub-to-GitHub clickable link to the submodule will be present in diff and tree views, so you can follow it and see the yellow notebox. People may or may not do so. For best effect, an attack would use cross-forge submodules to decrease the chance anyone will manually construct the url to check up on it.

GHA has a very similar problem. The only difference between it and a submodule is that there is no clickable link to the submodule so it is even harder to check. Also, third party contributors are more likely to submit PRs to update a GHA pin than they are to submit PRs updating a submodule.

...

tl;dr every commit object within a GitHub fork network is fetchable from all repositories in that fork network, without ever having been present in the repo you fetched from, despite the fact that they really shouldn't be.

[eli-schwartz]

However, should we keep pinning the major version only, or the full version? Namely, should it be actions/checkout@v7 or actions/checkout@v7.0.1 or something in between?

"Do you trust actions/checkout to follow SemVer?"

(yes)

[dnicolodi]

I don't think the commit hash pin for GitHub-owned actions is very useful either way. However for random actions from third-parties it does help I believe.

I went for this compromise.

[thesamesam]

FWIW we rejected it in xz as well: tukaani-project/xz#227

[rgommers]

@eli-schwartz you're completely right, thanks for the reminder! I think I knew that once and shelved it on tape storage.

When using zizmor, any mismatch gets caught indeed, which helps - using that in a CI job might be a useful follow-up. And I wouldn't aim to bump often nor accept bumps from random strangers unless they have a good reason for bumping (and then carefully check indeed).