mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-19 15:18:03 +00:00
fix(env-passthrough): fail closed when provider blocklist import fails
When tools.environments.local can't be imported (partial install, import-time error), _is_hermes_provider_credential() returned False — fail-open. A skill could then register a Hermes provider credential (ANTHROPIC_API_KEY, etc.) as env passthrough; _scrub_child_env lets passthrough vars bypass the secret-substring net (rule 1), so the operator's real key would land in the execute_code child. Reopens the GHSA-rhgp-j443-p4rf bypass. Fail closed instead: on import failure, treat the name as a protected provider credential and refuse passthrough. Regression test exercises the full register -> scrub path under a simulated import failure. Co-authored-by: Hermes Agent <noreply@nousresearch.com>
This commit is contained in:
parent
58c36b1798
commit
d7a1052424
3 changed files with 63 additions and 2 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue