Commit 96c08aedad9004d39d1bd7978a28d71f50a734a3
1 parent
bd6b0bc9
Exists in
colab
and in
4 other branches
Verify GitLab headers on push webhook
Gitlab sends the 'X-Gitlab-Event' header for all the webhook requests. Verify it so we reduce the chance that robots call the hooks accidentally. Signed off by: Daniel Miranda <danielkza2@gmail.com>
Showing
2 changed files
with
27 additions
and
12 deletions
Show diff stats
app/controllers/repositories_controller.rb
| ... | ... | @@ -97,13 +97,17 @@ class RepositoriesController < ApplicationController |
| 97 | 97 | end |
| 98 | 98 | |
| 99 | 99 | def notify_push |
| 100 | + gitlab_event = request.headers['X-Gitlab-Event'] | |
| 101 | + if gitlab_event.nil? || !gitlab_event.end_with?('Push Hook') | |
| 102 | + return render nothing: true, status: :unprocessable_entity | |
| 103 | + end | |
| 100 | 104 | set_repository |
| 101 | - @repository.cancel_processing_of_repository if @repository.last_processing_state.end_with? 'ING' | |
| 105 | + @repository.cancel_processing_of_repository unless %w(READY, ERROR).include? @repository.last_processing_state | |
| 102 | 106 | @repository.process |
| 103 | - render :nothing => true, :status => :ok | |
| 107 | + render nothing: true, status: :ok | |
| 104 | 108 | end |
| 105 | 109 | |
| 106 | -private | |
| 110 | + private | |
| 107 | 111 | def set_project_id_repository_types_and_configurations |
| 108 | 112 | @project_id = params[:project_id] |
| 109 | 113 | @repository_types = Repository.repository_types | ... | ... |
spec/controllers/repositories_controller_spec.rb
| ... | ... | @@ -475,9 +475,14 @@ describe RepositoriesController, :type => :controller do |
| 475 | 475 | end |
| 476 | 476 | |
| 477 | 477 | describe 'notify_push' do |
| 478 | - context 'with a valid repository' do | |
| 479 | - let(:repository) { FactoryGirl.build(:repository) } | |
| 478 | + let(:repository) { FactoryGirl.build(:repository) } | |
| 480 | 479 | |
| 480 | + def post_push | |
| 481 | + @request.env['HTTP_X_GITLAB_EVENT'] = ['Push Hook', 'Tag Push Hook'].sample | |
| 482 | + post :notify_push, id: repository.id | |
| 483 | + end | |
| 484 | + | |
| 485 | + context 'with a valid repository' do | |
| 481 | 486 | before :each do |
| 482 | 487 | Repository.expects(:find).with(repository.id).returns(repository) |
| 483 | 488 | end |
| ... | ... | @@ -487,7 +492,7 @@ describe RepositoriesController, :type => :controller do |
| 487 | 492 | repository.expects(:last_processing_state).returns('INTERPRETING') |
| 488 | 493 | repository.expects(:cancel_processing_of_repository).once |
| 489 | 494 | repository.expects(:process).once |
| 490 | - post :notify_push, id: repository.id | |
| 495 | + post_push | |
| 491 | 496 | end |
| 492 | 497 | |
| 493 | 498 | it { is_expected.to respond_with(:ok) } |
| ... | ... | @@ -497,7 +502,7 @@ describe RepositoriesController, :type => :controller do |
| 497 | 502 | before do |
| 498 | 503 | repository.expects(:last_processing_state).returns('ERROR') |
| 499 | 504 | repository.expects(:process).once |
| 500 | - post :notify_push, id: repository.id | |
| 505 | + post_push | |
| 501 | 506 | end |
| 502 | 507 | |
| 503 | 508 | it { is_expected.to respond_with(:ok) } |
| ... | ... | @@ -507,7 +512,7 @@ describe RepositoriesController, :type => :controller do |
| 507 | 512 | before do |
| 508 | 513 | repository.expects(:last_processing_state).returns('READY') |
| 509 | 514 | repository.expects(:process).once |
| 510 | - post :notify_push, id: repository.id | |
| 515 | + post_push | |
| 511 | 516 | end |
| 512 | 517 | |
| 513 | 518 | it { is_expected.to respond_with(:ok) } |
| ... | ... | @@ -515,14 +520,20 @@ describe RepositoriesController, :type => :controller do |
| 515 | 520 | end |
| 516 | 521 | |
| 517 | 522 | context 'with an invalid repository' do |
| 518 | - let(:repository_id) { 1 } | |
| 519 | - | |
| 520 | 523 | before :each do |
| 521 | - Repository.expects(:find).with(repository_id).raises(KalibroClient::Errors::RecordNotFound) | |
| 522 | - post :notify_push, id: repository_id | |
| 524 | + Repository.expects(:find).with(repository.id).raises(KalibroClient::Errors::RecordNotFound) | |
| 525 | + post_push | |
| 523 | 526 | end |
| 524 | 527 | |
| 525 | 528 | it { is_expected.to respond_with(:not_found) } |
| 526 | 529 | end |
| 530 | + | |
| 531 | + context 'with an invalid header' do | |
| 532 | + before :each do | |
| 533 | + post :notify_push, id: repository.id | |
| 534 | + end | |
| 535 | + | |
| 536 | + it { is_expected.to respond_with(:unprocessable_entity) } | |
| 537 | + end | |
| 527 | 538 | end |
| 528 | 539 | end | ... | ... |