Skip to content
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

Merged
merged 11 commits into from
Apr 24, 2024

Conversation

nellshamrell
Copy link
Contributor

@nellshamrell nellshamrell commented Mar 2, 2020

Rendered

Signed-off-by: Nell Shamrell nellshamrell@gmail.com

@Lokathor
Copy link
Contributor

Lokathor commented Mar 2, 2020

I think that working groups with their own orgs that are outside the rust-lang org should continue to manage their own affairs.

@jonas-schievink jonas-schievink added the T-core Relevant to the core team, which will review and decide on the RFC. label Mar 3, 2020
@ehuss
Copy link
Contributor

ehuss commented Mar 3, 2020

Permissions should only be given to teams within the Rust-Lang organization, not to individuals.

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.

@nellshamrell
Copy link
Contributor Author

Thank you for the feedback, @ehuss! I will add clarity around those points today.

@nellshamrell
Copy link
Contributor Author

nellshamrell commented Mar 3, 2020

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?

@ehuss
Copy link
Contributor

ehuss commented Mar 4, 2020

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 rust-lang/rust wants to give write access to various people from different teams, would there be a "rust-writers" GitHub team (or would they all go to rust-push with write access? I can't see the permissions for most of these things.). I guess I'm still a bit confused by the phrase "must be administered through GitHub teams" when often the permissions need to be adjusted per-individual. It seems like that would be an explosion of hundreds of GitHub teams to create subsets of rust-lang teams.

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).

@nrc nrc changed the title adds rfc for rust-lang org github access policy rust-lang org GitHub access policy Mar 12, 2020

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.

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?

Copy link
Member

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).

Copy link

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.

@BurntSushi
Copy link
Member

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.

@pietroalbini
Copy link
Member

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.

@nellshamrell
Copy link
Contributor Author

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

@BurntSushi
Copy link
Member

I don't see a way to make the moderation team work with GitHub permissions.

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.

@XAMPPRocky
Copy link
Member

@BurntSushi Would you be able to clarify the moderation team's situation with GitHub for those of us unaware?

@BurntSushi
Copy link
Member

Ah sure. Basically, the moderation team is charged with moderating official Rust spaces. That generally includes everything in the rust-lang organization on GitHub. In recent times, the number of repositories in this organization has grown quite a bit. Every so often, the moderators get a report of something happening that could use our attention in one of these repositories. In some of those cases, the moderators do not have access to close/lock/delete issues or issue comments. This means we either can't respond, need someone else to respond on our behalf or need to request ad hoc permission to that repository in order to take action.

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.

@XAMPPRocky
Copy link
Member

XAMPPRocky commented Mar 25, 2020

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 infra-admins except for the mod team. At least then someone on the team is able to handle it.

@BurntSushi
Copy link
Member

How much of that problem do you think would be solved by having selected trusted moderation team members with sufficient permissions? Like infra-admins except for the mod team. At least then someone on the team is able to handle it.

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.

@pietroalbini
Copy link
Member

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:

  • Write, Maintainer or Admin required:
    • Editing or deleting comments
    • Hiding comments
    • Locking conversations
  • Maintainer or Admin required:
    • Limiting interactions in a repository
  • Admin required:
    • Deleting issues

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a 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!

text/0000-github-access-policy.md Outdated Show resolved Hide resolved
text/0000-github-access-policy.md Outdated Show resolved Hide resolved
text/0000-github-access-policy.md Outdated Show resolved Hide resolved
@nellshamrell
Copy link
Contributor Author

Thank you for the feedback, everyone, I have updated the RFC based on this feedback and added clarification where needed.

@BurntSushi
Copy link
Member

@nellshamrell What are your thoughts on incorporating moderation into this RFC somehow? I left a few comments above, but happy to clarify things further!

@nellshamrell
Copy link
Contributor Author

@BurntSushi

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.

@nellshamrell
Copy link
Contributor Author

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 :)

@nellshamrell
Copy link
Contributor Author

@BurntSushi - text has been added!

@BurntSushi
Copy link
Member

@nellshamrell Sounds good! Appreciate it! Although it sounds like that is not entirely consistent with @pietroalbini's stance here? Or maybe I'm misunderstanding?

@nellshamrell
Copy link
Contributor Author

I think it's consistent - but could you confirm, @pietroalbini?

@pietroalbini
Copy link
Member

@BurntSushi @nellshamrell well, the current snippet is not wrong, but it doesn't touch much on the last two paragraphs of my comment.

@nellshamrell
Copy link
Contributor Author

Thank you @pietroalbini!

Responding to the last two paragraphs in your comment:

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.

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?

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.

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?

@pietroalbini
Copy link
Member

@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.

nellshamrell and others added 8 commits March 9, 2024 10:44
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.
@ehuss ehuss force-pushed the add-github-access-policy-rfc branch from d5fb5e6 to 181851e Compare March 9, 2024 21:19
@ehuss
Copy link
Contributor

ehuss commented Mar 9, 2024

@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. rust-lang/rust is going to be one of the most difficult ones to resolve, but I think it is doable. (Some tracking information is here and in open team PRs).

@ehuss
Copy link
Contributor

ehuss commented Mar 9, 2024

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.

@Kobzol
Copy link

Kobzol commented Mar 10, 2024

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.

@Kobzol
Copy link

Kobzol commented Mar 18, 2024

This should also be probably kept in sync with rust-lang/compiler-team#19.

@ehuss
Copy link
Contributor

ehuss commented Apr 1, 2024

@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.

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 1, 2024

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.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Apr 1, 2024
text/2872-github-access-policy.md Show resolved Hide resolved

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:
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 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.

Copy link
Contributor

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.

Copy link
Member

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)

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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!

Copy link
Contributor

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).
Copy link
Contributor

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!

Copy link
Contributor

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.


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.
Copy link
Contributor

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?

Copy link
Contributor

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:
Copy link
Contributor

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.

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Apr 12, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 12, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Apr 12, 2024
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.
Copy link
Contributor

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 …”.

Suggested change
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.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Apr 22, 2024
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 22, 2024

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.

@ehuss
Copy link
Contributor

ehuss commented Apr 24, 2024

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.

@ehuss ehuss merged commit 55f9512 into rust-lang:master Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-leadership-council Relevant to the Leadership Council, which will review and decide on this RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.