Skip to content

Commit

Permalink
support commands/users/groups with spaces or other strange characters
Browse files Browse the repository at this point in the history
  • Loading branch information
@grosser
grosser committed Feb 18, 2019
1 parent baff92c commit 3bc7ade
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
18 changes: 10 additions & 8 deletions lib/sshkit/command.rb
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'digest/sha1'
require 'securerandom'
require 'shellwords'

# @author Lee Hambley
module SSHKit
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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]} <<EOC \\\"%s\\\" EOC" % %Q{#{yield}}
end
Expand Down Expand Up @@ -213,7 +213,9 @@ def with_redaction

def to_s
if should_map?
[SSHKit.config.command_map[command.to_sym], *Array(args)].join(' ')
arguments = Array(args)
arguments = (arguments.any? ? arguments.shelljoin : [])
[SSHKit.config.command_map[command.to_sym], *arguments].join(" ")
else
command.to_s
end
Expand Down
48 changes: 34 additions & 14 deletions test/unit/test_command.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ def test_maps_a_command
assert_equal '/usr/bin/env example', c.to_command
end

def test_maps_a_command_with_arguments
c = Command.new('example', 'hello world')
assert_equal '/usr/bin/env example hello\\ world', c.to_command
end

def test_not_mapping_a_builtin
%w{if test time}.each do |builtin|
c = Command.new(builtin)
Expand All @@ -33,60 +38,65 @@ def test_leading_and_trailing_space_is_stripped
def test_including_the_env
SSHKit.config = nil
c = Command.new(:rails, 'server', env: {rails_env: :production})
assert_equal %{( export RAILS_ENV="production" ; /usr/bin/env rails server )}, c.to_command
assert_equal %{( export RAILS_ENV=production ; /usr/bin/env rails server )}, c.to_command
end

def test_including_the_env_with_multiple_keys
SSHKit.config = nil
c = Command.new(:rails, 'server', env: {rails_env: :production, foo: 'bar'})
assert_equal %{( export RAILS_ENV="production" FOO="bar" ; /usr/bin/env rails server )}, c.to_command
assert_equal %{( export RAILS_ENV=production FOO=bar ; /usr/bin/env rails server )}, c.to_command
end

def test_including_the_env_with_string_keys
SSHKit.config = nil
c = Command.new(:rails, 'server', env: {'FACTER_env' => :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
c = Command.new(:ls, '-l', in: "/opt/sites")
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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 3bc7ade

Please sign in to comment.