Merge pull request #892 from danchr/file-perms

adjust default file mask to be consistent with git

dulwich/file.py

@@ -66,7 +66,7 @@ def _fancy_rename(oldname, newname):
     os.remove(tmpfile)
 
 
-def GitFile(filename, mode="rb", bufsize=-1):
+def GitFile(filename, mode="rb", bufsize=-1, mask=0o644):
     """Create a file object that obeys the git file locking protocol.
 
     Returns: a builtin file object or a _GitFile object
@@ -77,6 +77,10 @@ def GitFile(filename, mode="rb", bufsize=-1):
     are not.  To read and write from the same file, you can take advantage of
     the fact that opening a file for write does not actually open the file you
     request.
+
+    The default file mask makes any created files user-writable and
+    world-readable.
+
     """
     if "a" in mode:
         raise IOError("append mode not supported for Git files")
@@ -85,7 +89,7 @@ def GitFile(filename, mode="rb", bufsize=-1):
     if "b" not in mode:
         raise IOError("text mode not supported for Git files")
     if "w" in mode:
-        return _GitFile(filename, mode, bufsize)
+        return _GitFile(filename, mode, bufsize, mask)
     else:
         return io.open(filename, mode, bufsize)
 
@@ -136,7 +140,7 @@ class _GitFile(object):
         "writelines",
     )
 
-    def __init__(self, filename, mode, bufsize):
+    def __init__(self, filename, mode, bufsize, mask):
         self._filename = filename
         if isinstance(self._filename, bytes):
             self._lockfilename = self._filename + b".lock"
@@ -146,6 +150,7 @@ class _GitFile(object):
             fd = os.open(
                 self._lockfilename,
                 os.O_RDWR | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0),
+                mask,
             )
         except FileExistsError:
             raise FileLocked(filename, self._lockfilename)

dulwich/object_store.py

@@ -70,6 +70,8 @@ from dulwich.refs import ANNOTATED_TAG_SUFFIX
 INFODIR = "info"
 PACKDIR = "pack"
 
+PACK_MODE = 0o444
+
 
 class BaseObjectStore(object):
     """Object store interface."""
@@ -805,7 +807,7 @@ class DiskObjectStore(PackBasedObjectStore):
         os.rename(path, target_pack)
 
         # Write the index.
-        index_file = GitFile(pack_base_name + ".idx", "wb")
+        index_file = GitFile(pack_base_name + ".idx", "wb", mask=PACK_MODE)
         try:
             write_pack_index_v2(index_file, entries, pack_sha)
             index_file.close()
@@ -837,6 +839,7 @@ class DiskObjectStore(PackBasedObjectStore):
 
         fd, path = tempfile.mkstemp(dir=self.path, prefix="tmp_pack_")
         with os.fdopen(fd, "w+b") as f:
+            os.chmod(path, PACK_MODE)
             indexer = PackIndexer(f, resolve_ext_ref=self.get_raw)
             copier = PackStreamCopier(read_all, read_some, f, delta_iter=indexer)
             copier.verify()
@@ -856,7 +859,7 @@ class DiskObjectStore(PackBasedObjectStore):
             basename = self._get_pack_basepath(entries)
             index_name = basename + ".idx"
             if not os.path.exists(index_name):
-                with GitFile(index_name, "wb") as f:
+                with GitFile(index_name, "wb", mask=PACK_MODE) as f:
                     write_pack_index_v2(f, entries, p.get_stored_checksum())
         for pack in self.packs:
             if pack._basename == basename:
@@ -885,6 +888,7 @@ class DiskObjectStore(PackBasedObjectStore):
 
         fd, path = tempfile.mkstemp(dir=self.pack_dir, suffix=".pack")
         f = os.fdopen(fd, "wb")
+        os.chmod(path, PACK_MODE)
 
         def commit():
             f.flush()
@@ -916,7 +920,7 @@ class DiskObjectStore(PackBasedObjectStore):
             pass
         if os.path.exists(path):
             return  # Already there, no need to write again
