diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 8a5fe6bc64..cb39d48e21 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -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): diff --git a/tests/hermes_cli/test_config.py b/tests/hermes_cli/test_config.py index 12d35c3180..3d04b4e0ab 100644 --- a/tests/hermes_cli/test_config.py +++ b/tests/hermes_cli/test_config.py @@ -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): diff --git a/tests/hermes_cli/test_mcp_catalog_env_boundary.py b/tests/hermes_cli/test_mcp_catalog_env_boundary.py index 38d46a04a0..a1256e4c8d 100644 --- a/tests/hermes_cli/test_mcp_catalog_env_boundary.py +++ b/tests/hermes_cli/test_mcp_catalog_env_boundary.py @@ -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" - ) \ No newline at end of file + ) + + +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