pfexec become plugin: fix broken defaults for illumos/SmartOS by LuminousMonkey · Pull Request #11623 · ansible-collections/community.general

LuminousMonkey · 2026-03-19T15:23:22Z

SUMMARY

The pfexec become plugin has had incorrect defaults since it was migrated from Ansible core, making it unusable on illumos/SmartOS/OmniOS/OpenIndiana without manual workarounds in the user's inventory or playbook configuration.

Two default values are wrong:

become_flags defaults to "-H -S -n" — these are sudo flags, not pfexec flags. The illumos pfexec command does not accept any of these options (-H sets HOME in sudo, -S reads password from stdin, -n is non-interactive mode). This causes exec: illegal option -- H on every invocation.
wrap_exe defaults to false. Unlike sudo, pfexec does not interpret shell constructs internally — it calls exec() directly on its argument. Since Ansible generates compound commands containing shell operators (e.g. echo BECOME-SUCCESS-xxx ; /opt/local/bin/python3), these must be wrapped in /bin/sh -c '...' for pfexec to execute them. Without wrapping, pfexec tries to exec the compound string as a single binary name and fails silently.

History:

Originally reported in 2016: become-method pfexec does not work ansible/ansible#15642
Migrated to community.general: pfexec become plugin does not work on illumos distros #3671
Partially fixed by Make pfexec become usable for illumos #3889 in 2022 (corrected quoting, but not the defaults)
The issue was closed with a documented workaround (become_flags: "" + ansible_pfexec_wrap_execution: yes), but the defaults were never corrected
The broken defaults have persisted for the entire lifetime of the plugin

Changes in this PR:

become_flags default: "-H -S -n" → "" (pfexec accepts no flags by default)
wrap_exe default: false → true (required for Ansible's compound commands)
build_become_command: handle empty flags without injecting extra whitespace
Improved wrap_exe description explaining why it should be enabled
Updated unit tests to match corrected defaults
Added test case for custom flags

Tested on: SmartOS (illumos) native zones with Ansible 13.4.0 / community.general 12.4.0, managing infrastructure via pfexec with RBAC profiles. Confirmed working with ansible.builtin.user, ansible.builtin.command, ansible.builtin.file, ansible.builtin.template, ansible.builtin.uri, and ansible.builtin.lineinfile modules.

Fixes #3671

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

plugins/become/pfexec.py

ADDITIONAL INFORMATION

Before (broken):

$ ansible-playbook site.yml --become --become-method=community.general.pfexec
fatal: [illumos-host]: FAILED! => {"msg": "exec: illegal option -- H"}

Users had to add workarounds to every inventory or playbook:

ansible_become_flags: ""
ansible_pfexec_wrap_execution: yes

After (works with no user configuration):

- hosts: illumos_zones
  become: true
  become_method: community.general.pfexec
  tasks:
    - ansible.builtin.user:
        name: admin
        profile: "Primary Administrator"

No become_flags or wrap_exe overrides needed.

Why these defaults are correct for pfexec:

Empty flags: pfexec is RBAC-based. Privileges are granted by profiles in /etc/security/user_attr and /etc/security/exec_attr. There is no password prompting (-S/-n are meaningless), and pfexec preserves the caller's environment (-H is meaningless). The pfexec binary accepts only pfexec [-P privset] cmd [arg ..].
wrap_exe=true: sudo has an internal shell parser that handles compound commands. pfexec does not — it calls exec() directly. Since Ansible always generates compound commands (the BECOME-SUCCESS echo followed by the Python interpreter), the command must be wrapped in /bin/sh -c '...' for pfexec to execute it.

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 ansible-collections#3671, and partially fixed by PR ansible-collections#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

ansibullbot · 2026-03-19T15:29:31Z

cc @None
click here for bot help

felixfontein · 2026-03-19T21:07:49Z

Thanks for your contribution!

This one is tricky. Changing defaults usually requires a longer deprecation, since they are breaking changes.

The current default of become_flags doesn't seem to work with any implementation of pfexec. For that reason, I think it's OK to change this as a bugfix. Or is anyone aware of a pfexec implementation that supported these flags? Are there other opinions?
The current default of wrap_exe is usable, for example with the ansible.builtin.raw action. It only works in this extremely special case (and potentially some action plugins that run specific commands directly) though. This might be a case where a deprecation warning makes sense, though I'd do a rather short deprecation period - if we decide to have a deprecation at all (and not just change the default).

In any case, the plugin is usable in its current form - but you have to set two configuration options (namely the ones whose default is changed by this PR).

Pinging in particular @russoz and @bcoca.

felixfontein · 2026-03-19T21:08:04Z

(The integration test failures are unrelated, but the extra sanity test failures need to be fixed.)

LuminousMonkey · 2026-03-19T22:37:01Z

Sorry about that, I admit that I don't actually touch any Python, so it was just a formatting thing for that sanity check?

I was just using Ansible for working with our SmartOS server, and got a bit of a surprise using the pfexec component.
(Really appreciate everyone's work on Ansible)

I think I've sorted the sanity check now?

bcoca · 2026-03-20T03:41:08Z

changes lgtm, the flags are legacy from when become plugins shared global defaults

felixfontein · 2026-03-20T07:09:32Z

Sorry about that, I admit that I don't actually touch any Python, so it was just a formatting thing for that sanity check?

I was just using Ansible for working with our SmartOS server, and got a bit of a surprise using the pfexec component. (Really appreciate everyone's work on Ansible)

I think I've sorted the sanity check now?

I think so, since everything's green now :)

russoz · 2026-03-20T07:10:45Z

Very tricky indeed.

About the become_flags, I am happy to take @bcoca at his word and change the default. I am not an user of pfexec, but a quick search around seems to corroborate the claim that those are sudo options and will not work in pfexec.

About the wrap_exec, will wrap_exec=true also work for the cases that are not failing now? (like ansible.builtin.raw as @felixfontein mentioned)? If those are still working, then I don't see a breaking change. The other cases were not working anyways.

russoz

Just a couple of comments on docs and code.

russoz · 2026-03-20T06:49:52Z

plugins/become/pfexec.py

+      - 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.


This comment is kinda redundant by the fact that the default is now true.

russoz · 2026-03-20T06:55:16Z

plugins/become/pfexec.py

+        if flags:
+            return f"{exe} {flags} {become_cmd}"
+        return f"{exe} {become_cmd}"


I see no point in the conditional: become_flags defaults to "", so this could be made simply:

Suggested change

if flags:

return f"{exe} {flags} {become_cmd}"

return f"{exe} {become_cmd}"

return f"{exe} {flags} {become_cmd}"

Why not do return " ".join(part for part in [exe, flags, become_cmd] if part) instead? That has no conditional, and doesn't insert unnecessary spaces.

Much better! Although, just to pick at you, it does have 3 conditionals ;-). Totally worth the trade.

Remove redundant 'should generally be left enabled' description line and simplify become command return by removing unnecessary flags conditional.

russoz · 2026-03-20T21:08:07Z

tests/unit/plugins/become/test_pfexec.py

    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


Please note that the unit tests are failing now because of the extra space generated (which is not a problem in itself), requiring the regexp to be more flexible:

Suggested change

assert re.match(f"""{pfexec_exe} {default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None

assert re.match(rf"""{pfexec_exe}\s+{default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None

Match double space in test assertions when become_flags defaults to empty string, consistent with doas, dzdo, and pbrun test patterns.

LuminousMonkey · 2026-03-22T23:27:30Z

I've updated the test regexes to use double spaces for empty flags, matching the existing pattern in test_doas, test_dzdo, and test_pbrun. Unit tests all pass locally, sorry, I should have run them before the last commit.

wrap_exe=true and raw, works okay with my books on my SmartOS server:

$ ansible gitea-testing -m raw -a "id" --become --become-method=community.general.pfexec
gitea-testing | CHANGED | rc=0 >>
uid=0(root) gid=0(root) groups=0(root),1(other)

wrap_exe=true is transparent for raw commands wrapping in /bin/sh -c doesn't seem to change behaviour.

felixfontein · 2026-03-23T05:40:13Z

I've updated the test regexes to use double spaces for empty flags, matching the existing pattern in test_doas, test_dzdo, and test_pbrun.

Why not use #11623 (comment) instead of adjusting the whitespcae in the tests?

felixfontein · 2026-03-23T06:07:22Z

wrap_exe=true and raw, works okay with my books on my SmartOS server:
$ ansible gitea-testing -m raw -a "id" --become --become-method=community.general.pfexec
gitea-testing | CHANGED | rc=0 >>
uid=0(root) gid=0(root) groups=0(root),1(other)
wrap_exe=true is transparent for raw commands wrapping in /bin/sh -c doesn't seem to change behaviour.

In that case we should deprecate the current default and switch it in a later version.

To deprecate the default, remove it (the default: xxx line) and add code to show a deprecation warning if the value is None (display.deprecated()) and treat the value as False in that case.

LuminousMonkey · 2026-03-23T06:36:29Z

I've updated the test regexes to use double spaces for empty flags, matching the existing pattern in test_doas, test_dzdo, and test_pbrun.

Why not use #11623 (comment) instead of adjusting the whitespcae in the tests?

I figured just make it consistent with all the other tests, like I said, I'm not really 100% with Python, just a tourist here. :)

I'm away from my computer, so I'll make the update inline with that comment when I can, and the other changes you suggested.

LuminousMonkey added 2 commits March 19, 2026 23:21

Update changelog fragment with PR number

47ec4f6

ansibullbot added become become plugin bug This issue/PR relates to a bug new_contributor Help guide this first time contributor plugins plugin (any type) tests tests unit tests/unit needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Mar 19, 2026

Fix ruff formatting in test_pfexec.py

192759b

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Mar 19, 2026

russoz reviewed Mar 20, 2026

View reviewed changes

Address review feedback from russoz

0abb9a5

Remove redundant 'should generally be left enabled' description line and simplify become command return by removing unnecessary flags conditional.

ansibullbot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Mar 20, 2026

russoz reviewed Mar 20, 2026

View reviewed changes

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-12 Automatically create a backport for the stable-12 branch labels Mar 22, 2026

Fix unit test regexes for empty default flags

3b3c945

Match double space in test assertions when become_flags defaults to empty string, consistent with doas, dzdo, and pbrun test patterns.

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label Mar 22, 2026

ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Mar 31, 2026

	assert re.match(f"""{pfexec_exe} {default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None
	assert re.match(rf"""{pfexec_exe}\s+{default_exe} -c 'echo {success}; {default_cmd}'""", cmd) is not None

Conversation

LuminousMonkey commented Mar 19, 2026

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

Uh oh!

ansibullbot commented Mar 19, 2026

Uh oh!

felixfontein commented Mar 19, 2026

Uh oh!

felixfontein commented Mar 19, 2026

Uh oh!

LuminousMonkey commented Mar 19, 2026

Uh oh!

bcoca commented Mar 20, 2026

Uh oh!

felixfontein commented Mar 20, 2026

Uh oh!

russoz commented Mar 20, 2026

Uh oh!

russoz left a comment

Choose a reason for hiding this comment

Uh oh!

russoz Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

russoz Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

felixfontein Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

russoz Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

russoz Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

LuminousMonkey commented Mar 22, 2026

Uh oh!

felixfontein commented Mar 23, 2026

Uh oh!

felixfontein commented Mar 23, 2026

Uh oh!

LuminousMonkey commented Mar 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants