tmpdir: prevent symlink attacks and TOCTOU races (CVE-2025-71176)

[laurac8r]

Summary

Replace the predictable pytest-of-<user> rootdir with a randomly-named rootdir created via tempfile.mkdtemp to prevent the entire class of predictable-name attacks (symlink races, DoS via pre-created files/dirs). As defense-in-depth, open the rootdir with os.open using O_NOFOLLOW and O_DIRECTORY flags and perform ownership/permission checks via fd-based fstat/fchmod to eliminate TOCTOU races.

Fixes CVE-2025-71176.

closes #13669

Changes

• Randomly-named rootdir: Use tempfile.mkdtemp(prefix=f"pytest-of-{user}-", dir=temproot) instead of a fixed pytest-of-{user} directory, making pre-creation attacks infeasible.
• _safe_open_dir context manager: Opens the rootdir with O_NOFOLLOW | O_DIRECTORY (where available), yields the fd, and guarantees cleanup. Symlinks are rejected at the os.open level.
• fd-based ownership & permission checks: fstat and fchmod operate on the open file descriptor, eliminating the TOCTOU window between stat/chmod and the actual directory.
• _cleanup_old_rootdirs: Garbage-collects stale randomly-named rootdirs from previous sessions, respecting the retention count. The current session's rootdir is never removed.
• Comprehensive tests: Symlink rejection, wrong-owner rejection, missing O_NOFOLLOW/O_DIRECTORY fallback, predictable-path DoS immunity, _cleanup_old_rootdirs behavior, _safe_open_dir fd lifecycle, config validation, and session-finish edge cases.

---

• Include documentation when adding new features.
• Include new tests or update existing tests when applicable.
• Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.

If this change fixes an issue, please:

• Add text like closes #XYZW to the PR description and/or commits (where XYZW is the issue number). See the github docs for more information.

Important

Unsupervised agentic contributions are not accepted. See our AI/LLM-Assisted Contributions Policy.

• If AI agents were used, they are credited in Co-authored-by commit trailers.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

• Create a new changelog file in the changelog directory, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.

  Write sentences in the past or present tense, examples:

  • Improved verbose diff output with sequences.
  • Terminal summary statistics now use multiple colors.

  Also make sure to end the sentence with a ..

• Add yourself to AUTHORS in alphabetical order.

[orlitzky]

This certainly looks like an improvement, but it's still possible for any user to DoS the machine until a reboot with for x in $(cat /etc/passwd | cut -f1 -d:); do touch /tmp/pytest-of-$x; done;.

[bluetech]

Not a proper review, but some comments from a quick read.

[bluetech]

I don't think that just because some poor soul happens to be using pytest on a system where avoids_symlink_attacks is false, we should warn them every time. There's nothing they can do about it after all.

[orlitzky]

I think all recent (>= POSIX 2008) platforms should be safe, so maybe it is a better idea to bail out here rather than run an unsafe recursive delete on the user's behalf? (In other words, let them do it, so pytest can't be blamed.) With the understanding that this will rarely happen on real systems.

[bluetech]

For me os.shutil already refuses a symlink argument. Is this check needed on our side then?

[bluetech]

Seems excessive to me. mkdtemp documentation already says "Creates a temporary directory in the most secure manner possible", why do we need to double check it?

[bluetech]

The previous code comment said "use a sub-directory in the temproot to speed-up make_numbered_dir() call", and that's relevant here -- now this call will do a full scandir of /tmp on every cleanup, and the problem is that /tmp can accumulate a bunch of garbage via various programs, and seems like the previous code tried to avoid scanning it by using a sub-directory.

[nicoddemus]

Good catch. 👍

This PR now creates this structure:

/tmp/
  pytest-of-user-kjnasjnd/
  pytest-of-user-deadbeef/
  pytest-of-user-basu1234/
  pytest-of-user-olsdanj1/

If instead it created this:

/tmp/
  pytest-of-user/
    kjnasjnd/
    deadbeef/
    basu1234/
    olsdanj1/

Would that still work to prevent the security concerns this PR addresses?

I'm guessing the fact that an attack can create files in /tmp/pytest-of-user would still be a problem?