caution tape on makedirs_safe (#55241)

* removed usages in winrm/psrp to be consistent with other connection plugins

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

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:

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:

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.
     '''