fix(security): strip dynamic Hermes secrets (AUXILIARY_*/GATEWAY_RELAY_*) from all subprocess spawn env + route codex through hermes_subprocess_env

agent/transports/codex_app_server.py

@@ -25,6 +25,8 @@
 from dataclasses import dataclass, field
 from typing import Any, Optional
 
+from tools.environments.local import hermes_subprocess_env
+
 # Default minimum codex version we test against. The PR sets this from the
 # `codex --version` parsed at install time; bumping is a one-line change here.
 MIN_CODEX_VERSION = (0, 125, 0)
@@ -74,7 +76,18 @@ def __init__(
         env: Optional[dict[str, str]] = None,
     ) -> None:
         self._codex_bin = codex_bin
-        spawn_env = os.environ.copy()
+        # codex app-server is a model-driving CLI executor: it runs a
+        # model-chosen agentic loop that executes shell commands, so it
+        # legitimately needs LLM provider credentials (inherit_credentials=True)
+        # to authenticate against the model endpoint. But the previous
+        # `os.environ.copy()` also handed it every Tier-1 Hermes secret — gateway
+        # bot tokens, GitHub auth, Modal/Daytona infra tokens, the dashboard
+        # session token, AUXILIARY_* side-LLM keys, GATEWAY_RELAY_* auth — none
+        # of which a coding subprocess has any use for. Route through the
+        # centralized helper so Tier-1 + dynamic-internal secrets are always
+        # stripped while provider creds still flow, matching copilot_acp_client
+        # (#29157 sibling spawn-site gap).
+        spawn_env = hermes_subprocess_env(inherit_credentials=True)
         if env:
             spawn_env.update(env)
         if codex_home:

tests/agent/transports/test_codex_app_server_runtime.py

@@ -295,3 +295,86 @@ def kill(self):
         )
         assert "sandbox_workspace_write.network_access=false" in cmd
         assert all("danger" not in part for part in cmd)
+
+
+class TestSpawnEnvSecretStripping:
+    """codex app-server routes its spawn env through hermes_subprocess_env(
+    inherit_credentials=True) instead of a raw os.environ.copy().
+
+    codex is a model-driving CLI executor: it legitimately needs LLM provider
+    credentials to authenticate, but it must NOT inherit Tier-1 Hermes secrets
+    (gateway bot tokens, GitHub/infra auth, dashboard session token) or the
+    dynamic-internal secrets (AUXILIARY_*_API_KEY / _BASE_URL side-LLM keys,
+    GATEWAY_RELAY_* relay-auth) — a coding subprocess has no use for those and
+    a model-controlled action could exfiltrate them. This closes the #29157
+    sibling spawn-site gap (copilot_acp_client already routes through the
+    helper; codex app-server predated it).
+    """
+
+    @staticmethod
+    def _capture_spawn_env(monkeypatch):
+        import subprocess
+        from agent.transports import codex_app_server as cas
+
+        captured = {}
+
+        class FakePopen:
+            def __init__(self, cmd, *args, **kwargs):
+                captured["env"] = kwargs.get("env", {}).copy()
+                self.stdin = None
+                self.stdout = None
+                self.stderr = None
+                self.pid = 1
+                self.returncode = None
+
+            def poll(self):
+                return None
+
+            def terminate(self):
+                pass
+
+            def wait(self, timeout=None):
+                return 0
+
+            def kill(self):
+                pass
+
+        monkeypatch.setattr(subprocess, "Popen", FakePopen)
+        client = cas.CodexAppServerClient(codex_bin="codex")
+        client._closed = True
+        return captured["env"]
+
+    def test_tier1_and_internal_secrets_stripped_from_spawn_env(self, monkeypatch):
+        for var, val in {
+            "GH_TOKEN": "ghp-secret",
+            "TELEGRAM_BOT_TOKEN": "bot-secret",
+            "MODAL_TOKEN_SECRET": "modal-secret",
+            "HERMES_DASHBOARD_SESSION_TOKEN": "dash-secret",
+            "AUXILIARY_VISION_API_KEY": "aux-secret",
+            "GATEWAY_RELAY_SECRET": "relay-secret",
+            "GATEWAY_RELAY_ID": "relay-id",
+            "GATEWAY_RELAY_DELIVERY_KEY": "relay-delivery",
+        }.items():
+            monkeypatch.setenv(var, val)
+
+        env = self._capture_spawn_env(monkeypatch)
+        for var in (
+            "GH_TOKEN", "TELEGRAM_BOT_TOKEN", "MODAL_TOKEN_SECRET",
+            "HERMES_DASHBOARD_SESSION_TOKEN", "AUXILIARY_VISION_API_KEY",
+            "GATEWAY_RELAY_SECRET", "GATEWAY_RELAY_ID", "GATEWAY_RELAY_DELIVERY_KEY",
+        ):
+            assert var not in env, f"{var} leaked into codex app-server spawn env"
+
+    def test_provider_credentials_still_reach_codex(self, monkeypatch):
+        """codex authenticates against the model endpoint — provider keys must
+        still flow through (inherit_credentials=True)."""
+        monkeypatch.setenv("OPENAI_API_KEY", "sk-codex-needs-this")
+        env = self._capture_spawn_env(monkeypatch)
+        assert env.get("OPENAI_API_KEY") == "sk-codex-needs-this"
+
+    def test_home_still_preserved_through_helper(self, monkeypatch):
+        """Regression guard: routing through hermes_subprocess_env must not
+        rewrite HOME (codex's shell tool spawns gh/git/aws that need it)."""
+        monkeypatch.setenv("HOME", "/users/alice")
+        env = self._capture_spawn_env(monkeypatch)
+        assert env.get("HOME") == "/users/alice"

