Revert "Change environment modifications to escape with double quotes (#36789)" (#42780)

This reverts commit 690394fabc, as it causes arbitrary code execution.

lib/spack/spack/test/util/environment.py

@@ -160,22 +160,13 @@ def test_reverse_environment_modifications(working_env):
     assert os.environ == start_env
 
 
-def test_escape_double_quotes_in_shell_modifications():
+def test_shell_modifications_are_properly_escaped():
-    to_validate = envutil.EnvironmentModifications()
+    """Test that variable values are properly escaped so that they can safely be eval'd."""
+    changes = envutil.EnvironmentModifications()
+    changes.set("VAR", "$PATH")
+    changes.append_path("VAR", "$ANOTHER_PATH")
+    changes.set("RM_RF", "$(rm -rf /)")
 
-    to_validate.set("VAR", "$PATH")
+    script = changes.shell_modifications(shell="sh")
-    to_validate.append_path("VAR", "$ANOTHER_PATH")
+    assert f"export VAR='$PATH{os.pathsep}$ANOTHER_PATH'" in script
-
+    assert "export RM_RF='$(rm -rf /)'" in script
-    to_validate.set("QUOTED_VAR", '"MY_VAL"')
-
-    if sys.platform == "win32":
-        cmds = to_validate.shell_modifications(shell="bat")
-        assert r'set "VAR=$PATH;$ANOTHER_PATH"' in cmds
-        assert r'set "QUOTED_VAR="MY_VAL"' in cmds
-        cmds = to_validate.shell_modifications(shell="pwsh")
-        assert "$Env:VAR='$PATH;$ANOTHER_PATH'" in cmds
-        assert "$Env:QUOTED_VAR='\"MY_VAL\"'" in cmds
-    else:
-        cmds = to_validate.shell_modifications()
-        assert 'export VAR="$PATH:$ANOTHER_PATH"' in cmds
-        assert r'export QUOTED_VAR="\"MY_VAL\""' in cmds

lib/spack/spack/util/environment.py

@@ -11,6 +11,7 @@
 import os.path
 import pickle
 import re
+import shlex
 import sys
 from functools import wraps
 from typing import Any, Callable, Dict, List, MutableMapping, Optional, Tuple, Union
@@ -63,26 +64,6 @@
 ModificationList = List[Union["NameModifier", "NameValueModifier"]]
 
 
-_find_unsafe = re.compile(r"[^\w@%+=:,./-]", re.ASCII).search
-
-
-def double_quote_escape(s):
-    """Return a shell-escaped version of the string *s*.
-
-    This is similar to how shlex.quote works, but it escapes with double quotes
-    instead of single quotes, to allow environment variable expansion within
-    quoted strings.
-    """
-    if not s:
-        return '""'
-    if _find_unsafe(s) is None:
-        return s
-
-    # use double quotes, and escape double quotes in the string
-    # the string $"b is then quoted as "$\"b"
-    return '"' + s.replace('"', r"\"") + '"'
-
-
 def system_env_normalize(func):
     """Decorator wrapping calls to system env modifications,
     converting all env variable names to all upper case on Windows, no-op
@@ -182,7 +163,7 @@ def _nix_env_var_to_source_line(var: str, val: str) -> str:
             fname=BASH_FUNCTION_FINDER.sub(r"\1", var), decl=val
         )
     else:
-        source_line = f"{var}={double_quote_escape(val)}; export {var}"
+        source_line = f"{var}={shlex.quote(val)}; export {var}"
     return source_line
 
 
@@ -691,11 +672,10 @@ def shell_modifications(
                 if new is None:
                     cmds += _SHELL_UNSET_STRINGS[shell].format(name)
                 else:
-                    if sys.platform != "win32":
+                    value = new_env[name]
-                        new_env_name = double_quote_escape(new_env[name])
+                    if shell not in ("bat", "pwsh"):
-                    else:
+                        value = shlex.quote(value)
-                        new_env_name = new_env[name]
+                    cmd = _SHELL_SET_STRINGS[shell].format(name, value)
-                    cmd = _SHELL_SET_STRINGS[shell].format(name, new_env_name)
                     cmds += cmd
         return cmds