pfexec become plugin: fix broken defaults for illumos/SmartOS

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/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)."

plugins/become/pfexec.py

@@ -46,7 +46,7 @@
   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,11 @@
       - 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.
+    default: true
     type: bool
     ini:
       - section: pfexec_become_plugin
@@ -103,4 +106,5 @@ def build_become_command(self, cmd, shell):
 
         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)
+        return f"{exe} {flags} {become_cmd}"

tests/unit/plugins/become/test_pfexec.py

@@ -15,13 +15,13 @@
 
 
 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,65 @@ 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 +100,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