mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(env_passthrough): reject Hermes provider credentials from skill passthrough (#13523)
A skill declaring `required_environment_variables: [ANTHROPIC_TOKEN]` in its SKILL.md frontmatter silently bypassed the `execute_code` sandbox's credential-scrubbing guarantee. `register_env_passthrough` had no blocklist, so any name a skill chose flipped `is_env_passthrough(name) => True`, which shortcircuits the sandbox's secret filter. Fix: reject registration when the name appears in `_HERMES_PROVIDER_ENV_BLOCKLIST` (the canonical list of Hermes-managed credentials — provider keys, gateway tokens, etc.). Log a warning naming GHSA-rhgp-j443-p4rf so operators see the rejection in logs. Non-Hermes third-party API keys (TENOR_API_KEY for gif-search, NOTION_TOKEN for notion skills, etc.) remain legitimately registerable — they were never in the sandbox scrub list in the first place. Tests: 16 -> 17 passing. Two old tests that documented the bypass (`test_passthrough_allows_blocklisted_var`, `test_make_run_env_passthrough`) are rewritten to assert the new fail-closed behavior. New `test_non_hermes_api_key_still_registerable` locks in that legitimate third-party keys are unaffected. Reported in GHSA-rhgp-j443-p4rf by @q1uf3ng. Hardening; not CVE-worthy on its own per the decision matrix (attacker must already have operator consent to install a malicious skill).
This commit is contained in:
parent
7fc1e91811
commit
ba4357d13b
2 changed files with 92 additions and 17 deletions
|
|
@ -172,28 +172,60 @@ class TestTerminalIntegration:
|
||||||
assert blocked_var not in result
|
assert blocked_var not in result
|
||||||
assert "PATH" in result
|
assert "PATH" in result
|
||||||
|
|
||||||
def test_passthrough_allows_blocklisted_var(self):
|
def test_passthrough_cannot_override_provider_blocklist(self):
|
||||||
from tools.environments.local import _sanitize_subprocess_env, _HERMES_PROVIDER_ENV_BLOCKLIST
|
"""GHSA-rhgp-j443-p4rf: register_env_passthrough must NOT accept
|
||||||
|
Hermes provider credentials — that was the bypass where a skill
|
||||||
|
could declare ANTHROPIC_TOKEN / OPENAI_API_KEY as passthrough and
|
||||||
|
defeat the execute_code sandbox scrubbing."""
|
||||||
|
from tools.environments.local import (
|
||||||
|
_sanitize_subprocess_env,
|
||||||
|
_HERMES_PROVIDER_ENV_BLOCKLIST,
|
||||||
|
)
|
||||||
|
|
||||||
blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
|
blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
|
||||||
|
# Attempt to register — must be silently refused (logged warning).
|
||||||
register_env_passthrough([blocked_var])
|
register_env_passthrough([blocked_var])
|
||||||
|
|
||||||
|
# is_env_passthrough must NOT report it as allowed
|
||||||
|
assert not is_env_passthrough(blocked_var)
|
||||||
|
|
||||||
|
# Sanitizer still strips the var from subprocess env
|
||||||
env = {blocked_var: "secret_value", "PATH": "/usr/bin"}
|
env = {blocked_var: "secret_value", "PATH": "/usr/bin"}
|
||||||
result = _sanitize_subprocess_env(env)
|
result = _sanitize_subprocess_env(env)
|
||||||
assert blocked_var in result
|
assert blocked_var not in result
|
||||||
assert result[blocked_var] == "secret_value"
|
assert "PATH" in result
|
||||||
|
|
||||||
def test_make_run_env_passthrough(self, monkeypatch):
|
def test_make_run_env_blocklist_override_rejected(self):
|
||||||
from tools.environments.local import _make_run_env, _HERMES_PROVIDER_ENV_BLOCKLIST
|
"""_make_run_env must NOT expose a blocklisted var to subprocess env
|
||||||
|
even after a skill attempts to register it via passthrough."""
|
||||||
|
import os
|
||||||
|
from tools.environments.local import (
|
||||||
|
_make_run_env,
|
||||||
|
_HERMES_PROVIDER_ENV_BLOCKLIST,
|
||||||
|
)
|
||||||
|
|
||||||
blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
|
blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
|
||||||
monkeypatch.setenv(blocked_var, "secret_value")
|
os.environ[blocked_var] = "secret_value"
|
||||||
|
try:
|
||||||
# Without passthrough — blocked
|
# Without passthrough — blocked
|
||||||
result_before = _make_run_env({})
|
result_before = _make_run_env({})
|
||||||
assert blocked_var not in result_before
|
assert blocked_var not in result_before
|
||||||
|
|
||||||
# With passthrough — allowed
|
# Skill tries to register it — must be refused, so still blocked
|
||||||
register_env_passthrough([blocked_var])
|
register_env_passthrough([blocked_var])
|
||||||
result_after = _make_run_env({})
|
result_after = _make_run_env({})
|
||||||
assert blocked_var in result_after
|
assert blocked_var not in result_after
|
||||||
|
finally:
|
||||||
|
os.environ.pop(blocked_var, None)
|
||||||
|
|
||||||
|
def test_non_hermes_api_key_still_registerable(self):
|
||||||
|
"""Third-party API keys (TENOR_API_KEY, NOTION_TOKEN, etc.) are NOT
|
||||||
|
Hermes provider credentials and must still pass through — skills
|
||||||
|
that legitimately wrap third-party APIs must keep working."""
|
||||||
|
# TENOR_API_KEY is a real example — used by the gif-search skill
|
||||||
|
register_env_passthrough(["TENOR_API_KEY"])
|
||||||
|
assert is_env_passthrough("TENOR_API_KEY")
|
||||||
|
|
||||||
|
# Arbitrary skill-specific var
|
||||||
|
register_env_passthrough(["MY_SKILL_CUSTOM_CONFIG"])
|
||||||
|
assert is_env_passthrough("MY_SKILL_CUSTOM_CONFIG")
|
||||||
|
|
|
||||||
|
|
@ -44,14 +44,57 @@ def _get_allowed() -> set[str]:
|
||||||
_config_passthrough: frozenset[str] | None = None
|
_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:
|
def register_env_passthrough(var_names: Iterable[str]) -> None:
|
||||||
"""Register environment variable names as allowed in sandboxed environments.
|
"""Register environment variable names as allowed in sandboxed environments.
|
||||||
|
|
||||||
Typically called when a skill declares ``required_environment_variables``.
|
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:
|
for name in var_names:
|
||||||
name = name.strip()
|
name = name.strip()
|
||||||
if name:
|
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)
|
_get_allowed().add(name)
|
||||||
logger.debug("env passthrough: registered %s", name)
|
logger.debug("env passthrough: registered %s", name)
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue