Skip to content

Commit

Permalink
Avoid deleting a project environment directory if we cannot tell if a…
Browse files Browse the repository at this point in the history
… `pyvenv.cfg` file exists (astral-sh#8012)

I was exploring a fix to an [apparent
bug](https://github.com/astral-sh/uv/actions/runs/11240101202/job/31248937280?pr=8010)
but this was actually just a CI change
astral-sh#8013

Regardless, I think this code is safer?
  • Loading branch information
zanieb authored Oct 8, 2024
1 parent 651fe6f commit 5bc726a
Showing 1 changed file with 31 additions and 15 deletions.
46 changes: 31 additions & 15 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,24 +525,40 @@ pub(crate) async fn get_or_init_environment(
let venv = workspace.venv();

// Avoid removing things that are not virtual environments
if venv.exists() && !venv.join("pyvenv.cfg").exists() {
return Err(ProjectError::InvalidProjectEnvironmentDir(
venv,
"it is not a compatible environment but cannot be recreated because it is not a virtual environment".to_string(),
));
}
let should_remove = match (venv.try_exists(), venv.join("pyvenv.cfg").try_exists()) {
// It's a virtual environment we can remove it
(_, Ok(true)) => true,
// It doesn't exist at all, we should use it without deleting it to avoid TOCTOU bugs
(Ok(false), Ok(false)) => false,
// If it's not a virtual environment, bail
(Ok(true), Ok(false)) => {
return Err(ProjectError::InvalidProjectEnvironmentDir(
venv,
"it is not a compatible environment but cannot be recreated because it is not a virtual environment".to_string(),
));
}
// Similarly, if we can't _tell_ if it exists we should bail
(_, Err(err)) | (Err(err), _) => {
return Err(ProjectError::InvalidProjectEnvironmentDir(
venv,
format!("it is not a compatible environment but cannot be recreated because uv cannot determine if it is a virtual environment: {err}"),
));
}
};

// Remove the existing virtual environment if it doesn't meet the requirements.
match fs_err::remove_dir_all(&venv) {
Ok(()) => {
writeln!(
printer.stderr(),
"Removed virtual environment at: {}",
venv.user_display().cyan()
)?;
if should_remove {
match fs_err::remove_dir_all(&venv) {
Ok(()) => {
writeln!(
printer.stderr(),
"Removed virtual environment at: {}",
venv.user_display().cyan()
)?;
}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
Err(e) => return Err(e.into()),
}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
Err(e) => return Err(e.into()),
}

writeln!(
Expand Down

0 comments on commit 5bc726a

Please sign in to comment.