python / importlib_metadata

Library to access metadata for Python packages
https://importlib-metadata.readthedocs.io
Apache License 2.0
123 stars 80 forks source link

Set github workflow to minimal permissions #438

Closed joycebrum closed 1 year ago

joycebrum commented 1 year ago

I would like to suggest setting the permissions to the github workflows (currently the main.yml file) as read only on the top level and any write permission be given at the run level.

This is necessary due to a behavior of github workflow to grant to GITHUB_TOKEN write permissions to all types of permissions, regardless of they being used or not. In case of the workflow getting compromised, an attacker can exploit this permissions.

This can be seen in the Action run step "Set up job" such as https://github.com/python/importlib_metadata/actions/runs/4380969737/jobs/7668590805

image

Thus, it is both a recommendation from OpenSSF Scorecard and the Github to always use credentials that are minimally scoped.

Let me know if a PR is welcome and I'll submit it as soon as possible.

Thanks!

Disclosure: I'm from Google, working with the OpenSSF to help many open source projects to increase their supply-chain security.

FFY00 commented 1 year ago

@joycebrum thank you for opening this issue, it is very appreciated!

We do have CI configured to only run automatically for previous contributors, which should mitigate a significant portion of such attacks, but there's indeed no reason to have these permissions enabled. It's actually surprising that this is the default.

Currently, we only have one job that needs such permissions, the release one, but it uses an external GITHUB_TOKEN, which I am assuming only has release upload permissions, but @jaraco could confirm.

I think with the implicit (probably not the best word :sweat_smile:) GITHUB_TOKEN token, we don't actually need to specify a GITHUB_TOKEN, but I don't see any configuration key in the documentation you linked that is obvious to me to target releases.

Also, given that we do have a GITHUB_TOKEN secret already, do you know if it shadows the implicit one, or overwrites it? If that's the case, then we just need to make sure the token has the correct permissions, but it doesn't hurt to set the permissions settings anyway for future-proofing.

joycebrum commented 1 year ago

Good questions.

I've performed some tests and the GITHUBTOKEN you use is the implicit one, github does not allow us to create a secret with name starting with `GITHUB` (Github - Naming your Secrets), so the one you are using in the release step is indeed the default one.

Regarding access to create or edit releases, it is the contents: write which can be granted only to the release step. Sometimes release steps may need id-token: write if it opens connection to other services (OpenID Connect), other than that AFAIK no other permission is needed.

I'll submit a PR and test it as much as possible (considering that it seems the tests are not passing in the current runs) and also try to figure out if tox releases needs id-token write or not.

jaraco commented 1 year ago

Thanks for the suggestion(s). This project derives from and relies on jaraco/skeleton for best practices around packaging and CI (including the main.yml). Any contributions that aren't specific to this project should go there (and thus have impact across hundreds of other projects).

I should be able to cherry-pick the contribution to that project and then merge it back here.

tests are not passing in the current runs

Sorry for any inconvenience. I'll get that straightened out today.

jaraco commented 1 year ago

Is there any reason that this change shouldn't be applied as the Github default, rather than implemented by each project individually? Is there an upstream issue that would track such an effort that would eventually obviate the need for this setting in the downstream projects?

jaraco commented 1 year ago

Superseded by jaraco/skeleton#76. I'd still be interested to hear if there's any one working on applying this default upstream.