feat: Replace eval with ${VAR} env expansion in Target

python/gvtest/targets.py

@@ -1,7 +1,8 @@
 #!/usr/bin/env python3
 
 #
-# Copyright (C) 2023 ETH Zurich, University of Bologna and GreenWaves Technologies
+# Copyright (C) 2023 ETH Zurich, University of Bologna
+#     and GreenWaves Technologies
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -11,25 +12,43 @@
 #
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# permissions and limitations under the License.
 #
 
 """
-Target configuration — platform targets with properties, env vars, and sourceme scripts.
+Target configuration — platform targets with properties,
+env vars, and sourceme scripts.
+
+Env var expansion: Use ``${VAR}`` in sourceme and envvars
+values to expand environment variables at runtime.
+Missing variables expand to an empty string.
 """
 
 from __future__ import annotations
 
-import ast
 import json
-from typing import Any, Optional
+import os
+import re
+from typing import Any
+
+
+_ENV_VAR_RE = re.compile(r'\$\{([^}]+)\}')
+
+
+def _expand_env(value: str) -> str:
+    """Replace all ``${VAR}`` references with their env value."""
+    return _ENV_VAR_RE.sub(
+        lambda m: os.environ.get(m.group(1), ''), value
+    )
 
 
 class Target(object):
 
-    def __init__(self, name: str, config: str | None = None) -> None:
+    def __init__(
+        self, name: str, config: str | None = None
+    ) -> None:
         self.name: str = name
         if config is None:
             config = '{}'
@@ -40,27 +59,18 @@ def get_name(self) -> str:
 
     def get_sourceme(self) -> str | None:
         sourceme = self.config.get('sourceme')
-
         if sourceme is not None:
-            return ast.literal_eval(sourceme)
-
+            return _expand_env(sourceme)
         return None
 
     def get_envvars(self) -> dict[str, str] | None:
         envvars = self.config.get('envvars')
-
         if envvars is not None:
             result: dict[str, str] = {}
             for key, value in envvars.items():
-                try:
-                    eval_value = ast.literal_eval(value)
-                    if eval_value is None:
-                        eval_value = ""
-                    result[key] = eval_value
-                except:
-                    result[key] = ""
+                expanded = _expand_env(value)
+                result[key] = expanded
             return result
-
         return None
 
     def format_properties(self, str: str) -> str:

tests/test_target.py

