From 5fa26ba750998b406e1744bb8b281a50dbebb4aa Mon Sep 17 00:00:00 2001 From: Michael Grosser Date: Mon, 18 Feb 2019 16:56:49 -0800 Subject: [PATCH 1/3] fix and unify shell escaping for user/group/directory --- lib/sshkit/command.rb | 7 ++++--- test/unit/test_command.rb | 29 ++++++++++++++++++++++------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/lib/sshkit/command.rb b/lib/sshkit/command.rb index 62557f7c..99236f75 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] - "cd #{options[:in]} && #{yield}" + "cd #{options[:in].shellescape} && #{yield}" end def environment_hash @@ -167,7 +168,7 @@ 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) @@ -182,7 +183,7 @@ def umask(&_block) 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]} < Date: Tue, 19 Feb 2019 21:38:50 -0800 Subject: [PATCH 2/3] avoid duplicate calls to env methods --- CHANGELOG.md | 1 + lib/sshkit/backends/abstract.rb | 14 ++++++++------ lib/sshkit/command.rb | 8 +++++--- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa3c2558..d607c934 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ appear at the top. ## [Unreleased][] * Your contribution here! + * [#453](https://github.com/capistrano/sshkit/pull/453): quotes/special characters no loner break commands when using user/group/directory - [@grosser](https://github.com/grosser) ## [1.18.2][] (2019-02-03) diff --git a/lib/sshkit/backends/abstract.rb b/lib/sshkit/backends/abstract.rb index 79d32235..ba368a65 100644 --- a/lib/sshkit/backends/abstract.rb +++ b/lib/sshkit/backends/abstract.rb @@ -1,3 +1,5 @@ +require 'shellwords' + module SSHKit module Backend @@ -81,12 +83,12 @@ def execute(*args) def within(directory, &_block) (@pwd ||= []).push directory.to_s execute <<-EOTEST, verbosity: Logger::DEBUG - if test ! -d #{File.join(@pwd)} - then echo "Directory does not exist '#{File.join(@pwd)}'" 1>&2 + if test ! -d #{File.join(@pwd).shellescape} + then echo "Directory does not exist '#{File.join(@pwd).shellescape}'" 1>&2 false fi - EOTEST - yield + EOTEST + yield ensure @pwd.pop end @@ -108,8 +110,8 @@ def as(who, &_block) @group = nil end execute <<-EOTEST, verbosity: Logger::DEBUG - if ! sudo -u #{@user} whoami > /dev/null - then echo "You cannot switch to user '#{@user}' using sudo, please check the sudoers file" 1>&2 + if ! sudo -u #{@user.to_s.shellescape} whoami > /dev/null + then echo "You cannot switch to user '#{@user.to_s.shellescape}' using sudo, please check the sudoers file" 1>&2 false fi EOTEST diff --git a/lib/sshkit/command.rb b/lib/sshkit/command.rb index 99236f75..f57d8d79 100644 --- a/lib/sshkit/command.rb +++ b/lib/sshkit/command.rb @@ -162,13 +162,15 @@ def environment_string end def with(&_block) - return yield unless environment_hash.any? - "( export #{environment_string} ; #{yield} )" + env_string = environment_string + return yield if env_string.empty? + "( export #{env_string} ; #{yield} )" end def user(&_block) return yield unless options[:user] - "sudo -u #{options[:user].to_s.shellescape} #{environment_string + " " unless environment_string.empty?}-- sh -c #{yield.shellescape}" + env_string = environment_string + "sudo -u #{options[:user].to_s.shellescape} #{env_string + " " unless env_string.empty?}-- sh -c #{yield.shellescape}" end def in_background(&_block) From dfe81d69a13df109376ff3fe3239df374fb96de9 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Thu, 7 Mar 2019 18:20:58 -0800 Subject: [PATCH 3/3] Improve CHANGELOG description --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d607c934..1f9e51b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ appear at the top. ## [Unreleased][] * Your contribution here! - * [#453](https://github.com/capistrano/sshkit/pull/453): quotes/special characters no loner break commands when using user/group/directory - [@grosser](https://github.com/grosser) + * [#455](https://github.com/capistrano/sshkit/pull/455): Ensure UUID of commands are stable in logging - [@lazyatom](https://github.com/lazyatom) + * [#453](https://github.com/capistrano/sshkit/pull/453): `as` and `within` now properly escape their user/group/path arguments, and the command nested within an `as` block is now properly escaped before passing to `sh -c`. In the unlikely case that you were manually escaping commands passed to SSHKit as a workaround, you will no longer need to do this. See [#458](https://github.com/capistrano/sshkit/issues/458) for examples of what has been fixed. - [@grosser](https://github.com/grosser) ## [1.18.2][] (2019-02-03)