hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 28 Packages Projects Releases Wiki Activity

fix(env_loader): restore only values a source is authoritative for

Browse Source 
The per-home snapshot (`_SECRET_SOURCE_VALUES_BY_HOME`) deliberately also holds names the source
supplied but .env/shell WON (`skipped_existing`, #102041) — `build_profile_secret_scope` needs them.
Re-asserting the whole snapshot on a dotenv reload therefore froze a gap-filled or preserve-listed
name at its first-load value and a later .env edit lost, where a cold start would have honoured it.

`AppliedVar.authoritative` (= `override_existing` and not `preserve_existing`, computed where the
precedence is decided) feeds a per-home restore subset `_SECRET_SOURCE_RESTORE_BY_HOME`; the reload
re-asserts only that. The contributor's pre-load snapshot is dropped: nothing between the dotenv loads
writes the module dict, so reading the subset at the restore site is equivalent and skips the copy.
This commit is contained in:
kshitijk4poor
2026-09-19 14:57:25 +05:30
committed by kshitij
parent 303d839133
commit da2e571b98
3 changed files with 49 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
agent/secret_sources/registry.py
View File

@@ -52,6 +52,9 @@ class AppliedVar:
source: str # SecretSource.name
shape: str # "mapped" | "bulk"
overrode_env: bool # replaced a pre-existing .env/shell value
# The source may beat .env/shell for this var (``override_existing`` and not ``preserve_existing``), so a
# dotenv reload may re-assert it; a gap-fill or preserved name must keep following .env edits (#74265).
authoritative: bool = False
@dataclass
@@ -359,7 +362,8 @@ class _Applier:
self.env[var] = value
self.claimed[var] = source.name
sr.applied.append(var)
self.report.provenance[var] = AppliedVar(var, source.name, source.shape, overrode_env=existed)
self.report.provenance[var] = AppliedVar(var, source.name, source.shape, overrode_env=existed,
authoritative=override and var not in self.preserve)
return True

27
hermes_cli/env_loader.py
View File

@@ -34,6 +34,8 @@ _SCOPED_SKIP_LOGGED: set[str] = set() # routed profile homes whose multiplex d
_SECRET_SOURCES: dict[str, str] = {}
# Immutable per-home snapshots: os.environ is shared across profiles and a later home's apply may overwrite it.
_SECRET_SOURCE_VALUES_BY_HOME: dict[str, dict[str, str]] = {}
# Per home: the subset of the snapshot a dotenv reload may re-assert — see ``AppliedVar.authoritative`` (#74265).
_SECRET_SOURCE_RESTORE_BY_HOME: dict[str, dict[str, str]] = {}
# HERMES_HOME paths already pulled external secrets for: load_hermes_dotenv() runs at import time from
# several hot modules, so without this the Bitwarden status line prints 3-5x per startup and the config
# re-parse + ASCII sweep re-run each time (Bitwarden's own cache only saves the network call).
@@ -120,6 +122,7 @@ def _hydrate_profile_secret_sources(home: Path) -> dict[str, str]:
# A retry must not keep serving a partial result after the source is removed, disabled, or can no
# longer be evaluated. Publish only the snapshot established by this attempt.
_SECRET_SOURCE_VALUES_BY_HOME.pop(home_key, None)
_SECRET_SOURCE_RESTORE_BY_HOME.pop(home_key, None)
try:
cfg = _load_secrets_config(home)
@@ -177,10 +180,12 @@ def reset_secret_source_cache(hermes_home: str | os.PathLike | None = None) -> N
_APPLIED_HOMES.clear()
_SECRET_SOURCES.clear()
_SECRET_SOURCE_VALUES_BY_HOME.clear()
_SECRET_SOURCE_RESTORE_BY_HOME.clear()
return
home_key = str(Path(hermes_home).resolve())
_APPLIED_HOMES.discard(home_key)
_SECRET_SOURCE_VALUES_BY_HOME.pop(home_key, None)
_SECRET_SOURCE_RESTORE_BY_HOME.pop(home_key, None)
def format_secret_source_suffix(env_var: str) -> str:
@@ -410,13 +415,6 @@ def load_hermes_dotenv(
project_env_path = Path(project_env) if project_env else None
load_pass = next(_DOTENV_PASSES) # one pass: later layers below see the earlier layers' output
# Snapshot the values an external secret source (Bitwarden, 1Password, ...) already resolved for THIS
# home on an earlier load. load_dotenv(override=True) below would write the raw .env placeholder
# (``__BITWARDEN_MANAGED__``) or a stale token back over them, and _apply_external_secret_sources() is a
# once-per-home no-op (``_APPLIED_HOMES``), so the clobber would stick for the life of the process
# (#74265). Per-home on purpose: a process-global snapshot would leak profile A's secrets into B.
protected_secret_values = get_secret_source_values(home_path)
if user_env.exists(): # normalize formatting / strip NULs before parsing
_sanitize_env_file_if_needed(user_env)
if project_env_path and project_env_path.exists():
@@ -438,11 +436,14 @@ def load_hermes_dotenv(
_load_dotenv_with_fallback(project_env_path, override=not loaded, load_pass=load_pass)
loaded.append(project_env_path)
# Undo the dotenv clobber of external-source secrets before the (no-op on reload) source pass. Managed
# scope, applied last with override=True, still beats a source value on purpose.
for name, value in protected_secret_values.items():
if os.environ.get(name) != value:
os.environ[name] = value
# The override=True loads above wrote the raw .env line (``__BITWARDEN_MANAGED__`` placeholder, stale
# token) back over a value an external source resolved on an earlier call, and the source pass below is a
# once-per-home no-op — so the clobber stuck for the life of the process (#74265). Re-assert only what the
# source is authoritative for; managed scope, applied last with override=True, still wins on purpose.
if _SECRET_SOURCE_RESTORE_BY_HOME:
for name, value in _SECRET_SOURCE_RESTORE_BY_HOME.get(str(home_path.resolve()), {}).items():
if os.environ.get(name) != value:
os.environ[name] = value
# External sources are skipped for the updater (dotenv + managed env still load): ``update`` must not
# import optional secret-manager libs (Bitwarden → cryptography → _rust.pyd) into the process replacing
@@ -573,6 +574,8 @@ def _apply_external_secret_sources(home_path: Path) -> None:
values[name] = os.environ[name]
if values:
_SECRET_SOURCE_VALUES_BY_HOME[home_key] = values
_SECRET_SOURCE_RESTORE_BY_HOME[home_key] = {
n: values[n] for n, a in report.provenance.items() if a.authoritative and n in values}
for src in report.sources:
if src.applied:

33
tests/agent/test_env_loader_reload_restore.py
View File

@@ -26,11 +26,9 @@ def _fresh_state():
from agent.secret_sources import registry as reg_module
reg_module._reset_registry_for_tests()
env_loader._SECRET_SOURCES.clear()
env_loader.reset_secret_source_cache()
yield
reg_module._reset_registry_for_tests()
env_loader._SECRET_SOURCES.clear()
env_loader.reset_secret_source_cache()
@@ -54,10 +52,13 @@ def _register_fake_source(values: dict[str, str], *, override_existing: bool):
reg_module.register_source(_Fake(), replace=True)
def _make_home(tmp_path: Path, monkeypatch, env_text: str) -> Path:
def _make_home(tmp_path: Path, monkeypatch, env_text: str, *, preserve: str = "") -> Path:
home = tmp_path / ".hermes"
home.mkdir()
(home / "config.yaml").write_text("secrets:\n fakebulk:\n enabled: true\n", encoding="utf-8")
secrets = "secrets:\n fakebulk:\n enabled: true\n"
if preserve:
secrets += f" preserve_existing: [{preserve}]\n"
(home / "config.yaml").write_text(secrets, encoding="utf-8")
(home / ".env").write_text(env_text, encoding="utf-8")
monkeypatch.setenv("HERMES_HOME", str(home))
monkeypatch.delenv("GLM_API_KEY", raising=False)
@@ -69,6 +70,7 @@ def test_source_secret_survives_second_load_hermes_dotenv(tmp_path, monkeypatch)
placeholder on load 1; load 2 (gateway import / per-turn reload / cron fire) must keep the
resolved value instead of writing the placeholder back."""
home = _make_home(tmp_path, monkeypatch, "GLM_API_KEY=__BITWARDEN_MANAGED__\n")
# override_existing=True: the source is authoritative over .env, which is what makes the restore legal.
_register_fake_source({"GLM_API_KEY": "vault-value"}, override_existing=True)
env_loader.load_hermes_dotenv(hermes_home=home)
@@ -78,3 +80,26 @@ def test_source_secret_survives_second_load_hermes_dotenv(tmp_path, monkeypatch)
assert os.environ["GLM_API_KEY"] == "vault-value", (
"second load_hermes_dotenv() wrote the .env placeholder back over the source-resolved value"
)
@pytest.mark.parametrize(
("override_existing", "preserve"),
[(False, ""), (True, "GLM_API_KEY")],
ids=["gap-fill-source", "preserve_existing-name"],
)
def test_reload_restore_keeps_dotenv_precedence(tmp_path, monkeypatch, override_existing, preserve):
"""Names .env must win are never restored over a later .env edit: an ``override_existing: false``
source only fills gaps, and a ``secrets.preserve_existing`` name keeps .env's value even against an
overriding source. Both land in the per-home snapshot on load 1 (no .env value yet), so restoring
the whole snapshot froze them at the source value; a cold start would have yielded ``local``."""
home = _make_home(tmp_path, monkeypatch, "", preserve=preserve)
_register_fake_source({"GLM_API_KEY": "vault-value"}, override_existing=override_existing)
env_loader.load_hermes_dotenv(hermes_home=home)
assert os.environ["GLM_API_KEY"] == "vault-value"
(home / ".env").write_text("GLM_API_KEY=local\n", encoding="utf-8")
env_loader.load_hermes_dotenv(hermes_home=home)
assert os.environ["GLM_API_KEY"] == "local", (
"reload restore re-asserted a source value over the user's .env edit"
)