fix(venv): run venv/installer via spawn_executable, no shell

ensure_venv and dependency installation built shell command strings and
ran them through os:cmd, with quote/1 single-quote wrapping that did not
escape embedded quotes -- a venv path/requirements/extras value could
break out and inject. Run the executables via
open_port({spawn_executable, Exe}, [{args, Args}, ...]) with literal
argument lists instead. For uv, VIRTUAL_ENV is passed via the port
{env, ...} option rather than a shell prefix.

Adds py_venv_SUITE:test_venv_path_metacharacters.

Author: benoitc

CHANGELOG.md

@@ -4,6 +4,11 @@
 
 ### Security
 
+- **No shell for venv/installer commands** - `py:ensure_venv` and dependency
+  installation now run the executables via `open_port({spawn_executable, ...})` with an
+  argument list instead of building a shell string for `os:cmd`. Venv paths, requirement
+  files, and extras are passed literally, so shell metacharacters can't be injected. For
+  `uv`, `VIRTUAL_ENV` is passed via the port `{env, ...}` option rather than a shell prefix.
 - **Bounded shared state + safe stream/log builders** - `py_state` gained an optional
   `max_state_entries` cap (default `infinity`, unchanged behavior) enforced with atomic
   admission so Python-driven `state_set` can't exhaust node memory, and its size counter

src/py.erl

@@ -900,14 +900,13 @@ create_venv(Path, Opts) ->
         undefined -> get_python_executable();
         P -> P
     end,
-    Cmd = case Installer of
+    case Installer of
         uv ->
             %% uv venv is faster, use --python to match the running interpreter
-            io_lib:format("uv venv --python ~s ~s", [quote(Python), quote(Path)]);
+            run_cmd(uv_exe(), ["venv", "--python", Python, Path], []);
         pip ->
-            io_lib:format("~s -m venv ~s", [quote(Python), quote(Path)])
-    end,
-    run_cmd(lists:flatten(Cmd)).
+            run_cmd(Python, ["-m", "venv", Path], [])
+    end.
 
 %% @private Get the Python executable path
 %% When embedded, sys.executable returns the embedding app (beam.smp)
@@ -927,30 +926,28 @@ get_python_executable() ->
 -spec install_deps(string(), string(), list()) -> ok | {error, term()}.
 install_deps(Path, RequirementsFile, Opts) ->
     Installer = detect_installer(Opts),
-    PipPath = pip_path(Path, Installer),
+    {Exe, BaseArgs, PortOpts} = pip_command(Path, Installer),
     Extras = proplists:get_value(extras, Opts, []),
 
-    %% Determine file type and build install command
-    Cmd = case filename:extension(RequirementsFile) of
+    %% Determine file type and build the install argument list (no shell).
+    Args = case filename:extension(RequirementsFile) of
         ".txt" ->
-            %% requirements.txt
-            io_lib:format("~s install -r ~s", [PipPath, quote(RequirementsFile)]);
+            BaseArgs ++ ["install", "-r", RequirementsFile];
         ".toml" ->
-            %% pyproject.toml - install as editable
+            %% pyproject.toml - install as editable.
             %% filename:dirname returns "." for files without directory component
             InstallPath = filename:dirname(RequirementsFile),
             case Extras of
                 [] ->
-                    io_lib:format("~s install -e ~s", [PipPath, quote(InstallPath)]);
+                    BaseArgs ++ ["install", "-e", InstallPath];
                 _ ->
                     ExtrasStr = string:join(Extras, ","),
-                    io_lib:format("~s install -e \"~s[~s]\"", [PipPath, InstallPath, ExtrasStr])
+                    BaseArgs ++ ["install", "-e", InstallPath ++ "[" ++ ExtrasStr ++ "]"]
             end;
         _ ->
-            %% Assume requirements.txt format
-            io_lib:format("~s install -r ~s", [PipPath, quote(RequirementsFile)])
+            BaseArgs ++ ["install", "-r", RequirementsFile]
     end,
-    run_cmd(lists:flatten(Cmd)).
+    run_cmd(Exe, Args, PortOpts).
 
 %% @private Detect which installer to use (uv or pip)
 -spec detect_installer(list()) -> uv | pip.
@@ -965,40 +962,74 @@ detect_installer(Opts) ->
             Installer
     end.
 
-%% @private Get pip/uv pip command path
--spec pip_path(string(), uv | pip) -> string().
-pip_path(VenvPath, uv) ->
-    %% uv pip uses venv from env var or --python flag
-    "VIRTUAL_ENV=" ++ quote(VenvPath) ++ " uv pip";
-pip_path(VenvPath, pip) ->
-    %% Use pip from the venv
-    case os:type() of
+%% @private Resolve the installer into {Executable, BaseArgs, PortOpts}.
+%% For uv the venv is selected via the VIRTUAL_ENV port env option (not a shell
+%% prefix); for pip we use the venv's own pip binary.
+-spec pip_command(string(), uv | pip) -> {string(), [string()], list()}.
+pip_command(VenvPath, uv) ->
+    {uv_exe(), ["pip"], [{env, [{"VIRTUAL_ENV", VenvPath}]}]};
+pip_command(VenvPath, pip) ->
+    PipExe = case os:type() of
         {win32, _} ->
             filename:join([VenvPath, "Scripts", "pip"]);
         _ ->
             filename:join([VenvPath, "bin", "pip"])
