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

tempdir hardening fails when filesystem doesn't track ownership #13194

Open
moroten opened this issue Feb 5, 2025 · 5 comments
Open

tempdir hardening fails when filesystem doesn't track ownership #13194

moroten opened this issue Feb 5, 2025 · 5 comments

Comments

@moroten
Copy link

moroten commented Feb 5, 2025

#8516 verifies that tmpdir and tmp_path etc. are owned by the user. This fails on filesystems that do not track ownership.

In my use case, I am using Buildbarn's FUSE storage on Linux where ownership is always reported as 0. This has the benefit that actions reading the ownership, e.g. tar, will be deterministic.

My workaround is to use the following in my test file:

if __name__ == "__main__":
    sys.exit(pytest.main([
        "--basetemp",
        Path(os.environ["TEST_TMPDIR"]),
        __file__,
    ]))

Would it make sense to allow rootdir_stat.st_uid == 0 in src/_pytest/tmpdir.py? An alternative is to disable the check with an environment variable.

Related issues: #8414 and #10738.

@bluetech
Copy link
Member

bluetech commented Feb 5, 2025

In this filesystem, you are able to write to a private directory which is reported as owned by another user? I'm surprised you are not having more issues.

Can you explain how the workaround fixes the issue for you?

@moroten
Copy link
Author

moroten commented Feb 5, 2025

The file system is used inside a containerized environment (but is not necessary to be containerized. I'm able to write anywhere there and there is no other processes running there.

$ ls -la /tmp/
total 0
drwxrwxrwx. 1 root root 0 Feb  5 10:04 .
drwxrwxrwx. 1 root root 0 Feb  5 10:04 ..
$ id
uid=1007 gid=1007 groups=1007

Can you explain how the workaround fixes the issue for you?

When --basetemp is set, the id check is not performed at all: if self._given_basetemp is not None:

@bluetech
Copy link
Member

bluetech commented Feb 5, 2025

Maybe we can also skip the check if PYTEST_DEBUG_TEMPROOT is set? The rationale being, the user explicitly pointed at there, let's assume they know what they're doing and don't need the extra safety check.

@moroten
Copy link
Author

moroten commented Feb 5, 2025

That sounds like a good solution. I'll look at it after lunch.

@moroten
Copy link
Author

moroten commented Feb 5, 2025

After thinking about your suggestion, it is not a perfect fit for my use case. I'm using Bazel to run the tests and I can set hard coded environment variables in the BUILD.bazel files. This means that I will set /tmp for Linux but what should I set on Windows? Bazel does provide TEST_TMPDIR, but then I need a wrapper to set PYTEST_DEBUG_TEMPROOT to either $TEST_TMPDIR or system default.

From our user experience, it would be easier to just disable the check with an new environment variable or to allow PYTEST_DEBUG_TEMPROOT=$env with the special case PYTEST_DEBUG_TEMPROOT=$TMPDIR also being resolved to Python default in the case TMPDIR is missing.

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

No branches or pull requests

2 participants