diff --git a/tests/tools/test_env_passthrough.py b/tests/tools/test_env_passthrough.py index 6e48ee5c3..eba84bdb2 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 b4686cb13..07bf333a6 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]: