Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

A bot to manage the members of the Org #22

Closed
williamkapke opened this issue Mar 11, 2017 · 42 comments
Closed

A bot to manage the members of the Org #22

williamkapke opened this issue Mar 11, 2017 · 42 comments

Comments

@williamkapke
Copy link
Contributor

Fellow CommComm-ers: Managing the members of a GitHub Org has been a challenge. Soon we'll start adding sub groups around here and we'll want to manage the Teams.

The TSC recently closed my long standing issue to do this with a bot on there end. That's ok. I've kinda been on an island with my efforts. A bot was started that has been handling many other tasks, but not the ones I set out to do. I still believe this is the proper way to avoid burdening the TSC for these requests.

SO- I offer CommComm this: https://github.com/williamkapke/orgbot

It is purposely not specific to the Node.js GitHub Organization. My hope is that other Organizations could use it and it will gather more contributors focused on solving this problem.

This is only an offering. We can choose to go a different route. BUT,I'm just happy that I finally have this solution out there so we can no longer say "IF we had a bot for this" 😄

I have it running on my test Org. If you would like to play around with it, just let me know and I'll add you to the Org!

Let me know your thoughts. If positive, we'll pursue putting this in production.

@jasnell
Copy link
Member

jasnell commented Mar 11, 2017

Just a quick point, not commenting on the work itself, but the @nodejs/tsc will need to be consulted in order to enable this on the Node.js Github organization. Please make sure to brief the TSC on the intent and purpose.

@ChALkeR
Copy link
Member

ChALkeR commented Mar 11, 2017

It would be more convenvient, sure, but from the security point of view I am not totally convinced that having such a bot would be more secure than just having several people doing that.

@ChALkeR
Copy link
Member

ChALkeR commented Mar 11, 2017

@williamkapke could you add me to the test org and give me write access to some repo (e.g. a new one), please?

@ChALkeR
Copy link
Member

ChALkeR commented Mar 11, 2017

Well, if I understand things correctly, this potentially makes things less secure and let me explain why.

Situation A: the org SomeOrg has 20 owners, who basically could do anything, other members have limited access, e.g. write access to some repos, and most of them don't have the ability to break things badly.

Situation B: the org SomeOrg has a bot with full permissions, 5 real owners, and 600 members (including outside collaborators), each of whom who can interact with the bot in some way in their repo.

Situation B is worse than Situation A if the estimated probability of any potential vulnerability in the bot or its dependencies (or the server it operates on) that would allow some member to do arbitrary things from the bot user is higher than 2.5%. For Situation B to be better, we should estimate that probability as lower than 2.5%. I wouldn't count on that atm.

Note that those 2.5% should be estimated regardless of someone willing to exploit that vulnerability and do malicious things, it doesn't include that, it just includes the technical possibility and discovering such a vulnerability.

Also note that the above calculations were performed under an assumption that all members of SomeOrg, including owners in Situation A, have an equal chance of going crazy. If the org SomeOrg has some filter and chooses owners not randomly from its members but through some more complex procedure, than that could significantly lower the borderline for the vulnerability probability in the bot, and it would become significantly lower than 2.5%.


Note: I could be wrong, I did not dig the details about this bot, and this is just a quick assumption.
If I am wrong, please explain me why =).

@williamkapke
Copy link
Contributor Author

williamkapke commented Mar 11, 2017

@jasnell Of course- there just isn't a reason to bother the TSC if I find people don't want it. Right?

@ChALkeR I added you by using the bot! I added you to the pretend https://github.com/noduh/evangelism team 😄

@ChALkeR
Copy link
Member

ChALkeR commented Mar 12, 2017

TL;DR: I am worrying about giving a bot too much power in the org with a lot of users that could interact with the bot — that could be worse, because the bot, or its deps, or its server, or its platform could have vulnerabilities.

@jasnell
Copy link
Member

jasnell commented Mar 12, 2017 via email

@williamkapke
Copy link
Contributor Author

@ChALkeR there is already a bot running on the Node.js Organization that is working under the exact same conditions.

@williamkapke
Copy link
Contributor Author

