Skip to content

🐛 fix(create): reject an empty destination instead of using the working directory - #3491

Open
darrenhuai wants to merge 2 commits into
pypa:mainfrom
darrenhuai:fix/reject-empty-destination
Open

darrenhuai wants to merge 2 commits into
pypa:mainfrom
darrenhuai:fix/reject-empty-destination

Conversation

@darrenhuai

Copy link
Copy Markdown
Contributor

If the destination argument is empty, virtualenv builds the environment in the current directory, and with --clear it deletes everything there first. The usual way to end up with one is a variable that isn't set. Here it is on Linux with Python 3.12 and main at 861cff7 (run as python3 -m virtualenv from that checkout), in a throwaway folder:

$ ls -A
IMPORTANT.txt
src
$ virtualenv --clear --no-seed "$VENV"
created virtual environment CPython3.12.3.final.0-64-x86_64 in 2167ms
  creator CPython3Posix(dest=/tmp/tmp.gj1SZc4kIL/project, clear=True, no_vcs_ignore=False, venv_redirect=None, global=False)
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator,XonshActivator
$ ls -A
$ cd . && ls -A
.gitignore
CACHEDIR.TAG
bin
include
lib
pyvenv.cfg

IMPORTANT.txt and src are gone. The first ls -A prints nothing because the folder itself was removed and made again under the shell. Without --clear nothing gets deleted, but the same command still writes bin, lib, pyvenv.cfg, CACHEDIR.TAG and, when the folder has none, a .gitignore holding * straight into the project. On Windows a destination made only of spaces acts like an empty one, because Windows drops trailing spaces and dots from path names. In a Windows probe with safe_delete swapped for a recorder, "" and " " with --clear both handed it the working directory.

The cause is in Creator.validate_dest. It builds Path(raw_value) and then Path(os.path.abspath(...)).resolve(), and since Path("") is Path("."), the empty string comes back as the working directory. That directory exists and is writable, so every later check passes, and Creator.run calls safe_delete(self.dest) when --clear is set. The --clear guard from #3422 only refuses symlink and reparse point destinations, and #3426 only adds the line boundary check for the venv creator, so neither catches this. It is the same problem #3422 went after: --clear removing a folder the user never named.

validate_dest now refuses an empty or whitespace-only destination before its other checks. The same run on this branch, usage lines trimmed:

$ virtualenv --clear --no-seed "$VENV"
virtualenv: error: argument dest: destination '' must not be empty or whitespace; pass . to use the current directory
SystemExit: 2
$ ls -A
IMPORTANT.txt
src

. still works, with or without --clear, so you can create or clear an environment in the current directory when you ask for it. Venv.validate_dest calls the base method, so every creator gets the check. Whitespace-only names are refused on POSIX too, even though " " is a legal folder name there. Refusing them only on Windows would make one command line mean different things per platform, and a name that is nothing but spaces is far more likely a quoting slip than a choice. If you'd rather keep " " working on POSIX I can narrow it.

On Windows, names made only of dots and spaces, such as ... or . ., also resolve to the working directory. They aren't blank, though, and refusing them would mean copying Windows' own name normalization, so I kept this PR to empty and whitespace-only values. Stdlib python -m venv --clear "" has the same problem, but virtualenv already checks the destination more strictly than venv does (#3399, #3426), and this is one more of those checks.

test_blank_destination_leaves_working_directory swaps safe_delete for a recorder and runs cli_run from inside tmp_path on "" (alone, with --clear and with VIRTUALENV_CLEAR=1), on " " with --clear, and on "\t". Each case has to exit with code 2, print the new message, never reach safe_delete, and leave the folder holding only its marker file. test_dot_destination_is_working_directory checks that . --clear still resolves to the working directory. All five blank cases fail on main.

Thanks for contributing, make sure you address all the checklists (for details on how see development documentation)

  • ran the linter to address style issues (tox -e fix). I ran the same hooks through prek on the changed files; the semgrep hook only looks at activate.sh.

  • wrote descriptive pull request text

  • ensured there are test(s) validating the fix

  • added a docs/changelog/+short-description.<type>.rst fragment for each user-facing change, or applied
    skip news

  • updated/extended the documentation (no page covers the destination beyond its help text; I updated the validate_dest docstring)

  • for changes to activation scripts, pyvenv.cfg, wheel downloads, app-data or release workflows (none of these change; the fix only refuses a blank destination argument before anything is written)

  • ran tests on Python 3.9 and 3.15 (tox run -e 3.9,3.15). I ran test_creator.py on 3.9 and left the full 3.9 and 3.15 runs to CI.

  • linked upstream issues beside any temporary interpreter overrides (none)

  • kept the PR in draft until CI passes

…ng directory

validate_dest accepted an empty destination and resolved it to the
working directory, because Path("") is "." and nothing checked for it.
On Windows a destination made only of spaces did the same, since the
path API drops trailing spaces. The usual way to pass one is an unset
shell variable, as in virtualenv --clear "$VENV". Without --clear that
writes bin, lib, pyvenv.cfg and a catch-all .gitignore into the project
folder. With --clear, Creator.run hands the working directory to
safe_delete and everything in it is gone before the environment is
built.

validate_dest now refuses an empty or whitespace-only destination before
any other check, and the error says to pass . for the current directory,
which still works as before. Whitespace is refused on POSIX too, where a
folder named " " is legal, so the same command behaves the same on every
platform; such a name is far more likely a quoting slip than a choice.
@darrenhuai
darrenhuai marked this pull request as ready for review October 29, 2026 15:07

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

1 participant