protect protected branched to force updates #128
Conversation
lib/gitlab_update.rb Outdated
There was a problem hiding this comment.
This is insecure. You should use:
unknow_refs = IO.popen(%W(git rev-list #{@oldrev}..#{@newrev} --)).readThe -- makes it clear to git that #{@oldrev}..#{@newrev} specifies a commit range. The IO.popen(%W(...)) avoids shell injection caused by incorrect tokenization.
There was a problem hiding this comment.
Also, shouldn't the order of the refs be git rev-list oldrev ^newrev?
There was a problem hiding this comment.
I am not sure. I believe this checks if there are commits on old that aren't part of new.
I just checked: you are right.
According to the git manual about hooks it should be:
missed_refs = IO.popen(%W(git rev-list #{@newrev}..#{@oldrev} --)).read missed_refs.split("\n").size > 0 There was a problem hiding this comment.
@Popl7 newrev..oldrev looks good. I think the line we are discussing has it turned around (oldrev..newrev).
| @Popl7 does it affects both git over http and git over ssh? |
| I am not sure. It works from the post-commit hook that is already used for the checking of authorization of the repo. |
There was a problem hiding this comment.
@randx 👍 for the IO.popen
| @Popl7 can you check tests? |
| I will try to make some tests
|
| @Popl7 Can you add some tests and make it mergeable? |
| hi Dimitri, I can work on this tonight and try to get it mergable. On 25 mrt. 2014, at 09:33, Dmitriy Zaporozhets notifications@github.com wrote:
|
rebased for new code
| @Popl7 thanks. Ping me when ready |
…st-history-rewrite-and-deletion protect protected branched to force updates
| wow it does not work |
There was a problem hiding this comment.
no such variable forced_push
| also it do not pass force_push variable via net |
| Fixed by d299e7e |
Popl7 deleted the add-better-branch-protection-against-history-rewrite-and-deletion branch 4 participants
this is a first (working) setup.
There are changes to the api from gitlab-ce.
I will post a PR for this as well. (#6190 gitlabhq/gitlabhq#6190)
Tests must be made.