mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(security): strip dynamic Hermes secrets from all subprocess spawn env
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_<TASK>_API_KEY` / `AUXILIARY_<TASK>_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 <sswdarius@gmail.com> Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com> Co-authored-by: claudlos <claudlos@agentmail.to>
This commit is contained in:
parent
053424c486
commit
a658f3b28b
8 changed files with 401 additions and 6 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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_<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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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_<TASK>_API_KEY`` / ``AUXILIARY_<TASK>_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_<TASK>_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)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue