fix(tasks): keep proxy and CA variables for ignore_env commands

osism/tasks/__init__.py

@@ -19,6 +19,37 @@
 # Regex pattern for extracting hosts from Ansible output
 HOST_PATTERN = re.compile(r"^(ok|changed|failed|skipping|unreachable):\s+\[([^\]]+)\]")
 
+# Environment variables a subprocess started with ignore_env=True inherits from
+# the worker container. Starting from an empty environment keeps the OS_*
+# variables of openstack.env away from the manager tools, where they would
+# override the --cloud selection taken from clouds.yaml, but it also strips the
+# proxy configuration, the CA bundles, and the basics a subprocess cannot work
+# without. Those are passed through by name instead.
+ISOLATED_ENV_NAMES = frozenset(
+    {
+        "PATH",
+        "HOME",
+        "LANG",
+        "TZ",
+        "SSL_CERT_FILE",
+        "SSL_CERT_DIR",
+        "REQUESTS_CA_BUNDLE",
+        "CURL_CA_BUNDLE",
+        "HTTP_PROXY",
+        "HTTPS_PROXY",
+        "NO_PROXY",
+        "ALL_PROXY",
+        # aria2c, which openstack-image-manager spawns to prefetch images, only
+        # reads the lowercase spellings.
+        "http_proxy",
+        "https_proxy",
+        "no_proxy",
+        "all_proxy",
+    }
+)
+
+ISOLATED_ENV_PREFIXES = ("LC_",)
+
 
 class AnsibleFailure(Exception):
     """Raised when an Ansible run in a worker container exits non-zero.
@@ -425,6 +456,23 @@ def run_ansible_in_environment(
             )
 
 
+def build_isolated_env(env):
+    """Build the environment for a subprocess started with ignore_env=True.
+
+    Returns the allowlisted variables inherited from os.environ, overlaid with
+    env. Variables outside the allowlist are not inherited; entries passed
+    explicitly through env are always kept, including the OS_* credentials of
+    openstack.env if a caller ever passes them.
+    """
+    command_env = {
+        key: value
+        for key, value in os.environ.items()
+        if key in ISOLATED_ENV_NAMES or key.startswith(ISOLATED_ENV_PREFIXES)
+    }
+    command_env.update(env)
+    return command_env
+
+
 def run_command(
     request_id,
     command,
@@ -438,7 +486,7 @@ def run_command(
     result = ""
 
     if ignore_env:
-        command_env = env
+        command_env = build_isolated_env(env)
     else:
         command_env = os.environ.copy()
         command_env.update(env)

tests/unit/tasks/test_init.py

@@ -727,10 +727,86 @@ def test_run_command_popen_argv_no_shell(command_mocks):
     assert "shell" not in kwargs
 
 
-def test_run_command_ignore_env_passes_env_verbatim(command_mocks):
-    env = {"FOO": "bar"}
+# The names run_command(ignore_env=True) inherits from the worker container.
+# Kept as a literal instead of being derived from tasks.ISOLATED_ENV_NAMES so
+# that a misspelt or deleted entry in the allowlist fails here instead of being
+# mirrored into the expectation.
+ISOLATED_ENV_EXPECTED_NAMES = [
+    "PATH",
+    "HOME",
+    "LANG",
+    "TZ",
+    "SSL_CERT_FILE",
+    "SSL_CERT_DIR",
+    "REQUESTS_CA_BUNDLE",
+    "CURL_CA_BUNDLE",
+    "HTTP_PROXY",
+    "HTTPS_PROXY",
+    "NO_PROXY",
+    "ALL_PROXY",
+    "http_proxy",
+    "https_proxy",
+    "no_proxy",
+    "all_proxy",
+]
+
+
+def test_isolated_env_names_match_expected_inventory():
+    assert set(tasks.ISOLATED_ENV_NAMES) == set(ISOLATED_ENV_EXPECTED_NAMES)
+    assert tasks.ISOLATED_ENV_PREFIXES == ("LC_",)
+
+
+@pytest.mark.parametrize("name", ISOLATED_ENV_EXPECTED_NAMES)
+def test_run_command_ignore_env_inherits_allowlisted_name(
+    command_mocks, monkeypatch, name
+):
+    monkeypatch.setenv(name, "from-worker")
+
+    tasks.run_command("req-1", "echo", {}, ignore_env=True)
+
+    assert command_mocks.popen.call_args.kwargs["env"][name] == "from-worker"
+
+
+@pytest.mark.parametrize("name", ["LC_ALL", "LC_CTYPE", "LC_MESSAGES"])
+def test_run_command_ignore_env_inherits_lc_prefixed_name(
+    command_mocks, monkeypatch, name
+):
+    monkeypatch.setenv(name, "C.UTF-8")
+
+    tasks.run_command("req-1", "echo", {}, ignore_env=True)
+
+    assert command_mocks.popen.call_args.kwargs["env"][name] == "C.UTF-8"
+
+
+def test_run_command_ignore_env_drops_openstack_variables(command_mocks, monkeypatch):
+    monkeypatch.setenv("OS_AUTH_URL", "https://keystone:5000")
+    monkeypatch.setenv("OS_PASSWORD", "secret")
+    monkeypatch.setenv("OS_CLOUD", "from-openstack-env")
+    monkeypatch.setenv("SOME_OTHER_VAR", "leaked")
+    # Contains the LC_ prefix without starting with it.
+    monkeypatch.setenv("XLC_ALL", "leaked")
+
+    tasks.run_command("req-1", "echo", {}, ignore_env=True)
+
+    passed = command_mocks.popen.call_args.kwargs["env"]
+    assert "OS_AUTH_URL" not in passed
+    assert "OS_PASSWORD" not in passed
+    assert "OS_CLOUD" not in passed
+    assert "SOME_OTHER_VAR" not in passed
+    assert "XLC_ALL" not in passed
+
+
+def test_run_command_ignore_env_lets_caller_env_win(command_mocks, monkeypatch):
+    monkeypatch.setenv("HTTPS_PROXY", "http://inherited:3128")
+    env = {"HTTPS_PROXY": "http://explicit:3128", "TOOL_SPECIFIC": "x"}
+
     tasks.run_command("req-1", "echo", env, ignore_env=True)
-    assert command_mocks.popen.call_args.kwargs["env"] is env
+
+    passed = command_mocks.popen.call_args.kwargs["env"]
+    assert passed["HTTPS_PROXY"] == "http://explicit:3128"
+    assert passed["TOOL_SPECIFIC"] == "x"
+    assert passed is not env
+    assert env == {"HTTPS_PROXY": "http://explicit:3128", "TOOL_SPECIFIC": "x"}
 
 
 def test_run_command_merges_env_with_os_environ(command_mocks, monkeypatch):