Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support commands/users/groups/directories with spaces or other shell characters #451

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

support commands/users/groups/directories with spaces or other shell characters #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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