diff --git a/bin/gitlab-shell b/bin/gitlab-shell index 815402931b..084f0c9c91 100755 --- a/bin/gitlab-shell +++ b/bin/gitlab-shell @@ -5,6 +5,9 @@ unless ENV['SSH_CONNECTION'] exit end +key_id = /key-[0-9]+/.match(ARGV.join).to_s +original_cmd = ENV['SSH_ORIGINAL_COMMAND'] + require_relative '../lib/gitlab_init' # @@ -14,7 +17,7 @@ require_relative '../lib/gitlab_init' # require File.join(ROOT_PATH, 'lib', 'gitlab_shell') -if GitlabShell.new.exec +if GitlabShell.new(key_id, original_cmd).exec exit 0 else exit 1 diff --git a/hooks/post-receive b/hooks/post-receive index 301f639e99..da0ff416c6 100755 --- a/hooks/post-receive +++ b/hooks/post-receive @@ -7,6 +7,9 @@ refs = ARGF.read key_id = ENV['GL_ID'] repo_path = Dir.pwd +# reset GL_ID env since we already got its value +ENV['GL_ID'] = nil + require_relative '../lib/gitlab_custom_hook' require_relative '../lib/gitlab_post_receive' diff --git a/lib/gitlab_custom_hook.rb b/lib/gitlab_custom_hook.rb index a099e79b27..ac6837de26 100644 --- a/lib/gitlab_custom_hook.rb +++ b/lib/gitlab_custom_hook.rb @@ -4,24 +4,21 @@ class GitlabCustomHook def pre_receive(changes, repo_path) hook = hook_file('pre-receive', repo_path) return true if hook.nil? - if call_receive_hook(hook, changes) - return true - else - # reset GL_ID env since we stop git push here - ENV['GL_ID'] = nil - return false - end + + call_receive_hook(hook, changes) end def post_receive(changes, repo_path) hook = hook_file('post-receive', repo_path) return true if hook.nil? - call_receive_hook(hook, changes) ? true : false + + call_receive_hook(hook, changes) end def update(ref_name, old_value, new_value, repo_path) hook = hook_file('update', repo_path) return true if hook.nil? + system(hook, ref_name, old_value, new_value) end diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb index 3c601eeb8e..ede64f2f45 100644 --- a/lib/gitlab_post_receive.rb +++ b/lib/gitlab_post_receive.rb @@ -13,10 +13,6 @@ def initialize(repo_path, actor, changes) end def exec - # reset GL_ID env since we already - # get value from it - ENV['GL_ID'] = nil - result = update_redis begin diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index d075048705..7249836e0a 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -7,11 +7,13 @@ class AccessDeniedError < StandardError; end class DisallowedCommandError < StandardError; end class InvalidRepositoryPathError < StandardError; end + GIT_COMMANDS = %w(git-upload-pack git-receive-pack git-upload-archive git-annex-shell).freeze + attr_accessor :key_id, :repo_name, :git_cmd, :repos_path, :repo_name - def initialize - @key_id = /key-[0-9]+/.match(ARGV.join).to_s - @origin_cmd = ENV['SSH_ORIGINAL_COMMAND'] + def initialize(key_id, origin_cmd) + @key_id = key_id + @origin_cmd = origin_cmd @config = GitlabConfig.new @repos_path = @config.repos_path end @@ -24,12 +26,7 @@ def exec parse_cmd - raise DisallowedCommandError unless git_cmds.include?(@git_cmd) - - ENV['GL_ID'] = @key_id - status = api.check_access(@git_cmd, @repo_name, @key_id, '_any') - - raise AccessDeniedError, status.message unless status.allowed? + verify_access process_cmd @@ -60,6 +57,8 @@ def parse_cmd args = Shellwords.shellwords(@origin_cmd) @git_cmd = args.first + raise DisallowedCommandError unless GIT_COMMANDS.include?(@git_cmd) + if @git_cmd == 'git-annex-shell' raise DisallowedCommandError unless @config.git_annex_enabled? @@ -73,8 +72,10 @@ def parse_cmd end end - def git_cmds - %w(git-upload-pack git-receive-pack git-upload-archive git-annex-shell) + def verify_access + status = api.check_access(@git_cmd, @repo_name, @key_id, '_any') + + raise AccessDeniedError, status.message unless status.allowed? end def process_cmd @@ -105,7 +106,7 @@ def process_cmd # This method is not covered by Rspec because it ends the current Ruby process. def exec_cmd(*args) - Kernel::exec({ 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'GL_ID' => ENV['GL_ID'] }, *args, unsetenv_others: true) + Kernel::exec({ 'PATH' => ENV['PATH'], 'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'], 'GL_ID' => @key_id }, *args, unsetenv_others: true) end def api diff --git a/spec/gitlab_post_receive_spec.rb b/spec/gitlab_post_receive_spec.rb index 5a9892a13c..3c1f3625f9 100644 --- a/spec/gitlab_post_receive_spec.rb +++ b/spec/gitlab_post_receive_spec.rb @@ -24,14 +24,6 @@ allow(gitlab_post_receive).to receive(:system).and_return(true) end - it "resets the GL_ID environment variable" do - ENV["GL_ID"] = actor - - gitlab_post_receive.exec - - expect(ENV["GL_ID"]).to be_nil - end - it "prints the broadcast message" do expect(gitlab_post_receive).to receive(:puts).ordered expect(gitlab_post_receive).to receive(:puts).with( diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 6bb4db40eb..77523d7f2d 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -13,7 +13,7 @@ subject do ARGV[0] = key_id - GitlabShell.new.tap do |shell| + GitlabShell.new(key_id, ssh_cmd).tap do |shell| shell.stub(exec_cmd: :exec_called) shell.stub(api: api) end @@ -27,6 +27,7 @@ end let(:key_id) { "key-#{rand(100) + 100}" } + let(:ssh_cmd) { nil } let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') } before do @@ -34,7 +35,7 @@ end describe :initialize do - before { ssh_cmd 'git-receive-pack' } + let(:ssh_cmd) { 'git-receive-pack' } its(:key_id) { should == key_id } its(:repos_path) { should == tmp_repos_path } @@ -43,8 +44,9 @@ describe :parse_cmd do describe 'git' do context 'w/o namespace' do + let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' } + before do - ssh_cmd 'git-upload-pack gitlab-ci.git' subject.send :parse_cmd end @@ -53,8 +55,9 @@ end context 'namespace' do + let(:ssh_cmd) { 'git-upload-pack dmitriy.zaporozhets/gitlab-ci.git' } + before do - ssh_cmd 'git-upload-pack dmitriy.zaporozhets/gitlab-ci.git' subject.send :parse_cmd end @@ -63,7 +66,7 @@ end context 'with an invalid number of arguments' do - before { ssh_cmd 'foobar' } + let(:ssh_cmd) { 'foobar' } it "should raise an DisallowedCommandError" do expect { subject.send :parse_cmd }.to raise_error(GitlabShell::DisallowedCommandError) @@ -74,6 +77,8 @@ describe 'git-annex' do let(:repo_path) { File.join(tmp_repos_path, 'dzaporozhets/gitlab.git') } + let(:ssh_cmd) { 'git-annex-shell inannex /~/dzaporozhets/gitlab.git SHA256E' } + before do GitlabConfig.any_instance.stub(git_annex_enabled?: true) @@ -82,7 +87,6 @@ cmd = %W(git --git-dir=#{repo_path} init --bare) system(*cmd) - ssh_cmd 'git-annex-shell inannex /~/dzaporozhets/gitlab.git SHA256E' subject.send :parse_cmd end @@ -97,7 +101,7 @@ describe :exec do context 'git-upload-pack' do - before { ssh_cmd 'git-upload-pack gitlab-ci.git' } + let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' } after { subject.exec } it "should process the command" do @@ -108,10 +112,6 @@ subject.should_receive(:exec_cmd).with("git-upload-pack", File.join(tmp_repos_path, 'gitlab-ci.git')) end - it "should set the GL_ID environment variable" do - ENV.should_receive("[]=").with("GL_ID", key_id) - end - it "should log the command execution" do message = "gitlab-shell: executing git command " message << " " @@ -126,7 +126,7 @@ end context 'git-receive-pack' do - before { ssh_cmd 'git-receive-pack gitlab-ci.git' } + let(:ssh_cmd) { 'git-receive-pack gitlab-ci.git' } after { subject.exec } it "should process the command" do @@ -146,7 +146,7 @@ end context 'arbitrary command' do - before { ssh_cmd 'arbitrary command' } + let(:ssh_cmd) { 'arbitrary command' } after { subject.exec } it "should not process the command" do @@ -163,8 +163,7 @@ end end - context 'no command' do - before { ssh_cmd nil } + context 'no command' do after { subject.exec } it "should call api.discover" do @@ -173,8 +172,9 @@ end context "failed connection" do + let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' } + before { - ssh_cmd 'git-upload-pack gitlab-ci.git' api.stub(:check_access).and_raise(GitlabNet::ApiUnreachableError) } after { subject.exec } @@ -189,9 +189,10 @@ end describe 'git-annex' do + let(:ssh_cmd) { 'git-annex-shell commit /~/gitlab-ci.git SHA256' } + before do GitlabConfig.any_instance.stub(git_annex_enabled?: true) - ssh_cmd 'git-annex-shell commit /~/gitlab-ci.git SHA256' end after { subject.exec } @@ -203,7 +204,7 @@ end describe :validate_access do - before { ssh_cmd 'git-upload-pack gitlab-ci.git' } + let(:ssh_cmd) { 'git-upload-pack gitlab-ci.git' } after { subject.exec } it "should call api.check_access" do @@ -220,7 +221,7 @@ end describe :exec_cmd do - let(:shell) { GitlabShell.new } + let(:shell) { GitlabShell.new(key_id, ssh_cmd) } before { Kernel.stub!(:exec) } it "uses Kernel::exec method" do @@ -230,21 +231,17 @@ end describe :api do - let(:shell) { GitlabShell.new } + let(:shell) { GitlabShell.new(key_id, ssh_cmd) } subject { shell.send :api } it { should be_a(GitlabNet) } end describe :escape_path do - let(:shell) { GitlabShell.new } + let(:shell) { GitlabShell.new(key_id, ssh_cmd) } before { File.stub(:absolute_path) { 'y' } } subject { -> { shell.send(:escape_path, 'z') } } it { should raise_error(GitlabShell::InvalidRepositoryPathError) } end - - def ssh_cmd(cmd) - ENV['SSH_ORIGINAL_COMMAND'] = cmd - end end