protect protected branched to force updates #128
Conversation
| @@ -22,6 +22,11 @@ def initialize(repo_path, key_id, refname) | |||
| @newrev = ARGV[2] | |||
| end | |||
|
|
|||
| def forced_push? | |||
| unknow_refs = `git rev-list #{ @oldrev }..#{ @newrev }` | |||
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. |
| def forced_push? | ||
| missed_refs = IO.popen(%W(git rev-list #{@newrev}..#{@oldrev} --)).read | ||
| missed_refs.split("\n").size > 0 | ||
| end |
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 |
| def exec | ||
| # reset GL_ID env since we already | ||
| # get value from it | ||
| ENV['GL_ID'] = nil | ||
|
|
||
| if api.allowed?('git-receive-pack', @repo_name, @actor, @ref_name, @oldrev, @newrev) | ||
| if api.allowed?('git-receive-pack', @repo_name, @actor, @ref_name, @oldrev, @newrev, forced_push) |
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
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.