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.
This commit is contained in:
Bartok9
Teknium
parent
299996f7e0
commit
7a7e73d310
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
@ -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.<name>.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:
|
||||
|
|
|
|||
Loading…
Reference in New Issue