Skip to content

Add general permissions policy document #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 16, 2023

Conversation

astrofrog
Copy link
Member

@astrofrog astrofrog commented Oct 3, 2020

This is a continuation and generalization of #99. It is not in any way complete but shows how I think we might want to organize things to be more general than #99, and be more general than just GitHub. Once complete, this fixes #143.

For now this is very incomplete, but I can try and work on it more if people are generally on board with the idea. Let's also make this what we want not what is currently the case (e.g. in this version the release coordinators have admin access to repos, not write access)

cc @eteq as the author of #99 - we may want to just merge #99 as the 'current policy' as this present PR may take a few weeks to hash out fully.

Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, overall LGTM. Just minor comments.

Maybe should also link to https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization, which explains the different repo permission levels.

@astrofrog astrofrog marked this pull request as draft October 3, 2020 22:02
## Coordination committee

The coordination committee members receive **owner access** to all repositories
in the astropy organization. In addition, they have access to the project
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is wrong. The end goal I think should be the efficient and safe operation of the project and the proposed access rights doesn't serve that goal.

HR people on the coco doesn't need to have owner access, in fact, it's a significant security risk if they lack the knowledge and insight of github to have this access. There are plenty of precedents when this approach went wrong (you can blame for not having the written policy to follow, or can be honest).

On the other hand an operation team need to have the owner access right to be able to e.g. add new people to the wide 'astropy collaborators' team, or to otherwise manage the teams. There are plenty in the project to choose from into this group who are around regularly enough to make the operation efficient.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should not give owner access to the Coco. As the only elected body they should have that authority and we elect general "coco member" not specific roles "coco position 1: code review", "coco position 2: personal", "coco position 3: strategic planning" etc. Thus, I don't think we can pre-select who on the coco does what - we don't even know who gets elected in the future!

If we ever elect someone who is not comfortable handling that they can simply not use the permissions or be educated about what they mean.

On the other hand, that does not mean that the Coco members have to be the only people with those permissions, personally, I'm fine with giving owner permissions to e.g. the operation team as well. That should go into a different section though, since this one is named "Coordination committee".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coco members can sit in the relevant group who is responsible for the Github accesses. But it should not come as default as it causes a huge security hole with no operational purpose other than giving power.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simply not use the permissions

which is not how it works, user accounts would not notice if something is only possible because they are owners. So, if you want non-technical people on the coco, then remove the unconditional access by default to all the technical and security side of the project based on coco membership and keep it with the technical team.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I read this there's a subtle distinction here. I'd say that better wording here would be
the modification I suggested in https://github.com/astropy/astropy-project/pull/148/files#r663198400. That's not the same as admin access to a repo, which is important because I think admin access should be given more freely than owner*, and should not be particular to the CoCo for the reasons @bsipocz says.

But assuming that clarification is made, it's instructive to look at this list to see what owner gives: https://docs.github.com/en/organizations/managing-peoples-access-to-your-organization-with-roles/permission-levels-for-an-organization .

That includes a lot of power that can 1) be abused and 2) sometimes needs to be used (responsibly). And while some of it is highly technical (e.g., code review settings and github apps), other parts are not, but do require a lot of care from a policy/social perspective
(e.g. managing comments that are disruptive or violate the CoC, keeping track of who should be on a team because of their role in the Project). This means it can't be given out freely with exactly the goal @bsipocz said (that I wholly agree with):

The end goal I think should be the efficient and safe operation of the project

But as @hamogu said, the whole point is that the group that APE0 says this authority is entrusted to is the CoCo, by virtue of the election process. Now in principle the CoCo can delegate some of that power, which is exactly why people have gotten admin or maintainer powers in lots of places. So if it's not the CoCo, who is it that should get every single one of those powers, and how is that group decided on? I'd say the whole point of the CoCo is to be the body to do that, but only as long as the community as a whole (not any individual) entrusts them with that.

To emphasize, I think it's a fair question whether people not in the CoCo should also get this power. There have been times when it made sense to add someone when they needed to exercise these powers, and then back them down to "standard" privledges as soon as they'd done this. Bbut I think APE0 pretty clearly says it's the CoCo who ave the authority to control this power, and mechanically the way they control who gets it is by having it themslves (since only owners can add/remove other owners).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(e.g. managing comments that are disruptive or violate the CoC, keeping track of who should be on a team because of their role in the Project)

managing comments are not a daily task the CoCo does. I bet none on the coco follows the comments anyway, they are notified of incidents if any.

of care from a policy/social perspective
That includes a lot of power that can 1) be abused and

Let's not go into this direction. One person has a track record of deleting emails from email lists and abusing power because they think so on their own and because they have the power. Keeping even more technical power including to delete anything in the org is a proven security hazard on top of things.

Copy link
Member

@bsipocz bsipocz Jul 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And to reiterate: "to ensure the efficient, safe, and secure operation of the project", technical keys should not be with people who don't know the technical side of github operation. It's unfortunate that some of the HR keys are also stored with the same access, but I firmly believe that the direction should go the other way. The technical team should add temporarily access to moderate when a situation arises and not everyone on CoCo by default to keep the tech keys and ability to delete the project, opt into services etc.

@eteq
Copy link
Member

eteq commented Jul 2, 2021

A bit of context that has been discussed out-of-band: given that there's currently a lot of effort going into implementing the APE0 governance process right now, I think the desire is to not push too hard on this until that's done, since setting down policies like this was one of the motivations for completing APE0 in the first place. Of course anyone interested can still discuss here, but I think we don't want to finalize anything on this until the APE0 initialization process is completed (current timeline sets that as by roughly the end of this summer).

@pllim
Copy link
Member

pllim commented Jan 11, 2022

@hamogu , maybe I was thinking of this at the dev telecon today. Looks like it is still in draft form.

## Core package release coordinators

Core package release coordinators receive **admin access** to the core
repository, as well as the astropy-helpers and extension-helpers repositories
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@astrofrog @bsipocz @eteq Trying to clean up old PRs a bit to see if we can move this forward. Just to check: astropy-helpers is no longer requiered, but extension-helpers is, right? Anything else that has been added since @astrofrog wrote this draft?

@hamogu
Copy link
Member

hamogu commented Jan 11, 2022

While reading through this in response to today's dev-telecon, I've decided to merge typographical and less contriversion suggestions, just so the text is better readable.

@hamogu
Copy link
Member

hamogu commented Jun 5, 2023

As discussed in the Coco today: For several roles that involve important keys (e.g. GH organization admins, Pypi organization admin/maintainers) we should have at least three people with access to ensure resiliency in case someone leave the project, dies, is long-term ill etc. If the team that naturally fills that role (e.g. the release team for pypi organization admin access) is < 3 people, I suggest to fill up with Coco member until three people have keys. The Coco members should not DO anything, but just hold a copy of the keys to ensure that the roles can re-filled if needed.

@pllim pllim self-assigned this Jun 5, 2023
@pllim
Copy link
Member

pllim commented Jun 5, 2023

CoCo has decided to revive this, but I will take over the revision. @astrofrog , is it okay for me to push to this branch still, or you rather I open a new PR?

@astrofrog
Copy link
Member Author

Please take over this branch!

@pllim pllim force-pushed the general-permissions branch from d28cd7e to 0f23cd0 Compare June 9, 2023 13:59
@pllim
Copy link
Member

pllim commented Jun 9, 2023

I resolved the conflict but I haven't taken account of all the convo here yet or why there was a conflict.

@pllim
Copy link
Member

pllim commented Jun 9, 2023

2023-06-23 -- I have tried my best to bring the contents up-to-date w.r.t. current practices in the Project. I also expanded the contents based on how I understand the previous discussions, and from my own experience in the Project.

@pllim pllim force-pushed the general-permissions branch from 6e29150 to 7669322 Compare September 12, 2023 18:17
@pllim pllim marked this pull request as ready for review September 12, 2023 18:18
@pllim
Copy link
Member

pllim commented Sep 12, 2023

Okay, I removed the mention of 2FA that turned out to be so controversial.

I think this is ready for another round of review.

Co-authored-by: Marten van Kerkwijk <[email protected]>
@hamogu hamogu merged commit 105f314 into astropy:main Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need written policy on GitHub permissions
8 participants