@jasnell you had over a year to express these concerns- and didn't. What changed?

@jasnell
Copy link
Member

jasnell commented Mar 12, 2017

I have expressed such concerns before on several occasions. What's changed is that you just now opened this thread today and I am responding to your request for feedback. I'm simply not and haven't been convinced there is a justifying need for an owner-bot, particularly with the risks involved.

The other bot that is operational does not have owner permissions and is therefore a bit less of a risk.

That said, I've given my thoughts and I'd like to hear what others have to say about it.

@williamkapke
Copy link
Contributor Author

williamkapke commented Mar 12, 2017

The TSC approved the permissions and were enabled on Sept 24th.

In the 2016-09-08 TSC meeting, you approved of having a bot like just like this. Again, I'm really curious what changed?

@ChALkeR
Copy link
Member

ChALkeR commented Mar 12, 2017

@williamkapke

@ChALkeR there is already a bot running on the Node.js Organization that is working under the exact same conditions.

Which one?

@williamkapke
Copy link
Contributor Author

williamkapke commented Mar 12, 2017 via email

@ChALkeR
Copy link
Member

ChALkeR commented Mar 12, 2017

@williamkapke The access level for that one differs — it's not org-wide (correct me if I'm wrong).
Also, which repos is it enabled on? If that's only the core one, then only Node.js core collaborators could interact with it there.

Btw, I am not confident that it's such a good idea, as that one is used as a user and GitHub doesn't provide means of restricting the API usage to only issues/prs actions.

But if it's enabled only on the core repo (and perhaps on others with a limited number of collaborators who are also trusted in the core repo) — it looks fine. The key question is — which and how many org members could interact with the bot?

If not — the nodejs-github-bot account should probably be split in separate ones, each of which should have access to each own repo and will be running inside an isolated kvm container.

I will try to take a closer look on both bots this month.

@bnoordhuis
Copy link
Member

I've commented on it before but a script that runs unsupervised makes me uncomfortable.

Humans can evaluate a request to block/ban/add/etc but a bot simply does what it's told, no questions asked. The potential for mayhem in case of a bug or compromised environment is considerable.

@williamkapke
Copy link
Contributor Author

@ChALkeR Thanks for your questions, thoughts, and review.

  • Yes, the current bot is org wide, but We intentionally do not allow Org Hooks. Individual hooks need to be added to the repos that want to use the bot. (Same would be true for this bot)
  • It is on the Core repo- but I'm unsure of the others. I do not have access to see.
  • THIS bot's (my OrgBot) actions are gated. It was built so that it only does the actions when files are pushed. That limits it to people with push access on a per-repo basis. This allows the changes to be PRd and reviewed like all other changes.
  • I'm not sure what you mean by "used as a user"... It uses a Personal Access Token. The GitHub API is granular. The token used on the github-bot has admin:org -- but that is NOT owner access. It allows the bot to "Fully manage organization, teams, and memberships." It cannot do anything else with that permission. (e.g.: Delete repos, Access private repos). Here's a screenshot of the scope options to give you an idea what of what the bot can & CANNOT do:
    API Scopes
    https://developer.github.com/v3/oauth/#scopes

@targos
Copy link
Member

targos commented Mar 15, 2017

Have you considered using the GitHub integrations for that? Permissions are much more flexible and you don't need to create an account with owner rights.
https://developer.github.com/early-access/integrations/

@williamkapke
Copy link
Contributor Author

Hey @targos , the API endpoints that are needed for this are not available (yet?).
https://developer.github.com/early-access/integrations/available-endpoints/

@williamkapke
Copy link
Contributor Author

From nodejs/TSC#219 (comment):

@williamkapke ... I know you previously had done some work on a bot for automating administrative tasks and I know that I had some general concerns around the security of that. I'd like to see if we can revisit that issue. If (a) the bot can be hosted on secured foundation infrastructure and (b) the permissions for the bot can be limited to a specific set of actions such as adding/removing/banning, etc, then I think my concerns around the security can be addressed.

@jasnell here's that thread. ;)

