From 2254155de8b3b8722ca6352203e2ae8d4579406b Mon Sep 17 00:00:00 2001
From: Alex Dergachev <alex@evolvingweb.ca>
Date: Fri, 5 Dec 2014 21:54:29 +0000
Subject: [PATCH] Stop using insecure shell interpolation, fixes #5

---
 app/models/repository/git_remote.rb | 31 +++++++++++++++++------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/app/models/repository/git_remote.rb b/app/models/repository/git_remote.rb
index 4afd783..3e5fb5a 100644
--- a/app/models/repository/git_remote.rb
+++ b/app/models/repository/git_remote.rb
@@ -1,6 +1,7 @@
 require 'redmine/scm/adapters/git_adapter'
 require 'pathname'
 require 'fileutils'
+require 'open3'
 
 class Repository::GitRemote < Repository::Git
 
@@ -64,21 +65,23 @@ class Repository::GitRemote < Repository::Git
   def ensure_possibly_empty_clone_exists
     Repository::GitRemote.add_known_host(clone_host)
 
-    unless system "git ls-remote -h #{clone_url}"
+    unless system "git", "ls-remote",  "-h",  clone_url
       return "#{clone_url} is not a valid remote."
     end
 
     if Dir.exists? clone_path
-      existing_repo_remote = `git --git-dir #{clone_path} config --get remote.origin.url`
+      existing_repo_remote, err, status = Open3::capture3("git", "--git-dir", clone_path, "config", "--get", "remote.origin.url")
+      return "Unable to run: git --git-dir #{clone_path} config --get remote.origin.url" unless status.success?
+
       unless two_remotes_equal(existing_repo_remote, clone_url)
-        return "Clone path '#{clone_path}' already exits, unmatching clone url: #{existing_repo_remote}"
+        return "Directory '#{clone_path}' already exits, unmatching clone url: #{existing_repo_remote}"
       end
     else
-      unless system "git init --bare #{clone_path}"
-        return  "Unable to run git init at #{clone_path}"
+      unless system "git", "init", "--bare", clone_path
+        return  "Unable to run: git init --bare #{clone_path}"
       end
 
-      unless system "git --git-dir #{clone_path} remote add --tags --mirror=fetch origin #{clone_url}"
+      unless system "git", "--git-dir", clone_path, "remote", "add", "--tags", "--mirror=fetch", "origin",  clone_url
         return  "Unable to run: git --git-dir #{clone_path} remote add #{clone_url}"
       end
     end
@@ -113,7 +116,7 @@ class Repository::GitRemote < Repository::Git
     Rails.logger.warn err if err
 
     # If dir exists and non-empty, should be safe to 'git fetch'
-    unless system "git --git-dir #{clone_path} fetch --all"
+    unless system "git", "--git-dir", clone_path, "fetch", "--all"
       Rails.logger.warn "Unable to run 'git -c #{clone_path} fetch --all'"
     end
   end
@@ -121,20 +124,22 @@ class Repository::GitRemote < Repository::Git
   # Checks if host is in ~/.ssh/known_hosts, adds it if not present
   def self.add_known_host(host)
     # if not found...
-    if `ssh-keygen -F #{host} | grep 'found'` == ""
+    out, status = Open3::capture2("ssh-keygen", "-F", host)
+    raise "Unable to run 'ssh-keygen -F #{host}" unless status
+    unless out.match /found/
       # hack to work with 'docker exec' where HOME isn't set (or set to /)
       ssh_dir = (ENV['HOME'] == "/" || ENV['HOME'] == nil ? "/root" : ENV['HOME']) + "/.ssh"
       ssh_known_hosts = ssh_dir + "/known_hosts"
       begin
         FileUtils.mkdir_p ssh_dir
       rescue e
-        Rails.logger.warn "Unable to create directory #{ssh_dir}: " + "\n" + e.to_s
-        return
+        raise "Unable to create directory #{ssh_dir}: " + "\n\n" + e.to_s
       end
+
       puts "Adding #{host} to #{ssh_known_hosts}"
-      unless system `ssh-keyscan #{host} >> #{ssh_known_hosts}`
-        Rails.logger.warn "Unable to add known host #{host} to #{ssh_known_hosts}"
-      end
+      out, status = Open3::capture2("ssh-keyscan", host)
+      raise "Unable to run 'ssh-keyscan #{host}'" unless status
+      Kernel::open(ssh_known_hosts, 'a') { |f| f.puts out}
     end
   end
 end