Fix escaped quotes bug in Command#group (#425)

* Fix escaped quotes bug in Command#group Command#group incorrectly escapes double quotes, resulting in a syntax error when specifying the group a command should be executed as. This issue manifested when user command quotes [changed from double quotes to single quotes](fbe1775#diff-4eaa2c55e6802df7b49a8e7a2f7584d5). This fix removes the double quote escaping in order to prevent a syntax error when executing the command. * fixup! Fix escaped quotes bug in Command#group * fixup! Fix escaped quotes bug in Command#group
capistrano · May 15, 2018 · 40995bf · 40995bf
1 parent 0b60c66
commit 40995bf
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ appear at the top.
 
 ## [Unreleased][]
 
+  * [#425](https://github.com/capistrano/sshkit/pull/425): Command#group incorrectly escapes double quotes, resulting in a a syntax error when specifying the group execution using `as`. This issue manifested when user command quotes changed from double quotes to single quotes. This fix removes the double quote escaping - [@pblesi](https://github.com/pblesi).
   * Your contribution here!
 
 ## [1.16.0][] (2018-02-03)

diff --git a/lib/sshkit/command.rb b/lib/sshkit/command.rb
@@ -182,7 +182,7 @@ def umask(&_block)
 
     def group(&_block)
       return yield unless options[:group]
-      "sg #{options[:group]} -c \\\"%s\\\"" % %Q{#{yield}}
+      %Q(sg #{options[:group]} -c "#{yield}")
       # We could also use the so-called heredoc format perhaps:
       #"newgrp #{options[:group]} <<EOC \\\"%s\\\" EOC" % %Q{#{yield}}
     end

diff --git a/test/functional/backends/test_netssh.rb b/test/functional/backends/test_netssh.rb
@@ -42,6 +42,20 @@ def test_simple_netssh
         ], command_lines
       end
 
+      def test_group_netssh
+        Netssh.new(a_host) do
+          as user: :root, group: :admin do
+           execute :touch, 'restart.txt'
+          end
+        end.run
+
+        command_lines = @output.lines.select { |line| line.start_with?('Command:') }
+        assert_equal [
+          "Command: if ! sudo -u root whoami > /dev/null; then echo \"You cannot switch to user 'root' using sudo, please check the sudoers file\" 1>&2; false; fi\n",
+          "Command: sudo -u root -- sh -c 'sg admin -c \"/usr/bin/env touch restart.txt\"'\n"
+        ], command_lines
+      end
+
       def test_capture
         captured_command_result = nil
         Netssh.new(a_host) do |_host|

diff --git a/test/unit/test_command.rb b/test/unit/test_command.rb
@@ -102,12 +102,12 @@ def test_working_as_a_given_user
 
     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_user_and_group
       c = Command.new(:whoami, user: :anotheruser, group: :devvers)
-      assert_equal "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