diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 2c9da4ca62fd..35e308ac76fa 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -726,12 +726,7 @@ def _run( f"Specified cwd '{cwd}' either not absolute or does not exist" ) - if ( - python_shell is not True - and shell is not None - # and not salt.utils.platform.is_windows() - and not isinstance(cmd, list) - ): + if python_shell is not True and shell is not None and not isinstance(cmd, list): cmd = salt.utils.args.shlex_split(cmd) if success_retcodes is None: diff --git a/salt/platform/win.py b/salt/platform/win.py index c28621d2feca..a1c97691731b 100644 --- a/salt/platform/win.py +++ b/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()) @@ -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) diff --git a/salt/utils/win_runas.py b/salt/utils/win_runas.py index 00cd7d14d197..dcf0ac2e81aa 100644 --- a/salt/utils/win_runas.py +++ b/salt/utils/win_runas.py @@ -9,6 +9,7 @@ import subprocess import time +import salt.utils.path from salt.exceptions import CommandExecutionError try: @@ -68,7 +69,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 @@ -77,15 +78,30 @@ def create_env(user_token, inherit, timeout=1): start = time.time() env = None exc = None - while True: - try: - env = win32profile.CreateEnvironmentBlock(user_token, False) - except pywintypes.error as exc: - pass - else: - break - if time.time() - start > timeout: - break + profile_info_dict = {"UserName": username} + try: + profile_handle = win32profile.LoadUserProfile(user_token, profile_info_dict) + while True: + try: + env = win32profile.CreateEnvironmentBlock(user_token, inherit) + if env is not None: + break + except pywintypes.error as exc: + pass + else: + break + if time.time() - start > timeout: + break + except (win32api.error, pywintypes.error) as e: + msg = f"Failed to load user profile: {e}" + raise CommandExecutionError(msg) + + try: + win32profile.UnloadUserProfile(user_token, profile_handle) + except (win32api.error, pywintypes.error) as e: + msg = f"Failed to unload user profile: {e}" + raise CommandExecutionError(msg) + if env is not None: return env if exc is not None: @@ -95,8 +111,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. """ @@ -115,10 +131,12 @@ def runas(cmd, username, password=None, cwd=None): # Elevate the token from the current process access = win32security.TOKEN_QUERY | win32security.TOKEN_ADJUST_PRIVILEGES th = win32security.OpenProcessToken(win32api.GetCurrentProcess(), access) + import salt.platform.win + salt.platform.win.elevate_token(th) # Try to impersonate the SYSTEM user. This process needs to be running as a - # user who as been granted the SeImpersonatePrivilege, Administrator + # user who has been granted the SeImpersonatePrivilege, Administrator # accounts have this permission by default. try: impersonation_token = salt.platform.win.impersonate_sid( @@ -211,7 +229,14 @@ 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) + application_name = None + # TODO: Maybe it has something to do with applicationname + # application_name = salt.utils.path.which("cmd.exe") + # application_name = cmd + # import salt.utils.args + # if salt.utils.args.shlex_split(cmd)[0].endswith((".bat", "cmd", "cmd.exe")): + # application_name = salt.utils.path.which("cmd.exe") hProcess = None try: @@ -219,7 +244,7 @@ def runas(cmd, username, password=None, cwd=None): process_info = salt.platform.win.CreateProcessWithTokenW( int(user_token), logonflags=1, - applicationname=None, + applicationname=application_name, commandline=cmd, currentdirectory=cwd, creationflags=creationflags, @@ -232,7 +257,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) diff --git a/tests/pytests/functional/modules/cmd/test_script.py b/tests/pytests/functional/modules/cmd/test_script.py index 952e9c213904..d8e7f7714eaf 100644 --- a/tests/pytests/functional/modules/cmd/test_script.py +++ b/tests/pytests/functional/modules/cmd/test_script.py @@ -18,9 +18,8 @@ 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( """\ @echo off @@ -30,7 +29,6 @@ def echo_script(state_tree): """ ) else: - file_name = "echo_script.sh" contents = dedent( """\ #!/bin/bash @@ -39,14 +37,32 @@ def echo_script(state_tree): echo "a: $a, b: $b" """ ) - with pytest.helpers.temp_file(file_name, contents, state_tree / "echo-script"): + return contents + + +@pytest.fixture +def echo_script(state_tree, echo_script_contents): + if salt.utils.platform.is_windows(): + file_name = "echo_script.bat" + else: + file_name = "echo_script.sh" + with pytest.helpers.temp_file(file_name, echo_script_contents, state_tree): yield file_name @pytest.fixture -def pipe_script(state_tree): +def echo_script_with_space(state_tree, echo_script_contents): + if salt.utils.platform.is_windows(): + file_name = "echo script space.bat" + else: + file_name = "echo script space.sh" + with pytest.helpers.temp_file(file_name, echo_script_contents, state_tree): + yield file_name + + +@pytest.fixture +def pipe_script_contents(): if salt.utils.platform.is_windows(): - file_name = "pipe_script.bat" contents = dedent( """\ @echo off @@ -58,7 +74,6 @@ def pipe_script(state_tree): """ ) else: - file_name = "pipe_script.sh" contents = dedent( """\ #!/bin/bash @@ -69,7 +84,31 @@ def pipe_script(state_tree): fi """ ) - with pytest.helpers.temp_file(file_name, contents, state_tree / "echo-script") as f: + return contents + + +@pytest.fixture +def pipe_script(pipe_script_contents, state_tree): + if salt.utils.platform.is_windows(): + file_name = "pipe_script.bat" + else: + file_name = "pipe_script.sh" + with pytest.helpers.temp_file(file_name, pipe_script_contents, state_tree) as f: + if not salt.utils.platform.is_windows(): + current_perms = f.stat().st_mode + new_perms = current_perms | stat.S_IXUSR + f.chmod(new_perms) + f.chmod(0o755) + yield f + + +@pytest.fixture +def pipe_script_with_space(pipe_script_contents, state_tree): + if salt.utils.platform.is_windows(): + file_name = "pipe script space.bat" + else: + file_name = "pipe script space.sh" + with pytest.helpers.temp_file(file_name, pipe_script_contents, state_tree) as f: if not salt.utils.platform.is_windows(): current_perms = f.stat().st_mode new_perms = current_perms | stat.S_IXUSR @@ -87,11 +126,29 @@ 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 + """ + script = f"salt://{echo_script}" + 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_with_space(modules, echo_script_with_space, args, expected): """ Test argument processing with a batch script """ - script = f"salt://echo-script/{echo_script}" + script = f"salt://{echo_script_with_space}" result = modules.cmd.script(script, args=args) assert result["stdout"] == expected @@ -105,11 +162,36 @@ 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_runas(modules, account, echo_script, args, expected): + """ + Test argument processing with a batch/bash script and runas + """ + script = f"salt://{echo_script}" + result = modules.cmd.script( + script, + args=args, + runas=account.username, + password=account.password, + ) + 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_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}" + script = f"salt://{echo_script_with_space}" result = modules.cmd.script( script, args=args, @@ -119,7 +201,7 @@ def test_echo_runas(modules, account, echo_script, args, expected): assert result["stdout"] == expected -def test_pipe_run_python_shell_true(modules, pipe_script): +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 +210,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 +223,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 +235,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 +247,45 @@ 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" + + +###### FAILING +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"