Merge branch 'develop'

* develop: (32 commits) fix hooks in pre-receive-megaadmins-chef Version 1.9.1 Pass tag name to update hook too Version 1.9.0 Fix tests Send api.allowed? request for both http and ssh push version 1.8.5 Move batch-add-keys under the right version Add gitlab-keys batch-add-keys Generate key_line in separate method Version 1.8.4 Add process wait for import Dont import repo if exists Use KILL instead of TERM Use large repo instead one with prompt change seed repo Use Process spawn Changelog entry and new version Add support for import repo timeout Explain why GitlabShell#exec_cmd has no tests ...
Undev · Apr 16, 2014 · 54cd68f · 54cd68f
2 parents 40fd826 + b3ad263
commit 54cd68f
Show file tree

Hide file tree

Showing 15 changed files with 239 additions and 94 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,27 @@
+v1.9.1
+  - Update hook sends branch and tag name 
+
+v1.9.0
+  - Call api in update hook for both ssdh and http push. Requires GitLab 6.7+
+  - Pass oldrev and newrev to api.allowed?
+
+v1.8.5
+  - Add `gitlab-keys batch-add-keys` subcommand for authorized_keys rebuilds
+
+v1.8.4
+  - Dont do import if repository exists
+
+v1.8.3
+  - Add timeout option for repository import
+
+v1.8.2
+  - Fix broken 1.8.1
+
+v1.8.1
+  - Restrict Environment Variables
+  - Add bin/create-hooks command
+  - More safe shell execution
+
 v1.8.0
   - Fix return values in GitlabKeys
 

diff --git a/README.md b/README.md
@@ -1,5 +1,7 @@
 ### gitlab-shell: ssh access and repository management
 
