fix(secrets): secret-source re-pull no longer latches an empty snapshot or wipes sibling profiles

Symptom (#102041): under a multiplex gateway the default profile's vault/1Password/ Bitwarden/plugin-sourced credentials vanished for the rest of the process after the first cron fire or the post-discovery plugin refresh; with the key already in the process env (systemd EnvironmentFile=) the scope was empty from boot. Every get_secret() read then failed closed ("No usable credentials", every Telegram sender rejected). Why: _apply_external_secret_sources marked the home applied after any real fetch, but only snapshotted names in report.provenance — the NEWLY applied ones. On a re-apply the previous apply's own write-back makes every key `skipped_existing`, so the snapshot latched to {} and _hydrate_profile_secret_sources returned that empty snapshot forever. Separately, reset_secret_source_cache() was process-wide, so one profile's cron re-pull dropped every sibling's hydrated snapshot (1aa62ceb45 isolated the routed reload but not the reset). Change: - env_loader: snapshot every name a source supplied (provenance + skipped_existing) from the home's effective environment, so a shadowed re-apply keeps the values it had. - reset_secret_source_cache(hermes_home=None): optional per-home reset; global clear kept for tests/config edits. - cron per-fire re-pull and plugins._refresh_secret_sources_after_discovery reset + reload only the home they resolve to. - Two invariant tests (red on base) in tests/test_env_loader_secret_sources.py; docs note in secret-source-plugin.md. Reported-by: luochen1990 Addresses #102041
2026-09-10 12:10:12 -07:00
parent 580322ef1e
commit 08830efd96
7 changed files with 122 additions and 25 deletions
--- a/cron/scheduler.py
+++ b/cron/scheduler.py
@@ -2089,7 +2089,7 @@ def _reload_dotenv_and_publish_delivery_target(job: dict) -> None:
    from hermes_cli.env_loader import load_hermes_dotenv, reset_secret_source_cache
    from gateway.session_context import _VAR_MAP

-    reset_secret_source_cache()
+    reset_secret_source_cache(_get_hermes_home())
    load_hermes_dotenv(hermes_home=_get_hermes_home())

    delivery_target = _resolve_delivery_target(job)
--- a/hermes_cli/env_loader.py
+++ b/hermes_cli/env_loader.py
@@ -156,11 +156,21 @@ def _hydrate_profile_secret_sources(home: Path) -> dict[str, str]:
    return dict(values)


-def reset_secret_source_cache() -> None:
-    """Forget applied homes so the next load re-pulls (tests, long-running processes after config edits)."""
-    _APPLIED_HOMES.clear()
-    _SECRET_SOURCES.clear()
-    _SECRET_SOURCE_VALUES_BY_HOME.clear()
+def reset_secret_source_cache(hermes_home: str | os.PathLike | None = None) -> None:
+    """Forget applied homes so the next load re-pulls (tests, long-running processes after config edits).
+
+    ``hermes_home`` limits the reset to ONE home: a multiplex gateway keeps every profile's snapshot in
+    this process, and a per-fire cron re-pull or a plugin-discovery refresh for one home must not wipe
+    a sibling's hydrated snapshot — the sibling's next scope build would run empty until it re-hydrated
+    (#102041)."""
+    if hermes_home is None:
+        _APPLIED_HOMES.clear()
+        _SECRET_SOURCES.clear()
+        _SECRET_SOURCE_VALUES_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)


 def format_secret_source_suffix(env_var: str) -> str:
@@ -476,19 +486,26 @@ def _apply_external_secret_sources(home_path: Path) -> None:
    # Marking AFTER the attempt keeps the earlier failure paths retryable.
    _APPLIED_HOMES.add(home_key)

-    # A real fetch attempt happened (success OR error). Mark the home now so the 3-5 import-time
-    # load_hermes_dotenv() calls per startup don't re-fetch / re-print — error retries within one process
-    # are opt-in via reset_secret_source_cache(). Marking AFTER the attempt (not before, see #40597) is what
-    # lets the earlier failure paths stay retryable.
    if report.applied_any:
        _sanitize_loaded_credentials()  # vault values carry the same copy-paste corruption risk as .env
-        # Re-run the ASCII sanitization pass: vault values are user-supplied and might have the same
-        # copy-paste corruption as a manually edited .env (see #6843).
-        values: dict[str, str] = {}
        for name, applied in report.provenance.items():
            _SECRET_SOURCES[name] = applied.source
-            if name in os.environ:
-                values[name] = os.environ[name]
+
+    # Snapshot EVERY name a source supplied, not just the newly applied ones. A name the source supplied
+    # but the pre-existing process value won (``skipped_existing``) is still this home's effective value
+    # for it — and on the unscoped path os.environ IS this home's environment. Skipping those names
+    # latched an EMPTY snapshot whenever the key was already in the env: after the first cron/plugin
+    # re-pull (the previous apply's own write-back shadows every key) or from boot under systemd
+    # ``EnvironmentFile=``. Under multiplex the scope is the only credential source, so an empty
+    # snapshot failed every default-profile turn for the process lifetime (#102041).
+    values: dict[str, str] = {}
+    supplied = set(report.provenance)
+    for src in report.sources:
+        supplied.update(src.skipped_existing)
+    for name in supplied:
+        if name in os.environ:
+            values[name] = os.environ[name]
+    if values:
        _SECRET_SOURCE_VALUES_BY_HOME[home_key] = values

    for src in report.sources:
