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

GitHub Workflows security hardening #3081

Merged
merged 4 commits into from
Apr 17, 2023
Merged

Conversation

sashashura
Copy link
Contributor

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.

Signed-off-by: Alex <aleksandrosansan@gmail.com>
Signed-off-by: Alex <aleksandrosansan@gmail.com>
Signed-off-by: Alex <aleksandrosansan@gmail.com>
Signed-off-by: Alex <aleksandrosansan@gmail.com>
@Zombaya
Copy link

Zombaya commented Nov 10, 2022

@sashashura , I noticed you are creating PR's like this to a lot of high profile projects and wanted to express my gratitude for trying to create a safer environment for the general public by having major projects increase the security.

I'm no maintainer to this project, so I will not be able to help with this particular PR but just wanted to send good vibes your direction.

Kudos!

I can see you automated the creation of this PR's a bit, so it might be interesting to add a link to your project in the text of your PRs or star it on your profile in some way so others might contribute or learn from it.

@sashashura
Copy link
Contributor Author

@Zombaya Thank you. I plan to exterminate this kind of misconfiguration in high profile projects and educate maintainers along the way :) The process is semi-manual and I have automated forking and creating pull requests with github api to speed up the things. It is a mess, maybe one day I'll open source it.

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Nov 10, 2022

Thanks for the feedback. It's a shame that GitHub can't provide more secure defaults, and I'm reluctant to merge this in case there are product changes coming down the line.

@sashashura
Copy link
Contributor Author

@GrahamCampbell who can review it?

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Dec 1, 2022

I am waiting for feedback from a contact I have at GitHub r.e. this.

@GrahamCampbell
Copy link
Member

@sashashura
Copy link
Contributor Author

sashashura commented Feb 2, 2023

For new orgs/repositories. Which is good in the long term.
However I fail to understand how this PR is hanging for 3 months.
Regardless of what default are, you could flip the setting at any time, but it would break the jobs that require specific write permissions. This PR fixes that. Otherwise there is no difference what GitHub default setting policy is - the branch-alias.yml work with full permissions only.

@GrahamCampbell GrahamCampbell merged commit 5b71b8f into guzzle:master Apr 17, 2023
23 checks passed
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.

None yet

3 participants