+GitLab Shell is an application that allows you to execute git commands and provide ssh access to git repositories. It is not a unix shell nor a replacement for Bash or Zsh.
+
 #### Code status
 
 * [![CI](http://ci.gitlab.org/projects/4/status.png?ref=master)](http://ci.gitlab.org/projects/4?ref=master)
@@ -34,7 +36,11 @@ Remove repo
 
 Import repo 
 
-    ./bin/gitlab-projects import-project randx/six.git https://github.com/randx/six.git
+    # Default timeout is 2 minutes 
+    ./bin/gitlab-projects import-project randx/six.git https://github.com/randx/six.git 
+
+    # Override timeout in seconds
+    ./bin/gitlab-projects import-project randx/six.git https://github.com/randx/six.git 90
 
 Fork repo
 

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.8.0
+1.9.1
diff --git a/bin/create-hooks b/bin/create-hooks
@@ -0,0 +1,12 @@
+#!/usr/bin/env ruby
+
+# Recreate GitLab hooks in the Git repositories managed by GitLab.
+#
+# This script is used when restoring a GitLab backup.
+
+require_relative '../lib/gitlab_init'
+require File.join(ROOT_PATH, 'lib', 'gitlab_projects')
+
+Dir["#{GitlabConfig.new.repos_path}/*/*.git"].each do |repo|
+  GitlabProjects.create_hooks(repo)
+end
diff --git a/bin/gitlab-keys b/bin/gitlab-keys
@@ -8,9 +8,11 @@ require_relative '../lib/gitlab_init'
 # Ex.
 #   /bin/gitlab-keys add-key key-782 "ssh-rsa AAAAx321..."
 #
+#   printf "key-782\tssh-rsa AAAAx321...\n" | /bin/gitlab-keys batch-add-keys
+#
 #   /bin/gitlab-keys rm-key key-23 "ssh-rsa AAAAx321..."
 #
-#   /bin/gitlab-keys clear"
+#   /bin/gitlab-keys clear
 #
 
 require File.join(ROOT_PATH, 'lib', 'gitlab_keys')

diff --git a/bin/install b/bin/install
@@ -10,18 +10,18 @@ config = GitlabConfig.new
 key_dir = File.dirname("#{config.auth_file}")
 
 commands = [
-  "mkdir -p #{config.repos_path}",
-  "mkdir -p #{key_dir}",
-  "chmod 700 #{key_dir}",
-  "touch #{config.auth_file}",
-  "chmod 600 #{config.auth_file}",
-  "chmod -R ug+rwX,o-rwx #{config.repos_path}",
-  "find #{config.repos_path} -type d -print0 | xargs -0 chmod g+s"
+  %W(mkdir -p #{config.repos_path}),
+  %W(mkdir -p #{key_dir}),
+  %W(chmod 700 #{key_dir}),
+  %W(touch #{config.auth_file}),
+  %W(chmod 600 #{config.auth_file}),
+  %W(chmod -R ug+rwX,o-rwx #{config.repos_path}),
+  %W(find #{config.repos_path} -type d -exec chmod g+s {} ;)
 ]
 
 commands.each do |cmd|
-  print "#{cmd}: "
-  if system(cmd)
+  print "#{cmd.join(' ')}: "
+  if system(*cmd)
     puts 'OK'
   else
     puts 'Failed'

diff --git a/hooks/update b/hooks/update
@@ -6,7 +6,7 @@
 
 refname = ARGV[0]
 key_id  = ENV['GL_ID']
-repo_path = `pwd`
+repo_path = Dir.pwd
 
 require_relative '../lib/gitlab_update'
 

diff --git a/lib/gitlab_keys.rb b/lib/gitlab_keys.rb
@@ -18,6 +18,7 @@ def initialize
   def exec
     case @command
     when 'add-key'; add_key
+    when 'batch-add-keys'; batch_add_keys
     when 'rm-key';  rm_key
     when 'clear';  clear
     else
@@ -31,11 +32,32 @@ def exec
 
   def add_key
     $logger.info "Adding key #{@key_id} => #{@key.inspect}"
-    auth_line = "command=\"#{@config.gitlab_shell_path}/bin/gitlab-shell #{@key_id}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{@key}"
+    auth_line = key_line(@key_id, @key)
     open(auth_file, 'a') { |file| file.puts(auth_line) }
     true
   end
 
+  def batch_add_keys
+    open(auth_file, 'a') do |file|
+      stdin.each_line do |input|
+        tokens = input.strip.split("\t")
+        abort("#{$0}: invalid input #{input.inspect}") unless tokens.count == 2
+        key_id, public_key = tokens
+        $logger.info "Adding key #{key_id} => #{public_key.inspect}"
+        file.puts(key_line(key_id, public_key))
+      end
+    end
+    true
+  end
+
+  def stdin
+    $stdin
+  end
+
+  def key_line(key_id, public_key)
+    "command=\"#{@config.gitlab_shell_path}/bin/gitlab-shell #{key_id}\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty #{public_key}"
+  end
+
   def rm_key
     $logger.info "Removing key #{@key_id}"
     Tempfile.open('authorized_keys') do |temp|

diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb
@@ -6,7 +6,7 @@
 require_relative 'gitlab_logger'
 
 class GitlabNet
-  def allowed?(cmd, repo, key, ref)
+  def allowed?(cmd, repo, actor, ref, oldrev = nil, newrev = nil)
     project_name = repo.gsub("'", "")
     project_name = project_name.gsub(/\.git\Z/, "")
     project_name = project_name.gsub(/\A\//, "")
@@ -16,9 +16,22 @@ def allowed?(cmd, repo, key, ref)
     project_arr.shift if project_arr.size == 3
     project_name = project_arr.join("/")
 
-    key_id = key.gsub("key-", "")
+    params = {
+      action: cmd,
+      ref: ref,
+      project: project_name,
+    }
+
+    params.merge!(oldrev: oldrev) if oldrev
+    params.merge!(newrev: newrev) if newrev
+
+    if actor =~ /\Akey\-\d+\Z/
+      params.merge!(key_id: actor.gsub("key-", ""))
+    elsif actor =~ /\Auser\-\d+\Z/
+      params.merge!(user_id: actor.gsub("user-", ""))
+    end
 
-    url = "#{host}/allowed?key_id=#{key_id}&action=#{cmd}&ref=#{ref}&project=#{project_name}"
+    url = "#{host}/allowed?" + URI.encode_www_form(params)
     resp = get(url)
 
     !!(resp.code == '200' && resp.body == 'true')

diff --git a/lib/gitlab_projects.rb b/lib/gitlab_projects.rb
@@ -1,4 +1,5 @@
 require 'fileutils'
+require 'timeout'
 
 require_relative 'gitlab_config'
 require_relative 'gitlab_logger'
@@ -16,6 +17,12 @@ class GitlabProjects
   # Ex /home/git/repositories/test.git
   attr_reader :full_path
 
+  def self.create_hooks(path)
+    hooks = File.join(path, 'hooks')
+    FileUtils.rm_rf(hooks) if File.exists?(hooks)
+    File.symlink(File.join(@config.gitlab_shell_path, 'hooks'), hooks)
+  end
+
   def initialize
     @command = ARGV.shift
     @project_name = ARGV.shift
@@ -77,16 +84,10 @@ def add_project
     $logger.info "Adding project #{@project_name} at <#{full_path}>."
     FileUtils.mkdir_p(full_path, mode: 0770)
     cmd = %W(git --git-dir=#{full_path} init --bare)
-    system(*cmd) && create_hooks(full_path)
+    system(*cmd) && self.class.create_hooks(full_path)
     FileUtils.chmod_R 0770, full_path
   end
 
-  def create_hooks(path)
-    hooks = File.join(path, 'hooks')
-    FileUtils.rm_rf(hooks) if File.exists?(hooks)
-    File.symlink(File.join(@config.gitlab_shell_path, 'hooks'), hooks)
-  end
-
   def rm_project
     $logger.info "Removing project #{@project_name} from <#{full_path}>."
     FileUtils.rm_rf(full_path)
@@ -95,10 +96,32 @@ def rm_project
   # Import project via git clone --bare
   # URL must be publicly cloneable
   def import_project
+    # Skip import if repo already exists
+    return false if File.exists?(full_path)
+
     @source = ARGV.shift
+
+    # timeout for clone
+    timeout = (ARGV.shift || 120).to_i
     $logger.info "Importing project #{@project_name} from <#{@source}> to <#{full_path}>."
     cmd = %W(git clone --bare -- #{@source} #{full_path})
-    system(*cmd) && create_hooks(full_path)
+
+    pid = Process.spawn(*cmd)
+
+    begin
+      Timeout.timeout(timeout) do
+        Process.wait(pid)
+      end
+    rescue Timeout::Error
+      $logger.error "Importing project #{@project_name} from <#{@source}> failed due to timeout."
+
+      Process.kill('KILL', pid)
+      Process.wait
+      FileUtils.rm_rf(full_path)
+      false
+    else
+      self.class.create_hooks(full_path)
+    end
   end
 
   # Move repository from one directory to another
@@ -174,7 +197,7 @@ def fork_project
 
     $logger.info "Forking project from <#{full_path}> to <#{full_destination_path}>."
     cmd = %W(git clone --bare -- #{full_path} #{full_destination_path})
-    system(*cmd) && create_hooks(full_destination_path)
+    system(*cmd) && self.class.create_hooks(full_destination_path)
   end
 
   def update_head

diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb
@@ -60,8 +60,9 @@ def validate_access
     api.allowed?(@git_cmd, @repo_name, @key_id, '_any')
   end
 
-  def exec_cmd *args
-    Kernel::exec *args
+  # This method is not covered by Rspec because it ends the current Ruby process.
+  def exec_cmd(*args)
+    Kernel::exec({'PATH' => ENV['PATH'], 'GL_ID' => ENV['GL_ID']}, *args, unsetenv_others: true)
   end
 
   def api

diff --git a/lib/gitlab_update.rb b/lib/gitlab_update.rb
@@ -5,7 +5,7 @@
 class GitlabUpdate
   attr_reader :config
 
-  def initialize(repo_path, key_id, refname)
+  def initialize(repo_path, actor, ref)
     @config = GitlabConfig.new
 
     @repo_path = repo_path.strip
@@ -14,9 +14,9 @@ def initialize(repo_path, key_id, refname)
     @repo_name.gsub!(/\.git$/, "")
     @repo_name.gsub!(/^\//, "")
 
-    @key_id = key_id
-    @refname = refname
-    @branch_name = /refs\/heads\/([\/\w\.-]+)/.match(refname).to_a.last
+    @actor = actor
+    @ref = ref
+    @ref_name = ref.gsub(/\Arefs\/(tags|heads)\//, '')
 
     @oldrev  = ARGV[1]
     @newrev  = ARGV[2]
@@ -27,19 +27,12 @@ def exec
     # get value from it
     ENV['GL_ID'] = nil
 
-    # If its push over ssh
-    # we need to check user permission per branch first
-    if ssh?
-      if api.allowed?('git-receive-pack', @repo_name, @key_id, @branch_name)
-        update_redis
-        exit 0
-      else
-        puts "GitLab: You are not allowed to access #{@branch_name}!"
-        exit 1
-      end
-    else
+    if api.allowed?('git-receive-pack', @repo_name, @actor, @ref_name, @oldrev, @newrev)
       update_redis
       exit 0
+    else
+      puts "GitLab: You are not allowed to access #{@ref_name}!"
+      exit 1
     end
   end
 
@@ -49,13 +42,9 @@ def api
     GitlabNet.new
   end
 
-  def ssh?
-    @key_id =~ /\Akey\-\d+\Z/
-  end
-
   def update_redis
     queue = "#{config.redis_namespace}:queue:post_receive"
-    msg = JSON.dump({'class' => 'PostReceive', 'args' => [@repo_path, @oldrev, @newrev, @refname, @key_id]})
+    msg = JSON.dump({'class' => 'PostReceive', 'args' => [@repo_path, @oldrev, @newrev, @ref, @actor]})
     unless system(*config.redis_command, 'rpush', queue, msg, err: '/dev/null', out: '/dev/null')
       puts "GitLab: An unexpected error occurred (redis-cli returned #{$?.exitstatus})."
       exit 1

diff --git a/spec/gitlab_keys_spec.rb b/spec/gitlab_keys_spec.rb
@@ -1,5 +1,6 @@
 require_relative 'spec_helper'
 require_relative '../lib/gitlab_keys'
+require 'stringio'
 
 describe GitlabKeys do
   before do
@@ -39,6 +40,47 @@
     end
   end
 
+  describe :batch_add_keys do
+    let(:gitlab_keys) { build_gitlab_keys('batch-add-keys') }
+    let(:fake_stdin) { StringIO.new("key-12\tssh-dsa ASDFASGADG\nkey-123\tssh-rsa GFDGDFSGSDFG\n", 'r') }
+    before do
+      create_authorized_keys_fixture
+      gitlab_keys.stub(stdin: fake_stdin)
+    end
+
+    it "adds lines at the end of the file" do
+      gitlab_keys.send :batch_add_keys
+      auth_line1 = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-12\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-dsa ASDFASGADG"
+      auth_line2 = "command=\"#{ROOT_PATH}/bin/gitlab-shell key-123\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa GFDGDFSGSDFG"
+      File.read(tmp_authorized_keys_path).should == "existing content\n#{auth_line1}\n#{auth_line2}\n"
+    end
+
+    context "with invalid input" do
+      let(:fake_stdin) { StringIO.new("key-12\tssh-dsa ASDFASGADG\nkey-123\tssh-rsa GFDGDFSGSDFG\nfoo\tbar\tbaz\n", 'r') }
+
+      it "aborts" do
+        gitlab_keys.should_receive(:abort)
+        gitlab_keys.send :batch_add_keys
+      end
+    end
+
+    context "without file writing" do
+      before do
+        gitlab_keys.should_receive(:open).and_yield(mock(:file, puts: nil))
+      end
+
+      it "should log an add-key event" do
+        $logger.should_receive(:info).with('Adding key key-12 => "ssh-dsa ASDFASGADG"')
+        $logger.should_receive(:info).with('Adding key key-123 => "ssh-rsa GFDGDFSGSDFG"')
+        gitlab_keys.send :batch_add_keys
+      end
+
+      it "should return true" do
+        gitlab_keys.send(:batch_add_keys).should be_true
+      end
+    end
+  end
+
   describe :rm_key do
     let(:gitlab_keys) { build_gitlab_keys('rm-key', 'key-741', 'ssh-rsa AAAAB3NzaDAxx2E') }