Fix checking retcodes with runas on Windows

changelog/59977.fixed.md

@@ -0,0 +1,2 @@
+Fixed an issue on Windows where checking success_retcodes when using the
+runas parameter would fail. Now success_retcodes are checked correctly

salt/modules/cmdmod.py

@@ -458,8 +458,6 @@ def _run(
         if isinstance(cmd, (list, tuple)):
             cmd = " ".join(cmd)
 
-        return win_runas(cmd, runas, password, cwd)
-
     if runas and salt.utils.platform.is_darwin():
         # We need to insert the user simulation into the command itself and not
         # just run it from the environment on macOS as that method doesn't work
@@ -492,7 +490,7 @@ def _run(
         # hang.
         runas = None
 
-    if runas:
+    if runas and not salt.utils.platform.is_windows():
         # Save the original command before munging it
         try:
             pwd.getpwnam(runas)
@@ -513,7 +511,7 @@ def _run(
         else:
             use_sudo = True
 
-    if runas or group:
+    if (runas or group) and not salt.utils.platform.is_windows():
         try:
             # Getting the environment for the runas user
             # Use markers to thwart any stdout noise
@@ -752,90 +750,104 @@ def _run(
 
     if not use_vt:
         # This is where the magic happens
-        try:
+
+        if runas and salt.utils.platform.is_windows():
+
+            # We can't use TimedProc with runas on Windows
             if change_windows_codepage:
                 salt.utils.win_chcp.set_codepage_id(windows_codepage)
-            try:
-                proc = salt.utils.timed_subprocess.TimedProc(cmd, **new_kwargs)
-            except OSError as exc:
-                msg = "Unable to run command '{}' with the context '{}', reason: {}".format(
-                    cmd if output_loglevel is not None else "REDACTED",
-                    new_kwargs,
-                    exc,
-                )
-                raise CommandExecutionError(msg)
 
-            try:
+            ret = win_runas(cmd, runas, password, cwd)
-                proc.run()
+
-            except TimedProcTimeoutError as exc:
-                ret["stdout"] = str(exc)
-                ret["stderr"] = ""
-                ret["retcode"] = None
-                ret["pid"] = proc.process.pid
-                # ok return code for timeouts?
-                ret["retcode"] = 1
-                return ret
-        finally:
             if change_windows_codepage:
                 salt.utils.win_chcp.set_codepage_id(previous_windows_codepage)
 
-        if output_loglevel != "quiet" and output_encoding is not None:
+        else:
-            log.debug(
+            try:
-                "Decoding output from command %s using %s encoding",
+                if change_windows_codepage:
-                cmd,
+                    salt.utils.win_chcp.set_codepage_id(windows_codepage)
-                output_encoding,
+                try:
-            )
+                    proc = salt.utils.timed_subprocess.TimedProc(cmd, **new_kwargs)
+                except OSError as exc:
+                    msg = "Unable to run command '{}' with the context '{}', reason: {}".format(
+                        cmd if output_loglevel is not None else "REDACTED",
+                        new_kwargs,
+                        exc,
+                    )
+                    raise CommandExecutionError(msg)
 
-        try:
+                try:
-            out = salt.utils.stringutils.to_unicode(
+                    proc.run()
-                proc.stdout, encoding=output_encoding
+                except TimedProcTimeoutError as exc:
-            )
+                    ret["stdout"] = str(exc)
-        except TypeError:
+                    ret["stderr"] = ""
-            # stdout is None
+                    ret["retcode"] = None
-            out = ""
+                    ret["pid"] = proc.process.pid
-        except UnicodeDecodeError:
+                    # ok return code for timeouts?
-            out = salt.utils.stringutils.to_unicode(
+                    ret["retcode"] = 1
-                proc.stdout, encoding=output_encoding, errors="replace"
+                    return ret
-            )
+            finally:
-            if output_loglevel != "quiet":
+                if change_windows_codepage:
-                log.error(
+                    salt.utils.win_chcp.set_codepage_id(previous_windows_codepage)
-                    "Failed to decode stdout from command %s, non-decodable "
+
-                    "characters have been replaced",
+            if output_loglevel != "quiet" and output_encoding is not None:
-                    _log_cmd(cmd),
+                log.debug(
+                    "Decoding output from command %s using %s encoding",
+                    cmd,
+                    output_encoding,
                 )
 
-        try:
+            try:
-            err = salt.utils.stringutils.to_unicode(
+                out = salt.utils.stringutils.to_unicode(
-                proc.stderr, encoding=output_encoding
+                    proc.stdout, encoding=output_encoding
-            )
-        except TypeError:
-            # stderr is None
-            err = ""
-        except UnicodeDecodeError:
-            err = salt.utils.stringutils.to_unicode(
-                proc.stderr, encoding=output_encoding, errors="replace"
-            )
-            if output_loglevel != "quiet":
-                log.error(
-                    "Failed to decode stderr from command %s, non-decodable "
-                    "characters have been replaced",
-                    _log_cmd(cmd),
                 )
+            except TypeError:
+                # stdout is None
+                out = ""
+            except UnicodeDecodeError:
+                out = salt.utils.stringutils.to_unicode(
+                    proc.stdout, encoding=output_encoding, errors="replace"
+                )
+                if output_loglevel != "quiet":
+                    log.error(
+                        "Failed to decode stdout from command %s, non-decodable "
+                        "characters have been replaced",
+                        _log_cmd(cmd),
+                    )
+
+            try:
+                err = salt.utils.stringutils.to_unicode(
+                    proc.stderr, encoding=output_encoding
+                )
+            except TypeError:
+                # stderr is None
+                err = ""
+            except UnicodeDecodeError:
+                err = salt.utils.stringutils.to_unicode(
+                    proc.stderr, encoding=output_encoding, errors="replace"
+                )
+                if output_loglevel != "quiet":
+                    log.error(
+                        "Failed to decode stderr from command %s, non-decodable "
+                        "characters have been replaced",
+                        _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()
+                if err is not None:
+                    err = err.rstrip()
+            ret["pid"] = proc.process.pid
+            ret["retcode"] = proc.process.returncode
+            ret["stdout"] = out
+            ret["stderr"] = err
 
-        # 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()
-            if err is not None:
-                err = err.rstrip()
-        ret["pid"] = proc.process.pid
-        ret["retcode"] = proc.process.returncode
         if ret["retcode"] in success_retcodes:
             ret["retcode"] = 0
-        ret["stdout"] = out
-        ret["stderr"] = err
         if any(
             [stdo in ret["stdout"] for stdo in success_stdout]
             + [stde in ret["stderr"] for stde in success_stderr]

tests/pytests/functional/modules/cmd/test_run_win.py

@@ -0,0 +1,50 @@
+import pytest
+
+pytestmark = [
+    pytest.mark.core_test,
+    pytest.mark.windows_whitelisted,
+]
+
+
+@pytest.fixture(scope="module")
+def account():
+    with pytest.helpers.create_account() as _account:
+        yield _account
+
+
+@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows")
+@pytest.mark.parametrize(
+    "exit_code, return_code, result",
+    [
+        (300, 0, True),
+        (299, 299, False),
+    ],
+)
+def test_windows_script_exitcode(modules, state_tree, exit_code, return_code, result):
+    ret = modules.state.single(
+        "cmd.run", name=f"cmd.exe /c exit {exit_code}", success_retcodes=[2, 44, 300]
+    )
+    assert ret.result is result
+    assert ret.filtered["changes"]["retcode"] == return_code
+
+
+@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows")
+@pytest.mark.parametrize(
+    "exit_code, return_code, result",
+    [
+        (300, 0, True),
+        (299, 299, False),
+    ],
+)
+def test_windows_script_exitcode_runas(
+    modules, state_tree, exit_code, return_code, result, account
+):
+    ret = modules.state.single(
+        "cmd.run",
+        name=f"cmd.exe /c exit {exit_code}",
+        success_retcodes=[2, 44, 300],
+        runas=account.username,
+        password=account.password,
+    )
+    assert ret.result is result
+    assert ret.filtered["changes"]["retcode"] == return_code