[FR] Set permissions for GitHub actions

[joycebrum]

What's the problem this feature will solve?

Hi, I work on behalf of Google and OpenSSF to help Open Source Projects to improve their Supply Chain Security.

I saw this interesting PR #3272 in the project that saddly was closed without further discussion and, if that's ok for you, I would like to reopen the discussion with some informations I think that wasn't consider.

In fact pull_request_target is one of the triggers that have all permissions as write by default, but it is not the only one.

In the set up job step of any action we can see the permissions granted to the GITHUB_TOKEN and, in the case of both main.yml and ci-sage.yml all permissions are set to write (see the evidences in the Additional Context section).

Since it is not always clear what permissions are being granted for a workflow, it is both a recommendation from OpenSSF Scorecard and the Github to always use credentials that are minimally scoped.

I also selected some thread vectors known for each write permission granted:

• statuses - May allow an attacker to change the result of pre-submit checks and get a PR merged.
• checks - May allow an attacker to remove pre-submit checks and introduce a bug.
• security-events - May allow an attacker to read vulnerability reports before a patch is available. Should only be granted to recognized actions for uploading SARIF results.
• deployments - May allow an attacker to charge repo owner by triggering VM runs, and tiny chance an attacker can trigger a remote service with code they own if server accepts code/location variables unsanitized.
• contents - Allows an attacker to commit unreviewed code. Should only be granted to recognized packaging actions or commands.
• packages - Allows an attacker to publish packages. Should only be granted to recognized packaging actions or commands.
• actions - May allow an attacker to steal GitHub secrets by approving to run an action that needs approval.

Describe the solution you'd like

If that's ok for you to try integrate this supply-chain security feature again, I would like to suggest a PR defining the top-level permission as read only and each write permission needed on run level.

I also will provide a correct run evidence for each workflow to be sure nothing is break.

Let me know if this change is welcome.

Alternative Solutions

No response

Additional context

ci-sage.yml

main.yml

Code of Conduct

• I agree to follow the PSF Code of Conduct

[abravalheri]

Hi @joycebrum thank you very much for reaching out. I think there is no problem in discussing how to improve the security, specially if we have a PR :)

One thing to consider is that setuptools also shares some common CI code with https://github.com/jaraco/skeleton, so I think it would be interesting to also propose the same changes there...

I have a few comments about your post (but I am no security specialist or specialist in GHA, so they are only comments).

You mentioned:

statuses - May allow an attacker to change the result of pre-submit checks and get a PR merged.
checks - May allow an attacker to remove pre-submit checks and introduce a bug.
contents - Allows an attacker to commit unreviewed code. Should only be granted to recognized packaging actions or commands.
packages - Allows an attacker to publish packages. Should only be granted to recognized packaging actions or commands.

I think we would like to keep the possibility of maintainers manually intervening (merging, committing, pushing, publishing tags, etc...). Of course this should not be extended to all participants. If we can preserve this ability while increasing security it would be great.

[joycebrum]

Great to hear that, soon I'll send a PR

About your concern, don't worry, this does not affect the overall funcionality of the action or the PRs, it only prevent extra damage in case of a compromised CI.

[rffontenelle]

Default values for permissions changed to read-only as per this GitHub blog post from Feb 2, 2023, but I agree it is safer to set permissions to read-only.