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

@@ -979,8 +979,8 @@ def run_winexe_command(cmd, args, host, username, password, port=445):
     """
     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}"
+    cmd = f"winexe {creds} {cmd} {args}"
     return win_cmd(cmd, logging_command=logging_cmd)
 
 

tests/pytests/unit/utils/test_cloud.py

@@ -810,3 +810,76 @@ def test_userdata_template():
                 "renderer": "jinja",
             }
             assert cloud.userdata_template(opts=opts, vm_={}, userdata="test") == "True"
+
+
+def test_run_winexe_command_redacts_password_54910():
+    """
+    The logging_command handed to win_cmd must redact the password and must
+    not double the winexe prefix (issue #54910). Before the fix, cmd was
+    reassigned to the full command line before logging_command was built, so
+    the "redacted" string interpolated the already-built command and leaked
+    the plaintext password behind a second, unredacted winexe -U fragment.
+    """
+    # 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"]
+    assert (
+        logging_command
+        == "winexe -U 'Administrator%XXX-REDACTED-XXX' //1.1.1.1 sc query winexesvc"
+    )
+    # The plaintext password must never appear in the logged command.
+    assert "s3cr3t-pw" not in logging_command
+    # A single "winexe -U" fragment; the doubling bug embedded a second one
+    # carrying the real password. (Counting bare "winexe" would false-match
+    # the "winexesvc" service name in the queried command.)
+    assert logging_command.count("winexe -U ") == 1
+
+
+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()