Repository: asdf-format/asdf

Issue:
asdf produces files with file permissions that differ from default files
```python
with open('test', 'wb') as f:
    pass
```
produces a file with
`-rw-r--r--`

```python
af = asdf.AsdfFile({})
af.write_to('test.asdf')
```
produces a file with
`-rw-------`

Discussion:

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index b81f09d9..4deda47b 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -34,6 +34,7 @@ The ASDF Standard is at v1.6.0
 - Deprecate AsdfExtenion, AsdfExtensionList, BuiltinExtension [#1429]
 - Add AsdfDeprecationWarning to asdf_extensions entry point [#1361]
 - Deprecate asdf.tests.helpers [#1440]
+- respect umask when determining file permissions for written files [#1451]
 
 2.14.3 (2022-12-15)
 -------------------
diff --git a/asdf/generic_io.py b/asdf/generic_io.py
index 251b6442..2597d975 100644
--- a/asdf/generic_io.py
+++ b/asdf/generic_io.py
@@ -27,6 +27,10 @@ from .util import patched_urllib_parse
 __all__ = ["get_file", "get_uri", "resolve_uri", "relative_uri"]
 
 
+_FILE_PERMISSIONS_DEFAULT_UMASK = 0o22
+_FILE_PERMISSIONS_ALL = 0o777
+_FILE_PERMISSIONS_NO_EXECUTE = 0o666
+
 _local_file_schemes = ["", "file"]
 if sys.platform.startswith("win"):  # pragma: no cover
     import string
@@ -399,6 +403,7 @@ class GenericFile(metaclass=util.InheritDocstrings):
         """
         if self._close:
             self._fd.close()
+            self._fix_permissions()
 
     def truncate(self, size=None):
         """
@@ -816,10 +821,42 @@ class RealFile(RandomAccessFile):
     def read_into_array(self, size):
         return np.fromfile(self._fd, dtype=np.uint8, count=size)
 
+    def _fix_permissions(self):
+        """
+        atomicfile internally uses tempfile.NamedTemporaryFile
+        which uses tempfile.mkstemp which makes a file that is
+
+        "readable and writable only by the creating user ID."
+
+        this creates files with mode 0o600 regardless of umask
+        Rather than modify atomicfile, this will use
+        the umask to determine the file permissions and modify
+        the resulting file permission bits
+        """
+        if isinstance(self._fd, atomicfile._AtomicWFile):
+            fn = self._fd._filename
+            if not os.path.exists(fn):
+                return
+            # there is no way to read the umask without setting it
+            # so set it to the typical default 0o22
+            umask = os.umask(_FILE_PERMISSIONS_DEFAULT_UMASK)
+            if umask != _FILE_PERMISSIONS_DEFAULT_UMASK:
+                # restore the read value if it differs from the default
+                os.umask(umask)
+            permissions = _FILE_PERMISSIONS_ALL if os.path.isdir(fn) else _FILE_PERMISSIONS_NO_EXECUTE
+            os.chmod(self._fd._filename, permissions & ~umask)
+
+    def __exit__(self, type_, value, traceback):
+        super().__exit__(type_, value, traceback)
+        if self._close:
+            self._fix_permissions()
+
     def close(self):
         self.flush_memmap()
         super().close()
         self.close_memmap()
+        if self._close:
+            self._fix_permissions()
 
 
 class MemoryIO(RandomAccessFile):


Test patch:
diff --git a/asdf/tests/test_generic_io.py b/asdf/tests/test_generic_io.py
index 4c1c2234..255c6bf0 100644
--- a/asdf/tests/test_generic_io.py
+++ b/asdf/tests/test_generic_io.py
@@ -1,6 +1,7 @@
 import io
 import os
 import re
+import stat
 import sys
 import urllib.request as urllib_request
 
@@ -821,3 +822,19 @@ def test_fsspec_http(httpserver):
         gf = generic_io.get_file(f)
         r = gf.read(len(ref))
         assert r == ref, (r, ref)
+
+
+@pytest.mark.skipif(
+    sys.platform.startswith("win"),
+    reason="Cannot test write file permissions on windows",
+)
+@pytest.mark.parametrize("umask", range(512))
+def test_write_file_permissions(tmp_path, umask):
+    previous_umask = os.umask(umask)
+    fn = tmp_path / "foo"
+    with generic_io.get_file(fn, mode="w"):
+        pass
+    permissions = os.stat(fn)[stat.ST_MODE] & 0o777
+    os.umask(previous_umask)
+    target_permissions = generic_io._FILE_PERMISSIONS_NO_EXECUTE & ~umask
+    assert permissions == target_permissions