Merge branch 'aevri/win32_tempfilename' into 'master'

_artifact: use win32-compatible named temp file

See merge request BuildStream/buildstream!1440

2 files changed, 65 insertions, 6 deletions

diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index 02adb3404..ffe92db16 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -29,7 +29,6 @@ artifact composite interaction away from Element class
 """
 
 import os
-import tempfile
 
 from ._protos.buildstream.v2.artifact_pb2 import Artifact as ArtifactProto
 from . import _yaml
@@ -148,9 +147,9 @@ class Artifact():
             size += filesvdir.get_size()
 
         # Store public data
-        with tempfile.NamedTemporaryFile(dir=self._tmpdir) as tmp:
-            _yaml.dump(publicdata, tmp.name)
-            public_data_digest = self._cas.add_object(path=tmp.name, link_directly=True)
+        with utils._tempnamedfile_name(dir=self._tmpdir) as tmpname:
+            _yaml.dump(publicdata, tmpname)
+            public_data_digest = self._cas.add_object(path=tmpname, link_directly=True)
             artifact.public_data.CopyFrom(public_data_digest)
             size += public_data_digest.size_bytes
 
diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py
index 0e2cdeb43..03ff67bf2 100644
--- a/src/buildstream/utils.py
+++ b/src/buildstream/utils.py
@@ -998,7 +998,7 @@ def _tempdir(suffix="", prefix="tmp", dir=None):  # pylint: disable=redefined-bu
 #    prefix (str): A prefix for the temporary file name
 #
 # Yields:
-#    (str): The temporary file handle
+#    (tempfile.NamedTemporaryFile): The temporary file handle
 #
 # Do not use tempfile.NamedTemporaryFile() directly, as this will
 # leak files on the filesystem when BuildStream exits a process
@@ -1013,10 +1013,70 @@ def _tempnamedfile(suffix="", prefix="tmp", dir=None):  # pylint: disable=redefi
             temp.close()
 
     with _signals.terminator(close_tempfile), \
-        tempfile.NamedTemporaryFile(suffix=suffix, prefix=prefix, dir=dir) as temp:
+            tempfile.NamedTemporaryFile(suffix=suffix, prefix=prefix, dir=dir) as temp:
         yield temp
 
 
+# _tempnamedfile_name()
+#
+# A context manager for doing work on a temporary file, via the filename only.
+#
+# Note that a Windows restriction prevents us from using both the file
+# descriptor and the file name of tempfile.NamedTemporaryFile, the same file
+# cannot be opened twice at the same time. This wrapper makes it easier to
+# operate in a Windows-friendly way when only filenames are needed.
+#
+# Takes care to delete the file even if interrupted by SIGTERM. Note that there
+# is a race-condition, so this is done on a best-effort basis.
+#
+# Args:
+#    dir (str): A path to a parent directory for the temporary file, this is
+#               not optional for security reasons, please see below.
+#
+# Note that 'dir' should not be a directory that may be shared with potential
+# attackers that have rights to affect the directory, e.g. the system temp
+# directory. This is because an attacker can replace the temporary file with
+# e.g. a symlink to another location, that the BuildStream user has rights to
+# but the attacker does not.
+#
+# See here for more information:
+# https://www.owasp.org/index.php/Insecure_Temporary_File
+#
+# Yields:
+#    (str): The temporary file name
+#
+@contextmanager
+def _tempnamedfile_name(dir):  # pylint: disable=redefined-builtin
+    filename = None
+
+    def rm_tempfile():
+        nonlocal filename
+        if filename is not None:
+            # Note that this code is re-entrant - it's possible for us to
+            # "delete while we're deleting" if we are responding to SIGTERM.
+            #
+            # For simplicity, choose to leak the file in this unlikely case,
+            # rather than introducing a scheme for handling the file sometimes
+            # being missing. Note that we will still leak tempfiles on forced
+            # termination anyway.
+            #
+            # Note that we must not try to delete the file more than once. If
+            # we delete the file then another temporary file can be created
+            # with the same name.
+            #
+            filename2 = filename
+            filename = None
+            os.remove(filename2)
+
+    with _signals.terminator(rm_tempfile):
+        fd, filename = tempfile.mkstemp(dir=dir)
+        os.close(fd)
+        try:
+            yield filename
+        finally:
+            rm_tempfile()
+
+
 # _kill_process_tree()
 #
 # Brutally murder a process and all of its children