Skip to content
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

Hash-pin GitHub Actions that have access to sensitive permissions and secrets #7119

Open
diogoteles08 opened this issue Oct 20, 2023 · 2 comments · May be fixed by #7153
Open

Hash-pin GitHub Actions that have access to sensitive permissions and secrets #7119

diogoteles08 opened this issue Oct 20, 2023 · 2 comments · May be fixed by #7153

Comments

@diogoteles08
Copy link
Contributor

Hi, I'm Diogo and I raised the issue #7069 suggesting you to set minimal permissions to your workflows. Now I'm coming back to suggest a modification that would provide extra safety for the workflows that yet require dangerous permissions (e.g., contents: write).

Problem

Some of your workflows (e.g. auto_author_assign.yml or playwright-update.yml) are using dangerous permissions permissions while running external dependencies pinned only by tag. At publish-release.yml they're not called with dangerous permissions but have access to important secrets (although at this case I see that it's a jupyter-lab action, so I'd understand if you prefer to just trust this one). Those patterns could be dangerous because if any of those actions get hijacked (and at the end they're all repositories and are susceptible to attacks like any other), an attacker could change the code that your tags point to, gaining access to your secrets and/or write permissions to your code.

Proposed Solution

A simple solution for this problem would be to hash-pin those sensitive actions, pointing the actions to the very specific commit of that release. It follows and example of the change:

- uses: r-lib/actions/pr-fetch@v2 
would become
- uses: r-lib/actions/pr-fetch@11a22a908006c25fe054c4ef0ac0436b1de3edbe # v1.3.1

It shouldn't require any more maintenance, because dependabot is also able to keep them updated. It would update them at the same pace you're already used to, and it would still keep a comment with the human-readable version used =).

Conclusion

Let me know if you are interested in this change and I'd be happy to send a PR solving this.

Thanks!

@krassowski
Copy link
Member

👋 It might be easier to assess a pull request, as seeing which specific actions you suggest to pin may make it easier to understand the implications.

although at this case I see that it's a jupyter-lab action, so I'd understand if you prefer to just trust this one

For that reason, it might be more valuable to hash-pin actions on which the maintainer-tools depend rather than hash pin in this repository, but that will vary on case-by-case basis. Also, some actions may be replaced altogether with native tooling which might be preferred (especially if for example git or github CLI gained specific capabilities since the action was first written),

@diogoteles08
Copy link
Contributor Author

Hey @krassowski, sorry for the (very) late reply. I'm raising the PR with the mentioned changes for you.

it might be more valuable to hash-pin actions on which the maintainer-tools depend rather than hash pin in this repository

I agree with you, and also liked a lot the idea of replacing external actions with native tooling.

My point is that, for now, the best we can do is still hashpin all the actions that access to sensitive permissions or secrets. In the case of the actions from maintainer-tools, it would indeed be more valuable to hash-pin the actions in there, but it's also valuable to hash-pin how you call them because make you safe against anything going possibly wrong on their repository. And also there is a big "why not" factor here, because you are already using dependabot to update your actions. If at some point the updates become annoying, we can change the frequency of the version checks or group together all the PRs updating actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants