github: consider downscoping "GitHub Editors" access to Triage role

[dmitshur]

The Go project offers access to a set of people to edit metadata on GitHub issues, in order to help with gardening. This access is documented at https://golang.org/wiki/GithubAccess#editors.

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 gitmirror. However, accidents happen occasionally, and people may unintentionally create new branches (e.g., see https://groups.google.com/d/msg/golang-dev/EqqZf5kTRqI/9BEDmjHwBwAJ).

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.

/cc @golang/osp-team @katiehockman @FiloSottile

[FiloSottile]

Oh nice, strong support for this. I think Write also allows doing things like creating releases, which can easily lead to very effective social engineering attacks.

[mvdan]

Yes please. I can't think of anything else github access should be granting. Perhaps moderating the wiki? Though I think that's mostly done by the Go team anyway.

[dmitshur]

Given that we recently went through a change on Gerrit side for #40699 (/cc @rsc @andybons), I wanted to revisit this now. This issue has received favorable feedback so far.

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:

• Apply labels
• Close, reopen, and assign all issues
• Apply milestones
• Mark duplicate issues and pull requests

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:

• Edit and delete anyone's comments on commits, pull requests, and issues
• Hide anyone's comments
• Lock conversations
• Transfer issues

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.

[ALTree]

I wouldn't be too happy if this is implemented.

I use all of these:

• Edit and delete anyone's comments on [...] issues
• Hide anyone's comments
• Lock conversations
• Transfer issues

more or less regularly.

• Transferring issues is useful when an issue meant for the tools or
  tour trackers is opened here.
• I hide comments to keep threads tidy when, in the early stages of
  triaging, people -including me- post comments that are subsequently
  made outdated or uninteresting by a clarification from the OP; I
  also hide comments that are not on-topic but not so ill-mannered
  as to require deletion. I delete out-of-topic vulgar comments, and
  spam.
• Locking threads can be useful to prevent people from having
  conversations on a closed issue, and as a stronger form of 'close',
  to signal that no further reply is welcome, usually on issues
  that get closed for being obviously trollish.
• I regularly edit issues with messed up formatting, opened by people
  who don't know well how to use github, to make them readable.

It looks like the main goal is to prevent people from creating new
branches by mistake, but I've never done that, and it seems unlikely
to happen, since I interact with the repository exclusively from the
command line using codereview, and the tool probably doesn't even know
what github is, let alone how to push branches there.

I don't know what exactly a "social engineering attack" is, but I'm
also pretty sure no one will ever be able to convince me to create a
github release through sheer persuasion.

This change clearly has downsides, since we wouldn't just be taking
away rights to do things people are not supposed to do anyway (like
pushing a branch or creating a release), but also rights to do things
that some people -e.g. me- have been doing on the issue tracker.

[mvdan]

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?

[FiloSottile]

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.

[ALTree]

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.

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.