Skip to content

GitHub Workflows security hardening #31863

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sashashura
Copy link

This PR adds explicit permissions section to workflows. This is a security best practice because by default workflows run with extended set of permissions (except from on: pull_request from external forks). By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection or compromised third party tool or action) is restricted.
It is recommended to have most strict permissions on the top level and grant write permissions on job level case by case.

@hashicorp-cla
Copy link

hashicorp-cla commented Sep 25, 2022

CLA assistant check
All committers have signed the CLA.

@crw
Copy link
Contributor

crw commented Sep 27, 2022

Thanks for this submission! The safety and security of our products is deeply important to us. Our build process is designed and vetted by our internal release engineering and security teams. Although we do not typically accept pull requests for the build and release process, I will run this PR past those teams for review. Thanks for your consideration on this issue.

@sashashura
Copy link
Author

Hi @crw, what was their response? Do you have any questions?

@sashashura
Copy link
Author

An example of a recent workflow run with unrestricted permissions:
image

@sashashura
Copy link
Author

@liamcervante @apparentlymart could you please review it?

@crw
Copy link
Contributor

crw commented Dec 2, 2022

The internal security team approved the general approach (per @sarahethompson). The core maintainers would still need to review and accept this specific PR. I will raise it with the team. Apologies for the delay.

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

Successfully merging this pull request may close these issues.

3 participants