Skip to content

fix(security): keep Hermes secrets out of env passthrough#3948

Closed
Gutslabs wants to merge 1 commit into
NousResearch:mainfrom
Gutslabs:fix/protect-hermes-secret-passthrough
Closed

fix(security): keep Hermes secrets out of env passthrough#3948
Gutslabs wants to merge 1 commit into
NousResearch:mainfrom
Gutslabs:fix/protect-hermes-secret-passthrough

Conversation

@Gutslabs

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a sandbox secret-filter bypass in skill and config env passthrough.

Hermes strips provider API keys, gateway tokens, and tool credentials from terminal / execute_code child environments by default. However, a skill could declare one of those Hermes-managed secrets in required_environment_variables, or a user could list it in terminal.env_passthrough, and the passthrough registry would re-allow it.

That meant loading a third-party skill that declared something like OPENAI_API_KEY could silently punch Hermes-managed secrets back into sandboxed child processes.

This change keeps Hermes-managed secrets blocked even when a skill or config entry tries to pass them through, and updates the docs to match the enforced behavior.

Type of Change

  • Bug fix
  • Security fix
  • Tests
  • Documentation

Changes Made

  • Added a shared Hermes-sensitive env blocklist module
  • Reused that blocklist in the local terminal sanitizer
  • Rejected Hermes-managed secrets from skill-scoped env passthrough
  • Ignored Hermes-managed secrets in terminal.env_passthrough
  • Added regression tests covering skill and config bypass attempts
  • Updated passthrough docs to describe the enforced restriction

How to Test

  1. Run source .venv/bin/activate
  2. Run python -m pytest tests/tools/test_env_passthrough.py tests/tools/test_skill_env_passthrough.py tests/tools/test_local_env_blocklist.py tests/tools/test_skills_tool.py -q
  3. Load a skill that declares required_environment_variables: [OPENAI_API_KEY, TENOR_API_KEY]
  4. Confirm OPENAI_API_KEY is not registered/passed through, while TENOR_API_KEY still is
  5. Add OPENAI_API_KEY to terminal.env_passthrough and confirm it is ignored

Validation

  • python -m pytest tests/tools/test_env_passthrough.py tests/tools/test_skill_env_passthrough.py tests/tools/test_local_env_blocklist.py tests/tools/test_skills_tool.py -q117 passed
  • Manual repro before the fix: a skill declaring OPENAI_API_KEY caused is_env_passthrough("OPENAI_API_KEY") == True and _sanitize_subprocess_env(...) kept the secret
  • Manual repro after the fix: OPENAI_API_KEY stays blocked while normal skill-specific keys like TENOR_API_KEY still pass through

@dieutx

dieutx commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Nice catch on the env passthrough. I've been working on credential file protection in #2714 (read-path deny list). Worth checking that the secret patterns blocked here in env passthrough align with the credential file deny list — both approaches should share the same definition of what counts as a 'secret' to avoid gaps.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround tool/terminal Terminal execution and process management tool/skills Skills system (list, view, manage) labels May 2, 2026
@UmbraAtrox

Copy link
Copy Markdown

One of my agent's skills does exactly that. required_environment_variables OPENROUTER_API_KEY to curl for credits remaining. Is there a mechanic panned to unblock this block? What do we do for skills that need hermes' secrets?

@teknium1

Copy link
Copy Markdown
Contributor

Closing — the underlying config.yaml passthrough bypass you identified IS real, but it just landed via #27794 (3ab7e2a on main) using the narrower approach of reusing the existing _HERMES_PROVIDER_ENV_BLOCKLIST constant in tools/environments/local.py rather than introducing a parallel tools/sensitive_env.py module.

Your PR refactors the blocklist into a new module that's then imported by env_passthrough + environments/local. That's a reasonable architectural shape, but on top of the existing usage we'd end up touching every importer to switch sources — net churn for the same security guarantee. The narrower #27794 path closes the gap without moving the source of truth.

Thanks for flagging the skill-side + config-side passthrough asymmetry — that framing is what drove the narrow fix.

@teknium1 teknium1 closed this May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 High — major feature broken, no workaround tool/skills Skills system (list, view, manage) tool/terminal Terminal execution and process management type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants