From 0d4e808550d78764045ad74ae46ea11eb6ec8c5b Mon Sep 17 00:00:00 2001 From: Paul Fioravanti Date: Thu, 10 Oct 2024 18:12:38 +1100 Subject: [PATCH] Pass command arguments to subprocess.run as a list, rather than string for maximum compatibility across different platforms --- setup.cfg | 2 +- src/plover_local_env_var/config/loader.py | 2 +- src/plover_local_env_var/env_var/command.py | 22 +++++---- src/plover_local_env_var/env_var/expander.py | 6 +-- src/plover_local_env_var/extension.py | 2 +- test/config/test_config.py | 48 +++++++++++--------- test/conftest.py | 9 ++-- test/env_var/conftest.py | 3 +- test/env_var/test_env_var.py | 30 ++++++------ 9 files changed, 69 insertions(+), 55 deletions(-) diff --git a/setup.cfg b/setup.cfg index ee34b2b..7be031f 100644 --- 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 index 58c370a..7c5cbec 100644 --- 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 index 1016b0c..d812961 100644 --- 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 index 44088c5..bdf890c 100644 --- 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 index 4a8802c..fd056f0 100644 --- 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 index 227bbae..3f03db9 100644 --- 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 index 69d01af..c73a5b5 100644 --- 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 index 86aa70c..df7b1fe 100644 --- 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 index dfb3f80..e6e9095 100644 --- 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" )