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)