diff --git a/changelog/59664.fixed b/changelog/59664.fixed new file mode 100644 index 000000000000..d81b6fcc9e89 --- /dev/null +++ b/changelog/59664.fixed @@ -0,0 +1,2 @@ +Allow "extra_filerefs" as sanitized kwargs for SSH client. +Fix regression on "cmd.run" when passing tuples as cmd. diff --git a/changelog/59748.fixed b/changelog/59748.fixed new file mode 100644 index 000000000000..92edd790d289 --- /dev/null +++ b/changelog/59748.fixed @@ -0,0 +1 @@ +Allow all ssh kwargs as sanitized kwargs for SSH client. diff --git a/salt/client/mixins.py b/salt/client/mixins.py index 204f64277680..e5cd202156a4 100644 --- a/salt/client/mixins.py +++ b/salt/client/mixins.py @@ -28,7 +28,6 @@ import salt.utils.state import salt.utils.user import salt.utils.versions -from salt.ext import six log = logging.getLogger(__name__) diff --git a/salt/client/ssh/client.py b/salt/client/ssh/client.py index c892ef2a61e3..f3312813e415 100644 --- a/salt/client/ssh/client.py +++ b/salt/client/ssh/client.py @@ -1,14 +1,8 @@ -# -*- coding: utf-8 -*- - -# Import Python libs -from __future__ import absolute_import, print_function, unicode_literals - import copy import logging import os import random -# Import Salt libs import salt.config import salt.syspaths as syspaths import salt.utils.args @@ -17,7 +11,7 @@ log = logging.getLogger(__name__) -class SSHClient(object): +class SSHClient: """ Create a client object for executing routines via the salt-ssh backend @@ -58,10 +52,34 @@ def sanitize_kwargs(self, kwargs): ("ssh_identities_only", bool), ("ssh_remote_port_forwards", str), ("ssh_options", list), + ("ssh_max_procs", int), + ("ssh_askpass", bool), + ("ssh_key_deploy", bool), + ("ssh_update_roster", bool), + ("ssh_scan_ports", str), + ("ssh_scan_timeout", int), + ("ssh_timeout", int), + ("ssh_log_file", str), + ("raw_shell", bool), + ("refresh_cache", bool), + ("roster", str), ("roster_file", str), ("rosters", list), ("ignore_host_keys", bool), ("raw_shell", bool), + ("extra_filerefs", str), + ("min_extra_mods", str), + ("thin_extra_mods", str), + ("verbose", bool), + ("static", bool), + ("ssh_wipe", bool), + ("rand_thin_dir", bool), + ("regen_thin", bool), + ("python2_bin", str), + ("python3_bin", str), + ("ssh_run_pre_flight", bool), + ("no_host_keys", bool), + ("saltfile", str), ] sane_kwargs = {} for name, kind in roster_vals: @@ -126,8 +144,7 @@ def cmd_iter( .. versionadded:: 2015.5.0 """ ssh = self._prep_ssh(tgt, fun, arg, timeout, tgt_type, kwarg, **kwargs) - for ret in ssh.run_iter(jid=kwargs.get("jid", None)): - yield ret + yield from ssh.run_iter(jid=kwargs.get("jid", None)) def cmd( self, tgt, fun, arg=(), timeout=None, tgt_type="glob", kwarg=None, **kwargs diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index feae915b91fa..f24e7cc9ae5c 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -78,7 +78,7 @@ def __virtual__(): def _log_cmd(cmd): - if not isinstance(cmd, list): + if isinstance(cmd, str): return cmd.split()[0].strip() return cmd[0].strip() @@ -838,7 +838,9 @@ def _run( if not ignore_retcode and ret["retcode"] != 0: if output_loglevel < LOG_LEVELS["error"]: output_loglevel = LOG_LEVELS["error"] - msg = "Command '{}' failed with return code: {}".format(_log_cmd(cmd), ret["retcode"]) + msg = "Command '{}' failed with return code: {}".format( + _log_cmd(cmd), ret["retcode"] + ) log.error(log_callback(msg)) if ret["stdout"]: log.log(output_loglevel, "stdout: %s", log_callback(ret["stdout"])) @@ -1208,7 +1210,9 @@ def run( if not ignore_retcode and ret["retcode"] != 0: if lvl < LOG_LEVELS["error"]: lvl = LOG_LEVELS["error"] - msg = "Command '{}' failed with return code: {}".format(_log_cmd(cmd), ret["retcode"]) + msg = "Command '{}' failed with return code: {}".format( + _log_cmd(cmd), ret["retcode"] + ) log.error(log_callback(msg)) if raise_err: raise CommandExecutionError( diff --git a/tests/integration/wheel/test_pillar_roots.py b/tests/integration/wheel/test_pillar_roots.py index 98512af95ef1..904bfd0aa28b 100644 --- a/tests/integration/wheel/test_pillar_roots.py +++ b/tests/integration/wheel/test_pillar_roots.py @@ -14,11 +14,11 @@ def setUp(self): def tearDown(self): try: os.remove(os.path.join(self.pillar_dir, "foo")) - except Exception: + except Exception: # pylint: disable=broad-except pass try: os.remove(os.path.join(self.traversed_dir, "foo")) - except Exception: + except Exception: # pylint: disable=broad-except pass del self.wheel diff --git a/tests/pytests/unit/client/test_ssh.py b/tests/pytests/unit/client/test_ssh.py index b006e7e02f15..a8d18b2195ab 100644 --- a/tests/pytests/unit/client/test_ssh.py +++ b/tests/pytests/unit/client/test_ssh.py @@ -1,8 +1,11 @@ import pytest +import salt.client.ssh.client import salt.utils.msgpack from salt.client import ssh from tests.support.mock import MagicMock, patch +pytestmark = [pytest.mark.skip_if_binaries_missing("ssh", "ssh-keygen", check_all=True)] + @pytest.fixture def ssh_target(tmpdir): @@ -57,3 +60,76 @@ def test_cmd_block_python_version_error(ssh_target): with patch_shim: ret = single.cmd_block() assert "ERROR: Python version error. Recommendation(s) follow:" in ret[0] + + +@pytest.mark.parametrize( + "test_opts", + [ + ("extra_filerefs", "salt://foobar", True), + ("host", "testhost", False), + ("ssh_user", "testuser", True), + ("ssh_passwd", "testpasswd", True), + ("ssh_port", 23, False), + ("ssh_sudo", True, True), + ("ssh_sudo_user", "sudouser", False), + ("ssh_priv", "test_priv", True), + ("ssh_priv_passwd", "sshpasswd", True), + ("ssh_identities_only", True, True), + ("ssh_remote_port_forwards", "test", True), + ("ssh_options", ["test1", "test2"], True), + ("ssh_max_procs", 2, True), + ("ssh_askpass", True, True), + ("ssh_key_deploy", True, True), + ("ssh_update_roster", True, True), + ("ssh_scan_ports", "test", True), + ("ssh_scan_timeout", 1.0, True), + ("ssh_timeout", 1, False), + ("ssh_log_file", "/tmp/test", True), + ("raw_shell", True, True), + ("refresh_cache", True, True), + ("roster", "/test", True), + ("roster_file", "/test1", True), + ("rosters", ["test1"], False), + ("ignore_host_keys", True, True), + ("min_extra_mods", "test", True), + ("thin_extra_mods", "test1", True), + ("verbose", True, True), + ("static", True, True), + ("ssh_wipe", True, True), + ("rand_thin_dir", True, True), + ("regen_thin", True, True), + ("python2_bin", "python2", True), + ("python3_bin", "python3", True), + ("ssh_run_pre_flight", True, True), + ("no_host_keys", True, True), + ("saltfile", "/tmp/test", True), + ("doesnotexist", None, False), + ], +) +def test_ssh_kwargs(test_opts): + """ + test all ssh kwargs are not excluded from kwargs + when preparing the SSH opts + """ + opt_key = test_opts[0] + opt_value = test_opts[1] + # Is the kwarg in salt.utils.parsers? + in_parser = test_opts[2] + + opts = { + "eauth": "auto", + "username": "test", + "password": "test", + "client": "ssh", + "tgt": "localhost", + "fun": "test.ping", + opt_key: opt_value, + } + client = salt.client.ssh.client.SSHClient(disable_custom_roster=True) + if in_parser: + ssh_kwargs = salt.utils.parsers.SaltSSHOptionParser().defaults + assert opt_key in ssh_kwargs + + with patch("salt.roster.get_roster_file", MagicMock(return_value="")): + ssh_obj = client._prep_ssh(**opts) + assert ssh_obj.opts.get(opt_key, None) == opt_value diff --git a/tests/pytests/unit/modules/test_cmdmod.py b/tests/pytests/unit/modules/test_cmdmod.py new file mode 100644 index 000000000000..66be53fdf6ae --- /dev/null +++ b/tests/pytests/unit/modules/test_cmdmod.py @@ -0,0 +1,662 @@ +""" + :codeauthor: Nicole Thomas + + Unit tests for the salt.modules.cmdmod module +""" + +import builtins +import logging +import os +import re +import sys +import tempfile + +import pytest +import salt.modules.cmdmod as cmdmod +import salt.utils.files +import salt.utils.platform +import salt.utils.stringutils +from salt.exceptions import CommandExecutionError +from salt.log.setup import LOG_LEVELS +from tests.support.mock import MagicMock, Mock, MockTimedProc, mock_open, patch +from tests.support.runtests import RUNTIME_VARS + +DEFAULT_SHELL = "foo/bar" +MOCK_SHELL_FILE = "# List of acceptable shells\n" "\n" "/bin/bash\n" + + +@pytest.fixture(autouse=True) +def setup_loader(): + setup_loader_modules = {cmdmod: {}} + with pytest.helpers.loader_mock(setup_loader_modules) as loader_mock: + yield loader_mock + + +@pytest.fixture(scope="module") +def mock_loglevels(): + return { + "info": "foo", + "all": "bar", + "critical": "bar", + "trace": "bar", + "garbage": "bar", + "error": "bar", + "debug": "bar", + "warning": "bar", + "quiet": "bar", + } + + +def test_render_cmd_no_template(): + """ + Tests return when template=None + """ + assert cmdmod._render_cmd("foo", "bar", None) == ("foo", "bar") + + +def test_render_cmd_unavailable_engine(): + """ + Tests CommandExecutionError raised when template isn't in the + template registry + """ + with pytest.raises(CommandExecutionError): + cmdmod._render_cmd("boo", "bar", "baz") + + +def test_check_loglevel_bad_level(mock_loglevels): + """ + Tests return of providing an invalid loglevel option + """ + with patch.dict(LOG_LEVELS, mock_loglevels): + assert cmdmod._check_loglevel(level="bad_loglevel") == "foo" + + +def test_check_loglevel_bad_level_not_str(mock_loglevels): + """ + Tests the return of providing an invalid loglevel option that is not a string + """ + with patch.dict(LOG_LEVELS, mock_loglevels): + assert cmdmod._check_loglevel(level=1000) == "foo" + + +def test_check_loglevel_quiet(mock_loglevels): + """ + Tests the return of providing a loglevel of 'quiet' + """ + with patch.dict(LOG_LEVELS, mock_loglevels): + assert cmdmod._check_loglevel(level="quiet") is None + + +def test_parse_env_not_env(): + """ + Tests the return of an env that is not an env + """ + assert cmdmod._parse_env(None) == {} + + +def test_parse_env_list(): + """ + Tests the return of an env that is a list + """ + ret = {"foo": None, "bar": None} + assert ret == cmdmod._parse_env(["foo", "bar"]) + + +def test_parse_env_dict(): + """ + Test the return of an env that is not a dict + """ + assert cmdmod._parse_env("test") == {} + + +def test_run_shell_is_not_file(): + """ + Tests error raised when shell is not available after _is_valid_shell error msg + and os.path.isfile returns False + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=False)): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar") + + +def test_run_shell_file_no_access(): + """ + Tests error raised when shell is not available after _is_valid_shell error msg, + os.path.isfile returns True, but os.access returns False + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=False)): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar") + + +def test_run_runas_with_windows(): + """ + Tests error raised when runas is passed on windows + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=True)): + with patch( + "salt.utils.win_chcp.get_codepage_id", MagicMock(return_value=65001) + ): + with patch.dict(cmdmod.__grains__, {"os": "fake_os"}): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar", runas="baz") + + +def test_run_with_tuple(): + """ + Tests return when cmd is a tuple + """ + mock_true = MagicMock(return_value=True) + with patch("salt.modules.cmdmod._is_valid_shell", mock_true): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", mock_true): + with patch("os.access", mock_true): + cmdmod._run(("echo", "foo"), python_shell=True) + + +def test_run_user_not_available(): + """ + Tests return when runas user is not available + """ + mock_true = MagicMock(return_value=True) + with patch("salt.modules.cmdmod._is_valid_shell", mock_true): + with patch("os.path.isfile", mock_true): + with patch("os.access", mock_true): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar", runas="baz") + + +def test_run_zero_umask(): + """ + Tests error raised when umask is set to zero + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar", umask=0) + + +def test_run_invalid_umask(): + """ + Tests error raised when an invalid umask is given + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + pytest.raises( + CommandExecutionError, cmdmod._run, "foo", "bar", umask="baz", + ) + + +def test_run_invalid_cwd_not_abs_path(): + """ + Tests error raised when cwd is not an absolute path + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar") + + +def test_run_invalid_cwd_not_dir(): + """ + Tests error raised when cwd is not a dir + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + with patch("os.path.isabs", MagicMock(return_value=True)): + with pytest.raises(CommandExecutionError): + cmdmod._run("foo", "bar") + + +def test_run_no_vt_os_error(): + """ + Tests error raised when not useing vt and OSError is provided + """ + expected_error = "expect error" + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + with patch( + "salt.utils.timed_subprocess.TimedProc", + MagicMock(side_effect=OSError(expected_error)), + ): + with pytest.raises(CommandExecutionError) as error: + cmdmod.run("foo", cwd="/") + assert error.value.args[0].endswith(expected_error) + + +def test_run_no_vt_io_error(): + """ + Tests error raised when not useing vt and IOError is provided + """ + expected_error = "expect error" + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + with patch( + "salt.utils.timed_subprocess.TimedProc", + MagicMock(side_effect=IOError(expected_error)), + ): + with pytest.raises(CommandExecutionError) as error: + cmdmod.run("foo", cwd="/") + assert error.value.args[0].endswith(expected_error) + + +@pytest.mark.skip(reason="Test breaks unittests runs") +@pytest.mark.skip_on_windows +def test_run(): + """ + Tests end result when a command is not found + """ + with patch("salt.modules.cmdmod._is_valid_shell", MagicMock(return_value=True)): + with patch("salt.utils.platform.is_windows", MagicMock(return_value=False)): + with patch("os.path.isfile", MagicMock(return_value=True)): + with patch("os.access", MagicMock(return_value=True)): + ret = cmdmod._run("foo", cwd=os.getcwd(), use_vt=True).get("stderr") + assert "foo" in ret + + +@pytest.mark.skip_unless_on_windows +def test_powershell(): + """ + Tests cmd.powershell with a string value output + """ + mock_run = {"pid": 1234, "retcode": 0, "stderr": "", "stdout": '"foo"'} + with patch("salt.modules.cmdmod._run", return_value=mock_run): + ret = cmdmod.powershell("Set-ExecutionPolicy RemoteSigned") + assert ret == "foo" + + +@pytest.mark.skip_unless_on_windows +def test_powershell_empty(): + """ + Tests cmd.powershell when the output is an empty string + """ + mock_run = {"pid": 1234, "retcode": 0, "stderr": "", "stdout": ""} + with patch("salt.modules.cmdmod._run", return_value=mock_run): + ret = cmdmod.powershell("Set-ExecutionPolicy RemoteSigned") + assert ret == {} + + +def test_is_valid_shell_windows(): + """ + Tests return if running on windows + """ + with patch("salt.utils.platform.is_windows", MagicMock(return_value=True)): + assert cmdmod._is_valid_shell("foo") + + +@pytest.mark.skip_on_windows +def test_is_valid_shell_none(): + """ + Tests return of when os.path.exists(/etc/shells) isn't available + """ + with patch("os.path.exists", MagicMock(return_value=False)): + assert cmdmod._is_valid_shell("foo") is None + + +def test_is_valid_shell_available(): + """ + Tests return when provided shell is available + """ + with patch("os.path.exists", MagicMock(return_value=True)): + with patch("salt.utils.files.fopen", mock_open(read_data=MOCK_SHELL_FILE)): + assert cmdmod._is_valid_shell("/bin/bash") + + +@pytest.mark.skip_on_windows +def test_is_valid_shell_unavailable(): + """ + Tests return when provided shell is not available + """ + with patch("os.path.exists", MagicMock(return_value=True)): + with patch("salt.utils.files.fopen", mock_open(read_data=MOCK_SHELL_FILE)): + assert not cmdmod._is_valid_shell("foo") + + +@pytest.mark.skip_on_windows +def test_os_environment_remains_intact(): + """ + Make sure the OS environment is not tainted after running a command + that specifies runas. + """ + with patch("pwd.getpwnam") as getpwnam_mock: + with patch("subprocess.Popen") as popen_mock: + environment = os.environ.copy() + + popen_mock.return_value = Mock( + communicate=lambda *args, **kwags: [b"", None], + pid=lambda: 1, + retcode=0, + ) + + with patch.dict( + cmdmod.__grains__, {"os": "Darwin", "os_family": "Solaris"} + ): + if sys.platform.startswith(("freebsd", "openbsd")): + shell = "/bin/sh" + else: + shell = "/bin/bash" + + cmdmod._run( + "ls", cwd=tempfile.gettempdir(), runas="foobar", shell=shell + ) + + environment2 = os.environ.copy() + + assert environment == environment2 + + if not salt.utils.platform.is_darwin(): + getpwnam_mock.assert_called_with("foobar") + + +@pytest.mark.skip_unless_on_darwin +def test_shell_properly_handled_on_macOS(): + """ + cmd.run should invoke a new bash login only + when bash is the default shell for the selected user + """ + + class _CommandHandler: + """ + Class for capturing cmd + """ + + def __init__(self): + self.cmd = None + + def clear(self): + self.cmd = None + + cmd_handler = _CommandHandler() + + def mock_proc(__cmd__, **kwargs): + cmd_handler.cmd = " ".join(__cmd__) + return MagicMock(return_value=MockTimedProc(stdout=None, stderr=None)) + + with patch("pwd.getpwnam") as getpwnam_mock: + with patch("salt.utils.timed_subprocess.TimedProc", mock_proc): + + # User default shell is '/usr/local/bin/bash' + user_default_shell = "/usr/local/bin/bash" + with patch.dict( + cmdmod.__salt__, + {"user.info": MagicMock(return_value={"shell": user_default_shell})}, + ): + + cmd_handler.clear() + cmdmod._run( + "ls", cwd=tempfile.gettempdir(), runas="foobar", use_vt=False + ) + + assert re.search( + "{} -l -c".format(user_default_shell), cmd_handler.cmd + ), "cmd invokes right bash session on macOS" + + # User default shell is '/bin/zsh' + user_default_shell = "/bin/zsh" + with patch.dict( + cmdmod.__salt__, + {"user.info": MagicMock(return_value={"shell": user_default_shell})}, + ): + + cmd_handler.clear() + cmdmod._run( + "ls", cwd=tempfile.gettempdir(), runas="foobar", use_vt=False + ) + + assert not re.search( + "bash -l -c", cmd_handler.cmd + ), "cmd does not invoke user shell on macOS" + + +def test_run_cwd_doesnt_exist_issue_7154(): + """ + cmd.run should fail and raise + salt.exceptions.CommandExecutionError if the cwd dir does not + exist + """ + cmd = "echo OHAI" + cwd = "/path/to/nowhere" + with pytest.raises(CommandExecutionError): + cmdmod.run_all(cmd, cwd=cwd) + + +@pytest.mark.skip_on_darwin +@pytest.mark.skip_on_windows +def test_run_cwd_in_combination_with_runas(): + """ + cmd.run executes command in the cwd directory + when the runas parameter is specified + """ + cmd = "pwd" + cwd = "/tmp" + runas = os.getlogin() + + with patch.dict(cmdmod.__grains__, {"os": "Darwin", "os_family": "Solaris"}): + stdout = cmdmod._run(cmd, cwd=cwd, runas=runas).get("stdout") + assert stdout == cwd + + +def test_run_all_binary_replace(): + """ + Test for failed decoding of binary data, for instance when doing + something silly like using dd to read from /dev/urandom and write to + /dev/stdout. + """ + # Since we're using unicode_literals, read the random bytes from a file + rand_bytes_file = os.path.join(RUNTIME_VARS.BASE_FILES, "random_bytes") + with salt.utils.files.fopen(rand_bytes_file, "rb") as fp_: + stdout_bytes = fp_.read() + + # kitchen-salt uses unix2dos on all the files before copying them over + # to the vm that will be running the tests. It skips binary files though + # The file specified in `rand_bytes_file` is detected as binary so the + # Unix-style line ending remains. This should account for that. + stdout_bytes = stdout_bytes.rstrip() + os.linesep.encode() + + # stdout with the non-decodable bits replaced with the unicode + # replacement character U+FFFD. + stdout_unicode = "\ufffd\x1b\ufffd\ufffd" + os.linesep + stderr_bytes = ( + os.linesep.encode().join( + [ + b"1+0 records in", + b"1+0 records out", + b"4 bytes copied, 9.1522e-05 s, 43.7 kB/s", + ] + ) + + os.linesep.encode() + ) + stderr_unicode = stderr_bytes.decode() + + proc = MagicMock( + return_value=MockTimedProc(stdout=stdout_bytes, stderr=stderr_bytes) + ) + with patch("salt.utils.timed_subprocess.TimedProc", proc): + ret = cmdmod.run_all( + "dd if=/dev/urandom of=/dev/stdout bs=4 count=1", rstrip=False + ) + + assert ret["stdout"] == stdout_unicode + assert ret["stderr"] == stderr_unicode + + +def test_run_all_none(): + """ + Tests cases when proc.stdout or proc.stderr are None. These should be + caught and replaced with empty strings. + """ + proc = MagicMock(return_value=MockTimedProc(stdout=None, stderr=None)) + with patch("salt.utils.timed_subprocess.TimedProc", proc): + ret = cmdmod.run_all("some command", rstrip=False) + + assert ret["stdout"] == "" + assert ret["stderr"] == "" + + +def test_run_all_unicode(): + """ + Ensure that unicode stdout and stderr are decoded properly + """ + stdout_unicode = "Here is some unicode: спам" + stderr_unicode = "Here is some unicode: яйца" + stdout_bytes = stdout_unicode.encode("utf-8") + stderr_bytes = stderr_unicode.encode("utf-8") + + proc = MagicMock( + return_value=MockTimedProc(stdout=stdout_bytes, stderr=stderr_bytes) + ) + + with patch("salt.utils.timed_subprocess.TimedProc", proc), patch.object( + builtins, "__salt_system_encoding__", "utf-8" + ): + ret = cmdmod.run_all("some command", rstrip=False) + + assert ret["stdout"] == stdout_unicode + assert ret["stderr"] == stderr_unicode + + +def test_run_all_output_encoding(): + """ + Test that specifying the output encoding works as expected + """ + stdout = "Æ" + stdout_latin1_enc = stdout.encode("latin1") + + proc = MagicMock(return_value=MockTimedProc(stdout=stdout_latin1_enc)) + + with patch("salt.utils.timed_subprocess.TimedProc", proc), patch.object( + builtins, "__salt_system_encoding__", "utf-8" + ): + ret = cmdmod.run_all("some command", output_encoding="latin1") + + assert ret["stdout"] == stdout + + +def test_run_all_output_loglevel_quiet(caplog): + """ + Test that specifying quiet for loglevel + does not log the command. + """ + stdout = b"test" + proc = MagicMock(return_value=MockTimedProc(stdout=stdout)) + + msg = "Executing command 'some command' in directory" + with patch("salt.utils.timed_subprocess.TimedProc", proc): + with caplog.at_level(logging.DEBUG, logger="salt.modules.cmdmod"): + ret = cmdmod.run_all("some command", output_loglevel="quiet") + assert msg not in caplog.text + + assert ret["stdout"] == salt.utils.stringutils.to_unicode(stdout) + + +def test_run_all_output_loglevel_debug(caplog): + """ + Test that specifying debug for loglevel + does log the command. + """ + stdout = b"test" + proc = MagicMock(return_value=MockTimedProc(stdout=stdout)) + + msg = "Executing command 'some' in directory" + with patch("salt.utils.timed_subprocess.TimedProc", proc): + with caplog.at_level(logging.DEBUG, logger="salt.modules.cmdmod"): + ret = cmdmod.run_all("some command", output_loglevel="debug") + assert msg in caplog.text + + assert ret["stdout"] == salt.utils.stringutils.to_unicode(stdout) + + +def test_run_chroot_mount(): + """ + Test cmdmod.run_chroot mount / umount balance + """ + mock_mount = MagicMock() + mock_umount = MagicMock() + mock_run_all = MagicMock() + with patch.dict( + cmdmod.__salt__, {"mount.mount": mock_mount, "mount.umount": mock_umount} + ): + with patch("salt.modules.cmdmod.run_all", mock_run_all): + cmdmod.run_chroot("/mnt", "cmd") + assert mock_mount.call_count == 3 + assert mock_umount.call_count == 3 + + +def test_run_chroot_mount_bind(): + """ + Test cmdmod.run_chroot mount / umount balance with bind mount + """ + mock_mount = MagicMock() + mock_umount = MagicMock() + mock_run_all = MagicMock() + with patch.dict( + cmdmod.__salt__, {"mount.mount": mock_mount, "mount.umount": mock_umount} + ): + with patch("salt.modules.cmdmod.run_all", mock_run_all): + cmdmod.run_chroot("/mnt", "cmd", binds=["/var"]) + assert mock_mount.call_count == 4 + assert mock_umount.call_count == 4 + + +@pytest.mark.skip_on_windows +def test_run_chroot_runas(): + """ + Test run_chroot when a runas parameter is provided + """ + with patch.dict( + cmdmod.__salt__, {"mount.mount": MagicMock(), "mount.umount": MagicMock()} + ): + with patch("salt.modules.cmdmod.run_all") as run_all_mock: + cmdmod.run_chroot("/mnt", "ls", runas="foobar", shell="/bin/sh") + run_all_mock.assert_called_with( + "chroot --userspec foobar: /mnt /bin/sh -c ls", + bg=False, + clean_env=False, + cwd=None, + env=None, + ignore_retcode=False, + log_callback=None, + output_encoding=None, + output_loglevel="quiet", + pillar=None, + pillarenv=None, + python_shell=True, + reset_system_locale=True, + rstrip=True, + saltenv="base", + shell="/bin/sh", + stdin=None, + success_retcodes=None, + template=None, + timeout=None, + umask=None, + use_vt=False, + ) + + +def test_cve_2021_25284(caplog): + proc = MagicMock( + return_value=MockTimedProc(stdout=b"foo", stderr=b"wtf", returncode=2) + ) + with patch("salt.utils.timed_subprocess.TimedProc", proc): + with caplog.at_level(logging.DEBUG, logger="salt.modules.cmdmod"): + cmdmod.run("testcmd -p ImAPassword", output_loglevel="error") + assert "ImAPassword" not in caplog.text diff --git a/tests/unit/auth/test_auth.py b/tests/unit/auth/test_auth.py index f2baaf1e237f..3e3de7eb192a 100644 --- a/tests/unit/auth/test_auth.py +++ b/tests/unit/auth/test_auth.py @@ -1,15 +1,13 @@ -# -*- coding: utf-8 -*- import os import time + import salt.auth import salt.config from tests.support.runtests import RUNTIME_VARS from tests.support.unit import TestCase - class AuthTest(TestCase): - def test_cve_2021_3244(self): opts = { "extension_modules": "", @@ -19,22 +17,19 @@ def test_cve_2021_3244(self): "eauth_tokens": "localfs", "token_dir": RUNTIME_VARS.TMP, "token_expire_user_override": True, - "external_auth": { - "auto": { - "foo": [] - } - } + "external_auth": {"auto": {"foo": []}}, } auth = salt.auth.LoadAuth(opts) load = { "eauth": "auto", "username": "foo", "password": "foo", - "token_expire": -1} + "token_expire": -1, + } t_data = auth.mk_token(load) - assert t_data['expire'] < time.time() - token_file = os.path.join(RUNTIME_VARS.TMP, t_data['token']) + assert t_data["expire"] < time.time() + token_file = os.path.join(RUNTIME_VARS.TMP, t_data["token"]) assert os.path.exists(token_file) - t_data = auth.get_tok(t_data['token']) + t_data = auth.get_tok(t_data["token"]) assert not os.path.exists(token_file) assert t_data == {} diff --git a/tests/unit/client/test_ssh.py b/tests/unit/client/test_ssh.py index cc5eee71d272..3b8f9ee5a58d 100644 --- a/tests/unit/client/test_ssh.py +++ b/tests/unit/client/test_ssh.py @@ -8,6 +8,7 @@ import shutil import tempfile +import salt.client.ssh.client import salt.config import salt.roster import salt.utils.files @@ -667,3 +668,26 @@ def test_parse_tgt_no_user(self): assert client.parse_tgt["hostname"] == host assert client.parse_tgt["user"] == opts["ssh_user"] assert self.opts.get("ssh_cli_tgt") == host + + def test_extra_filerefs(self): + """ + test "extra_filerefs" are not excluded from kwargs + when preparing the SSH opts + """ + opts = { + "eauth": "auto", + "username": "test", + "password": "test", + "client": "ssh", + "tgt": "localhost", + "fun": "test.ping", + "ssh_port": 22, + "extra_filerefs": "salt://foobar", + } + roster = os.path.join(RUNTIME_VARS.TMP_CONF_DIR, "roster") + client = salt.client.ssh.client.SSHClient( + mopts=self.opts, disable_custom_roster=True + ) + with patch("salt.roster.get_roster_file", MagicMock(return_value=roster)): + ssh_obj = client._prep_ssh(**opts) + assert ssh_obj.opts.get("extra_filerefs", None) == "salt://foobar" diff --git a/tests/unit/modules/test_cmdmod.py b/tests/unit/modules/test_cmdmod.py index 29167e9ab66b..e53410ed1474 100644 --- a/tests/unit/modules/test_cmdmod.py +++ b/tests/unit/modules/test_cmdmod.py @@ -568,7 +568,7 @@ def test_run_all_output_loglevel_debug(self): stdout = b"test" proc = MagicMock(return_value=MockTimedProc(stdout=stdout)) - msg = "INFO:Executing command 'some command' in directory" + msg = "INFO:Executing command 'some' in directory" with patch("salt.utils.timed_subprocess.TimedProc", proc): with TstSuiteLoggingHandler() as log_handler: ret = cmdmod.run_all("some command", output_loglevel="debug") @@ -643,11 +643,7 @@ def test_run_chroot_runas(self): def test_cve_2021_25284(self): proc = MagicMock( - return_value=MockTimedProc( - stdout=b"foo", - stderr=b"wtf", - returncode=2 - ) + return_value=MockTimedProc(stdout=b"foo", stderr=b"wtf", returncode=2) ) with patch("salt.utils.timed_subprocess.TimedProc", proc): with TstSuiteLoggingHandler() as log_handler: diff --git a/tests/unit/test_module_names.py b/tests/unit/test_module_names.py index a248ac6116e8..bfc4a612dd5e 100644 --- a/tests/unit/test_module_names.py +++ b/tests/unit/test_module_names.py @@ -205,6 +205,7 @@ def test_module_name_source_match(self): "unit.utils.scheduler.test_run_job", "unit.utils.scheduler.test_schedule", "unit.utils.scheduler.test_skip", + "unit.auth.test_auth", ) errors = []