Answers:
a) That's the ONLY place it should run.
b) Indeed they're limited. It was designed to be very constrained.

@jasnell
Copy link
Member

jasnell commented Aug 23, 2017

Ok. So the proposal is to create a new nodejs/admin repo. What I'd like to explore is a bot that monitors (a) posts only in that repo, (b) only opened by people with write access to that repo, and (c) have LGTM's from other people with write access to that repo.

If possible, the bot would do several things:

A) Add/Remove people from the org
B) Add/Remove people from teams
C) Manage temporary and indefinite bans (e.g. for temporary bans, it would ban then automatically un-ban after a given period of time)
D) Add new repos
E) Maintain an audit log of all of these actions (e.g. by posting updates to the issue threads)

@williamkapke
Copy link
Contributor Author

It does most of that list. It doesn't:

  • Remove people. I didn't know the use case for this.
  • Add new repos. All collaborators have that permission I believe & I'm not sure if we can limit it. If we can- this would be a great option!
  • Maintain an audit log. It does update threads- but there isn't a central log... and I think there should be. I only didn't add one in because I didn't know a good central spot to log to. I have some ideas though.

Additionally, your Org administration policy has some great triggers proposed that the bot could maintain. Like- If someone isn't a voting member of TSC/CommComm then their owner permission is revoked.

Maybe we can target some collab summit time around this.

@jasnell
Copy link
Member

jasnell commented Aug 23, 2017

Sounds good to me. My Time Available To Write Additional Code is limited at the moment or else I'd jump in to help with the technical changes myself.

@RichardLitt
Copy link
Contributor

NodeSchool uses a similar bot. It is highly probably that much of the code would apply directly here, and that we can cut down on development time with it.

@rachelnicole
Copy link
Contributor

Here's the source of the NodeSchool bot that @RichardLitt is referencing. https://github.com/mafintosh/nodeschoolbot

I agree that it seems like we could adapt it v easily for anything else we need. I don't mind helping out with it as well, if needed.

@gr2m
Copy link
Contributor

gr2m commented Aug 23, 2017

when the bot is capable to create users and give them admin rights, that is a potential loophole.

I’ve build several GitHub apps in the past and would be happy to help in any way I can, after August.

@bnb
Copy link
Contributor

bnb commented Aug 23, 2017

I'd be more than happy to help out with this / learn a bit with pairing. ❤️

@RichardLitt
Copy link
Contributor

It looks like a few people may be able to help out. I think it might be a good idea to create a repository to track project management for this endeavor, and to open issues with "prs accepted" to help guide this enthusiasm.

@gibfahn
Copy link
Member

gibfahn commented Aug 23, 2017

Quick question, would this make more sense as a standalone bot, or as part of the existing nodejs/github-bot repo/team?

From that repo's README:

The Node.js Foundation's uses this bot to help manage the repositories of the GitHub organization.

cc/ @nodejs/github-bot

EDIT: Rereading the earlier comments, I guess that keeping this proposed owner-bot, with it's more dangerous admin permissions, separate from the existing github-bot might make sense.

@williamkapke
Copy link
Contributor Author

