Remix.run Logo
rmunn 2 days ago

Back when GitHub Actions first came out, I used commit hashes rather than tags in all my `uses:` lines. Some of my colleagues disagreed, saying that tags were secure enough. I eventually said, "Well, for well-known actions like actions/checkout, sure; if that one gets compromised it'll be all over the news within minutes." But for all the third-party actions, I kept commit hashes.

I feel rather vindicated now. There's still a small possibility of getting supply-chain attacked via a SHA collision, or a relatively much larger (though still small in absolute terms) possibility of getting supply-chain attacked via NPM dependencies of the action you're relying on.

But if you're not using a commit hash in your `uses:` lines, go switch to it now. And if you're just using major-version-only tags like `v5` then do it RIGHT now, before that action gets a compromised version uploaded with a `v5.2.3` tag.

maxloh 2 days ago | parent | next [-]

GitHub Actions doesn't have a lock file, so your repo is still prone to transitive attacks if the SHA-locked actions you use also happen to use other composite actions by tags, which could be compromised in the future.

mmarian 2 days ago | parent | next [-]

Agreed. Good news is GitHub will address that with Immutable Releases https://github.blog/news-insights/product-news/whats-coming-... You won't even need to use commit SHA as long as the maintainer follows this approach.

phist_mcgee 2 days ago | parent | next [-]

What an absolute joke that it has taken GitHub this long to clean up it's act when it comes to supply chain security.

cyberclimb 12 hours ago | parent | prev [-]

The actions/checkout repo still doesn't even use immutable releases so I'll believe it when I see it

https://github.com/actions/checkout/issues/2316

mmarian 11 hours ago | parent [-]

Yes, it's maddening. Especially since it's a fair amount of effort to move to commit SHA pinning and establish a good maintenance/monitoring process around it; if I knew it would be adopted quickly, I could argue that people should just wait and accept temporary risk.

Munksgaard 2 days ago | parent | prev | next [-]

Even with a lock file, the action can download and execute arbitrary code from the internet.

shykes 2 days ago | parent [-]

It would be cool if CI could inject a platform-wide lockfile into every remote download or lookup made by your scripts. So if you pull a container or git tag, the CI platform would automatically ensure that the exact digest downloaded is controlled by a lock file that you can inspect, check in, etc.

xenophonf 2 days ago | parent | prev [-]

"Require actions to be pinned to a full-length commit SHA" applies to composite actions, too. I had to replace pre-commit/action as a result.

arionmiles 2 days ago | parent | prev | next [-]

I feel pretty happy we use Renovator (EDIT: It's Renovate) at my current workplace which by default will raise PRs to change any tags for actions with the SHA instead. Then, even when it bumps the version in future PRs, it bumps the SHA (with a comment of which tag version it represents)

jamietanna 2 days ago | parent | next [-]

Glad to hear you're enjoying Renovate - I'm biased, but I agree that the SHA pinning PR updates are a very nice feature

We recently found (in Renovate) some edge cases with how tags work in GitHub Actions which was fun (https://news.ycombinator.com/item?id=47892740) and there's a few things in there Dependabot doesn't seem to support too

mmarian 2 days ago | parent | prev | next [-]

If you auto merge those PRs you're back to square 1 as you're not vetting your dependency updates. And if you don't, you incur operational overhead unless you put in a fair amount of effort centralizing. Wrote a couple of posts that touched on this https://developerwithacat.com/blog/202604/github-actions-sup...

arionmiles a day ago | parent | next [-]

Valid point. We have minimum age requirements set on some rules to avoid absorbing every latest change instantly.

mmarian a day ago | parent [-]

How would that solve the problem though? You're still bringing compromises in, just with a delay. And the fixes will come in after the compromise, in accordance with the delay policy.

To make matters worse, you'd lose getting alerts on vulnerabilities. Dependabot won't send them, and neither will Renovate last time I checked.

pabs3 a day ago | parent | prev [-]

How many people actually audit the code changes in their dependencies when updating them?

mmarian 21 hours ago | parent [-]

Few, if any. Which is why I'm highlighting that you can't just use commit SHA + Renovate then call it a day.

tecleandor 2 days ago | parent | prev [-]

Is it Renovator or Renovate? I'm trying to find it to check it out...

arionmiles 2 days ago | parent [-]

Oops, my bad. We keep calling it Renovator internally but the name is RenovateBot or Renovate.

https://docs.renovatebot.com/

tecleandor 2 days ago | parent [-]

Thanks! I'll take a look :)

rileymichael 2 days ago | parent | prev | next [-]

just noting that pinning within your own actions is not enough, you also need to ensure any composite actions do not use mutable references (for actions, docker images, etc.)

samuelknight 2 days ago | parent | prev | next [-]

There is no realistic risk of a SHA collision attack. Getting supply chain attacked via NPM dependencies is much more likely. Hopefully the actions creators are also pinning their hashes.

kbolino 2 days ago | parent [-]

> There is no realistic risk of a SHA collision attack.

Indeed. To illustrate why:

1. It is not possible to "retroactively" find a SHA-1 collision for an already known hash. If somebody has produced a SHA-1 hash non-maliciously at any point in the past, it is safe from collisions. This is due to second-preimage resistance, which hasn't been broken for SHA-1 and doesn't seem likely to be broken any time soon.

2. The only way to obtain a SHA-1 collision is to do so knowingly when producing the original hash. You generate a pair of inputs at the same time that both hash to the same value. Certainly, this is an imaginable scenario; e.g. a trusted committer could push one half of the pair wittingly or a reviewer could be fooled into accepting one half of the pair unwittingly, both scenarios creating a timebomb where the malicious actor swaps the commit to the second half of the pair (which presumably carries a malicious payload) later. However, there are two blockers to this approach: Git (not just GitHub) will not accept a commit with a duplicate hash, always sticking with the original one, and GitHub specifically has implemented signature detection for the known SHA-1 collision-generating methods and will reject both halves of such a pair.

In short, there's just no practical way to exploit this weakness of SHA-1 with Git.

koolba a day ago | parent | prev | next [-]

Even SHA pinning only lets you go one hop. If the pinned action itself uses any non pinned actions, you’re still susceptible.

I don’t think this problem is fixable without a higher level way to specify the full nested tree. Something like TOFU for the first time your action ran (pinning all children as of that run) might be an improvement, but that is still can be gamed by a timed attack that modifies the action at a later date (literally, if time greater than X do …).

AlecBG 2 days ago | parent | prev | next [-]

You can enforce at the org level to only allow actions pinned to hashes. You can also choose a small whitelist of actions to allow.

mmarian 2 days ago | parent [-]

I used to think whitelist could be a partial solution. But after Checkmarx KICS got compromised I can't see this working. I would've considered a well-established brand, in security industry of all places, to be in the whitelist.

mmarian 2 days ago | parent | prev | next [-]

There are downsides to it though. You... - lose vulnerability alerts - increase maintenance overhead - take on all that for value that will go to 0 once Immutable Releases gets widely adopted

I wrote a couple of blog posts on it, and a makeshift way of tackling that https://developerwithacat.com/blog/202604/github-actions-sup...

woodruffw 2 days ago | parent | next [-]

You lose vulnerability alerts, on GitHub. This is a (ridiculous, IMO) platform limitation that GitHub could lift by applying more engineering time to Dependabot and Dependabot's integrated security alerts feature.

zizmor (and other tools) correctly recovers vulnerability information for SHA-pinned actions[1].

[1]: https://docs.zizmor.sh/audits/#known-vulnerable-actions

mmarian 2 days ago | parent [-]

I agree, silly limitation.

On zizmor, there's no mention of coverage on commit SHA the section you've linked, nor in the entire page when I do Ctrl+F. Is there anything I'm missing?

woodruffw 2 days ago | parent [-]

Oh, I guess I didn't document it explicitly. My bad!

You can see it in the source here[1].

[1]: https://github.com/zizmorcore/zizmor/blob/db5ed6b3bb445848a8...

mmarian 2 days ago | parent [-]

Oh, nice, will look into it, thanks! Let me know if you're aware of any other tools that do this. I had a look before and couldn't find any.

baby_souffle 2 days ago | parent | prev [-]

The maintenance aspect is relatively straightforward to automate.

Renovate handles this well. Ratchet and pinact can also be used

mmarian 2 days ago | parent [-]

I mention in the posts the problem with the likes of Renovate. Auto merging is equivalent to semantic versioning. You have to properly vet the influx of updates, and that unfortunately won't work in practice.

deepsun a day ago | parent | prev [-]

Maybe it's better to pull that dependency source in your action altogether?

rmunn a day ago | parent | next [-]

I hadn't previously considered vendoring GHA dependencies, but yes, that might be a good idea. Perhaps not in all circumstances, but for anything that might be at risk of supply-chain compromise, the same arguments that apply to NPM apply to GHA.

pabs3 a day ago | parent | prev [-]

Better to treat it as a dependency still, but audit each new commit/release as it comes in, and pin to the exact last commit id that you verified.