Properly shell escape git:wrapper steps

The `upload!` method does escaping of its own internally, so we don't
need to do anything special there. Likewise environment variables are
already handled. The only place we need to explicitly `shellescape` is
when executing the `chmod` command.

Verify this all works as expected by changing the user in the Cucumber
features to be `(GitHub Web Flow) via ShipIt`. This user is now used
when exercising the `git:check` task.

Fixes #1842.

CHANGELOG.md

@@ -8,6 +8,7 @@ https://github.com/capistrano/capistrano/compare/v3.7.2...HEAD
 
 * [#1835](https://github.com/capistrano/capistrano/pull/1835): Stopped printing parenthesis in ask prompt if no default or nil was passed as argument [(@chamini2)](https://github.com/chamini2)
 * [#1840](https://github.com/capistrano/capistrano/pull/1840): Git plugin: shellescape git_wrapper_path [(@olleolleolle)](https://github.com/olleolleolle)
+* [#1843](https://github.com/capistrano/capistrano/pull/1843): Properly shell escape git:wrapper steps - [@mattbrictson](https://github.com/mattbrictson)
 * Your contribution here!
 
 ## `3.7.2` (2017-01-27)

features/step_definitions/assertions.rb

@@ -1,10 +1,12 @@
+require "shellwords"
+
 Then(/^references in the remote repo are listed$/) do
   expect(@output).to include("refs/heads/master")
 end
 
 Then(/^git wrapper permissions are 0700$/) do
-  permissions_test = %Q([ $(stat -c "%a" #{TestApp.git_wrapper_path}) == "700" ])
-  expect(vagrant_cli_command("ssh -c '#{permissions_test}'")).to be_success
+  permissions_test = %Q([ $(stat -c "%a" #{TestApp.git_wrapper_path.shellescape}) == "700" ])
+  expect(vagrant_cli_command("ssh -c #{permissions_test.shellescape}")).to be_success
 end
 
 Then(/^the shared path is created$/) do

lib/capistrano/scm/git.rb

@@ -7,7 +7,7 @@ class Capistrano::SCM::Git < Capistrano::SCM::Plugin
     set_if_empty :git_wrapper_path, lambda {
       # Try to avoid permissions issues when multiple users deploy the same app
       # by using different file names in the same dir for each deployer and stage.
-      suffix = [:application, :stage, :local_user].map { |key| fetch(key).to_s }.join("-").shellescape
+      suffix = [:application, :stage, :local_user].map { |key| fetch(key).to_s }.join("-")
       "#{fetch(:tmp_dir)}/git-ssh-#{suffix}.sh"
     }
     set_if_empty :git_environmental_variables, lambda {

lib/capistrano/scm/tasks/git.rake

@@ -5,9 +5,9 @@ namespace :git do
   desc "Upload the git wrapper script, this script guarantees that we can script git without getting an interactive prompt"
   task :wrapper do
     on release_roles :all do
-      execute :mkdir, "-p", File.dirname(fetch(:git_wrapper_path))
+      execute :mkdir, "-p", File.dirname(fetch(:git_wrapper_path)).shellescape
       upload! StringIO.new("#!/bin/sh -e\nexec /usr/bin/ssh -o PasswordAuthentication=no -o StrictHostKeyChecking=no \"$@\"\n"), fetch(:git_wrapper_path)
-      execute :chmod, "700", fetch(:git_wrapper_path)
+      execute :chmod, "700", fetch(:git_wrapper_path).shellescape
     end
   end
 

spec/lib/capistrano/scm/git_spec.rb

@@ -28,13 +28,13 @@ module Capistrano
     end
 
     describe "#set_defaults" do
-      it "sets ::git_wrapper_path in a shell-safe way" do
+      it "makes git_wrapper_path using application, stage, and local_user" do
         env.set(:tmp_dir, "/tmp")
         env.set(:application, "my_app")
         env.set(:stage, "staging")
         env.set(:local_user, "(Git Web User) via ShipIt")
         subject.set_defaults
-        expect(env.fetch(:git_wrapper_path)).to eq('/tmp/git-ssh-my_app-staging-\(Git\ Web\ User\)\ via\ ShipIt.sh')
+        expect(env.fetch(:git_wrapper_path)).to eq("/tmp/git-ssh-my_app-staging-(Git Web User) via ShipIt.sh")
       end
     end
 

spec/support/test_app.rb

@@ -19,6 +19,7 @@ module TestApp
       set :linked_files, #{linked_files}
       set :linked_dirs, #{linked_dirs}
       set :format_options, log_file: nil
+      set :local_user, #{current_user.inspect}
     CONFIG
   end
 
@@ -154,7 +155,7 @@ module TestApp
   end
 
   def current_user
-    `whoami`.chomp
+    "(GitHub Web Flow) via ShipIt"
   end
 
   def task_dir