diff --git a/scripts/release.py b/scripts/release.py index 3ce210ade921..ef343c30ae3c 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -47,6 +47,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json" AUTHOR_MAP = { "der@konsi.org": "konsisumer", # PR #19608 salvage (read-modify-write merge in write_credential_pool to preserve concurrently-added credentials; #19566) "linyubin@users.noreply.github.com": "linyubin", # PR #50228 salvage (eager fallback on persistent transport timeout/overloaded; #22277) + "65363919+coygeek@users.noreply.github.com": "coygeek", # PR #37951 salvage (fail closed when provider env blocklist import fails; #37950) "5261694+djstunami@users.noreply.github.com": "djstunami", # PR #5316 salvage / co-author (suppress transient check_fn flakes so subagents keep file/terminal tools; #21658 / #5304) "jmmaloney4@gmail.com": "jmmaloney4", # PR #25206 salvage (re-select credential pool on primary runtime restore; #25205) "dale@dalenguyen.me": "dalenguyen", # PR #53678 salvage (strip VIRTUAL_ENV/CONDA_PREFIX from terminal subprocess env; #23473) diff --git a/tests/tools/test_env_passthrough.py b/tests/tools/test_env_passthrough.py index 974911e588d3..a9d706636cbb 100644 --- a/tests/tools/test_env_passthrough.py +++ b/tests/tools/test_env_passthrough.py @@ -228,3 +228,52 @@ class TestTerminalIntegration: # Arbitrary skill-specific var register_env_passthrough(["MY_SKILL_CUSTOM_CONFIG"]) assert is_env_passthrough("MY_SKILL_CUSTOM_CONFIG") + + def test_provider_blocklist_import_failure_fails_closed(self, monkeypatch): + """If the dynamic provider blocklist can't be imported, provider + credentials must be treated as protected and refused passthrough — + otherwise a skill could tunnel a Hermes credential into the + execute_code child (regression for #37950 / GHSA-rhgp-j443-p4rf). + + Verifies the full path: _is_hermes_provider_credential returns True, + register_env_passthrough refuses the var, and _scrub_child_env keeps + it out of the child env. A non-Hermes key is also rejected here (the + fallback is conservative: when we can't tell, we fail closed), which + is the safe direction. + """ + import builtins + + from tools.code_execution_tool import _scrub_child_env + + real_import = builtins.__import__ + + def fail_local_import(name, *args, **kwargs): + if name == "tools.environments.local": + raise ImportError("synthetic blocklist import failure") + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", fail_local_import) + + # Every name is now treated as a protected provider credential. + assert _ep_mod._is_hermes_provider_credential("OPENAI_API_KEY") + assert _ep_mod._is_hermes_provider_credential("ANTHROPIC_API_KEY") + assert _ep_mod._is_hermes_provider_credential("GH_TOKEN") + + # Registration is refused while the blocklist is unavailable. + register_env_passthrough(["OPENAI_API_KEY", "ANTHROPIC_API_KEY"]) + assert not is_env_passthrough("OPENAI_API_KEY") + assert not is_env_passthrough("ANTHROPIC_API_KEY") + + # And the credential never reaches the execute_code child. + child_env = _scrub_child_env( + { + "OPENAI_API_KEY": "synthetic-secret", + "ANTHROPIC_API_KEY": "synthetic-secret", + "PATH": "/usr/bin", + }, + is_passthrough=is_env_passthrough, + is_windows=False, + ) + assert "OPENAI_API_KEY" not in child_env + assert "ANTHROPIC_API_KEY" not in child_env + assert child_env["PATH"] == "/usr/bin" diff --git a/tools/env_passthrough.py b/tools/env_passthrough.py index 5efee177d003..51bff8defdf7 100644 --- a/tools/env_passthrough.py +++ b/tools/env_passthrough.py @@ -59,11 +59,22 @@ def _is_hermes_provider_credential(name: str) -> bool: 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. + + Fail closed: if the authoritative blocklist cannot be imported (partial + install, import-time error, etc.) we treat the name as a protected + provider credential and refuse passthrough, rather than fall open and + let a skill tunnel a Hermes credential into the execute_code child. """ try: from tools.environments.local import _HERMES_PROVIDER_ENV_BLOCKLIST - except Exception: - return False + except Exception as e: + logger.warning( + "env passthrough: provider credential blocklist import failed; " + "failing closed and refusing passthrough registration for %r: %s", + name, + e, + ) + return True return name in _HERMES_PROVIDER_ENV_BLOCKLIST