Skip to content

Hash-pin GitHub Actions #4026

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 3 commits into from
Closed

Hash-pin GitHub Actions #4026

wants to merge 3 commits into from

Conversation

pnacht
Copy link

@pnacht pnacht commented Aug 23, 2023

Summary of changes

Closes #4025.

This PR hash-pins all GitHub Actions used in workflows to protect the project from supply-chain attacks. It also configures dependabot to keep the Actions up-to-date.

Pull Request Checklist

pnacht added 3 commits August 23, 2023 14:56
Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
Signed-off-by: Pedro Kaj Kjellerup Nacht <[email protected]>
@abravalheri
Copy link
Contributor

abravalheri commented Aug 23, 2023

Thank you very much for the contribution Pedro. I will leave this one for @jaraco to judge.

Although, I have to say even with dependabot, this sounds very inconvenient 😅.
You mentioned in the issue "malicious attacker". The malicious attacker would have to be a maintainer of the specific action (or a person that has admin access, e.g. a Github employee), right?
In that sense, it does not look different to me from any other runtime/build-time dependency... Does it mean that we will have to start pinning everything from now on?

Maybe a better approach would be for the next link of the dependency chain to pin setuptools instead...

@abravalheri abravalheri requested a review from jaraco August 23, 2023 16:54
@pnacht
Copy link
Author

pnacht commented Aug 23, 2023

Hey @abravalheri, thanks for the quick reply!

So, this is just about hardening your CI/CD. Since your workflows already run with minimal permissions (the permissions.contents: read block at the top of the workflows), the main risk from a malicious Action would be leaking credentials or poisoned releases.

main.yml's release job seems to publish to PyPi. If one of the Actions became malicious, it could poison your environment and cause the wheel/dist to contain malicious behavior not found in the actual source code. Alternatively, they could simply steal your PyPi token and push their own malicious version.

I can't quite understand what ci-sage.yml does, but it seems to push a docker image which consumes an artifact created by your workflow. I suspect this is just for testing purposes, but if this docker image is actually sensitive, the Actions could also be used to poison that image.


But yes, this does add some overhead. setuptools currently has the following Actions:

  • actions/checkout (new release every ~2 months)
  • actions/upload-artifact (every ~5 months)
  • actions/setup-python (~1 month)
  • actions/cache (~1 month)
  • codecov/codecov-action ( ~6 months)
  • re-actors/alls-green (hasn't been updated since October 2022, but used to be updated every ~2 months... honestly looks abandoned now...)
  • cygwin/cygwin-install-action (~6 months)

So this would likely average to something like 1 dependabot PR every 1-2 weeks.

@webknjaz
Copy link
Member

  • re-actors/alls-green (hasn't been updated since October 2022, but used to be updated every ~2 months... honestly looks abandoned now...)

Not abandoned but feature-complete.

- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "weekly"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weekly is going to be quite noisy

@@ -73,14 +73,14 @@ jobs:
SETUPTOOLS_USE_DISTUTILS: ${{ matrix.distutils || 'local' }}
timeout-minutes: 75
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
Copy link
Member

@webknjaz webknjaz Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure this is not going to be compatible with the current mass-maintenance workflow because it will be actively introducing merge conflicts all the time: #4025 (comment).

Copy link
Member

@jaraco jaraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As proposed, this change is untenable, as it happens per-repo and increases the per-repo maintenance burden substantially. As webknjaz has described, this project is maintained alongside the hundreds of other projects I maintain, many of which are derived from jaraco/skeleton. Before we embark on recommending the change there, I'd like to take a step back and understand better what's being proposed (why it's beneficial).

@jaraco jaraco closed this Aug 25, 2023
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.

[FR] Hash-pin GitHub workflow Actions
4 participants