From 6dc910c13a944c45303f6993db7056547208135c Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 9 Feb 2016 19:05:41 -0500 Subject: [PATCH] shell + become fixes 1 less level of shell + quoting simplified become commands, less quote and shell --- lib/ansible/playbook/play_context.py | 8 ++++---- lib/ansible/plugins/action/__init__.py | 6 +++--- test/units/playbook/test_play_context.py | 12 ++++++------ test/units/plugins/action/test_action.py | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index d465789d78f..a7c333a5520 100644 --- a/lib/ansible/playbook/play_context.py +++ b/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 diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 9a149895a36..dfa44645927 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/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) diff --git a/test/units/playbook/test_play_context.py b/test/units/playbook/test_play_context.py index cc9441dab87..2e2c2238cc2 100644 --- a/test/units/playbook/test_play_context.py +++ b/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") diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index afb5d767e10..401d1363e3e 100644 --- a/test/units/plugins/action/test_action.py +++ b/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