From 153f0c34dbf7147b1e6c2fadd2f90bad6a66a5ed Mon Sep 17 00:00:00 2001 From: xsmile Date: Mon, 1 Sep 2025 20:31:30 +0200 Subject: [PATCH 1/4] cmd: change shell and python_shell defaults --- salt/modules/cmdmod.py | 2 +- salt/states/cmd.py | 35 +++++++++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 6fdd53d5e789..e34dbb01a555 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2998,7 +2998,7 @@ def _cleanup_tempfile(path): if isinstance(args, (list, tuple)): new_cmd = [path, *args] if args else [path] else: - new_cmd = [path, str(args)] if args else [path] + new_cmd = [path, *salt.utils.args.shlex_split(args)] if args else [path] ret = {} try: diff --git a/salt/states/cmd.py b/salt/states/cmd.py index df0aec96ef88..fcc8e585e85c 100644 --- a/salt/states/cmd.py +++ b/salt/states/cmd.py @@ -325,6 +325,7 @@ def wait( root=None, runas=None, shell=None, + python_shell=True, env=(), stateful=False, output_loglevel="debug", @@ -476,6 +477,7 @@ def wait_script( cwd=None, runas=None, shell=None, + python_shell=None, env=None, stateful=False, use_vt=False, @@ -614,10 +616,12 @@ def wait_script( def run( name, + args=None, cwd=None, root=None, runas=None, shell=None, + python_shell=True, env=None, prepend_path=None, stateful=False, @@ -834,7 +838,8 @@ def run( "root": root, "runas": runas, "use_vt": use_vt, - "shell": shell or __grains__["shell"], + "shell": shell, + "python_shell": python_shell, "env": env, "prepend_path": prepend_path, "output_loglevel": output_loglevel, @@ -855,12 +860,24 @@ def run( ret["comment"] = f'Desired working directory "{cwd}" is not available' return ret + if shell is None and python_shell is not True: + # The program is executed directly. Use a list to clearly separate the + # program from the arguments. String splitting may be unreliable + if isinstance(args, (list, tuple)): + cmd = [name, *args] if args else [name] + else: + cmd = [name, *salt.utils.args.shlex_split(args)] if args else [name] + else: + # Execute a shell command. Always pass the command as a string + if isinstance(args, (list, tuple)): + cmd = " ".join([name, *args]) if args else name + else: + cmd = " ".join([name, args]) if args else name + # Wow, we passed the test, run this sucker! try: run_cmd = "cmd.run_all" if not root else "cmd.run_chroot" - cmd_all = __salt__[run_cmd]( - cmd=name, timeout=timeout, python_shell=True, **cmd_kwargs - ) + cmd_all = __salt__[run_cmd](cmd=cmd, timeout=timeout, **cmd_kwargs) except Exception as err: # pylint: disable=broad-except ret["comment"] = str(err) return ret @@ -893,6 +910,7 @@ def script( runas=None, password=None, shell=None, + python_shell=None, env=None, stateful=False, timeout=None, @@ -1122,7 +1140,8 @@ def script( { "runas": runas, "password": password, - "shell": shell or __grains__["shell"], + "shell": shell, + "python_shell": python_shell, "env": env, "cwd": cwd, "template": template, @@ -1141,7 +1160,7 @@ def script( run_check_cmd_kwargs = { "cwd": cwd, "runas": runas, - "shell": shell or __grains__["shell"], + "shell": shell, } # Change the source to be the name arg if it is not specified @@ -1163,7 +1182,7 @@ def script( # Wow, we passed the test, run this sucker! try: - cmd_all = __salt__["cmd.script"](source, python_shell=True, **cmd_kwargs) + cmd_all = __salt__["cmd.script"](source, **cmd_kwargs) except (CommandExecutionError, SaltRenderError, OSError) as err: ret["comment"] = str(err) return ret @@ -1226,7 +1245,7 @@ def call( cmd_kwargs = { "cwd": kwargs.get("cwd"), "runas": kwargs.get("user"), - "shell": kwargs.get("shell") or __grains__["shell"], + "shell": kwargs.get("shell"), "env": kwargs.get("env"), "use_vt": use_vt, "output_loglevel": output_loglevel, From a5d5ef37267aeac1f11f2d04a0af68a44fdf840a Mon Sep 17 00:00:00 2001 From: xsmile Date: Mon, 1 Sep 2025 20:32:43 +0200 Subject: [PATCH 2/4] cmd: docs --- salt/modules/cmdmod.py | 16 +++++++---- salt/states/cmd.py | 62 ++++++++++++++++++++++++++++++++++++------ 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index e34dbb01a555..148d9c29e0b0 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2729,18 +2729,24 @@ def script( located on the master in the directory named spam, and is called eggs, the source string is salt://spam/eggs - :param str args: String of command line args to pass to the script. Only - used if no args are specified as part of the `name` argument. To pass a - string containing spaces in YAML, you will need to doubly-quote it. - Additionally, if you need to pass falsey values (e.g., "0", "", "False"), - you should doubly-quote them to ensure they are correctly interpreted: + :param list|str args: List or string of command line args to pass to the script. + A list should be preferred to avoid issues with quoting and special + characters. To pass a string containing spaces in YAML, you will need to + doubly-quote it. Additionally, if you need to pass falsey values + (e.g., "0", "", "False"), you should doubly-quote them to ensure they are + correctly interpreted: .. code-block:: bash + salt myminion cmd.script salt://foo.sh "['arg1','arg two','arg3']" salt myminion cmd.script salt://foo.sh "arg1 'arg two' arg3" salt myminion cmd.script salt://foo.sh "''0''" salt myminion cmd.script salt://foo.sh "''False''" + .. versionchanged:: 3006.15 + + Support a list of command line args. + :param str cwd: The directory from which to execute the command. Defaults to the directory returned from Python's tempfile.mkstemp. diff --git a/salt/states/cmd.py b/salt/states/cmd.py index fcc8e585e85c..8207e0f709aa 100644 --- a/salt/states/cmd.py +++ b/salt/states/cmd.py @@ -362,6 +362,12 @@ def wait( shell The shell to use for execution, defaults to /bin/sh + python_shell + Use the shell as the program to execute. Set to False to disable shell + features, such as pipes or redirection + + .. versionadded:: 3006.16 + env A list of environment variables to be set prior to execution. Example: @@ -517,6 +523,12 @@ def wait_script( shell The shell to use for execution, defaults to the shell grain + python_shell + Use the shell as the program to execute. Set to True to use shell features, + such as pipes or redirection + + .. versionadded:: 3006.16 + env A list of environment variables to be set prior to execution. Example: @@ -651,8 +663,19 @@ def run( refer to the :ref:`cmdmod documentation `. name - The command to execute, remember that the command will execute with the - path and permissions of the salt-minion. + The command or program to execute. When running a program, consider + disabling shell features with ``python_shell`` and using ``args`` if required. + Otherwise character escaping and quoting needs to be handled manually. + Remember that the command will execute with the path and permissions of the + salt-minion. + + args + List or string of command line args to pass to the executable. A list should + be preferred to avoid issues with quoting and special characters. To pass a + string containing spaces in YAML, you will need to doubly-quote it: "arg1 + 'arg two' arg3" + + .. versionadded:: 3006.16 cwd The current working directory to execute the command in, defaults to @@ -668,6 +691,12 @@ def run( shell The shell to use for execution, defaults to the shell grain + python_shell + Use the shell as the program to execute. Set to False to disable shell + features, such as pipes or redirection + + .. versionadded:: 3006.16 + env A list of environment variables to be set prior to execution. Example: @@ -941,6 +970,16 @@ def script( Either "cmd arg1 arg2 arg3..." (cmd is not used) or a source "salt://...". + args + List or string of command line args to pass to the executable. A list should + be preferred to avoid issues with quoting and special characters. To pass a + string containing spaces in YAML, you will need to doubly-quote it: "arg1 + 'arg two' arg3" + + .. versionchanged:: 3006.15 + + Support a list of command line args. + cwd The current working directory to execute the command in, defaults to /root @@ -975,7 +1014,18 @@ def script( parameter will be ignored on non-Windows platforms. shell - The shell to use for execution. The default is set in grains['shell'] + The shell to use for execution. Defaults to the shell grain if python_shell + is True. Examples: /bin/sh, cmd.exe + + .. versionchanged:: 3006.16 + + Unset by default to run programs without using a shell + + python_shell + Use the shell as the program to execute. Set to True to use shell features, + such as pipes or redirection + + .. versionadded:: 3006.16 env A list of environment variables to be set prior to execution. @@ -1033,12 +1083,6 @@ def script( If the command has not terminated after timeout seconds, send the subprocess sigterm, and if sigterm is ignored, follow up with sigkill - args - String of command line args to pass to the script. Only used if no - args are specified as part of the `name` argument. To pass a string - containing spaces in YAML, you will need to doubly-quote it: "arg1 - 'arg two' arg3" - creates Only run if the file specified by ``creates`` do not exist. If you specify a list of files then this state will only run if **any** of From fa6d35af722c8804722fd6a85cff02a0b15a2015 Mon Sep 17 00:00:00 2001 From: xsmile Date: Mon, 1 Sep 2025 20:33:12 +0200 Subject: [PATCH 3/4] cmd: tests --- tests/integration/files/file/base/echo.ps1 | 5 + tests/integration/files/file/base/echo.sh | 2 + tests/integration/states/test_cmd.py | 194 ++++++++++++++++++++- 3 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 tests/integration/files/file/base/echo.ps1 create mode 100644 tests/integration/files/file/base/echo.sh diff --git a/tests/integration/files/file/base/echo.ps1 b/tests/integration/files/file/base/echo.ps1 new file mode 100644 index 000000000000..a546b821a4c1 --- /dev/null +++ b/tests/integration/files/file/base/echo.ps1 @@ -0,0 +1,5 @@ +param ( + [string]$a, + [string]$b +) +Write-Output "a: $a, b: $b" diff --git a/tests/integration/files/file/base/echo.sh b/tests/integration/files/file/base/echo.sh new file mode 100644 index 000000000000..2d3d6abddd9a --- /dev/null +++ b/tests/integration/files/file/base/echo.sh @@ -0,0 +1,2 @@ +#!/usr/bin/env sh +echo "a: $1, b: $2" diff --git a/tests/integration/states/test_cmd.py b/tests/integration/states/test_cmd.py index 1e68ed354ffb..167de94dd12e 100644 --- a/tests/integration/states/test_cmd.py +++ b/tests/integration/states/test_cmd.py @@ -1,5 +1,5 @@ """ -Tests for the file state +Tests for the cmd state """ import errno @@ -16,6 +16,10 @@ from tests.support.mixins import SaltReturnAssertsMixin from tests.support.runtests import RUNTIME_VARS +pytestmark = [ + pytest.mark.windows_whitelisted, +] + class CMDTest(ModuleCase, SaltReturnAssertsMixin): """ @@ -273,3 +277,191 @@ def test_run_watch(self): ret = self.run_function("state.sls", [self.state_name]) self.assertTrue(ret[saltines_key]["result"]) self.assertTrue(ret[biscuits_key]["result"]) + + +@pytest.mark.slow_test +class CMDRun(ModuleCase, SaltReturnAssertsMixin): + """ + Validate the run function of the cmd state + """ + + def test_run_shell(self): + """ + cmd.run with shell functionality + """ + expected = "foo bar" + ret = self.run_state( + "cmd.run", + name="echo foo bar", + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_run_no_shell(self): + """ + cmd.run without shell functionality + """ + ret = self.run_state( + "cmd.run", + name="whoami", + python_shell=False, + ) + self.assertSaltTrueReturn(ret) + + def test_run_no_shell_fail(self): + """ + expect cmd.run without shell functionality to fail when running builtin + shell commands + """ + expected = "Unable to run command" + ret = self.run_state( + "cmd.run", + name="echo foo bar", + python_shell=False, + ) + self.assertSaltFalseReturn(ret) + self.assertInSaltComment(expected, ret) + + @pytest.mark.skip_on_windows + def test_run_args_string(self): + """ + processing of arguments passed as a string with cmd.run while + executing a binary/script + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.run", + name="printf", + args='"a: %s, b: %s\n" "foo bar" "baz qux"', + python_shell=False, + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + @pytest.mark.skip_on_windows + def test_run_args_list(self): + """ + processing of arguments passed as a list with cmd.run while + executing a binary/script + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.run", + name="printf", + args=["a: %s, b: %s\n", "foo bar", "baz qux"], + python_shell=False, + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + +@pytest.mark.slow_test +class CMDScript(ModuleCase, SaltReturnAssertsMixin): + """ + Validate the script function of the cmd state + """ + + @classmethod + def setUpClass(cls): + if salt.utils.platform.is_windows(): + cls.__script = "salt://echo.ps1" + else: + cls.__script = "salt://echo.sh" + + def test_script_name(self): + """ + cmd.script with the script passed via the name parameter + """ + expected = "a: , b:" + ret = self.run_state( + "cmd.script", + name=self.__script, + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_script_source(self): + """ + cmd.script with the script passed via the source parameter + """ + expected = "a: , b:" + ret = self.run_state( + "cmd.script", + name="_", + source=self.__script, + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_script_source_nameargs(self): + """ + cmd.script with the script passed via the source and arguments passed via + the name parameter. name is split on whitespace and the first element is + discarded + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.script", + name='_ "foo bar" "baz qux"', + source=self.__script, + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_script_args_string_with_source(self): + """ + cmd.script with the script passed via the source parameter and arguments + passed via the args parameter as a string + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.script", + name="_", + source=self.__script, + args='"foo bar" "baz qux"', + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_script_args_list_with_source(self): + """ + cmd.script with the script passed via the source parameter and arguments + passed via the args parameter as a list + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.script", + name="_", + source=self.__script, + args=["foo bar", "baz qux"], + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_script_args_string(self): + """ + cmd.script with the script passed via the name parameter and arguments + passed via the args parameter as a string + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.script", + name=self.__script, + args='"foo bar" "baz qux"', + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") + + def test_script_args_list(self): + """ + cmd.script with the script passed via the name parameter and arguments + passed via the args parameter as a list + """ + expected = "a: foo bar, b: baz qux" + ret = self.run_state( + "cmd.script", + name=self.__script, + args=["foo bar", "baz qux"], + ) + self.assertSaltTrueReturn(ret) + self.assertSaltStateChangesEqual(ret, expected, "stdout") From 74e4af0fad8554d5cefba7a085c0308578def3e2 Mon Sep 17 00:00:00 2001 From: xsmile Date: Mon, 1 Sep 2025 21:50:53 +0200 Subject: [PATCH 4/4] cmd: changelog --- changelog/68298.fixed.md | 1 + changelog/68301.changed.md | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 changelog/68298.fixed.md create mode 100644 changelog/68301.changed.md diff --git a/changelog/68298.fixed.md b/changelog/68298.fixed.md new file mode 100644 index 000000000000..1ca4b01d1ad7 --- /dev/null +++ b/changelog/68298.fixed.md @@ -0,0 +1 @@ +Fix passing arguments between the cmd state and execution modules diff --git a/changelog/68301.changed.md b/changelog/68301.changed.md new file mode 100644 index 000000000000..867f5792a80b --- /dev/null +++ b/changelog/68301.changed.md @@ -0,0 +1,2 @@ +Expose paramter python_shell to allow disabling shell functionality for the cmd state module. +Allow passing args to the cmd state module for cmd.run.