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

Chmod on directories has no effect on readability #496

Closed
ian-h-chamberlain opened this issue Oct 9, 2019 · 21 comments
Closed

Chmod on directories has no effect on readability #496

ian-h-chamberlain opened this issue Oct 9, 2019 · 21 comments
Labels

Comments

@ian-h-chamberlain
Copy link

Describe the bug
After creating a directory with files in it in the fake filesystem, then changing its permissions to disallow r/w/x, it is still possible to read the contents of the directory.

In a real filesystem, a directory with perms 000 would raise PermissionsError when attempting to read the directory contents.

This is not an instance of #489 – these tests still fail even when run as non-root user.

How To Reproduce

"""Minimal reproduction of directory permissions issues"""

import os
import pathlib
import stat

import pytest


@pytest.fixture
def unreadable_dir(fs):
    fs.create_file("/tmp/unreadable/file")
    os.chmod("/tmp/unreadable", 0o000)
    return fs


def test_read_dir_os(unreadable_dir):
    with pytest.raises(OSError):
        os.listdir("/tmp/unreadable")  # Does not raise

    with pytest.raises(OSError):
        os.stat("/tmp/unreadable/file")  # Does not raise


def test_dir_permissions_os(unreadable_dir):
    assert stat.S_IMODE(os.stat("/tmp/unreadable").st_mode) == 0o000


def test_read_dir_pathlib(unreadable_dir):
    path = pathlib.Path("/tmp/unreadable")

    with pytest.raises(OSError):
        path.iterdir()  # Does not raise

    with pytest.raises(OSError):
        (path / "file").stat()  # Does not raise


def test_dir_permissions_pathlib(unreadable_dir):
    path = pathlib.Path("/tmp/unreadable")
    assert stat.S_IMODE(path.stat().st_mode) == 0o000

Your enviroment

$ python -c "import platform; print(platform.platform())"
Darwin-18.7.0-x86_64-i386-64bit
$ python -c "import sys; print('Python', sys.version)"
Python 3.6.5 (default, Jan 29 2019, 13:07:11)
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)]
$ python -c "from pyfakefs.fake_filesystem import __version__; print('pyfakefs', __version__)"
pyfakefs 3.6.1

The same symptoms seem to occur when tested on platform

Linux-3.10.0-514.el7.x86_64-x86_64-with-centos-7.5.1804-Core
@mrbean-bremen
Copy link
Member

Thanks for the report! This case is indeed not implemented, and has not been noticed so far.
It's always a day after a release that a new bug pops up...

@mrbean-bremen
Copy link
Member

I can reproduce the listdir and stat problem, but iterdir() does not raise in the Travis tests for Linux (Ubuntu 16.04) and MacOS (10.13.3).

@ian-h-chamberlain
Copy link
Author

Thanks for the quick response! I am running macOS 10.14.6 – not sure if that would impact it. If there is any additional information you need from me to track it down please let me know.

@mrbean-bremen
Copy link
Member

Hm, actually I cannot reproduce any of these problems under Linux (in the Travis environment, as non-root user). I can reproduce all but the iterdir one under MacOS...
I need to investigate further - this seems to depend on the OS and also the OS version, and I'm not sure if it makes much sense to fix this, if it is not reliable in the real OS. Is there a POSIX standard that defines this? Or maybe this depends on some other condition?
@jmcgeheeiv - do you have any insight here?

@mrbean-bremen
Copy link
Member

@ian-h-chamberlain - just to make sure: did you make these tests also on the real filesystem? Especially under Linux?

@jmcgeheeiv
Copy link
Contributor

jmcgeheeiv commented Oct 13, 2019 via email

@ian-h-chamberlain
Copy link
Author

@mrbean-bremen – my apologies, it seems my test was incomplete  – iterdir() returns a generator and that generator must be exhausted for the OS to raise.

