diff options
| -rw-r--r-- | src/buildstream/_artifact.py | 7 | ||||
| -rw-r--r-- | src/buildstream/utils.py | 64 |
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 |