diff --git a/lib/ansible/plugins/connection/__init__.py b/lib/ansible/plugins/connection/__init__.py index 5bedc6d1543..14973dd6f3a 100644 --- a/lib/ansible/plugins/connection/__init__.py +++ b/lib/ansible/plugins/connection/__init__.py @@ -207,7 +207,7 @@ class ConnectionBase(AnsiblePlugin): @ensure_connect @abstractmethod def fetch_file(self, in_path, out_path): - """Fetch a file from remote to local""" + """Fetch a file from remote to local; callers are expected to have pre-created the directory chain for out_path""" pass @abstractmethod diff --git a/lib/ansible/plugins/connection/psrp.py b/lib/ansible/plugins/connection/psrp.py index 5704153a917..f521e6db076 100644 --- a/lib/ansible/plugins/connection/psrp.py +++ b/lib/ansible/plugins/connection/psrp.py @@ -277,7 +277,6 @@ from ansible.plugins.connection import ConnectionBase from ansible.plugins.shell.powershell import _common_args from ansible.utils.display import Display from ansible.utils.hashing import secure_hash -from ansible.utils.path import makedirs_safe HAS_PYPSRP = True PYPSRP_IMP_ERR = None @@ -540,12 +539,11 @@ if ($bytes_read -gt 0) { raise AnsibleError("failed to setup file stream for fetch '%s': %s" % (out_path, to_native(stderr))) elif stdout.strip() == '[DIR]': - # in_path was a dir so we need to create the dir locally - makedirs_safe(out_path) + # to be consistent with other connection plugins, we assume the caller has created the target dir return b_out_path = to_bytes(out_path, errors='surrogate_or_strict') - makedirs_safe(os.path.dirname(b_out_path)) + # to be consistent with other connection plugins, we assume the caller has created the target dir offset = 0 with open(b_out_path, 'wb') as out_file: while True: diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 4564ef76edb..87d1d66d8eb 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -123,7 +123,6 @@ from ansible.module_utils.six import binary_type, PY3 from ansible.plugins.connection import ConnectionBase from ansible.plugins.shell.powershell import _parse_clixml from ansible.utils.hashing import secure_hash -from ansible.utils.path import makedirs_safe from ansible.utils.display import Display # getargspec is deprecated in favour of getfullargspec in Python 3 but @@ -623,9 +622,9 @@ class Connection(ConnectionBase): super(Connection, self).fetch_file(in_path, out_path) in_path = self._shell._unquote(in_path) out_path = out_path.replace('\\', '/') + # consistent with other connection plugins, we assume the caller has created the target dir display.vvv('FETCH "%s" TO "%s"' % (in_path, out_path), host=self._winrm_host) buffer_size = 2**19 # 0.5MB chunks - makedirs_safe(os.path.dirname(out_path)) out_file = None try: offset = 0 @@ -668,7 +667,6 @@ class Connection(ConnectionBase): else: data = base64.b64decode(result.std_out.strip()) if data is None: - makedirs_safe(out_path) break else: if not out_file: diff --git a/lib/ansible/utils/path.py b/lib/ansible/utils/path.py index 41ed017ef43..37dd4c9139f 100644 --- a/lib/ansible/utils/path.py +++ b/lib/ansible/utils/path.py @@ -60,11 +60,17 @@ def unfrackpath(path, follow=True, basedir=None): def makedirs_safe(path, mode=None): - '''Safe way to create dirs in muliprocess/thread environments. - - :arg path: A byte or text string representing a directory to be created + ''' + A *potentially insecure* way to ensure the existence of a directory chain. The "safe" in this function's name + refers only to its ability to ignore `EEXIST` in the case of multiple callers operating on the same part of + the directory chain. This function is not safe to use under world-writable locations when the first level of the + path to be created contains a predictable component. Always create a randomly-named element first if there is any + chance the parent directory might be world-writable (eg, /tmp) to prevent symlink hijacking and potential + disclosure or modification of sensitive file contents. + + :arg path: A byte or text string representing a directory chain to be created :kwarg mode: If given, the mode to set the directory to - :raises AnsibleError: If the directory cannot be created and does not already exists. + :raises AnsibleError: If the directory cannot be created and does not already exist. :raises UnicodeDecodeError: if the path is not decodable in the utf-8 encoding. '''