From 7eaf551b24cf01664fa73328db26277a4de79183 Mon Sep 17 00:00:00 2001 From: Mike Aldred Date: Thu, 19 Mar 2026 23:21:12 +0800 Subject: [PATCH 1/4] 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 From 47ec4f62d58721c4a4155e6759f9eeb82921d9d7 Mon Sep 17 00:00:00 2001 From: Mike Aldred Date: Thu, 19 Mar 2026 23:23:48 +0800 Subject: [PATCH 2/4] Update changelog fragment with PR number --- changelogs/fragments/pfexec-fix-defaults.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelogs/fragments/pfexec-fix-defaults.yml b/changelogs/fragments/pfexec-fix-defaults.yml index 7975c9e772..c714e833d3 100644 --- a/changelogs/fragments/pfexec-fix-defaults.yml +++ b/changelogs/fragments/pfexec-fix-defaults.yml @@ -1,3 +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)." + - "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/11623)." + - "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/11623)." From 192759b85da57011efca2062d2b7687e5d402398 Mon Sep 17 00:00:00 2001 From: Mike Aldred Date: Fri, 20 Mar 2026 06:30:30 +0800 Subject: [PATCH 3/4] Fix ruff formatting in test_pfexec.py --- tests/unit/plugins/become/test_pfexec.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/plugins/become/test_pfexec.py b/tests/unit/plugins/become/test_pfexec.py index f8f17f2239..5b58ad68b9 100644 --- a/tests/unit/plugins/become/test_pfexec.py +++ b/tests/unit/plugins/become/test_pfexec.py @@ -77,7 +77,9 @@ def test_pfexec_custom_flags(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} {default_exe} -c '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): From 0abb9a557e0ab5f883c3cc48b3a644423f140a1d Mon Sep 17 00:00:00 2001 From: Mike Aldred Date: Fri, 20 Mar 2026 17:52:41 +0800 Subject: [PATCH 4/4] Address review feedback from russoz Remove redundant 'should generally be left enabled' description line and simplify become command return by removing unnecessary flags conditional. --- plugins/become/pfexec.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/become/pfexec.py b/plugins/become/pfexec.py index 7efd21cd8d..772cfa5383 100644 --- a/plugins/become/pfexec.py +++ b/plugins/become/pfexec.py @@ -77,7 +77,6 @@ options: - 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: @@ -108,6 +107,4 @@ class BecomeModule(BecomeBase): flags = self.get_option("become_flags") noexe = not self.get_option("wrap_exe") become_cmd = self._build_success_command(cmd, shell, noexe=noexe) - if flags: - return f"{exe} {flags} {become_cmd}" - return f"{exe} {become_cmd}" + return f"{exe} {flags} {become_cmd}"