From 9c92ad51ed29243e535fdb44ec494681c040e62b Mon Sep 17 00:00:00 2001
From: Matt Brictson <matt@mattbrictson.com>
Date: Thu, 31 Dec 2020 10:50:39 -0800
Subject: [PATCH] Security: randomize path of git wrapper script

Before, the `:git_wrapper_path` was a somewhat predictable value and
located in `/tmp` by default, which is world-writable. That meant that
there was a chance (albeit very small) that another process could guess
the path and overwrite it with something malicious.

Fix by randomly generating a path name so that the git wrapper script
location cannot be predicted.

This change should be transparent to capistrano users since the
`:git_wrapper_path` is only intended to be used internally. If you need
a predictable value for this path, set a custom value for
`:git_wrapper_path` in your `deploy.rb` file.
---
 features/step_definitions/assertions.rb | 2 +-
 lib/capistrano/scm/git.rb               | 8 ++++----
 spec/lib/capistrano/scm/git_spec.rb     | 7 ++-----
 spec/support/test_app.rb                | 4 ++--
 4 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/features/step_definitions/assertions.rb b/features/step_definitions/assertions.rb
index cc965e9d..4a7f6382 100644
--- a/features/step_definitions/assertions.rb
+++ b/features/step_definitions/assertions.rb
@@ -5,7 +5,7 @@ Then(/^references in the remote repo are listed$/) do
 end
 
 Then(/^git wrapper permissions are 0700$/) do
-  permissions_test = %Q([ $(stat -c "%a" #{TestApp.git_wrapper_path.shellescape}) == "700" ])
+  permissions_test = %Q([ $(stat -c "%a" #{TestApp.git_wrapper_path_glob}) == "700" ])
   _stdout, _stderr, status = vagrant_cli_command("ssh -c #{permissions_test.shellescape}")
 
   expect(status).to be_success
diff --git a/lib/capistrano/scm/git.rb b/lib/capistrano/scm/git.rb
index 97ca9d01..9a95cb8c 100644
--- a/lib/capistrano/scm/git.rb
+++ b/lib/capistrano/scm/git.rb
@@ -1,5 +1,6 @@
 require "capistrano/scm/plugin"
 require "cgi"
+require "securerandom"
 require "shellwords"
 require "uri"
 
@@ -7,10 +8,9 @@ class Capistrano::SCM::Git < Capistrano::SCM::Plugin
   def set_defaults
     set_if_empty :git_shallow_clone, false
     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 = %i(application stage local_user).map { |key| fetch(key).to_s }.join("-")
-      "#{fetch(:tmp_dir)}/git-ssh-#{suffix}.sh"
+      # Use a unique name that won't collide with other deployments, and
+      # that cannot be guessed by other processes that have access to /tmp.
+      "#{fetch(:tmp_dir)}/git-ssh-#{SecureRandom.hex(10)}.sh"
     }
     set_if_empty :git_environmental_variables, lambda {
       {
diff --git a/spec/lib/capistrano/scm/git_spec.rb b/spec/lib/capistrano/scm/git_spec.rb
index 86631de8..6dbdb250 100644
--- a/spec/lib/capistrano/scm/git_spec.rb
+++ b/spec/lib/capistrano/scm/git_spec.rb
@@ -28,13 +28,10 @@ module Capistrano
     end
 
     describe "#set_defaults" do
-      it "makes git_wrapper_path using application, stage, and local_user" do
+      it "makes git_wrapper_path using a random hex value" 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 match(%r{/tmp/git-ssh-\h{20}\.sh})
       end
 
       it "makes git_max_concurrent_connections" do
diff --git a/spec/support/test_app.rb b/spec/support/test_app.rb
index 6ff6a7ce..4010c647 100644
--- a/spec/support/test_app.rb
+++ b/spec/support/test_app.rb
@@ -185,8 +185,8 @@ module TestApp
     FileUtils.mv(config_path, location)
   end
 
-  def git_wrapper_path
-    "/tmp/git-ssh-my_app_name-#{stage}-#{current_user}.sh"
+  def git_wrapper_path_glob
+    "/tmp/git-ssh-*.sh"
   end
 
   def with_clean_bundler_env(&block)