Merge pull request #14488 from bcoca/become_shell_fixes

shell + become fixes

lib/ansible/playbook/play_context.py

@@ -464,8 +464,7 @@ class PlayContext(Base):
                     return bool(SU_PROMPT_LOCALIZATIONS_RE.match(data))
                 prompt = detect_su_prompt
 
-                su_success_cmd =  '%s -c %s' % (executable, success_cmd) # this is here cause su too succeptible to overquoting
-                becomecmd = '%s %s %s -c %s' % (exe, flags, self.become_user, su_success_cmd) #works with sh
+                becomecmd = '%s %s %s -c %s' % (exe, flags, self.become_user, pipes.quote('%s -c %s' % (executable, success_cmd)))
 
             elif self.become_method == 'pbrun':
 
@@ -479,7 +478,7 @@ class PlayContext(Base):
 
             elif self.become_method == 'runas':
                 raise AnsibleError("'runas' is not yet implemented")
-                #TODO: figure out prompt
+                #FIXME: figure out prompt
                 # this is not for use with winrm plugin but if they ever get ssh native on windoez
                 becomecmd = '%s %s /user:%s "%s"' % (exe, flags, self.become_user, success_cmd)
 
@@ -494,6 +493,7 @@ class PlayContext(Base):
                 if self.become_user:
                     flags += ' -u %s ' % self.become_user
 
+                #FIXME: make shell independant
                 becomecmd = '%s %s echo %s && %s %s env ANSIBLE=true %s' % (exe, flags, success_key, exe, flags, cmd)
 
             else:
@@ -502,7 +502,7 @@ class PlayContext(Base):
             if self.become_pass:
                 self.prompt = prompt
             self.success_key = success_key
-            return ('%s -c %s' % (executable, pipes.quote(becomecmd)))
+            return becomecmd
 
         return cmd
 

lib/ansible/plugins/action/__init__.py

@@ -507,9 +507,6 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             replacement strategy (python3 could use surrogateescape)
         '''
 
-        if executable is not None and self._connection.allow_executable:
-            cmd = executable + ' -c ' + pipes.quote(cmd)
-
         display.debug("_low_level_execute_command(): starting")
         if not cmd:
             # this can happen with powershell modules when there is no analog to a Windows command (like chmod)
@@ -522,6 +519,9 @@ class ActionBase(with_metaclass(ABCMeta, object)):
             display.debug("_low_level_execute_command(): using become for this command")
             cmd = self._play_context.make_become_cmd(cmd, executable=executable)
 
+        if executable is not None and self._connection.allow_executable:
+            cmd = executable + ' -c ' + pipes.quote(cmd)
+
         display.debug("_low_level_execute_command(): executing: %s" % (cmd,))
         rc, stdout, stderr = self._connection.exec_command(cmd, in_data=in_data, sudoable=sudoable)
 

test/units/playbook/test_play_context.py

@@ -140,28 +140,28 @@ class TestPlayContext(unittest.TestCase):
 
         play_context.become_method = 'sudo'
         cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash")
-        self.assertEqual(cmd, """%s -c '%s %s -u %s %s -c '"'"'echo %s; %s'"'"''""" % (default_exe, sudo_exe, sudo_flags, play_context.become_user, default_exe, play_context.success_key, default_cmd))
+        self.assertEqual(cmd, """%s %s -u %s %s -c 'echo %s; %s'""" % (sudo_exe, sudo_flags, play_context.become_user, default_exe, play_context.success_key, default_cmd))
         play_context.become_pass = 'testpass'
         cmd = play_context.make_become_cmd(cmd=default_cmd, executable=default_exe)
-        self.assertEqual(cmd, """%s -c '%s %s -p "%s" -u %s %s -c '"'"'echo %s; %s'"'"''""" % (default_exe, sudo_exe, sudo_flags.replace('-n',''), play_context.prompt, play_context.become_user, default_exe, play_context.success_key, default_cmd))
+        self.assertEqual(cmd, """%s %s -p "%s" -u %s %s -c 'echo %s; %s'""" % (sudo_exe, sudo_flags.replace('-n',''), play_context.prompt, play_context.become_user, default_exe, play_context.success_key, default_cmd))
 
         play_context.become_pass = None
 
         play_context.become_method = 'su'
         cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash")
-        self.assertEqual(cmd, """%s -c '%s  %s -c %s -c '"'"'echo %s; %s'"'"''""" % (default_exe, su_exe, play_context.become_user, default_exe, play_context.success_key, default_cmd))
+        self.assertEqual(cmd, """%s  %s -c '%s -c '"'"'echo %s; %s'"'"''""" % (su_exe, play_context.become_user, default_exe, play_context.success_key, default_cmd))
 
         play_context.become_method = 'pbrun'
         cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash")
-        self.assertEqual(cmd, """%s -c '%s -b %s -u %s '"'"'echo %s; %s'"'"''""" % (default_exe, pbrun_exe, pbrun_flags, play_context.become_user, play_context.success_key, default_cmd))
+        self.assertEqual(cmd, """%s -b %s -u %s 'echo %s; %s'""" % (pbrun_exe, pbrun_flags, play_context.become_user, play_context.success_key, default_cmd))
 
         play_context.become_method = 'pfexec'
         cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash")
-        self.assertEqual(cmd, """%s -c '%s %s "'"'"'echo %s; %s'"'"'"'""" % (default_exe, pfexec_exe, pfexec_flags, play_context.success_key, default_cmd))
+        self.assertEqual(cmd, '''%s %s "'echo %s; %s'"''' % (pfexec_exe, pfexec_flags, play_context.success_key, default_cmd))
 
         play_context.become_method = 'doas'
         cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash")
-        self.assertEqual(cmd, """%s -c '%s %s echo %s && %s %s env ANSIBLE=true %s'""" % (default_exe, doas_exe, doas_flags, play_context.success_key, doas_exe, doas_flags, default_cmd))
+        self.assertEqual(cmd, """%s %s echo %s && %s %s env ANSIBLE=true %s""" % (doas_exe, doas_flags, play_context.success_key, doas_exe, doas_flags, default_cmd))
 
         play_context.become_method = 'bad'
         self.assertRaises(AnsibleError, play_context.make_become_cmd, cmd=default_cmd, executable="/bin/bash")

test/units/plugins/action/test_action.py

@@ -49,7 +49,7 @@ class TestActionBase(unittest.TestCase):
 
         play_context.remote_user = 'apo'
         action_base._low_level_execute_command('ECHO', sudoable=True)
-        play_context.make_become_cmd.assert_called_once_with("/bin/sh -c ECHO", executable='/bin/sh')
+        play_context.make_become_cmd.assert_called_once_with("ECHO", executable='/bin/sh')
 
         play_context.make_become_cmd.reset_mock()
 
@@ -58,6 +58,6 @@ class TestActionBase(unittest.TestCase):
         try:
             play_context.remote_user = 'root'
             action_base._low_level_execute_command('ECHO SAME', sudoable=True)
-            play_context.make_become_cmd.assert_called_once_with("/bin/sh -c 'ECHO SAME'", executable='/bin/sh')
+            play_context.make_become_cmd.assert_called_once_with("ECHO SAME", executable='/bin/sh')
         finally:
             C.BECOME_ALLOW_SAME_USER = become_allow_same_user