From 9aee0d94e57901f8b5e875232a5a32ed2dd8349b Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Sat, 1 Aug 2026 17:18:27 -0700 Subject: [PATCH] Skip saltutil runner/wheel privilege drop on invalid user state.orchestrate overwrites __opts__["user"] with __user__ (the publishing user, which is either salt.utils.user.get_specific_user() -> "sudo_" under sudo, or the reactor's "Reactor" sentinel when the orchestration was triggered by a reactor). The post-#67716 privilege-drop path in saltutil.runner/saltutil.wheel reads that value as the runas target and asks chugid to switch to it, which then raises KeyError from pwd.getpwnam wrapped in CommandExecutionError: Failed to run 'cache.grains' as user 'sudo_alice': KeyError: "getpwnam(): name not found: 'sudo_alice'" Failed to run 'queue.insert' as user 'Reactor': KeyError: "getpwnam(): name not found: 'Reactor'" Validate the candidate against the passwd database in _master_user_runas and skip the privilege drop when it does not resolve to a real account, falling back to historical in-process behavior. Merge-forward of #69609 (which landed on 3006.x/3007.x for #69600) plus a regression test for the reactor variant. Fixes #69833 --- changelog/69600.fixed.md | 1 + changelog/69833.fixed.md | 1 + salt/modules/saltutil.py | 20 ++++++++ tests/pytests/unit/modules/test_saltutil.py | 57 ++++++++++++++++++++- 4 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 changelog/69600.fixed.md create mode 100644 changelog/69833.fixed.md diff --git a/changelog/69600.fixed.md b/changelog/69600.fixed.md new file mode 100644 index 000000000000..af8611001e61 --- /dev/null +++ b/changelog/69600.fixed.md @@ -0,0 +1 @@ +Fixed ``saltutil.runner`` and ``saltutil.wheel`` raising ``KeyError: "getpwnam(): name not found: 'sudo_'"`` when an orchestration (``salt-run state.orchestrate``) was launched under ``sudo`` and the rendered SLS called ``salt.saltutil.runner`` from Jinja. ``state.orchestrate`` overwrites ``__opts__["user"]`` with the publishing user (``salt.utils.user.get_specific_user()``, which returns ``"sudo_"`` under ``sudo``), and the post-#67716 privilege-drop path then tried to ``chugid`` to that non-existent account. The privilege-drop helper now validates the candidate against the passwd database and skips the drop when the configured ``user`` is not a real account, falling back to the historical in-process behavior. diff --git a/changelog/69833.fixed.md b/changelog/69833.fixed.md new file mode 100644 index 000000000000..c4fc668129ca --- /dev/null +++ b/changelog/69833.fixed.md @@ -0,0 +1 @@ +Fixed ``saltutil.runner`` raising ``KeyError: "getpwnam(): name not found: 'Reactor'"`` when a reactor-triggered orchestration rendered an SLS that called ``salt.saltutil.runner`` from Jinja. The reactor stamps ``__user__ = "Reactor"`` on runner/wheel low data (to distinguish reactor-fired events from user-fired ones); ``state.orchestrate`` then copies that sentinel into ``__opts__["user"]``, and the post-#67716 privilege-drop path tried to ``chugid`` to a non-existent ``Reactor`` account. Same underlying fix as #69600: ``_master_user_runas`` now validates the candidate against the passwd database and skips the privilege drop when it does not resolve to a real user. diff --git a/salt/modules/saltutil.py b/salt/modules/saltutil.py index f60be5e4dac0..a8ae9653a6a6 100644 --- a/salt/modules/saltutil.py +++ b/salt/modules/saltutil.py @@ -1952,6 +1952,15 @@ def _master_user_runas(opts): the Salt master runs as the ``salt`` user by default, so those functions would otherwise touch master-owned resources (the git_pillar/gitfs cache, the pki tree, ...) as the wrong user. See #67716. + + The ``user`` value in ``opts`` is not always the master's configured + daemon user: ``state.orchestrate`` overwrites ``__opts__['user']`` with + the publishing user (``salt.utils.user.get_specific_user()``), which + returns ``"sudo_"`` when the call was made under ``sudo``. That + is not a real account, so attempting to drop to it would later raise + ``KeyError`` from ``pwd.getpwnam`` inside ``chugid``. Validate the + candidate against the passwd database and skip the privilege drop when + it does not resolve to a real user. See #69600. """ runas = opts.get("user") if not runas or runas == salt.utils.user.get_user(): @@ -1959,6 +1968,17 @@ def _master_user_runas(opts): # Changing users requires root; otherwise keep the historical behavior. if not hasattr(os, "geteuid") or os.geteuid() != 0: return None + if pwd is not None: + try: + pwd.getpwnam(runas) + except KeyError: + log.debug( + "Not dropping privileges: '%s' is not a real user on this " + "system (likely the publishing user copied into opts by " + "state.orchestrate, e.g. 'sudo_').", + runas, + ) + return None return runas diff --git a/tests/pytests/unit/modules/test_saltutil.py b/tests/pytests/unit/modules/test_saltutil.py index 16ea9666896c..ff7dca30171a 100644 --- a/tests/pytests/unit/modules/test_saltutil.py +++ b/tests/pytests/unit/modules/test_saltutil.py @@ -235,7 +235,12 @@ def cmd(self, name, **kwargs): ({}, 0, "root", None), ), ) -def test_master_user_runas(opts, euid, current_user, expected): +def test_master_user_runas(opts, euid, current_user, expected, monkeypatch): + # The candidate user is validated against the passwd database; stub it + # so the configured ``salt`` user appears to exist on the test host. + monkeypatch.setattr( + saltutil, "pwd", types.SimpleNamespace(getpwnam=lambda user: None) + ) with patch("os.geteuid", return_value=euid), patch( "salt.utils.user.get_user", return_value=current_user ): @@ -401,6 +406,56 @@ def _raise(user): assert os.environ["HOME"] == "/root" +def test_master_user_runas_unknown_user_returns_none(monkeypatch): + """ + When ``opts['user']`` is not a real account on the system, + ``_master_user_runas`` must return ``None`` instead of returning a + name that would later blow up in ``pwd.getpwnam`` inside + ``_client_cmd_as`` / ``chugid`` (#69600). + + Regression: ``state.orchestrate`` overwrites ``__opts__['user']`` + with ``__user__`` (the value of ``salt.utils.user.get_specific_user()``), + which is ``"sudo_"`` whenever ``salt-run`` was launched under + ``sudo``. That name has no passwd entry, so attempting to drop to it + raised ``KeyError: "getpwnam(): name not found: 'sudo_'"`` + wrapped in ``CommandExecutionError``. + """ + + def _raise(user): + raise KeyError(user) + + monkeypatch.setattr(saltutil, "pwd", types.SimpleNamespace(getpwnam=_raise)) + with patch("os.geteuid", return_value=0), patch( + "salt.utils.user.get_user", return_value="root" + ): + assert saltutil._master_user_runas({"user": "sudo_alice"}) is None + + +def test_master_user_runas_reactor_sentinel_returns_none(monkeypatch): + """ + Same class of bug as #69600 but triggered from the reactor: when the + master (running as root) reacts to an event that runs + ``state.orchestrate``, ``salt.utils.reactor.ReactWrap`` stamps + ``__user__ = "Reactor"`` on runner/wheel low data so reactor-fired + events can be told apart from user-fired ones. + ``state.orchestrate`` copies that value into ``__opts__["user"]``, so + any downstream ``salt['saltutil.runner'](...)`` call from a rendered + SLS reaches ``_master_user_runas`` with ``runas="Reactor"``, which + has no passwd entry. ``_master_user_runas`` must return ``None`` + instead of returning ``"Reactor"`` and blowing up later in + ``chugid``. See #69833. + """ + + def _raise(user): + raise KeyError(user) + + monkeypatch.setattr(saltutil, "pwd", types.SimpleNamespace(getpwnam=_raise)) + with patch("os.geteuid", return_value=0), patch( + "salt.utils.user.get_user", return_value="root" + ): + assert saltutil._master_user_runas({"user": "Reactor"}) is None + + def test_align_runas_environment_without_pwd_is_noop(monkeypatch): """On platforms without the pwd module (Windows) the helper is a no-op.""" monkeypatch.setattr(saltutil, "pwd", None)