feat: env var passthrough for skills and user config (#2807)

* feat: env var passthrough for skills and user config Skills that declare required_environment_variables now have those vars passed through to sandboxed execution environments (execute_code and terminal). Previously, execute_code stripped all vars containing KEY, TOKEN, SECRET, etc. and the terminal blocklist removed Hermes infrastructure vars — both blocked skill-declared env vars. Two passthrough sources: 1. Skill-scoped (automatic): when a skill is loaded via skill_view and declares required_environment_variables, vars that are present in the environment are registered in a session-scoped passthrough set. 2. Config-based (manual): terminal.env_passthrough in config.yaml lets users explicitly allowlist vars for non-skill use cases. Changes: - New module: tools/env_passthrough.py — shared passthrough registry - hermes_cli/config.py: add terminal.env_passthrough to DEFAULT_CONFIG - tools/skills_tool.py: register available skill env vars on load - tools/code_execution_tool.py: check passthrough before filtering - tools/environments/local.py: check passthrough in _sanitize_subprocess_env and _make_run_env - 19 new tests covering all layers * docs: add environment variable passthrough documentation Document the env var passthrough feature across four docs pages: - security.md: new 'Environment Variable Passthrough' section with full explanation, comparison table, and security considerations - code-execution.md: update security section, add passthrough subsection, fix comparison table - creating-skills.md: add tip about automatic sandbox passthrough - skills.md: add note about passthrough after secure setup docs Live-tested: launched interactive CLI, loaded a skill with required_environment_variables, verified TEST_SKILL_SECRET_KEY was accessible inside execute_code sandbox (value: passthrough-test-value-42).
2026-03-24 08:19:34 -07:00 · 2026-03-24 08:19:34 -07:00 · 745859babb
commit 745859babb
parent ad1bf16f28
11 changed files with 527 additions and 6 deletions
--- a/tests/tools/test_env_passthrough.py
+++ b/tests/tools/test_env_passthrough.py
@ -0,0 +1,199 @@
+"""Tests for tools.env_passthrough — skill and config env var passthrough."""
+
+import os
+import pytest
+import yaml
+
+from tools.env_passthrough import (
+    clear_env_passthrough,
+    get_all_passthrough,
+    is_env_passthrough,
+    register_env_passthrough,
+    reset_config_cache,
+)
+
+
+@pytest.fixture(autouse=True)
+def _clean_passthrough():
+    """Ensure a clean passthrough state for every test."""
+    clear_env_passthrough()
+    reset_config_cache()
+    yield
+    clear_env_passthrough()
+    reset_config_cache()
+
+
+class TestSkillScopedPassthrough:
+    def test_register_and_check(self):
+        assert not is_env_passthrough("TENOR_API_KEY")
+        register_env_passthrough(["TENOR_API_KEY"])
+        assert is_env_passthrough("TENOR_API_KEY")
+
+    def test_register_multiple(self):
+        register_env_passthrough(["FOO_TOKEN", "BAR_SECRET"])
+        assert is_env_passthrough("FOO_TOKEN")
+        assert is_env_passthrough("BAR_SECRET")
+        assert not is_env_passthrough("OTHER_KEY")
+
+    def test_clear(self):
+        register_env_passthrough(["TENOR_API_KEY"])
+        assert is_env_passthrough("TENOR_API_KEY")
+        clear_env_passthrough()
+        assert not is_env_passthrough("TENOR_API_KEY")
+
+    def test_get_all(self):
+        register_env_passthrough(["A_KEY", "B_TOKEN"])
+        result = get_all_passthrough()
+        assert "A_KEY" in result
+        assert "B_TOKEN" in result
+
+    def test_strips_whitespace(self):
+        register_env_passthrough(["  SPACED_KEY  "])
+        assert is_env_passthrough("SPACED_KEY")
+
+    def test_skips_empty(self):
+        register_env_passthrough(["", "  ", "VALID_KEY"])
+        assert is_env_passthrough("VALID_KEY")
+        assert not is_env_passthrough("")
+
+
+class TestConfigPassthrough:
+    def test_reads_from_config(self, tmp_path, monkeypatch):
+        config = {"terminal": {"env_passthrough": ["MY_CUSTOM_KEY", "ANOTHER_TOKEN"]}}
+        config_path = tmp_path / "config.yaml"
+        config_path.write_text(yaml.dump(config))
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        reset_config_cache()
+
+        assert is_env_passthrough("MY_CUSTOM_KEY")
+        assert is_env_passthrough("ANOTHER_TOKEN")
+        assert not is_env_passthrough("UNRELATED_VAR")
+
+    def test_empty_config(self, tmp_path, monkeypatch):
+        config = {"terminal": {"env_passthrough": []}}
+        config_path = tmp_path / "config.yaml"
+        config_path.write_text(yaml.dump(config))
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        reset_config_cache()
+
+        assert not is_env_passthrough("ANYTHING")
+
+    def test_missing_config_key(self, tmp_path, monkeypatch):
+        config = {"terminal": {"backend": "local"}}
+        config_path = tmp_path / "config.yaml"
+        config_path.write_text(yaml.dump(config))
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        reset_config_cache()
+
+        assert not is_env_passthrough("ANYTHING")
+
+    def test_no_config_file(self, tmp_path, monkeypatch):
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        reset_config_cache()
+
+        assert not is_env_passthrough("ANYTHING")
+
+    def test_union_of_skill_and_config(self, tmp_path, monkeypatch):
+        config = {"terminal": {"env_passthrough": ["CONFIG_KEY"]}}
+        config_path = tmp_path / "config.yaml"
+        config_path.write_text(yaml.dump(config))
+        monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+        reset_config_cache()
+
+        register_env_passthrough(["SKILL_KEY"])
+        all_pt = get_all_passthrough()
+        assert "CONFIG_KEY" in all_pt
+        assert "SKILL_KEY" in all_pt
+
+
+class TestExecuteCodeIntegration:
+    """Verify that the passthrough is checked in execute_code's env filtering."""
+
+    def test_secret_substring_blocked_by_default(self):
+        """TENOR_API_KEY should be blocked without passthrough."""
+        _SAFE_ENV_PREFIXES = ("PATH", "HOME", "USER", "LANG", "LC_", "TERM",
+                              "TMPDIR", "TMP", "TEMP", "SHELL", "LOGNAME",
+                              "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA")
+        _SECRET_SUBSTRINGS = ("KEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL",
+                              "PASSWD", "AUTH")
+
+        test_env = {"PATH": "/usr/bin", "TENOR_API_KEY": "test123", "HOME": "/home/user"}
+        child_env = {}
+        for k, v in test_env.items():
+            if is_env_passthrough(k):
+                child_env[k] = v
+                continue
+            if any(s in k.upper() for s in _SECRET_SUBSTRINGS):
+                continue
+            if any(k.startswith(p) for p in _SAFE_ENV_PREFIXES):
+                child_env[k] = v
+
+        assert "PATH" in child_env
+        assert "HOME" in child_env
+        assert "TENOR_API_KEY" not in child_env
+
+    def test_passthrough_allows_secret_through(self):
+        """TENOR_API_KEY should pass through when registered."""
+        _SAFE_ENV_PREFIXES = ("PATH", "HOME", "USER", "LANG", "LC_", "TERM",
+                              "TMPDIR", "TMP", "TEMP", "SHELL", "LOGNAME",
+                              "XDG_", "PYTHONPATH", "VIRTUAL_ENV", "CONDA")
+        _SECRET_SUBSTRINGS = ("KEY", "TOKEN", "SECRET", "PASSWORD", "CREDENTIAL",
+                              "PASSWD", "AUTH")
+
+        register_env_passthrough(["TENOR_API_KEY"])
+
+        test_env = {"PATH": "/usr/bin", "TENOR_API_KEY": "test123", "HOME": "/home/user"}
+        child_env = {}
+        for k, v in test_env.items():
+            if is_env_passthrough(k):
+                child_env[k] = v
+                continue
+            if any(s in k.upper() for s in _SECRET_SUBSTRINGS):
+                continue
+            if any(k.startswith(p) for p in _SAFE_ENV_PREFIXES):
+                child_env[k] = v
+
+        assert "PATH" in child_env
+        assert "HOME" in child_env
+        assert "TENOR_API_KEY" in child_env
+        assert child_env["TENOR_API_KEY"] == "test123"
+
+
+class TestTerminalIntegration:
+    """Verify that the passthrough is checked in terminal's env sanitizers."""
+
+    def test_blocklisted_var_blocked_by_default(self):
+        from tools.environments.local import _sanitize_subprocess_env, _HERMES_PROVIDER_ENV_BLOCKLIST
+
+        # Pick a var we know is in the blocklist
+        blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
+        env = {blocked_var: "secret_value", "PATH": "/usr/bin"}
+        result = _sanitize_subprocess_env(env)
+        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
+
+        blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
+        register_env_passthrough([blocked_var])
+
+        env = {blocked_var: "secret_value", "PATH": "/usr/bin"}
+        result = _sanitize_subprocess_env(env)
+        assert blocked_var in result
+        assert result[blocked_var] == "secret_value"
+
+    def test_make_run_env_passthrough(self, monkeypatch):
+        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")
+
+        # Without passthrough — blocked
+        result_before = _make_run_env({})
+        assert blocked_var not in result_before
+
+        # With passthrough — allowed
+        register_env_passthrough([blocked_var])
+        result_after = _make_run_env({})
+        assert blocked_var in result_after
--- a/tests/tools/test_skill_env_passthrough.py
+++ b/tests/tools/test_skill_env_passthrough.py
@ -0,0 +1,105 @@
+"""Test that skill_view registers required env vars in the passthrough registry."""
+
+import json
+import os
+from pathlib import Path
+from unittest.mock import patch
+
+import pytest
+
+from tools.env_passthrough import clear_env_passthrough, is_env_passthrough, reset_config_cache
+
+
+@pytest.fixture(autouse=True)
+def _clean_passthrough():
+    clear_env_passthrough()
+    reset_config_cache()
+    yield
+    clear_env_passthrough()
+    reset_config_cache()
+
+
+def _create_skill(tmp_path, name, frontmatter_extra=""):
+    """Create a minimal skill directory with SKILL.md."""
+    skill_dir = tmp_path / name
+    skill_dir.mkdir(parents=True, exist_ok=True)
+    (skill_dir / "SKILL.md").write_text(
+        f"---\n"
+        f"name: {name}\n"
+        f"description: Test skill\n"
+        f"{frontmatter_extra}"
+        f"---\n\n"
+        f"# {name}\n\n"
+        f"Test content.\n"
+    )
+    return skill_dir
+
+
+class TestSkillViewRegistersPassthrough:
+    def test_available_env_vars_registered(self, tmp_path, monkeypatch):
+        """When a skill declares required_environment_variables and the var IS set,
+        it should be registered in the passthrough."""
+        _create_skill(
+            tmp_path,
+            "test-skill",
+            frontmatter_extra=(
+                "required_environment_variables:\n"
+                "  - name: TENOR_API_KEY\n"
+                "    prompt: Enter your Tenor API key\n"
+            ),
+        )
+        monkeypatch.setattr(
+            "tools.skills_tool.SKILLS_DIR", tmp_path
+        )
+        # Set the env var so it's "available"
+        monkeypatch.setenv("TENOR_API_KEY", "test-value-123")
+
+        # Patch the secret capture callback to not prompt
+        with patch("tools.skills_tool._secret_capture_callback", None):
+            from tools.skills_tool import skill_view
+
+            result = json.loads(skill_view(name="test-skill"))
+
+        assert result["success"] is True
+        assert is_env_passthrough("TENOR_API_KEY")
+
+    def test_missing_env_vars_not_registered(self, tmp_path, monkeypatch):
+        """When a skill declares required_environment_variables but the var is NOT set,
+        it should NOT be registered in the passthrough."""
+        _create_skill(
+            tmp_path,
+            "test-skill",
+            frontmatter_extra=(
+                "required_environment_variables:\n"
+                "  - name: NONEXISTENT_SKILL_KEY_XYZ\n"
+                "    prompt: Enter your key\n"
+            ),
+        )
+        monkeypatch.setattr(
+            "tools.skills_tool.SKILLS_DIR", tmp_path
+        )
+        monkeypatch.delenv("NONEXISTENT_SKILL_KEY_XYZ", raising=False)
+
+        with patch("tools.skills_tool._secret_capture_callback", None):
+            from tools.skills_tool import skill_view
+
+            result = json.loads(skill_view(name="test-skill"))
+
+        assert result["success"] is True
+        assert not is_env_passthrough("NONEXISTENT_SKILL_KEY_XYZ")
+
+    def test_no_env_vars_skill_no_registration(self, tmp_path, monkeypatch):
+        """Skills without required_environment_variables shouldn't register anything."""
+        _create_skill(tmp_path, "simple-skill")
+        monkeypatch.setattr(
+            "tools.skills_tool.SKILLS_DIR", tmp_path
+        )
+
+        with patch("tools.skills_tool._secret_capture_callback", None):
+            from tools.skills_tool import skill_view
+
+            result = json.loads(skill_view(name="simple-skill"))
+
+        assert result["success"] is True
+        from tools.env_passthrough import get_all_passthrough
+        assert len(get_all_passthrough()) == 0