Skip to content

Commit f8f7ec4

Browse files
use a custom mkstemp with mode support, fixes borgbackup#6933, fixes borgbackup#6400
hopefully this is the final fix. after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there was a new problem that chmod was not supported on some fs. even after fixing that, there were other issues, see the ACLs issue documented in borgbackup#6933. the root cause of all this is tempfile.mkstemp internally using a very secure, but hardcoded and for our use case problematic mode of 0o600. mkstemp_mode (mosty copy&paste from python stdlib tempfile module + "black" formatting applied) supports giving the mode via the api, that is the only change needed. slightly dirty due to the _xxx imports from tempfile, but hopefully this will be supported in some future python version.
1 parent 99c72c5 commit f8f7ec4

File tree

2 files changed

+71
-14
lines changed

2 files changed

+71
-14
lines changed

src/borg/helpers/fs.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,3 +418,72 @@ def umount(mountpoint):
418418
return subprocess.call(["fusermount", "-u", mountpoint], env=env)
419419
except FileNotFoundError:
420420
return subprocess.call(["umount", mountpoint], env=env)
421+
422+
423+
# below is a slightly modified tempfile.mkstemp that has an additional mode parameter.
424+
# see https://github.com/borgbackup/borg/issues/6933 and https://github.com/borgbackup/borg/issues/6400
425+
426+
import os as _os
427+
import sys as _sys
428+
import errno as _errno
429+
from tempfile import _sanitize_params, _get_candidate_names, TMP_MAX, _text_openflags, _bin_openflags
430+
431+
432+
def _mkstemp_inner(dir, pre, suf, flags, output_type, mode=0o600):
433+
"""Code common to mkstemp, TemporaryFile, and NamedTemporaryFile."""
434+
435+
dir = _os.path.abspath(dir)
436+
names = _get_candidate_names()
437+
if output_type is bytes:
438+
names = map(_os.fsencode, names)
439+
440+
for seq in range(TMP_MAX):
441+
name = next(names)
442+
file = _os.path.join(dir, pre + name + suf)
443+
_sys.audit("tempfile.mkstemp", file)
444+
try:
445+
fd = _os.open(file, flags, mode)
446+
except FileExistsError:
447+
continue # try again
448+
except PermissionError:
449+
# This exception is thrown when a directory with the chosen name
450+
# already exists on windows.
451+
if _os.name == "nt" and _os.path.isdir(dir) and _os.access(dir, _os.W_OK):
452+
continue
453+
else:
454+
raise
455+
return fd, file
456+
457+
raise FileExistsError(_errno.EEXIST, "No usable temporary file name found")
458+
459+
460+
def mkstemp_mode(suffix=None, prefix=None, dir=None, text=False, mode=0o600):
461+
"""User-callable function to create and return a unique temporary
462+
file. The return value is a pair (fd, name) where fd is the
463+
file descriptor returned by os.open, and name is the filename.
464+
If 'suffix' is not None, the file name will end with that suffix,
465+
otherwise there will be no suffix.
466+
If 'prefix' is not None, the file name will begin with that prefix,
467+
otherwise a default prefix is used.
468+
If 'dir' is not None, the file will be created in that directory,
469+
otherwise a default directory is used.
470+
If 'text' is specified and true, the file is opened in text
471+
mode. Else (the default) the file is opened in binary mode.
472+
If any of 'suffix', 'prefix' and 'dir' are not None, they must be the
473+
same type. If they are bytes, the returned name will be bytes; str
474+
otherwise.
475+
The file is readable and writable only by the creating user ID.
476+
If the operating system uses permission bits to indicate whether a
477+
file is executable, the file is executable by no one. The file
478+
descriptor is not inherited by children of this process.
479+
Caller is responsible for deleting the file when done with it.
480+
"""
481+
482+
prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir)
483+
484+
if text:
485+
flags = _text_openflags
486+
else:
487+
flags = _bin_openflags
488+
489+
return _mkstemp_inner(dir, prefix, suffix, flags, output_type, mode)

src/borg/platform/base.py

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -232,8 +232,9 @@ def __init__(self, path, binary=False):
232232

233233
def __enter__(self):
234234
from .. import platform
235+
from ..helpers.fs import mkstemp_mode
235236

236-
self.tmp_fd, self.tmp_fname = tempfile.mkstemp(prefix=self.tmp_prefix, suffix=".tmp", dir=self.dir)
237+
self.tmp_fd, self.tmp_fname = mkstemp_mode(prefix=self.tmp_prefix, suffix=".tmp", dir=self.dir, mode=0o666)
237238
self.f = platform.SyncFile(self.tmp_fname, fd=self.tmp_fd, binary=self.binary)
238239
return self.f
239240

@@ -246,19 +247,6 @@ def __exit__(self, exc_type, exc_val, exc_tb):
246247
safe_unlink(self.tmp_fname) # with-body has failed, clean up tmp file
247248
return # continue processing the exception normally
248249

249-
try:
250-
# tempfile.mkstemp always uses owner-only file permissions for the temp file,
251-
# but as we'll rename it to the non-temp permanent file now, we need to respect
252-
# the umask and change the file mode to what a normally created file would have.
253-
# thanks to the crappy os.umask api, we can't query the umask without setting it. :-(
254-
umask = os.umask(UMASK_DEFAULT)
255-
os.umask(umask)
256-
os.chmod(self.tmp_fname, mode=0o666 & ~umask)
257-
except OSError:
258-
# chmod might fail if the fs does not support it.
259-
# this is not harmful, the file will still have permissions for the owner.
260-
pass
261-
262250
try:
263251
os.replace(self.tmp_fname, self.path) # POSIX: atomic rename
264252
except OSError:

0 commit comments

Comments
 (0)