From c68d27c7c53576dea6c794de12879fb5460c7c52 Mon Sep 17 00:00:00 2001
From: Mike Hendricks <mikeh@blur.com>
Date: Wed, 22 Nov 2023 18:10:13 -0800
Subject: [PATCH] When using `Config.launch` expand environment variables

The variables being passed to subprocess are expected to be expanded.
---
 hab/formatter.py        | 16 +++++++++++-----
 hab/parsers/hab_base.py |  3 ++-
 tests/test_formatter.py | 36 ++++++++++++++++++++++++------------
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/hab/formatter.py b/hab/formatter.py
index e21dce4..c853046 100644
--- a/hab/formatter.py
+++ b/hab/formatter.py
@@ -9,7 +9,8 @@ class Formatter(string.Formatter):
 
     Adds support for the "!e" conversion flag. This will fill in the key as a properly
     formatted environment variable specifier. For example ''{PATH!e}'' will be converted
-    to ``$PATH`` for the sh language, and ``%env:PATH`` for the ps language.
+    to ``$PATH`` for the sh language, and ``%env:PATH`` for the ps language. You can
+    convert "!e" to "!s" by setting expand to True. This simulates `os.path.expandvars`.
 
     This also converts ``{;}`` to the language specific path separator for environment
     variables. On linux this is ``:`` on windows(even in bash) this is ``;``.
@@ -50,19 +51,24 @@ class Formatter(string.Formatter):
         },
     }
 
-    def __init__(self, language):
+    def __init__(self, language, expand=False):
         super().__init__()
         self.language = self.language_from_ext(language)
         self.current_field_name = None
+        self.expand = expand
 
     def convert_field(self, value, conversion):
         if conversion == "e":
+            # Expand the env var to the real string value simulating `os.path.expandvars`
+            if self.expand:
+                return super().convert_field(value, "s")
+
+            # Otherwise insert the correct shell script env var reference
             return self.shell_formats[self.language]["env_var"].format(
                 self.current_field_name
             )
-        else:
-            ret = super().convert_field(value, conversion)
-        return ret
+
+        return super().convert_field(value, conversion)
 
     def get_field(self, field_name, args, kwargs):
         self.current_field_name = field_name
diff --git a/hab/parsers/hab_base.py b/hab/parsers/hab_base.py
index 7fc480d..e929061 100644
--- a/hab/parsers/hab_base.py
+++ b/hab/parsers/hab_base.py
@@ -664,7 +664,8 @@ def update_environ(self, env, alias_name=None, include_global=True, formatter=No
         """
         ext = utils.Platform.default_ext()
         if formatter is None:
-            formatter = Formatter(ext)
+            # Make sure to expand environment variables when formatting.
+            formatter = Formatter(ext, expand=True)
 
         def _apply(data):
             for key, value in data.items():
diff --git a/tests/test_formatter.py b/tests/test_formatter.py
index 451c035..8758b81 100644
--- a/tests/test_formatter.py
+++ b/tests/test_formatter.py
@@ -1,23 +1,35 @@
+import os
+
+import pytest
+
 from hab import utils
 from hab.formatter import Formatter
 from hab.parsers import Config
 
 
-def test_e_format():
-    assert Formatter("sh").format("-{PATH!e}-") == "-$PATH-"
-    assert Formatter("sh").format("-{;}-") == "-:-"
-    # Bash formatting is different on windows for env vars
-    assert Formatter("shwin").format("-{PATH!e}-") == "-$PATH-"
-    assert Formatter("shwin").format("-{;}-") == "-:-"
+@pytest.mark.parametrize(
+    "language,shell,pathsep",
+    (
+        ("sh", "-$PATH-", "-:-"),
+        # Bash formatting is different on windows for env vars
+        ("shwin", "-$PATH-", "-:-"),
+        ("ps", "-$env:PATH-", "-;-"),
+        ("batch", "-%PATH%-", "-;-"),
+        (None, "-{PATH!e}-", "-{;}-"),
+    ),
+)
+def test_e_format(language, shell, pathsep):
+    """Check that "{VAR_NAME!e}" is properly formatted."""
+    path = os.environ["PATH"]
 
-    assert Formatter("ps").format("-{PATH!e}-") == "-$env:PATH-"
-    assert Formatter("ps").format("-{;}-") == "-;-"
+    # Check that "!e" is converted to the correct shell specific specifier.
+    assert Formatter(language).format("-{PATH!e}-") == shell
 
-    assert Formatter("batch").format("-{PATH!e}-") == "-%PATH%-"
-    assert Formatter("batch").format("-{;}-") == "-;-"
+    # Check that "!e" uses the env var value if `expand=True` not the shell specifier.
+    assert Formatter(language, expand=True).format("-{PATH!e}-") == f"-{path}-"
 
-    assert Formatter(None).format("-{PATH!e}-") == "-{PATH!e}-"
-    assert Formatter(None).format("-{;}-") == "-{;}-"
+    # Check that the pathsep variable `{;}` is converted to the correct value
+    assert Formatter(language).format("-{;}-") == pathsep
 
 
 def test_language_from_ext(monkeypatch):