Creating a write-only team for Charcoal projects

[ArtOfCode-]

I'm leaving this here because it seems like more people see our GitHub issues than they do chat messages.

This was an idea I had a couple days ago. What if we created another team on the Charcoal-SE organization, with write access to all of Charcoal's projects (as opposed to the admin access that the Core team have)? That allows us to give people who commonly contribute to the projects write access, instead of forcing them to rely on pull requests - which, let's be honest, aren't much fun when you're just waiting around.

I'm thinking of people like Kyll, Ashish, and angussidney, who have sent us the majority of our recent pull requests. There are probably more people I've missed out there, but that's the general idea.

Obviously, they'd need to be people (a) who we can trust, and (b) with privileges for Smokey. I'm not proposing adding just anyone who submits a PR; rather, people who have a track record of good contributions. Those people are a big resource for Charcoal; this would reduce the friction to them helping out.

Thoughts?

[DavidPostill]

I would have no objection :). If I understood the code and was proficient in js I would be contributing myself. Unfortunately js is a language I have never learnt (although I know many other programming languages ...)

[ArtOfCode-]

@DavidPostill Smokey itself is written in Python - only the userscripts are JS. metasmoke is Ruby on Rails. Those are our major three languages, I think.

[DavidPostill]

Lol. I don't know any of those languages. I'm limited to traditional languages Java/C++/C etc ... :)

[Undo1]

Disclaimer: this is probably a giant ramble because I wrote it in my phone early in the morning. Coherence is irrelevant.

Another option is to just add those folks to the collaborators list on the projects they tend to contribute to. If we end up adding someone to multiple projects, it's probably time to start thinking about making them a regular/full member.

Having a write access group and moving people into it who don't need admin access might be a good idea from a security perspective, though.

Also note that, from a future-looking perspective, I'd vote to keep Smokey fairly locked down. If this integration thing happens, it'd be good to have as few people as feasible with access to a giant remote-execution-vulnerability-as-a-feature.

Realistically, I'd probably view to move as many admins as possible into a write group, then use it as you say.

Also, if this integration thing happens, it might make sense to limit who can pull from chat... but that's down the road and easy to implement.

On Sun, Oct 2, 2016, 6:16 AM DavidPostill notifications@github.com wrote:

Lol. I don't know any of those languages. I'm limited to traditional languages Java/C++/C etc ... :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Charcoal-SE/SmokeDetector/issues/231#issuecomment-250968465, or mute the thread https://github.com/notifications/unsubscribe-auth/AE7FZnRbJo38wZ_u9RddE7N0Bq1s1Bwvks5qv6ClgaJpZM4KL_g_ .

[ByteCommander]

Nice idea, I could imagine myself to apply for membership of this write access group in the future, once I found time and motivation to make myself familiar with the Smokey sources. Being able to contribute directly without having to wait up to days for getting a pull request merged would definitely increase the motivation factor.

[angussidney]

Obviously I am a little biased by saying that yes this is a good idea, but I think undo has a point: if I'm not familiar with a language or a project, I don't want my changes to be automatically merged and pulled. It would be better just to give me access to the repos I contribute to.

Also @Undo1 with the pull limiting - maybe you could change it up a bit so that only changes in findspam.py can be pulled, whereas changes anywhere else need to be pulled by an admin.

(Apologies for mobile typing)

[Undo1]

If we're dealing with a malicious actor, changes in findspam can still do anything that the rest of the code can. Realistically, it'd need to be any pull... but that's an issue we can address when necessary and isn't strictly relevant to this.

On Sun, Oct 2, 2016, 4:40 PM angussidney notifications@github.com wrote:

Obviously I am a little biased by saying that yes this is a good idea, but I think undo has a point: if I'm not familiar with a language or a project, I don't want my changes to be automatically merged and pulled. It would be better just to give me access to the repos I contribute to.

Also @Undo1 https://github.com/Undo1 with the pull limiting - maybe you could change it up a bit so that only changes in findspam.py can be pulled, whereas changes anywhere else need to be pulled by an admin.

(Apologies for mobile typing)

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/Charcoal-SE/SmokeDetector/issues/231#issuecomment-251001253, or mute the thread https://github.com/notifications/unsubscribe-auth/AE7FZj1YCRzJ-wx30hCLhL47UbVjvO9_ks5qwDLpgaJpZM4KL_g_ .

[ferrybig]

I think we should finish the migration of the blacklisted domains and patterns to metasmoke, this is better security wise (only data injection, and no code injection), and we can even implement automatic checks on the blacklisted domains, like checking if they aren't found by other checks.

[Manishearth]

That allows us to give people who commonly contribute to the projects write access, instead of forcing them to rely on pull requests

In other projects I maintain (e.g. http://github.com/manishearth/rust-clippy/) I and my co-maintainers have a policy for using pull requests even if you have write access, and getting sign off from another maintainer. In many cases these rules aren't ironclad, for example for trivial PRs or time-sensitive ones we just merge first (perhaps pinging another maintainer to let them know that this happened), and sometimes we ask involved community members for review. This works out really well. IME projects where everyone pushes to master tend to reach a state where everyone breaks everyone's code, and folks understand less of the codebase.

Aside from that, +1 for broadening write access. I echo Undo's concerns about smokey. Since many smokey PRs are for just adding users or regexes, perhaps we could factor that out into a config file or a separate repo, one which has more write access.

[Undo1]

Art's created these teams w/ my approval, so closing this. Nothing's actually changed yet, some folks have pending invites to some limited write access.

@Manishearth I've thought for a while about doing that. We could even sync the regexes to metasmoke and have granular control over permissions there - it wouldn't be hard.