tests/tools/test_env_passthrough.py

@@ -195,6 +195,40 @@ def test_passthrough_cannot_override_provider_blocklist(self):
         assert blocked_var not in result
         assert "PATH" in result
 
+    def test_passthrough_cannot_override_internal_dynamic_secret(self):
+        """A skill must NOT be able to register dynamically-named Hermes
+        secrets (AUXILIARY_*_API_KEY / _BASE_URL, GATEWAY_RELAY_* auth) as
+        passthrough — they aren't in the static blocklist, so this is the
+        defense-in-depth layer that keeps env_passthrough consistent with the
+        unconditional strip in the sanitizers."""
+        from tools.environments.local import _sanitize_subprocess_env
+
+        for var in (
+            "AUXILIARY_VISION_API_KEY",
+            "AUXILIARY_VISION_BASE_URL",
+            "GATEWAY_RELAY_SECRET",
+            "GATEWAY_RELAY_DELIVERY_KEY",
+        ):
+            register_env_passthrough([var])
+            assert not is_env_passthrough(var), (
+                f"{var} should be refused passthrough registration"
+            )
+            result = _sanitize_subprocess_env({var: "secret", "PATH": "/usr/bin"})
+            assert var not in result
+            assert "PATH" in result
+
+    def test_passthrough_allows_auxiliary_non_secret_routing(self):
+        """AUXILIARY_*_PROVIDER / _MODEL and GATEWAY_RELAY routing hints are not
+        secrets, so a skill may still register them (they're not protected)."""
+        register_env_passthrough([
+            "AUXILIARY_VISION_PROVIDER",
+            "AUXILIARY_VISION_MODEL",
+            "GATEWAY_RELAY_URL",
+        ])
+        assert is_env_passthrough("AUXILIARY_VISION_PROVIDER")
+        assert is_env_passthrough("AUXILIARY_VISION_MODEL")
+        assert is_env_passthrough("GATEWAY_RELAY_URL")
+
     def test_make_run_env_blocklist_override_rejected(self):
         """_make_run_env must NOT expose a blocklisted var to subprocess env
         even after a skill attempts to register it via passthrough."""

tests/tools/test_hermes_subprocess_env.py

@@ -149,3 +149,63 @@ def test_browser_keys_recoverable_after_strip(self):
         # Provider + gateway secrets must NOT come back.
         assert "ANTHROPIC_API_KEY" not in env
         assert "TELEGRAM_BOT_TOKEN" not in env
