Pass command arguments to subprocess.run as a list, rather than strin…

…g for maximum compatibility across different platforms
paulfioravanti · Oct 10, 2024 · 0d4e808 · 0d4e808
1 parent 7cc6a72
commit 0d4e808
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 55 deletions.
diff --git a/setup.cfg b/setup.cfg
@@ -16,7 +16,7 @@ long_description = file: README.md
 long_description_content_type = text/markdown
 name = plover_local_env_var
 url = https://github.com/paulfioravanti/plover-local-env-var
-version = 0.3.8
+version = 0.3.9
 
 [options]
 install_requires =

diff --git a/src/plover_local_env_var/config/loader.py b/src/plover_local_env_var/config/loader.py
@@ -16,7 +16,7 @@
 
 
 def load(
-    shell_command: Callable[[str], str],
+    shell_command: Callable[[str], list[str]],
     config_filepath: Path
 ) -> dict[str, str]:
     """

diff --git a/src/plover_local_env_var/env_var/command.py b/src/plover_local_env_var/env_var/command.py
@@ -9,19 +9,22 @@
 from typing import Callable
 
 
-_POWERSHELL_COMMAND: Callable[[str], str] = lambda env_var: (
-    "powershell -command "
-    f"\"$ExecutionContext.InvokeCommand.ExpandString({env_var})\""
+_POWERSHELL_COMMAND: Callable[[str], list[str]] = lambda env_var: (
+    [
+        "powershell",
+        "-command",
+        f"$ExecutionContext.InvokeCommand.ExpandString(\"{env_var}\")"
+    ]
 )
 # NOTE: Using an interactive mode command (bash/zsh/fish -ic) seemed to be
 # the only way to access a user's env vars on a Mac outside Plover's
 # environment.
-_SHELL_COMMAND: Callable[[str], Callable[[str], str]] = lambda shell: (
-    lambda env_var: f"{shell} -ic 'echo {env_var}'"
+_SHELL_COMMAND: Callable[[str], Callable[[str], list[str]]] = lambda shell: (
+    lambda env_var: [f"{shell}", "-ic", f"echo {env_var}"]
 )
 _DEFAULT_SHELL: str = "bash"
 
-def resolve_command() -> Callable[[str], str]:
+def resolve_command() -> Callable[[str], list[str]]:
     """
     Resolves a shell command for a given platform.
     """
@@ -33,19 +36,18 @@ def resolve_command() -> Callable[[str], str]:
     )
 
 def run_command(
-    shell_command_resolver: Callable[[str], str],
+    shell_command_resolver: Callable[[str], list[str]],
     target: str
 ) -> str:
     """
     Runs a provided shell command against target in a subprocess.
     """
-    command: str = shell_command_resolver(target)
+    command: list[str] = shell_command_resolver(target)
     result: str = subprocess.run(
         command,
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     ).stdout.strip()
 
     return result
diff --git a/src/plover_local_env_var/env_var/expander.py b/src/plover_local_env_var/env_var/expander.py
@@ -15,7 +15,7 @@
 _ENV_VAR: Pattern[str] = re.compile(r"(\$[A-Za-z_][A-Za-z_0-9]*)")
 _VAR_DIVIDER: str = "##"
 
-def expand(shell_command_resolver: Callable[[str], str], var: str) -> str:
+def expand(shell_command_resolver: Callable[[str], list[str]], var: str) -> str:
     """
     Fetches and returns a single local env var value.
 
