Try to handle script with spaces

Author: twangboy

salt/modules/cmdmod.py

@@ -729,7 +729,6 @@ def _run(
     if (
         python_shell is not True
         and shell is not None
-        # and not salt.utils.platform.is_windows()
         and not isinstance(cmd, list)
     ):
         cmd = salt.utils.args.shlex_split(cmd)

salt/platform/win.py

@@ -1116,7 +1116,7 @@ def grant_winsta_and_desktop(th):
     """
     current_sid = win32security.GetTokenInformation(th, win32security.TokenUser)[0]
     # Add permissions for the sid to the current windows station and thread id.
-    # This prevents windows error 0xC0000142.
+    # This prevents Windows error 0xC0000142.
     winsta = win32process.GetProcessWindowStation()
     set_user_perm(winsta, WINSTA_ALL, current_sid)
     desktop = win32service.GetThreadDesktop(win32api.GetCurrentThreadId())
@@ -1162,8 +1162,8 @@ def CreateProcessWithTokenW(
         ctypes.byref(startupinfo),
         ctypes.byref(process_info),
     )
+    winerr = win32api.GetLastError()
     if ret == 0:
-        winerr = win32api.GetLastError()
         exc = OSError(win32api.FormatMessage(winerr))
         exc.winerror = winerr
         raise exc
@@ -1341,11 +1341,12 @@ def CreateProcessWithLogonW(
 
 def prepend_cmd(win_shell, cmd):
     """
-    Prep cmd when shell is cmd.exe. Always use a command string instead of a list to satisfy
-    both CreateProcess and CreateProcessWithToken.
+    Prep cmd when shell is cmd.exe. Always use a command string instead of a
+    list to satisfy both CreateProcess and CreateProcessWithToken.
 
-    cmd must be double-quoted to ensure proper handling of space characters. The first opening
-    quote and the closing quote are stripped automatically by the Win32 API.
+    cmd must be double-quoted to ensure proper handling of space characters.
+    The first opening quote and the closing quote are stripped automatically by
+    the Win32 API.
     """
     if isinstance(cmd, (list, tuple)):
         args = subprocess.list2cmdline(cmd)

salt/utils/win_runas.py

@@ -68,7 +68,7 @@ def split_username(username):
     return str(user_name), str(domain)
 
 
-def create_env(user_token, inherit, timeout=1):
+def create_env(username, user_token, inherit=False, timeout=1):
     """
     CreateEnvironmentBlock might fail when we close a login session and then
     try to re-open one very quickly. Run the method multiple times to work
@@ -79,11 +79,16 @@ def create_env(user_token, inherit, timeout=1):
     exc = None
     while True:
         try:
-            env = win32profile.CreateEnvironmentBlock(user_token, False)
+            profile_info_dict = {"UserName": username}
+            profile_handle = win32profile.LoadUserProfile(user_token, profile_info_dict)
+            env = win32profile.CreateEnvironmentBlock(user_token, inherit)
         except pywintypes.error as exc:
             pass
         else:
             break
+        finally:
+            win32profile.UnloadUserProfile(user_token, profile_handle)
+
         if time.time() - start > timeout:
             break
     if env is not None:
@@ -95,8 +100,8 @@ def create_env(user_token, inherit, timeout=1):
 def runas(cmd, username, password=None, cwd=None):
     """
     Run a command as another user. If the process is running as an admin or
-    system account this method does not require a password. Other non
-    privileged accounts need to provide a password for the user to runas.
+    system account, this method does not require a password. Other
+    non-privileged accounts need to provide a password for the user to "runas".
     Commands are run in with the highest level privileges possible for the
     account provided.
     """
@@ -211,7 +216,7 @@ def runas(cmd, username, password=None, cwd=None):
     )
 
     # Create the environment for the user
-    env = create_env(user_token, False)
+    env = create_env(username, user_token, inherit=False)
 
     hProcess = None
     try:
@@ -232,7 +237,7 @@ def runas(cmd, username, password=None, cwd=None):
         dwProcessId = process_info.dwProcessId
         dwThreadId = process_info.dwThreadId
 