-        with GitFile(path, "wb") as f:
+        with GitFile(path, "wb", mask=PACK_MODE) as f:
             f.write(
                 obj.as_legacy_object(compression_level=self.loose_compression_level)
             )

dulwich/tests/test_object_store.py

@@ -438,6 +438,13 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
         for alt_path in store._read_alternate_paths():
             self.assertNotIn("#", alt_path)
 
+    def test_file_modes(self):
+        self.store.add_object(testobject)
+        path = self.store._get_shafile_path(testobject.id)
+        mode = os.stat(path).st_mode
+
+        self.assertEqual(oct(mode), "0o100444")
+
     def test_corrupted_object_raise_exception(self):
         """Corrupted sha1 disk file should raise specific exception"""
         self.store.add_object(testobject)
@@ -448,8 +455,11 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
         self.assertIsNotNone(self.store._get_loose_object(testobject.id))
 
         path = self.store._get_shafile_path(testobject.id)
+        old_mode = os.stat(path).st_mode
+        os.chmod(path, 0o600)
         with open(path, "wb") as f:  # corrupt the file
             f.write(b"")
+        os.chmod(path, old_mode)
 
         expected_error_msg = "Corrupted empty file detected"
         try:

dulwich/tests/test_pack.py

@@ -338,6 +338,7 @@ class TestPackData(PackTests):
             p.create_index_v1(filename)
             idx1 = load_pack_index(filename)
             idx2 = self.get_pack_index(pack1_sha)
+            self.assertEqual(oct(os.stat(filename).st_mode), "0o100644")
             self.assertEqual(idx1, idx2)
 
     def test_create_index_v2(self):
@@ -346,6 +347,7 @@ class TestPackData(PackTests):
             p.create_index_v2(filename)
             idx1 = load_pack_index(filename)
             idx2 = self.get_pack_index(pack1_sha)
+            self.assertEqual(oct(os.stat(filename).st_mode), "0o100644")
             self.assertEqual(idx1, idx2)
 
     def test_compute_file_sha(self):

dulwich/tests/test_repository.py

@@ -21,10 +21,11 @@
 
 """Tests for the repository."""
 
+import glob
 import locale
 import os
-import stat
 import shutil
+import stat
 import sys
 import tempfile
 import warnings
@@ -80,6 +81,21 @@ class CreateRepositoryTests(TestCase):
             config_text = f.read()
             self.assertTrue(barestr in config_text, "%r" % config_text)
 
+        if isinstance(repo, Repo):
+            expected_mode = '0o100644' if expect_filemode else '0o100666'
+            expected = {
+                'HEAD': expected_mode,
+                'config': expected_mode,
+                'description': expected_mode,
+            }
+            actual = {
+                f[len(repo._controldir) + 1:]: oct(os.stat(f).st_mode)
+                for f in glob.glob(os.path.join(repo._controldir, '*'))
+                if os.path.isfile(f)
+            }
+
+            self.assertEqual(expected, actual)
+
     def test_create_memory(self):
         repo = MemoryRepo.init_bare([], {})
         self._check_repo_contents(repo, True)

dulwich/tests/utils.py

@@ -24,6 +24,7 @@
 import datetime
 import os
 import shutil
+import stat
 import tempfile
 import time
 import types
@@ -82,11 +83,20 @@ def open_repo(name, temp_dir=None):
     return Repo(temp_repo_dir)
 
 
+def safe_rmtree(path):
+    """Version of shutil.rmtree() that handles unwritable files"""
+    def really_delete(action, name, exc):
+        os.chmod(name, stat.S_IWRITE)
+        os.remove(name)
+
+    shutil.rmtree(path, onerror=really_delete)
+
+
 def tear_down_repo(repo):
     """Tear down a test repository."""
     repo.close()
     temp_dir = os.path.dirname(repo.path.rstrip(os.sep))
-    shutil.rmtree(temp_dir)
+    safe_rmtree(temp_dir)
 
 
 def make_object(cls, **attrs):