[WIP] cmd state module fixes

changelog/68298.fixed.md

@@ -0,0 +1 @@
+Fix passing arguments between the cmd state and execution modules

changelog/68301.changed.md

@@ -0,0 +1,2 @@
+Expose paramter python_shell to allow disabling shell functionality for the cmd state module.
+Allow passing args to the cmd state module for cmd.run.

salt/modules/cmdmod.py

@@ -2729,18 +2729,24 @@ def script(
         located on the master in the directory named spam, and is called eggs,
         the source string is salt://spam/eggs
 
-    :param str args: String of command line args to pass to the script. Only
-        used if no args are specified as part of the `name` argument. To pass a
-        string containing spaces in YAML, you will need to doubly-quote it.
-        Additionally, if you need to pass falsey values (e.g., "0", "", "False"),
-        you should doubly-quote them to ensure they are correctly interpreted:
+    :param list|str args: List or string of command line args to pass to the script.
+        A list should be preferred to avoid issues with quoting and special
+        characters. To pass a string containing spaces in YAML, you will need to
+        doubly-quote it. Additionally, if you need to pass falsey values
+        (e.g., "0", "", "False"), you should doubly-quote them to ensure they are
+        correctly interpreted:
 
         .. code-block:: bash
 
+            salt myminion cmd.script salt://foo.sh "['arg1','arg two','arg3']"
             salt myminion cmd.script salt://foo.sh "arg1 'arg two' arg3"
             salt myminion cmd.script salt://foo.sh "''0''"
             salt myminion cmd.script salt://foo.sh "''False''"
 
+        .. versionchanged:: 3006.15
+
+            Support a list of command line args.
+
     :param str cwd: The directory from which to execute the command. Defaults
         to the directory returned from Python's tempfile.mkstemp.
 
@@ -2998,7 +3004,7 @@ def _cleanup_tempfile(path):
     if isinstance(args, (list, tuple)):
         new_cmd = [path, *args] if args else [path]
     else:
-        new_cmd = [path, str(args)] if args else [path]
+        new_cmd = [path, *salt.utils.args.shlex_split(args)] if args else [path]
 
     ret = {}
     try:

salt/states/cmd.py

@@ -325,6 +325,7 @@ def wait(
     root=None,
     runas=None,
     shell=None,
+    python_shell=True,
     env=(),
     stateful=False,
     output_loglevel="debug",
@@ -361,6 +362,12 @@ def wait(
     shell
         The shell to use for execution, defaults to /bin/sh
 
+    python_shell
+        Use the shell as the program to execute. Set to False to disable shell
+        features, such as pipes or redirection
+
+        .. versionadded:: 3006.16
+
     env
         A list of environment variables to be set prior to execution.
         Example:
@@ -476,6 +483,7 @@ def wait_script(
     cwd=None,
     runas=None,
     shell=None,
+    python_shell=None,
     env=None,
     stateful=False,
     use_vt=False,
@@ -515,6 +523,12 @@ def wait_script(
     shell
         The shell to use for execution, defaults to the shell grain
 
+    python_shell
+        Use the shell as the program to execute. Set to True to use shell features,
+        such as pipes or redirection
+
+        .. versionadded:: 3006.16
+
     env
         A list of environment variables to be set prior to execution.
         Example:
@@ -614,10 +628,12 @@ def wait_script(
 
 def run(
     name,
+    args=None,
     cwd=None,
     root=None,
     runas=None,
     shell=None,
+    python_shell=True,
     env=None,
     prepend_path=None,
     stateful=False,
@@ -647,8 +663,19 @@ def run(
        refer to the :ref:`cmdmod documentation <cmdmod-module>`.
 
     name
-        The command to execute, remember that the command will execute with the
-        path and permissions of the salt-minion.
+        The command or program to execute. When running a program, consider
+        disabling shell features with ``python_shell`` and using ``args`` if required.
+        Otherwise character escaping and quoting needs to be handled manually.
+        Remember that the command will execute with the path and permissions of the
+        salt-minion.
+
+    args
+        List or string of command line args to pass to the executable. A list should
+        be preferred to avoid issues with quoting and special characters. To pass a
+        string containing spaces in YAML, you will need to doubly-quote it: "arg1
+        'arg two' arg3"
+
+        .. versionadded:: 3006.16
 
     cwd
         The current working directory to execute the command in, defaults to
@@ -664,6 +691,12 @@ def run(
     shell
         The shell to use for execution, defaults to the shell grain
 
+    python_shell
+        Use the shell as the program to execute. Set to False to disable shell
+        features, such as pipes or redirection
+
+        .. versionadded:: 3006.16
+
     env
         A list of environment variables to be set prior to execution.
         Example:
@@ -834,7 +867,8 @@ def run(
             "root": root,
             "runas": runas,
             "use_vt": use_vt,
-            "shell": shell or __grains__["shell"],
+            "shell": shell,
+            "python_shell": python_shell,
             "env": env,
             "prepend_path": prepend_path,
             "output_loglevel": output_loglevel,
@@ -855,12 +889,24 @@ def run(
         ret["comment"] = f'Desired working directory "{cwd}" is not available'
         return ret
 
+    if shell is None and python_shell is not True:
+        # The program is executed directly. Use a list to clearly separate the
+        # program from the arguments. String splitting may be unreliable
+        if isinstance(args, (list, tuple)):
+            cmd = [name, *args] if args else [name]
+        else:
+            cmd = [name, *salt.utils.args.shlex_split(args)] if args else [name]
+    else:
+        # Execute a shell command. Always pass the command as a string
+        if isinstance(args, (list, tuple)):
+            cmd = " ".join([name, *args]) if args else name
+        else:
+            cmd = " ".join([name, args]) if args else name
+
     # Wow, we passed the test, run this sucker!
     try:
         run_cmd = "cmd.run_all" if not root else "cmd.run_chroot"
-        cmd_all = __salt__[run_cmd](
-            cmd=name, timeout=timeout, python_shell=True, **cmd_kwargs
-        )
+        cmd_all = __salt__[run_cmd](cmd=cmd, timeout=timeout, **cmd_kwargs)
     except Exception as err:  # pylint: disable=broad-except
         ret["comment"] = str(err)
         return ret
@@ -893,6 +939,7 @@ def script(
     runas=None,
     password=None,
     shell=None,
+    python_shell=None,
     env=None,
     stateful=False,
     timeout=None,
@@ -923,6 +970,16 @@ def script(
         Either "cmd arg1 arg2 arg3..." (cmd is not used) or a source
         "salt://...".
 
+    args
+        List or string of command line args to pass to the executable. A list should
+        be preferred to avoid issues with quoting and special characters. To pass a
+        string containing spaces in YAML, you will need to doubly-quote it: "arg1
+        'arg two' arg3"
+
+        .. versionchanged:: 3006.15
+
+            Support a list of command line args.
+
     cwd
         The current working directory to execute the command in, defaults to
         /root
@@ -957,7 +1014,18 @@ def script(
         parameter will be ignored on non-Windows platforms.
 
     shell
-        The shell to use for execution. The default is set in grains['shell']
+        The shell to use for execution. Defaults to the shell grain if python_shell
+        is True. Examples: /bin/sh, cmd.exe
+
+        .. versionchanged:: 3006.16
+
+            Unset by default to run programs without using a shell
+
+    python_shell
+        Use the shell as the program to execute. Set to True to use shell features,
+        such as pipes or redirection
+
+        .. versionadded:: 3006.16
 
     env
         A list of environment variables to be set prior to execution.
@@ -1015,12 +1083,6 @@ def script(
         If the command has not terminated after timeout seconds, send the
         subprocess sigterm, and if sigterm is ignored, follow up with sigkill
 
-    args
-        String of command line args to pass to the script.  Only used if no
-        args are specified as part of the `name` argument. To pass a string
-        containing spaces in YAML, you will need to doubly-quote it:  "arg1
-        'arg two' arg3"
-
     creates
         Only run if the file specified by ``creates`` do not exist. If you
         specify a list of files then this state will only run if **any** of
@@ -1122,7 +1184,8 @@ def script(
         {
             "runas": runas,
             "password": password,
-            "shell": shell or __grains__["shell"],
+            "shell": shell,
+            "python_shell": python_shell,
             "env": env,
             "cwd": cwd,
             "template": template,
@@ -1141,7 +1204,7 @@ def script(
     run_check_cmd_kwargs = {
         "cwd": cwd,
         "runas": runas,
-        "shell": shell or __grains__["shell"],
+        "shell": shell,
     }
 
     # Change the source to be the name arg if it is not specified
@@ -1163,7 +1226,7 @@ def script(
 
     # Wow, we passed the test, run this sucker!
     try:
-        cmd_all = __salt__["cmd.script"](source, python_shell=True, **cmd_kwargs)
+        cmd_all = __salt__["cmd.script"](source, **cmd_kwargs)
     except (CommandExecutionError, SaltRenderError, OSError) as err:
         ret["comment"] = str(err)
         return ret
@@ -1226,7 +1289,7 @@ def call(
     cmd_kwargs = {
         "cwd": kwargs.get("cwd"),
         "runas": kwargs.get("user"),
-        "shell": kwargs.get("shell") or __grains__["shell"],
+        "shell": kwargs.get("shell"),
         "env": kwargs.get("env"),
         "use_vt": use_vt,
         "output_loglevel": output_loglevel,

tests/integration/files/file/base/echo.ps1

@@ -0,0 +1,5 @@
+param (
+    [string]$a,
+    [string]$b
+)
+Write-Output "a: $a, b: $b"

tests/integration/files/file/base/echo.sh

@@ -0,0 +1,2 @@
+#!/usr/bin/env sh
+echo "a: $1, b: $2"