Fix issues with the cmd module on Windows

stderr is no longer piped to stdout by default for cmd.run
Scripts are called using the -File paramter for powershell.exe
stderr is cleared if it contains CLIXML (only for encoded commands)
cmd.powershell now accepts lists as commands
Makes sure returned JSON data is valid before trying to load it
Strips whitespace from the stdout in win_runas

changelog/61166.fixed.md

@@ -0,0 +1,6 @@
+Fixes multiple issues with the cmd module on Windows. ``stderr`` is no
+longer piped to ``stdout`` by default on ``cmd.run``. Scripts are called
+using the ``-File`` parameter to the ``powershell.exe`` binary. ``CLIXML``
+data in stderr is now removed (only applies to encoded commands). Commands can
+now be sent to ``cmd.powershell`` as a list. Makes sure JSON data returned is
+valid. Strips whitespace from the return when using ``runas``.

salt/modules/cmdmod.py

@@ -256,32 +256,36 @@ def _check_avail(cmd):
     return bret and wret
 
 
-def _prep_powershell_cmd(shell, cmd, stack, encoded_cmd):
+def _prep_powershell_cmd(win_shell, cmd, encoded_cmd):
     """
-    Prep cmd when shell is powershell
+    Prep cmd when shell is powershell. If we were called by script(), then fake
+    out the Windows shell to run a Powershell script. Otherwise, just run a
+    Powershell command.
     """
+    # Find the full path to the shell
+    win_shell = f"{salt.utils.path.which(win_shell)}"
 
-    # If this is running on Windows wrap
-    # the shell in quotes in case there are
-    # spaces in the paths.
-    if salt.utils.platform.is_windows():
-        shell = f'"{shell}"'
+    new_cmd = [win_shell, "-NonInteractive", "-NoProfile", "-ExecutionPolicy", "Bypass"]
 
     # extract_stack() returns a list of tuples.
     # The last item in the list [-1] is the current method.
     # The third item[2] in each tuple is the name of that method.
-    if stack[-2][2] == "script":
-        cmd = (
-            "{} -NonInteractive -NoProfile -ExecutionPolicy Bypass -Command {}".format(
-                shell, cmd
-            )
-        )
+    stack = traceback.extract_stack(limit=3)
+    if stack[-3][2] == "script":
+        # If this is cmd.script, then we're running a file
+        new_cmd.extend(["-File", f"{cmd}"])
     elif encoded_cmd:
-        cmd = f"{shell} -NonInteractive -NoProfile -EncodedCommand {cmd}"
+        new_cmd.extend(["-EncodedCommand", f"{cmd}"])
     else:
-        cmd = f'{shell} -NonInteractive -NoProfile -Command "{cmd}"'
+        # Strip whitespace
+        if isinstance(cmd, str):
+            cmd = cmd.strip()
+        elif isinstance(cmd, list):
+            cmd = " ".join(cmd).strip()
+        new_cmd.extend(["-Command", f"& {{{cmd}}}"])
 
-    return cmd
+    log.debug(new_cmd)
+    return new_cmd
 
 
 def _run(
@@ -384,19 +388,7 @@ def _run(
     # The powershell core binary is "pwsh"
     # you can also pass a path here as long as the binary name is one of the two
     if any(word in shell.lower().strip() for word in ["powershell", "pwsh"]):
-        # Strip whitespace
-        if isinstance(cmd, str):
-            cmd = cmd.strip()
-        elif isinstance(cmd, list):
-            cmd = " ".join(cmd).strip()
-        cmd = cmd.replace('"', '\\"')
-
-        # If we were called by script(), then fakeout the Windows
-        # shell to run a Powershell script.
-        # Else just run a Powershell command.
-        stack = traceback.extract_stack(limit=2)
-
-        cmd = _prep_powershell_cmd(shell, cmd, stack, encoded_cmd)
+        cmd = _prep_powershell_cmd(shell, cmd, encoded_cmd)
 
     # munge the cmd and cwd through the template
     (cmd, cwd) = _render_cmd(cmd, cwd, template, saltenv, pillarenv, pillar_override)
@@ -809,6 +801,9 @@ def _run(
                     _log_cmd(cmd),
                 )
 
+        # Encoded commands dump CLIXML data in stderr. It's not an actual error
+        if encoded_cmd and "CLIXML" in err:
+            err = ""
         if rstrip:
             if out is not None:
                 out = out.rstrip()
@@ -1055,6 +1050,7 @@ def run(
     ignore_retcode=False,
     saltenv=None,
     use_vt=False,
+    redirect_stderr=False,
     bg=False,
     password=None,
     encoded_cmd=False,
@@ -1190,6 +1186,12 @@ def run(
     :param bool use_vt: Use VT utils (saltstack) to stream the command output
         more interactively to the console and the logs. This is experimental.
 
+    :param bool redirect_stderr: If set to ``True``, then stderr will be
+        redirected to stdout. This is helpful for cases where obtaining both
+        the retcode and output is desired.
+
+        .. versionadded:: 3006.9
+
     :param bool encoded_cmd: Specify if the supplied command is encoded.
         Only applies to shell 'powershell' and 'pwsh'.
 
@@ -1301,6 +1303,7 @@ def run(
         salt '*' cmd.run cmd='sed -e s/=/:/g'
     """
     python_shell = _python_shell_default(python_shell, kwargs.get("__pub_jid", ""))
+    stderr = subprocess.STDOUT if redirect_stderr else subprocess.PIPE
     ret = _run(
         cmd,
         runas=runas,
@@ -1309,7 +1312,7 @@ def run(
         python_shell=python_shell,
         cwd=cwd,
         stdin=stdin,
-        stderr=subprocess.STDOUT,
+        stderr=stderr,
         env=env,
         clean_env=clean_env,
         prepend_path=prepend_path,
@@ -4057,6 +4060,9 @@ def powershell(
     else:
         python_shell = True
 
+    if isinstance(cmd, list):
+        cmd = " ".join(cmd)
+
     # Append PowerShell Object formatting
     # ConvertTo-JSON is only available on PowerShell 3.0 and later
     psversion = shell_info("powershell")["psversion"]
@@ -4085,7 +4091,7 @@ def powershell(
         encoded_cmd = False
 
     # Retrieve the response, while overriding shell with 'powershell'
-    response = run(
+    response = run_stdout(
         cmd,
         cwd=cwd,
         stdin=stdin,
@@ -4113,9 +4119,8 @@ def powershell(
         **kwargs,
     )
 
-    # Sometimes Powershell returns an empty string, which isn't valid JSON
-    if response == "":
-        response = "{}"
+    response = _prep_powershell_json(response)
+
     try:
         return salt.utils.json.loads(response)
     except Exception:  # pylint: disable=broad-except
@@ -4419,10 +4424,16 @@ def powershell_all(
     else:
         python_shell = True
 
+    if isinstance(cmd, list):
+        cmd = " ".join(cmd)
+
     # Append PowerShell Object formatting
-    cmd += " | ConvertTo-JSON"
-    if depth is not None:
-        cmd += f" -Depth {depth}"
+    # ConvertTo-JSON is only available on PowerShell 3.0 and later
+    psversion = shell_info("powershell")["psversion"]
+    if salt.utils.versions.version_cmp(psversion, "2.0") == 1:
+        cmd += " | ConvertTo-JSON"
+        if depth is not None:
+            cmd += f" -Depth {depth}"
 
     if encode_cmd:
         # Convert the cmd to UTF-16LE without a BOM and base64 encode.
@@ -4474,6 +4485,8 @@ def powershell_all(
             response["result"] = []
         return response
 
+    stdoutput = _prep_powershell_json(stdoutput)
+
     # If we fail to parse stdoutput we will raise an exception
     try:
         result = salt.utils.json.loads(stdoutput)
@@ -4492,9 +4505,30 @@ def powershell_all(
     else:
         # result type is list so the force_list param has no effect
         response["result"] = result
+
+    # Encoded commands dump CLIXML data in stderr. It's not an actual error
+    if "CLIXML" in response["stderr"]:
+        response["stderr"] = ""
+
     return response
 
 
+def _prep_powershell_json(text):
+    """
+    Try to fix the output from OutputTo-JSON in powershell commands to make it
+    valid JSON
+    """
+    # An empty string just needs to be an empty quote
+    if text == "":
+        text = '""'
+    else:
+        # Raw text needs to be quoted
+        starts_with = ['"', "{", "["]
+        if not any(text.startswith(x) for x in starts_with):
+            text = f'"{text}"'
+    return text
+
+
 def run_bg(
     cmd,
     cwd=None,

salt/utils/win_runas.py

@@ -52,6 +52,9 @@ def __virtual__():
 
 
 def split_username(username):
+    """
+    Splits out the username from the domain name and returns both.
+    """
     domain = "."
     user_name = username
     if "@" in username:
@@ -234,7 +237,7 @@ def runas(cmdLine, username, password=None, cwd=None):
         fd_out = msvcrt.open_osfhandle(stdout_read.handle, os.O_RDONLY | os.O_TEXT)
         with os.fdopen(fd_out, "r") as f_out:
             stdout = f_out.read()
-            ret["stdout"] = stdout
+            ret["stdout"] = stdout.strip()
 
         # Read standard error
         fd_err = msvcrt.open_osfhandle(stderr_read.handle, os.O_RDONLY | os.O_TEXT)