Repository: ansible/ansible
Pull request: Fix tb for when env var contains %
##### SUMMARY
By constructing the string utilizing f-string only.

Fixes #83498
<!--- Describe the change below, including rationale and design decisions -->

<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE

<!--- Pick one below and delete the rest -->

- Bugfix Pull Request


Problem statement:
[2.17][command] Failure in environment construction due to special characters in variable
### Summary

On Ansible-Core 2.17 we came across a failing task that uses the `command` module. The task has an environment set that is constructed from a variable. One of the env variables to be set contained a `%D` which turned out to be the problem.
The task has worked fine on Ansible-Core 2.16.

Further experimenting revealed that the `shell` module suffers from the same problem.

### Issue Type

Bug Report

### Component Name

command

### Ansible Version

```console
$ ansible --version
ansible [core 2.17.1]
  config file = /home/charly/eclipseprojects/spp_playground/ansible.cfg
  configured module search path = ['/home/charly/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/ansible/lib/python3.11/site-packages/ansible
  ansible collection location = /home/charly/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.11.9 (main, Apr 10 2024, 13:16:36) [GCC 13.2.0] (/opt/ansible/bin/python3)
  jinja version = 3.1.4
  libyaml = True
```


### Configuration

```console
$ ansible-config dump --only-changed -t all
CALLBACKS_ENABLED(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = ['profile_tasks']
CONFIG_FILE() = /home/charly/eclipseprojects/spp_playground/ansible.cfg
DEFAULT_GATHERING(env: ANSIBLE_GATHERING) = explicit
DEFAULT_LOG_PATH(env: ANSIBLE_LOG_PATH) = /tmp/ansible.log
DEFAULT_ROLES_PATH(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = ['/home/charly/eclipseprojects/spp_playground/external_roles']
DEFAULT_STDOUT_CALLBACK(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = yaml
EDITOR(env: EDITOR) = vi
GALAXY_IGNORE_CERTS(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = False
HOST_KEY_CHECKING(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = False
INTERPRETER_PYTHON(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = /usr/bin/python3
RETRY_FILES_ENABLED(env: ANSIBLE_RETRY_FILES_ENABLED) = False

CONNECTION:
==========

paramiko_ssh:
____________
host_key_checking(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = False

ssh:
___
control_path(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = %(directory)s/%%h-%%r
host_key_checking(/home/charly/eclipseprojects/spp_playground/ansible.cfg) = False
```


### OS / Environment

Controller:
```
$ lsb_release -a
No LSB modules are available.
Distributor ID:	Debian
Description:	Debian GNU/Linux trixie/sid
Release:	n/a
Codename:	trixie
```
Managed host:
```
$ lsb_release -a
No LSB modules are available.
Distributor ID:	Debian
Description:	Debian GNU/Linux 12 (bookworm)
Release:	12
Codename:	bookworm
```

### Steps to Reproduce

<!--- Paste example playbooks or commands between quotes below -->
```yaml (paste below)
---

- hosts: all
  become: true
  serial: 1

  vars:
    env_vars:
      FOO: bar%D

  tasks:
    - name: run command with env
      command: /usr/bin/uptime -p
      environment: '{{ env_vars }}'
```


### Expected Results

Environment should be constructed and command being run. Expected result as produced by Ansible-Core 2.16.
```shell
$ ansible-playbook -i environments/dev/inventory --user pammer --ask-become-pass env_bug.yml -vv
ansible-playbook [core 2.16.8]
[...]
PLAYBOOK: env_bug.yml *********************
1 plays in env_bug.yml

PLAY [all] **********************

TASK [run command with env] **********************
task path: /home/charly/eclipseprojects/spp_playground/env_bug.yml:12
Donnerstag 27 Juni 2024  14:16:00 +0200 (0:00:00.010)       0:00:00.010 ******* 
changed: [charly-dev01.server.lan] => changed=true 
  cmd:
  - /usr/bin/uptime
  - -p
  delta: '0:00:00.074528'
  end: '2024-06-27 14:16:04.154977'
  msg: ''
  rc: 0
  start: '2024-06-27 14:16:04.080449'
  stderr: ''
  stderr_lines: <omitted>
  stdout: up 1 week, 1 day, 6 hours, 6 minutes
  stdout_lines: <omitted>

PLAY RECAP **********************
charly-dev01.server.lan    : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
```

### Actual Results

