cascache.py: Fix file modes in checkout()

Do not copy file mode from casd object file. Do not change
non-executable mode bits if file is executable.

3 files changed, 25 insertions, 19 deletions

diff --git a/src/buildstream/_cas/cascache.py b/src/buildstream/_cas/cascache.py
index 9394ac5b2..6b2f0d1a4 100644
--- a/src/buildstream/_cas/cascache.py
+++ b/src/buildstream/_cas/cascache.py
@@ -241,21 +241,20 @@ class CASCache:
             if can_link and mtime is None:
                 utils.safe_link(self.objpath(filenode.digest), fullpath)
             else:
-                utils.safe_copy(self.objpath(filenode.digest), fullpath)
+                utils.safe_copy(self.objpath(filenode.digest), fullpath, copystat=False)
                 if mtime is not None:
                     utils._set_file_mtime(fullpath, mtime)
 
             if filenode.is_executable:
-                os.chmod(
-                    fullpath,
-                    stat.S_IRUSR
-                    | stat.S_IWUSR
-                    | stat.S_IXUSR
-                    | stat.S_IRGRP
-                    | stat.S_IXGRP
-                    | stat.S_IROTH
-                    | stat.S_IXOTH,
-                )
+                st = os.stat(fullpath)
+                mode = st.st_mode
+                if mode & stat.S_IRUSR:
+                    mode |= stat.S_IXUSR
+                if mode & stat.S_IRGRP:
+                    mode |= stat.S_IXGRP
+                if mode & stat.S_IROTH:
+                    mode |= stat.S_IXOTH
+                os.chmod(fullpath, mode)
 
         for dirnode in directory.directories:
             fullpath = os.path.join(dest, dirnode.name)
diff --git a/tests/frontend/pull.py b/tests/frontend/pull.py
index 6a5a41977..c1fa76ab0 100644
--- a/tests/frontend/pull.py
+++ b/tests/frontend/pull.py
@@ -442,6 +442,8 @@ def test_pull_access_rights(cli, tmpdir, datafiles):
     project = str(datafiles)
     checkout = os.path.join(str(tmpdir), "checkout")
 
+    umask = utils.get_umask()
+
     # Work-around datafiles not preserving mode
     os.chmod(os.path.join(project, "files/bin-files/usr/bin/hello"), 0o0755)
 
@@ -467,15 +469,15 @@ def test_pull_access_rights(cli, tmpdir, datafiles):
 
         st = os.lstat(os.path.join(checkout, "usr/include/pony.h"))
         assert stat.S_ISREG(st.st_mode)
-        assert stat.S_IMODE(st.st_mode) == 0o0644
+        assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask
 
         st = os.lstat(os.path.join(checkout, "usr/bin/hello"))
         assert stat.S_ISREG(st.st_mode)
-        assert stat.S_IMODE(st.st_mode) == 0o0755
+        assert stat.S_IMODE(st.st_mode) == 0o0777 & ~umask
 
         st = os.lstat(os.path.join(checkout, "usr/share/big-file"))
         assert stat.S_ISREG(st.st_mode)
-        assert stat.S_IMODE(st.st_mode) == 0o0644
+        assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask
 
         shutil.rmtree(checkout)
 
@@ -493,15 +495,15 @@ def test_pull_access_rights(cli, tmpdir, datafiles):
 
         st = os.lstat(os.path.join(checkout, "usr/include/pony.h"))
         assert stat.S_ISREG(st.st_mode)
-        assert stat.S_IMODE(st.st_mode) == 0o0644
+        assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask
 
         st = os.lstat(os.path.join(checkout, "usr/bin/hello"))
         assert stat.S_ISREG(st.st_mode)
-        assert stat.S_IMODE(st.st_mode) == 0o0755
+        assert stat.S_IMODE(st.st_mode) == 0o0777 & ~umask
 
         st = os.lstat(os.path.join(checkout, "usr/share/big-file"))
         assert stat.S_ISREG(st.st_mode)
-        assert stat.S_IMODE(st.st_mode) == 0o0644
+        assert stat.S_IMODE(st.st_mode) == 0o0666 & ~umask
 
 
 # Tests `bst artifact pull $artifact_ref`
diff --git a/tests/sources/remote.py b/tests/sources/remote.py
index d546c6131..416d2d53c 100644
--- a/tests/sources/remote.py
+++ b/tests/sources/remote.py
@@ -5,6 +5,7 @@ import os
 import stat
 import pytest
 
+from buildstream import utils
 from buildstream.testing import ErrorDomain
 from buildstream.testing import generate_project
 from buildstream.testing import cli  # pylint: disable=unused-import
@@ -71,8 +72,12 @@ def test_simple_file_build(cli, tmpdir, datafiles):
     mode = os.stat(checkout_file).st_mode
     # Assert not executable by anyone
     assert not mode & (stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
-    # Assert not writeable by anyone other than me
-    assert not mode & (stat.S_IWGRP | stat.S_IWOTH)
+
+    # Assert not writeable by anyone other than me (unless umask allows it)
+    if utils.get_umask() & stat.S_IWGRP:
+        assert not mode & stat.S_IWGRP
+    if utils.get_umask() & stat.S_IWOTH:
+        assert not mode & stat.S_IWOTH
 
 
 @pytest.mark.datafiles(os.path.join(DATA_DIR, "single-file-custom-name"))