diff --git a/providers/__init__.py b/providers/__init__.py index b34bc4f61c..011e84afa7 100644 --- a/providers/__init__.py +++ b/providers/__init__.py @@ -160,6 +160,18 @@ def _discover_entry_point_providers() -> None: (``module`` — imported for its module-level ``register_provider`` side effect, mirroring the directory-plugin ``__init__.py`` contract). + Gating and safety: + + * **Opt-in.** Entry-point plugins are subject to the same + ``plugins.enabled`` allow-list (and ``plugins.disabled`` deny-list) the + general PluginManager enforces — a pip package is never imported just + because it is installed. An entry point whose name is not enabled is + skipped without loading. + * **Provider targets only.** The ``hermes_agent.plugins`` group is shared + with general plugins whose target is ``register(ctx)``. Callables that + require arguments are skipped here (the PluginManager owns them); + provider registration hooks take no arguments by contract. + Failures are swallowed per-entry (a broken third-party package must not break provider discovery) and logged at warning level. This scan runs first, so filesystem plugins (bundled + ``$HERMES_HOME``) keep their @@ -172,6 +184,18 @@ def _discover_entry_point_providers() -> None: except Exception: # pragma: no cover — importlib.metadata always present ≥3.8 return + # Same opt-in gate as the general PluginManager: only entry points named + # in ``plugins.enabled`` load, and ``plugins.disabled`` always wins. + try: + from hermes_cli.plugins import _get_disabled_plugins, _get_enabled_plugins + + enabled = _get_enabled_plugins() # None = nothing enabled yet (opt-in default) + disabled = _get_disabled_plugins() + except Exception: # pragma: no cover — config layer unavailable + enabled, disabled = None, set() + if not enabled: + return + group = "hermes_agent.plugins" try: eps = _md.entry_points() @@ -185,6 +209,11 @@ def _discover_entry_point_providers() -> None: return for ep in group_eps: + if ep.name not in enabled or ep.name in disabled: + logger.debug( + "entry-point provider %r skipped: not enabled in config", ep.name + ) + continue try: loaded = ep.load() except Exception as exc: @@ -193,8 +222,18 @@ def _discover_entry_point_providers() -> None: ) continue # ``module:func`` → callable we invoke; bare ``module`` → import side - # effect already happened during load(). Only call when it's callable. + # effect already happened during load(). Only call when it's callable + # AND zero-arg: general plugins in this shared group expose + # ``register(ctx)`` (requires an argument) and belong to the + # PluginManager, not the provider registry. if callable(loaded): + if _requires_arguments(loaded): + logger.debug( + "entry-point %r skipped by provider scan: target requires " + "arguments (general plugin owned by PluginManager)", + ep.name, + ) + continue try: loaded() except Exception as exc: @@ -205,6 +244,30 @@ def _discover_entry_point_providers() -> None: ) +def _requires_arguments(fn) -> bool: + """True when ``fn`` cannot be called with zero arguments. + + Used to distinguish provider registration hooks (zero-arg by contract) + from general plugin hooks (``register(ctx)``) sharing the same entry-point + group. Unintrospectable callables (C extensions) are treated as zero-arg + and left to the per-entry exception guard. + """ + import inspect + + try: + sig = inspect.signature(fn) + except (TypeError, ValueError): # pragma: no cover — builtins/C callables + return False + for param in sig.parameters.values(): + if param.kind in ( + inspect.Parameter.POSITIONAL_ONLY, + inspect.Parameter.POSITIONAL_OR_KEYWORD, + inspect.Parameter.KEYWORD_ONLY, + ) and param.default is inspect.Parameter.empty: + return True + return False + + def _discover_providers() -> None: """Populate the registry by importing every provider plugin. diff --git a/tests/providers/test_entry_point_discovery.py b/tests/providers/test_entry_point_discovery.py index b82e748cfa..86965f47fb 100644 --- a/tests/providers/test_entry_point_discovery.py +++ b/tests/providers/test_entry_point_discovery.py @@ -60,6 +60,19 @@ class _FakeEP: return self._loader() +def _enable(monkeypatch, *names, disabled=()): + """Gate helper: mark entry-point names enabled/disabled in config. + + ``_discover_entry_point_providers`` enforces the PluginManager's + ``plugins.enabled`` opt-in allow-list, so tests must enable their fake + entry points explicitly. + """ + import hermes_cli.plugins as hp + + monkeypatch.setattr(hp, "_get_enabled_plugins", lambda: set(names)) + monkeypatch.setattr(hp, "_get_disabled_plugins", lambda: set(disabled)) + + class _FakeEntryPoints: def __init__(self, eps): self._eps = eps @@ -100,6 +113,7 @@ def test_entry_point_callable_and_module_targets(monkeypatch): import importlib.metadata as md monkeypatch.setattr(md, "entry_points", lambda: fake_eps) + _enable(monkeypatch, "ep-callable", "ep-module") _clear_provider_caches() try: assert providers.get_provider_profile("ep-callable") is not None @@ -109,6 +123,57 @@ def test_entry_point_callable_and_module_targets(monkeypatch): _clear_provider_caches() +def test_entry_point_not_enabled_is_skipped(monkeypatch): + """Entry points honor the plugins.enabled opt-in gate — installed ≠ loaded.""" + fake_eps = _FakeEntryPoints([_FakeEP("ep-callable", _register_via_callable)]) + import importlib.metadata as md + + monkeypatch.setattr(md, "entry_points", lambda: fake_eps) + _enable(monkeypatch, "some-other-plugin") # ep-callable NOT enabled + _clear_provider_caches() + try: + assert providers.get_provider_profile("ep-callable") is None + finally: + _clear_provider_caches() + + +def test_entry_point_disabled_wins_over_enabled(monkeypatch): + """plugins.disabled is a deny-list that beats plugins.enabled.""" + fake_eps = _FakeEntryPoints([_FakeEP("ep-callable", _register_via_callable)]) + import importlib.metadata as md + + monkeypatch.setattr(md, "entry_points", lambda: fake_eps) + _enable(monkeypatch, "ep-callable", disabled=("ep-callable",)) + _clear_provider_caches() + try: + assert providers.get_provider_profile("ep-callable") is None + finally: + _clear_provider_caches() + + +def test_general_plugin_register_ctx_not_invoked(monkeypatch): + """A register(ctx)-style general plugin sharing the group is never called.""" + calls = [] + + def _general_plugin_target(): + def register(ctx): # requires an argument — PluginManager contract + calls.append(ctx) + + return register + + fake_eps = _FakeEntryPoints([_FakeEP("general-plugin", _general_plugin_target)]) + import importlib.metadata as md + + monkeypatch.setattr(md, "entry_points", lambda: fake_eps) + _enable(monkeypatch, "general-plugin") + _clear_provider_caches() + try: + providers._discover_providers() + assert calls == [] # never invoked (would have been a TypeError anyway) + finally: + _clear_provider_caches() + + def test_entry_point_failure_is_isolated(monkeypatch): def _boom(): raise RuntimeError("broken plugin") @@ -122,6 +187,7 @@ def test_entry_point_failure_is_isolated(monkeypatch): import importlib.metadata as md monkeypatch.setattr(md, "entry_points", lambda: fake_eps) + _enable(monkeypatch, "broken", "ep-callable") _clear_provider_caches() try: # A broken entry point must not prevent the good one from registering. @@ -131,7 +197,9 @@ def test_entry_point_failure_is_isolated(monkeypatch): def test_filesystem_plugins_win_over_entry_points(monkeypatch): - """Entry points scan last, so a bundled/user profile of the same name wins.""" + """Entry points are discovered FIRST (lowest precedence): last-writer-wins + in register_provider() means a bundled/user profile of the same name + overrides a pip impostor.""" from providers.base import ProviderProfile def _register_ep_openrouter(): @@ -146,6 +214,7 @@ def test_filesystem_plugins_win_over_entry_points(monkeypatch): import importlib.metadata as md monkeypatch.setattr(md, "entry_points", lambda: fake_eps) + _enable(monkeypatch, "openrouter") # enabled, so precedence is what's tested _clear_provider_caches() try: p = providers.get_provider_profile("openrouter") diff --git a/website/docs/developer-guide/model-provider-plugin.md b/website/docs/developer-guide/model-provider-plugin.md index ea48f80b66..5127107fa3 100644 --- a/website/docs/developer-guide/model-provider-plugin.md +++ b/website/docs/developer-guide/model-provider-plugin.md @@ -264,16 +264,33 @@ The target may be either: `register_provider(...)` side effect, mirroring the directory-plugin `__init__.py` contract. -`providers/__init__.py` discovers these entry points itself (the general -`PluginManager` records model-provider manifests but never imports them, so it -cannot register the profile). Entry-point plugins are discovered **before** -filesystem plugins, giving them the lowest precedence: because -`register_provider()` is last-writer-wins, a bundled or `$HERMES_HOME` profile -of the same name always overrides a pip-installed one. A pip package can add a -genuinely new provider, but cannot silently hijack a first-party provider name. +`providers/__init__.py` discovers these entry points itself — the general +`PluginManager` never invokes provider registration for pip packages (its +entry-point path targets `register(ctx)`-style general plugins, gated by +`plugins.enabled`), so the provider registry does its own scan. Two rules +apply: -A broken entry point is isolated — it is logged at warning level and skipped, -and never blocks discovery of the other providers. +- **Opt-in required.** The same `plugins.enabled` allow-list (and + `plugins.disabled` deny-list) from `config.yaml` governs this scan. A pip + package is never imported just because it is installed — users must add the + entry-point name to `plugins.enabled`: + + ```yaml + plugins: + enabled: + - acme-inference + ``` + +- **Lowest precedence.** Entry-point plugins are discovered **before** + filesystem plugins: because `register_provider()` is last-writer-wins, a + bundled or `$HERMES_HOME` profile of the same name always overrides a + pip-installed one. A pip package can add a genuinely new provider, but + cannot silently hijack a first-party provider name. + +Targets that require arguments (a general plugin's `register(ctx)`) are +skipped by the provider scan — they belong to the `PluginManager`. A broken +entry point is isolated — it is logged at warning level and skipped, and never +blocks discovery of the other providers. See [Building a Hermes Plugin](/developer-guide/plugins#distribute-via-pip) for the full entry-points setup.