Correctly handle file permissions for different classes

- always consider the current user group while evaluating
  file permissions for user/group/other classes

CHANGES.md

@@ -3,11 +3,19 @@ The released versions correspond to PyPI releases.
 
 ## Unreleased
 
+### Changes
+* The handling of file permissions under Posix is should now mostly match the behavior
+  of the real filesystem, which may change the behavior of some tests
+
 ### Fixes
 * Fixed a specific problem on reloading a pandas-related module (see [#947](../../issues/947)),
   added possibility for unload hooks for specific modules
 * Use this also to reload django views (see [#932](../../issues/932))
 * Fixed `EncodingWarning` for Python >= 3.11 (see [#957](../../issues/957))
+* Consider directory ownership while adding or removing directory entries
+  (see [#959](../../issues/959))
+* Fixed handling of directory enumeration and search permissions under Posix systems
+  (see [#960](../../issues/960))
 
 ## [Version 5.3.5](https://pypi.python.org/pypi/pyfakefs/5.3.5) (2024-01-30)
 Fixes a regression.

pyfakefs/fake_file.py

@@ -391,6 +391,21 @@ def __setattr__(self, key: str, value: Any) -> None:
     def __str__(self) -> str:
         return "%r(%o)" % (self.name, self.st_mode)
 
+    def has_permission(self, permission_bits: int) -> bool:
+        """Checks if the given permissions are set in the fake file.
+
+        Args:
+            permission_bits: The permission bits as set for the user.
+
+        Returns:
+            True if the permissions are set in the correct class (user/group/other).
+        """
+        if helpers.get_uid() == self.stat_result.st_uid:
+            return self.st_mode & permission_bits == permission_bits
+        if helpers.get_gid() == self.stat_result.st_gid:
+            return self.st_mode & (permission_bits >> 3) == permission_bits >> 3
+        return self.st_mode & (permission_bits >> 6) == permission_bits >> 6
+
 
 class FakeNullFile(FakeFile):
     def __init__(self, filesystem: "FakeFilesystem") -> None:
@@ -504,8 +519,8 @@ def add_entry(self, path_object: FakeFile) -> None:
         """
         if (
             not helpers.is_root()
-            and not self.st_mode & helpers.PERM_WRITE
             and not self.filesystem.is_windows_fs
+            and not self.has_permission(helpers.PERM_WRITE)
         ):
             raise OSError(errno.EACCES, "Permission Denied", self.path)
 
@@ -572,9 +587,8 @@ def remove_entry(self, pathname_name: str, recursive: bool = True) -> None:
             if self.filesystem.has_open_file(entry):
                 self.filesystem.raise_os_error(errno.EACCES, pathname_name)
         else:
-            if not helpers.is_root() and (
-                self.st_mode & (helpers.PERM_WRITE | helpers.PERM_EXE)
-                != helpers.PERM_WRITE | helpers.PERM_EXE
+            if not helpers.is_root() and not self.has_permission(
+                helpers.PERM_WRITE | helpers.PERM_EXE
             ):
                 self.filesystem.raise_os_error(errno.EACCES, pathname_name)
 

pyfakefs/fake_filesystem.py

@@ -674,14 +674,15 @@ def stat(self, entry_path: AnyStr, follow_symlinks: bool = True):
                 follow_symlinks,
                 allow_fd=True,
                 check_read_perm=False,
+                check_exe_perm=False,
             )
         except TypeError:
             file_object = self.resolve(entry_path)
         if not is_root():
             # make sure stat raises if a parent dir is not readable
             parent_dir = file_object.parent_dir
             if parent_dir:
-                self.get_object(parent_dir.path)  # type: ignore[arg-type]
+                self.get_object(parent_dir.path, check_read_perm=False)  # type: ignore[arg-type]
 
         self.raise_for_filepath_ending_with_separator(
             entry_path, file_object, follow_symlinks
@@ -1597,6 +1598,7 @@ def get_object_from_normpath(
         self,
         file_path: AnyPath,
         check_read_perm: bool = True,
+        check_exe_perm: bool = True,
         check_owner: bool = False,
     ) -> AnyFile:
         """Search for the specified filesystem object within the fake
@@ -1607,6 +1609,8 @@ def get_object_from_normpath(
                 path that has already been normalized/resolved.
             check_read_perm: If True, raises OSError if a parent directory
                 does not have read permission
+            check_exe_perm: If True, raises OSError if a parent directory
+                does not have execute (e.g. search) permission
             check_owner: If True, and check_read_perm is also True,
                 only checks read permission if the current user id is
                 different from the file object user id
@@ -1638,24 +1642,27 @@ def get_object_from_normpath(
                 target = target.get_entry(component)  # type: ignore
                 if (
                     not is_root()
-                    and check_read_perm
+                    and (check_read_perm or check_exe_perm)
                     and target
-                    and not self._can_read(target, check_owner)
+                    and not self._can_read(
+                        target, check_read_perm, check_exe_perm, check_owner
+                    )
                 ):
                     self.raise_os_error(errno.EACCES, target.path)
         except KeyError:
             self.raise_os_error(errno.ENOENT, path)
         return target
 
     @staticmethod
-    def _can_read(target, owner_can_read):
-        if target.st_uid == helpers.get_uid():
-            if owner_can_read or target.st_mode & 0o400:
-                return True
-        if target.st_gid == get_gid():
-            if target.st_mode & 0o040:
-                return True
-        return target.st_mode & 0o004
+    def _can_read(target, check_read_perm, check_exe_perm, owner_can_read):
+        if owner_can_read and target.st_uid == helpers.get_uid():
+            return True
+        permission = helpers.PERM_READ if check_read_perm else 0
+        if S_ISDIR(target.st_mode) and check_exe_perm:
+            permission |= helpers.PERM_EXE
+        if not permission:
+            return True
+        return target.has_permission(permission)
 
     def get_object(self, file_path: AnyPath, check_read_perm: bool = True) -> FakeFile:
         """Search for the specified filesystem object within the fake
@@ -1684,6 +1691,7 @@ def resolve(
         follow_symlinks: bool = True,
         allow_fd: bool = False,
         check_read_perm: bool = True,
+        check_exe_perm: bool = True,
         check_owner: bool = False,
     ) -> FakeFile:
         """Search for the specified filesystem object, resolving all links.
@@ -1695,6 +1703,8 @@ def resolve(
             allow_fd: If `True`, `file_path` may be an open file descriptor
             check_read_perm: If True, raises OSError if a parent directory
                 does not have read permission
+            check_read_perm: If True, raises OSError if a parent directory
+                does not have execute permission
             check_owner: If True, and check_read_perm is also True,
                 only checks read permission if the current user id is
                 different from the file object user id
@@ -1714,6 +1724,7 @@ def resolve(
             return self.get_object_from_normpath(
                 self.resolve_path(file_path, allow_fd),
                 check_read_perm,
+                check_exe_perm,
                 check_owner,
             )
         return self.lresolve(file_path)
@@ -1756,7 +1767,7 @@ def lresolve(self, path: AnyPath) -> FakeFile:
                 if not self.is_windows_fs and isinstance(parent_obj, FakeFile):
                     self.raise_os_error(errno.ENOTDIR, path_str)
                 self.raise_os_error(errno.ENOENT, path_str)
-            if not parent_obj.st_mode & helpers.PERM_READ:
+            if not parent_obj.has_permission(helpers.PERM_READ):
                 self.raise_os_error(errno.EACCES, parent_directory)
             return (
                 parent_obj.get_entry(to_string(child_name))
@@ -1781,7 +1792,10 @@ def add_object(self, file_path: AnyStr, file_object: AnyFile) -> None:
         if not file_path:
             target_directory = self.root_dir
         else:
-            target_directory = cast(FakeDirectory, self.resolve(file_path))
+            target_directory = cast(
+                FakeDirectory,
+                self.resolve(file_path, check_read_perm=False, check_exe_perm=True),
+            )
             if not S_ISDIR(target_directory.st_mode):
                 error = errno.ENOENT if self.is_windows_fs else errno.ENOTDIR
                 self.raise_os_error(error, file_path)
@@ -2859,14 +2873,22 @@ def isjunction(self, path: AnyPath) -> bool:
             return False
 
     def confirmdir(
-        self, target_directory: AnyStr, check_owner: bool = False
+        self,
+        target_directory: AnyStr,
+        check_read_perm: bool = True,
+        check_exe_perm: bool = True,
+        check_owner: bool = False,
     ) -> FakeDirectory:
         """Test that the target is actually a directory, raising OSError
         if not.
 
         Args:
             target_directory: Path to the target directory within the fake
                 filesystem.
+            check_read_perm: If True, raises OSError if the directory
+                does not have read permission
+            check_exe_perm: If True, raises OSError if the directory
+                does not have execute (e.g. search) permission
             check_owner: If True, only checks read permission if the current
                 user id is different from the file object user id
 
@@ -2878,7 +2900,12 @@ def confirmdir(
         """
         directory = cast(
             FakeDirectory,
-            self.resolve(target_directory, check_owner=check_owner),
+            self.resolve(
+                target_directory,
+                check_read_perm=check_read_perm,
+                check_exe_perm=check_exe_perm,
+                check_owner=check_owner,
+            ),
         )
         if not directory.st_mode & S_IFDIR:
             self.raise_os_error(errno.ENOTDIR, target_directory, 267)
@@ -2972,7 +2999,7 @@ def listdir(self, target_directory: AnyStr) -> List[AnyStr]:
             OSError: if the target is not a directory.
         """
         target_directory = self.resolve_path(target_directory, allow_fd=True)
-        directory = self.confirmdir(target_directory)
+        directory = self.confirmdir(target_directory, check_exe_perm=False)
         directory_contents = list(directory.entries.keys())
         if self.shuffle_listdir_results:
             random.shuffle(directory_contents)

pyfakefs/fake_open.py

@@ -270,8 +270,8 @@ def _init_file_object(
     ) -> FakeFile:
         if file_object:
             if not is_root() and (
-                (open_modes.can_read and not file_object.st_mode & PERM_READ)
-                or (open_modes.can_write and not file_object.st_mode & PERM_WRITE)
+                (open_modes.can_read and not file_object.has_permission(PERM_READ))
+                or (open_modes.can_write and not file_object.has_permission(PERM_WRITE))
             ):
                 self.filesystem.raise_os_error(errno.EACCES, file_path)
             if open_modes.can_write:

pyfakefs/fake_os.py

@@ -422,7 +422,7 @@ def chdir(self, path: AnyStr) -> None:
         directory = self.filesystem.resolve(path)
         # A full implementation would check permissions all the way
         # up the tree.
-        if not is_root() and not directory.st_mode | PERM_EXE:
+        if not is_root() and not directory.has_permission(PERM_EXE):
             self.filesystem.raise_os_error(errno.EACCES, directory.name)
         self.filesystem.cwd = path  # type: ignore[assignment]
 

pyfakefs/fake_scandir.py

@@ -146,7 +146,7 @@ def __init__(self, filesystem, path):
             path = make_string_path(path)
             self.abspath = self.filesystem.absnormpath(path)
             self.path = to_string(path)
-        entries = self.filesystem.confirmdir(self.abspath).entries
+        entries = self.filesystem.confirmdir(self.abspath, check_exe_perm=False).entries
         self.entry_iter = iter(entries)
 
     def __iter__(self):

pyfakefs/tests/fake_os_test.py

@@ -2139,6 +2139,27 @@ def test_chown_nonexisting_file_should_raise_os_error(self):
         self.assertFalse(self.os.path.exists(file_path))
         self.assert_raises_os_error(errno.ENOENT, self.os.chown, file_path, 100, 100)
 
+    def test_fail_add_entry_to_readonly_dir(self):
+        # regression test for #959
+        self.check_posix_only()
+        self.skip_real_fs()  # cannot change owner to root
+        if is_root():
+            self.skipTest("Non-root test only")
+
+        # create directory owned by root with permissions 0o755 (rwxr-xr-x)
+        ro_dir = self.make_path("readonly-dir")
+        self.create_dir(ro_dir, perm=0o755)
+        self.os.chown(ro_dir, 0, 0)
+
+        # adding a new entry to the readonly subdirectory should fail
+        with self.assertRaises(PermissionError):
+            with self.open(f"{ro_dir}/file.txt", "w"):
+                pass
+        file_path = self.make_path("file.txt")
+        self.create_file(file_path)
+        with self.assertRaises(PermissionError):
+            self.os.link(file_path, self.os.path.join(ro_dir, "file.txt"))
+
     def test_classify_directory_contents(self):
         """Directory classification should work correctly."""
         root_directory = self.make_path("foo")
@@ -2920,6 +2941,51 @@ def test_listdir_on_symlink(self):
         files.sort()
         self.assertEqual(files, sorted(self.os.listdir(self.make_path("SymLink"))))
 
+    def test_listdir_possible_without_exe_permission(self):
+        # regression test for #960
+        self.check_posix_only()
+        directory = self.make_path("testdir")
+        file_path = self.os.path.join(directory, "file.txt")
+        self.create_file(file_path, contents="hey", perm=0o777)
+        self.os.chmod(directory, 0o655)  # rw-r-xr-x
+        # We cannot create any files in the directory, because that requires
+        # searching it
+        another_file = self.make_path("file.txt")
+        self.create_file(another_file, contents="hey")
+        with self.assertRaises(PermissionError):
+            self.os.link(another_file, self.os.path.join(directory, "link.txt"))
+        # We can enumerate the directory using listdir and scandir:
+        assert self.os.listdir(directory) == ["file.txt"]
+        assert len(list(self.os.scandir(directory))) == 1
+
+        # We cannot read files inside of the directory,
+        # even if we have read access to the file
+        with self.assertRaises(PermissionError):
+            self.os.stat(file_path)
+        with self.assertRaises(PermissionError):
+            with self.open(file_path) as f:
+                f.read()
+
+    def test_listdir_impossible_without_read_permission(self):
+        # regression test for #960
+        self.check_posix_only()
+        directory = self.make_path("testdir")
+        file_path = self.os.path.join(directory, "file.txt")
+        self.create_file(file_path, contents="hey", perm=0o777)
+        self.os.chmod(directory, 0o355)  # -wxr-xr-x
+        another_file = self.make_path("file.txt")
+        self.create_file(another_file, contents="hey")
+        self.os.link(another_file, self.os.path.join(directory, "link.txt"))
+        # We cannot enumerate the directory using listdir or scandir:
+        with self.assertRaises(PermissionError):
+            self.os.listdir(directory)
+        with self.assertRaises(PermissionError):
+            self.os.scandir(directory)
+        # we can access the file if we know the file name
+        assert self.os.stat(file_path).st_mode & 0o777 == 0o777
+        with self.open(file_path) as f:
+            assert f.read() == "hey"
+
     def test_fdopen_mode(self):
         self.skip_real_fs()
         file_path1 = self.make_path("some_file1")
@@ -5475,14 +5541,20 @@ def test_listdir_group_readable_dir_from_other_group(self):
             self.assertEqual([], self.os.listdir(dir_path))
 
     def test_listdir_other_readable_dir_from_other_group(self):
+        self.check_posix_only()
         self.skip_real_fs()  # won't change user in real fs
+        user_id = get_uid()
         group_id = get_gid()
-        set_gid(group_id + 1)
         dir_path = self.make_path("dir1")
-        self.create_dir(dir_path, perm=0o004)
+        self.create_dir(dir_path, 0o004)
+        set_uid(user_id + 1)
+        set_gid(group_id + 1)
         self.assertTrue(self.os.path.exists(dir_path))
-        set_gid(group_id)
         self.assertEqual([], self.os.listdir(dir_path))
+        set_uid(user_id)
+        set_gid(group_id)
+        with self.assertRaises(PermissionError):
+            self.os.listdir(dir_path)
 
     def test_stat_unreadable_dir(self):
         self.assertEqual(0, self.os.stat(self.dir_path).st_mode & 0o666)