+
+
+_INTERNAL_DYNAMIC_SAMPLE = {
+    "AUXILIARY_VISION_API_KEY": "sk-vision",
+    "AUXILIARY_VISION_BASE_URL": "http://internal:1234/v1",
+    "AUXILIARY_WEB_EXTRACT_API_KEY": "sk-webx",
+    "GATEWAY_RELAY_SECRET": "relay-secret",
+    "GATEWAY_RELAY_DELIVERY_KEY": "relay-delivery",
+}
+
+
+class TestInternalDynamicSecrets:
+    """AUXILIARY_*_API_KEY / _BASE_URL and GATEWAY_RELAY_* auth are stripped on
+    BOTH paths — including inherit_credentials=True — since a model-driving CLI
+    (codex/copilot) never needs them even when it needs provider keys."""
+
+    def test_stripped_by_default(self):
+        result = _build(_INTERNAL_DYNAMIC_SAMPLE)
+        for var in _INTERNAL_DYNAMIC_SAMPLE:
+            assert var not in result, f"{var} leaked with inherit_credentials=False"
+
+    def test_stripped_even_when_inheriting(self):
+        result = _build(
+            {**_PROVIDER_SAMPLE, **_INTERNAL_DYNAMIC_SAMPLE},
+            inherit_credentials=True,
+        )
+        for var in _INTERNAL_DYNAMIC_SAMPLE:
+            assert var not in result, (
+                f"{var} must be stripped even with inherit_credentials=True"
+            )
+        # ...while genuine provider keys survive so codex can authenticate.
+        for var in _PROVIDER_SAMPLE:
+            assert var in result
+
+    def test_auxiliary_non_secrets_preserved(self):
+        """AUXILIARY_*_PROVIDER / _MODEL routing config survives (not secrets)."""
+        result = _build(
+            {"AUXILIARY_VISION_PROVIDER": "openai", "AUXILIARY_VISION_MODEL": "gpt-4o"},
+        )
+        assert result.get("AUXILIARY_VISION_PROVIDER") == "openai"
+        assert result.get("AUXILIARY_VISION_MODEL") == "gpt-4o"
+
+    def test_gateway_relay_id_stripped_even_when_inheriting(self):
+        """GATEWAY_RELAY_ID has no secret suffix (predicate skips it) but is
+        gateway-identifying auth material provisioned alongside the relay
+        secret. It's in _ALWAYS_STRIP_KEYS so it's stripped on the inherit path
+        too — closes the codex/copilot leak the predicate alone would miss."""
+        result = _build(
+            {**_PROVIDER_SAMPLE, "GATEWAY_RELAY_ID": "relay-id"},
+            inherit_credentials=True,
+        )
+        assert "GATEWAY_RELAY_ID" not in result
+        # provider keys still flow (codex auth)
+        for var in _PROVIDER_SAMPLE:
+            assert var in result
+
+    def test_relay_triplet_in_always_strip(self):
+        assert {
+            "GATEWAY_RELAY_ID", "GATEWAY_RELAY_SECRET", "GATEWAY_RELAY_DELIVERY_KEY",
+        } <= _ALWAYS_STRIP_KEYS

tests/tools/test_local_env_blocklist.py

@@ -611,3 +611,116 @@ def test_make_run_env_injects_hermes_bin_dir(self, monkeypatch):
         entries = result["PATH"].split(os.pathsep)
         assert entries[0] == "/opt/hermes/bin"
         assert "/usr/bin" in entries