-        # We don't use these so let's close the handle
+        # We don't use these, so let's close the handle
         salt.platform.win.kernel32.CloseHandle(stdin_write.handle)
         salt.platform.win.kernel32.CloseHandle(stdout_write.handle)
         salt.platform.win.kernel32.CloseHandle(stderr_write.handle)

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

@@ -18,7 +18,7 @@ def account():
 
 
 @pytest.fixture
-def echo_script(state_tree):
+def echo_script_contents():
     if salt.utils.platform.is_windows():
         file_name = "echo_script.bat"
         contents = dedent(
@@ -39,12 +39,25 @@ def echo_script(state_tree):
             echo "a: $a, b: $b"
             """
         )
+    return file_name, contents
+
+
+@pytest.fixture
+def echo_script(state_tree, echo_script_contents):
+    file_name, contents = echo_script_contents
     with pytest.helpers.temp_file(file_name, contents, state_tree / "echo-script"):
         yield file_name
 
 
 @pytest.fixture
-def pipe_script(state_tree):
+def echo_script_with_space(state_tree, echo_script_contents):
+    file_name, contents = echo_script_contents
+    with pytest.helpers.temp_file(file_name, contents, state_tree / "echo script"):
+        yield file_name
+
+
+@pytest.fixture
+def pipe_script_contents():
     if salt.utils.platform.is_windows():
         file_name = "pipe_script.bat"
         contents = dedent(
@@ -69,6 +82,12 @@ def pipe_script(state_tree):
             fi
             """
         )
+    return file_name, contents
+
+
+@pytest.fixture
+def pipe_script(pipe_script_contents, state_tree):
+    file_name, contents = pipe_script_contents
     with pytest.helpers.temp_file(file_name, contents, state_tree / "echo-script") as f:
         if not salt.utils.platform.is_windows():
             current_perms = f.stat().st_mode
@@ -78,6 +97,22 @@ def pipe_script(state_tree):
         yield f
 
 
+@pytest.fixture
+def pipe_script_with_space(pipe_script_contents, state_tree):
+    file_name, contents = pipe_script_contents
+    with pytest.helpers.temp_directory() as temp_path:
+        temp_file_path = temp_path / "dir with spaces" / file_name
+        temp_file_path.parent.mkdir(parents=True)
+        assert temp_file_path.parent.exists()
+        temp_file_path.write_text(contents)
+        if not salt.utils.platform.is_windows():
+            current_perms = temp_file_path.stat().st_mode
+            new_perms = current_perms | stat.S_IXUSR
+            temp_file_path.chmod(new_perms)
+            temp_file_path.chmod(0o755)
+        yield temp_file_path
+
+
 @pytest.mark.parametrize(
     "args, expected",
     [
@@ -87,7 +122,7 @@ def pipe_script(state_tree):
         (["foo foo", "bar bar"], "a: foo foo, b: bar bar"),
     ],
 )
-def test_echo(modules, echo_script, args, expected):
+def test_script_args(modules, echo_script, args, expected):
     """
     Test argument processing with a batch script
     """
@@ -105,7 +140,25 @@ def test_echo(modules, echo_script, args, expected):
         (["foo foo", "bar bar"], "a: foo foo, b: bar bar"),
     ],
 )
-def test_echo_runas(modules, account, echo_script, args, expected):
+def test_script_args_with_space(modules, echo_script_with_space, args, expected):
+    """
+    Test argument processing with a batch script
+    """
+    script = f"salt://echo script/{echo_script_with_space}"
+    result = modules.cmd.script(script, args=args)
+    assert result["stdout"] == expected
+
+
+@pytest.mark.parametrize(
+    "args, expected",
+    [
+        ("foo bar", "a: foo, b: bar"),
+        ('foo "bar bar"', "a: foo, b: bar bar"),
+        (["foo", "bar"], "a: foo, b: bar"),
+        (["foo foo", "bar bar"], "a: foo foo, b: bar bar"),
+    ],
+)
+def test_script_args_runas(modules, account, echo_script, args, expected):
     """
     Test argument processing with a batch/bash script and runas
     """
