From 7a7e73d3106a2a55f451ab0e8c0e5692099f02df Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Tue, 14 Jul 2026 01:47:43 -0400 Subject: [PATCH] fix(plugins): re-pull plugin secret sources after discovery (#64177) After plugins register SecretSource backends, reset the env-loader cache and re-run load_hermes_dotenv when an enabled plugin secret source is configured. Closes the first-process bootstrap gap where import-time env load stale-outs plugin vaults (tommck / Community ask). Fail-open, no-op without plugin sources. Docs: first-process bootstrap timing on secret-source plugin guide. Tests: unit coverage for noop / enabled re-pull / discover hook. Part of #64182 plugin-interface expansion. --- hermes_cli/plugins.py | 69 +++++++++++++-- .../test_secret_source_bootstrap.py | 84 +++++++++++++++++++ .../developer-guide/secret-source-plugin.md | 14 ++++ 3 files changed, 158 insertions(+), 9 deletions(-) create mode 100644 tests/hermes_cli/test_secret_source_bootstrap.py diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 13ea0a32269a8..c2bf971e8731c 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -859,15 +859,12 @@ class PluginContext: ordering, mapped-vs-bulk precedence, conflict warnings, and provenance; the source only fetches. - NOTE ON TIMING: plugin discovery happens later in startup than - the first ``load_hermes_dotenv()`` call, so a plugin-registered - source is not consulted by the initial env load of the process - that discovers it. It IS consulted by every subsequently - spawned Hermes process (gateway children, cron sessions, - subagents), and immediately after a - ``reset_secret_source_cache()`` re-pull. Plugin sources are - therefore best for supplying credentials to the running fleet; - the bundled sources cover first-process bootstrap. + NOTE ON TIMING: ``load_hermes_dotenv()`` usually runs at import + *before* plugin discovery. After discovery completes, the plugin + manager re-pulls enabled plugin secret sources (``reset_secret_source_cache`` + + ``load_hermes_dotenv``) so the first process sees them (#64177). + Child processes that load env after plugins still work without that + re-pull. Failed re-pulls never block startup. Contract requirements (rejected with a warning otherwise): inherit from ``SecretSource``, ``api_version`` matching @@ -1373,10 +1370,64 @@ class PluginManager: self._discovered = True try: self._discover_and_load_inner() + # Plugin secret sources register during discover; the initial + # load_hermes_dotenv() already ran at import time. Re-pull so the + # first process sees plugin backends (tracking #64177). + self._refresh_secret_sources_after_discovery() except BaseException: self._discovered = False raise + def _refresh_secret_sources_after_discovery(self) -> None: + """If any non-bundled secret source is enabled, reset cache and re-apply. + + No-op when only built-in sources exist or no secrets config is enabled. + Fail-open: never raise into discover_and_load. + """ + try: + from agent.secret_sources.registry import list_sources + from hermes_cli.env_loader import load_hermes_dotenv, reset_secret_source_cache + except Exception: + return + try: + sources = list_sources() + except Exception: + return + builtin = {"bitwarden", "onepassword", "1password"} + plugin_names = [ + getattr(s, "name", "") for s in sources if getattr(s, "name", "") not in builtin + ] + if not plugin_names: + return + # Only re-pull when at least one plugin source appears enabled in config. + try: + from hermes_cli.config import load_config + + cfg = load_config() or {} + secrets = cfg.get("secrets") or {} + except Exception: + secrets = {} + enabled_plugin = False + for name in plugin_names: + section = secrets.get(name) + if isinstance(section, dict) and section.get("enabled"): + enabled_plugin = True + break + if section is True: + enabled_plugin = True + break + if not enabled_plugin: + return + try: + reset_secret_source_cache() + load_hermes_dotenv() + logger.debug( + "Re-applied secret sources after plugin discovery for: %s", + ", ".join(sorted(plugin_names)), + ) + except Exception as exc: + logger.debug("secret source re-apply after discovery failed: %s", exc) + def _discover_and_load_inner(self) -> None: """The actual discovery sweep — see :meth:`discover_and_load`.""" manifests: List[PluginManifest] = self._collect_directory_manifests() diff --git a/tests/hermes_cli/test_secret_source_bootstrap.py b/tests/hermes_cli/test_secret_source_bootstrap.py new file mode 100644 index 0000000000000..137f7cab3fc94 --- /dev/null +++ b/tests/hermes_cli/test_secret_source_bootstrap.py @@ -0,0 +1,84 @@ +"""Tests for plugin secret-source first-process re-pull (#64177).""" +from __future__ import annotations + +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from hermes_cli.plugins import PluginManager + + +def test_refresh_secret_sources_noop_without_plugin_sources(monkeypatch): + mgr = PluginManager() + called = {"reset": 0, "load": 0} + + def _list(): + return [] + + monkeypatch.setattr( + "agent.secret_sources.registry.list_sources", _list, raising=False + ) + import agent.secret_sources.registry as reg + + monkeypatch.setattr(reg, "list_sources", _list) + + def _boom_reset(): + called["reset"] += 1 + + def _boom_load(**kwargs): + called["load"] += 1 + + monkeypatch.setattr( + "hermes_cli.env_loader.reset_secret_source_cache", _boom_reset + ) + monkeypatch.setattr("hermes_cli.env_loader.load_hermes_dotenv", _boom_load) + + mgr._refresh_secret_sources_after_discovery() + assert called["reset"] == 0 + assert called["load"] == 0 + + +def test_refresh_secret_sources_repulls_when_plugin_enabled(monkeypatch): + mgr = PluginManager() + called = {"reset": 0, "load": 0} + + class _Src: + name = "myvault" + + import agent.secret_sources.registry as reg + + monkeypatch.setattr(reg, "list_sources", lambda: [_Src()]) + + monkeypatch.setattr( + "hermes_cli.config.load_config", + lambda: {"secrets": {"myvault": {"enabled": True}}}, + ) + + def _reset(): + called["reset"] += 1 + + def _load(**kwargs): + called["load"] += 1 + + monkeypatch.setattr( + "hermes_cli.env_loader.reset_secret_source_cache", _reset + ) + monkeypatch.setattr("hermes_cli.env_loader.load_hermes_dotenv", _load) + + mgr._refresh_secret_sources_after_discovery() + assert called["reset"] == 1 + assert called["load"] == 1 + + +def test_discover_and_load_invokes_refresh(monkeypatch): + mgr = PluginManager() + hits = {"n": 0} + monkeypatch.setattr(PluginManager, "_discover_and_load_inner", lambda self: None) + monkeypatch.setattr( + PluginManager, + "_refresh_secret_sources_after_discovery", + lambda self: hits.__setitem__("n", hits["n"] + 1), + ) + mgr.discover_and_load() + assert hits["n"] == 1 diff --git a/website/docs/developer-guide/secret-source-plugin.md b/website/docs/developer-guide/secret-source-plugin.md index 5df9f803a6923..21605c5a3df31 100644 --- a/website/docs/developer-guide/secret-source-plugin.md +++ b/website/docs/developer-guide/secret-source-plugin.md @@ -12,6 +12,20 @@ Secret sources resolve provider credentials from an external secret manager (a v The bundled set is deliberately closed, same policy as [memory providers](/developer-guide/memory-provider-plugin): PRs adding new vault backends under `agent/secret_sources/` are closed with a pointer to this guide. Publish your backend as a standalone plugin repo and share it in the Nous Research Discord (`#plugins-skills-and-skins`). ::: +## First-process bootstrap timing + +`load_hermes_dotenv()` often runs at import time **before** plugins register. +Hermes then re-pulls secrets after plugin discovery when any **enabled** +plugin secret source is configured (`secrets..enabled: true`). That +closes the "replace Bitwarden with my vault" first-process gap (#64177). + +- Re-pull is idempotent and fail-open (never blocks startup). +- Sources only supply env vars through the orchestrator; there is **no** + plugin API to dump other plugins' or the user's entire secret store beyond + what your source's own config allows. +- Reading `os.environ` after load is possible for any in-process code — the + trust boundary remains "enabled plugins run with agent privilege". + ## What the framework owns vs. what you own The orchestrator (`agent.secret_sources.registry.apply_all`) owns everything security- and precedence-sensitive, so a backend cannot get it wrong: