-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
rust-lang org GitHub access policy #2872
rust-lang org GitHub access policy #2872
Conversation
I think that working groups with their own orgs that are outside the rust-lang org should continue to manage their own affairs. |
This statement is a little confusing to me, can you clarify it? On its own, I read it that permissions would only be granted to an entire team, but then later it implies that permissions can be doled out to individuals in a team. Is it trying to say that permissions shouldn't be given to anyone not in any team? Can I give access to random people from other teams? Does everyone on every team become a member of the rust-lang organization? If not, how does org membership work? It's also not clear if this is assuming there is some correlation between a repo and a team. Many repos have nebulous or non-existent relationships. rust-lang/rust itself isn't owned by any one team, and it's not really clear which teams are responsible for it. Which teams would get "write" access? I am much more productive with write access. Under these rules, how would I get that? There are repos like rust-lang/mdbook or rust-lang/rust-enhanced which I manage, but don't have any teams associated with them. Would every repo spawn a new team? How would that work with the teams website? There are 133 repos, and many are not associated with a team. |
Thank you for the feedback, @ehuss! I will add clarity around those points today. |
Hello again @ehuss! I've added in some clarification that when I say permissions should be administered through teams I mean the GitHub notion of teams, not the Rust notion of teams. A GitHub team can have access to either one GitHub repo or multiple GitHub repos. Additionally, there can be multiple GitHub teams with access to one or more repos. Some Rust teams - like the Infrastructure team - have multiple GitHub teams associated with them (i.e. Infra and Infra Admins) with different levels of GitHub permissions. Does that help? |
Ah, thanks for the clarification. I'm still a bit uncertain. Will this mean new GitHub teams will need to be created for all the repositories that currently use individual permissions? Will these GitHub teams be driven by rust-lang/team, or is it ad-hoc in the GitHub UI? If a big repo like Another thing that is not clear to me, is the intent here to manage all permissions via rust-lang/team? Or will there still be ad-hoc permissions managed in the GitHub UI? The RFC says "Rust-Lang GitHub teams are administered through the Team repository.", but AFAIK that is only partially true today (only some GitHub teams are managed that way). |
text/0000-github-access-policy.md
Outdated
|
||
Selected members of the [Infrastructure Team](https://github.com/rust-lang/team/blob/master/teams/infra.toml) can also be organization owners if their work requires it. | ||
|
||
Owners should use a separate account from their main GitHub account dedicated to managing the organization. This account may not be used to commit code and must have 2FA enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recognize I am merely a curious bystander, so if my comments are unwelcome please let me know.
The requirement to use a separate account seems odd to me. It seems to increase the attack surface, decrease the availability of each account since you can only be logged into one account at a time, and seems to imply that it is acceptable to not use 2FA on the account used to commit code.
What is the benefit to using a separate account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recognize I am merely a curious bystander, so if my comments are unwelcome please let me know.
All comments and suggestions are welcome!
and seems to imply that it is acceptable to not use 2FA on the account used to commit code.
2FA is already enforced org-wide, so nobody can really have an account without it enabled. If it creates confusion, we can remove the 2FA mention in that sentence.
What is the benefit to using a separate account?
The intent of the separate account is to reduce the amount of permissions someone has in their main session, with the idea they would log in an out the separate account only when needing elevated privileges.
This will reduce the risk when someone loses access to their device, which could happen with laptops and especially smartphones. As a data point, I personally stopped being logged in on my phone when I became org owner, as I didn't feel safe having a owner session in my pocket.
The fact that GitHub's sudo mode doesn't cover everything doesn't help with the situation (i.e. you can delete the whole org without being asked for the password).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decrease the availability of each account since you can only be logged into one account at a time
One way to work around this is to only log into the owner account from a "private browsing" session, only for the needed admin duration. That provides no disruption to the working account in either the GUI or from the CLI.
Would it be possible for this RFC to include the moderation team in it? AFAIK, our permissions are currently given on a very ad hoc basis. We are somewhat limited (AIUI) by GitHub's inflexible permission model, but it would still be good to address it here if we're trying to clean up our processes. |
I don't see a way to make the moderation team work with GitHub permissions. A more feasible approach is probably to create a "moderation platform" where (for example) you copy the link to a comment and a bot will delete it. |
Hi @ehuss! Following up on your questions. The intent is for all permissions to be managed via rust-lang/team. This makes it much easier to understand who has what permissions and, should someone depart from a team, to remove permissions that are no longer needed or desired. Having this be done in one place would make managing the permissions much more straightforward. It is true that there are teams managing permissions ad-hoc via the GitHub UI. Should this RFC be adopted, they should move (which may take some time) to managing permissions via rustlang/team. For repos that currently grant permissions to individuals, rather than teams, they would need to move to managing permissions through teams only. Again, this may take a little bit of time to do and that is ok. My advice to them would be to evaluate the existing teams currently in rust-lang/teams and see if there are any that meet their needs. If there is not an existing team that would work for them, they could choose to create a new team. It is not required that every repo have a dedicated team solely for that repo. If they need one team with certain permissions and another team with different permissions, they would need to create both those teams. This may result in several new teams being administered through rustlang/team - but I still believe this is an improvement over the current ad hoc way we manage permissions now. Should this RFC be accepted as it is right now, then all repos that are currently tuning permissions in the GitHub UI should switch to using |
I think the RFC should address it in some way. In particular, there is a trade off being made here. Moderation could work with GitHub permissions, but it would sacrifice some other property that we care about. I think those kinds of things should be made explicit so that we can be clear on what our shared values are here. |
@BurntSushi Would you be able to clarify the moderation team's situation with GitHub for those of us unaware? |
Ah sure. Basically, the moderation team is charged with moderating official Rust spaces. That generally includes everything in the Moderation is already an emotionally difficult task, and adding more barriers to doing it kind of sucks. If the bot approach mentioned by @pietroalbini above is easy and it works, then I don't think I'd have many complaints there. But it would definitely need the ability to lock issues, for examples. Now, thankfully, we don't get such reports very often. At this point in time, they are quite rare. Which is why I'm not particularly keen on pushing strongly for a "fix" to this necessarily at this point in time. For the most part, all I'd really like is to see the moderation team represented in some fashion in this RFC to at least indicate that we've put thought into it. For example, I think one of the reasons why the GitHub permission model doesn't work well here is that there is no way to assign granular permissions to mods, such that giving them the ability to delete/lock issues ends up giving them too much power. Documenting this sort of thing as something that we've given thought to, and the things we are giving up, seems like a good idea to me. Giving a bit more space to future possible solutions to this problem would also be great. |
That's understandable. How much of that problem do you think would be solved by having selected trusted moderation team members with sufficient permissions? Like |
I think that's probably the crux of the matter. I actually don't understand all of the technical details here, so maybe @pietroalbini could elaborate a bit more. But I definitely get the high order bit here: we would ideally lock down permissions on repositories as much as possible. Having some trusted members above that is a knock against that strategy. FWIW, there are only four people on the moderation team right now. So a "subset" might actually be the "whole set" at this point in time. :-) But yes, in principle, I don't have any objections to a subset model from the moderation side. |
Unfortunately the permission levels GitHub offers are not really suited for us, and the moderation team would need write or admin access to every repository in the organization to properly do their work (without the need to ping an organization owner). Giving those permissions across the org would grant access to private repositories containing sensitive information (such as security stuff) and allow to push to any repository that doesn't have branch protection configured (some of them also automatically deploy stuff to production). It's probably fine to give the moderation team write access to the most active repositories (like rust-lang/rust and rust-lang/rfcs) for now, as those repositories contain only public information and have security measures such as protected branches in place. That would allow them to do most of their work without the need to bring an org owner in the loop, reducing the amount of times they need to ping someone. Thinking more long-term, if we decide it's worth having the moderation team with write access to every public repositories we'd have to implement automation to manage that, otherwise some sort of moderation platform they can authenticate to would be the best choice. By the way, here is the privileges needed to do common moderation actions, extracted from the docs:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to see some codification of our processes!
Thank you for the feedback, everyone, I have updated the RFC based on this feedback and added clarification where needed. |
@nellshamrell What are your thoughts on incorporating moderation into this RFC somehow? I left a few comments above, but happy to clarify things further! |
Thanks for the comment. I believe the best course of action for the moderators team would be to form a GitHub team (managed through the teams repo) that has write access to all of the major repos (there are some private repos - usually security related - that they would not have access to). This seems to fit in well with the guidelines around teams and permissions in the RFC as it is, I'm not sure I see the need to specifically call out the moderation team in the text of the RFC. |
Actually...I just re-read all the previous comments and saw the notes about including text about the moderation team to show that we considered it. I'll add in some text right now :) |
@BurntSushi - text has been added! |
@nellshamrell Sounds good! Appreciate it! Although it sounds like that is not entirely consistent with @pietroalbini's stance here? Or maybe I'm misunderstanding? |
I think it's consistent - but could you confirm, @pietroalbini? |
@BurntSushi @nellshamrell well, the current snippet is not wrong, but it doesn't touch much on the last two paragraphs of my comment. |
Thank you @pietroalbini! Responding to the last two paragraphs in your comment:
I feel we can do this through the GitHub teams repo - set up a moderators team and grant it write access to the most active repositories including (but not limited to) rust-lang/rust and rust-lang/rfcs. Is there more I should address re: this in this RFC?
I agree on this - but I feel addressing it may be out of scope for this particular RFC. Could this be something that is addressed in a future RFC? |
@nellshamrell I'd explain a bit that we can't give access to all repositories to a team, as there are sensitive private repos in the org, and that if a team needs access to everything tooling can be developed to selectively give them the level of access they need. |
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
Signed-off-by: Nell Shamrell <nellshamrell@gmail.com>
In consultation with the infra team at https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/GitHub.20access.20policy we have decided to separate this concern out.
We have decided that the leadership council does not get special access.
d5fb5e6
to
181851e
Compare
@rust-lang/leadership-council, I have made some significant edits to this RFC to try to bring it up-to-date and ready to merge. A significant change is to drop the requirement that org admins use separate accounts after consultation with the infra team. If you have a chance, I would appreciate if you could review this new revision. I would like to move this towards FCP within a few weeks if there isn't any significant feedback. We are currently in the process of migrating repository permissions to the team database. Most repositories have some team or group that is obviously associated with them. There are a few outliers, and we are working towards resolving them, by either creating a new team or archiving or moving the repository. |
cc @Kobzol Just FYI, I'm not sure if you have seen this, since you have been doing a lot of the work to clean this up. |
I haven't seen the updates, thanks! Btw most of the remaining repos are (or should be) archived, so we're just waiting for a merge (and perhaps a test) of the archival sync-team infra. |
This should also be probably kept in sync with rust-lang/compiler-team#19. |
@rfcbot fcp merge @rust-lang/leadership-council I'm proposing to go ahead and merge this RFC. I do not think this RFC represents a significant change from how we are currently approaching repo management, but it does write down and clarify exactly what our policy is. |
Team member @ehuss has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
||
GitHub provides several permission levels for access to a repository. Please refer to [GitHub's documentation](https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization) for details on permission levels and what each level can do. | ||
|
||
Repositories in the Rust-Lang organization should follow these permission guidelines: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's worth specifying these here. Instead, it might be worth stating something like "It is generally up to the owning team for a repo on what permissions to grant, but it is strongly recommended to grant more minimal permissions when possible. There a number of bots that can be utilized to help reduce the level of permissions granted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you say more about why we should not provide guidelines on how teams should assign their permissions? I see people get confused about this when setting up team
repo PRs, so I think it would be good to provide guidance and expectations that we all should be following.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, the guidelines are effectively "as minimal permissions as needed", but I think listing out specific permission groups isn't great.
First, it ties us exactly to how github does permissions today. There could very well be a future with changed or custom permissions - of course we could modify the RFC, but that seems not ideal for what is effectively an implementation detail.
Second, in terms of "confusion", I don't think RFCs are the right place to reduce that. Instead, I think documentation on either the repo itself, or on the forge is better. I don't think people generally look to RFCs for docs.
Third, I dont even think these descriptions as-is are super helpful: namely, the difference between write and maintain are not elaborated. (Sure, we could edit the text to better elaborate, but again I dont think this is the place for it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we added something like "permissions should be as minimal as is feasible. In terms of the GitHub permissions as of this writing, the following are recommended"? I found some of the rational behind the various permission levels (for example, how they interact with bors) to be helpful. Documentation might be the right place to keep the normative guidelines/rules, but I see some value here in giving examples about the spirit of the guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI, roles with custom permissions exist now, but they’re only available to paid GitHub Enterprise Cloud repositories (roughly $20 per month per user in the org):
https://docs.github.com/en/enterprise-cloud@latest/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/about-custom-repository-roles
If they were cheaper, they would be a good way to configure a Moderator role.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see this is already mentioned in Future Possibilities!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just FYI, the rust-lang org is on GHEC already.
|
||
This RFC proposes a policy for managing permissions to the [Rust-Lang GitHub Organization](https://www.github.com/rust-lang) and repositories within this organization. | ||
|
||
This RFC was written in consultation with the Governance Working Group and the Infrastructure team. Most discussion took place on [this issue](https://github.com/rust-lang/wg-governance/issues/4) and [this pull request](https://github.com/rust-lang/wg-governance/pull/42). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the governance working group still active, or was their job mostly to create the Leadership Council? If they're still around, I think there's a lot of work the Council would like to delegate to them!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The governance wg was unrelated to the council formation afaik. It was created about 5 years ago, and retired about 3 years ago after it had been dormant for some time.
text/2872-github-access-policy.md
Outdated
|
||
Repositories in the Rust-Lang organization should follow these permission guidelines: | ||
|
||
* **Admin** --- No users or teams except for org admins should have this permission level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are "org admins" the same as "org owners" mentioned above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I have updated to fix the terminology. To my understanding: Owners are entities (people) who have ownership of the org. Admin is a role. Owners have admin permissions on all repositories.
|
||
GitHub provides several permission levels for access to a repository. Please refer to [GitHub's documentation](https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization) for details on permission levels and what each level can do. | ||
|
||
Repositories in the Rust-Lang organization should follow these permission guidelines: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we added something like "permissions should be as minimal as is feasible. In terms of the GitHub permissions as of this writing, the following are recommended"? I found some of the rational behind the various permission levels (for example, how they interact with bors) to be helpful. Documentation might be the right place to keep the normative guidelines/rules, but I see some value here in giving examples about the spirit of the guidelines.
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Use correct GitHub terminology. Owner is an entity, admin is a role. Owners have admin permissions.
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
There can be exceptional cases where a team wants to give repository access to an individual to assist with their work. Requiring them to join or create a team in order to perform that work can be a significant hassle. Teams who find they need this frequently should consider creating a "contributors" subteam for that purpose, or to investigate other tooling to assist with what they need. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible typo: “should consider … , or investigate …” is more consistent than “should consider … , or to investigate …”.
There can be exceptional cases where a team wants to give repository access to an individual to assist with their work. Requiring them to join or create a team in order to perform that work can be a significant hassle. Teams who find they need this frequently should consider creating a "contributors" subteam for that purpose, or to investigate other tooling to assist with what they need. | |
There can be exceptional cases where a team wants to give repository access to an individual to assist with their work. Requiring them to join or create a team in order to perform that work can be a significant hassle. Teams who find they need this frequently should consider creating a "contributors" subteam for that purpose, or investigate other tooling to assist with what they need. |
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. This will be merged soon. |
Thanks everyone! The @rust-lang/leadership-council has decided to accept this RFC. I will be following up with updating documentation on the forge and team repos to incorporate this RFC. |
Rendered
Signed-off-by: Nell Shamrell nellshamrell@gmail.com