```console
PLAYBOOK: env_bug.yml *********************
1 plays in env_bug.yml

PLAY [all] **********************

TASK [run command with env] **********************
task path: /home/charly/eclipseprojects/spp_playground/env_bug.yml:12
Donnerstag 27 Juni 2024  14:13:29 +0200 (0:00:00.011)       0:00:00.011 ******* 
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ValueError: unsupported format character 'D' (0x44) at index 8
fatal: [charly-dev01.server.lan]: FAILED! => 
  msg: 'Unexpected failure during module execution: unsupported format character ''D'' (0x44) at index 8'
  stdout: ''

PLAY RECAP **********************
charly-dev01.server.lan    : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored

Run using -vvv yielded some more insights:
The full traceback is:
Traceback (most recent call last):
  File "/opt/ansible/lib/python3.11/site-packages/ansible/executor/task_executor.py", line 164, in run
    res = self._execute()
          ^^^^^^^^^^^^^^^
  File "/opt/ansible/lib/python3.11/site-packages/ansible/executor/task_executor.py", line 636, in _execute
    result = self._handler.run(task_vars=vars_copy)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/ansible/lib/python3.11/site-packages/ansible/plugins/action/command.py", line 20, in run
    results = merge_hash(results, self._execute_module(module_name='ansible.legacy.command', task_vars=task_vars, wrap_async=wrap_async))
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/ansible/lib/python3.11/site-packages/ansible/plugins/action/__init__.py", line 1146, in _execute_module
    cmd = self._connection._shell.build_module_command(environment_string, shebang, cmd, arg_path=args_file_path).strip()
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/ansible/lib/python3.11/site-packages/ansible/plugins/shell/__init__.py", line 214, in build_module_command
    return f'{env_string}%s' % shlex.join(cps for cp in cmd_parts if cp and (cps := cp.strip()))
           ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ValueError: unsupported format character 'D' (0x44) at index 8
fatal: [charly-dev01.server.lan]: FAILED! => 
  msg: 'Unexpected failure during module execution: unsupported format character ''D'' (0x44) at index 8'
  stdout: ''
```


### Code of Conduct

- [X] I agree to follow the Ansible Code of Conduct

Hints from issue discussion:
Files identified in the description:

* [`lib/ansible/modules/command.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/command.py)
* [`lib/ansible/plugins/action/command.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/action/command.py)

If these files are incorrect, please update the `component name` section of the description or use the [component](https://github.com/ansible/ansibotmini#commands) bot command.

<!--- boilerplate: components_banner --->

Commit message: Fix tb for when env var contains %

By constructing the string utilizing f-string only.

Fixes #83498

Patch to review:
diff --git a/changelogs/fragments/83498-command-tb-env.yml b/changelogs/fragments/83498-command-tb-env.yml
new file mode 100644
index 00000000000000..b28ad18114adbd
--- /dev/null
+++ b/changelogs/fragments/83498-command-tb-env.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - Fix a traceback when an environment variable contains certain special characters (https://github.com/ansible/ansible/issues/83498)
diff --git a/lib/ansible/plugins/shell/__init__.py b/lib/ansible/plugins/shell/__init__.py
index dd9f6553ecac12..3873ca3d0be3ec 100644
--- a/lib/ansible/plugins/shell/__init__.py
+++ b/lib/ansible/plugins/shell/__init__.py
@@ -211,7 +211,7 @@ def build_module_command(self, env_string, shebang, cmd, arg_path=None):
             arg_path,
         ]
 
-        return f'{env_string}%s' % self.join(cps for cp in cmd_parts if cp and (cps := cp.strip()))
+        return f'{env_string}{self.join(cps for cp in cmd_parts if cp and (cps := cp.strip()))}'
 
     def append_command(self, cmd, cmd_to_append):
         """Append an additional command if supported by the shell"""
diff --git a/test/integration/targets/shell/tasks/command-building.yml b/test/integration/targets/shell/tasks/command-building.yml
index bd452618b52a30..d22f67467c99c4 100644
--- a/test/integration/targets/shell/tasks/command-building.yml
+++ b/test/integration/targets/shell/tasks/command-building.yml
@@ -28,6 +28,7 @@
         ANSIBLE_REMOTE_TMP: '{{ atd }}'
         ANSIBLE_NOCOLOR: "1"
         ANSIBLE_FORCE_COLOR: "0"
+        TEST: "foo%D"
       register: command_building
       delegate_to: localhost
 


Code review comments:
diff_hunk: @@ -211,7 +211,7 @@ def build_module_command(self, env_string, shebang, cmd, arg_path=None):
             arg_path,
         ]
 
-        return f'{env_string}%s' % self.join(cps for cp in cmd_parts if cp and (cps := cp.strip()))
+        return f'{env_string}{self.join(cps for cp in cmd_parts if cp and (cps := cp.strip()))}'
path: lib/ansible/plugins/shell/__init__.py
text: @user1:
This highly expression is fairly difficult to consume. Mind splitting it?
```suggestion
        cleaned_up_cmd = self.join(
            stripped_cmd_part for raw_cmd_part in cmd_parts
            if raw_cmd_part and (stripped_cmd_part := raw_cmd_part.strip())
        )
        return ''.join((env_string, cleaned_up_cmd))
```