tasks: quote argument tokens for the shell

osism/tasks/__init__.py

@@ -4,6 +4,7 @@
 import json
 import os
 import re
+import shlex
 import shutil
 import subprocess
 import tempfile
@@ -161,7 +162,40 @@ def run_ansible_in_environment(
     extracted_hosts = set()  # Local set for host deduplication
 
     if type(arguments) == list:
-        joined_arguments = " ".join(arguments)
+        # The command below runs through `/bin/sh -c`, and callers rely on that
+        # shell to tokenize list elements: several deliberately pack more than
+        # one shell word into ONE element, e.g. commands/set.py and
+        # commands/noset.py pass ["-e status=True", f"-l {host}"], and
+        # commands/apply.py prepends f"-e kolla_action={action}". The
+        # run-<environment>.sh scripts forward args via "$@" without
+        # re-splitting, so that tokenization is load-bearing.
+        #
+        # Tokenize each element the way the shell would, then quote the
+        # resulting tokens. Quoting whole elements instead would glue
+        # "-e status=True" into one token and break -e/-l parsing (guarded by
+        # test_run_ansible_list_multitoken_element_word_split_not_quoted).
+        # str.split() is not sufficient either: an element may use quoting or a
+        # backslash to hold whitespace inside a single value, and splitting on
+        # raw whitespace cuts that value into malformed arguments. shlex.split
+        # honours both, matching what /bin/sh does today.
+        #
+        # The gain is that a value containing shell metacharacters becomes safe.
+        # Without it an `-e` value such as the regex alternation "(A|B)" reaches
+        # /bin/sh unquoted and the command dies with `Syntax error: "("
+        # unexpected` before ansible runs at all.
+        quoted_arguments = []
+        for argument in arguments:
+            try:
+                quoted_arguments.extend(
+                    shlex.quote(token) for token in shlex.split(argument)
+                )
+            except ValueError:
+                # Unbalanced quoting: shlex cannot tokenize it. Emit the element
+                # verbatim so /bin/sh reports the same error it reports today,
+                # rather than raising here and turning a shell-level error into
+                # a worker traceback.
+                quoted_arguments.append(argument)
+        joined_arguments = " ".join(quoted_arguments)
     else:
         joined_arguments = arguments
 

tests/unit/tasks/test_init.py

@@ -385,6 +385,56 @@ def test_run_ansible_list_multitoken_element_word_split_not_quoted(runner_mocks)
     assert "'-e status=True'" not in command
 
 
+def test_run_ansible_list_element_with_metacharacters_is_quoted(runner_mocks):
+    # A value containing shell metacharacters must reach /bin/sh quoted. The
+    # motivating case is a tempest regex alternation: unquoted, `/bin/sh -c`
+    # parses the parens and the command dies with
+    # `Syntax error: "(" unexpected` before ansible ever runs.
+    run_ansible(arguments=["-e", "tempest_include_regex=(A|B)"])
+    command = runner_mocks.popen.call_args.args[0]
+    assert "'tempest_include_regex=(A|B)'" in command
+
+
+def test_run_ansible_multitoken_element_tokens_quoted_individually(runner_mocks):
+    # The two requirements together: a multi-token element still word-splits
+    # (see the regression guard above), AND a metacharacter inside one of its
+    # tokens is still quoted -- so the split happens before quoting, not after.
+    run_ansible(arguments=["-e status=True", "-e regex=(A|B)"])
+    command = runner_mocks.popen.call_args.args[0]
+    assert command.endswith("-e status=True -e 'regex=(A|B)'")
+    assert "'-e status=True'" not in command
+
+
+def test_run_ansible_quoted_whitespace_stays_one_value(runner_mocks):
+    # A packed element may use shell quoting to hold whitespace inside ONE
+    # value. Tokenizing must respect that quoting: naive str.split() cuts
+    # inside the quotes and yields several malformed arguments.
+    run_ansible(arguments=["-e foo='hello world'"])
+    command = runner_mocks.popen.call_args.args[0]
+    assert command.endswith("-e 'foo=hello world'")
+
+
+def test_run_ansible_double_quoted_whitespace_stays_one_value(runner_mocks):
+    run_ansible(arguments=['-e foo="hello world"'])
+    command = runner_mocks.popen.call_args.args[0]
+    assert command.endswith("-e 'foo=hello world'")
+
+
+def test_run_ansible_backslash_escaped_whitespace_stays_one_value(runner_mocks):
+    run_ansible(arguments=["-e path=a\\ b"])
+    command = runner_mocks.popen.call_args.args[0]
+    assert command.endswith("-e 'path=a b'")
+
+
+def test_run_ansible_unbalanced_quote_passed_through_verbatim(runner_mocks):
+    # shlex cannot tokenize unbalanced quoting. Emit the element unchanged so
+    # /bin/sh reports the same error it does today, rather than raising here
+    # and turning a shell error into a worker traceback.
+    run_ansible(arguments=["-e foo='unbalanced"])
+    command = runner_mocks.popen.call_args.args[0]
+    assert command.endswith("-e foo='unbalanced")
+
+
 def test_run_ansible_string_arguments_passed_through(runner_mocks):
     run_ansible(arguments="-e a=b")
     command = runner_mocks.popen.call_args.args[0]