From b53c50cf8ab98121ac7c6ca94acdf39eea9719cf Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 03:36:57 -0700 Subject: [PATCH] fix(config): resolve ${VAR} config refs through the profile secret scope (#84079) `_env_expand_match` read `os.environ` directly, so under a multiplexed gateway every secondary profile whose config.yaml carried `${MATRIX_ACCESS_TOKEN}` (or `${env:...}`) expanded to the DEFAULT profile's token loaded at startup -- each profile "had" the credential and one inbound message fanned out across all of them. This is the residual half of #84079 the secondary credential gate cannot see (the expanded token is non-empty). Add `_env_ref_lookup`: outside a secret scope it is the same `os.environ.get`; inside a scope it goes through `get_secret`, which is authoritative under multiplexing and an environ overlay otherwise -- the same policy `gateway.config._getenv` and `get_env_value` already follow. The cache env-snapshot (#58514) uses the same lookup so a scoped load is not served another scope's cached expansion. --- hermes_cli/config.py | 34 ++++++++++++++++--- tests/hermes_cli/test_config_env_expansion.py | 28 +++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/hermes_cli/config.py b/hermes_cli/config.py index 1b61e9c99a..3bdadcb78e 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -2985,13 +2985,36 @@ def _strip_dotted_keys(cfg: dict, dotted_keys: set) -> Tuple[dict, set]: return cfg, stripped +def _env_ref_lookup(name: str) -> Optional[str]: + """Resolve the env var behind a ``${VAR}`` / ``${env:VAR}`` config ref. + + Outside a profile secret scope this is a plain ``os.environ`` read — the + default profile and every single-profile caller keep their legacy + behavior. Inside a scope (a multiplexed gateway turn, a secondary + profile's config load, a cron job) the read goes through + ``agent.secret_scope.get_secret`` so the ref resolves against *that* + profile's ``.env``: under multiplexing a miss is a miss, never another + profile's ``os.environ`` value (#84079 — every profile "had" the default + profile's ``${MATRIX_ACCESS_TOKEN}`` and fanned out). Same policy as + ``gateway.config._getenv`` and ``get_env_value``. + """ + try: + from agent.secret_scope import current_secret_scope, get_secret as _get_secret + except Exception: + return os.environ.get(name) + if current_secret_scope() is None: + return os.environ.get(name) + return _get_secret(name) + + def _env_expand_match(m: re.Match) -> str: """Expand one ``${...}`` config reference. Two accepted shapes, matching what MCP server config already resolves (``tools/mcp_tool.py::_env_ref_name``): - * ``${VAR}`` — legacy bare name, resolved via ``os.environ``. + * ``${VAR}`` — legacy bare name, resolved via ``_env_ref_lookup`` + (``os.environ``, or the active profile secret scope). * ``${env:VAR}`` — Cursor-style SecretRef, same resolution after the ``env:`` prefix is stripped. Before this, the prefixed form worked in MCP config but stayed a literal string in config.yaml — a confusing @@ -3009,7 +3032,7 @@ def _env_expand_match(m: re.Match) -> str: name = inner[len("env:"):].strip() if not name: return raw - val = os.environ.get(name) + val = _env_ref_lookup(name) if val is not None: return val logger.warning( @@ -3030,7 +3053,8 @@ def _env_expand_match(m: re.Match) -> str: ) return raw # Legacy ``${VAR}`` — bare name. - return os.environ.get(inner, raw) + val = _env_ref_lookup(inner) + return val if val is not None else raw def _env_ref_var_name(ref: str) -> Optional[str]: @@ -3083,7 +3107,7 @@ def _env_ref_snapshot(obj, snapshot=None): for raw in re.findall(r"\${([^}]+)}", obj): name = _env_ref_var_name(raw) if name is not None: - snapshot[name] = os.environ.get(name) + snapshot[name] = _env_ref_lookup(name) elif isinstance(obj, dict): for value in obj.values(): _env_ref_snapshot(value, snapshot) @@ -4097,7 +4121,7 @@ def _load_config_impl(*, want_deepcopy: bool) -> Dict[str, Any]: # pins unexpanded literals (e.g. auxiliary..api_key) for the # life of the process (#58514). env_snapshot = cached[5] if len(cached) > 5 else {} - if all(os.environ.get(k) == v for k, v in env_snapshot.items()): + if all(_env_ref_lookup(k) == v for k, v in env_snapshot.items()): return copy.deepcopy(cached[4]) if want_deepcopy else cached[4] config = copy.deepcopy(DEFAULT_CONFIG) diff --git a/tests/hermes_cli/test_config_env_expansion.py b/tests/hermes_cli/test_config_env_expansion.py index 207ae5625f..6571015245 100644 --- a/tests/hermes_cli/test_config_env_expansion.py +++ b/tests/hermes_cli/test_config_env_expansion.py @@ -122,3 +122,31 @@ class TestLoadCliConfigExpansion: config = load_cli_config() assert config["auxiliary"]["vision"]["api_key"] == "${UNSET_CLI_VAR_ABC}" + + +class TestExpansionUnderProfileScope: + """``${VAR}`` refs must resolve against the active profile's secret scope, + not the shared process environment (#84079): under multiplex every + secondary profile otherwise "had" the default profile's token and fanned + out. Outside multiplex the scope is an overlay and environ still applies.""" + + def test_scoped_ref_never_reads_another_profiles_environ(self, monkeypatch): + from agent import secret_scope as ss + + monkeypatch.setenv("MATRIX_ACCESS_TOKEN", "default-token") + was_active = ss.is_multiplex_active() + ss.set_multiplex_active(True) + token = ss.set_secret_scope({"OTHER_KEY": "x"}) # profile-b: no matrix token + try: + assert _expand_env_vars("${MATRIX_ACCESS_TOKEN}") == "${MATRIX_ACCESS_TOKEN}" + assert _expand_env_vars("${env:MATRIX_ACCESS_TOKEN}") == "${env:MATRIX_ACCESS_TOKEN}" + finally: + ss.reset_secret_scope(token) + token = ss.set_secret_scope({"MATRIX_ACCESS_TOKEN": "c-token"}) + try: + assert _expand_env_vars("${MATRIX_ACCESS_TOKEN}") == "c-token" + finally: + ss.reset_secret_scope(token) + ss.set_multiplex_active(was_active) + # Unscoped (default profile / single-profile CLI): legacy environ read. + assert _expand_env_vars("${MATRIX_ACCESS_TOKEN}") == "default-token"