Pin 6 unpinned action(s)#117987
Conversation
|
I'm not convinced this is inherently adding any real security, for a couple of reasons (once the commits have been verified as being correct for the tags of course):
There's also a question of what the real risk is regardless of this, what the possible attack vector would be and what we expect might happen in the unlikely scenario of this specific kind of attack happening, weighed against those other considerations |
|
@AThousandShips these are fair points. Let me address each one. On current commits not being guaranteed safe: if the current commits are already compromised then you are already owned and have a much bigger problem on your hands lol. Pinning does not make that situation worse, but it does protect against the far more likely scenario of future tampering, which is the specific threat model here. The tj-actions attack, the Trivy compromise, and the Axios compromise from last night all worked by pushing malicious code to existing tags or branches. The downstream repos were never directly compromised. Their workflows just trusted the upstream reference to be safe, and when it wasn't, the malicious version was pulled automatically on the next run. On preventing security fixes: this is the most common concern and it's valid. The tradeoff is real. Tools like Dependabot and Renovate can auto-update SHA pins when new versions are released, so you still get updates but they come through as visible PRs that someone reviews rather than being pulled silently. That said, if the maintenance burden is a concern for your team, that is a reasonable reason to decide against it. On the tag deletion alerting scenario: that's an interesting point I haven't seen raised before. In practice though, the compromised actions we have tracked did not delete tags to alert downstream. They pushed malicious code to existing tags precisely because it is silent. The attack relies on nobody noticing. On the real risk: four confirmed supply chain attacks in four weeks, each one escalating (tj-actions, Trivy, LiteLLM, Axios last night). The attack vector is always the same: compromise a maintainer account or CI pipeline upstream, push to a mutable reference, and every downstream consumer pulls the poisoned version automatically. Godot's workflows reference third-party actions by mutable tags, which means you are inheriting the security posture of every upstream maintainer. I understand if the tradeoffs don't make sense for your project. No pressure either way.
|
|
That's a lot of words not really responding to what I said, and missing some of those points as well, I didn't say that attackers would delete tags, I don't know how you got that from what I said I'm concerned by the sheer number of these you have opened across various repos and the issues with the explanations given And to be clear I'm not questioning the nominal security of pinning the actions, but the question of what attacks might feasibly occur, what the risks would be (i.e. what could an attacker actually do with a compromised action), and most importantly the process of ensuring that these pinned versions are safe, just pinning things without a coherent evaluation of the actions risks being something that creates false security and added risks and concerns, so it IMO isn't something that should be done by mass PRs across hundreds of repos A compromised action isn't the only risk, there are considerations of broken workflows upstream that could be more problematic with pinned versions, and it's hard to evaluate without clear details |
|
@AThousandShips I hear you and I apologize for misreading your point about tag deletion. You were talking about legitimate maintainers using it as an alert mechanism, not attackers. That is a fair point. To answer your direct question about what an attacker could actually do: a compromised action runs inside your CI runner with access to whatever secrets and permissions that workflow has. In the Trivy attack, the compromised action harvested AWS keys, GCP credentials, Kubernetes secrets, and database credentials from every pipeline that ran it. Cisco lost over 300 source code repositories as a direct result. In the Axios attack last night, a full remote access trojan was deployed to every machine that installed it, attributed to North Korean hackers by Google. These are the attacks we have been tracking. This is not theoretical. This is happening right now, to real companies, through this exact vector. On verifying the pinned SHAs: each SHA was resolved from the current tagged release at the time of submission. They point to the same code that was running before, just locked to that specific commit. I understand your concern about the volume. 28 projects have merged these fixes so far, including Next.js, Apache Superset, Microsoft Terminal, Keras, Svelte, and others. I have been trying to get ahead of an escalating campaign helping maintainers secure their CI/CD so they do not get attacked and the back and forth on each repo takes time I do not have. If you do not feel this is necessary for Godot, close it. If you want to merge it, it is ready. Either way, no hard feelings.
|
|
That's not how humans speak nor write. As your claude agreed on Doing so would trigger dependabot for each action commit instead of actual update. As it also agreed if no write permission nor secret access, this is not solving a problem, but creates one. This is also clearly a bot made mostly, if not totally, with AI, and you aren't even hiding it in the source of your tool nor in the first spam PRs that were open, with some commit descriptions even mentioning it was an automated commit, such as here "Automated security fixes applied by Runner Guard". And here you checked you didn't use AI on the PR while the commit was from a tool that was made with AI with same copy and pasted paragraph as argument that didn't fix the potential security issue. Godot both, discourages usage and also prohibits contributions made entirely with AI. and requires disclosure. The If claw-code left unattended, mention on answer. |
4 participants
Re-submission of #117858. Had a problem with my fork and had to delete it, which closed the original PR. Apologies for the noise.
Summary
This PR pins all GitHub Actions to immutable commit SHAs instead of mutable version tags.
How to verify
Review the diff, each change is mechanical and preserves workflow behavior:
action@v3becomesaction@abc123 # v3, original version preserved as commentI've been researching CI/CD supply chain attack vectors and submitting fixes to affected repos. Based on that research I built a scanner called Runner Guard and open sourced it here so you can scan yourself if you want to. I'll be posting more advisories over the next few weeks on Twitter if you want to stay in the loop.
If you have any questions, reach out. I'll be monitoring comms.
- Chris (dagecko)