Here is a revised version, I was able to reproduce within a Linux container (don't have access to a real linux machine atm) as well as on macOS 10.14.6:

"""Minimal reproduction of directory permissions issues"""

import os
import pathlib
import stat

import pytest


_PATH = os.path.dirname(__file__) + "/test_dir"


@pytest.fixture(params=[pytest.param(True, id="real_fs"), pytest.param(False, id="fake_fs")])
def unreadable_dir(request, fs):
    use_real_fs = request.param

    if use_real_fs:
        fs.pause()
    else:
        fs.create_file(_PATH + "/file")
        os.chmod(_PATH, 0o000)

    yield fs
    fs.resume()


def test_read_dir_os(unreadable_dir):
    with pytest.raises(OSError):
        os.listdir(_PATH)  # Does not raise

    with pytest.raises(OSError):
        os.stat(_PATH + "/file")  # Does not raise


def test_dir_permissions_os(unreadable_dir):
    assert stat.S_IMODE(os.stat(_PATH).st_mode) == 0o000


def test_read_dir_pathlib(unreadable_dir):
    path = pathlib.Path(_PATH)

    with pytest.raises(OSError):
        list(path.iterdir())  # Does not raise

    with pytest.raises(OSError):
        (path / "file").stat()  # Does not raise


def test_dir_permissions_pathlib(unreadable_dir):
    path = pathlib.Path(_PATH)
    assert stat.S_IMODE(path.stat().st_mode) == 0o000

Output:

$ ls
total 16
drwxr-xr-x   7 ianchamberlain  staff   224B Oct 13 15:46 ./
drwx------@ 34 ianchamberlain  staff   1.1K Oct 13 15:04 ../
drwxr-xr-x   6 ianchamberlain  staff   192B Oct 13 15:20 .pytest_cache/
-rw-r--r--   1 ianchamberlain  staff     9B Oct 13 15:20 .python-version
drwxr-xr-x   4 ianchamberlain  staff   128B Oct 13 15:46 __pycache__/
-rw-r--r--   1 ianchamberlain  staff   1.1K Oct 13 15:44 example_test.py
d---------+  3 ianchamberlain  staff    96B Oct 13 15:21 test_dir/

$ pytest example_test.py
Users/ianchamberlain/Documents/pyfakefs/example_test.py .F...F..                                               [100%]

====================================================== FAILURES ======================================================
_____________________________________________ test_read_dir_os[fake_fs] ______________________________________________

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7fd5bdff5cd0>

    def test_read_dir_os(unreadable_dir):
        with pytest.raises(OSError):
>           os.listdir(_PATH)  # Does not raise
E           Failed: DID NOT RAISE <class 'OSError'>

/Users/ianchamberlain/Documents/pyfakefs/example_test.py:29: Failed
___________________________________________ test_read_dir_pathlib[fake_fs] ___________________________________________

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7fd5bd7c6ad0>

    def test_read_dir_pathlib(unreadable_dir):
        path = pathlib.Path(_PATH)

        with pytest.raises(OSError):
>           list(path.iterdir())  # Does not raise
E           Failed: DID NOT RAISE <class 'OSError'>

/Users/ianchamberlain/Documents/pyfakefs/example_test.py:43: Failed
============================================ 2 failed, 6 passed in 0.57s =============================================

Docker command shows the same output:

$ docker pull python
$ docker run -i -t -v $(pwd):$(pwd)  python /bin/bash -c "pip install pytest pyfakefs && pytest $(pwd)/example_test.py"

...

Users/ianchamberlain/Documents/pyfakefs/example_test.py .F...F..                                               [100%]

====================================================== FAILURES ======================================================
_____________________________________________ test_read_dir_os[fake_fs] ______________________________________________

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7fd5bdff5cd0>

    def test_read_dir_os(unreadable_dir):
        with pytest.raises(OSError):
>           os.listdir(_PATH)  # Does not raise
E           Failed: DID NOT RAISE <class 'OSError'>

/Users/ianchamberlain/Documents/pyfakefs/example_test.py:29: Failed
___________________________________________ test_read_dir_pathlib[fake_fs] ___________________________________________

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7fd5bd7c6ad0>

    def test_read_dir_pathlib(unreadable_dir):
        path = pathlib.Path(_PATH)

        with pytest.raises(OSError):
>           list(path.iterdir())  # Does not raise
E           Failed: DID NOT RAISE <class 'OSError'>

/Users/ianchamberlain/Documents/pyfakefs/example_test.py:43: Failed
============================================ 2 failed, 6 passed in 0.57s =============================================

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 14, 2019

Hm, as I said - I can reproduce the behavior under MacOS, but not under Linux (in Travis).
I added some changes in a separate branch which changes the behavior under MacOS only (if I change it under Linux, the real OS tests will fail).

Your tests look good, so there has to be some reason for the different behavior under Linux...
Just to be sure: can you please check the error code of the exception you get under Linux? It should be errno.EACCES. Also, I guess your test in the container do not run as root (otherwise I wouldn't expect the exception to occur).

EDIT: If possible, it would be nice if you could test the aforementioned branch under MacOS. I'm not sure if the changes are sufficient.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Oct 14, 2019
- changes are for MacOS only, as long as Linux behavior is unclear
- see pytest-dev#496
@mrbean-bremen
Copy link
Member

@ian-h-chamberlain - I added a PR with the changes (MacOS only, as I wrote), maybe you can check it out.

mrbean-bremen added a commit that referenced this issue Oct 18, 2019
- changes are for MacOS only, as long as Linux behavior is unclear
- see #496
@ian-h-chamberlain
Copy link
Author

@mrbean-bremen Sorry for the delay. I was able to test on Linux and as far as I can tell the behavior is the same as on macOS for me – my test fails in the same way:

[ichamberlain@a96311cf9faf] i95code# pytest os_dirs_test.py
================================================ test session starts =================================================
platform linux -- Python 3.6.5, pytest-5.2.1, py-1.8.0, pluggy-0.13.0
rootdir: /i95code
plugins: pyfakefs-3.5.8
collected 8 items

os_dirs_test.py .F...F..                                                                                       [100%]

====================================================== FAILURES ======================================================
_____________________________________________ test_read_dir_os[fake_fs] ______________________________________________

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7f732a69c908>

    def test_read_dir_os(unreadable_dir):
        with pytest.raises(OSError) as excinfo:
>           os.listdir(_PATH)  # Does not raise
E           Failed: DID NOT RAISE <class 'OSError'>

/i95code/os_dirs_test.py:30: Failed
___________________________________________ test_read_dir_pathlib[fake_fs] ___________________________________________

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7f7324b24438>

    def test_read_dir_pathlib(unreadable_dir):
        path = pathlib.Path(_PATH)

        with pytest.raises(OSError) as excinfo:
>           list(path.iterdir())  # Does not raise
E           Failed: DID NOT RAISE <class 'OSError'>

/i95code/os_dirs_test.py:48: Failed
============================================ 2 failed, 6 passed in 0.55s =============================================
[ichamberlain@a96311cf9faf] i95code# ls -lart test_dir/
ls: cannot open directory test_dir/: Permission denied
[ichamberlain@a96311cf9faf] i95code# sudo ls -lart test_dir/
total 8
-rw-rw-r--  1 ichamberlain ichamberlain    0 Oct 18 14:17 file
d---------  2 ichamberlain ichamberlain 4096 Oct 18 14:17 .
drwxrwxr-x 23 ichamberlain ichamberlain 4096 Oct 18 14:20 ..
[ichamberlain@a96311cf9faf] i95code#

The error number in the OSError is indeed errno.EACCES, verified by adding an

assert excinfo.value.errno == errno.EACCES

in each test case where an error is expected.
Note this was also tested in a Docker container, but in this case the host is

Linux-3.10.0-514.el7.x86_64-x86_64-with-centos-7.3.1611-Core

Hope this helps, I'm not sure why you're not able to reproduce this on your system (perhaps CentOS handles this differently than the Linux distro in your CI)?

@ian-h-chamberlain
Copy link
Author

Second bit – my test fails on macOS using latest `master as well:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x1068caba8>, errno = 13, filename = '/Users/ichamberlain/Downloads/test_dir'
winerror = None

    def raise_os_error(self, errno, filename=None, winerror=None):
        """Raises OSError.
            The error message is constructed from the given error code and shall
            start with the error string issued in the real system.
            Note: this is not true under Windows if winerror is given - in this
            case a localized message specific to winerror will be shown in the
            real file system.

            Args:
                errno: A numeric error code from the C variable errno.
                filename: The name of the affected file, if any.
                winerror: Windows only - the specific Windows error code.
            """
        message = self._error_message(errno)
        if (winerror is not None and sys.platform == 'win32' and
                self.is_windows_fs):
            if IS_PY2:
                raise WindowsError(winerror, message, filename)
            raise OSError(errno, message, filename, winerror)
>       raise OSError(errno, message, filename)
E       PermissionError: [Errno 13] Permission denied in the fake filesystem: '/Users/ichamberlain/Downloads/test_dir'

/Users/ichamberlain/Documents/thirdparty/pyfakefs/pyfakefs/fake_filesystem.py:984: PermissionError

During handling of the above exception, another exception occurred:

unreadable_dir = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x1068caba8>

    def test_dir_permissions_os(unreadable_dir):
>       assert stat.S_IMODE(os.stat(_PATH).st_mode) == 0o000

/Users/ichamberlain/Downloads/os_dirs_test.py:41:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
/Users/ichamberlain/Documents/thirdparty/pyfakefs/pyfakefs/fake_filesystem.py:4122: in stat
    return self.filesystem.stat(entry_path, follow_symlinks)
/Users/ichamberlain/Documents/thirdparty/pyfakefs/pyfakefs/fake_filesystem.py:1187: in stat
    self.raise_os_error(io_error.errno, entry_path, winerror=winerror)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

As far as I can tell, macOS (and Linux for that matter) default behavior is to allow stat on files, even if they are 0o000 permissions – but the fake filesystem raises an error when attempting to stat the file.

@mrbean-bremen
Copy link
Member

Hm, thanks for the information! The CI build is using Ubuntu 16.04. I may try to change that and test other systems, but as long as I don't understand where the difference comes from, I won't change it in the code. Maybe there is some setting that differs... I actually would have expected the behavior you describe, at least for listdir and iterdir, as these should need read permission for the directory (at least if not run as root). Maybe I'm doing something wrong...
The current master explicitely checks that these calls succeed under Linux (both in the fake and real OS), and fail under MacOS.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Oct 18, 2019

Ok, didn't see your second post about MacOS - this looks like a bug I did introduce. I agree that stat shall work in this case.
Will fix this in a moment... or tomorrow, as the MacOS CI seems to be down.

mrbean-bremen added a commit that referenced this issue Oct 18, 2019
- changes are for MacOS only, as long as Linux behavior is unclear
- see #496
@mrbean-bremen
Copy link
Member

I did some branch mixup, but it shall be fixed now for MacOs.

@mrbean-bremen
Copy link
Member

@ian-h-chamberlain - just to let you know: I tried the same with a CentOs docker image, and got the same behavior you got. So at least CentOS and Ubuntu seem to behave differently in this respect. I think I will test with some other systems later, though that may take some time (I won't have time/access to a computer at least until next month).

@mrbean-bremen
Copy link
Member

Ok, I just found out that probably the behavior does not depend on the distribution, but on the fact that the test is run in a docker container. Running the tests in an Ubuntu docker container shows the same behavior as for CentOs. This is a bit unfortunate, as I don't know how to test other systems not in a docker image - both Travis.CI and GitHub actions use Ubuntu images only. I know that inodes behave differently in a docker image for some reason, and I could adapt the behavior to be dependent on them running in a docker image.

What this also means is that you should probably not rely on this kind of behavior if you want to have portable code.

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Nov 3, 2019
@mrbean-bremen
Copy link
Member

Due to #500 some of the results may not be valid...

mrbean-bremen added a commit to mrbean-bremen/pyfakefs that referenced this issue Nov 3, 2019
- enable for Linux, disable for root users
- see pytest-dev#496
@mrbean-bremen
Copy link
Member

Ok, sorry for all the confusion - all my tests have been void due to an incorrect Travis.CI script that does not fail the build if one of the test runs (except the last) fails. As the last test under Linux (but not under MacOS and not in a docker container) was running as a root user, the behavior was different here...
I will merge the fixed version to master in a moment - please check if that works for you now!

mrbean-bremen added a commit that referenced this issue Nov 3, 2019
- enable for Linux, disable for root users
- see #496
@mrbean-bremen
Copy link
Member

Shall be fixed now - please re-open if something is missing.

@jmcgeheeiv
Copy link
Contributor

That was some fine work on a difficult problem, @mrbean-bremen. Very well done.

@mrbean-bremen
Copy link
Member

Thanks - though this was an error of my own making - didn't think about error propagation while writing the test script for Travis...

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

No branches or pull requests

3 participants