Skip to content

Pin GitHub Actions to specific commit SHAs for improved security and stability #7076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

brenoepics
Copy link

@brenoepics brenoepics commented Apr 13, 2025

This PR updates all GitHub Actions in the workflow files to use exact commit SHAs instead of floating tags like @v1 or @v2.

Why?

  • Security: Tags like @v1 can be changed by the action maintainers or compromised if their repository is attacked. Pinning to a SHA ensures we're using the exact code we reviewed and trust.
  • Reproducibility: This guarantees consistent CI behavior across runs, making debugging and audit trails more reliable.
  • Best practice: GitHub officially recommends using commit SHAs for critical or trusted actions such as actions/checkout, setup-*, or deployment steps.

This change helps future-proof CI/CD pipeline and reduces the risk of unexpected behavior due to upstream changes in third-party actions.

Automatic Updates

The repository already use Dependabot, which will continue monitoring and automatically bump these SHAs when new versions are released, so we keep getting security updates and improvements without relying on floating tags.

ref:
https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions
example incident:
https://www.stepsecurity.io/blog/harden-runner-detection-tj-actions-changed-files-action-is-compromised
https://www.wiz.io/blog/new-github-action-supply-chain-attack-reviewdog-action-setup

@mitchellh
Copy link
Contributor

GitHub officially recommends using commit SHAs for critical or trusted actions such as actions/checkout, setup-*, or deployment steps.

Can you show the source for this?

I understand why here. I'm not against it, GHA's are definitely scary.

Before merging though I need to really carefully review every SHA you put here points to the right place. This seems like an excellent opportunity to actually introduce a hack.

@tristan957
Copy link
Member

Can you show the source for this?

The first ref link has some supporting documentation

@brenoepics
Copy link
Author

Can you show the source for this?

https://docs.github.com/en/packages/managing-github-packages-using-github-actions-workflows/publishing-and-installing-a-package-with-github-actions#publishing-a-package-using-an-action

You can also find the same in the first reference I sent and basically in all the examples they give in Java, Swift, Python...

Before merging though I need to really carefully review every SHA you put here points to the right place. This seems like an excellent opportunity to actually introduce a hack.

Sure thing! take your time, all care is important with this kind of thing, when you go to review, I have some other suggestions that I haven't implemented that might be interesting.

I also noticed that I don't see any permissions: declarations, so probably all actions are using the organization's base permissions, it's not really a problem depending on how org was configured but the ideal would be to leave only the necessary permissions for each job and avoid problems, specially in workflows that runs on Pull requests such as nix (if you require approval to run workflows from non-maintainers, that's fine)

I also suggest giving Zizmor a try, there are some other interesting suggestions there.

Thanks for the Ghostty project, it's really great and has become my way to go for a while.

(btw I replaced tags to hashes with pinact, it might be useful for you)

@bswck
Copy link

bswck commented Apr 20, 2025

This may be a nice counter-insight to the idea :)
pypa/setuptools#4025 (comment)

And especially this one
pypa/setuptools#4025 (comment)

(Keep in mind those concern also a scaling problem with "skeleton", a scaffolding actively used in 100 other projects, irrelevant to this project)

In my view, it may be a good idea for private projects where working at tail is more secure
So that's a -1 for me, unless maybe in some fragile parts like releasing... but that would mean discrepancies if there's no clear convention whether all are pinned or none, which can be also bad, or maybe it doesn't matter

@brenoepics
Copy link
Author

This discussion brought up a few good points that I think are worth addressing—thanks for bringing them up.

Regarding the first comment, I get it and feel their pain. I’ve been maintaining several public/private GitHub Actions inside organizations, and keeping them updated across many repositories is certainly a pain—whether you’re using tags or SHA pinning. Especially during major releases, tools like Renovate or other automation approaches might be better suited for this kind of use case.
But as you stated, this isn't really the case for Ghostty.

Immutable tags would actually be a game-changing feature, but it's been stale for about 3 years (github/roadmap#592), as someone pointed out.

The most interesting comment is this one: pypa/setuptools#4025 (comment)

