Fix permissions issue with DirectoryStore (#493)

* Set file permissions in DirectoryStorage according to umask

Fixes #325

* Use built-in open function

* Adds link to GH issue

* Adds release notes entry

Author: jrbourbeau

docs/release.rst

@@ -24,6 +24,9 @@ Upcoming Release
 * Removed support for Python 2.
   By :user:`jhamman`; :issue:`393`, :issue:`470`.
 
+* Update ``DirectoryStore`` to create files with more permissive permissions.
+  By :user:`Eduardo Gonzalez <eddienko>` and :user:`James Bourbeau <jrbourbeau>`; :issue:`493`
+
 .. _release_2.3.2:
 
 2.3.2

zarr/storage.py

@@ -32,6 +32,7 @@
 from os import scandir
 from pickle import PicklingError
 from threading import Lock, RLock
+import uuid
 
 from numcodecs.compat import ensure_bytes, ensure_contiguous_ndarray
 from numcodecs.registry import codec_registry
@@ -768,20 +769,19 @@ def __setitem__(self, key, value):
                     raise KeyError(key)
 
         # write to temporary file
-        temp_path = None
+        # note we're not using tempfile.NamedTemporaryFile to avoid restrictive file permissions
+        temp_name = file_name + '.' + uuid.uuid4().hex + '.partial'
+        temp_path = os.path.join(dir_path, temp_name)
         try:
-            with tempfile.NamedTemporaryFile(mode='wb', delete=False, dir=dir_path,
-                                             prefix=file_name + '.',
-                                             suffix='.partial') as f:
-                temp_path = f.name
+            with open(temp_path, mode='wb') as f:
                 f.write(value)
 
             # move temporary file into place
             replace(temp_path, file_path)
 
         finally:
             # clean up if temp file still exists for whatever reason
-            if temp_path is not None and os.path.exists(temp_path):  # pragma: no cover
+            if os.path.exists(temp_path):  # pragma: no cover
                 os.remove(temp_path)
 
     def __delitem__(self, key):

zarr/tests/test_storage.py

@@ -753,6 +753,14 @@ def test_filesystem_path(self):
         store['foo'] = b'bar'
         assert os.path.isdir(path)
 
+        # check correct permissions
+        # regression test for https://github.com/zarr-developers/zarr-python/issues/325
+        stat = os.stat(path)
+        mode = stat.st_mode & 0o666
+        umask = os.umask(0)
+        os.umask(umask)
+        assert mode == (0o666 & ~umask)
+
         # test behaviour with file path
         with tempfile.NamedTemporaryFile() as f:
             with pytest.raises(ValueError):