fix(security): centralize credential-safe subprocess env (#29157)

Subprocesses spawned outside the terminal/execute_code path (agent-browser,
copilot ACP, dep-ensure, lazy_deps uv install, TUI Node host, cli.exec)
inherited the operator's full credential environment via os.environ.copy().
The terminal path was already scrubbed by _HERMES_PROVIDER_ENV_BLOCKLIST
(#1002/#1264/#32314); these spawn sites bypassed it.

Adds hermes_subprocess_env(inherit_credentials=) in tools/environments/local.py
reusing the existing dynamic blocklist as the single source of truth:

  - Tier 1 (_ALWAYS_STRIP_KEYS): gateway bot tokens, GitHub auth, infra
    secrets -- stripped even for credential-inheriting children.
  - Tier 2 (_HERMES_PROVIDER_ENV_BLOCKLIST): provider/tool keys -- stripped
    unless inherit_credentials=True. The opt-in is grep-able for audit.

Browser worker keeps a _BROWSER_PASSTHROUGH_KEYS allowlist (BROWSERBASE/
FIRECRAWL) re-added after the strip. Model-driving children (ACP, TUI Node
host, cli.exec) use inherit_credentials=True so they still get provider keys
while losing Tier-1 secrets. Installers (dep-ensure, lazy_deps) inherit
nothing sensitive. cua_backend already routed through _sanitize_subprocess_env
on main -- left as-is. Gateway adapter utility spawns (gh pr comment, ffmpeg)
are left inheriting env: gh needs GH_TOKEN by design, ffmpeg is a trusted
system binary -- no untrusted-dependency exposure.

This is defense-in-depth (personal-assistant trust model: same-user spawns),
making the existing scrub policy uniform across the spawn surface; the main
real payoff is shrinking the blast radius if a transitive npm dep in
agent-browser is compromised.

Reconstructed on current main from the design in #31959 (Tranquil-Flow);
also credits #39003 (rodboev), #37843 (coygeek), #35769 (egilewski).

Co-authored-by: Tranquil-Flow <tranquil_flow@protonmail.com>
Co-authored-by: rodboev <rod.boev@gmail.com>
Co-authored-by: egilewski <egilewski@egilewski.com>

agent/copilot_acp_client.py

@@ -23,6 +23,7 @@ from typing import Any
 
 from agent.file_safety import get_read_block_error, is_write_denied
 from agent.redact import redact_sensitive_text
+from tools.environments.local import hermes_subprocess_env
 
 ACP_MARKER_BASE_URL = "acp://copilot"
 _DEFAULT_TIMEOUT_SECONDS = 900.0
@@ -94,7 +95,10 @@ def _resolve_home_dir() -> str:
 
 
 def _build_subprocess_env() -> dict[str, str]:
-    env = os.environ.copy()
+    # Copilot ACP is a model-driving CLI executor: it legitimately needs LLM
+    # provider credentials. Route through the central helper so Tier-1 secrets
+    # (gateway bot tokens, GitHub auth, infra) are still stripped (#29157).
+    env = hermes_subprocess_env(inherit_credentials=True)
     home = _resolve_home_dir()
     env["HOME"] = home
     from hermes_constants import apply_subprocess_home_env

hermes_cli/dep_ensure.py

@@ -23,6 +23,7 @@ import sys
 from pathlib import Path
 
 from hermes_constants import agent_browser_runnable
+from tools.environments.local import hermes_subprocess_env
 
 _IS_WINDOWS = platform.system() == "Windows"
 
@@ -148,7 +149,8 @@ def ensure_dependency(
     else:
         cmd = ["bash", str(script), "--ensure", dep]
 
-    run_env = {**os.environ, "IS_INTERACTIVE": "false"}
+    run_env = hermes_subprocess_env(inherit_credentials=False)
+    run_env["IS_INTERACTIVE"] = "false"
     result = subprocess.run(
         cmd,
         env=run_env,

tests/tools/test_hermes_subprocess_env.py

@@ -0,0 +1,151 @@
+"""Tests for hermes_subprocess_env() — the centralized credential-safe env
+builder for the non-terminal subprocess spawn surface.
+
+Covers GHSA-m4m8-xjp4-5rmm / issue #29157: subprocesses spawned by the
+gateway/browser/ACP/installer paths must not blindly inherit the operator's
+full credential environment. Two tiers:
+
+  * Tier 1 (_ALWAYS_STRIP_KEYS): gateway bot tokens, GitHub auth, infra
+    secrets — stripped even when inherit_credentials=True.
+  * Tier 2 (_HERMES_PROVIDER_ENV_BLOCKLIST): LLM provider/tool keys — stripped
+    unless the caller opts into inherit_credentials=True.
+"""
+
+import os
+from unittest.mock import patch
+
+from tools.environments.local import (
+    hermes_subprocess_env,
+    _ALWAYS_STRIP_KEYS,
+    _HERMES_PROVIDER_ENV_FORCE_PREFIX,
+)
+
+
+_TIER1_SAMPLE = {
+    "GH_TOKEN": "ghp_secret",
+    "TELEGRAM_BOT_TOKEN": "bot-token",
+    "SLACK_APP_TOKEN": "xapp-secret",
+    "MODAL_TOKEN_SECRET": "modal-secret",
+    "HERMES_DASHBOARD_SESSION_TOKEN": "dash-secret",
+}
+
+_PROVIDER_SAMPLE = {
+    "OPENAI_API_KEY": "sk-fake",
+    "ANTHROPIC_API_KEY": "ant-fake",
+    "OPENROUTER_API_KEY": "or-fake",
+}
+
+_SAFE_SAMPLE = {
+    "PATH": "/usr/bin:/bin",
+    "HOME": "/home/user",
+    "USER": "testuser",
+    "MY_APP_VAR": "keep-me",
+}
+
+
+def _build(extra=None, *, inherit_credentials=False):
+    env = dict(_SAFE_SAMPLE)
+    if extra:
+        env.update(extra)
+    with patch.dict(os.environ, env, clear=True):
+        return hermes_subprocess_env(inherit_credentials=inherit_credentials)
+
+
+class TestStripByDefault:
+    def test_provider_keys_stripped_by_default(self):
+        result = _build(_PROVIDER_SAMPLE)
+        for var in _PROVIDER_SAMPLE:
+            assert var not in result, f"{var} leaked with inherit_credentials=False"
+
+    def test_tier1_secrets_stripped_by_default(self):
+        result = _build(_TIER1_SAMPLE)
+        for var in _TIER1_SAMPLE:
+            assert var not in result, f"{var} leaked (Tier-1) with inherit_credentials=False"
+
+    def test_safe_vars_preserved(self):
+        result = _build()
+        assert result["HOME"] == "/home/user"
+        assert result["USER"] == "testuser"
+        assert "PATH" in result
+        assert result["MY_APP_VAR"] == "keep-me"
+
+    def test_force_prefix_hints_stripped(self):
+        result = _build({f"{_HERMES_PROVIDER_ENV_FORCE_PREFIX}OPENAI_API_KEY": "sk-x"})
+        assert f"{_HERMES_PROVIDER_ENV_FORCE_PREFIX}OPENAI_API_KEY" not in result
+        assert "OPENAI_API_KEY" not in result
+
+    def test_pythonutf8_set(self):
+        result = _build()
+        assert result.get("PYTHONUTF8") == "1"
+
+
+class TestInheritCredentials:
+    def test_provider_keys_preserved_when_inheriting(self):
+        result = _build(_PROVIDER_SAMPLE, inherit_credentials=True)
+        for var, val in _PROVIDER_SAMPLE.items():
+            assert result.get(var) == val, f"{var} should survive inherit_credentials=True"
+
+    def test_tier1_secrets_stripped_even_when_inheriting(self):
+        """The whole point of Tier 1: gateway/GitHub/infra secrets never reach
+        a child, even a model-driving CLI that legitimately needs provider keys."""
+        result = _build({**_PROVIDER_SAMPLE, **_TIER1_SAMPLE}, inherit_credentials=True)
+        for var in _TIER1_SAMPLE:
+            assert var not in result, (
+                f"{var} (Tier-1) must be stripped even with inherit_credentials=True"
+            )
+        # ...while provider keys survive.
+        for var in _PROVIDER_SAMPLE:
+            assert var in result
+
+    def test_pythonutf8_set_when_inheriting(self):
+        assert _build(inherit_credentials=True).get("PYTHONUTF8") == "1"
+
+
+class TestTierInvariants:
+    def test_tier1_always_stripped_both_paths(self):
+        """Behavioral invariant: every Tier-1 key is stripped on BOTH the
+        default path and the inherit_credentials=True path. This is what
+        guarantees no gap, regardless of whether the key also happens to be
+        in the provider blocklist."""
+        sample = {k: f"secret-{k}" for k in _ALWAYS_STRIP_KEYS}
+        for inherit in (False, True):
+            result = _build(sample, inherit_credentials=inherit)
+            leaked = {k for k in _ALWAYS_STRIP_KEYS if k in result}
+            assert not leaked, (
+                f"Tier-1 keys leaked with inherit_credentials={inherit}: {sorted(leaked)}"
+            )
+
+    def test_tier1_covers_gateway_bot_token(self):
+        assert "TELEGRAM_BOT_TOKEN" in _ALWAYS_STRIP_KEYS
+
+    def test_tier1_covers_github_auth(self):
+        assert {"GH_TOKEN", "GITHUB_TOKEN"} <= _ALWAYS_STRIP_KEYS
+
+    def test_tier1_covers_infra_secrets(self):
+        assert {"MODAL_TOKEN_ID", "MODAL_TOKEN_SECRET", "DAYTONA_API_KEY"} <= _ALWAYS_STRIP_KEYS
+
+
+class TestBrowserPassthroughPattern:
+    def test_browser_keys_recoverable_after_strip(self):
+        """Browser tool pattern: strip everything, then re-add the browser
+        backend keys agent-browser actually needs."""
+        from tools.browser_tool import _BROWSER_PASSTHROUGH_KEYS
+
+        leaked = {
+            "BROWSERBASE_API_KEY": "bb-key",
+            "BROWSERBASE_PROJECT_ID": "bb-proj",
+            "FIRECRAWL_API_KEY": "fc-key",
+            "ANTHROPIC_API_KEY": "ant-should-go",
+            "TELEGRAM_BOT_TOKEN": "bot-should-go",
+        }
+        with patch.dict(os.environ, {**_SAFE_SAMPLE, **leaked}, clear=True):
+            env = hermes_subprocess_env(inherit_credentials=False)
+            for key in _BROWSER_PASSTHROUGH_KEYS:
+                if key in os.environ:
+                    env[key] = os.environ[key]
+
+        assert env["BROWSERBASE_API_KEY"] == "bb-key"
+        assert env["FIRECRAWL_API_KEY"] == "fc-key"
+        # Provider + gateway secrets must NOT come back.
+        assert "ANTHROPIC_API_KEY" not in env
+        assert "TELEGRAM_BOT_TOKEN" not in env

tools/browser_tool.py

@@ -69,6 +69,22 @@ from hermes_constants import agent_browser_runnable, get_hermes_home
 from utils import env_int, is_truthy_value
 from hermes_cli.config import DEFAULT_CONFIG, cfg_get
 from hermes_cli._subprocess_compat import windows_hide_flags
+from tools.environments.local import hermes_subprocess_env
+
+# Browser-specific tool keys passed through to the agent-browser subprocess
+# AFTER credential stripping.  agent-browser is a Node process loading npm
+# deps; handing it the full operator keyring (#29157 / GHSA-m4m8-xjp4-5rmm)
+# means a compromised transitive dependency could read every Hermes secret
+# straight out of process.env.  Strip by default, then re-add only the
+# browser-backend keys the worker legitimately needs.
+_BROWSER_PASSTHROUGH_KEYS: tuple[str, ...] = (
+    "BROWSERBASE_API_KEY",
+    "BROWSERBASE_PROJECT_ID",
+    "BROWSER_USE_API_KEY",
+    "FIRECRAWL_API_KEY",
+    "FIRECRAWL_API_URL",
+    "FIRECRAWL_BROWSER_TTL",
+)
 
 try:
     from tools.website_policy import check_website_access
@@ -872,7 +888,11 @@ def _run_chrome_fallback_command(
 
     task_socket_dir = os.path.join(_socket_safe_tmpdir(), f"agent-browser-{tmp_session}")
     os.makedirs(task_socket_dir, mode=0o700, exist_ok=True)
-    browser_env = {**os.environ, "AGENT_BROWSER_SOCKET_DIR": task_socket_dir}
+    browser_env = hermes_subprocess_env(inherit_credentials=False)
+    for _key in _BROWSER_PASSTHROUGH_KEYS:
+        if _key in os.environ:
+            browser_env[_key] = os.environ[_key]
+    browser_env["AGENT_BROWSER_SOCKET_DIR"] = task_socket_dir
     browser_env["PATH"] = _merge_browser_path(browser_env.get("PATH", ""))
 
     if "AGENT_BROWSER_IDLE_TIMEOUT_MS" not in browser_env:
@@ -2125,7 +2145,10 @@ def _run_browser_command(
         logger.debug("browser cmd=%s task=%s socket_dir=%s (%d chars)",
                      command, task_id, task_socket_dir, len(task_socket_dir))
 
-        browser_env = {**os.environ}
+        browser_env = hermes_subprocess_env(inherit_credentials=False)
+        for _key in _BROWSER_PASSTHROUGH_KEYS:
+            if _key in os.environ:
+                browser_env[_key] = os.environ[_key]
 
         # Ensure subprocesses inherit the same browser-specific PATH fallbacks
         # used during CLI discovery.

tools/environments/local.py

@@ -250,6 +250,100 @@ def _sanitize_subprocess_env(base_env: dict | None, extra_env: dict | None = Non
     return sanitized
 
 
+# Tier-1 secrets: stripped from EVERY spawned subprocess unconditionally —
+# even when the caller opts into credential inheritance for a model-driving
+# CLI (claude / codex / gemini).  These are not LLM provider credentials; no
+# legitimate child Hermes spawns needs them, and they are the highest-value
+# secrets to keep out of a compromised dependency's reach (gateway bot tokens,
+# GitHub auth, remote-compute tokens, dashboard session secret).  The set is a
+# narrow subset of _HERMES_PROVIDER_ENV_BLOCKLIST; provider keys are handled by
+# the conditional Tier-2 strip in hermes_subprocess_env().
+_ALWAYS_STRIP_KEYS: frozenset[str] = frozenset({
+    # GitHub auth
+    "GH_TOKEN",
+    "GITHUB_TOKEN",
+    "GITHUB_APP_ID",
+    "GITHUB_APP_PRIVATE_KEY_PATH",
+    "GITHUB_APP_INSTALLATION_ID",
+    # Gateway / messaging bot tokens and access control
+    "TELEGRAM_BOT_TOKEN",
+    "DISCORD_BOT_TOKEN",
+    "SLACK_BOT_TOKEN",
+    "SLACK_APP_TOKEN",
+    "SLACK_SIGNING_SECRET",
+    "GATEWAY_ALLOWED_USERS",
+    "GATEWAY_ALLOW_ALL_USERS",
+    "HASS_TOKEN",
+    "EMAIL_PASSWORD",
+    "HERMES_DASHBOARD_SESSION_TOKEN",
+    # Remote-compute / infrastructure secrets
+    "MODAL_TOKEN_ID",
+    "MODAL_TOKEN_SECRET",
+    "DAYTONA_API_KEY",
+})
+
+
+def hermes_subprocess_env(*, inherit_credentials: bool = False) -> dict[str, str]:
+    """Build a sanitized environment dict for a spawned subprocess.
+
+    Centralized helper for the **non-terminal** spawn surface (browser,
+    ACP/CLI executors, computer-use driver, dep-ensure, TUI Node host,
+    detached gateway).  Use this instead of copying ``os.environ`` directly
+    so strip-by-default is the uniform policy across every spawn site, with a
+    single source of truth (``_HERMES_PROVIDER_ENV_BLOCKLIST``).  The terminal
+    / execute_code path keeps using :func:`_sanitize_subprocess_env`, which is
+    skill-aware (``env_passthrough``); this helper is for spawns that have no
+    skill-passthrough concept.
+
+    Two-tier stripping:
+
+    * **Tier 1 (always):** ``_ALWAYS_STRIP_KEYS`` — gateway bot tokens, GitHub
+      auth, and remote-compute secrets are removed regardless of
+      ``inherit_credentials``.  No child Hermes spawns legitimately needs them.
+    * **Tier 2 (conditional):** the rest of ``_HERMES_PROVIDER_ENV_BLOCKLIST``
+      (LLM provider API keys, tool secrets) is removed unless the caller passes
+      ``inherit_credentials=True``.
+
+    Pass ``inherit_credentials=True`` **only** when the child legitimately
+    needs LLM provider credentials — a user-blessed ``claude`` / ``codex`` /
+    ``gemini`` CLI executor, or the TUI Node host that makes model calls.  The
+    flag is grep-able for audit: ``grep -rn 'inherit_credentials=True'`` lists
+    every spawn site that still receives provider credentials.
+
+    Callers that need a *specific* non-provider secret (e.g. the browser worker
+    needs ``BROWSERBASE_API_KEY`` / ``FIRECRAWL_API_KEY``) should call with
+    ``inherit_credentials=False`` and copy just those keys back from
+    ``os.environ`` into the returned dict.
+    """
+    env = os.environ.copy()
+
+    # Tier 1 — always strip.
+    for key in _ALWAYS_STRIP_KEYS:
+        env.pop(key, None)
+    # Internal routing hints must never reach a child.
+    for key in list(env):
+        if key.startswith(_HERMES_PROVIDER_ENV_FORCE_PREFIX):
+            env.pop(key, None)
+
+    if not inherit_credentials:
+        # Tier 2 — strip provider/tool credentials unless explicitly inherited.
+        for key in _HERMES_PROVIDER_ENV_BLOCKLIST:
+            env.pop(key, None)
+
+    # Windows UTF-8 safety for spawned processes (#31420).
+    env.setdefault("PYTHONUTF8", "1")
+
+    _inject_context_hermes_home(env)
+    from hermes_constants import apply_subprocess_home_env
+    apply_subprocess_home_env(env)
+
+    # Active-venv markers must not clobber another project's environment.
+    for _marker in _ACTIVE_VENV_MARKER_VARS:
+        env.pop(_marker, None)
+
+    return env
+
+
 def _find_bash() -> str:
     """Find bash for command execution."""
     if not _IS_WINDOWS:

tools/lazy_deps.py

@@ -613,7 +613,9 @@ def _venv_pip_install(specs: tuple[str, ...], *, timeout: int = 300) -> _Install
 
     try:
         venv_root = Path(sys.executable).parent.parent
-        uv_env = {**os.environ, "VIRTUAL_ENV": str(venv_root)}
+        from tools.environments.local import hermes_subprocess_env
+        uv_env = hermes_subprocess_env(inherit_credentials=False)
+        uv_env["VIRTUAL_ENV"] = str(venv_root)
 
         # Tier 1: uv (preferred — fast, doesn't need pip in the venv)
         uv_bin = shutil.which("uv")

tui_gateway/server.py

@@ -25,6 +25,7 @@ from hermes_constants import (
 )
 from hermes_cli.env_loader import load_hermes_dotenv
 from utils import is_truthy_value
+from tools.environments.local import hermes_subprocess_env
 from tui_gateway import git_probe
 from tui_gateway.transport import (
     StdioTransport,
@@ -284,7 +285,9 @@ class _SlashWorker:
             text=True,
             bufsize=1,
             cwd=os.getcwd(),
-            env=os.environ.copy(),
+            # slash_worker runs the Hermes agent → needs provider credentials.
+            # Tier-1 secrets (gateway/GitHub/infra) are still stripped (#29157).
+            env=hermes_subprocess_env(inherit_credentials=True),
         )
         threading.Thread(target=self._drain_stdout, daemon=True).start()
         threading.Thread(target=self._drain_stderr, daemon=True).start()
@@ -11131,7 +11134,9 @@ def _(rid, params: dict) -> dict:
             text=True,
             timeout=min(int(params.get("timeout", 240)), 600),
             cwd=os.getcwd(),
-            env=os.environ.copy(),
+            # cli.exec runs `python -m hermes_cli.main` (can drive the agent) →
+            # needs provider credentials. Tier-1 secrets still stripped (#29157).
+            env=hermes_subprocess_env(inherit_credentials=True),
             stdin=subprocess.DEVNULL,
         )
         parts = [r.stdout or "", r.stderr or ""]