fix(plugins): one unreadable plugin dir no longer aborts loader or list discovery
`scan_directory` (the PluginManager sweep every CLI/gateway/Desktop backend runs at startup) and `plugins_cmd._scan_level` (`hermes plugins list`, the dashboard plugins hub, the TUI plugin picker) probed `plugin.yaml` with `Path.exists()` outside any error handling. `stat()` raises instead of returning False when the plugin directory itself is unsearchable — Windows ACLs (WinError 5, the #111804 report) or a POSIX mode-000 folder — so a single bad plugin folder took every other plugin down with it and the Desktop backend exited before announcing its port. Both scans now warn and skip that one directory, matching the dashboard manifest scan fixed in the preceding (salvaged) commit. Part of #111804
This commit is contained in:
@@ -1274,9 +1274,14 @@ def _scan_level(base: Path, source: str, skip_names: set, prefix: str, depth: in
|
||||
if not base.is_dir():
|
||||
return
|
||||
for d in sorted(base.iterdir()):
|
||||
if not d.is_dir() or (depth == 0 and skip_names and d.name in skip_names):
|
||||
try:
|
||||
if not d.is_dir() or (depth == 0 and skip_names and d.name in skip_names):
|
||||
continue
|
||||
info = _read_manifest_info(d, prefix)
|
||||
except OSError as exc:
|
||||
# Mirrors scan_directory: an unsearchable plugin dir (WinError 5 / mode 000) is skipped, not fatal.
|
||||
logger.warning("Skipping unreadable plugin directory %s: %s", d, exc)
|
||||
continue
|
||||
info = _read_manifest_info(d, prefix)
|
||||
if info is None:
|
||||
if depth == 0:
|
||||
_scan_level(d, source, set(), f"{prefix}/{d.name}" if prefix else d.name, 1, seen)
|
||||
|
||||
@@ -110,15 +110,22 @@ def scan_directory(
|
||||
if not path.is_dir():
|
||||
return manifests
|
||||
for child in sorted(path.iterdir()):
|
||||
if not child.is_dir() or (depth == 0 and skip_names and child.name in skip_names):
|
||||
try:
|
||||
if not child.is_dir() or (depth == 0 and skip_names and child.name in skip_names):
|
||||
continue
|
||||
manifest_file = next((f for f in (child / "plugin.yaml", child / "plugin.yml") if f.exists()), None)
|
||||
portable_file = child / "plugin.json"
|
||||
has_portable = portable_file.exists() or portable_file.is_symlink()
|
||||
except OSError as exc:
|
||||
# stat() raises (not "False") on an unsearchable directory — Windows ACLs (WinError 5) or a
|
||||
# mode-000 dir; one such plugin must not abort discovery for every other plugin (#111804).
|
||||
logger.warning("Skipping unreadable plugin directory %s: %s", child, exc)
|
||||
continue
|
||||
manifest_file = next((f for f in (child / "plugin.yaml", child / "plugin.yml") if f.exists()), None)
|
||||
portable_file = child / "plugin.json"
|
||||
if manifest_file is not None:
|
||||
manifest = parse_manifest_file(manifest_file, child, source, prefix)
|
||||
if manifest is not None:
|
||||
manifests.append(manifest)
|
||||
elif portable_file.exists() or portable_file.is_symlink():
|
||||
elif has_portable:
|
||||
try:
|
||||
manifests.append(portable_plugin_manifest(child, source, prefix))
|
||||
except Exception as exc:
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import importlib.metadata
|
||||
import argparse
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import plugins_cmd
|
||||
|
||||
|
||||
@@ -128,3 +132,32 @@ def test_declared_capabilities_for_entrypoint_uses_distribution_metadata(
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name == "nt", reason="chmod is a no-op on Windows")
|
||||
@pytest.mark.skipif(getattr(os, "geteuid", lambda: 1)() == 0, reason="root ignores file permissions")
|
||||
def test_unreadable_plugin_dir_is_skipped_by_every_manifest_scan(monkeypatch, tmp_path, caplog):
|
||||
"""One plugin directory the process cannot stat() into (Windows WinError 5, POSIX mode 000)
|
||||
must be warned about and skipped — not abort discovery for every other plugin (#111804).
|
||||
Covers the loader scan (``scan_directory``) and the list/hub scan (``_scan_level``)."""
|
||||
from hermes_cli.plugins_discovery import scan_directory
|
||||
|
||||
user_dir = tmp_path / "plugins"
|
||||
bundled_dir = tmp_path / "bundled"
|
||||
bundled_dir.mkdir()
|
||||
for name in ("denied", "good"):
|
||||
(user_dir / name).mkdir(parents=True)
|
||||
(user_dir / name / "plugin.yaml").write_text(f"name: {name}\nversion: 1.0.0\n", encoding="utf-8")
|
||||
(user_dir / "denied").chmod(0)
|
||||
monkeypatch.setattr(plugins_cmd, "_plugins_dir", lambda: user_dir)
|
||||
monkeypatch.setattr("hermes_cli.plugins.get_bundled_plugins_dir", lambda: bundled_dir)
|
||||
monkeypatch.setattr(importlib.metadata, "entry_points", lambda: [])
|
||||
|
||||
try:
|
||||
with caplog.at_level(logging.WARNING):
|
||||
loader_names = [m.name for m in scan_directory(user_dir, "user")]
|
||||
listed_names = [entry[0] for entry in plugins_cmd._discover_all_plugins()]
|
||||
finally:
|
||||
(user_dir / "denied").chmod(0o700)
|
||||
|
||||
assert loader_names == ["good"]
|
||||
assert listed_names == ["good"]
|
||||
assert caplog.text.count("Skipping unreadable plugin directory") == 2
|
||||
|
||||
Reference in New Issue
Block a user