--- a/hermes_cli/plugins.py
+++ b/hermes_cli/plugins.py
@@ -1275,8 +1275,13 @@ class PluginManager(PluginLoaderMixin, PluginDispatchMixin, PluginLedgerMixin):
        if not enabled_names:
            return
        try:
-            reset_secret_source_cache()
-            load_hermes_dotenv()
+            # Reset and reload the SAME home the process (or routed turn) resolves to: under multiplex this
+            # runs at gateway boot after sibling profiles may already have hydrated, and a global clear
+            # wiped their snapshots; a routed discovery must rebuild the profile it just dropped.
+            from hermes_constants import get_hermes_home
+            home = get_hermes_home()
+            reset_secret_source_cache(home)
+            load_hermes_dotenv(hermes_home=home)
            logger.debug("Re-applied secret sources after plugin discovery for: %s",
                         ", ".join(sorted(enabled_names)))
        except Exception as exc:
--- a/tests/cron/test_scheduler.py
+++ b/tests/cron/test_scheduler.py
@@ -975,7 +975,7 @@ class TestRunJobSessionPersistence:
        fake_db = MagicMock()
        call_order = []

-        def _record_reset():
+        def _record_reset(*_args):
            call_order.append("reset")

        def _record_load(*args, **kwargs):
--- a/tests/hermes_cli/test_secret_source_bootstrap.py
+++ b/tests/hermes_cli/test_secret_source_bootstrap.py
@@ -42,7 +42,7 @@ def test_refresh_secret_sources_noop_without_plugin_sources(monkeypatch):
    monkeypatch.setattr(reg, "list_plugin_sources", lambda: [])
    monkeypatch.setattr(
        "hermes_cli.env_loader.reset_secret_source_cache",
-        lambda: called.__setitem__("reset", called["reset"] + 1),
+        lambda *a, **kw: called.__setitem__("reset", called["reset"] + 1),
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.load_hermes_dotenv",
@@ -68,7 +68,7 @@ def test_refresh_secret_sources_noop_when_only_builtins(monkeypatch):
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.reset_secret_source_cache",
-        lambda: called.__setitem__("reset", called["reset"] + 1),
+        lambda *a, **kw: called.__setitem__("reset", called["reset"] + 1),
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.load_hermes_dotenv",
@@ -92,7 +92,7 @@ def test_refresh_secret_sources_repulls_when_plugin_enabled(monkeypatch):
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.reset_secret_source_cache",
-        lambda: called.__setitem__("reset", called["reset"] + 1),
+        lambda *a, **kw: called.__setitem__("reset", called["reset"] + 1),
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.load_hermes_dotenv",
@@ -120,7 +120,7 @@ def test_refresh_respects_custom_is_enabled(monkeypatch):
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.reset_secret_source_cache",
-        lambda: called.__setitem__("reset", called["reset"] + 1),
+        lambda *a, **kw: called.__setitem__("reset", called["reset"] + 1),
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.load_hermes_dotenv",
@@ -147,7 +147,7 @@ def test_refresh_skips_custom_source_when_not_activated(monkeypatch):
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.reset_secret_source_cache",
-        lambda: called.__setitem__("reset", called["reset"] + 1),
+        lambda *a, **kw: called.__setitem__("reset", called["reset"] + 1),
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.load_hermes_dotenv",
@@ -175,7 +175,7 @@ def test_refresh_skips_source_whose_is_enabled_raises(monkeypatch):
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.reset_secret_source_cache",
-        lambda: called.__setitem__("reset", called["reset"] + 1),
+        lambda *a, **kw: called.__setitem__("reset", called["reset"] + 1),
    )
    monkeypatch.setattr(
        "hermes_cli.env_loader.load_hermes_dotenv",
--- a/tests/test_env_loader_secret_sources.py
+++ b/tests/test_env_loader_secret_sources.py
@@ -600,3 +600,78 @@ def test_apply_external_secret_sources_bad_ttl_does_not_crash(tmp_path, monkeypa

    # Coerced to the 300s default rather than raising ValueError.
    assert captured["cache_ttl_seconds"] == 300
+
+
+@pytest.fixture
+def _fresh_registry():
+    from agent.secret_sources import registry as reg_module
+
+    reg_module._reset_registry_for_tests()
+    yield
+    reg_module._reset_registry_for_tests()
+
+
+def _register_fake_bulk_source(value_for_home):
+    """One bulk source supplying GLM_API_KEY, resolved per home."""
+    from agent.secret_sources import registry as reg_module
+    from agent.secret_sources.base import FetchResult, SecretSource
+
+    class _Fake(SecretSource):
+        name = "fakebulk"
+        label = "Fake"
+        shape = "bulk"
+
+        def fetch(self, cfg, home_path):
+            result = FetchResult()
+            result.secrets = {"GLM_API_KEY": value_for_home(Path(home_path))}
+            return result
+
+    reg_module.register_source(_Fake(), replace=True)
+
+
+def test_env_shadowed_reapply_keeps_home_snapshot(tmp_path, monkeypatch, _fresh_registry):
+    """#102041: a re-apply whose every key is ``skipped_existing`` (the previous apply's own write-back,
+    or a systemd ``EnvironmentFile=`` value) must still snapshot the home's effective values. Latching
+    an empty snapshot made ``build_profile_secret_scope`` drop every vault credential for the process
+    lifetime under multiplex."""
+    from agent.secret_scope import build_profile_secret_scope
+
+    home = tmp_path / ".hermes"
+    home.mkdir()
+    (home / "config.yaml").write_text("secrets:\n  fakebulk:\n    enabled: true\n", encoding="utf-8")
+    monkeypatch.setenv("HERMES_HOME", str(home))
+    monkeypatch.delenv("GLM_API_KEY", raising=False)
+    _register_fake_bulk_source(lambda _home: "vault-value")
+
+    env_loader.load_hermes_dotenv(hermes_home=home)
+    assert env_loader.get_secret_source_values(home) == {"GLM_API_KEY": "vault-value"}
+
+    # cron per-fire / plugin-discovery re-pull: reset + reload with the key now shadowing itself.
+    env_loader.reset_secret_source_cache()
+    env_loader.load_hermes_dotenv(hermes_home=home)
+
+    assert str(home.resolve()) in env_loader._APPLIED_HOMES
+    assert env_loader.hydrate_profile_secret_sources(home) == {"GLM_API_KEY": "vault-value"}
+    assert build_profile_secret_scope(home)["GLM_API_KEY"] == "vault-value"
+
+
+def test_home_scoped_reset_preserves_sibling_snapshot(tmp_path, monkeypatch, _fresh_registry):
+    """A cron fire / discovery refresh for one profile resets only THAT home: a multiplex sibling's
+    hydrated snapshot stays intact instead of running empty until it re-hydrates."""
+    home = tmp_path / ".hermes"
+    sibling = home / "profiles" / "b"
+    sibling.mkdir(parents=True)
+    for h in (home, sibling):
+        (h / "config.yaml").write_text("secrets:\n  fakebulk:\n    enabled: true\n", encoding="utf-8")
+    monkeypatch.setenv("HERMES_HOME", str(home))
+    monkeypatch.delenv("GLM_API_KEY", raising=False)
+    _register_fake_bulk_source(lambda h: f"vault-{h.name}")
+
+    env_loader.load_hermes_dotenv(hermes_home=home)
+    assert env_loader.hydrate_profile_secret_sources(sibling) == {"GLM_API_KEY": "vault-b"}
+
+    env_loader.reset_secret_source_cache(home)
+
+    assert env_loader.get_secret_source_values(home) == {}
+    assert env_loader.get_secret_source_values(sibling) == {"GLM_API_KEY": "vault-b"}
+    assert str(sibling.resolve()) in env_loader._APPLIED_HOMES
--- a/website/docs/developer-guide/secret-source-plugin.md
+++ b/website/docs/developer-guide/secret-source-plugin.md
@@ -143,7 +143,7 @@ def register(ctx):
 Registration is rejected (with a log warning, never a crash) for: non-`SecretSource` instances, invalid/duplicate names, a `scheme` another source owns, wrong `api_version`, or a `shape` outside `mapped`/`bulk`.

 :::note Timing
-Plugin discovery runs later in startup than the first `load_hermes_dotenv()` call. Immediately after discovery, Hermes re-pulls enabled plugin secret sources (`reset_secret_source_cache()` + `load_hermes_dotenv()`), so the discovering process *does* pick them up — see [First-process bootstrap timing](#first-process-bootstrap-timing) above (#64177). The re-pull is fail-open and skipped when no plugin source is enabled. Any code that reads `os.environ` during the plugin module's import or `register(ctx)` still runs before the re-pull and cannot depend on credentials supplied by that same source; keep credentialed work inside `fetch()`. Gateway, cron, and subagent processes perform the same discovery/re-pull sequence.
+Plugin discovery runs later in startup than the first `load_hermes_dotenv()` call. Immediately after discovery, Hermes re-pulls enabled plugin secret sources (`reset_secret_source_cache()` + `load_hermes_dotenv()`), so the discovering process *does* pick them up — see [First-process bootstrap timing](#first-process-bootstrap-timing) above (#64177). The re-pull is fail-open and skipped when no plugin source is enabled. Any code that reads `os.environ` during the plugin module's import or `register(ctx)` still runs before the re-pull and cannot depend on credentials supplied by that same source; keep credentialed work inside `fetch()`. Gateway, cron, and subagent processes perform the same discovery/re-pull sequence. The re-pull (and the per-fire cron re-pull) resets only the resolving home's cache, so under a multiplex gateway sibling profiles keep their hydrated snapshots; and a re-pull whose keys already sit in the process environment (`skipped_existing`, e.g. the previous apply's own write-back) still records the home's effective values, so `override_existing` is never required just to survive a re-pull.
 :::

 ## Users configure it like any other source