From f059a74d4411319b327fdf15af09315589bb31f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20HORTA?= Date: Mon, 2 Mar 2026 18:07:52 +0100 Subject: [PATCH] Remove shell override in synchronize action plugin Setting the shell from the executable basename is technically wrong, Ansible will determine the right shell to use on its own so setting it to None is safer --- plugins/action/synchronize.py | 4 ---- .../unit/plugins/action/fixtures/synchronize/basic/meta.yaml | 3 ++- .../action/fixtures/synchronize/basic_become/meta.yaml | 3 ++- .../action/fixtures/synchronize/basic_become_cli/meta.yaml | 3 ++- .../action/fixtures/synchronize/basic_vagrant/meta.yaml | 3 ++- .../fixtures/synchronize/basic_vagrant_become_cli/meta.yaml | 3 ++- .../action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml | 3 ++- .../fixtures/synchronize/basic_with_private_key/meta.yaml | 3 ++- 8 files changed, 14 insertions(+), 11 deletions(-) diff --git a/plugins/action/synchronize.py b/plugins/action/synchronize.py index a171a2b..baeae50 100644 --- a/plugins/action/synchronize.py +++ b/plugins/action/synchronize.py @@ -17,8 +17,6 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import os.path - from ansible import constants as C from ansible.module_utils.six import string_types from ansible.module_utils.six.moves import shlex_quote @@ -301,8 +299,6 @@ class ActionModule(ActionBase): break if localhost_shell: break - else: - localhost_shell = os.path.basename(C.DEFAULT_EXECUTABLE) self._play_context.shell = localhost_shell # Unlike port, there can be only one executable diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic/meta.yaml index 28b7045..b3d9d90 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic/meta.yaml @@ -11,7 +11,8 @@ hostvars: asserts: - hasattr(SAM._connection, 'ismock') - SAM._connection.transport == 'local' - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self.execute_called - self.final_module_args['_local_rsync_path'] == 'rsync' - self.final_module_args['src'] == '/tmp/deleteme' diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic_become/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic_become/meta.yaml index 1eb0b92..4a62cbe 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic_become/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic_become/meta.yaml @@ -32,7 +32,8 @@ asserts: - self.final_module_args['dest'] == 'root@el6host:/tmp/deleteme' - self.task.become == True - self.task.become_user == None - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self._play_context.remote_addr == 'el6host' - self._play_context.remote_user == 'root' - self._play_context.become == False diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml index 1bec3b4..e03c0da 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml @@ -32,7 +32,8 @@ asserts: - self.final_module_args['dest'] == 'root@el6host:/tmp/deleteme' - self.task.become == None - self.task.become_user == None - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self._play_context.remote_addr == 'el6host' - self._play_context.remote_user == 'root' - self._play_context.become == False diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml index 574ee6a..b7e38bb 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml @@ -22,7 +22,8 @@ asserts: - self.final_module_args['dest_port'] == 2202 - self.final_module_args['src'] == '/tmp/deleteme' - self.final_module_args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme' - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self._play_context.remote_addr == '127.0.0.1' - self._play_context.remote_user == 'vagrant' - self._play_context.become == False diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml index eb0d5b1..ca8cb55 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml @@ -25,7 +25,8 @@ asserts: - self.final_module_args['dest_port'] == 2202 - self.final_module_args['src'] == '/tmp/deleteme' - self.final_module_args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme' - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self._play_context.remote_addr == '127.0.0.1' - self._play_context.remote_user == 'vagrant' - self._play_context.become == False diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml index 574ee6a..b7e38bb 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml @@ -22,7 +22,8 @@ asserts: - self.final_module_args['dest_port'] == 2202 - self.final_module_args['src'] == '/tmp/deleteme' - self.final_module_args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme' - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self._play_context.remote_addr == '127.0.0.1' - self._play_context.remote_user == 'vagrant' - self._play_context.become == False diff --git a/tests/unit/plugins/action/fixtures/synchronize/basic_with_private_key/meta.yaml b/tests/unit/plugins/action/fixtures/synchronize/basic_with_private_key/meta.yaml index 7405cb6..9a95dd5 100644 --- a/tests/unit/plugins/action/fixtures/synchronize/basic_with_private_key/meta.yaml +++ b/tests/unit/plugins/action/fixtures/synchronize/basic_with_private_key/meta.yaml @@ -18,7 +18,8 @@ task_args: asserts: - hasattr(SAM._connection, 'ismock') - SAM._connection.transport == 'local' - - self._play_context.shell == 'sh' + - self._play_context.shell == None + - self._play_context.executable == '/bin/sh' - self.execute_called - self.final_module_args['_local_rsync_path'] == 'rsync' - self.final_module_args['src'] == '/tmp/deleteme'