fix(config): keep .env publishes inside the routed profile scope under multiplex (#88441)
`save_env_value` / `remove_env_value` already write the right FILE (`get_env_path()` honors the profile-home override, so a routed turn lands in `profiles/<p>/.env`, not the root -- #77490's premise), but the in-process mirror went to `os.environ` unconditionally. Under a multiplexed gateway a `/pair` grant mirrored into `DISCORD_ALLOWED_USERS` from profile B therefore published B's allowlist into the SHARED process env, and B's own installed scope never saw the new value. Add `_publish_env_value`: when multiplex is active and a secret scope is installed, update the installed scope mapping (so same-turn scope reads see the grant) and leave `os.environ` untouched; every other caller keeps the legacy `os.environ` publish. Replace the stale TODO in gateway/pairing.py.
This commit is contained in:
+5
-4
@@ -185,10 +185,11 @@ def _read_allowlist_env(env_var: str) -> str:
|
||||
borrowing the process value. Unscoped callers (single-profile CLI /
|
||||
admin endpoints) keep the legacy ``os.getenv`` read.
|
||||
|
||||
TODO(profile-secrets): the grant mirror below still WRITES through
|
||||
``hermes_cli.config.save_env_value`` / ``remove_env_value``, which target
|
||||
the root ``.env`` — those writes need a profile-aware counterpart before
|
||||
pairing grants can be mirrored correctly under multiplexing.
|
||||
The grant mirror below writes through ``hermes_cli.config.save_env_value``
|
||||
/ ``remove_env_value``: the file target is the active profile's ``.env``
|
||||
(``get_env_path()`` honors the profile-home override) and, under
|
||||
multiplexing, the in-process publish updates the installed scope mapping
|
||||
rather than the shared ``os.environ`` (#88441).
|
||||
"""
|
||||
try:
|
||||
from agent.secret_scope import UnscopedSecretError, get_secret
|
||||
|
||||
+35
-3
@@ -4682,6 +4682,38 @@ def _env_line_defines_key(
|
||||
) == _env_var_policy_name(key, is_windows=is_windows)
|
||||
|
||||
|
||||
def _publish_env_value(key: str, value: Optional[str]) -> None:
|
||||
"""Publish a just-persisted ``.env`` change to the live process.
|
||||
|
||||
``save_env_value`` / ``remove_env_value`` already target the right file
|
||||
(``get_env_path()`` honors the profile-home override), but the in-process
|
||||
mirror historically went straight to ``os.environ``. Under a multiplexed
|
||||
gateway a routed profile's write (e.g. a ``/pair`` grant mirrored into
|
||||
``DISCORD_ALLOWED_USERS``) would then land in the SHARED process env and
|
||||
be visible to every other profile (#88441, #77490). In that case update
|
||||
the installed scope mapping instead so same-turn reads see the change,
|
||||
and leave ``os.environ`` alone. Every other caller keeps the legacy
|
||||
``os.environ`` publish.
|
||||
"""
|
||||
try:
|
||||
from agent.secret_scope import current_secret_scope, is_multiplex_active
|
||||
|
||||
scope = current_secret_scope() if is_multiplex_active() else None
|
||||
except Exception:
|
||||
scope = None
|
||||
if scope is not None:
|
||||
if isinstance(scope, dict):
|
||||
if value is None:
|
||||
scope.pop(key, None)
|
||||
else:
|
||||
scope[key] = value
|
||||
return
|
||||
if value is None:
|
||||
os.environ.pop(key, None)
|
||||
else:
|
||||
os.environ[key] = value
|
||||
|
||||
|
||||
def save_env_value(key: str, value: str):
|
||||
"""Save or update a value in ~/.hermes/.env."""
|
||||
if is_managed():
|
||||
@@ -4770,7 +4802,7 @@ def save_env_value(key: str, value: str):
|
||||
pass
|
||||
raise
|
||||
|
||||
os.environ[key] = value
|
||||
_publish_env_value(key, value)
|
||||
invalidate_env_cache()
|
||||
|
||||
|
||||
@@ -4817,7 +4849,7 @@ def remove_env_value(key: str) -> bool:
|
||||
raise ValueError(f"Invalid environment variable name: {key!r}")
|
||||
env_path = get_env_path()
|
||||
if not env_path.exists():
|
||||
os.environ.pop(key, None)
|
||||
_publish_env_value(key, None)
|
||||
return False
|
||||
|
||||
read_kw = {"encoding": "utf-8-sig", "errors": "replace"}
|
||||
@@ -4861,7 +4893,7 @@ def remove_env_value(key: str) -> bool:
|
||||
pass
|
||||
raise
|
||||
|
||||
os.environ.pop(key, None)
|
||||
_publish_env_value(key, None)
|
||||
invalidate_env_cache()
|
||||
return found
|
||||
|
||||
|
||||
@@ -85,3 +85,40 @@ def test_pairing_store_scoped_to_profile_dir(tmp_path, monkeypatch):
|
||||
assert "profiles/ops/platforms/pairing" in str(store._dir).replace("\\", "/"), (
|
||||
f"store not profile-scoped: {store._dir}"
|
||||
)
|
||||
|
||||
|
||||
def test_routed_pairing_grant_mirror_stays_in_profile_scope(tmp_path, monkeypatch):
|
||||
"""A /pair grant mirrored under a routed profile scope must update THAT
|
||||
profile's .env and installed scope, never the shared os.environ (#88441,
|
||||
#77490). Outside multiplex the legacy os.environ publish is unchanged."""
|
||||
import os
|
||||
|
||||
from agent import secret_scope as ss
|
||||
from gateway.pairing import _sync_allowlist_add
|
||||
from gateway.run import _profile_runtime_scope
|
||||
from hermes_cli.config import save_env_value
|
||||
|
||||
root = tmp_path / ".hermes"
|
||||
prof = root / "profiles" / "b"
|
||||
prof.mkdir(parents=True)
|
||||
(root / ".env").write_text("DISCORD_ALLOWED_USERS=default-admin\n")
|
||||
(prof / ".env").write_text("DISCORD_ALLOWED_USERS=b-admin\n")
|
||||
monkeypatch.setenv("HERMES_HOME", str(root))
|
||||
monkeypatch.setenv("DISCORD_ALLOWED_USERS", "default-admin")
|
||||
|
||||
was_active = ss.is_multiplex_active()
|
||||
ss.set_multiplex_active(True)
|
||||
try:
|
||||
with _profile_runtime_scope(prof):
|
||||
_sync_allowlist_add("discord", "111")
|
||||
assert ss.get_secret("DISCORD_ALLOWED_USERS") == "b-admin,111"
|
||||
finally:
|
||||
ss.set_multiplex_active(was_active)
|
||||
|
||||
assert (prof / ".env").read_text().strip() == "DISCORD_ALLOWED_USERS=b-admin,111"
|
||||
assert (root / ".env").read_text().strip() == "DISCORD_ALLOWED_USERS=default-admin"
|
||||
assert os.environ["DISCORD_ALLOWED_USERS"] == "default-admin"
|
||||
|
||||
# Single-profile: no multiplex -> save still publishes to the process env.
|
||||
save_env_value("DISCORD_ALLOWED_USERS", "default-admin,222")
|
||||
assert os.environ["DISCORD_ALLOWED_USERS"] == "default-admin,222"
|
||||
|
||||
Reference in New Issue
Block a user