diff --git a/hermes_cli/plugins_cmd.py b/hermes_cli/plugins_cmd.py index db426668097..1388e56ce23 100644 --- a/hermes_cli/plugins_cmd.py +++ b/hermes_cli/plugins_cmd.py @@ -76,22 +76,42 @@ def _plugins_dir() -> Path: return plugins -def _sanitize_plugin_name(name: str, plugins_dir: Path) -> Path: +def _sanitize_plugin_name( + name: str, + plugins_dir: Path, + *, + allow_subdir: bool = False, +) -> Path: """Validate a plugin name and return the safe target path inside *plugins_dir*. Raises ``ValueError`` if the name contains path-traversal sequences or would resolve outside the plugins directory. + + ``allow_subdir=True`` permits a single forward slash inside *name* so + category-namespaced plugin keys like ``observability/langfuse`` or + ``image_gen/openai`` (the registry keys emitted by ``_discover_all_plugins``) + can be looked up. ``..`` and backslash are still rejected, leading and + trailing slashes are stripped, and the resolved target must still live + inside *plugins_dir*. Install paths leave this at the default ``False`` + because a freshly-cloned plugin always lands top-level under + ``~/.hermes/plugins//``. """ if not name: raise ValueError("Plugin name must not be empty.") + if allow_subdir: + name = name.strip("/") + if not name: + raise ValueError("Plugin name must not be empty.") + if name in {".", ".."}: raise ValueError( f"Invalid plugin name '{name}': must not reference the plugins directory itself." ) # Reject obvious traversal characters - for bad in ("/", "\\", ".."): + bad_chars = ("\\", "..") if allow_subdir else ("/", "\\", "..") + for bad in bad_chars: if bad in name: raise ValueError(f"Invalid plugin name '{name}': must not contain '{bad}'.") @@ -326,7 +346,7 @@ def _display_removed(name: str, plugins_dir: Path) -> None: def _require_installed_plugin(name: str, plugins_dir: Path, console) -> Path: """Return the plugin path if it exists, or exit with an error listing installed plugins.""" - target = _sanitize_plugin_name(name, plugins_dir) + target = _sanitize_plugin_name(name, plugins_dir, allow_subdir=True) if not target.exists(): installed = ", ".join(d.name for d in plugins_dir.iterdir() if d.is_dir()) or "(none)" console.print( @@ -1508,7 +1528,7 @@ def _user_installed_plugin_dir(name: str) -> Optional[Path]: """Resolved path under ``~/.hermes/plugins/`` if it exists.""" plugins_dir = _plugins_dir() try: - target = _sanitize_plugin_name(name, plugins_dir) + target = _sanitize_plugin_name(name, plugins_dir, allow_subdir=True) except ValueError: return None return target if target.is_dir() else None diff --git a/tests/hermes_cli/test_plugins_cmd.py b/tests/hermes_cli/test_plugins_cmd.py index 5a421f018f9..8184c373b77 100644 --- a/tests/hermes_cli/test_plugins_cmd.py +++ b/tests/hermes_cli/test_plugins_cmd.py @@ -65,6 +65,36 @@ class TestSanitizePluginName: with pytest.raises(ValueError, match="must not be empty"): _sanitize_plugin_name("", tmp_path) + # ── allow_subdir=True ── + + def test_allow_subdir_accepts_single_slash(self, tmp_path): + target = _sanitize_plugin_name( + "observability/langfuse", tmp_path, allow_subdir=True + ) + assert target == (tmp_path / "observability" / "langfuse").resolve() + + def test_allow_subdir_strips_leading_trailing_slash(self, tmp_path): + target = _sanitize_plugin_name( + "/image_gen/openai/", tmp_path, allow_subdir=True + ) + assert target == (tmp_path / "image_gen" / "openai").resolve() + + def test_allow_subdir_still_rejects_dot_dot(self, tmp_path): + with pytest.raises(ValueError, match="must not contain"): + _sanitize_plugin_name("foo/../bar", tmp_path, allow_subdir=True) + + def test_allow_subdir_still_rejects_backslash(self, tmp_path): + with pytest.raises(ValueError, match="must not contain"): + _sanitize_plugin_name("foo\\bar", tmp_path, allow_subdir=True) + + def test_allow_subdir_rejects_empty_after_strip(self, tmp_path): + with pytest.raises(ValueError, match="must not be empty"): + _sanitize_plugin_name("///", tmp_path, allow_subdir=True) + + def test_allow_subdir_resolves_inside_plugins_dir(self, tmp_path): + target = _sanitize_plugin_name("a/b/c", tmp_path, allow_subdir=True) + assert target.is_relative_to(tmp_path.resolve()) + # ── _resolve_git_url ──────────────────────────────────────────────────────