From 3bc7adec71f2ff482b59b79805bc0a75078303c9 Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Mon, 18 Feb 2019 15:27:46 -0800 Subject: [PATCH] support commands/users/groups with spaces or other strange characters --- CHANGELOG.md | 1 + lib/sshkit/command.rb | 18 ++++++++------- test/unit/test_command.rb | 48 +++++++++++++++++++++++++++------------ 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa3c2558..ef3efce0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ appear at the top. ## [Unreleased][] * Your contribution here! + * [#451](https://github.com/capistrano/sshkit/pull/451): support commands/users/groups/directories with spaces or other shell characters - [@grosser](https://github.com/grosser) ## [1.18.2][] (2019-02-03) diff --git a/lib/sshkit/command.rb b/lib/sshkit/command.rb index 77847a4e..77510a56 100644 --- a/lib/sshkit/command.rb +++ b/lib/sshkit/command.rb @@ -1,5 +1,6 @@ require 'digest/sha1' require 'securerandom' +require 'shellwords' # @author Lee Hambley module SSHKit @@ -145,7 +146,7 @@ def should_map? def within(&_block) return yield unless options[:in] - sprintf("cd #{options[:in]} && %s", yield) + "cd #{options[:in].shellescape} && #{yield}" end def environment_hash @@ -155,8 +156,7 @@ def environment_hash def environment_string environment_hash.collect do |key,value| key_string = key.is_a?(Symbol) ? key.to_s.upcase : key.to_s - escaped_value = value.to_s.gsub(/"/, '\"') - %{#{key_string}="#{escaped_value}"} + "#{key_string}=#{value.to_s.shellescape}" end.join(' ') end @@ -167,22 +167,22 @@ def with(&_block) def user(&_block) return yield unless options[:user] - "sudo -u #{options[:user]} #{environment_string + " " unless environment_string.empty?}-- sh -c '#{yield}'" + "sudo -u #{options[:user].to_s.shellescape} #{environment_string + " " unless environment_string.empty?}-- sh -c #{yield.shellescape}" end def in_background(&_block) return yield unless options[:run_in_background] - sprintf("( nohup %s > /dev/null & )", yield) + "( nohup #{yield} > /dev/null & )" end def umask(&_block) return yield unless SSHKit.config.umask - sprintf("umask #{SSHKit.config.umask} && %s", yield) + "umask #{SSHKit.config.umask} && #{yield}" end def group(&_block) return yield unless options[:group] - %Q(sg #{options[:group]} -c "#{yield}") + "sg #{options[:group].to_s.shellescape} -c #{yield.shellescape}" # We could also use the so-called heredoc format perhaps: #"newgrp #{options[:group]} < :production, foo: 'bar'}) - assert_equal %{( export FACTER_env="production" FOO="bar" ; /usr/bin/env rails server )}, c.to_command + assert_equal %{( export FACTER_env=production FOO=bar ; /usr/bin/env rails server )}, c.to_command end def test_double_quotes_are_escaped_in_env SSHKit.config = nil c = Command.new(:rails, 'server', env: {foo: 'asdf"hjkl'}) - assert_equal %{( export FOO="asdf\\\"hjkl" ; /usr/bin/env rails server )}, c.to_command + assert_equal %{( export FOO=asdf\\\"hjkl ; /usr/bin/env rails server )}, c.to_command end def test_percentage_symbol_handled_in_env SSHKit.config = nil c = Command.new(:rails, 'server', env: {foo: 'asdf%hjkl'}, user: "anotheruser") - assert_equal %{( export FOO="asdf%hjkl" ; sudo -u anotheruser FOO=\"asdf%hjkl\" -- sh -c '/usr/bin/env rails server' )}, c.to_command + assert_equal %{( export FOO=asdf\\%hjkl ; sudo -u anotheruser FOO=asdf\\%hjkl -- sh -c /usr/bin/env\\ rails\\ server )}, c.to_command end def test_including_the_env_doesnt_addressively_escape SSHKit.config = nil c = Command.new(:rails, 'server', env: {path: '/example:$PATH'}) - assert_equal %{( export PATH="/example:$PATH" ; /usr/bin/env rails server )}, c.to_command + assert_equal %{( export PATH=/example:\\$PATH ; /usr/bin/env rails server )}, c.to_command end def test_global_env SSHKit.config = nil SSHKit.config.default_env = { default: 'env' } c = Command.new(:rails, 'server', env: {}) - assert_equal %{( export DEFAULT="env" ; /usr/bin/env rails server )}, c.to_command + assert_equal %{( export DEFAULT=env ; /usr/bin/env rails server )}, c.to_command end def test_default_env_is_overwritten_with_locally_defined SSHKit.config.default_env = { foo: 'bar', over: 'under' } c = Command.new(:rails, 'server', env: { over: 'write'}) - assert_equal %{( export FOO="bar" OVER="write" ; /usr/bin/env rails server )}, c.to_command + assert_equal %{( export FOO=bar OVER=write ; /usr/bin/env rails server )}, c.to_command end def test_working_in_a_given_directory @@ -84,9 +89,14 @@ def test_working_in_a_given_directory assert_equal "cd /opt/sites && /usr/bin/env ls -l", c.to_command end + def test_working_in_a_given_weird_directory + c = Command.new(:ls, '-l', in: "/opt/sites and stuff") + assert_equal "cd /opt/sites\\ and\\ stuff && /usr/bin/env ls -l", c.to_command + end + def test_working_in_a_given_directory_with_env c = Command.new(:ls, '-l', in: "/opt/sites", env: {a: :b}) - assert_equal %{cd /opt/sites && ( export A="b" ; /usr/bin/env ls -l )}, c.to_command + assert_equal %{cd /opt/sites && ( export A=b ; /usr/bin/env ls -l )}, c.to_command end def test_having_a_host_passed @@ -97,17 +107,27 @@ def test_having_a_host_passed def test_working_as_a_given_user c = Command.new(:whoami, user: :anotheruser) - assert_equal "sudo -u anotheruser -- sh -c '/usr/bin/env whoami'", c.to_command + assert_equal "sudo -u anotheruser -- sh -c /usr/bin/env\\ whoami", c.to_command + end + + def test_working_as_a_given_weird_user + c = Command.new(:whoami, user: "mr space |") + assert_equal "sudo -u mr\\ space\\ \\| -- sh -c /usr/bin/env\\ whoami", c.to_command end def test_working_as_a_given_group c = Command.new(:whoami, group: :devvers) - assert_equal 'sg devvers -c "/usr/bin/env whoami"', c.to_command + assert_equal 'sg devvers -c /usr/bin/env\\ whoami', c.to_command + end + + def test_working_as_a_given_weird_group + c = Command.new(:whoami, group: "space | group") + assert_equal "sg space\\ \\|\\ group -c /usr/bin/env\\ whoami", c.to_command end def test_working_as_a_given_user_and_group c = Command.new(:whoami, user: :anotheruser, group: :devvers) - assert_equal %Q(sudo -u anotheruser -- sh -c 'sg devvers -c "/usr/bin/env whoami"'), c.to_command + assert_equal %Q(sudo -u anotheruser -- sh -c sg\\ devvers\\ -c\\ /usr/bin/env\\\\\\ whoami), c.to_command end def test_umask @@ -125,13 +145,13 @@ def test_umask_with_working_directory def test_umask_with_working_directory_and_user SSHKit.config.umask = '007' c = Command.new(:touch, 'somefile', in: '/var', user: 'alice') - assert_equal "cd /var && umask 007 && sudo -u alice -- sh -c '/usr/bin/env touch somefile'", c.to_command + assert_equal "cd /var && umask 007 && sudo -u alice -- sh -c /usr/bin/env\\ touch\\ somefile", c.to_command end def test_umask_with_env_and_working_directory_and_user SSHKit.config.umask = '007' c = Command.new(:touch, 'somefile', user: 'bob', env: {a: 'b'}, in: '/var') - assert_equal %{cd /var && umask 007 && ( export A="b" ; sudo -u bob A="b" -- sh -c '/usr/bin/env touch somefile' )}, c.to_command + assert_equal %{cd /var && umask 007 && ( export A=b ; sudo -u bob A=b -- sh -c /usr/bin/env\\ touch\\ somefile )}, c.to_command end def test_verbosity_defaults_to_logger_info