From b7c76fdeaaa1f5e5580a6b28a54ced7d99d3178d Mon Sep 17 00:00:00 2001
From: Rob Browning <rlb@defaultvalue.org>
Date: Fri, 24 Jun 2022 16:36:04 -0500
Subject: [PATCH] PackWriter: respect umask/sgid/etc. when creating pack/idx

Use atomically_replaced_file to ensure that the new packfile and index
respect the current umask, any directory sgid bit, etc., and use an
ExitStack to try to make sure all the corner cases are covered.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/git.py | 52 ++++++++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 31 deletions(-)

diff --git a/lib/bup/git.py b/lib/bup/git.py
index 9163c40..d6a745c 100644
--- a/lib/bup/git.py
+++ b/lib/bup/git.py
@@ -4,17 +4,18 @@ interact with the Git data structures.
 """
 
 from __future__ import absolute_import, print_function
-import os, sys, zlib, subprocess, struct, stat, re, tempfile, glob
+import os, sys, zlib, subprocess, struct, stat, re, glob
 from array import array
 from binascii import hexlify, unhexlify
 from collections import namedtuple
+from contextlib import ExitStack
 from itertools import islice
+from shutil import rmtree
 
 from bup import _helpers, hashsplit, path, midx, bloom, xstat
 from bup.compat import (buffer,
                         byte_int, bytes_from_byte, bytes_from_uint,
                         environ,
-                        ExitStack,
                         pending_raise,
                         reraise)
 from bup.io import path_msg
@@ -28,6 +29,7 @@ from bup.helpers import (Sha1, add_error, chunkyreader, debug1, debug2,
                          mmap_read, mmap_readwrite,
                          nullcontext_if_not,
                          progress, qprogress, stat_if_exists,
+                         temp_dir,
                          unlink,
                          utc_offset_str)
 
@@ -780,7 +782,7 @@ class PackWriter(object):
         self.parentfd = None
         self.count = 0
         self.outbytes = 0
-        self.filename = None
+        self.tmpdir = None
         self.idx = None
         self.objcache_maker = objcache_maker
         self.objcache = None
@@ -808,24 +810,15 @@ class PackWriter(object):
 
     def _open(self):
         if not self.file:
-            objdir = dir = os.path.join(self.repo_dir, b'objects')
-            fd, name = tempfile.mkstemp(suffix=b'.pack', dir=objdir)
-            try:
-                self.file = os.fdopen(fd, 'w+b')
-            except:
-                os.close(fd)
-                raise
-            try:
-                self.parentfd = os.open(objdir, os.O_RDONLY)
-            except:
-                f = self.file
-                self.file = None
-                f.close()
-                raise
-            assert name.endswith(b'.pack')
-            self.filename = name[:-5]
-            self.file.write(b'PACK\0\0\0\2\0\0\0\0')
-            self.idx = PackIdxV2Writer()
+            with ExitStack() as err_stack:
+                objdir = dir = os.path.join(self.repo_dir, b'objects')
+                self.tmpdir = err_stack.enter_context(temp_dir(dir=objdir, prefix=b'pack-tmp-'))
+                self.file = err_stack.enter_context(open(self.tmpdir + b'/pack', 'w+b'))
+                self.parentfd = err_stack.enter_context(finalized(os.open(objdir, os.O_RDONLY),
+                                                                  lambda x: os.close(x)))
+                self.file.write(b'PACK\0\0\0\2\0\0\0\0')
+                self.idx = PackIdxV2Writer()
+                err_stack.pop_all()
 
     def _raw_write(self, datalist, sha):
         self._open()
@@ -914,6 +907,7 @@ class PackWriter(object):
 
     def _end(self, run_midx=True, abort=False):
         # Ignores run_midx during abort
+        self.tmpdir, tmpdir = None, self.tmpdir
         self.parentfd, pfd, = None, self.parentfd
         self.file, f = None, self.file
         self.idx, idx = None, self.idx
@@ -921,11 +915,7 @@ class PackWriter(object):
             with nullcontext_if_not(self.objcache), \
                  finalized(pfd, lambda x: x is not None and os.close(x)), \
                  nullcontext_if_not(f):
-                if not f:
-                    return None
-
-                if abort:
-                    os.unlink(self.filename + b'.pack')
+                if abort or not f:
                     return None
 
                 # update object count
@@ -945,13 +935,11 @@ class PackWriter(object):
                 fdatasync(f.fileno())
                 f.close()
 
-                idx.write(self.filename + b'.idx', packbin)
+                idx.write(tmpdir + b'/idx', packbin)
                 nameprefix = os.path.join(self.repo_dir,
                                           b'objects/pack/pack-' +  hexlify(packbin))
-                if os.path.exists(self.filename + b'.map'):
-                    os.unlink(self.filename + b'.map')
-                os.rename(self.filename + b'.pack', nameprefix + b'.pack')
-                os.rename(self.filename + b'.idx', nameprefix + b'.idx')
+                os.rename(tmpdir + b'/pack', nameprefix + b'.pack')
+                os.rename(tmpdir + b'/idx', nameprefix + b'.idx')
                 os.fsync(pfd)
                 if run_midx:
                     auto_midx(os.path.join(self.repo_dir, b'objects/pack'))
@@ -959,6 +947,8 @@ class PackWriter(object):
                     self.on_pack_finish(nameprefix)
                 return nameprefix
         finally:
+            if tmpdir:
+                rmtree(tmpdir)
             # Must be last -- some of the code above depends on it
             self.objcache = None
 
-- 
2.39.2