mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(vault): judge a login backend's name without running plugin code
The shared registrar read provider.name before the login-backend registry's static check, so a name property (or a __getattribute__ hook) ran plugin code there and its exception text reached the loader log. The login-backend row now runs agent.vault_backend_registry.static_name first: it inspects the class and instance dicts only, refuses attribute-access overrides and non-plain names, and logs a fixed refusal reason.
This commit is contained in:
1 parent
67ea21b293
commit
ded478f82f
4 files changed
+68
-9
No files matched your search
@@ -3,8 +3,9 @@
|
||||
Registration (``ctx.register_login_backend``) is refused for: a name that is not a lowercase
|
||||
identifier, a handle prefix other than ``<name>:``, a name or prefix in a built-in backend's namespace,
|
||||
a name another plugin already holds in this profile, a backend that wants Hermes' master-password
|
||||
prompt (``needs_unlock``), and ``name``/``prefix`` computed by a descriptor (they could change after
|
||||
validation). ``enabled_plugin_backends`` re-checks every stored key and wraps each backend in
|
||||
prompt (``needs_unlock``), and ``name``/``prefix`` computed by a descriptor or an attribute-access hook
|
||||
(they could change after validation, and reading them would run plugin code; :func:`static_name` judges
|
||||
them before anything reads them). ``enabled_plugin_backends`` re-checks every stored key and wraps each backend in
|
||||
:class:`agent.vault_backends.plugin.PluginLoginBackend`, the only form the vault tool ever sees.
|
||||
"""
|
||||
|
||||
@@ -41,17 +42,30 @@ def _namespace_error(name) -> str:
|
||||
return ""
|
||||
|
||||
|
||||
def register_provider(provider, *, scope=None):
|
||||
if not isinstance(provider, LoginBackend):
|
||||
def static_name(provider) -> str:
|
||||
"""The backend's name, judged without running plugin code (``ValueError`` with a fixed message when refused).
|
||||
|
||||
Registration reads ``provider.name``; a property, a ``__getattribute__`` hook or a ``__class__`` override
|
||||
would run the plugin's code there, outside any guard, and its exception text would reach the loader log.
|
||||
"""
|
||||
cls = type(provider)
|
||||
if not issubclass(cls, LoginBackend):
|
||||
raise ValueError("login backend must subclass LoginBackend")
|
||||
if cls.__getattribute__ is not object.__getattribute__ or any("__class__" in vars(k) for k in cls.__mro__[:-1]):
|
||||
raise ValueError("login backend must not override attribute access")
|
||||
name, prefix = inspect.getattr_static(provider, "name", None), inspect.getattr_static(provider, "prefix", None)
|
||||
if not isinstance(name, str) or not isinstance(prefix, str):
|
||||
if type(name) is not str or type(prefix) is not str:
|
||||
raise ValueError("login backend name and prefix must be plain string attributes")
|
||||
error = _namespace_error(name) or ("" if prefix == f"{name}:" else "login backend prefix must be '<name>:'")
|
||||
if error:
|
||||
raise ValueError(error)
|
||||
if inspect.getattr_static(provider, "needs_unlock", False) is not False:
|
||||
raise ValueError("plugin login backends cannot use the Hermes master-password prompt (needs_unlock)")
|
||||
return name
|
||||
|
||||
|
||||
def register_provider(provider, *, scope=None):
|
||||
name = static_name(provider)
|
||||
if name in _registry.merged(scope):
|
||||
raise ValueError(f"login backend name {name!r} is already registered")
|
||||
_registry.register(provider, scope=scope)
|
||||
|
||||
@@ -6,7 +6,9 @@ the displaced entry. ``hermes_cli.plugins`` generates one ctx method per row. Ro
|
||||
registry module, base-class module:attr, label, docstring, options). ``normalize``: ``strip``
|
||||
(default), ``lower`` (strip+lowercase) or ``None`` (raw name). ``admit``: ``module:function`` taking
|
||||
the plugin's manifest and returning why it may not register this kind ("" = it may), judged under the
|
||||
manager's home before the registry is touched. Object-taking rows also need a
|
||||
manager's home before the registry is touched. ``identity``: a registry function judging the provider's
|
||||
name without running its code, before the shared body reads ``provider.name`` (``ValueError`` = refused,
|
||||
logged with ``reject_message``). Object-taking rows also need a
|
||||
``plugin_isolation.HOST_OBJECT_BASES`` entry so the provider works under ``plugins.isolation: host``.
|
||||
"""
|
||||
|
||||
@@ -28,7 +30,7 @@ _SCOPED_PROVIDER_REGISTRARS: tuple[tuple[str, str, str, str, str, str, dict[str,
|
||||
"Backend instances must read active-profile configuration lazily. Accepted only from bundled "
|
||||
"plugins and catalog installs at tier verified or official.",
|
||||
{"normalize": None, "reject_message": "Plugin '%s' login backend rejected: %s",
|
||||
"admit": "hermes_cli.plugins_cmd_catalog:login_backend_provenance_error"}),
|
||||
"admit": "hermes_cli.plugins_cmd_catalog:login_backend_provenance_error", "identity": "static_name"}),
|
||||
("register_image_gen_provider", "image_gen_provider", "agent.image_gen_registry",
|
||||
"agent.image_gen_provider:ImageGenProvider", "image_gen provider",
|
||||
"Register an :class:`agent.image_gen_provider.ImageGenProvider`; "
|
||||
@@ -95,7 +97,7 @@ def make_scoped_provider_registrar(method_name, kind, registry_mod, base_ref, la
|
||||
"""Build one ``register_<category>_provider`` method from a ``_SCOPED_PROVIDER_REGISTRARS`` row."""
|
||||
normalize_fn = _NAME_NORMALIZERS[options.get("normalize", "strip")]
|
||||
register_name = options.get("register")
|
||||
admit = options.get("admit")
|
||||
admit, identity = options.get("admit"), options.get("identity")
|
||||
|
||||
def register(self, provider):
|
||||
if admit:
|
||||
@@ -105,6 +107,12 @@ def make_scoped_provider_registrar(method_name, kind, registry_mod, base_ref, la
|
||||
logger.warning(options["reject_message"], self.manifest.name, refusal)
|
||||
return None
|
||||
registry = importlib.import_module(registry_mod)
|
||||
if identity:
|
||||
try:
|
||||
getattr(registry, identity)(provider)
|
||||
except ValueError as exc:
|
||||
logger.warning(options["reject_message"], self.manifest.name, exc)
|
||||
return None
|
||||
return self._register_scoped_provider(
|
||||
provider, kind=kind, base_class=_resolve(base_ref),
|
||||
registry=registry, label=label, article=options.get("article", "a"), normalize=normalize_fn,
|
||||
|
||||
@@ -465,3 +465,40 @@ def test_errors_reading_plugin_metadata_get_the_fixed_message(vault_home, page):
|
||||
assert json.loads(listed)["errors"] == [{"backend": "lazy", "error": "The lazy login backend failed."}]
|
||||
assert json.loads(filled)["error_type"] == "backend_unavailable" and page.secret_evals == []
|
||||
|
||||
|
||||
_COMPUTED_NAME_BACKENDS = '''
|
||||
from agent.vault_backends.base import LoginBackend
|
||||
|
||||
class Base(LoginBackend):
|
||||
prefix, display_name = "computed:", "Computed"
|
||||
def list_items(self):
|
||||
return []
|
||||
def get_meta(self, handle):
|
||||
return None
|
||||
def resolve_password(self, handle):
|
||||
return "unused"
|
||||
|
||||
class PropertyName(Base):
|
||||
@property
|
||||
def name(self):
|
||||
raise RuntimeError("manager said {leak}")
|
||||
|
||||
class HookedName(Base):
|
||||
name = "computed"
|
||||
def __getattribute__(self, field):
|
||||
if field == "name":
|
||||
raise RuntimeError("manager said {leak}")
|
||||
return super().__getattribute__(field)
|
||||
'''
|
||||
|
||||
|
||||
def test_registration_judges_the_name_without_running_plugin_code(vault_home, caplog):
|
||||
home, _local, load = vault_home
|
||||
_write_plugin(home, "computed_vault", _COMPUTED_NAME_BACKENDS.format(leak=LEAK),
|
||||
" ctx.register_login_backend(PropertyName())\n ctx.register_login_backend(HookedName())\n")
|
||||
caplog.set_level(logging.DEBUG)
|
||||
load(["computed_vault"], {"computed": "true"})
|
||||
|
||||
assert _backend("computed") is None
|
||||
assert LEAK not in caplog.text
|
||||
assert caplog.text.count("login backend rejected") == 2
|
||||
@@ -83,7 +83,7 @@ hold however the plugin implements its class:
|
||||
| **Own namespace only** | Your handles are `<name>:…`. Hermes decides ownership from the registered name, so it never calls your `owns()` and never passes you a `vault_…` (Hermes vault), `op:…`, `bw:…` or another plugin's handle. Items your `list_items()` returns with an id outside your namespace are dropped, and a `get_meta()` answer whose id is not the requested handle is ignored. |
|
||||
| **Handle in, value out** | `list_items()` gets no arguments; `get_meta()`, `resolve_password()`, `resolve_otp()` and `resolve_secret()` get your handle and nothing else. You never receive the page, the CDP/browser session, the task, other vault entries, or decrypted values from any other backend. |
|
||||
| **No master-password prompt** | Hermes' unlock prompt is reserved for built-in managers. A backend with `needs_unlock = True` is refused at registration, and a plugin backend is never offered the `unlock` action, so the user is never asked to type a master password into a dialog a plugin labelled. Authenticate through your manager's own agent, session or OS keychain. |
|
||||
| **Reserved namespaces** | The names `local`, `onepassword`, `bitwarden`, `vault`, `op` and `bw`, and any prefix inside a built-in prefix (`vault_x:`), are refused. So is a name another plugin already holds in the profile: no backend, built-in or plugin, can be replaced or shadowed. `name` and `prefix` must be plain string attributes, not properties. |
|
||||
| **Reserved namespaces** | The names `local`, `onepassword`, `bitwarden`, `vault`, `op` and `bw`, and any prefix inside a built-in prefix (`vault_x:`), are refused. So is a name another plugin already holds in the profile: no backend, built-in or plugin, can be replaced or shadowed. `name` and `prefix` must be plain string attributes, not properties, and the class may not override `__getattribute__` or `__class__`: Hermes judges the registration without running your code. |
|
||||
| **Off by default** | Only `vault.<name>.enabled: true` turns a backend on. |
|
||||
| **In-process only** | `ctx.register_login_backend` is refused under `plugins.isolation: host`, because `fill_context()` is a server-local context manager. |
|
||||
| **Same fill path** | Plugin items go through the same exact-origin pre-check, in-page origin re-assertion, supervisor CDP secret evaluation (never argv) and payment confirmation as built-in items. |
|
||||
|
||||
Reference in new issue
Block a user