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.
This commit is contained in:
+29
-5
@@ -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.<task>.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)
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user