+
+
+class TestHermesInternalDynamicSecrets:
+    """Dynamically-named Hermes secrets injected at gateway/CLI startup must
+    not leak into terminal subprocesses.
+
+    The static ``_HERMES_PROVIDER_ENV_BLOCKLIST`` is name-based and derived
+    from provider/tool registries, so it cannot enumerate:
+
+    - ``AUXILIARY_<TASK>_API_KEY`` / ``AUXILIARY_<TASK>_BASE_URL`` — per-task
+      side-LLM credentials bridged from ``config.yaml[auxiliary]`` by
+      ``gateway/run.py`` and ``cli.py``.
+    - ``GATEWAY_RELAY_*_SECRET`` / ``_KEY`` / ``_TOKEN`` — relay-auth material
+      provisioned by ``gateway/relay``.
+
+    ``_is_hermes_internal_secret`` is the single source of truth; every spawn
+    path (``_sanitize_subprocess_env``, ``_make_run_env``,
+    ``hermes_subprocess_env``, Docker forward filter, ``env_passthrough``)
+    consults it. These tests exercise the terminal execute path + predicate.
+    """
+
+    def test_predicate_matches_auxiliary_api_key(self):
+        from tools.environments.local import _is_hermes_internal_secret
+        assert _is_hermes_internal_secret("AUXILIARY_VISION_API_KEY")
+        assert _is_hermes_internal_secret("AUXILIARY_WEB_EXTRACT_API_KEY")
+        assert _is_hermes_internal_secret("AUXILIARY_APPROVAL_API_KEY")
+        # plugin-registered task names are covered by the pattern
+        assert _is_hermes_internal_secret("AUXILIARY_MY_PLUGIN_TASK_API_KEY")
+
+    def test_predicate_matches_auxiliary_base_url(self):
+        from tools.environments.local import _is_hermes_internal_secret
+        assert _is_hermes_internal_secret("AUXILIARY_VISION_BASE_URL")
+        assert _is_hermes_internal_secret("AUXILIARY_COMPRESSION_BASE_URL")
+
+    def test_predicate_matches_gateway_relay_auth(self):
+        from tools.environments.local import _is_hermes_internal_secret
+        assert _is_hermes_internal_secret("GATEWAY_RELAY_SECRET")
+        assert _is_hermes_internal_secret("GATEWAY_RELAY_DELIVERY_KEY")
+        assert _is_hermes_internal_secret("GATEWAY_RELAY_SESSION_TOKEN")
+
+    def test_predicate_allows_auxiliary_non_secrets(self):
+        """AUXILIARY_*_PROVIDER / _MODEL and GATEWAY_RELAY_* routing hints are
+        NOT secrets and must remain visible so tooling that reads them works."""
+        from tools.environments.local import _is_hermes_internal_secret
+        assert not _is_hermes_internal_secret("AUXILIARY_VISION_PROVIDER")
+        assert not _is_hermes_internal_secret("AUXILIARY_VISION_MODEL")
+        assert not _is_hermes_internal_secret("GATEWAY_RELAY_URL")
+        assert not _is_hermes_internal_secret("GATEWAY_RELAY_PLATFORMS")
+        assert not _is_hermes_internal_secret("GATEWAY_RELAY_ID")  # not a secret suffix
+        # unrelated vars pass through
+        assert not _is_hermes_internal_secret("PATH")
+        assert not _is_hermes_internal_secret("MY_APP_KEY")
+
+    def test_auxiliary_secrets_stripped_from_subprocess(self):
+        """AUXILIARY_*_API_KEY / _BASE_URL injected into os.environ must not
+        reach the terminal subprocess, while _PROVIDER / _MODEL survive."""
+        result_env = _run_with_env(extra_os_env={
+            "AUXILIARY_VISION_API_KEY": "sk-vision-secret",
+            "AUXILIARY_VISION_BASE_URL": "http://internal:1234/v1",
+            "AUXILIARY_WEB_EXTRACT_API_KEY": "sk-webx-secret",
+            "AUXILIARY_VISION_PROVIDER": "openai",
+            "AUXILIARY_VISION_MODEL": "gpt-4o",
+        })
+        assert "AUXILIARY_VISION_API_KEY" not in result_env
+        assert "AUXILIARY_VISION_BASE_URL" not in result_env
+        assert "AUXILIARY_WEB_EXTRACT_API_KEY" not in result_env
+        # Non-secret routing config is preserved.
+        assert result_env.get("AUXILIARY_VISION_PROVIDER") == "openai"
+        assert result_env.get("AUXILIARY_VISION_MODEL") == "gpt-4o"
+
+    def test_gateway_relay_secret_stripped_from_subprocess(self):
+        result_env = _run_with_env(extra_os_env={
+            "GATEWAY_RELAY_SECRET": "relay-signing-secret",
+            "GATEWAY_RELAY_DELIVERY_KEY": "relay-delivery-key",
+            "GATEWAY_RELAY_URL": "https://relay.example.com",
+        })
+        assert "GATEWAY_RELAY_SECRET" not in result_env
+        assert "GATEWAY_RELAY_DELIVERY_KEY" not in result_env
+        # Non-secret routing hint stays visible.
+        assert result_env.get("GATEWAY_RELAY_URL") == "https://relay.example.com"
+
+    def test_auxiliary_secret_stripped_even_when_passthrough_registered(self):
+        """A skill registering AUXILIARY_*_API_KEY as env_passthrough must NOT
+        be able to tunnel it into a subprocess — the strip is unconditional."""
+        with patch(
+            "tools.env_passthrough.is_env_passthrough",
+            side_effect=lambda name: name == "AUXILIARY_VISION_API_KEY",
+        ):
+            result_env = _run_with_env(extra_os_env={
+                "AUXILIARY_VISION_API_KEY": "sk-vision-secret",
+            })
+        assert "AUXILIARY_VISION_API_KEY" not in result_env
+
+    def test_make_run_env_strips_internal_secrets(self):
+        """The foreground _make_run_env path strips the same dynamic secrets."""
+        from tools.environments.local import _make_run_env
+        with patch.dict(os.environ, {
+            "PATH": "/usr/bin:/bin",
+            "AUXILIARY_VISION_API_KEY": "sk-secret",
+            "GATEWAY_RELAY_SECRET": "relay-secret",
+            "AUXILIARY_VISION_PROVIDER": "openai",
+        }, clear=True):
+            run_env = _make_run_env({})
+        assert "AUXILIARY_VISION_API_KEY" not in run_env
+        assert "GATEWAY_RELAY_SECRET" not in run_env
+        assert run_env.get("AUXILIARY_VISION_PROVIDER") == "openai"
+
+    def test_gateway_relay_static_names_in_blocklist(self):
+        """The static relay names are also added to the name-based blocklist so
+        the exact-match path catches them independently of the predicate."""
+        assert "GATEWAY_RELAY_SECRET" in _HERMES_PROVIDER_ENV_BLOCKLIST
+        assert "GATEWAY_RELAY_DELIVERY_KEY" in _HERMES_PROVIDER_ENV_BLOCKLIST
+        assert "GATEWAY_RELAY_ID" in _HERMES_PROVIDER_ENV_BLOCKLIST

tools/env_passthrough.py

@@ -66,7 +66,10 @@ def _is_hermes_provider_credential(name: str) -> bool:
     let a skill tunnel a Hermes credential into the execute_code child.
     """
     try:
-        from tools.environments.local import _HERMES_PROVIDER_ENV_BLOCKLIST
+        from tools.environments.local import (
+            _HERMES_PROVIDER_ENV_BLOCKLIST,
+            _is_hermes_internal_secret,
+        )
     except Exception as e:
         logger.warning(
             "env passthrough: provider credential blocklist import failed; "
@@ -75,6 +78,13 @@ def _is_hermes_provider_credential(name: str) -> bool:
             e,
         )
         return True
+    # Dynamically-generated Hermes-internal secrets (AUXILIARY_*_API_KEY /
+    # _BASE_URL side-LLM credentials, GATEWAY_RELAY_* relay-auth) are provider
+    # credentials the static blocklist can't enumerate — they're injected per
+    # task/relay at gateway startup. A skill must not be able to register them
+    # as passthrough and tunnel them into an execute_code / terminal child.
+    if _is_hermes_internal_secret(name):
+        return True
     return name in _HERMES_PROVIDER_ENV_BLOCKLIST
 
 

tools/environments/docker.py

@@ -17,7 +17,10 @@
 from typing import Optional
 
 from tools.environments.base import BaseEnvironment, _popen_bash
-from tools.environments.local import _HERMES_PROVIDER_ENV_BLOCKLIST
+from tools.environments.local import (
+    _HERMES_PROVIDER_ENV_BLOCKLIST,
+    _is_hermes_internal_secret,
+)
 
 logger = logging.getLogger(__name__)
 
@@ -992,8 +995,13 @@ def _build_init_env_args(self) -> list[str]:
             pass
         # Explicit docker_forward_env entries are an intentional opt-in and must
         # win over the generic Hermes secret blocklist. Only implicit passthrough
-        # keys are filtered.
-        forward_keys = explicit_forward_keys | (passthrough_keys - _HERMES_PROVIDER_ENV_BLOCKLIST)
+        # keys are filtered. Also strip Hermes-internal dynamic secrets
+        # (AUXILIARY_*_API_KEY / _BASE_URL, GATEWAY_RELAY_* auth) that the
+        # name-based blocklist doesn't cover — see _is_hermes_internal_secret.
+        _implicit_forward = {
+            k for k in passthrough_keys if not _is_hermes_internal_secret(k)
+        }
+        forward_keys = explicit_forward_keys | (_implicit_forward - _HERMES_PROVIDER_ENV_BLOCKLIST)
         hermes_env = _load_hermes_env_vars() if forward_keys else {}
         for key in sorted(forward_keys):
             value = os.getenv(key)