My concern is that neither Dependabot nor the reviewer will have special knowledge about the state of the Action repo when the PR is created. Of course, if the repo is compromised while the dependency is pinned, that will delay the adoption of that tag until the next refresh. If the Action is compromised shortly before the PR is sent, however, Dependabot will pin the exploited version (and likely extend the duration of exposure).

I think the real issue being discussed here is when people either auto-merge or merge Dependabot PRs without actually reviewing the changelog or code changes of the actions. I agree with that—if you don't review what you're merging, pinning actions to tags, SHAs, or branches won’t be effective, and you can easily break the CI or introduce a vulnerability. (I understand that when you're maintaining so many repositories, it’s hard to review all those Dependabot updates alone.)

If the repo is compromised while the dependency is pinned, that will delay the adoption of that tag until the next refresh. If the Action is compromised shortly before the PR is sent, however, Dependabot will pin the exploited version (and likely extend the duration of exposure).

Thinking about it statistically, the average expected exposure will be unimproved over the unpinned behavior unless additional investment is put into validating a trusted pin.

Specifically about this point—their repo seems to be checked by Dependabot monthly, and given that this discussion is almost two years old, that might have been an issue back then. But Ghostty checks dependencies daily now. I understand that even 24 hours might be too long in some situations, but this concern isn’t as valid today. If you check https://github.com/ghostty-org/ghostty/network/dependencies?q=ecosystem%3A%22GitHub+Actions%22, GitHub indexes all your dependencies (GA, package managers, etc.).

In a scenario where either:

  • Someone gains access to a maintainer’s account and compromises a tag, and you mistakenly pin the GA to its commit SHA, or
  • A dependency (transitive or direct) or the repo itself has a vulnerability

When the compromise is discovered, the maintainer/GitHub will take down the compromised release(s)/commits and issue a GHSA (https://github.com/advisories). Any repository that depends on the compromised tag or SHA will get an alert in the Security tab and via GitHub notifications (an example is when tj-actions/changed-files was compromised—see the first incident example link).

Moreover, it introduces an additional attack vector in Dependabot+GitHub vs. simply trusting GitHub. Imagine that Dependabot is compromised never to update pins if an exploit has been pinned. Moreover, since an attacker has visibility to Dependabot configs, they could easily optimize the timing of their attack to take advantage of the known pinning periods of target projects to amplify their attack.

Unless you're only using GitHub-owned Actions, this isn’t always true. In fact, you’re inherently trusting all the actions you use (and their dependencies too!), which is another thing highlighted in the first reference link. (dependabot is owned by GitHub btw)

Regarding trust in Dependabot—not only Dependabot but also GitHub Advanced Security would have to be compromised in such a scenario, as it also warns about GHSA advisories, CVEs, etc. (including transitive ones). Tags/Releases from third-party actions being compromised is a far more realistic scenario. If GHSA, Dependabot, or even actions/checkout or other GitHub-owned Actions were compromised, that would arguably be one of the biggest supply chain attacks ever.

In my view, it may be a good idea for private projects where working at tail is more secure
So that's a -1 for me, unless maybe for fragile parts like releasing... but that would cause discrepancies if there’s no clear convention on whether everything is pinned or nothing is—this inconsistency could also be problematic. Or maybe it doesn’t matter that much.

I agree with you—private projects definitely benefit more from SHA pinning, especially since even read permissions can be a problem. As for pinning just some actions (like third-party ones) or only in certain workflows, that’s up to Ghostty maintainers. But we need to keep in mind that “individual jobs in a workflow can interact with (and compromise) other jobs.”. As I mentioned in another comment, I don’t see the permissions: keyword used per workflow or job—actions are likely running using the organization’s default workflow permissions (either Read and write or Read repository contents and packages).

Based on all those points, I still think Ghostty would benefit from pinning to SHAs rather than tags—at least while GitHub doesn’t support immutable releases (if they ever do).

Like I said before, these are small things that may not be problems right now, just like the things I noticed with zizmor, but the combination of these small things could become a bigger issue later on.

@mitchellh
Copy link
Contributor

Thanks for the writeup and sorry for the delay on this. We've had a bunch of churn on our GHAs. If you fix this up and open a new PR I'll merge it quickly. I'm satisfied with pinning so long as dependabot keeps updating us.

@mitchellh mitchellh closed this Jun 27, 2025
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.

4 participants