@@ -119,7 +172,30 @@ def test_echo_runas(modules, account, echo_script, args, expected):
     assert result["stdout"] == expected
 
 
-def test_pipe_run_python_shell_true(modules, pipe_script):
+@pytest.mark.parametrize(
+    "args, expected",
+    [
+        ("foo bar", "a: foo, b: bar"),
+        ('foo "bar bar"', "a: foo, b: bar bar"),
+        (["foo", "bar"], "a: foo, b: bar"),
+        (["foo foo", "bar bar"], "a: foo foo, b: bar bar"),
+    ],
+)
+def test_script_args_runas_with_space(modules, account, echo_script_with_space, args, expected):
+    """
+    Test argument processing with a batch/bash script and runas
+    """
+    script = f"salt://echo script/{echo_script_with_space}"
+    result = modules.cmd.script(
+        script,
+        args=args,
+        runas=account.username,
+        password=account.password,
+    )
+    assert result["stdout"] == expected
+
+
+def test_run_pipe_python_shell_true(modules, pipe_script):
     if salt.utils.platform.is_windows():
         cmd = f'{str(pipe_script)} | find /c /v ""'
     else:
@@ -128,7 +204,7 @@ def test_pipe_run_python_shell_true(modules, pipe_script):
     assert result == "1"
 
 
-def test_pipe_run_python_shell_false(modules, pipe_script):
+def test_run_pipe_python_shell_false(modules, pipe_script):
     if salt.utils.platform.is_windows():
         cmd = f'{str(pipe_script)} | find /c /v ""'
         # Behavior is different on Windows, I think it has to do with how cmd
@@ -141,7 +217,7 @@ def test_pipe_run_python_shell_false(modules, pipe_script):
     assert result == expected
 
 
-def test_pipe_run_default(modules, pipe_script):
+def test_run_pipe_default(modules, pipe_script):
     if salt.utils.platform.is_windows():
         cmd = f'{str(pipe_script)} | find /c /v ""'
     else:
@@ -153,7 +229,7 @@ def test_pipe_run_default(modules, pipe_script):
     assert result == "1"
 
 
-def test_pipe_run_shell(modules, pipe_script):
+def test_run_pipe_shell(modules, pipe_script):
     if salt.utils.platform.is_windows():
         cmd = f'{str(pipe_script)} | find /c /v ""'
         shell = "cmd"
@@ -165,3 +241,44 @@ def test_pipe_run_shell(modules, pipe_script):
     # test suite, the value is empty
     result = modules.cmd.run(cmd, shell=shell, __pub_jid="test")
     assert result == "1"
+
+
+def test_run_spaces(modules, pipe_script_with_space):
+    cmd = f"{str(pipe_script_with_space)}"
+    result = modules.cmd.run(cmd)
+    assert result == "fine"
+
+
+def test_run_spaces_runas(modules, pipe_script_with_space, account):
+    cmd = f"{str(pipe_script_with_space)}"
+    result = modules.cmd.run(
+        cmd,
+        runas=account.username,
+        password=account.password,
+    )
+    assert result == "fine"
+
+
+def test_script_pipe_spaces(modules, pipe_script_with_space):
+    cmd = f"{str(pipe_script_with_space)}"
+    if salt.utils.platform.is_windows():
+        args = '| find /c /v ""'
+    else:
+        args = "| wc -l"
+    result = modules.cmd.script(cmd, args=args)
+    assert result["stdout"] == "1"
+
+
+def test_script_pipe_spaces_runas(modules, pipe_script_with_space, account):
+    cmd = f"{str(pipe_script_with_space)}"
+    if salt.utils.platform.is_windows():
+        args = '| find /c /v ""'
+    else:
+        args = "| wc -l"
+    result = modules.cmd.script(
+        cmd,
+        args=args,
+        runas=account.username,
+        password=account.password,
+    )
+    assert result["stdout"] == "1"