@@ -3,6 +3,7 @@
 """
 
 import json
+import os
 import pytest
 from gvtest.runner import Target
 
@@ -38,10 +39,28 @@ def test_no_sourceme(self):
         assert t.get_sourceme() is None
 
     def test_with_sourceme(self):
-        config = json.dumps({'sourceme': "'my_env.sh'"})
+        config = json.dumps({'sourceme': 'my_env.sh'})
         t = Target('rv64', config)
         assert t.get_sourceme() == 'my_env.sh'
 
+    def test_sourceme_env_expansion(self, monkeypatch):
+        """${VAR} in sourceme should expand from environment."""
+        monkeypatch.setenv('SDK_HOME', '/opt/sdk')
+        config = json.dumps({
+            'sourceme': '${SDK_HOME}/configs/setup.sh'
+        })
+        t = Target('rv64', config)
+        assert t.get_sourceme() == '/opt/sdk/configs/setup.sh'
+
+    def test_sourceme_missing_env_expands_empty(self, monkeypatch):
+        """Missing env vars expand to empty string."""
+        monkeypatch.delenv('NONEXISTENT_VAR_XYZ', raising=False)
+        config = json.dumps({
+            'sourceme': '${NONEXISTENT_VAR_XYZ}/file.sh'
+        })
+        t = Target('rv64', config)
+        assert t.get_sourceme() == '/file.sh'
+
 
 class TestTargetEnvvars:
     """Tests for target environment variable resolution."""
@@ -50,33 +69,73 @@ def test_no_envvars(self):
         t = Target('rv64')
         assert t.get_envvars() is None
 
-    def test_with_envvars(self):
-        config = json.dumps({'envvars': {'MY_VAR': "'hello'"}})
+    def test_with_envvars_plain(self):
+        """Plain string values without ${} pass through."""
+        config = json.dumps({'envvars': {'MY_VAR': 'hello'}})
         t = Target('rv64', config)
         envvars = t.get_envvars()
         assert envvars == {'MY_VAR': 'hello'}
 
-    def test_envvars_eval_failure(self):
-        """Env vars that fail eval should get empty string."""
-        config = json.dumps({'envvars': {'BAD': 'undefined_variable'}})
+    def test_with_envvars_env_expansion(self, monkeypatch):
+        """${VAR} in envvar values should expand."""
+        monkeypatch.setenv('GCC_PATH', '/usr/local/gcc')
+        config = json.dumps({
+            'envvars': {'TOOLCHAIN': '${GCC_PATH}'}
+        })
         t = Target('rv64', config)
         envvars = t.get_envvars()
-        assert envvars == {'BAD': ''}
-
-    def test_envvars_none_becomes_empty(self):
-        """Env vars that eval to None should become empty string."""
-        config = json.dumps({'envvars': {'NONE_VAR': 'None'}})
+        assert envvars == {'TOOLCHAIN': '/usr/local/gcc'}
+
+    def test_envvars_multiple_expansions(self, monkeypatch):
+        """Multiple ${VAR} refs in one value."""
+        monkeypatch.setenv('BASE', '/opt')
+        monkeypatch.setenv('VER', '2.0')
+        config = json.dumps({
+            'envvars': {'PATH': '${BASE}/tools/${VER}/bin'}
+        })
         t = Target('rv64', config)
         envvars = t.get_envvars()
-        assert envvars == {'NONE_VAR': ''}
-
-    def test_envvars_no_code_execution(self):
-        """Env vars should use literal_eval, not eval — no arbitrary code."""
-        config = json.dumps({'envvars': {'EXPLOIT': '__import__("os").system("echo pwned")'}})
+        assert envvars == {'PATH': '/opt/tools/2.0/bin'}
+
+    def test_envvars_missing_env_expands_empty(self, monkeypatch):
+        """Missing env vars expand to empty string."""
+        monkeypatch.delenv('MISSING_VAR_ABC', raising=False)
+        config = json.dumps({
+            'envvars': {'X': '${MISSING_VAR_ABC}'}
+        })
+        t = Target('rv64', config)
+        envvars = t.get_envvars()
+        assert envvars == {'X': ''}
+
+    def test_envvars_no_code_execution(self, tmp_path):
+        """Code in envvar values is NOT executed."""
+        marker = tmp_path / 'pwned.txt'
+        config = json.dumps({
+            'envvars': {
+                'EXPLOIT': f'__import__("os").system("touch {marker}")'
+            }
+        })
         t = Target('rv64', config)
         envvars = t.get_envvars()
-        # Should fail safely (literal_eval rejects function calls), not execute code
-        assert envvars == {'EXPLOIT': ''}
+        # The value passes through as a literal string
+        assert '__import__' in envvars['EXPLOIT']
+        # And no file was created (code was not executed)
+        assert not marker.exists()
+
+    def test_envvars_path_join_pattern(self, monkeypatch):
+        """Real-world pattern: path building with ${VAR}."""
+        monkeypatch.setenv('PULP_SDK_HOME', '/home/user/sdk')
+        config = json.dumps({
+            'sourceme': '${PULP_SDK_HOME}/configs/pulp-open.sh',
+            'envvars': {
+                'TOOLCHAIN': '${PULP_SDK_HOME}/tools/gcc'
+            }
+        })
+        t = Target('rv64', config)
+        assert t.get_sourceme() == '/home/user/sdk/configs/pulp-open.sh'
+        assert t.get_envvars() == {
+            'TOOLCHAIN': '/home/user/sdk/tools/gcc'
+        }
 
 
 class TestTargetProperties:
@@ -87,9 +146,12 @@ def test_format_no_properties(self):
         assert t.format_properties('hello {name}') == 'hello {name}'
 
     def test_format_with_properties(self):
-        config = json.dumps({'properties': {'name': 'world', 'ver': '2'}})
+        config = json.dumps({
+            'properties': {'name': 'world', 'ver': '2'}
+        })
         t = Target('rv64', config)
-        assert t.format_properties('hello {name} v{ver}') == 'hello world v2'
+        result = t.format_properties('hello {name} v{ver}')
+        assert result == 'hello world v2'
 
     def test_get_property_exists(self):
         config = json.dumps({'properties': {'chip': 'rv64'}})