From 9c416e20abf31632d768682deac35e49f4214a10 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 26 Apr 2026 20:57:10 -0700 Subject: [PATCH] feat(skills): install skills from a direct HTTP(S) URL (#16323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(skills): install skills from a direct HTTP(S) URL Adds UrlSource adapter so `hermes skills install ` and `/skills install ` work as first-class operations — no more improvising with curl + patch + cp. - Claims identifiers that start with http(s):// and end in .md - Skips /.well-known/skills/ URLs (WellKnownSkillSource handles those) - Skill name from YAML frontmatter, URL-slug fallback - Single-file SKILL.md only (v1 scope — multi-file skills need a manifest) - Trust level 'community'; full security scan still runs - Lock file stores the URL as identifier so `hermes skills update` re-fetches from the same URL cleanly Scope matches real user need from @versun's docx feedback where `https://sharethis.chat/SKILL.md` had no first-class install path. * feat(skills): interactive name/category for URL installs + --name override Follow-up to the UrlSource adapter. The previous commit fell back to weak heuristics when frontmatter had no ``name:`` and could produce garbage names like ``SKILL`` or ``unnamed-skill``. Now: tools/skills_hub.py - ``UrlSource._is_valid_skill_name()`` — strict identifier check (``^[a-z][a-z0-9_-]*$``), rejects sentinel values (``SKILL``, ``README``, ``INDEX``, ``unnamed-skill``, empty, non-strings). - ``_resolve_skill_name()`` returns ``Optional[str]`` — ``None`` when nothing valid is resolvable. Also ignores unsafe frontmatter names (``../evil``) and falls through to URL slug instead of returning None immediately, so a URL with a bad frontmatter but a good path still works. - ``fetch()``/``inspect()`` carry an ``awaiting_name=True`` marker in metadata/extra when resolution fails, letting ``do_install`` decide whether to prompt, apply an override, or error out. hermes_cli/skills_hub.py - ``do_install`` gains a ``name_override`` parameter. - On URL-sourced bundles with ``awaiting_name=True``: 1. If ``name_override`` is valid → use it. 2. If ``name_override`` is invalid → refuse with a clear error. 3. Else if ``skip_confirm=True`` (non-interactive: slash / TUI / gateway / scripts) → refuse with an actionable retry hint pointing at ``--name `` on both CLI and slash forms. 4. Else (interactive TTY) → prompt for the name. - Interactive TTY also prompts for a category when none is given for a URL-sourced install, hinting existing category buckets so users can reuse ``productivity``, ``devops``, etc. Empty input → flat install. - ``_existing_categories()`` scans ``~/.hermes/skills/`` for subdirs that look like category buckets (contain nested SKILL.md files); skips top-level skills and hidden dirs. - ``_prompt_for_skill_name()`` / ``_prompt_for_category()`` helpers (EOF/Ctrl-C-safe, match the existing ``Confirm [y/N]`` prompt style). hermes_cli/main.py - ``hermes skills install`` argparse gains ``--name ``. hermes_cli/skills_hub.py (slash) - ``/skills install --name `` parsing added. Tests - tests/tools/test_skills_hub.py: updated ``UrlSource`` tests to assert the new ``awaiting_name`` metadata; added 4 new tests for ``_is_valid_skill_name`` rejection sets and the awaiting-name marker. - tests/hermes_cli/test_skills_hub.py: 8 new tests covering --name override accept/reject, non-interactive error, interactive name prompt, interactive category prompt, cancel-aborts-install, and ``_existing_categories`` scan behavior (buckets vs flat skills). - E2E verified all four paths (no-name/no-override → error; --name override → install; frontmatter name → install; invalid --name → rejection). --------- Co-authored-by: teknium1 --- hermes_cli/main.py | 8 +- hermes_cli/skills_hub.py | 176 +++++++++++++++++++++- tests/hermes_cli/test_skills_hub.py | 208 ++++++++++++++++++++++++++ tests/tools/test_skills_hub.py | 217 ++++++++++++++++++++++++++++ tools/skills_hub.py | 171 ++++++++++++++++++++++ 5 files changed, 773 insertions(+), 7 deletions(-) diff --git a/hermes_cli/main.py b/hermes_cli/main.py index efc41e579..80672d394 100644 --- a/hermes_cli/main.py +++ b/hermes_cli/main.py @@ -8681,11 +8681,17 @@ Examples: skills_install = skills_subparsers.add_parser("install", help="Install a skill") skills_install.add_argument( - "identifier", help="Skill identifier (e.g. openai/skills/skill-creator)" + "identifier", + help="Skill identifier (e.g. openai/skills/skill-creator) or a direct HTTP(S) URL to a SKILL.md file", ) skills_install.add_argument( "--category", default="", help="Category folder to install into" ) + skills_install.add_argument( + "--name", + default="", + help="Override the skill name (useful when installing from a URL whose SKILL.md has no `name:` frontmatter)", + ) skills_install.add_argument( "--force", action="store_true", help="Install despite blocked scan verdict" ) diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index 2e425eee8..88c0978a9 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -11,9 +11,10 @@ handler are thin wrappers that parse args and delegate. """ import json +import re import shutil from pathlib import Path -from typing import Any, Dict, Optional +from typing import Any, Dict, List, Optional from rich.console import Console from rich.panel import Panel @@ -141,6 +142,103 @@ def _derive_category_from_install_path(install_path: str) -> str: return "" if parent == "." else parent +# --------------------------------------------------------------------------- +# Interactive name/category resolution for URL-installed skills +# --------------------------------------------------------------------------- + +_VALID_NAME_RE = re.compile(r"^[a-z][a-z0-9_-]*$") +_VALID_CATEGORY_RE = re.compile(r"^[a-z][a-z0-9_/-]*$") + + +def _is_valid_installed_skill_name(name: str) -> bool: + """Accept identifier-shaped names, reject empty / sentinel-y values.""" + if not isinstance(name, str): + return False + candidate = name.strip().lower() + if not candidate or candidate in {"skill", "readme", "index", "unnamed-skill"}: + return False + return bool(_VALID_NAME_RE.match(candidate)) + + +def _existing_categories() -> List[str]: + """Return sorted subdirectory names under ``~/.hermes/skills/`` that look + like category buckets (contain at least one ``SKILL.md`` somewhere below). + + Used to suggest reusable categories when interactively installing from a + URL. Hidden dirs (``.hub``, ``.trash``) are skipped. + """ + from tools.skills_hub import SKILLS_DIR + out: List[str] = [] + try: + for entry in SKILLS_DIR.iterdir(): + if not entry.is_dir() or entry.name.startswith("."): + continue + # Only count as a category if it contains skills, not if it IS a skill. + # Heuristic: if ``/SKILL.md`` exists, it's a skill at the + # top level (no category); otherwise treat as a category bucket. + if (entry / "SKILL.md").exists(): + continue + # Has at least one nested SKILL.md? + try: + if any(entry.rglob("SKILL.md")): + out.append(entry.name) + except OSError: + continue + except (FileNotFoundError, OSError): + return [] + return sorted(set(out)) + + +def _prompt_for_skill_name(c: Console, url: str, default: str = "") -> Optional[str]: + """Prompt interactively for a skill name. Returns None on cancel/EOF.""" + c.print() + c.print( + f"[yellow]The SKILL.md at {url} doesn't declare a `name:` in its " + f"frontmatter,[/]\n[yellow]and the URL path doesn't produce a valid " + f"identifier either.[/]" + ) + default_hint = f" [{default}]" if default else "" + c.print( + f"[bold]Enter a skill name{default_hint}:[/] " + f"[dim](lowercase letters, digits, hyphens, underscores; starts with a letter)[/]" + ) + try: + answer = input("Name: ").strip() + except (EOFError, KeyboardInterrupt): + return None + if not answer and default: + answer = default + if not _is_valid_installed_skill_name(answer): + c.print(f"[bold red]Invalid name:[/] {answer!r}. Aborting install.\n") + return None + return answer + + +def _prompt_for_category(c: Console, existing: List[str]) -> str: + """Prompt interactively for a category. Empty/None input means flat install.""" + c.print() + if existing: + c.print( + "[bold]Pick a category[/] " + "[dim](reuse an existing bucket, type a new one, or press Enter to install flat)[/]" + ) + c.print(f"[dim]Existing: {', '.join(existing)}[/]") + else: + c.print( + "[bold]Category[/] [dim](optional — press Enter to install flat at ~/.hermes/skills//)[/]" + ) + try: + answer = input("Category: ").strip() + except (EOFError, KeyboardInterrupt): + return "" + if not answer: + return "" + if not _VALID_CATEGORY_RE.match(answer): + c.print(f"[dim]Invalid category {answer!r} — installing flat.[/]") + return "" + return answer + + def do_search(query: str, source: str = "all", limit: int = 10, console: Optional[Console] = None) -> None: """Search registries and display results as a Rich table.""" @@ -309,8 +407,17 @@ def do_browse(page: int = 1, page_size: int = 20, source: str = "all", def do_install(identifier: str, category: str = "", force: bool = False, console: Optional[Console] = None, skip_confirm: bool = False, - invalidate_cache: bool = True) -> None: - """Fetch, quarantine, scan, confirm, and install a skill.""" + invalidate_cache: bool = True, + name_override: str = "") -> None: + """Fetch, quarantine, scan, confirm, and install a skill. + + ``name_override`` lets non-interactive callers (slash commands, gateway, + scripts) supply a skill name when the upstream SKILL.md lacks a valid + ``name:`` frontmatter field. On interactive TTY surfaces, a missing name + triggers a prompt instead; ``skip_confirm=True`` means "non-interactive" + (so pair it with ``name_override`` when installing from a URL that has + no frontmatter). + """ from tools.skills_hub import ( GitHubAuth, create_source_router, ensure_hub_dirs, quarantine_bundle, install_from_quarantine, HubLockFile, @@ -354,6 +461,58 @@ def do_install(identifier: str, category: str = "", force: bool = False, c.print() return + # URL-sourced skills may arrive with an empty name when SKILL.md has no + # ``name:`` in frontmatter AND the URL path doesn't yield a valid + # identifier. Resolve by (1) --name override, (2) interactive prompt on + # a TTY, (3) refuse with an actionable error on non-interactive surfaces. + bundle_meta = getattr(bundle, "metadata", {}) or {} + if bundle.source == "url" and (not bundle.name or bundle_meta.get("awaiting_name")): + if name_override and _is_valid_installed_skill_name(name_override): + bundle.name = name_override.strip() + bundle_meta["awaiting_name"] = False + elif name_override: + c.print( + f"[bold red]Invalid --name:[/] {name_override!r}. " + "Must be a lowercase identifier (letters, digits, hyphens, " + "underscores; starts with a letter).\n" + ) + return + elif skip_confirm: + # Non-interactive surface (slash command / TUI / gateway). Can't + # prompt — emit an actionable error. + url = bundle_meta.get("url") or identifier + c.print( + f"[bold red]Cannot install from URL:[/] {url}\n" + "[yellow]The SKILL.md has no `name:` in its frontmatter, " + "and the URL path doesn't produce a valid identifier.[/]\n\n" + "Retry with an explicit name:\n" + f" [bold]/skills install {url} --name [/]\n" + f" [bold]hermes skills install {url} --name [/]\n\n" + "[dim]Or ask the SKILL.md's author to add a `name:` field to " + "its YAML frontmatter.[/]\n" + ) + return + else: + # Interactive TTY — prompt. + url = bundle_meta.get("url") or identifier + chosen = _prompt_for_skill_name(c, url) + if not chosen: + c.print("[dim]Installation cancelled.[/]\n") + return + bundle.name = chosen + bundle_meta["awaiting_name"] = False + # Keep SkillMeta in sync so downstream "already installed" checks, + # audit logs, and display all see the final name. + if meta is not None: + meta.name = bundle.name + meta.path = bundle.name + + # URL-sourced skills: offer to pick a category interactively when the + # caller didn't specify one (TTY only — non-interactive installs fall + # through to flat install, matching all other sources). + if bundle.source == "url" and not category and not skip_confirm: + category = _prompt_for_category(c, _existing_categories()) + # Auto-detect category for official skills (e.g. "official/autonomous-ai-agents/blackbox") if bundle.source == "official" and not category: id_parts = bundle.identifier.split("/") # ["official", "category", "skill"] @@ -1164,7 +1323,8 @@ def skills_command(args) -> None: do_search(args.query, source=args.source, limit=args.limit) elif action == "install": do_install(args.identifier, category=args.category, force=args.force, - skip_confirm=getattr(args, "yes", False)) + skip_confirm=getattr(args, "yes", False), + name_override=getattr(args, "name", "") or "") elif action == "inspect": do_inspect(args.identifier) elif action == "list": @@ -1221,6 +1381,7 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: /skills search kubernetes /skills install openai/skills/skill-creator /skills install openai/skills/skill-creator --force + /skills install https://example.com/path/SKILL.md /skills inspect openai/skills/skill-creator /skills list /skills list --source hub @@ -1297,10 +1458,11 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: elif action == "install": if not args: - c.print("[bold red]Usage:[/] /skills install [--category ] [--force] [--now]\n") + c.print("[bold red]Usage:[/] /skills install [--name ] [--category ] [--force] [--now]\n") return identifier = args[0] category = "" + name_override = "" # Slash commands run inside prompt_toolkit where input() hangs. # Always skip confirmation — the user typing the command is implicit consent. skip_confirm = True @@ -1311,9 +1473,11 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: for i, a in enumerate(args): if a == "--category" and i + 1 < len(args): category = args[i + 1] + elif a == "--name" and i + 1 < len(args): + name_override = args[i + 1] do_install(identifier, category=category, force=force, skip_confirm=skip_confirm, invalidate_cache=invalidate_cache, - console=c) + name_override=name_override, console=c) elif action == "inspect": if not args: diff --git a/tests/hermes_cli/test_skills_hub.py b/tests/hermes_cli/test_skills_hub.py index 386673092..fa611e1a5 100644 --- a/tests/hermes_cli/test_skills_hub.py +++ b/tests/hermes_cli/test_skills_hub.py @@ -316,3 +316,211 @@ def test_do_install_scans_with_resolved_identifier(monkeypatch, tmp_path, hub_en do_install("skils-sh/anthropics/skills/frontend-design", console=console, skip_confirm=True) assert scanned["source"] == canonical_identifier + + +# --------------------------------------------------------------------------- +# UrlSource-specific install paths: --name override, interactive prompts, +# non-interactive error, existing-category scan. +# --------------------------------------------------------------------------- + + +def _make_url_bundle_fetcher(name="", awaiting_name=True, url="https://example.com/SKILL.md"): + """Return a fake source that simulates ``UrlSource.fetch`` for a + URL-sourced skill whose name hasn't been auto-resolved.""" + + class _UrlSource: + def inspect(self, identifier): + return type("Meta", (), { + "extra": {"url": url, "awaiting_name": awaiting_name}, + "identifier": url, + "name": name, + "path": name, + })() + + def fetch(self, identifier): + return type("Bundle", (), { + "name": name, + "files": {"SKILL.md": "---\ndescription: ok\n---\n# body\n"}, + "source": "url", + "identifier": url, + "trust_level": "community", + "metadata": {"url": url, "awaiting_name": awaiting_name}, + })() + + return _UrlSource + + +def _install_mocks(monkeypatch, tmp_path, source_factory, category_hint=""): + """Wire the minimum set of monkeypatches for a do_install dry run.""" + import tools.skills_hub as hub + import tools.skills_guard as guard + + q_path = tmp_path / "skills" / ".hub" / "quarantine" / "pending" + q_path.mkdir(parents=True) + + install_calls: list = [] + + def _install_from_quarantine(q, name, category, bundle, result): + install_calls.append({"name": name, "category": category}) + install_dir = tmp_path / "skills" / (f"{category}/" if category else "") / name + install_dir.mkdir(parents=True, exist_ok=True) + return install_dir + + monkeypatch.setattr(hub, "ensure_hub_dirs", lambda: None) + monkeypatch.setattr(hub, "create_source_router", lambda auth: [source_factory()]) + monkeypatch.setattr(hub, "quarantine_bundle", lambda bundle: q_path) + monkeypatch.setattr(hub, "install_from_quarantine", _install_from_quarantine) + monkeypatch.setattr( + hub, "HubLockFile", + lambda: type("Lock", (), {"get_installed": lambda self, n: None})(), + ) + monkeypatch.setattr( + guard, "scan_skill", + lambda skill_path, source="community": guard.ScanResult( + skill_name="pending", source=source, trust_level="community", verdict="safe", + ), + ) + monkeypatch.setattr(guard, "format_scan_report", lambda result: "scan ok") + monkeypatch.setattr(guard, "should_allow_install", lambda result, force=False: (True, "ok")) + return install_calls + + +def test_url_install_uses_name_override_on_non_interactive_surface(monkeypatch, tmp_path, hub_env): + installs = _install_mocks(monkeypatch, tmp_path, _make_url_bundle_fetcher()) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_install( + "https://example.com/SKILL.md", + console=console, skip_confirm=True, + name_override="my-url-skill", + ) + + assert installs == [{"name": "my-url-skill", "category": ""}] + + +def test_url_install_rejects_invalid_name_override(monkeypatch, tmp_path, hub_env): + installs = _install_mocks(monkeypatch, tmp_path, _make_url_bundle_fetcher()) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_install( + "https://example.com/SKILL.md", + console=console, skip_confirm=True, + name_override="SKILL", # rejected by _is_valid_installed_skill_name + ) + + assert installs == [] # did NOT install + assert "Invalid --name" in sink.getvalue() + + +def test_url_install_actionable_error_on_non_interactive_with_no_name(monkeypatch, tmp_path, hub_env): + installs = _install_mocks(monkeypatch, tmp_path, _make_url_bundle_fetcher()) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_install( + "https://example.com/SKILL.md", + console=console, skip_confirm=True, + # No name_override — should error out with a retry hint. + ) + + assert installs == [] + out = sink.getvalue() + assert "Cannot install from URL" in out + assert "--name " in out + + +def test_url_install_prompts_interactively_when_tty(monkeypatch, tmp_path, hub_env): + installs = _install_mocks(monkeypatch, tmp_path, _make_url_bundle_fetcher()) + + # Simulate user typing "my-interactive" to name prompt, then "" to category. + answers = iter(["my-interactive", ""]) + monkeypatch.setattr("builtins.input", lambda prompt="": next(answers)) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_install( + "https://example.com/SKILL.md", + console=console, skip_confirm=False, # interactive + force=True, # skip the final confirm prompt (tested elsewhere) + ) + + assert installs == [{"name": "my-interactive", "category": ""}] + + +def test_url_install_prompts_category_and_uses_typed_value(monkeypatch, tmp_path, hub_env): + import tools.skills_hub as hub + installs = _install_mocks( + monkeypatch, tmp_path, + _make_url_bundle_fetcher(name="sharethis-chat", awaiting_name=False), + ) + + # Stage an existing category bucket so _existing_categories finds it. + (hub.SKILLS_DIR / "productivity" / "notion").mkdir(parents=True) + (hub.SKILLS_DIR / "productivity" / "notion" / "SKILL.md").write_text("# notion") + + # Name is already resolved (from frontmatter) → only category prompt fires. + answers = iter(["productivity"]) + monkeypatch.setattr("builtins.input", lambda prompt="": next(answers)) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_install( + "https://example.com/sharethis-chat/SKILL.md", + console=console, skip_confirm=False, force=True, + ) + + assert installs == [{"name": "sharethis-chat", "category": "productivity"}] + assert "Existing: productivity" in sink.getvalue() + + +def test_url_install_cancel_name_prompt_aborts(monkeypatch, tmp_path, hub_env): + installs = _install_mocks(monkeypatch, tmp_path, _make_url_bundle_fetcher()) + + # Empty input with no default → name prompt returns None → abort. + monkeypatch.setattr("builtins.input", lambda prompt="": "") + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + do_install( + "https://example.com/SKILL.md", + console=console, skip_confirm=False, force=True, + ) + + assert installs == [] + assert "Installation cancelled" in sink.getvalue() + + +# ── _existing_categories ──────────────────────────────────────────────────── + + +def test_existing_categories_skips_top_level_skills(monkeypatch, tmp_path, hub_env): + import tools.skills_hub as hub + from hermes_cli.skills_hub import _existing_categories + + # Category bucket with nested skill. + (hub.SKILLS_DIR / "productivity" / "notion").mkdir(parents=True) + (hub.SKILLS_DIR / "productivity" / "notion" / "SKILL.md").write_text("# notion") + + # Flat skill at top level (NOT a category). + (hub.SKILLS_DIR / "my-flat-skill").mkdir() + (hub.SKILLS_DIR / "my-flat-skill" / "SKILL.md").write_text("# flat") + + # Empty dir (NOT a category — no SKILL.md below). + (hub.SKILLS_DIR / "empty-dir").mkdir() + + # Hidden dir (ignored). + (hub.SKILLS_DIR / ".hub").mkdir(exist_ok=True) + + cats = _existing_categories() + assert cats == ["productivity"] + + +def test_existing_categories_returns_empty_when_skills_dir_missing(monkeypatch, tmp_path, hub_env): + # hub_env creates tmp_path/skills/.hub — we point SKILLS_DIR at a missing sibling. + import tools.skills_hub as hub + monkeypatch.setattr(hub, "SKILLS_DIR", tmp_path / "does-not-exist") + + from hermes_cli.skills_hub import _existing_categories + assert _existing_categories() == [] diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index 24d1e87af..8e3453c04 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -12,6 +12,7 @@ from tools.skills_hub import ( GitHubSource, LobeHubSource, SkillsShSource, + UrlSource, WellKnownSkillSource, OptionalSkillSource, SkillMeta, @@ -673,6 +674,211 @@ class TestWellKnownSkillSource: assert bundle is None +class TestUrlSource: + def _source(self): + return UrlSource() + + # ── _matches ──────────────────────────────────────────────────────── + def test_matches_bare_md_url(self): + assert self._source()._matches("https://example.com/path/SKILL.md") is True + + def test_matches_http_scheme(self): + assert self._source()._matches("http://example.com/SKILL.md") is True + + def test_rejects_non_md_url(self): + assert self._source()._matches("https://example.com/path/") is False + assert self._source()._matches("https://example.com/skills.json") is False + + def test_rejects_well_known_url(self): + # Leave these for WellKnownSkillSource. + assert self._source()._matches( + "https://example.com/.well-known/skills/git-workflow/SKILL.md" + ) is False + assert self._source()._matches( + "https://example.com/.well-known/skills/index.json" + ) is False + + def test_rejects_wrapped_identifiers(self): + assert self._source()._matches("github:owner/repo/skill") is False + assert self._source()._matches("well-known:https://example.com/x") is False + assert self._source()._matches("official/security/1password") is False + + def test_rejects_non_string(self): + assert self._source()._matches(None) is False # type: ignore[arg-type] + assert self._source()._matches(123) is False # type: ignore[arg-type] + + def test_search_returns_empty(self): + # Direct-URL source is not searchable. + assert self._source().search("anything") == [] + + # ── inspect ───────────────────────────────────────────────────────── + @patch("tools.skills_hub.httpx.get") + def test_inspect_reads_frontmatter_from_url(self, mock_get): + mock_get.return_value = MagicMock( + status_code=200, + text=( + "---\n" + "name: sharethis-chat\n" + "description: Share agent conversations.\n" + "metadata:\n" + " hermes:\n" + " tags: [sharing, chat]\n" + "---\n\n# Body\n" + ), + ) + meta = self._source().inspect("https://sharethis.chat/SKILL.md") + assert meta is not None + assert meta.name == "sharethis-chat" + assert meta.description == "Share agent conversations." + assert meta.source == "url" + assert meta.identifier == "https://sharethis.chat/SKILL.md" + assert meta.trust_level == "community" + assert meta.tags == ["sharing", "chat"] + assert meta.extra["awaiting_name"] is False + + @patch("tools.skills_hub.httpx.get") + def test_inspect_returns_none_when_url_not_md(self, mock_get): + # _matches filters first — no HTTP call. + meta = self._source().inspect("https://example.com/not-a-skill") + assert meta is None + mock_get.assert_not_called() + + @patch("tools.skills_hub.httpx.get") + def test_inspect_returns_none_on_404(self, mock_get): + mock_get.return_value = MagicMock(status_code=404) + assert self._source().inspect("https://example.com/SKILL.md") is None + + @patch("tools.skills_hub.httpx.get") + def test_inspect_returns_none_on_http_error(self, mock_get): + mock_get.side_effect = httpx.HTTPError("boom") + assert self._source().inspect("https://example.com/SKILL.md") is None + + @patch("tools.skills_hub.httpx.get") + def test_inspect_flags_awaiting_name_when_unresolvable(self, mock_get): + # No frontmatter name + a URL path that can't produce a valid slug + # (``SKILL`` isn't a valid skill name). + mock_get.return_value = MagicMock( + status_code=200, + text="---\ndescription: unnamed.\n---\n", + ) + meta = self._source().inspect("https://example.com/SKILL.md") + assert meta is not None + assert meta.name == "" + assert meta.extra["awaiting_name"] is True + + # ── fetch ─────────────────────────────────────────────────────────── + @patch("tools.skills_hub.httpx.get") + def test_fetch_builds_single_file_bundle(self, mock_get): + skill_md = ( + "---\n" + "name: sharethis-chat\n" + "description: Share.\n" + "---\n\n# Body\n" + ) + mock_get.return_value = MagicMock(status_code=200, text=skill_md) + + bundle = self._source().fetch("https://sharethis.chat/SKILL.md") + + assert bundle is not None + assert bundle.name == "sharethis-chat" + assert bundle.source == "url" + assert bundle.identifier == "https://sharethis.chat/SKILL.md" + assert bundle.trust_level == "community" + assert bundle.files == {"SKILL.md": skill_md} + assert bundle.metadata["url"] == "https://sharethis.chat/SKILL.md" + assert bundle.metadata["awaiting_name"] is False + + @patch("tools.skills_hub.httpx.get") + def test_fetch_falls_back_to_url_directory_name(self, mock_get): + # Frontmatter has no ``name:`` — we slug from the URL directory. + mock_get.return_value = MagicMock( + status_code=200, + text="---\ndescription: No name.\n---\n\n# Body\n", + ) + bundle = self._source().fetch("https://example.com/my-skill/SKILL.md") + assert bundle is not None + assert bundle.name == "my-skill" + assert bundle.metadata["awaiting_name"] is False + + @patch("tools.skills_hub.httpx.get") + def test_fetch_falls_back_to_filename_when_no_parent_dir(self, mock_get): + mock_get.return_value = MagicMock( + status_code=200, + text="---\ndescription: Bare file.\n---\n", + ) + bundle = self._source().fetch("https://example.com/my-skill.md") + assert bundle is not None + assert bundle.name == "my-skill" + assert bundle.metadata["awaiting_name"] is False + + @patch("tools.skills_hub.httpx.get") + def test_fetch_awaiting_name_when_unresolvable(self, mock_get): + # Bare ``SKILL.md`` at the domain root with no frontmatter name. + mock_get.return_value = MagicMock( + status_code=200, + text="---\ndescription: Bare.\n---\n\n# Body\n", + ) + bundle = self._source().fetch("https://example.com/SKILL.md") + assert bundle is not None + assert bundle.name == "" + assert bundle.metadata["awaiting_name"] is True + # File content still present — CLI will reuse it after picking a name. + assert bundle.files["SKILL.md"].startswith("---\n") + + @patch("tools.skills_hub.httpx.get") + def test_fetch_awaiting_name_rejects_sentinel_slug(self, mock_get): + # Frontmatter has no name AND the URL filename slug is ``README`` — + # our valid-name check rejects it, so we flag awaiting_name. + mock_get.return_value = MagicMock( + status_code=200, + text="---\ndescription: no name.\n---\n", + ) + bundle = self._source().fetch("https://example.com/README.md") + assert bundle is not None + assert bundle.name == "" + assert bundle.metadata["awaiting_name"] is True + + @patch("tools.skills_hub.httpx.get") + def test_fetch_ignores_unsafe_frontmatter_name_and_falls_through_to_slug(self, mock_get): + # Traversal / unsafe names are rejected by ``_is_valid_skill_name``; + # resolver falls through to URL slug (``my-skill`` here) and succeeds. + mock_get.return_value = MagicMock( + status_code=200, + text="---\nname: ../evil\ndescription: Bad.\n---\n", + ) + bundle = self._source().fetch("https://example.com/my-skill/SKILL.md") + assert bundle is not None + assert bundle.name == "my-skill" + + @patch("tools.skills_hub.httpx.get") + def test_fetch_returns_none_on_404(self, mock_get): + mock_get.return_value = MagicMock(status_code=404) + assert self._source().fetch("https://example.com/SKILL.md") is None + + @patch("tools.skills_hub.httpx.get") + def test_fetch_skips_non_matching_identifier(self, mock_get): + assert self._source().fetch("owner/repo/skill") is None + mock_get.assert_not_called() + + # ── _is_valid_skill_name ──────────────────────────────────────────── + def test_is_valid_skill_name_accepts_identifiers(self): + valid = ["my-skill", "my_skill", "sharethis-chat", "a", "skill-1", "s1"] + for name in valid: + assert UrlSource._is_valid_skill_name(name), f"should accept {name!r}" + + def test_is_valid_skill_name_rejects_sentinel_and_garbage(self): + invalid = [ + "", + "SKILL", "skill", "README", "readme", "INDEX", "index", + "unnamed-skill", + "../evil", "a/b", "has space", "has.dot", + "-leading-dash", "1-leading-digit", + None, 123, ["list"], + ] + for name in invalid: + assert not UrlSource._is_valid_skill_name(name), f"should reject {name!r}" + + class TestCheckForSkillUpdates: def test_bundle_content_hash_matches_installed_content_hash(self, tmp_path): from tools.skills_guard import content_hash @@ -755,6 +961,17 @@ class TestCreateSourceRouter: sources = create_source_router(auth=MagicMock(spec=GitHubAuth)) assert any(isinstance(src, WellKnownSkillSource) for src in sources) + def test_includes_url_source(self): + sources = create_source_router(auth=MagicMock(spec=GitHubAuth)) + assert any(isinstance(src, UrlSource) for src in sources) + + def test_url_source_runs_before_github_source(self): + # UrlSource must win over GitHubSource when both could claim a URL. + sources = create_source_router(auth=MagicMock(spec=GitHubAuth)) + url_idx = next(i for i, src in enumerate(sources) if isinstance(src, UrlSource)) + gh_idx = next(i for i, src in enumerate(sources) if isinstance(src, GitHubSource)) + assert url_idx < gh_idx + # --------------------------------------------------------------------------- # HubLockFile diff --git a/tools/skills_hub.py b/tools/skills_hub.py index 2b5216407..0ce1d9b34 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -931,6 +931,176 @@ class WellKnownSkillSource(SkillSource): return f"well-known:{base_url.rstrip('/')}/{skill_name}" +# --------------------------------------------------------------------------- +# Direct URL source adapter +# --------------------------------------------------------------------------- + +class UrlSource(SkillSource): + """Fetch a single-file SKILL.md skill directly from an HTTP(S) URL. + + The identifier IS the URL (e.g. ``https://example.com/path/SKILL.md``). + Only single-file skills are supported — multi-file skills with + ``references/`` or ``scripts/`` subfolders need a manifest we can't + discover from a bare URL. + + The skill name is read from the ``name:`` field in the SKILL.md YAML + frontmatter (with a URL-slug fallback). Trust level is always + ``community`` and the same security scan runs as for every other source. + """ + + def source_id(self) -> str: + return "url" + + def trust_level_for(self, identifier: str) -> str: + return "community" + + # Search is meaningless for a direct URL — skip (return empty). + def search(self, query: str, limit: int = 10) -> List[SkillMeta]: + return [] + + def _matches(self, identifier: str) -> bool: + """Return True iff this source should handle ``identifier``. + + We claim bare HTTP(S) URLs that end in ``.md`` (typically + ``.../SKILL.md``). Wrapped identifiers (``github:``, + ``well-known:``, etc.) and ``/.well-known/skills/`` URLs are + left for their respective adapters. + """ + if not isinstance(identifier, str): + return False + ident = identifier.strip() + if not ident.lower().startswith(("http://", "https://")): + return False + # Don't steal well-known URLs. + if "/.well-known/skills/" in ident or ident.rstrip("/").endswith("/index.json"): + return False + # Only claim URLs that look like a markdown file. + try: + path = urlparse(ident).path + except ValueError: + return False + return path.lower().endswith(".md") + + def inspect(self, identifier: str) -> Optional[SkillMeta]: + if not self._matches(identifier): + return None + url = identifier.strip() + text = self._fetch_text(url) + if text is None: + return None + fm = GitHubSource._parse_frontmatter_quick(text) + name = self._resolve_skill_name(fm, url) + description = str(fm.get("description") or "") + tags: List[str] = [] + metadata = fm.get("metadata", {}) + if isinstance(metadata, dict): + hermes_meta = metadata.get("hermes", {}) + if isinstance(hermes_meta, dict): + raw_tags = hermes_meta.get("tags", []) + if isinstance(raw_tags, list): + tags = [str(t) for t in raw_tags] + return SkillMeta( + name=name or "", + description=description, + source="url", + identifier=url, + trust_level="community", + path=name or "", + tags=tags, + extra={"url": url, "awaiting_name": name is None}, + ) + + def fetch(self, identifier: str) -> Optional[SkillBundle]: + if not self._matches(identifier): + return None + url = identifier.strip() + text = self._fetch_text(url) + if text is None: + return None + + fm = GitHubSource._parse_frontmatter_quick(text) + name = self._resolve_skill_name(fm, url) + + # When auto-resolution fails, return a bundle with an empty name and + # ``awaiting_name=True`` in metadata. The install flow (``do_install``) + # either prompts the user on a TTY or refuses with an actionable error + # on non-interactive surfaces. Keep the expensive HTTP fetch's result + # so the caller doesn't have to re-download after picking a name. + skill_name = "" + if name is not None: + try: + skill_name = _validate_skill_name(name) + except ValueError: + logger.warning("URL skill %s produced unsafe skill name: %r", url, name) + return None + + return SkillBundle( + name=skill_name, + files={"SKILL.md": text}, + source="url", + identifier=url, + trust_level="community", + metadata={"url": url, "awaiting_name": not skill_name}, + ) + + @staticmethod + def _fetch_text(url: str) -> Optional[str]: + try: + resp = httpx.get(url, timeout=20, follow_redirects=True) + if resp.status_code == 200: + return resp.text + except httpx.HTTPError as exc: + logger.debug("UrlSource fetch failed for %s: %s", url, exc) + return None + return None + + # Skill names must look like identifiers: lowercase letters/digits with + # optional hyphens/underscores. Blocks dangerous (``../evil``) AND useless + # (``SKILL``, ``README``, empty) candidates before they hit the disk. + _VALID_NAME_RE = re.compile(r"^[a-z][a-z0-9_-]*$") + + @classmethod + def _is_valid_skill_name(cls, name: Optional[str]) -> bool: + if not isinstance(name, str): + return False + candidate = name.strip().lower() + if not candidate or candidate in {"skill", "readme", "index", "unnamed-skill"}: + return False + return bool(cls._VALID_NAME_RE.match(candidate)) + + @classmethod + def _resolve_skill_name(cls, fm: dict, url: str) -> Optional[str]: + """Pick a skill name from frontmatter or URL. + + Returns ``None`` when neither source produces a valid identifier; + callers (CLI ``do_install``) then prompt the user or refuse. Preferring + a clean failure over a useless auto-name like ``SKILL`` or ``unnamed-skill``. + """ + # 1. Frontmatter ``name:`` is authoritative when present and valid. + fm_name = fm.get("name") if isinstance(fm, dict) else None + if isinstance(fm_name, str) and cls._is_valid_skill_name(fm_name): + return fm_name.strip() + + # 2. URL-slug heuristic: ``...//SKILL.md`` → ````; + # ``.../.md`` → ````. Validate each candidate. + try: + path = urlparse(url).path + except ValueError: + return None + parts = [p for p in path.split("/") if p] + if parts and parts[-1].lower() == "skill.md" and len(parts) >= 2: + candidate = parts[-2] + if cls._is_valid_skill_name(candidate): + return candidate + if parts: + candidate = re.sub(r"\.md$", "", parts[-1], flags=re.IGNORECASE) + if cls._is_valid_skill_name(candidate): + return candidate + + # Nothing usable — let the caller handle it. + return None + + # --------------------------------------------------------------------------- # skills.sh source adapter # --------------------------------------------------------------------------- @@ -2931,6 +3101,7 @@ def create_source_router(auth: Optional[GitHubAuth] = None) -> List[SkillSource] HermesIndexSource(auth=auth), # Centralized index (search + resolved install paths) SkillsShSource(auth=auth), WellKnownSkillSource(), + UrlSource(), # Direct HTTP(S) URL to a SKILL.md file GitHubSource(auth=auth, extra_taps=extra_taps), ClawHubSource(), ClaudeMarketplaceSource(auth=auth),