From a658f3b28b5b66492c13aee6835b07d4a2717ba4 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:16:29 +0530 Subject: [PATCH] fix(security): strip dynamic Hermes secrets from all subprocess spawn env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subprocesses spawned by the terminal tool, execute_code, Docker backend, and the codex app-server could inherit Hermes-internal secrets that the name-based `_HERMES_PROVIDER_ENV_BLOCKLIST` can't enumerate, because they're injected into `os.environ` at runtime under dynamic names: - `AUXILIARY__API_KEY` / `AUXILIARY__BASE_URL` — per-task side-LLM credentials bridged from `config.yaml[auxiliary]` by gateway/run.py and cli.py (vision, web_extract, approval, compression, plugin-registered tasks). Often separate, higher-spend keys plus base URLs pointing at private endpoints. - `GATEWAY_RELAY_*_SECRET` / `_KEY` / `_TOKEN` — relay-auth material provisioned by gateway/relay. Additionally, agent/transports/codex_app_server.py built its spawn env from a raw `os.environ.copy()`, bypassing the centralized `hermes_subprocess_env()` helper entirely — handing every codex subprocess the full Tier-1 secret set (GH_TOKEN, gateway bot tokens, Modal/Daytona infra tokens, dashboard session token) unfiltered. This is the #29157 sibling spawn-site gap; copilot_acp_client already routes through the helper. Fix — single chokepoint: - Add `_is_hermes_internal_secret(key)` in tools/environments/local.py as the single source of truth for the dynamic secret patterns. Matches AUXILIARY_*_API_KEY / _BASE_URL and GATEWAY_RELAY_*_SECRET/_KEY/_TOKEN; leaves non-secret AUXILIARY_*_PROVIDER/_MODEL and GATEWAY_RELAY routing hints visible. - Wire the predicate into every spawn path unconditionally (ignores skill env_passthrough opt-in AND inherit_credentials — a model-driving CLI never needs these): `_sanitize_subprocess_env` (both loops), `_make_run_env` (foreground), `hermes_subprocess_env` (Tier-1), and the Docker forward filter. - Add the static GATEWAY_RELAY_* names to `_HERMES_PROVIDER_ENV_BLOCKLIST` so the exact-match path catches them independently of the predicate. - Add the GATEWAY_RELAY_ID/_SECRET/_DELIVERY_KEY triplet to `_ALWAYS_STRIP_KEYS` (Tier-1) so it is stripped unconditionally on EVERY spawn surface — including the codex/copilot `inherit_credentials=True` path that skips the Tier-2 blocklist. `_SECRET`/`_DELIVERY_KEY` are already predicate-matched; `_ID` has no secret suffix, so enumerating it here is what closes its leak on the inherit path (self-review W1). - Defense in depth: env_passthrough.py `_is_hermes_provider_credential()` now consults the same predicate, so a skill can't register these names as passthrough and tunnel them into an execute_code / terminal child. - Route codex_app_server through `hermes_subprocess_env(inherit_credentials=True)` — strips Tier-1 + dynamic-internal secrets while provider creds (which codex needs to authenticate) still flow. Consolidates PRs #53715 (necoweb3 — the _is_hermes_internal_secret backbone + Docker filter), #53503 (srojk34 — env_passthrough guard), and #55709 (srojk34 — codex routing). Retires #52348 (claudlos): its copilot half is already on main, and its codex half used the full-strip `_sanitize_subprocess_env` which would break codex provider auth — the correct tier is `inherit_credentials=True`. Tests: TestHermesInternalDynamicSecrets (terminal + predicate + passthrough override), TestInternalDynamicSecrets (hermes_subprocess_env both tiers), TestSpawnEnvSecretStripping (codex spawn env), plus env_passthrough defense-in-depth cases. Co-authored-by: necoweb3 Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com> Co-authored-by: claudlos --- agent/transports/codex_app_server.py | 15 ++- .../test_codex_app_server_runtime.py | 83 +++++++++++++ tests/tools/test_env_passthrough.py | 34 ++++++ tests/tools/test_hermes_subprocess_env.py | 60 ++++++++++ tests/tools/test_local_env_blocklist.py | 113 ++++++++++++++++++ tools/env_passthrough.py | 12 +- tools/environments/docker.py | 14 ++- tools/environments/local.py | 76 +++++++++++- 8 files changed, 401 insertions(+), 6 deletions(-) diff --git a/agent/transports/codex_app_server.py b/agent/transports/codex_app_server.py index dff16e971da..273e44667d6 100644 --- a/agent/transports/codex_app_server.py +++ b/agent/transports/codex_app_server.py @@ -25,6 +25,8 @@ import time 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 @@ class CodexAppServerClient: 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: diff --git a/tests/agent/transports/test_codex_app_server_runtime.py b/tests/agent/transports/test_codex_app_server_runtime.py index e965d921b76..5c1c9bda60d 100644 --- a/tests/agent/transports/test_codex_app_server_runtime.py +++ b/tests/agent/transports/test_codex_app_server_runtime.py @@ -295,3 +295,86 @@ class TestSpawnEnvIsolation: ) 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" diff --git a/tests/tools/test_env_passthrough.py b/tests/tools/test_env_passthrough.py index a9d706636cb..2bff4c19862 100644 --- a/tests/tools/test_env_passthrough.py +++ b/tests/tools/test_env_passthrough.py @@ -195,6 +195,40 @@ class TestTerminalIntegration: 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.""" diff --git a/tests/tools/test_hermes_subprocess_env.py b/tests/tools/test_hermes_subprocess_env.py index 92f629e3999..303fd432112 100644 --- a/tests/tools/test_hermes_subprocess_env.py +++ b/tests/tools/test_hermes_subprocess_env.py @@ -149,3 +149,63 @@ class TestBrowserPassthroughPattern: # 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 diff --git a/tests/tools/test_local_env_blocklist.py b/tests/tools/test_local_env_blocklist.py index 005dd2a123f..914fdfa2cca 100644 --- a/tests/tools/test_local_env_blocklist.py +++ b/tests/tools/test_local_env_blocklist.py @@ -611,3 +611,116 @@ class TestHermesBinDirOnPath: 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__API_KEY`` / ``AUXILIARY__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 diff --git a/tools/env_passthrough.py b/tools/env_passthrough.py index 51bff8defdf..633f84566e2 100644 --- a/tools/env_passthrough.py +++ b/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 diff --git a/tools/environments/docker.py b/tools/environments/docker.py index cd4a3fcd86a..74f9fa82c8b 100644 --- a/tools/environments/docker.py +++ b/tools/environments/docker.py @@ -17,7 +17,10 @@ from pathlib import Path 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 @@ class DockerEnvironment(BaseEnvironment): 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) diff --git a/tools/environments/local.py b/tools/environments/local.py index 9324845a2e7..bfebad4ef0c 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -186,6 +186,9 @@ def _build_provider_env_blocklist() -> frozenset: "MODAL_TOKEN_ID", "MODAL_TOKEN_SECRET", "DAYTONA_API_KEY", + "GATEWAY_RELAY_ID", + "GATEWAY_RELAY_SECRET", + "GATEWAY_RELAY_DELIVERY_KEY", }) return frozenset(blocked) @@ -205,6 +208,51 @@ _HERMES_PROVIDER_ENV_BLOCKLIST = _build_provider_env_blocklist() _ACTIVE_VENV_MARKER_VARS = ("VIRTUAL_ENV", "CONDA_PREFIX") +def _is_hermes_internal_secret(key: str) -> bool: + """Return True for Hermes-internal secrets injected under *dynamic* names. + + ``_HERMES_PROVIDER_ENV_BLOCKLIST`` is name-based and derived from the + provider/tool registries, but the gateway and CLI also inject secrets into + ``os.environ`` at runtime under names no static registry knows about: + + - ``AUXILIARY__API_KEY`` / ``AUXILIARY__BASE_URL`` — per-task + side-LLM credentials bridged from ``config.yaml[auxiliary]`` by + ``gateway/run.py`` and ``cli.py`` (vision, web_extract, approval, + compression, and any plugin-registered auxiliary task). These are + separate, often higher-spend API keys plus base URLs that may point at + private endpoints; a model-authored shell command must never see them. + - ``GATEWAY_RELAY_*_SECRET`` / ``GATEWAY_RELAY_*_KEY`` / + ``GATEWAY_RELAY_*_TOKEN`` — relay-auth material provisioned by the + gateway (``GATEWAY_RELAY_SECRET``, ``GATEWAY_RELAY_DELIVERY_KEY``). + These are Tier-1 gateway secrets, like the messaging bot tokens in + ``_ALWAYS_STRIP_KEYS``. Non-secret ``GATEWAY_RELAY_*`` routing hints + (``GATEWAY_RELAY_URL``, ``GATEWAY_RELAY_PLATFORMS``, …) are NOT matched + and remain visible. + + ``code_execution_tool.py`` already catches these via substring matching on + ``KEY`` / ``SECRET`` / ``TOKEN``; the terminal backend's narrower name-based + blocklist did not, which is the leak this predicate closes. + + This is the single source of truth for "Hermes-internal dynamic secret" + across every spawn path — the terminal ``_make_run_env`` / + ``_sanitize_subprocess_env`` filters, the Docker passthrough filter, and the + non-terminal :func:`hermes_subprocess_env` helper all call it, so the + dynamic patterns are stripped **unconditionally** regardless of + ``env_passthrough`` skill registration or ``inherit_credentials``. Nothing + a model-driving CLI legitimately needs matches these patterns. + """ + upper = key.upper() + if upper.startswith("AUXILIARY_") and ( + upper.endswith("_API_KEY") or upper.endswith("_BASE_URL") + ): + return True + if upper.startswith("GATEWAY_RELAY_") and ( + upper.endswith("_SECRET") or upper.endswith("_KEY") or upper.endswith("_TOKEN") + ): + return True + return False + + def _inject_context_hermes_home(env: dict) -> None: """Bridge the context-local Hermes home override into subprocess env.""" try: @@ -229,13 +277,19 @@ def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = Non for key, value in (base_env or {}).items(): if key.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): continue + if _is_hermes_internal_secret(key): + continue if key not in _HERMES_PROVIDER_ENV_BLOCKLIST or _is_passthrough(key): sanitized[key] = value for key, value in (extra_env or {}).items(): if key.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): real_key = key[len(_HERMES_PROVIDER_ENV_FORCE_PREFIX):] + if _is_hermes_internal_secret(real_key): + continue sanitized[real_key] = value + elif _is_hermes_internal_secret(key): + continue elif key not in _HERMES_PROVIDER_ENV_BLOCKLIST or _is_passthrough(key): sanitized[key] = value @@ -273,6 +327,16 @@ _ALWAYS_STRIP_KEYS: frozenset[str] = frozenset({ "SLACK_SIGNING_SECRET", "GATEWAY_ALLOWED_USERS", "GATEWAY_ALLOW_ALL_USERS", + # Gateway relay auth — the ID/secret/delivery-key triplet the gateway + # provisions and persists to the 0600 .env. Stripped unconditionally on + # EVERY spawn surface (terminal + model-driving CLIs) so it can't drift + # between paths: _SECRET / _DELIVERY_KEY are also matched by + # _is_hermes_internal_secret, but _ID has no secret suffix, so it must be + # enumerated here to stay stripped on the inherit_credentials=True path + # (codex / copilot), which skips the Tier-2 blocklist. + "GATEWAY_RELAY_ID", + "GATEWAY_RELAY_SECRET", + "GATEWAY_RELAY_DELIVERY_KEY", "HASS_TOKEN", "EMAIL_PASSWORD", "HERMES_DASHBOARD_SESSION_TOKEN", @@ -320,10 +384,16 @@ def hermes_subprocess_env(*, inherit_credentials: bool = False) -> dict[str, str # Tier 1 — always strip. for key in _ALWAYS_STRIP_KEYS: env.pop(key, None) - # Internal routing hints must never reach a child. + # Internal routing hints and Hermes-internal dynamic secrets + # (``AUXILIARY__API_KEY`` / ``_BASE_URL`` side-LLM credentials, + # ``GATEWAY_RELAY_*`` relay-auth material) must never reach a child, + # regardless of ``inherit_credentials`` — a model-driving CLI has no + # legitimate use for them. See :func:`_is_hermes_internal_secret`. for key in list(env): if key.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): env.pop(key, None) + elif _is_hermes_internal_secret(key): + env.pop(key, None) if not inherit_credentials: # Tier 2 — strip provider/tool credentials unless explicitly inherited. @@ -610,7 +680,11 @@ def _make_run_env(env: dict) -> dict: for k, v in merged.items(): if k.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX): real_key = k[len(_HERMES_PROVIDER_ENV_FORCE_PREFIX):] + if _is_hermes_internal_secret(real_key): + continue run_env[real_key] = v + elif _is_hermes_internal_secret(k): + continue elif k not in _HERMES_PROVIDER_ENV_BLOCKLIST or _is_passthrough(k): run_env[k] = v path_key = _path_env_key(run_env)