Tighten permissions granted to secrets.GITHUB_TOKEN used in GitHub Workflows across the 2i2c-org

[consideRatio]

Description

The 2i2c-org GitHub org provides read & write permissions to the automatically provided GITHUB_TOKEN that can be used to read/write the content of a repo.

As discussed in jupyterhub/team-compass#404, I think we should also tighten the permissions given to this to be read only by default across the org. We should then explicitly set each of our GitHub Actions to have this permission where we want it so.

Benefit

Since it can be used in GitHub workflows to write content to branches, update comments, make PRs etc, the suggestion is to provide specific permissions to the token on a GitHub Job level (workflows has many jobs, jobs has many steps).

Tasks to complete

• Agree to reduce default permissions to read only. Decision prompt here: Tighten permissions granted to secrets.GITHUB_TOKEN used in GitHub Workflows across the 2i2c-org #359 (comment)
• Explicitly add the required permissions where GITHUB_TOKEN is referenced in the 2i2c-org's GitHub workflows across various repos. See Reducing security risk in our GitHub workflows jupyterhub/team-compass#404 for instructions on how to do this.
• Make the 2i2c-org's default permissions read only at: https://github.com/organizations/2i2c-org/settings/actions

[consideRatio]

Meta questions:

• is this too big to be a task, or is it more of a deliverable?
• would it made more sense for this to be posted in the meta git repository?

[choldgraf]

Let's add this to the deliverables board since it's important and requires a bit of thinking. I imagine that a clear "do X then Y" task will emerge from that discussion, which we can use to close out the deliverable. Does that makes sense?

[choldgraf]

@consideRatio to your other meta question, perhaps this issue can tackle both the "what do we do in this repository" and the "what is our team policy moving forward" question at the same time. So another task associated with this deliverable would be a change to the team compass to define the policy

[consideRatio]

Let's add this to the deliverables board [...]

Yes!

[...], perhaps this issue can tackle both the "what do we do in this repository" and the "what is our team policy moving forward" question at the same time.

Do you mean that we could define a set of "what we do in repository X" policies in the team-compass, and track that as tasks part of this deliverable?

[choldgraf]

@consideRatio I didn't imagine it needed to be repository-specific, just general team guidelines for this. But ya, in the team compass somewhere

[choldgraf]

@consideRatio I cleaned up the top comment a little bit, but I'm not quite sure that I understand the major implications here. It seems like the main problem is that an action would be compromised and do bad stuff, so we want to remove the "write" ability. That makes sense, but if we remove the ability by default but then manually add it to everything as well, doesn't this still retain the same vulnerability? It seems like functionally this would keep the same behavior.

[consideRatio]

Many workflow jobs will only require the permission to clone the repo (read), not to push back an update, create PRs, modify a wiki, etc. The idea is to only expose the permissions when needed.

For example, something updating the gh-pages branch when the docs has changed would need write "contents: write" permissions, by but not a pre-commit test job

[choldgraf]

Ahh I see, so you mean only add the write permissions when needed, not add it to everything. That makes sense

[consideRatio]

Ping @2i2c-org/tech-team, let's make our workflow files declare permissions required by our GITHUB_TOKEN's so that we can tighten down the permissions granted by default.

As an example, see https://github.com/jupyterhub/zero-to-jupyterhub-k8s/blob/9e63e3df4a65084410603098100b53ee19f6d4e6/.github/workflows/vuln-scan.yaml#L21-L25 where I both define the permissions needed for a Job within a workflow, and also add a comment as to why it is needed so it can be revised later with ease if something changes in the Job over time.

jobs:
  trivy_image_scan:
    if: github.repository == 'jupyterhub/zero-to-jupyterhub-k8s'
    runs-on: ubuntu-20.04
    # Write permissions granted for the peter-evans/create-pull-request action
    # to push to a branch and create/update a PR
    permissions:
      contents: write
      pull-requests: write

    # ...

[choldgraf]

OK let's just explicitly see if any team members object and we can move forward with this if not:

@2i2c-org/tech-team - Does anybody object to us implementing this issue so that we must explicitly specify the permissions given to actions? I'll leave this open for 48 hours to see if anybody objects.

I don't object, this sounds like a reasonable decision to me.

[choldgraf]

OK - I see many 👍 and no objections so let's go forward with this! I'll update the top comment accordingly and we can tackle it in a future sprint.

[consideRatio]

I went through all workflows, and it seems like they all define the permissions of relevance explicitly already. So, I tightened the permissions to read by default.

I made a note in our maintenance-notice channel in case we miss some permission in some workflow's job