@@ -30,7 +30,7 @@ def expand(shell_command_resolver: Callable[[str], str], var: str) -> str:
     return expanded
 
 def expand_list(
-    shell_command_resolver: Callable[[str], str],
+    shell_command_resolver: Callable[[str], list[str]],
     env_var_name_list: list[str]
 ) -> dict[str, str]:
     """
@@ -58,7 +58,7 @@ def expand_list(
     }
 
 def _perform_expansion(
-    shell_command_resolver: Callable[[str], str],
+    shell_command_resolver: Callable[[str], list[str]],
     target: str
 ) -> str:
     expanded: str = command.run_command(shell_command_resolver, target)

diff --git a/src/plover_local_env_var/extension.py b/src/plover_local_env_var/extension.py
@@ -33,7 +33,7 @@ class LocalEnvVar:
 
     _engine: StenoEngine
     _env_var_values: dict[str, str]
-    _shell_command: Callable[[str], str]
+    _shell_command: Callable[[str], list[str]]
 
     def __init__(self, engine: StenoEngine) -> None:
         self._engine = engine

diff --git a/test/config/test_config.py b/test/config/test_config.py
@@ -39,12 +39,15 @@ def test_expanding_existing_env_vars_on_windows(
 
     assert loaded_config == {"$ENV:BAR": "baz", "$ENV:FOO": "quux"}
     spy.assert_called_once_with(
-        "powershell -command "
-        "\"$ExecutionContext.InvokeCommand.ExpandString($ENV:BAR##$ENV:FOO)\"",
+        [
+            "powershell",
+            "-command",
+            "$ExecutionContext.InvokeCommand.ExpandString("
+            "\"$ENV:BAR##$ENV:FOO\")"
+        ],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
     # No change to original config file
@@ -72,11 +75,10 @@ def test_expanding_existing_env_vars_on_mac_or_linux(
 
     assert loaded_config == {"$BAR": "baz", "$FOO": "quux"}
     spy.assert_called_once_with(
-        "bash -ic 'echo $BAR##$FOO'",
+        ["bash", "-ic", "echo $BAR##$FOO"],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
     # No change to original config file
@@ -104,12 +106,15 @@ def test_expanding_non_existent_env_vars_on_windows(
 
     assert loaded_config == {}
     spy.assert_called_once_with(
-        "powershell -command "
-        "\"$ExecutionContext.InvokeCommand.ExpandString($ENV:BAR##$ENV:FOO)\"",
+        [
+            "powershell",
+            "-command",
+            "$ExecutionContext.InvokeCommand.ExpandString("
+            "\"$ENV:BAR##$ENV:FOO\")"
+        ],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
     # Original config file has been blanked out
@@ -137,11 +142,10 @@ def test_expanding_non_existent_env_vars_on_mac_or_linux(
 
     assert loaded_config == {}
     spy.assert_called_once_with(
-        "bash -ic 'echo $BAR##$FOO'",
+        ["bash", "-ic", "echo $BAR##$FOO"],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
     # Original config file has been blanked out
@@ -169,12 +173,15 @@ def test_expanding_some_existing_env_vars_on_windows(
 
     assert loaded_config == {"$ENV:BAR": "baz"}
     spy.assert_called_once_with(
-        "powershell -command "
-        "\"$ExecutionContext.InvokeCommand.ExpandString($ENV:BAR##$ENV:FOO)\"",
+        [
+            "powershell",
+            "-command",
+            "$ExecutionContext.InvokeCommand.ExpandString("
+            "\"$ENV:BAR##$ENV:FOO\")"
+        ],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
     # Original config file has had null variable BAR removed from it
@@ -202,11 +209,10 @@ def test_expanding_some_existing_env_vars_on_mac_or_linux(
 
     assert loaded_config == {"$FOO": "baz"}
     spy.assert_called_once_with(
-        "bash -ic 'echo $BAR##$FOO'",
+        ["bash", "-ic", "echo $BAR##$FOO"],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
     # Original config file has had null variable BAR removed from it

diff --git a/test/conftest.py b/test/conftest.py
@@ -3,13 +3,16 @@
 
 @pytest.fixture
 def bash_command():
-    return lambda env_var: f"bash -ic 'echo {env_var}'"
+    return lambda env_var: ["bash", "-ic", f"echo {env_var}"]
 
 @pytest.fixture
 def powershell_command():
     return lambda env_var: (
-        "powershell -command "
-        f"\"$ExecutionContext.InvokeCommand.ExpandString({env_var})\""
+        [
+            "powershell",
+            "-command",
+            f"$ExecutionContext.InvokeCommand.ExpandString(\"{env_var}\")"
+        ]
     )
 
 # NOTE: Given that the command passed in to `subprocess.run` will be different

diff --git a/test/env_var/conftest.py b/test/env_var/conftest.py
@@ -1,8 +1,9 @@
 import pytest
 
+
 @pytest.fixture
 def shell_command():
     def _method(shell):
-        return lambda env_var: f"{shell} -ic 'echo {env_var}'"
+        return lambda env_var: [f"{shell}", "-ic", f"echo {env_var}"]
 
     return _method
diff --git a/test/env_var/test_env_var.py b/test/env_var/test_env_var.py
@@ -43,11 +43,10 @@ def test_no_value_for_var_found_on_mac_on_linux(
         env_var.expand(bash_command, "$FOO")
 
     spy.assert_called_once_with(
-        "bash -ic 'echo $FOO'",
+        ["bash", "-ic", "echo $FOO"],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
 def test_no_value_for_var_found_on_windows(
@@ -65,12 +64,14 @@ def test_no_value_for_var_found_on_windows(
         env_var.expand(powershell_command, "$ENV:FOO")
 
     spy.assert_called_once_with(
-        "powershell -command "
-        "\"$ExecutionContext.InvokeCommand.ExpandString($ENV:FOO)\"",
+        [
+            "powershell",
+            "-command",
+            "$ExecutionContext.InvokeCommand.ExpandString(\"$ENV:FOO\")"
+        ],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
 def test_returns_expanded_value_of_found_env_var_on_mac_or_linux(
@@ -83,11 +84,10 @@ def test_returns_expanded_value_of_found_env_var_on_mac_or_linux(
 
     assert env_var.expand(bash_command, "$FOO") == "Bar"
     spy.assert_called_once_with(
-        "bash -ic 'echo $FOO'",
+        ["bash", "-ic", "echo $FOO"],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )
 
 def test_returns_expanded_value_of_found_env_var_on_windows(
@@ -100,10 +100,12 @@ def test_returns_expanded_value_of_found_env_var_on_windows(
 
     assert env_var.expand(powershell_command, "$ENV:FOO") == "Bar"
     spy.assert_called_once_with(
-        "powershell -command "
-        "\"$ExecutionContext.InvokeCommand.ExpandString($ENV:FOO)\"",
+        [
+            "powershell",
+            "-command",
+            "$ExecutionContext.InvokeCommand.ExpandString(\"$ENV:FOO\")"
+        ],
         capture_output=True,
         check=False,
-        encoding="utf-8",
-        shell=True
+        encoding="utf-8"
     )