diff --git a/agent/secret_sources/registry.py b/agent/secret_sources/registry.py index e889b29414..d1f536cf4b 100644 --- a/agent/secret_sources/registry.py +++ b/agent/secret_sources/registry.py @@ -47,15 +47,12 @@ from agent.secret_sources.base import ( logger = logging.getLogger(__name__) # Ordered registry: name → source instance. Python dicts preserve -# insertion order, which doubles as the default apply order. +# insertion order, which doubles as the default apply order. Origin is +# recorded beside each source so consumers never infer ownership from names. _SOURCES: Dict[str, SecretSource] = {} +_SOURCE_ORIGINS: Dict[str, str] = {} _BUILTINS_LOADED = False -# Canonical names of the deliberately-closed bundled source set. Callers -# (e.g. the plugin-discovery re-pull) use this to distinguish plugin sources -# from in-tree ones without hard-coding a list at each call site. -BUILTIN_SOURCE_NAMES: frozenset = frozenset({"bitwarden", "onepassword"}) - @dataclass class AppliedVar: @@ -99,7 +96,12 @@ class ApplyReport: # --------------------------------------------------------------------------- -def register_source(source: SecretSource, *, replace: bool = False) -> bool: +def register_source( + source: SecretSource, + *, + replace: bool = False, + builtin: bool = False, +) -> bool: """Register a secret source. Returns True on success. Rejections are logged, never raised — a bad plugin must not take @@ -145,6 +147,7 @@ def register_source(source: SecretSource, *, replace: bool = False) -> bool: ) return False _SOURCES[name] = source + _SOURCE_ORIGINS[name] = "builtin" if builtin else "plugin" return True @@ -158,6 +161,16 @@ def list_sources() -> List[SecretSource]: return list(_SOURCES.values()) +def list_plugin_sources() -> List[SecretSource]: + """Return sources registered outside the bundled bootstrap set.""" + _ensure_builtin_sources() + return [ + source + for name, source in _SOURCES.items() + if _SOURCE_ORIGINS.get(name) == "plugin" + ] + + def _ensure_builtin_sources() -> None: """Idempotently register the bundled sources. @@ -171,21 +184,21 @@ def _ensure_builtin_sources() -> None: try: from agent.secret_sources.bitwarden import BitwardenSource - register_source(BitwardenSource()) + register_source(BitwardenSource(), builtin=True) except Exception: # noqa: BLE001 — never block startup logger.warning("Failed to register bundled Bitwarden secret source", exc_info=True) try: from agent.secret_sources.onepassword import OnePasswordSource - register_source(OnePasswordSource()) + register_source(OnePasswordSource(), builtin=True) except Exception: # noqa: BLE001 — never block startup logger.warning("Failed to register bundled 1Password secret source", exc_info=True) try: from agent.secret_sources.command import CommandSource - register_source(CommandSource()) + register_source(CommandSource(), builtin=True) except Exception: # noqa: BLE001 — never block startup logger.warning("Failed to register bundled command secret source", exc_info=True) @@ -194,6 +207,7 @@ def _ensure_builtin_sources() -> None: def _reset_registry_for_tests() -> None: global _BUILTINS_LOADED _SOURCES.clear() + _SOURCE_ORIGINS.clear() _BUILTINS_LOADED = False diff --git a/hermes_cli/plugins.py b/hermes_cli/plugins.py index 89f2f28a8e..f05013c450 100644 --- a/hermes_cli/plugins.py +++ b/hermes_cli/plugins.py @@ -1390,20 +1390,14 @@ class PluginManager: Fail-open: never raise into discover_and_load. """ try: - from agent.secret_sources.registry import ( - BUILTIN_SOURCE_NAMES, - list_sources, - ) + from agent.secret_sources.registry import list_plugin_sources from hermes_cli.env_loader import load_hermes_dotenv, reset_secret_source_cache except Exception: return try: - sources = list_sources() + plugin_sources = list_plugin_sources() except Exception: return - plugin_sources = [ - s for s in sources if getattr(s, "name", "") not in BUILTIN_SOURCE_NAMES - ] if not plugin_sources: return # Load the secrets config once; hand each source its own section and diff --git a/tests/hermes_cli/test_secret_source_bootstrap.py b/tests/hermes_cli/test_secret_source_bootstrap.py index 78f6758f05..f7f1188a5a 100644 --- a/tests/hermes_cli/test_secret_source_bootstrap.py +++ b/tests/hermes_cli/test_secret_source_bootstrap.py @@ -1,10 +1,9 @@ """Tests for plugin secret-source first-process re-pull (#64177).""" from __future__ import annotations +import os from pathlib import Path -import pytest - from agent.secret_sources.base import ( SECRET_SOURCE_API_VERSION, FetchResult, @@ -40,7 +39,7 @@ def test_refresh_secret_sources_noop_without_plugin_sources(monkeypatch): import agent.secret_sources.registry as reg - monkeypatch.setattr(reg, "list_sources", lambda: []) + monkeypatch.setattr(reg, "list_plugin_sources", lambda: []) monkeypatch.setattr( "hermes_cli.env_loader.reset_secret_source_cache", lambda: called.__setitem__("reset", called["reset"] + 1), @@ -61,9 +60,8 @@ def test_refresh_secret_sources_noop_when_only_builtins(monkeypatch): import agent.secret_sources.registry as reg - monkeypatch.setattr( - reg, "list_sources", lambda: [_StubSource(name="bitwarden")] - ) + reg._reset_registry_for_tests() + assert reg.list_plugin_sources() == [] monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"secrets": {"bitwarden": {"enabled": True}}}, @@ -87,7 +85,7 @@ def test_refresh_secret_sources_repulls_when_plugin_enabled(monkeypatch): import agent.secret_sources.registry as reg - monkeypatch.setattr(reg, "list_sources", lambda: [_StubSource()]) + monkeypatch.setattr(reg, "list_plugin_sources", lambda: [_StubSource()]) monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"secrets": {"myvault": {"enabled": True}}}, @@ -112,7 +110,9 @@ def test_refresh_respects_custom_is_enabled(monkeypatch): import agent.secret_sources.registry as reg - monkeypatch.setattr(reg, "list_sources", lambda: [_CustomActivationSource()]) + monkeypatch.setattr( + reg, "list_plugin_sources", lambda: [_CustomActivationSource()] + ) # No `enabled` key at all — only the source's custom contract decides. monkeypatch.setattr( "hermes_cli.config.load_config", @@ -137,7 +137,9 @@ def test_refresh_skips_custom_source_when_not_activated(monkeypatch): import agent.secret_sources.registry as reg - monkeypatch.setattr(reg, "list_sources", lambda: [_CustomActivationSource()]) + monkeypatch.setattr( + reg, "list_plugin_sources", lambda: [_CustomActivationSource()] + ) # `enabled: true` but the custom contract ignores it and requires vault_id. monkeypatch.setattr( "hermes_cli.config.load_config", @@ -166,7 +168,7 @@ def test_refresh_skips_source_whose_is_enabled_raises(monkeypatch): import agent.secret_sources.registry as reg - monkeypatch.setattr(reg, "list_sources", lambda: [_Boom()]) + monkeypatch.setattr(reg, "list_plugin_sources", lambda: [_Boom()]) monkeypatch.setattr( "hermes_cli.config.load_config", lambda: {"secrets": {"myvault": {"enabled": True}}}, @@ -198,34 +200,57 @@ def test_discover_and_load_invokes_refresh(monkeypatch): def test_real_plugin_source_discovery_applies_dotenv(monkeypatch, tmp_path): - """End-to-end: a real plugin source registered via discovery triggers a - re-pull that flows through the registry's is_enabled contract.""" + """A cold process discovers a real plugin and applies its credential.""" import agent.secret_sources.registry as reg + from hermes_cli import env_loader reg._reset_registry_for_tests() - - # Register a real plugin source the way PluginContext.register_secret_source - # would (lands in registry.register_source). - plugin_source = _StubSource(name="tmpvault", scheme="tmpvault") - assert reg.register_source(plugin_source) is True - assert any(getattr(s, "name", "") == "tmpvault" for s in reg.list_sources()) - - applied = {"reset": 0, "load": 0} - monkeypatch.setattr( - "hermes_cli.env_loader.reset_secret_source_cache", - lambda: applied.__setitem__("reset", applied["reset"] + 1), + env_loader.reset_secret_source_cache() + home = tmp_path / ".hermes" + plugin_dir = home / "plugins" / "fixture-secret-source" + plugin_dir.mkdir(parents=True) + (home / "config.yaml").write_text( + "plugins:\n" + " enabled: [fixture-secret-source]\n" + "secrets:\n" + " fixturevault:\n" + " enabled: true\n", + encoding="utf-8", ) - monkeypatch.setattr( - "hermes_cli.env_loader.load_hermes_dotenv", - lambda **kw: applied.__setitem__("load", applied["load"] + 1), + (plugin_dir / "plugin.yaml").write_text( + "name: fixture-secret-source\nversion: 0.1.0\n", + encoding="utf-8", ) - monkeypatch.setattr( - "hermes_cli.config.load_config", - lambda: {"secrets": {"tmpvault": {"enabled": True}}}, + (plugin_dir / "__init__.py").write_text( + "from pathlib import Path\n" + "from agent.secret_sources.base import (\n" + " SECRET_SOURCE_API_VERSION, FetchResult, SecretSource,\n" + ")\n\n" + "class FixtureVault(SecretSource):\n" + " name = 'fixturevault'\n" + " label = 'Fixture vault'\n" + " api_version = SECRET_SOURCE_API_VERSION\n" + " shape = 'bulk'\n\n" + " def fetch(self, cfg: dict, home_path: Path) -> FetchResult:\n" + " return FetchResult(secrets={\n" + " 'HERMES_TEST_PLUGIN_BOOTSTRAP': 'from-plugin',\n" + " })\n\n" + "def register(ctx):\n" + " ctx.register_secret_source(FixtureVault())\n", + encoding="utf-8", ) - mgr = PluginManager() - mgr._refresh_secret_sources_after_discovery() + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv("HERMES_TEST_PLUGIN_BOOTSTRAP", raising=False) - assert applied == {"reset": 1, "load": 1} - reg._reset_registry_for_tests() + try: + PluginManager().discover_and_load() + + assert os.environ["HERMES_TEST_PLUGIN_BOOTSTRAP"] == "from-plugin" + assert [source.name for source in reg.list_plugin_sources()] == [ + "fixturevault" + ] + finally: + os.environ.pop("HERMES_TEST_PLUGIN_BOOTSTRAP", None) + reg._reset_registry_for_tests() + env_loader.reset_secret_source_cache() diff --git a/website/docs/developer-guide/secret-source-plugin.md b/website/docs/developer-guide/secret-source-plugin.md index 36b0a04130..3c0dd465e2 100644 --- a/website/docs/developer-guide/secret-source-plugin.md +++ b/website/docs/developer-guide/secret-source-plugin.md @@ -16,8 +16,10 @@ The bundled set is deliberately closed, same policy as [memory providers](/devel `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). +plugin secret source is configured. Enablement uses the source's +`is_enabled(cfg)` contract; the standard form is +`secrets..enabled: true`, while custom activation remains supported. +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** @@ -141,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 read `os.environ` *before* discovery completes (i.e. at pure import time) still sees only the initial load; bundled sources remain the safest choice for the earliest bootstrap. Every subsequently spawned Hermes process (gateway children, cron sessions, subagents) also consults plugin sources on its own initial load. +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. ::: ## Users configure it like any other source