diff --git a/CHANGES.md b/CHANGES.md index 144d20bc..bfbad4a4 100644 --- a/CHANGES.md +++ b/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. diff --git a/pyfakefs/fake_file.py b/pyfakefs/fake_file.py index bc37a961..b4707ab2 100644 --- a/pyfakefs/fake_file.py +++ b/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) diff --git a/pyfakefs/fake_filesystem.py b/pyfakefs/fake_filesystem.py index 74f1a082..1d71f2b4 100644 --- a/pyfakefs/fake_filesystem.py +++ b/pyfakefs/fake_filesystem.py @@ -674,6 +674,7 @@ 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) @@ -681,7 +682,7 @@ def stat(self, entry_path: AnyStr, follow_symlinks: bool = True): # 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,9 +1642,11 @@ 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: @@ -1648,14 +1654,15 @@ def get_object_from_normpath( 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,7 +2873,11 @@ 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. @@ -2867,6 +2885,10 @@ def confirmdir( 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) diff --git a/pyfakefs/fake_open.py b/pyfakefs/fake_open.py index e6024296..29fea3d5 100644 --- a/pyfakefs/fake_open.py +++ b/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: diff --git a/pyfakefs/fake_os.py b/pyfakefs/fake_os.py index 660ba6c5..2374b4c4 100644 --- a/pyfakefs/fake_os.py +++ b/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] diff --git a/pyfakefs/fake_scandir.py b/pyfakefs/fake_scandir.py index 16c7c346..e01f0681 100644 --- a/pyfakefs/fake_scandir.py +++ b/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): diff --git a/pyfakefs/tests/fake_os_test.py b/pyfakefs/tests/fake_os_test.py index 9aff3c6e..b8d3da6f 100644 --- a/pyfakefs/tests/fake_os_test.py +++ b/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) diff --git a/pyfakefs/tests/fake_pathlib_test.py b/pyfakefs/tests/fake_pathlib_test.py index 15bb7a08..d26c5112 100644 --- a/pyfakefs/tests/fake_pathlib_test.py +++ b/pyfakefs/tests/fake_pathlib_test.py @@ -450,6 +450,49 @@ def test_iterdir_in_unreadable_dir(self): path = str(list(it)[0]) self.assertTrue(path.endswith("some_file")) + def test_iterdir_and_glob_without_exe_permission(self): + # regression test for #960 + self.check_posix_only() + directory = self.path(self.make_path("testdir")) + file_path = directory / "file.txt" + self.create_file(file_path, contents="hey", perm=0o777) + directory.chmod(0o655) # rw-r-xr-x + # We cannot create any files in the directory, because that requires + # searching it + another_file = self.path(self.make_path("file.txt")) + self.create_file(another_file, contents="hey") + with self.assertRaises(PermissionError): + self.os.link(another_file, directory / "link.txt") + # We can enumerate the directory using iterdir and glob: + assert len(list(directory.iterdir())) == 1 + assert list(directory.iterdir())[0] == file_path + assert len(list(directory.glob("*.txt"))) == 1 + assert list(directory.glob("*.txt"))[0] == file_path + + # We cannot read files inside of the directory, + # even if we have read access to the file + with self.assertRaises(PermissionError): + file_path.stat() + with self.assertRaises(PermissionError): + file_path.read_text() + + def test_listdir_impossible_without_read_permission(self): + # regression test for #960 + self.check_posix_only() + directory = self.path(self.make_path("testdir")) + file_path = directory / "file.txt" + self.create_file(file_path, contents="hey", perm=0o777) + directory.chmod(0o355) # -wxr-xr-x + + # We cannot enumerate the directory using iterdir: + with self.assertRaises(PermissionError): + list(directory.iterdir()) + # glob does not find the file + assert len(list(directory.glob("*.txt"))) == 0 + # we can access the file if we know the file name + assert file_path.stat().st_mode & 0o777 == 0o777 + assert file_path.read_text() == "hey" + def test_resolve_nonexisting_file(self): path = self.path(self.make_path("/path", "to", "file", "this can not exist")) self.assertEqual(path, path.resolve())