fix(config): harden MCP env policy on Windows
This commit is contained in:
committed by
Teknium
parent
08cf4fea5d
commit
5425ba14f2
+28
-3
@@ -216,6 +216,9 @@ _ENV_VAR_NAME_DENYLIST: frozenset[str] = frozenset({
|
||||
# HERMES_LANGFUSE_*, HERMES_SPOTIFY_*, ...) ARE allowed.
|
||||
"HERMES_HOME", "HERMES_PROFILE", "HERMES_CONFIG", "HERMES_ENV",
|
||||
"HERMES_CONFIG_PATH", "HERMES_ENV_PATH",
|
||||
# MCP catalog trust root. Package-manager wrappers may still provide this
|
||||
# in the process environment; only generic persistence writes are blocked.
|
||||
"HERMES_OPTIONAL_MCPS",
|
||||
# Hermes security policy / approval-routing context. These remain available
|
||||
# through their dedicated CLI/config/session controls, but a generic
|
||||
# credential writer must not persist them for the next process startup.
|
||||
@@ -226,13 +229,24 @@ _ENV_VAR_NAME_DENYLIST: frozenset[str] = frozenset({
|
||||
})
|
||||
|
||||
|
||||
def _env_var_policy_name(key: str, *, is_windows: Optional[bool] = None) -> str:
|
||||
"""Return the name used for environment policy comparisons.
|
||||
|
||||
Windows environment names are case-insensitive; POSIX names are not. The
|
||||
explicit override keeps both semantics directly testable without pretending
|
||||
the test interpreter is running on another host OS.
|
||||
"""
|
||||
windows = _IS_WINDOWS if is_windows is None else is_windows
|
||||
return key.upper() if windows else key
|
||||
|
||||
|
||||
def _reject_denylisted_env_var(key: str) -> None:
|
||||
"""Raise if ``key`` is in :data:`_ENV_VAR_NAME_DENYLIST`.
|
||||
|
||||
Centralised so both the regular and "secure" env writers share the
|
||||
same gate, and so the message is consistent for callers.
|
||||
"""
|
||||
if key in _ENV_VAR_NAME_DENYLIST:
|
||||
if _env_var_policy_name(key) in _ENV_VAR_NAME_DENYLIST:
|
||||
raise ValueError(
|
||||
f"Environment variable {key!r} is on the writer denylist. "
|
||||
"Names that influence subprocess execution (LD_PRELOAD, "
|
||||
@@ -4231,7 +4245,12 @@ def _quote_env_value(value: str) -> str:
|
||||
return f'"{escaped}"'
|
||||
|
||||
|
||||
def _env_line_defines_key(line: str, key: str) -> bool:
|
||||
def _env_line_defines_key(
|
||||
line: str,
|
||||
key: str,
|
||||
*,
|
||||
is_windows: Optional[bool] = None,
|
||||
) -> bool:
|
||||
"""True when a .env line assigns ``key`` — plain or ``export``-prefixed.
|
||||
|
||||
``load_env()`` accepts the bash-compatible ``export KEY=value`` form
|
||||
@@ -4242,7 +4261,13 @@ def _env_line_defines_key(line: str, key: str) -> bool:
|
||||
stripped = line.strip()
|
||||
if stripped.startswith("export "):
|
||||
stripped = stripped[7:].lstrip()
|
||||
return stripped.startswith(f"{key}=")
|
||||
assigned_key, separator, _value = stripped.partition("=")
|
||||
if not separator:
|
||||
return False
|
||||
return _env_var_policy_name(
|
||||
assigned_key,
|
||||
is_windows=is_windows,
|
||||
) == _env_var_policy_name(key, is_windows=is_windows)
|
||||
|
||||
|
||||
def save_env_value(key: str, value: str):
|
||||
|
||||
@@ -1094,6 +1094,7 @@ class TestEnvWriteDenylist:
|
||||
[
|
||||
"HERMES_CONFIG_PATH",
|
||||
"HERMES_ENV_PATH",
|
||||
"HERMES_OPTIONAL_MCPS",
|
||||
"HERMES_YOLO_MODE",
|
||||
"HERMES_ACCEPT_HOOKS",
|
||||
"HERMES_REDACT_SECRETS",
|
||||
@@ -1113,6 +1114,50 @@ class TestEnvWriteDenylist:
|
||||
|
||||
assert protected_key not in load_env()
|
||||
|
||||
def test_preexisting_optional_mcps_override_still_loads(self, tmp_path):
|
||||
"""The writer gate must not migrate or ignore operator-owned .env state."""
|
||||
from hermes_cli.config import invalidate_env_cache
|
||||
|
||||
catalog = tmp_path / "custom-mcp-catalog"
|
||||
(tmp_path / ".env").write_text(
|
||||
f"HERMES_OPTIONAL_MCPS={catalog}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
invalidate_env_cache()
|
||||
|
||||
assert load_env()["HERMES_OPTIONAL_MCPS"] == str(catalog)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("key", "expected"),
|
||||
[
|
||||
("Path", "PATH"),
|
||||
("Hermes_Yolo_Mode", "HERMES_YOLO_MODE"),
|
||||
("Hermes_Optional_Mcps", "HERMES_OPTIONAL_MCPS"),
|
||||
],
|
||||
)
|
||||
def test_windows_policy_names_are_case_insensitive(self, key, expected):
|
||||
from hermes_cli.config import _env_var_policy_name
|
||||
|
||||
assert _env_var_policy_name(key, is_windows=True) == expected
|
||||
|
||||
def test_posix_policy_names_remain_case_sensitive(self):
|
||||
from hermes_cli.config import _env_var_policy_name
|
||||
|
||||
assert _env_var_policy_name("Path", is_windows=False) == "Path"
|
||||
|
||||
@pytest.mark.parametrize("prefix", ["", "export "])
|
||||
def test_windows_env_assignment_matching_is_case_insensitive(self, prefix):
|
||||
from hermes_cli.config import _env_line_defines_key
|
||||
|
||||
line = f"{prefix}Path=C:\\Windows\\System32\n"
|
||||
assert _env_line_defines_key(line, "PATH", is_windows=True)
|
||||
assert not _env_line_defines_key(line, "PATH", is_windows=False)
|
||||
|
||||
@pytest.mark.windows_only
|
||||
def test_windows_writer_rejects_mixed_case_protected_name(self):
|
||||
with pytest.raises(ValueError, match="denylist"):
|
||||
save_env_value("Hermes_Yolo_Mode", "1")
|
||||
|
||||
|
||||
|
||||
def test_save_env_value_secure_inherits_denylist(self):
|
||||
|
||||
@@ -164,18 +164,29 @@ def test_catalog_accepts_declared_credential(
|
||||
).read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_generic_env_endpoint_rejects_yolo_control_key(
|
||||
@pytest.mark.parametrize(
|
||||
"protected_key",
|
||||
["HERMES_YOLO_MODE", "HERMES_OPTIONAL_MCPS"],
|
||||
)
|
||||
def test_generic_env_endpoint_rejects_protected_key(
|
||||
client: TestClient,
|
||||
catalog_env: Path,
|
||||
protected_key: str,
|
||||
):
|
||||
response = client.put(
|
||||
"/api/env",
|
||||
headers=HEADERS,
|
||||
json={"key": "HERMES_YOLO_MODE", "value": "1"},
|
||||
json={"key": protected_key, "value": "must-not-land"},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
env_path = catalog_env / ".env"
|
||||
assert not env_path.exists() or "HERMES_YOLO_MODE" not in env_path.read_text(
|
||||
assert not env_path.exists() or protected_key not in env_path.read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def test_process_supplied_catalog_root_remains_supported(catalog_env: Path):
|
||||
from hermes_cli.mcp_catalog import get_entry
|
||||
|
||||
assert get_entry("demo") is not None
|
||||
|
||||
Reference in New Issue
Block a user