[Bug]: Regression in command parsing of cmd.run module?

[DickerDackel]

What happened?

I just updated a few minions from 3007.1 to 3007.7, and now pipes no longer worked when calling cmd.run on the updated minions.

A small test script shows, that on these minions, the | character is passed verbatim to the arguments, while the documentation still says that the command is executed by either a given or the default shell.

# test script:
if [ "$1" == '|'; then
    echo b0rken
else
    echo fine
fi

On updated minions, this test fails unless I explicitely add shell=/bin/bash to the command.

I copied that to /tmp on an outdated minion and the master itself.

root@salt:~# salt $HOSTNAME cmd.run '/tmp/test-script | wc -l'
salt:
    b0rken
root@salt:~# salt $HOSTNAME cmd.run '/tmp/test-script | wc -l' shell=/bin/bash
salt:
    1
root@salt:~# # this is the outdated minion:
root@salt:~# salt ansible cmd.run '/tmp/test-script | wc -l' shell=/bin/bash
ansible:
    1
root@salt:~#

On the outdated minions, test-script never sees the |, since that's handled from the shell wrapping everything. But on current minions, the test-script gets |, wc and -l as additional parameters.

The issue shows both via salt cmd.run from the master, and salt-call cmd.run on the minion.

Type of salt install

Official rpm

Major version

3007.x

What supported OS are you seeing the problem on? Can select multiple. (If bug appears on an unsupported OS, please open a GitHub Discussion instead)

rockylinux-9

salt --versions-report output

root@salt:/srv/salt# salt --versions-report
Salt Version:
          Salt: 3007.7

Python Version:
        Python: 3.10.18 (main, Aug 27 2025, 22:35:38) [GCC 11.2.0]

Dependency Versions:
          cffi: 1.16.0
      cherrypy: unknown
  cryptography: 42.0.5
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.6
       libgit2: Not Installed
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.7
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 24.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.5.2
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: 0.20.5
         smmap: Not Installed
       timelib: 0.3.0
       Tornado: 6.4.2
           ZMQ: 4.3.4

Salt Package Information:
  Package Type: onedir

System Versions:
          dist: rocky 9.5 Blue Onyx
        locale: utf-8
       machine: x86_64
       release: 6.15.9-101.fc41.x86_64
        system: Linux
       version: Rocky Linux 9.5 Blue Onyx

root@salt:/srv/salt#

[twangboy]

@xsmile

[twangboy]

Try passing python_shell=True

[DickerDackel]

Try passing python_shell=True

That works, but does that matter, since
a. it worked without it before
b. the docs state that the default shell is used (which is /bin/bash on my system for the salt-minion user)
c. the examples in the docs further down use pipe symbols without that parameter.

And even if the project decided to create this breaking change and that this flag is mandatory in the future, shouldn't there be a version tag and a remark that this is a breaking change at either the shell= or the python_shell= docs?

[dwoz]

@twangboy @xsmile

We should not be introducing breaking chances in 3006 or 3007. I understand the fixes happened for a reason and the way it works now is what we desire but this should have happened on the master with clear messaging about it for 3008.

[DickerDackel]

We should not be introducing breaking chances in 3006 or 3007. I understand the fixes happened for a reason and the way it works now is what we desire but this should have happened on the master with clear messaging about it for 3008.

If this is the planned behaviour that we have to expect in the future, then the documentation needs to be fixed in at least 2, better 4 places:

• The mentioning that the default shell is used, suggests that shell functionality is available without extra options.
• The examples use pipes without the python_shell=True flag
• At the description body, and both the shell= and python_shell= sections, there should be version tags pointing at the incompatibility with previous versions

And a global setting in the minion config would also be useful, thinking of how many report and maintenance scripts we alone have, that would all need patching. I'm sure that's similar for many other installs.

Personally, I would offer this as a hardening flag to be enabled and leave the default as is.

[twangboy]

@xsmile ^^^