search

python-discord / infra

Infrastructure for Python Discord
https://python-discord.github.io/infra/
MIT License
9 stars 4 forks source link

Authenticating commits to main #372

Open jchristgit opened 3 weeks ago

jchristgit commented 3 weeks ago

At the moment we have a sort of implicit requirement that all commits to main must be signed, however, there is no verification of the signature taking place. The GitHub "verified" badge simply asserts that a commit was signed by a key that is associated with a user name, not that it was signed by a trusted key.

Inspired by Authenticate your Git checkouts!, I propose we 1. add a step to our CI that authenticates signing keys against the known set of DevOps keys, and 2. document how to perform this signing key verification locally.

I'm willing to implement this feature.

jchristgit commented 3 weeks ago

More information (and an interesting read in general): https://arxiv.org/pdf/2206.14606v1

jb3 commented 5 days ago

Given that this repo is intended to be open to all, available for all to contribute (primarily staff but not necessarily restricted to this), and a low burden to contribute new things to (e.g. core developers updating environment variables). How do we ensure this change does not prevent those who want to contribute from being able to contribute to the repository?

As an example, anyone making changes through the GitHub Web UI or even our own Core Developer team will be unable to contribute after this change, or at least will be unable to contribute without needing to open a separate contribution to add their key to the keyring.

I'm not sure we should proceed with this if it immediately disqualifies commits from anyone that is not sudo devops.

jchristgit commented 5 days ago

On Thu, Jul 04, 2024 at 07:12:23AM -0700, Joe Banks wrote:

Given that this repo is intended to be open to all, available for all to contribute (primarily staff but not necessarily restricted to this), and a low burden to contribute new things to (e.g. core developers updating environment variables). How do we ensure this change does not prevent those who want to contribute from being able to contribute to the repository?

Fair point. I'm afraid that's the downside of it unfortunately. Either we trust everyone (keeping the current model) or we use an extensible whitelist like this PR proposes. We could also not make it part of CI, but then I fear it would fall out of active usage.

jb3 commented 5 days ago

Moving the discussion from the commit over: I think we definitely need to maintain the GitHub Web Signing key. It's not realistic to expect every change to configuration and variables to come from the sudo devops team. We also need to remember that this is not inclusive of the entire DevOps team (there are 11 members of the Discord role).

I honestly think this is a little bit too far of a change to make, and opinion in the Discord channel seems to concur this. I personally would not be opposed to something like a workflow that validates that all commits at least have signatures.

If we want to be really crazy, we could have a cronjob to pull the recognised PGP keys of all staff members (e.g. https://github.com/jb3.gpg) and form one big staff keyring periodically, then validate in CI that the commits at least come from the staff keyring, though it still closes off contributions from non-staff members. Input from others would be appreciated on this.

jchristgit commented 4 days ago

I agree on all points. The reason for initially only including these keys is that most commits originate from them.

To be honest I think the cronjob idea is overkill. I would really like some way to verify the authenticity of all commits locally via signatures but I agree that this one, for all its benefits, comes with usability impairments that make it not a good solution.

I think it's best to close this for now.