From 7eaf551b24cf01664fa73328db26277a4de79183 Mon Sep 17 00:00:00 2001 From: Mike Aldred Date: Thu, 19 Mar 2026 23:21:12 +0800 Subject: [PATCH] pfexec become plugin: fix broken defaults for illumos/SmartOS The pfexec become plugin has had incorrect defaults since it was migrated from Ansible core, making it unusable on illumos without manual workarounds: 1. become_flags defaulted to '-H -S -n' which are sudo flags. pfexec does not accept any of these options, causing: 'exec: illegal option -- H' 2. wrap_exe defaulted to false. Unlike sudo, pfexec does not interpret shell constructs internally. Since Ansible generates compound commands (echo BECOME-SUCCESS-xxx ; python3), these must be wrapped in /bin/sh -c for pfexec to execute them. These issues were originally reported in 2016 (ansible/ansible#15642), migrated to community.general as #3671, and partially fixed by PR #3889 in 2022 (which corrected quoting but not the defaults). Users have had to work around this with explicit inventory settings ever since. Changes: - become_flags default: '-H -S -n' -> '' (empty) - wrap_exe default: false -> true - build_become_command: handle empty flags cleanly - Updated tests to match corrected defaults - Added test for custom flags - Improved wrap_exe description to explain why it should be enabled --- changelogs/fragments/pfexec-fix-defaults.yml | 3 ++ plugins/become/pfexec.py | 15 +++++-- tests/unit/plugins/become/test_pfexec.py | 43 ++++++++++++++++---- 3 files changed, 48 insertions(+), 13 deletions(-) create mode 100644 changelogs/fragments/pfexec-fix-defaults.yml diff --git a/changelogs/fragments/pfexec-fix-defaults.yml b/changelogs/fragments/pfexec-fix-defaults.yml new file mode 100644 index 0000000000..7975c9e772 --- /dev/null +++ b/changelogs/fragments/pfexec-fix-defaults.yml @@ -0,0 +1,3 @@ +bugfixes: + - "pfexec become plugin - fix default ``become_flags`` from ``-H -S -n`` (sudo flags) to empty string, as ``pfexec`` does not accept these options (https://github.com/ansible-collections/community.general/pull/XXXXX)." + - "pfexec become plugin - change default ``wrap_exe`` from ``false`` to ``true``, as ``pfexec`` does not interpret shell constructs internally and requires commands to be wrapped in a shell invocation (https://github.com/ansible-collections/community.general/pull/XXXXX)." diff --git a/plugins/become/pfexec.py b/plugins/become/pfexec.py index 8863b946d7..7efd21cd8d 100644 --- a/plugins/become/pfexec.py +++ b/plugins/become/pfexec.py @@ -46,7 +46,7 @@ options: become_flags: description: Options to pass to C(pfexec). type: string - default: -H -S -n + default: "" ini: - section: privilege_escalation key: become_flags @@ -73,8 +73,12 @@ options: - section: pfexec_become_plugin key: password wrap_exe: - description: Toggle to wrap the command C(pfexec) calls in C(shell -c) or not. - default: false + description: + - Toggle to wrap the command C(pfexec) calls in C(shell -c) or not. + - Unlike C(sudo), C(pfexec) does not interpret shell constructs internally, + so commands containing shell operators must be wrapped in a shell invocation. + - This should generally be left enabled. + default: true type: bool ini: - section: pfexec_become_plugin @@ -103,4 +107,7 @@ class BecomeModule(BecomeBase): flags = self.get_option("become_flags") noexe = not self.get_option("wrap_exe") - return f"{exe} {flags} {self._build_success_command(cmd, shell, noexe=noexe)}" + become_cmd = self._build_success_command(cmd, shell, noexe=noexe) + if flags: + return f"{exe} {flags} {become_cmd}" + return f"{exe} {become_cmd}" diff --git a/tests/unit/plugins/become/test_pfexec.py b/tests/unit/plugins/become/test_pfexec.py index 9ccddabd61..f8f17f2239 100644 --- a/tests/unit/plugins/become/test_pfexec.py +++ b/tests/unit/plugins/become/test_pfexec.py @@ -15,13 +15,13 @@ from .helper import call_become_plugin def test_pfexec_basic(mocker, parser, reset_cli_args): + """Test pfexec with default settings (no flags, wrap_exe enabled).""" options = parser.parse_args([]) context._init_global_context(options) default_cmd = "/bin/foo" default_exe = "/bin/bash" pfexec_exe = "pfexec" - pfexec_flags = "-H -S -n" success = "BECOME-SUCCESS-.+?" @@ -31,39 +31,63 @@ def test_pfexec_basic(mocker, parser, reset_cli_args): var_options = {} cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) print(cmd) - assert re.match(f"""{pfexec_exe} {pfexec_flags} 'echo {success}; {default_cmd}'""", cmd) is not None + # With wrap_exe=true (default), command is wrapped in shell -c + assert re.match(f"""{pfexec_exe} {default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None -def test_pfexec(mocker, parser, reset_cli_args): +def test_pfexec_no_wrap(mocker, parser, reset_cli_args): + """Test pfexec with wrap_exe disabled (legacy behaviour).""" options = parser.parse_args([]) context._init_global_context(options) default_cmd = "/bin/foo" default_exe = "/bin/bash" pfexec_exe = "pfexec" - pfexec_flags = "" success = "BECOME-SUCCESS-.+?" task = { - "become_user": "foo", + "become_method": "community.general.pfexec", + "become_flags": "", + } + var_options = { + "ansible_pfexec_wrap_execution": "false", + } + cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) + print(cmd) + assert re.match(f"""{pfexec_exe} 'echo {success}; {default_cmd}'""", cmd) is not None + + +def test_pfexec_custom_flags(mocker, parser, reset_cli_args): + """Test pfexec with custom flags.""" + options = parser.parse_args([]) + context._init_global_context(options) + + default_cmd = "/bin/foo" + default_exe = "/bin/bash" + pfexec_exe = "pfexec" + pfexec_flags = "-P basic" + + success = "BECOME-SUCCESS-.+?" + + task = { "become_method": "community.general.pfexec", "become_flags": pfexec_flags, } var_options = {} cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) print(cmd) - assert re.match(f"""{pfexec_exe} {pfexec_flags} 'echo {success}; {default_cmd}'""", cmd) is not None + assert re.match(f"""{pfexec_exe} {pfexec_flags} {default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None def test_pfexec_varoptions(mocker, parser, reset_cli_args): + """Test that var_options override task options.""" options = parser.parse_args([]) context._init_global_context(options) default_cmd = "/bin/foo" default_exe = "/bin/bash" pfexec_exe = "pfexec" - pfexec_flags = "" success = "BECOME-SUCCESS-.+?" @@ -74,8 +98,9 @@ def test_pfexec_varoptions(mocker, parser, reset_cli_args): } var_options = { "ansible_become_user": "bar", - "ansible_become_flags": pfexec_flags, + "ansible_become_flags": "", } cmd = call_become_plugin(task, var_options, cmd=default_cmd, executable=default_exe) print(cmd) - assert re.match(f"""{pfexec_exe} {pfexec_flags} 'echo {success}; {default_cmd}'""", cmd) is not None + # var_options override task flags, so flags should be empty + assert re.match(f"""{pfexec_exe} {default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None