+    end,
+    {PipExe, [], []}.
+
+%% @private Full path to the uv executable (falls back to the bare name).
+-spec uv_exe() -> string().
+uv_exe() ->
+    case os:find_executable("uv") of
+        false -> "uv";
+        P -> P
     end.
 
-%% @private Quote a path for shell
--spec quote(string()) -> string().
-quote(S) ->
-    "'" ++ S ++ "'".
-
-%% @private Run a shell command and return ok or error
--spec run_cmd(string()) -> ok | {error, term()}.
-run_cmd(Cmd) ->
-    %% Use os:cmd but check for errors
-    Result = os:cmd(Cmd ++ " 2>&1; echo \"::exitcode::$?\""),
-    %% Parse exit code from end of output
-    case string:split(Result, "::exitcode::", trailing) of
-        [Output, ExitCodeStr] ->
-            case string:trim(ExitCodeStr) of
-                "0" -> ok;
-                Code -> {error, {exit_code, list_to_integer(Code), string:trim(Output)}}
+%% @private Run an executable with an argv list (no shell) and return ok or error.
+-spec run_cmd(string(), [string()], list()) -> ok | {error, term()}.
+run_cmd(Exe, Args, ExtraOpts) ->
+    case resolve_exe(Exe) of
+        {error, _} = Err ->
+            Err;
+        ExeFull ->
+            try open_port({spawn_executable, ExeFull},
+                          [exit_status, stderr_to_stdout, binary, {args, Args} | ExtraOpts]) of
+                Port -> collect_port(Port, [])
+            catch
+                error:Reason -> {error, {spawn_failed, Exe, Reason}}
+            end
+    end.
+
+%% @private Resolve an executable name/path to a full path (spawn_executable does
+%% not search PATH).
+-spec resolve_exe(string()) -> string() | {error, term()}.
+resolve_exe(Exe) ->
+    case filename:pathtype(Exe) of
+        absolute ->
+            case filelib:is_file(Exe) of
+                true -> Exe;
+                false -> {error, {executable_not_found, Exe}}
             end;
         _ ->
-            %% Fallback - assume success if no error marker
-            ok
+            case os:find_executable(Exe) of
+                false -> {error, {executable_not_found, Exe}};
+                Found -> Found
+            end
+    end.
+
+%% @private Collect a spawned port's output and exit status.
+-spec collect_port(port(), [binary()]) -> ok | {error, term()}.
+collect_port(Port, Acc) ->
+    receive
+        {Port, {data, Data}} ->
+            collect_port(Port, [Data | Acc]);
+        {Port, {exit_status, 0}} ->
+            ok;
+        {Port, {exit_status, Code}} ->
+            {error, {exit_code, Code, iolist_to_binary(lists:reverse(Acc))}}
+    after 300000 ->
+        try port_close(Port) catch _:_ -> ok end,
+        {error, timeout}
     end.
 
 %% @private Convert to string

test/py_venv_SUITE.erl

@@ -36,7 +36,8 @@
     test_ensure_venv_force_recreate/1,
     test_activate_venv/1,
     test_deactivate_venv/1,
-    test_venv_info/1
+    test_venv_info/1,
+    test_venv_path_metacharacters/1
 ]).
 
 all() ->
@@ -51,7 +52,8 @@ groups() ->
         test_ensure_venv_force_recreate,
         test_activate_venv,
         test_deactivate_venv,
-        test_venv_info
+        test_venv_info,
+        test_venv_path_metacharacters
     ]}].
 
 init_per_suite(Config) ->
@@ -123,6 +125,21 @@ test_ensure_venv_creates_venv(Config) ->
     true = maps:get(<<"active">>, Info),
     ok.
 
+%% @doc A venv path containing a shell metacharacter (a single quote) must be
+%% treated literally by spawn_executable, not interpreted by a shell. Pre-fix
+%% this went through os:cmd with quote/1, where the embedded quote broke out of
+%% the quoting (and could inject). (A space is avoided here only because it
+%% breaks the venv's own pip shebang, which is unrelated to the shell fix.)
+test_venv_path_metacharacters(Config) ->
+    TempDir = ?config(temp_dir, Config),
+    VenvPath = filename:join(TempDir, "venv'q$x"),
+    ReqFile = filename:join(TempDir, "requirements_meta.txt"),
+    ok = file:write_file(ReqFile, <<"# empty\n">>),
+    %% Created and used at the exact literal path (a shell would have broken it).
+    ok = py:ensure_venv(VenvPath, ReqFile, [{installer, pip}]),
+    true = filelib:is_file(filename:join(VenvPath, "pyvenv.cfg")),
+    ok.
+
 test_ensure_venv_activates_existing(Config) ->
     TempDir = ?config(temp_dir, Config),
     VenvPath = filename:join(TempDir, "venv"),