Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.Sign up
github: consider downscoping "GitHub Editors" access to Triage role #39055
Back when the "go-approvers" group was created, GitHub did not offer granular access levels, it was either Read (no access to triage), Write (access to triage and push), Admin. We used Write since it was the most fitting option.
The GitHub repository at https://github.com/golang/go is a mirror of the canonical repository at https://go.googlesource.com/go where code review happens, and any changes to it are automatically overwritten by
By now, GitHub seems to offer more granularity in access controls, including a "Triage" role:
It's documented in more details at https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/repository-permission-levels-for-an-organization.
We should investigate and confirm whether it's safe to downscope the go-approvers team to Triage access without causing unintended inconvenience to people who rely on it, and if so, apply the change.
To investigate this, I read GitHub documentation further (it contains very detailed information). The Triage access level is described as "Recommended for contributors who need to proactively manage issues and pull requests without write access", which seems like a good fit for our intent for the "go-approvers" group, described as "the set of people who can edit metadata on issues."
According to the detailed access table, Triage level adds these permissions compared to Read level:
That should work well for most needs that come up during gardening.
However, there are some permissions granted only in the Write access level which are sometimes helpful or necessary, for example:
We definitely need some people to be able to perform those actions. As far as I can tell, we have golang organization admin groups that will continue to have these permissions, and that may be sufficient. (Unfortunately, this means some people will still be able to accidentally create branches on GitHub, etc., but at least that set of people will be greatly reduced.)
I think a good next step here is to apply this change and communicate that to golang-dev@, asking for feedback if it turns out to be problematic or more-harmful-than-helpful. Based on the feedback we hear, we may need to adjust groups and permissions further. But it's seems hard to learn more unless we actually try it.
If all that still sounds good and there aren't new concerns, I'll plan to do that next week or so.
I wouldn't be too happy if this is implemented.
I use all of these:
more or less regularly.
It looks like the main goal is to prevent people from creating new
I don't know what exactly a "social engineering attack" is, but I'm
This change clearly has downsides, since we wouldn't just be taking
I definitely want fewer people to have direct write access, but at the same time I agree with @ALTree. Hiding/deleting spam or hateful comments, locking heated threads, editing bad formatting, and transferring issues are the kind of actions I take on a weekly basis. Especially in the morning, well before the Go team wakes up, since pretty much all of them are in the US.
You could say that it wouldn't hurt that much to wait 4-6h to take action in those cases, but I would generally disagree. Early action is a good way to keep the issue tracker tidy.
Having said all the above, perhaps we should clarify how those moderator-like features should be used. For example, if a comment violates the code of conduct after a warning, should we hide or delete the comment? What about blatant cases like spam or trolling - are we OK with deleting those right away without a warning?
The social engineering risk is not that you'd get conned into doing something, but that there is a large group of people who can make a fake release and probably compromise a lot of downstream users. As the project grows, we need to find ways not to grow the attack surface uncontrollably.
Transfering and locking can be implemented in gopherbot pretty easily. CoC violations should be reported so they can be tracked: just deleting or hiding witholds information from the CoC team later on when they have to decide on a user. Editing is indeed unfortunate.
It seems unlikely though that there are dozens of users who need that permission, so maybe we can have a gardeners and a moderators group.
Oh, right. You meant the other way around.
I still think it's not a particularly interesting attack vector. Unless I'm mistaken, "making a release" only lets me choose an existing commit to tag. Since we already gatekeep commits, the opportunity to make real damage seems limited. I would need an accomplice with commit rights to sneak something in the repo, and if I'm able to do that I wouldn't tag a release on it (which would just make it more likely that someone notices); I would just wait for the next scheduled release.
I guess I could wait for a vulnerability to pop up, tag a minor release that I'm pretending fixed the vulnerability (but actually it's on a vulnerable commit), and then try to convince the victim to install my bad release (but really quickly, or the force push from googlesource.com will delete it(?)). It seems a pretty contrived scenario to me.
I'm not a big fan of the extra "gopherbot please do X" comment notifications, but I assume it's the only option we have if we absolutely want more granular permission access.
Fair enough. One could still report at the same time as hiding a comment, though, for example. They're not mutually exclusive.
+1. It seems reasonable that someone could take a screenshot of the comment/interaction, delete or hide it, then send that screenshot over to the email@example.com as an FYI. That's probably what we should be doing anyway, especially for clear violations. I'm not sure this has been explicitly documented though, and it probably should be once decisions are made. https://golang.org/conduct#conflict-resolution discusses it somewhat, but could be clearer imo.
Apologies if that is off-topic for the issue at hand, though. In general, I'm in favor of cleaning up our permissions for the repo
Thanks for the thoughtful feedback @ALTree and others.
Even though the "go-approvers" group is documented as "People who can edit issues in the "go" repo", in reality some people in it take up much more than just issue triage and gardening, and so a sudden change to the Triage level would be prohibitively limiting and unwelcome. At the same time, we probably want and could allow many more people in the "gardener" role than we need in the "moderator" role.
This original issue was about exploring a small change to the existing "go-approvers" group, primarily to prevent accidental branch creation, but it seems that it cannot be done without disruption and needing further careful thought.
I suspect a better way to proceed could be to leave "go-approvers" alone at first, and create a new "gardeners" group with Triage level access. It will become easier to add new people to the group, and we can also move anyone who'd like to be moved. Or maybe another approach.
I'll move this back to NeedsInvestigation for now. It's likely we'll want to make some changes eventually, but it's not yet clear what exactly, and bigger changes are probably best to consider outside of scope of this specific issue.