@RichardLitt @rachelnicole Yup- the NodeSchool bot is what spawned this project (See nodejs/TSC#51).

The bot is already written. (See OP)
It just needs to get set up within the GH infra.

@phillipj
Copy link
Member

phillipj commented Aug 23, 2017 via email

@gibfahn
Copy link
Member

gibfahn commented Sep 10, 2017

@williamkapke regarding the events the bot responds to, specifically this:

Accepting a PR sends a push event. So, any push- even ones outside the PR process, will get picked up by the bot.

Does this mean that if someone pushes directly to the branch without raising a PR then the bot will still take the action?

It would be good for the bot to only take action if the change came from an approved PR. I guess we could maybe achieve that by using protected branches with required reviews for Pull Requests.

image

@gibfahn
Copy link
Member

gibfahn commented Sep 10, 2017

Also reading through this it sounds like you were looking at logging bot activity somewhere, is that still something that needs to be implemented?

@williamkapke
Copy link
Contributor Author

@gibfahn

Does this mean that if someone pushes directly to the branch without raising a PR then the bot will still take the action?

Yup. It seemed like that'd be desired. But maybe this behavior needs to be different depending on the action? For instance- we MAY want to block a user immediately by pushing directly. But it seems unlikely we would manage teams in an immediate way like that. Yeah, we could have protected branches- but it's not hard to adjust the rules of when the script is fired either.

sounds like you were looking at logging bot activity somewhere, is that still something that needs to be implemented?

The main idea is that the bot should only react to what it sees committed to GitHub- making PRs and Commits a log on their own. However, yes, I'd like to see some kind of log file like we'd expect to see for any application. My preference, however, is that those logs are public. I've pondered ideas like making a repo where the bot could just append to issue comments or something (creating a new issue daily maybe?). I don't know- open to ideas.

@gibfahn
Copy link
Member

gibfahn commented Sep 11, 2017

The main idea is that the bot should only react to what it sees committed to GitHub- making PRs and Commits a log on their own.

Actually having the commit history for the file be the log is a pretty neat idea. You'd probably want some debugging logs somewhere in case the bot ever didn't do what it told Git that it did, but otherwise that's probably sufficient.

@bnb
Copy link
Contributor

bnb commented Dec 4, 2017

@williamkapke does this issue need to remain open? The Automation WG repos may be a better place for it at this point. Happy to leave it open if there's further discussion to be had 👍

@williamkapke
Copy link
Contributor Author

This issue is specific to CommComm and is ongoing.

@bnb
Copy link
Contributor

bnb commented Dec 4, 2017

@williamkapke awesome. Are there any explicit next steps or updates to surface?

@ChALkeR
Copy link
Member

ChALkeR commented Jan 29, 2018

@williamkapke Sorry, I missed most of this discussion from some point…

The GitHub API is granular. The token used on the github-bot has admin:org -- but that is NOT owner access. It allows the bot to "Fully manage organization, teams, and memberships." It cannot do anything else with that permission. (e.g.: Delete repos, Access private repos).

Am I correct that admin:org scope can manage admins in the org?
That's enough to delete repos, override history, and do anything else in the org.

@williamkapke
Copy link
Contributor Author

Yes, you are correct. The token could be used to make another account an admin- which could then allow that person to do bad things.

Thank you @ChALkeR for pointing this out. It is important to find and acknowledge the attack surfaces we create. My understanding is that we have similar scenarios already that are protected by the build team. Since I'm not on the build team I can't speak to how strong or weak their processes/systems are - but I've gotten the sense they're 💯 %. None-the-less, it is still an attack vector.

Here's my overall status on this:
I built the proof of concept for whatever it's worth in maybe advancing this forward... & I'm happy to have contributed that much. I'm unavailable to take on additional debates required though. While I do believe its the right thing to do, I do not believe we will achieve org-wide consensus. Without consensus, it would require a TSC vote (maybe CommComm too now?).
Also FYI: this was once voted on and approved by the TSC- but it still didn't happen.

@ChALkeR
Copy link
Member

ChALkeR commented Jan 30, 2018

My understanding is that we have similar scenarios already that are protected by the build team.

Could you unpack that, please? I am not sure that I understood that.

@williamkapke
Copy link
Contributor Author

I was just saying that the build team already has a few apps/processes/scripts, with elevated access, that run in their environment(s) that they keep shielded. So, from my understanding, they're all set to add a new bot if needed.

But- I could be wrong... & I defer to the @nodejs/build team to say for certain.

@bnb
Copy link
Contributor

bnb commented Jan 2, 2019

Going to close this issue for a few reasons:

  • No further discussion has happened in the previous six months.
    • The issue was stale prior to the last few engagements
  • There is a dedicated repository for this work
  • There is additional work (see nodejs/automation) happening around automation.

Thanks everyone for your time and effort on the GitHub bot. The two repos mentioned (nodejs/github-bot and nodejs/automation are going to be more productive locations for continuing this discussion in a way that can be resolved by people who are focused on this specific work ❤️

@bnb bnb closed this as completed Jan 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests