From ba4357d13b1f1ae29ebc202ffc557d32e99a04ce Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 21 Apr 2026 06:14:25 -0700 Subject: [PATCH] fix(env_passthrough): reject Hermes provider credentials from skill passthrough (#13523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- tests/tools/test_env_passthrough.py | 60 ++++++++++++++++++++++------- tools/env_passthrough.py | 49 +++++++++++++++++++++-- 2 files changed, 92 insertions(+), 17 deletions(-) diff --git a/tests/tools/test_env_passthrough.py b/tests/tools/test_env_passthrough.py index 6e48ee5c30..eba84bdb2c 100644 --- a/tests/tools/test_env_passthrough.py +++ b/tests/tools/test_env_passthrough.py @@ -172,28 +172,60 @@ class TestTerminalIntegration: assert blocked_var not in result assert "PATH" in result - def test_passthrough_allows_blocklisted_var(self): - from tools.environments.local import _sanitize_subprocess_env, _HERMES_PROVIDER_ENV_BLOCKLIST + def test_passthrough_cannot_override_provider_blocklist(self): + """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)) + # Attempt to register — must be silently refused (logged warning). 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"} result = _sanitize_subprocess_env(env) - assert blocked_var in result - assert result[blocked_var] == "secret_value" + assert blocked_var not in result + assert "PATH" in result - def test_make_run_env_passthrough(self, monkeypatch): - from tools.environments.local import _make_run_env, _HERMES_PROVIDER_ENV_BLOCKLIST + 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.""" + import os + from tools.environments.local import ( + _make_run_env, + _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 + result_before = _make_run_env({}) + assert blocked_var not in result_before - # Without passthrough — blocked - result_before = _make_run_env({}) - assert blocked_var not in result_before + # Skill tries to register it — must be refused, so still blocked + register_env_passthrough([blocked_var]) + result_after = _make_run_env({}) + assert blocked_var not in result_after + finally: + os.environ.pop(blocked_var, None) - # With passthrough — allowed - register_env_passthrough([blocked_var]) - result_after = _make_run_env({}) - assert blocked_var in result_after + 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") diff --git a/tools/env_passthrough.py b/tools/env_passthrough.py index b4686cb13f..07bf333a60 100644 --- a/tools/env_passthrough.py +++ b/tools/env_passthrough.py @@ -44,16 +44,59 @@ def _get_allowed() -> set[str]: _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 name: - _get_allowed().add(name) - logger.debug("env passthrough: registered %s", 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) + logger.debug("env passthrough: registered %s", name) def _load_config_passthrough() -> frozenset[str]: