From a2d13cc6f34a92420abac52c8176401bce14f534 Mon Sep 17 00:00:00 2001 From: Roger Luethi Date: Tue, 21 Jul 2026 13:43:31 +0200 Subject: [PATCH] test: guard run_ansible arg word-splitting run_ansible_in_environment joins a list of arguments with plain spaces (" ".join(arguments)) and runs the result through subprocess.Popen(..., shell=True), so /bin/sh word-splits each element into separate argv. Several callers depend on this: they pack multiple shell words into a single list element and rely on that split, e.g. commands/set.py and commands/noset.py pass ["-e status=True", "-l "], and commands/validate.py / commands/apply.py prepend "-e kolla_action=...". The run-.sh scripts forward args via "$@", so the outer-shell split is load-bearing. A well-meaning "fix" that shlex.quote()s each list element (to make a value with shell metacharacters safe, such as a tempest_include_regex of "(A|B)") would glue "-e status=True" into one token and silently break -e/-l parsing for those callers. The existing test_run_ansible_list_arguments_joined uses only single-word elements, on which shlex.quote is a no-op, so it does not catch this. Add a guard that pins the multi-token element behaviour: a space-containing element must stay word-split and must not be per-element quoted. It fails under a naive per-element quote and records the safe alternatives (split each element on whitespace before quoting, or normalise the callers to emit already-split tokens). Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi --- tests/unit/tasks/test_init.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/unit/tasks/test_init.py b/tests/unit/tasks/test_init.py index aa2612791..584d03ec5 100644 --- a/tests/unit/tasks/test_init.py +++ b/tests/unit/tasks/test_init.py @@ -363,6 +363,28 @@ def test_run_ansible_list_arguments_joined(runner_mocks): assert command.endswith("-e a=b -l all") +def test_run_ansible_list_multitoken_element_word_split_not_quoted(runner_mocks): + # REGRESSION GUARD -- do NOT "fix" the list join in + # run_ansible_in_environment by shlex.quote()-ing each element. Callers + # deliberately pack several shell words into ONE list element and rely on + # the unquoted " ".join(arguments) plus the /bin/sh -c in Popen(shell=True) + # to word-split them into separate argv, e.g.: + # commands/set.py, commands/noset.py: ["-e status=True", f"-l {host}"] + # commands/validate.py: "-e kolla_action=config_validate" + # commands/apply.py: [f"-e kolla_action={action}"] + args + # The run-.sh scripts forward args via "$@" (no re-split), so + # that outer-shell split is load-bearing. A per-element shlex.quote() would + # glue "-e status=True" into a single token and break -e/-l parsing. To make + # a value with shell metacharacters safe (e.g. a regex "(A|B)" in + # tempest_include_regex), split each element on whitespace FIRST and quote + # the resulting tokens, or normalize the callers to emit already-split + # tokens -- never a bare per-element quote. + run_ansible(arguments=["-e status=True", "-l host"]) + command = runner_mocks.popen.call_args.args[0] + assert command.endswith("-e status=True -l host") + assert "'-e status=True'" not in command + + def test_run_ansible_string_arguments_passed_through(runner_mocks): run_ansible(arguments="-e a=b") command = runner_mocks.popen.call_args.args[0]