environment: be more defensive when deleting roots for old views (#29636)
Currently `old_root` is computed by reading the symlink at `self.root`. We should be more defensive in removing it by checking that it is in the same directory as the new root. Otherwise, in the worst case, when someone runs `spack env create --with-view=./view -d .` and `view` already exists and is a symlink to `/`, Spack effectively runs `rm -rf /`.
This commit is contained in:
parent
f41c3a0fe9
commit
c300b92047
2 changed files with 19 additions and 2 deletions
|
@ -541,8 +541,14 @@ def regenerate(self, concretized_specs):
|
||||||
raise SpackEnvironmentViewError(msg)
|
raise SpackEnvironmentViewError(msg)
|
||||||
rename(tmp_symlink_name, self.root)
|
rename(tmp_symlink_name, self.root)
|
||||||
|
|
||||||
# remove old_root
|
# Remove the old root when it's in the same folder as the new root. This guards
|
||||||
if old_root and os.path.exists(old_root):
|
# against removal of an arbitrary path when the original symlink in self.root
|
||||||
|
# was not created by the environment, but by the user.
|
||||||
|
if (
|
||||||
|
old_root and
|
||||||
|
os.path.exists(old_root) and
|
||||||
|
os.path.samefile(os.path.dirname(new_root), os.path.dirname(old_root))
|
||||||
|
):
|
||||||
try:
|
try:
|
||||||
shutil.rmtree(old_root)
|
shutil.rmtree(old_root)
|
||||||
except (IOError, OSError) as e:
|
except (IOError, OSError) as e:
|
||||||
|
|
|
@ -2735,3 +2735,14 @@ def test_activate_temp(monkeypatch, tmpdir):
|
||||||
if ev.spack_env_var in line)
|
if ev.spack_env_var in line)
|
||||||
assert str(tmpdir) in active_env_var
|
assert str(tmpdir) in active_env_var
|
||||||
assert ev.is_env_dir(str(tmpdir))
|
assert ev.is_env_dir(str(tmpdir))
|
||||||
|
|
||||||
|
|
||||||
|
def test_env_view_fail_if_symlink_points_elsewhere(tmpdir, install_mockery, mock_fetch):
|
||||||
|
view = str(tmpdir.join('view'))
|
||||||
|
# Put a symlink to an actual directory in view
|
||||||
|
non_view_dir = str(tmpdir.mkdir('dont-delete-me'))
|
||||||
|
os.symlink(non_view_dir, view)
|
||||||
|
with ev.create('env', with_view=view):
|
||||||
|
add('libelf')
|
||||||
|
install('--fake')
|
||||||
|
assert os.path.isdir(non_view_dir)
|
||||||
|
|
Loading…
Reference in a new issue