diff --git a/hermes_cli/env_loader.py b/hermes_cli/env_loader.py index adc1e1ff8f..17c329f293 100644 --- a/hermes_cli/env_loader.py +++ b/hermes_cli/env_loader.py @@ -64,6 +64,23 @@ def _known_hermes_env_keys() -> set[str]: return set(OPTIONAL_ENV_VARS.keys()) | set(_EXTRA_ENV_KEYS) +# Behavioral routing keys a parent Hermes process injects into child env and +# that silently redirect a profile onto the wrong provider path (ACP auth +# method, copilot-ACP endpoints). These — and ONLY these — are scrubbed from +# os.environ at startup when absent from the profile's .env. Credential keys +# (API keys/tokens) are excluded: shell exports are a legitimate, +# documented way to supply them, and read-time secret-scope checks +# (agent/secret_scope.py) own cross-profile credential isolation. +_PROFILE_MANAGED_ENV_KEYS: frozenset[str] = frozenset({ + "HERMES_ACP_AUTH_METHOD", + "HERMES_ACP_AUTO_APPROVE", + "HERMES_COPILOT_ACP_COMMAND", + "HERMES_COPILOT_ACP_ARGS", + "COPILOT_CLI_PATH", + "COPILOT_ACP_BASE_URL", +}) + + def _env_keys_defined_in_dotenv(path: Path) -> set[str]: """Return KEY names assigned in a dotenv file (including empty ``KEY=``). @@ -93,18 +110,27 @@ def _env_keys_defined_in_dotenv(path: Path) -> set[str]: def _clear_known_keys_missing_from_dotenv(path: Path) -> None: - """Remove inherited known Hermes keys absent from the profile ``.env``. + """Remove inherited profile-managed Hermes keys absent from ``.env``. After the profile's ``.env`` has been loaded with ``override=True``, - scan the file for which known Hermes keys it explicitly defines and - delete any known key that exists in ``os.environ`` but is *not* present + scan the file for which profile-managed keys it explicitly defines and + delete any such key that exists in ``os.environ`` but is *not* present in the file. - This mirrors the semantics of ``reload_env()`` in ``config.py`` (which - already deletes missing known keys on hot-reload) and closes the gap - between startup and hot-reload: without this, a known key inherited - from the parent process leaks into the profile, silently mutating - provider / ACP / platform behaviour. + Scope is deliberately NARROW: only ``_PROFILE_MANAGED_ENV_KEYS`` — + behavioral routing keys (ACP auth method, copilot-ACP endpoints) that a + parent Hermes process injects and that silently change *which provider + path* a profile uses. Provider API keys (OPENAI_API_KEY, …) are + intentionally excluded: users legitimately export those in their shell + (``export OPENAI_API_KEY=…`` is a documented flow — see + ``tests/hermes_cli/test_dump_env_visibility.py``), and a startup scrub + cannot distinguish a shell export from parent-process leakage. Clearing + the full known-key set would delete user-exported credentials on every + ``hermes`` invocation. + + Cross-profile *credential* isolation is handled at read time by + ``agent.secret_scope.get_secret`` (scope authoritative under + multiplexing), not by mutating ``os.environ`` here. Does **not** run when the ``.env`` file does not exist (bare-profile case, which follows ``#66930`` / ``#67027`` semantics). @@ -112,7 +138,7 @@ def _clear_known_keys_missing_from_dotenv(path: Path) -> None: if not path.exists(): return defined = _env_keys_defined_in_dotenv(path) - for key in _known_hermes_env_keys(): + for key in _PROFILE_MANAGED_ENV_KEYS: if key not in defined and key in os.environ: del os.environ[key] diff --git a/tests/hermes_cli/test_env_loader.py b/tests/hermes_cli/test_env_loader.py index 13503ef7d6..9effed26e5 100644 --- a/tests/hermes_cli/test_env_loader.py +++ b/tests/hermes_cli/test_env_loader.py @@ -281,3 +281,47 @@ def test_export_prefixed_known_key_in_user_env_is_kept(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_ACP_AUTH_METHOD", "cursor_login") load_hermes_dotenv(hermes_home=home) assert os.getenv("HERMES_ACP_AUTH_METHOD") == "claude_code_cli" + + +def test_shell_exported_credentials_survive_cleanup(tmp_path, monkeypatch): + """User-shell-exported provider credentials must NOT be scrubbed. + + ``export OPENAI_API_KEY=…`` in the shell with a ``.env`` that doesn't + contain the key is a documented, legitimate flow (see + test_dump_env_visibility.py). The startup cleanup is scoped to + _PROFILE_MANAGED_ENV_KEYS (ACP routing keys) precisely so it can never + delete shell-supplied credentials — a process cannot distinguish a + shell export from parent-process leakage, so credential isolation is + owned by read-time secret scoping instead. + """ + home = tmp_path / "hermes" + home.mkdir() + (home / ".env").write_text("SOME_OTHER_KEY=x\n", encoding="utf-8") + + monkeypatch.setenv("OPENAI_API_KEY", "sk-from-shell") + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-from-shell") + monkeypatch.setenv("TELEGRAM_BOT_TOKEN", "12345:token-from-shell") + # A profile-managed routing key inherited alongside them IS cleared. + monkeypatch.setenv("HERMES_ACP_AUTH_METHOD", "cursor_login") + + load_hermes_dotenv(hermes_home=home) + + assert os.getenv("OPENAI_API_KEY") == "sk-from-shell" + assert os.getenv("ANTHROPIC_API_KEY") == "sk-ant-from-shell" + assert os.getenv("TELEGRAM_BOT_TOKEN") == "12345:token-from-shell" + assert "HERMES_ACP_AUTH_METHOD" not in os.environ + + +def test_cleanup_scope_is_the_profile_managed_set(): + """Lock the invariant: the startup scrub set contains only behavioral + ACP/routing keys — never credential-shaped keys. If this fails, someone + widened _PROFILE_MANAGED_ENV_KEYS toward the full known-key set, which + re-introduces the shell-export deletion bug. + """ + from hermes_cli.env_loader import _PROFILE_MANAGED_ENV_KEYS + + for key in _PROFILE_MANAGED_ENV_KEYS: + assert not key.endswith(("_API_KEY", "_TOKEN", "_SECRET")), ( + f"{key} looks credential-shaped; startup scrub must not " + "cover credentials — read-time secret scoping owns those" + )