rultor icon indicating copy to clipboard operation
rultor copied to clipboard

Published 20 hours ago • verified publisher icon yegor256

Non authorized persons can use rultor to merge stuff

Open Makman2 opened this issue 9 years ago • 13 comments
trafficstars

https://github.com/coala-analyzer/documentation/pull/78#issuecomment-241818795

tamj0rd2 shall not be able to merge the stuff, he has no write privileges.

This is quite critical as this is imposes a security issue^^

Makman2 avatar Aug 23 '16 19:08 Makman2

@Makman2 looking into this, I think the issue is rather one with documentation here though. If you define an (or multiple) architect(s) in the config, then this will be enforced. If you do not define one, the case is poorly handled/documented.

Will be looking into this, in the meantime I suggest defining the permissions in the .rultor.yml.

original-brownbear avatar Aug 23 '16 19:08 original-brownbear

@alex-palevsky valid bug.

original-brownbear avatar Aug 23 '16 19:08 original-brownbear

that was fast, thank you very much :D

Makman2 avatar Aug 23 '16 19:08 Makman2

@Makman2 time is always limited :) => the delays at times in here. But when it's serious issues affecting security or productivity, I'll try addressing them asap .

original-brownbear avatar Aug 23 '16 19:08 original-brownbear

@alex-palevsky valid bug.

@original-brownbear added "bug" tag to this issue

alex-palevsky avatar Aug 24 '16 13:08 alex-palevsky

@makman2 I set the milestone to 2.0 since there is nothing set yet

alex-palevsky avatar Aug 24 '16 15:08 alex-palevsky

wait by default rultor allows merging for read only guests? I shouldn't need to configure anything to prevent that from happening should I?

sils avatar Aug 28 '16 20:08 sils

CC @chauffer

sils avatar Aug 28 '16 20:08 sils

This does indeed seem like an issue which should need more attention. 😞

iakat avatar Aug 28 '16 20:08 iakat

So we still have the issue that any random person with read access can trigger merges, is there any workaround? Do we have to explicitly set the maintainers in the rultor yml?

sils avatar Sep 04 '16 08:09 sils

ping, at least a workaround would be good^^

Makman2 avatar Dec 28 '16 20:12 Makman2

@original-brownbear so, setting the maintainers manually isn't really feasible. The groups change occasionally and we have like a lot of repositories, we don't want to maintain this list manually. I think the best solution would be to set github teams as "architects"

sils avatar Dec 31 '16 15:12 sils

https://developer.github.com/v3/orgs/teams/#get-team-membership could be used to check if the user giving the merge order is part of a team from the architects list

sims1253 avatar Dec 31 '16 15:12 sims1253