diff --git a/changelog/61166.fixed.md b/changelog/61166.fixed.md index 1c2cdafff29..f197c324c9e 100644 --- a/changelog/61166.fixed.md +++ b/changelog/61166.fixed.md @@ -1,6 +1,5 @@ -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``. +Fixes multiple issues with the cmd module on Windows. 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``. diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 1324b0df125..c92a4aa4195 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -276,16 +276,21 @@ def _prep_powershell_cmd(win_shell, cmd, encoded_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}"]) + # You might be tempted to use -File here instead of -Command + # The problem with using -File is that any arguments that contain + # powershell commands themselves will not be evaluated + # See GitHub issue #56195 + new_cmd.append("-Command") + if isinstance(cmd, list): + cmd = " ".join(cmd) + new_cmd.append(f"& {cmd.strip()}") elif encoded_cmd: new_cmd.extend(["-EncodedCommand", f"{cmd}"]) else: # Strip whitespace - if isinstance(cmd, str): - cmd = cmd.strip() - elif isinstance(cmd, list): - cmd = " ".join(cmd).strip() - new_cmd.extend(["-Command", f"& {{{cmd}}}"]) + if isinstance(cmd, list): + cmd = " ".join(cmd) + new_cmd.extend(["-Command", f"& {{{cmd.strip()}}}"]) log.debug(new_cmd) return new_cmd diff --git a/salt/modules/win_dsc.py b/salt/modules/win_dsc.py index 997a72fd787..0ef4c2a2b9a 100644 --- a/salt/modules/win_dsc.py +++ b/salt/modules/win_dsc.py @@ -450,7 +450,9 @@ def get_config(): raise config = dict() - if raw_config: + if not raw_config: + raise CommandExecutionError("Not Configured") + else: # Does this Configuration contain a single resource if "ConfigurationName" in raw_config: # Load the single resource @@ -606,11 +608,13 @@ def test_config(): """ cmd = "Test-DscConfiguration" try: - _pshell(cmd, ignore_retcode=True) + result = _pshell(cmd, ignore_retcode=True) except CommandExecutionError as exc: if "Current configuration does not exist" in exc.info["stderr"]: raise CommandExecutionError("Not Configured") raise + if not result: + raise CommandExecutionError("Not Configured") return True @@ -635,11 +639,14 @@ def get_config_status(): "Type, Mode, RebootRequested, NumberofResources" ) try: - return _pshell(cmd, ignore_retcode=True) + result = _pshell(cmd, ignore_retcode=True) except CommandExecutionError as exc: if "No status information available" in exc.info["stderr"]: raise CommandExecutionError("Not Configured") raise + if not result: + raise CommandExecutionError("Not Configured") + return result def get_lcm_config(): diff --git a/tests/integration/modules/test_cmdmod.py b/tests/integration/modules/test_cmdmod.py index c13d31b527b..4f2a72c4560 100644 --- a/tests/integration/modules/test_cmdmod.py +++ b/tests/integration/modules/test_cmdmod.py @@ -596,39 +596,3 @@ class CMDModuleTest(ModuleCase): ).splitlines() self.assertIn("abc=123", out) self.assertIn("ABC=456", out) - - @pytest.mark.slow_test - @pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") - def test_windows_powershell_script_args(self): - """ - Ensure that powershell processes inline script in args - """ - val = "i like cheese" - args = ( - '-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)' - " -ErrorAction Stop".format(val) - ) - script = "salt://issue-56195/test.ps1" - ret = self.run_function( - "cmd.script", [script], args=args, shell="powershell", saltenv="base" - ) - self.assertEqual(ret["stdout"], val) - - @pytest.mark.slow_test - @pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") - @pytest.mark.skip_if_binaries_missing("pwsh") - def test_windows_powershell_script_args_pwsh(self): - """ - Ensure that powershell processes inline script in args with powershell - core - """ - val = "i like cheese" - args = ( - '-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)' - " -ErrorAction Stop".format(val) - ) - script = "salt://issue-56195/test.ps1" - ret = self.run_function( - "cmd.script", [script], args=args, shell="pwsh", saltenv="base" - ) - self.assertEqual(ret["stdout"], val) diff --git a/tests/pytests/functional/modules/cmd/test_powershell.py b/tests/pytests/functional/modules/cmd/test_powershell.py index 5b98bc5b70f..f8913db0493 100644 --- a/tests/pytests/functional/modules/cmd/test_powershell.py +++ b/tests/pytests/functional/modules/cmd/test_powershell.py @@ -15,6 +15,7 @@ def shell(request): This will run the test on powershell and powershell core (pwsh). If powershell core is not installed that test run will be skipped """ + if request.param == "pwsh" and salt.utils.path.which("pwsh") is None: pytest.skip("Powershell 7 Not Present") return request.param @@ -178,7 +179,9 @@ def test_cmd_run_encoded_cmd(shell, cmd, expected, encoded_cmd): """ Ensure that cmd.run supports running shell='powershell' """ - ret = cmdmod.run(cmd=cmd, shell=shell, encoded_cmd=encoded_cmd, redirect_stderr=False) + ret = cmdmod.run( + cmd=cmd, shell=shell, encoded_cmd=encoded_cmd, redirect_stderr=False + ) assert ret == expected diff --git a/tests/pytests/functional/modules/cmd/test_script.py b/tests/pytests/functional/modules/cmd/test_script.py new file mode 100644 index 00000000000..c272835f0bf --- /dev/null +++ b/tests/pytests/functional/modules/cmd/test_script.py @@ -0,0 +1,87 @@ +import pytest + +import salt.utils.path + +pytestmark = [ + pytest.mark.core_test, + pytest.mark.windows_whitelisted, +] + + +@pytest.fixture(scope="module") +def cmd(modules): + return modules.cmd + + +@pytest.fixture(params=["powershell", "pwsh"]) +def shell(request): + """ + This will run the test on powershell and powershell core (pwsh). If + powershell core is not installed that test run will be skipped + """ + if request.param == "pwsh" and salt.utils.path.which("pwsh") is None: + pytest.skip("Powershell 7 Not Present") + return request.param + + +@pytest.fixture(scope="module") +def account(): + with pytest.helpers.create_account() as _account: + yield _account + + +@pytest.fixture +def issue_56195(state_tree): + contents = """ + [CmdLetBinding()] + Param( + [SecureString] $SecureString + ) + $Credential = New-Object System.Net.NetworkCredential("DummyId", $SecureString) + $Credential.Password + """ + with pytest.helpers.temp_file("test.ps1", contents, state_tree / "issue-56195"): + yield + + +@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") +def test_windows_script_args_powershell(cmd, shell, issue_56195): + """ + Ensure that powershell processes an inline script with args where the args + contain powershell that needs to be rendered + """ + password = "i like cheese" + args = ( + "-SecureString (ConvertTo-SecureString -String '{}' -AsPlainText -Force)" + " -ErrorAction Stop".format(password) + ) + script = "salt://issue-56195/test.ps1" + + ret = cmd.script(source=script, args=args, shell="powershell", saltenv="base") + + assert ret["stdout"] == password + + +@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") +def test_windows_script_args_powershell_runas(cmd, shell, account, issue_56195): + """ + Ensure that powershell processes an inline script with args where the args + contain powershell that needs to be rendered + """ + password = "i like cheese" + args = ( + "-SecureString (ConvertTo-SecureString -String '{}' -AsPlainText -Force)" + " -ErrorAction Stop".format(password) + ) + script = "salt://issue-56195/test.ps1" + + ret = cmd.script( + source=script, + args=args, + shell="powershell", + saltenv="base", + runas=account.username, + password=account.password, + ) + + assert ret["stdout"] == password diff --git a/tests/pytests/functional/modules/test_win_useradd.py b/tests/pytests/functional/modules/test_win_useradd.py index 8f3a6e907e7..5e33ce36bd4 100644 --- a/tests/pytests/functional/modules/test_win_useradd.py +++ b/tests/pytests/functional/modules/test_win_useradd.py @@ -333,6 +333,9 @@ def test_setpassword_int(user, account_int): ) def test_update_str(user, value_name, new_value, info_field, expected, account_str): setting = {value_name: new_value} + # You can't expire an account if the password never expires + if value_name == "expired": + setting.update({"password_never_expires": not new_value}) ret = user.update(account_str.username, **setting) assert ret is True ret = user.info(account_str.username) diff --git a/tests/pytests/unit/modules/test_cmdmod.py b/tests/pytests/unit/modules/test_cmdmod.py index 26bf03ef21c..cfc031fc063 100644 --- a/tests/pytests/unit/modules/test_cmdmod.py +++ b/tests/pytests/unit/modules/test_cmdmod.py @@ -1125,8 +1125,8 @@ def test_prep_powershell_cmd_script(): "-NoProfile", "-ExecutionPolicy", "Bypass", - "-File", - script, + "-Command", + f"& {script}", ] assert ret == expected