diff options
author | Harmen Stoppels <harmenstoppels@gmail.com> | 2022-03-23 15:54:55 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-23 08:54:55 -0600 |
commit | c300b9204778a4ed21f32d9d77dc11a17c4f46a0 (patch) | |
tree | 8f3f4087af327ffbf5cd313aa48e3f0fd67a90b4 /lib | |
parent | f41c3a0fe96df09e3d6887dbeda6cb822e32e7f5 (diff) | |
download | spack-c300b9204778a4ed21f32d9d77dc11a17c4f46a0.tar.gz spack-c300b9204778a4ed21f32d9d77dc11a17c4f46a0.tar.bz2 spack-c300b9204778a4ed21f32d9d77dc11a17c4f46a0.tar.xz spack-c300b9204778a4ed21f32d9d77dc11a17c4f46a0.zip |
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 /`.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/environment/environment.py | 10 | ||||
-rw-r--r-- | lib/spack/spack/test/cmd/env.py | 11 |
2 files changed, 19 insertions, 2 deletions
diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index a2f37c9e1b..4feecfab53 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -541,8 +541,14 @@ class ViewDescriptor(object): raise SpackEnvironmentViewError(msg) rename(tmp_symlink_name, self.root) - # remove old_root - if old_root and os.path.exists(old_root): + # Remove the old root when it's in the same folder as the new root. This guards + # 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: shutil.rmtree(old_root) except (IOError, OSError) as e: diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 868eb8ec45..cf2c947bdc 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -2735,3 +2735,14 @@ def test_activate_temp(monkeypatch, tmpdir): if ev.spack_env_var in line) assert str(tmpdir) in active_env_var 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) |