What happens when you don’t read the docs - Code to Cloud check-in

Permission problems or “How I ruined my Monday morning with one PR”

This week I’m bringing you a peek behind the scenes here in the GitHub community team, and a reminder to always thoroughly read the documentation when you’re changing something as important as permissions!

Here at GitHub we use GitHub to build GitHub. Each team has a repository and in that repo we store useful files and documentation, keep track of projects using project boards, and create issues to track what the team is working on. We also use Actions to automate a lot of this.

The GitHub org will soon be enforcing explicit permission definitions for the GITHUB_TOKEN meaning that we needed to update our workflows in order for them to continue working. Since all of our workflows in the community repo only really check out the repo then create and close issues using templates I added the following to all our workflows and called it a day:

permissions:
  issues: write

Our workflows are scheduled, so it wasn’t until Monday morning, when I noticed that every single one of our workflows had failed over the weekend! After spending a good hour poring through our docs I found this line:

When the permissions key is used, all unspecified permissions are set to no access, with the exception of the metadata scope, which always gets read access.

The fix for this was easy enough, once I understood that all our workflows needed the contents permission in order to check out the repo:

permissions:
  contents: write
  issues: write

I hope this story helps anyone who might be implementing similar security policies and serves as a stark reminder to always read the docs before changing your important permissions!

Other items

10 Actions resources to bookmark

The GitHub blog recently posted a great article on 10 Actions resources to bookmark. If you’re just getting started with Actions, then this is a great post to get an idea of what’s possible.

Game off continues

The GitHub Game Off is still on! If you’d like to take part, there are still 16 days left to submit your entry. If you want to take part but have never made a game before, there’re plenty of options listed for different languages, so pick the one you’re most comfortable with and get going! I can’t wait to see what you come up with!

5 Likes

To be honest, I find this really counter-intuitive:

contentts: write does not sound like “needs to read for checkout purposes”. :man_shrugging:

The documentation you linked to says:

Setting the default to contents:read is sufficient for any workflows that simply need to clone and build.

So I wonder if there is more here, than meets the eye?

2 Likes

This is true, you only need read to checkout! Some of our workflows do touch back to the repo so write is needed if you need to do that.

2 Likes

“This is true, you only need read to checkout! Some of our workflows do touch back to the repo so write is needed if you need to do that.”

So they are all already broken… Set it to reac, evaluate those that are already broken. Or even write tooling to examine each workflow, so that if there is a workflow granted a permission that is not needed it is flagged in an automated audit [which could be included in the branch protection rules]

VERY DISAPPOINTING. Minimal permissions is always correct. Hard to trust a company that does not follow this internally. (Rouge Internal Staff does potentially exist everywhere - allowing workflows that are only supposed to read a resource have write access opens a huge security hole).