Escape invalid shell characters in git commands

lib/git/git.rb

@@ -49,11 +49,11 @@ def merge_branches(target_branch_name, source_branch_name, source_tag_name: nil,
         checkout_branch(target_branch_name)
       end
 
-      commit_message_argument = "-m \"#{commit_message}\"" if commit_message
+      commit_message_argument = "-m \"#{Shellwords.escape(commit_message)}\"" if commit_message
       if source_tag_name.present?
-        source = source_tag_name
+        source = Shellwords.escape(source_tag_name)
       else
-        source = "origin/#{source_branch_name}"
+        source = "origin/#{Shellwords.escape(source_branch_name)}"
       end
 
       raw_output = execute("merge --no-edit #{commit_message_argument} #{source}")
@@ -87,8 +87,7 @@ def clone_repository(default_branch_name)
         execute('clean -f -d')
 
         # move to the master branch
-        execute("checkout #{default_branch_name}")
-        reset
+        checkout_branch(default_branch_name)
 
         # remove branches that no longer exist on origin and update all branches that do
         execute('fetch --prune --all')
@@ -111,12 +110,12 @@ def push
 
     def checkout_branch(branch_name)
       reset
-      execute("checkout #{branch_name}")
+      execute("checkout #{Shellwords.escape(branch_name)}")
       reset
     end
 
     def reset
-      execute("reset --hard origin/#{get_current_branch_name}")
+      execute("reset --hard origin/#{Shellwords.escape(get_current_branch_name)}")
     end
 
     def lookup_tag(tag)
@@ -125,7 +124,7 @@ def lookup_tag(tag)
 
     def diff_branch_with_ancestor(branch_name, ancestor_branch_name)
       # gets the merge base of the branch and its ancestor, then gets a list of files changed since the merge base
-      raw_output = execute("diff --name-only $(git merge-base origin/#{ancestor_branch_name} origin/#{branch_name})..origin/#{branch_name}")
+      raw_output = execute("diff --name-only $(git merge-base origin/#{Shellwords.escape(ancestor_branch_name)} origin/#{Shellwords.escape(branch_name)}..origin/#{Shellwords.escape(branch_name)}")
       raw_output.split("\n")
     end
 

spec/lib/git/git_spec.rb

@@ -10,9 +10,9 @@ def create_mock_open_status(status)
     return status_object
   end
 
-  def mock_execute(stdout_andstderr_str, status, execution_count=1)
+  def mock_execute(stdout_andstderr_str, status, execution_count: 1, expected_command: anything)
     # mock the call and repsonse to execute the git command
-    expect(Open3).to receive(:capture2e).exactly(execution_count).times.and_return([stdout_andstderr_str, create_mock_open_status(status)])
+    expect(Open3).to receive(:capture2e).with(expected_command, anything).exactly(execution_count).times.and_return([stdout_andstderr_str, create_mock_open_status(status)])
   end
 
   it 'can be created' do
@@ -53,8 +53,9 @@ def mock_execute(stdout_andstderr_str, status, execution_count=1)
       end
 
       it 'can update a previously cloned repository' do
-        expect(@git).to receive(:reset).exactly(2).times
-        mock_execute('Success', 1, 4)
+        expect(@git).to receive(:reset).exactly(1).times
+        expect(@git).to receive(:checkout_branch)
+        mock_execute('Success', 1, execution_count: 3)
         FileUtils.mkpath(@git.repository_path)
         @git.clone_repository('default_branch')
       end
@@ -86,14 +87,26 @@ def mock_execute(stdout_andstderr_str, status, execution_count=1)
         mock_execute('Success', 1)
         @git.checkout_branch('branch_name')
       end
+
+      it 'escapes backticks in branch names' do
+        expect(@git).to receive(:reset).exactly(2).times
+        mock_execute('sample output', 1, expected_command: '/usr/bin/git checkout branch_\\`name')
+        @git.checkout_branch('branch_`name')
+      end
     end
 
     describe 'reset' do
       it 'can reset a branch to HEAD of origin' do
-        mock_execute('master', 1)
+        expect(@git).to receive(:get_current_branch_name).and_return('master')
         mock_execute("HEAD is now at beb5e09 Merge branch 'master'", 1)
         @git.reset
       end
+
+      it 'escapes backticks in branch names' do
+        expect(@git).to receive(:get_current_branch_name).and_return('branch_`name')
+        mock_execute('sample output', 1, expected_command: '/usr/bin/git reset --hard origin/branch_\\`name')
+        @git.reset
+      end
     end
 
     describe 'get_branch_list' do
@@ -245,6 +258,21 @@ def mock_execute(stdout_andstderr_str, status, execution_count=1)
                    '85/t/trello_adwords_dashboard_tiles_auto_adjust_font_size',
                    source_tag_name: 'tag_name')).to eq([true, nil])
       end
+
+      it 'escapes backticks in branch names and commit messages' do
+        expect(@git).to receive(:get_current_branch_name).and_return('target`name')
+        mock_execute(
+            "From github.com:/Invoca/web\n" +
+                " * branch            85/t/trello_adwords_dashboard_tiles_auto_adjust_font_size -> FETCH_HEAD\n" +
+                "Auto-merging test/workers/adwords_detail_worker_test.rb\n",
+            1,
+            expected_command: '/usr/bin/git merge --no-edit -m "commit\\`message" origin/source\\`name')
+
+        @git.merge_branches(
+            'target`name',
+            'source`name',
+            commit_message: 'commit`message')
+      end
     end
 
     describe 'lookup_tag' do
@@ -269,6 +297,14 @@ def mock_execute(stdout_andstderr_str, status, execution_count=1)
         mock_execute('', 1)
         expect(@git.diff_branch_with_ancestor('branch', 'ancestor_branch')).to eq([])
       end
+
+      it 'escapes backticks in branch names' do
+        mock_execute(
+            "file1.txt\nfile2.txt\n",
+            1,
+            expected_command: '/usr/bin/git diff --name-only $(git merge-base origin/ancestor\\`_branch origin/branch\\`name..origin/branch\\`name')
+        @git.diff_branch_with_ancestor('branch`name', 'ancestor`_branch')
+      end
     end
 
     describe 'get_current_branch_name' do