Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding/removing directory entries does not respect directory ownership #959

Closed
vector-of-bool opened this issue Mar 4, 2024 · 1 comment · Fixed by #961
Closed

Adding/removing directory entries does not respect directory ownership #959

vector-of-bool opened this issue Mar 4, 2024 · 1 comment · Fixed by #961
Labels

Comments

@vector-of-bool
Copy link

Describe the bug
Adding or removing entries from a directory requires that the user has write permissions to that directory. Trying to modify entries in a directory to which the user cannot write should raise PermissionError.

How To Reproduce
The following test cases should both succeed:

import os
import pytest
import pyfakefs.fake_filesystem

def test_fail_write_dir(fs):
    # Prereq: Create a directory at '/readonly-dir' that is owned by root and has
    # permissions 0o755 (rwxr-xr-x)
    rodir = "/readonly-dir"
    os.makedirs(rodir, exist_ok=True)
    st = os.stat(rodir)
    if st.st_uid != 0:
        os.chown(rodir, 0, 0)
        os.chmod(rodir, 0b111_101_101)
        st = os.stat(rodir)
    # Only modifyable by the root, but readable/traversable by all
    assert st.st_uid == 0
    assert st.st_mode & 0o777 == 0b111_101_101  # == 0o755
    # Try to add a new entry to the readonly subdirectory
    with pytest.raises(PermissionError):
        with open(f"{rodir}/file.txt", "w"):
            pass


def test_fail_fakefs_root_owned_dir():
    fs = pyfakefs.fake_filesystem.FakeFilesystem()
    fake_os = pyfakefs.fake_filesystem.FakeOsModule(fs)
    # Create a regular file
    fs.create_file("file.txt")
    # Create a directory that can only be written-to by root
    fake_os.mkdir("/readonly-dir", mode=0b111_101_101)
    fake_os.chown("/readonly-dir", 0, 0)
    # Creating a new entry should fail
    with pytest.raises(PermissionError):
        fake_os.link("/file.txt", "/readonly-dir/file.txt")

Your environment
Please run the following in the environment where the problem happened and
paste the output.

$ python -c "import platform; print(platform.platform())"
Linux-6.7.6-arch1-1-x86_64-with-glibc2.39
$ python -c "import sys; print('Python', sys.version)"
Python 3.11.7 (main, Jan 29 2024, 16:03:57) [GCC 13.2.1 20230801]
$ python -c "from pyfakefs import __version__; print('pyfakefs', __version__)"
pyfakefs 5.3.5
$ python -c "import pytest; print('pytest', pytest.__version__)"
pytest 7.2.0

If I understand correctly, the issue appears to be in FakeDirectory.add_entry, which starts with this guard:

if (
    not helpers.is_root()
    and not self.st_mode & helpers.PERM_WRITE
    and not self.filesystem.is_windows_fs
):
    raise OSError(errno.EACCES, "Permission Denied", self.path)

The check on st_mode checks that the S_IWUSR bit is set, but does not consider the case that st_uid != get_uid().

I'm trying to write code that is robust against "adverse" filesystem conditions, with non-writable directories being a common example.

@mrbean-bremen
Copy link
Member

Thanks! pyfakefs currently has only a rudimentary concept of user rights - it is currently mostly concerned with root vs. non-root users, so this (and your other issue) are not surprising.
Thanks for digging into this and providing the tests - I will have a go at this, but that may take a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants