mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-01 01:51:44 +00:00
The "cfg.get('X', {}).get('Y', default)" pattern appears 50+ times
across tools/, gateway/, and plugins/. Each call site manually handles
the same three gotchas:
1. Missing intermediate key → empty dict → chain works
2. Non-dict value at intermediate position → AttributeError
(uncaught in most sites, so a misconfigured YAML crashes the tool)
3. cfg is None → AttributeError
Introduces cfg_get(cfg, *keys, default=None) in hermes_cli/config.py
as the canonical helper. Handles all three uniformly, returns default
only when the final key is *absent* (matches dict.get semantics —
explicit None values are preserved, falsy values like 0 / False / ''
are preserved).
Named cfg_get rather than cfg_path to avoid shadowing the existing
'cfg_path = _hermes_home / "config.yaml"' local variable that appears
in gateway/run.py, cron/scheduler.py, hermes_cli/main.py, etc.
Migrated 20 call sites as the first-batch proof-of-value:
gateway/run.py 10 sites (agent/display subtrees)
tools/browser_tool.py 3 sites
tools/vision_tools.py 2 sites
tools/browser_camofox.py 1 site
tools/approval.py 1 site
tools/skills_tool.py 1 site
tools/skill_manager_tool.py 1 site
tools/credential_files.py 1 site
tools/env_passthrough.py 1 site
The remaining ~30 sites across plugins/ and smaller tool files can be
migrated opportunistically — the helper is now available and the
pattern is established.
Fixed a latent bug along the way: tools/vision_tools.py had its
cfg_get usage at line 560 inside a function that locally re-imports
'from hermes_cli.config import load_config', but the AST-based
migration script wrote the top-level cfg_get import to a different
function scope, leaving line 560's cfg_get as a NameError silently
swallowed by the surrounding try/except. Test
test_vision_uses_configured_temperature_and_timeout caught it. Fixed
by including cfg_get in the function-local import.
Verified:
- 7880/7893 tests/tools/ + tests/gateway/ + tests/hermes_cli/test_config
tests pass; all 13 failures pre-existing on main (MCP, delegate,
session_split_brain — verified earlier in the sweep).
- All 20 migrated sites AST-verified to have cfg_get in scope (either
module-level or function-local).
- Live 'hermes chat' smoke: 2 turns + /model switch + tool calls +
/quit, zero errors. Agent correctly counted 20 cfg_get hits across
8 tool files — matching the migration.
Semantic parity verified against the original pattern across 8 edge
cases (missing keys, None values, falsy values, empty strings, string
instead of dict, None cfg, nested levels).
145 lines
5.4 KiB
Python
145 lines
5.4 KiB
Python
"""Environment variable passthrough registry.
|
|
|
|
Skills that declare ``required_environment_variables`` in their frontmatter
|
|
need those vars available in sandboxed execution environments (execute_code,
|
|
terminal). By default both sandboxes strip secrets from the child process
|
|
environment for security. This module provides a session-scoped allowlist
|
|
so skill-declared vars (and user-configured overrides) pass through.
|
|
|
|
Two sources feed the allowlist:
|
|
|
|
1. **Skill declarations** — when a skill is loaded via ``skill_view``, its
|
|
``required_environment_variables`` are registered here automatically.
|
|
2. **User config** — ``terminal.env_passthrough`` in config.yaml lets users
|
|
explicitly allowlist vars for non-skill use cases.
|
|
|
|
Both ``code_execution_tool.py`` and ``tools/environments/local.py`` consult
|
|
:func:`is_env_passthrough` before stripping a variable.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import logging
|
|
from contextvars import ContextVar
|
|
from typing import Iterable
|
|
from hermes_cli.config import cfg_get
|
|
|
|
logger = logging.getLogger(__name__)
|
|
|
|
# Session-scoped set of env var names that should pass through to sandboxes.
|
|
# Backed by ContextVar to prevent cross-session data bleed in the gateway pipeline.
|
|
_allowed_env_vars_var: ContextVar[set[str]] = ContextVar("_allowed_env_vars")
|
|
|
|
|
|
def _get_allowed() -> set[str]:
|
|
"""Get or create the allowed env vars set for the current context/session."""
|
|
try:
|
|
return _allowed_env_vars_var.get()
|
|
except LookupError:
|
|
val: set[str] = set()
|
|
_allowed_env_vars_var.set(val)
|
|
return val
|
|
|
|
|
|
# Cache for the config-based allowlist (loaded once per process).
|
|
_config_passthrough: frozenset[str] | None = None
|
|
|
|
|
|
def _is_hermes_provider_credential(name: str) -> bool:
|
|
"""True if ``name`` is a Hermes-managed provider credential (API key,
|
|
token, or similar) per ``_HERMES_PROVIDER_ENV_BLOCKLIST``.
|
|
|
|
Skill-declared ``required_environment_variables`` frontmatter must
|
|
not be able to override this list — that was the bypass in
|
|
GHSA-rhgp-j443-p4rf where a malicious skill registered
|
|
``ANTHROPIC_TOKEN`` / ``OPENAI_API_KEY`` as passthrough and received
|
|
the credential in the ``execute_code`` child process, defeating the
|
|
sandbox's scrubbing guarantee.
|
|
|
|
Non-Hermes API keys (TENOR_API_KEY, NOTION_TOKEN, etc.) are NOT
|
|
in the blocklist and remain legitimately registerable — skills that
|
|
wrap third-party APIs still work.
|
|
"""
|
|
try:
|
|
from tools.environments.local import _HERMES_PROVIDER_ENV_BLOCKLIST
|
|
except Exception:
|
|
return False
|
|
return name in _HERMES_PROVIDER_ENV_BLOCKLIST
|
|
|
|
|
|
def register_env_passthrough(var_names: Iterable[str]) -> None:
|
|
"""Register environment variable names as allowed in sandboxed environments.
|
|
|
|
Typically called when a skill declares ``required_environment_variables``.
|
|
|
|
Variables that are Hermes-managed provider credentials (from
|
|
``_HERMES_PROVIDER_ENV_BLOCKLIST``) are rejected here to preserve
|
|
the ``execute_code`` sandbox's credential-scrubbing guarantee per
|
|
GHSA-rhgp-j443-p4rf. A skill that needs to talk to a Hermes-managed
|
|
provider should do so via the agent's main-process tools (web_search,
|
|
web_extract, etc.) where the credential remains safely in the main
|
|
process.
|
|
|
|
Non-Hermes third-party API keys (TENOR_API_KEY, NOTION_TOKEN, etc.)
|
|
pass through normally — they were never in the sandbox scrub list.
|
|
"""
|
|
for name in var_names:
|
|
name = name.strip()
|
|
if not name:
|
|
continue
|
|
if _is_hermes_provider_credential(name):
|
|
logger.warning(
|
|
"env passthrough: refusing to register Hermes provider "
|
|
"credential %r (blocked by _HERMES_PROVIDER_ENV_BLOCKLIST). "
|
|
"Skills must not override the execute_code sandbox's "
|
|
"credential scrubbing; see GHSA-rhgp-j443-p4rf.",
|
|
name,
|
|
)
|
|
continue
|
|
_get_allowed().add(name)
|
|
logger.debug("env passthrough: registered %s", name)
|
|
|
|
|
|
def _load_config_passthrough() -> frozenset[str]:
|
|
"""Load ``tools.env_passthrough`` from config.yaml (cached)."""
|
|
global _config_passthrough
|
|
if _config_passthrough is not None:
|
|
return _config_passthrough
|
|
|
|
result: set[str] = set()
|
|
try:
|
|
from hermes_cli.config import read_raw_config
|
|
cfg = read_raw_config()
|
|
passthrough = cfg_get(cfg, "terminal", "env_passthrough")
|
|
if isinstance(passthrough, list):
|
|
for item in passthrough:
|
|
if isinstance(item, str) and item.strip():
|
|
result.add(item.strip())
|
|
except Exception as e:
|
|
logger.debug("Could not read tools.env_passthrough from config: %s", e)
|
|
|
|
_config_passthrough = frozenset(result)
|
|
return _config_passthrough
|
|
|
|
|
|
def is_env_passthrough(var_name: str) -> bool:
|
|
"""Check whether *var_name* is allowed to pass through to sandboxes.
|
|
|
|
Returns ``True`` if the variable was registered by a skill or listed in
|
|
the user's ``tools.env_passthrough`` config.
|
|
"""
|
|
if var_name in _get_allowed():
|
|
return True
|
|
return var_name in _load_config_passthrough()
|
|
|
|
|
|
def get_all_passthrough() -> frozenset[str]:
|
|
"""Return the union of skill-registered and config-based passthrough vars."""
|
|
return frozenset(_get_allowed()) | _load_config_passthrough()
|
|
|
|
|
|
def clear_env_passthrough() -> None:
|
|
"""Reset the skill-scoped allowlist (e.g. on session reset)."""
|
|
_get_allowed().clear()
|
|
|
|
|