GitHub Workflows security hardening

[sashashura]

Make sure you have checked all steps below.

Prerequisite

• Please consider implementing the feature as a hook script or plugin as a first step.
  • pyenv has some powerful support for plugins and hook scripts. Please refer to Authoring plugins for details and try to implement it as a plugin if possible.
• Please consider contributing the patch upstream to rbenv, since we have borrowed most of the code from that project.
  • We occasionally import the changes from rbenv. In general, you can expect changes made in rbenv will be imported to pyenv too, eventually.
  • Generally speaking, we prefer not to make changes in the core in order to keep compatibility with rbenv.
• My PR addresses the following pyenv issue (if any)
  • Closes https://github.com/pyenv/pyenv/issues/XXXX

Description

• Here are some details about my PR

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.

Tests

• My PR adds the following unit tests (if any)

[native-api]

The flip side is we'll need to constantly figure out what each permission controls -- thus which permissions we need for each operation.
While the documentation at https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions, https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token and https://docs.github.com/en/rest/overview/permissions-required-for-github-apps#administration is extensive, as usual, it lacks fine details.

E.g.:

• Why did you grant any permissions to the job if it doesn't use the Github REST API, at all?
• What permissions do we need to e.g. create a PR or a release?
• Do we need any permissions for operations that stem from the workflow definition -- e.g. to run a dependent action or pass data to it?

---

How probable and severe are the risks anyway? E.g.:

• We have workflow start protection for new contributors to filter out bad agents
• Actions' token doesn't have permissions for Administration so even injecting malicious code can't hijack the project or the organization.

Let's compose a basic risk heat map so that we can decide which risks we defend against and which we don't.
(Since this requires a colored table, a Google spreadsheet would probably be the most convenient.)

[sashashura]

I agree that the documentation could have been better. Though I don't agree, that it introduces a constant burden. After the workflows run with minimal privileges it requires modification only if you add a job or step that requires new permission. If that happens you will see something like "Resource is not available". Often the missing permission is obvious from the purpose of the action: comment on issue, label pull request, etc.

The risk of not hardening the workflows is low:

• Workflows triggered on pull_request from forks already run with restricted permissions.
• 3 of 4 of the workflows are also triggered on push which means the changes were accepted and pushed to the repository. However any tool that is installed with, for example, brew install can be compromised for a short period of time and would run on any innocuous push to the repository.
• The no-response.yml runs on comments and by scheduler. The only risk here I see a compromise of the lee-dohm/no-response action. By restricting the token permissions, it could only open/close/modify issues. Otherwise it would be able to modify the content of the repository or modify releases.

From my point of view it is more like security hygene not running commands with sudo unnecessarily.

[native-api]

After the workflows run with minimal privileges it requires modification only if you add a job or step that requires new permission.

That's the problem. We do currently plan to add new workflows:

• Detect new Python releases, create PRs and probably merge them, too.
• Create releases recently after a new CPython release comes and bump Homebrew formula

[native-api]

So we can merge this, it won't hurt anything now, but whenever we start tinkering with workflow, it'll probably start causing problems and will be removed since it's wasted time and nerves for questionable gain...

[sashashura]

I would rather not merge then if you anticipate it will be removed.

[native-api]

@pyenv/pyenv-core-committers , any thoughts on this?

[amureki]

@pyenv/pyenv-core-committers , any thoughts on this?

I see that the current risk is low, but "better safe than sorry".
With new upcoming workflows, it might be a bumpy road to set it up, but it is not that often such new workflows are being made.
I don't mind trying this change out. 👍

[native-api]

Okay, in it goes.

@sashashura , thanks for adding explanations why the permissons are needed!