From ed1170cd8ba17740910315cce4711806290a9224 Mon Sep 17 00:00:00 2001 From: webtecnica Date: Sat, 1 Aug 2026 16:15:31 -0700 Subject: [PATCH] =?UTF-8?q?fix(config):=20make=20get=5Fenv=5Fvalue=20scope?= =?UTF-8?q?-aware=20=E2=80=94=20the=20last=20scope-blind=20credential=20re?= =?UTF-8?q?ader?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Salvaged premise from #67065 (@webtecnica, issue #67027), reimplemented: get_env_value() read os.environ first with no secret-scope check, so a multiplexed profile turn could serve another profile's credential. Its siblings get_env_value_prefer_dotenv and gateway.config._getenv were already scope-aware. Reimplementation note: the original diff called get_secret() but fell through to os.environ on a scoped miss — re-opening the exact leak it targeted (flagged by the sweeper review). This version delegates policy fully to agent.secret_scope.get_secret (global vars pass through; scope authoritative under multiplexing; legacy environ behavior when off; UnscopedSecretError propagates fail-closed), then falls back to .env. 6 regression tests incl. the #67027 repro (envless profile + multiplexed turn -> None, not the other profile's key); sabotage-verified RED on the old implementation. --- hermes_cli/config.py | 34 ++++++++- tests/hermes_cli/test_get_env_value_scope.py | 79 ++++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 tests/hermes_cli/test_get_env_value_scope.py diff --git a/hermes_cli/config.py b/hermes_cli/config.py index ed5c191bc1698..ff4629dacad78 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -4107,10 +4107,36 @@ def reload_env() -> int: def get_env_value(key: str) -> Optional[str]: - """Get a value from ~/.hermes/.env or environment.""" - # Check environment first - if key in os.environ: - return os.environ[key] + """Get a value from ``os.environ`` or ``~/.hermes/.env``, scope-aware. + + The ``os.environ`` read routes through ``agent.secret_scope.get_secret`` + so that, under an active profile scope (multiplexed gateway turn), this + is scope-checked rather than leaking another profile's raw ``os.environ`` + value. ``get_secret`` encodes the whole policy: global vars pass through; + scope is authoritative under multiplexing (miss -> None, no environ + fallthrough); when multiplexing is off it behaves exactly like the + legacy ``os.environ`` read. Its siblings ``get_env_value_prefer_dotenv`` + and ``gateway.config._getenv`` already work this way — this was the last + scope-blind reader of the trio (#67027). + """ + try: + from agent.secret_scope import ( + UnscopedSecretError, + get_secret as _get_secret, + ) + except Exception: + if key in os.environ: + return os.environ[key] + return load_env().get(key) + + try: + val = _get_secret(key) + except UnscopedSecretError: + raise + except Exception: + val = os.environ.get(key) + if val is not None: + return val # Then check .env file env_vars = load_env() diff --git a/tests/hermes_cli/test_get_env_value_scope.py b/tests/hermes_cli/test_get_env_value_scope.py new file mode 100644 index 0000000000000..7df90dbec816e --- /dev/null +++ b/tests/hermes_cli/test_get_env_value_scope.py @@ -0,0 +1,79 @@ +"""get_env_value must be scope-aware — the last scope-blind reader (#67027). + +Under a multiplexed profile turn, ``os.environ`` can hold another profile's +value. ``get_env_value`` previously returned it before any scope check; +its siblings (``get_env_value_prefer_dotenv``, ``gateway.config._getenv``) +were already scope-aware. Salvaged premise from PR #67065 (@webtecnica), +reimplemented to delegate policy fully to ``agent.secret_scope.get_secret`` +(the PR's own diff fell through to ``os.environ`` on a scoped miss, +re-opening the leak it targeted). +""" + +import contextlib + +import pytest + +from agent.secret_scope import ( + UnscopedSecretError, + reset_secret_scope, + set_multiplex_active, + set_secret_scope, +) +from hermes_cli.config import get_env_value + + +@contextlib.contextmanager +def _scope(secrets, *, multiplex: bool): + set_multiplex_active(multiplex) + token = set_secret_scope(secrets) if secrets is not None else None + try: + yield + finally: + if token is not None: + reset_secret_scope(token) + set_multiplex_active(False) + + +def test_scoped_value_wins_over_environ(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-other-profile") + with _scope({"OPENAI_API_KEY": "sk-this-profile"}, multiplex=True): + assert get_env_value("OPENAI_API_KEY") == "sk-this-profile" + + +def test_multiplexed_scope_miss_does_not_leak_environ(monkeypatch, tmp_path): + """The #67027 repro: envless named profile must NOT inherit the other + profile's credential from process environ during a multiplexed turn.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path / ".hermes")) + monkeypatch.setenv("OPENAI_API_KEY", "sk-other-profile") + with _scope({}, multiplex=True): # profile has no .env / empty scope + assert get_env_value("OPENAI_API_KEY") is None + + +def test_multiplex_off_scope_miss_falls_back_to_environ(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-from-shell") + with _scope({}, multiplex=False): + assert get_env_value("OPENAI_API_KEY") == "sk-from-shell" + + +def test_no_scope_single_profile_behaves_like_legacy(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-legacy") + with _scope(None, multiplex=False): + assert get_env_value("OPENAI_API_KEY") == "sk-legacy" + + +def test_multiplex_active_unscoped_read_fails_closed(monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "sk-should-not-leak") + with _scope(None, multiplex=True): + # No scope installed — must raise, not silently serve environ. + with pytest.raises(UnscopedSecretError): + get_env_value("OPENAI_API_KEY") + + +def test_dotenv_fallback_still_works(monkeypatch, tmp_path): + home = tmp_path / ".hermes" + home.mkdir(parents=True) + (home / ".env").write_text("MY_TEST_KEY=from-dotenv\n", encoding="utf-8") + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv("MY_TEST_KEY", raising=False) + with _scope(None, multiplex=False): + assert get_env_value("MY_TEST_KEY") == "from-dotenv"