Fix run_winexe_command leaking plaintext password in logs

changelog/54910.fixed.md

@@ -0,0 +1 @@
+Stop run_winexe_command leaking the plaintext password and doubling the winexe prefix in its logged command

salt/utils/cloud.py

@@ -978,10 +978,10 @@ def run_winexe_command(cmd, args, host, username, password, port=445):
     Run a command remotely via the winexe executable
     """
     creds = f"-U '{username}%{password}' //{host}"
-    logging_creds = f"-U '{username}%XXX-REDACTED-XXX' //{host}"
     cmd = f"winexe {creds} {cmd} {args}"
-    logging_cmd = f"winexe {logging_creds} {cmd} {args}"
-    return win_cmd(cmd, logging_command=logging_cmd)
+    # Only log the program name, never the arguments -- they carry the
+    # credentials. This mirrors how salt.modules.cmdmod logs commands.
+    return win_cmd(cmd, logging_command="winexe")
 
 
 def run_psexec_command(cmd, args, host, username, password, port=445):

tests/pytests/unit/utils/test_cloud.py

@@ -810,3 +810,73 @@ def test_userdata_template():
                 "renderer": "jinja",
             }
             assert cloud.userdata_template(opts=opts, vm_={}, userdata="test") == "True"
+
+
+def test_run_winexe_command_logs_program_name_only_54910():
+    """
+    The logging_command handed to win_cmd must be only the program name, so the
+    credentials (and every other argument) are never logged (issue #54910).
+    This mirrors how salt.modules.cmdmod logs commands. Before, cmd was
+    reassigned to the full credential-bearing command line before the logged
+    string was built, leaking the plaintext password into the logs.
+    """
+    # Production-exact argument shape: wait_for_winexe() calls
+    # run_winexe_command("sc", "query winexesvc", host, username, password, port)
+    win_cmd_mock = MagicMock(return_value=0)
+    with patch.object(cloud, "win_cmd", win_cmd_mock):
+        cloud.run_winexe_command(
+            "sc",
+            "query winexesvc",
+            "1.1.1.1",
+            "Administrator",
+            "s3cr3t-pw",
+        )
+    logging_command = win_cmd_mock.call_args.kwargs["logging_command"]
+    # Only the program name is logged -- no arguments at all.
+    assert logging_command == "winexe"
+    # The plaintext password, the username, the host and the remote command
+    # (all carried as arguments) must be absent from the logged string.
+    assert "s3cr3t-pw" not in logging_command
+    assert "Administrator" not in logging_command
+    assert "query winexesvc" not in logging_command
+
+
+def test_run_winexe_command_executes_correct_command_54910():
+    """
+    Must-not-regress sibling: the actually executed command was already
+    correct before the fix and must stay correct. This assertion passes both
+    with and without the swap, guarding against a change that repairs the
+    logging string but corrupts the real command line.
+    """
+    win_cmd_mock = MagicMock(return_value=0)
+    with patch.object(cloud, "win_cmd", win_cmd_mock):
+        cloud.run_winexe_command(
+            "sc",
+            "query winexesvc",
+            "1.1.1.1",
+            "Administrator",
+            "s3cr3t-pw",
+        )
+    executed_command = win_cmd_mock.call_args.args[0]
+    assert (
+        executed_command
+        == "winexe -U 'Administrator%s3cr3t-pw' //1.1.1.1 sc query winexesvc"
+    )
+
+
+def test_run_winexe_command_returns_win_cmd_result():
+    """
+    run_winexe_command should propagate win_cmd's return value to its caller
+    unchanged (wait_for_winexe relies on the return code to detect success).
+    """
+    win_cmd_mock = MagicMock(return_value=0)
+    with patch.object(cloud, "win_cmd", win_cmd_mock):
+        result = cloud.run_winexe_command(
+            "sc",
+            "query winexesvc",
+            "1.1.1.1",
+            "Administrator",
+            "s3cr3t-pw",
+        )
+    assert result == 0
+    win_cmd_mock.assert_called_once()