diff --git a/nanobot/channels/websocket/tests/test_websocket_http_routes.py b/nanobot/channels/websocket/tests/test_websocket_http_routes.py index 8420704cd..3c058e7de 100644 --- a/nanobot/channels/websocket/tests/test_websocket_http_routes.py +++ b/nanobot/channels/websocket/tests/test_websocket_http_routes.py @@ -671,10 +671,19 @@ async def test_webui_skills_marketplace_routes_search_and_install( "trends": {"acme/agent-skills/react-testing": [2, 4, 3, 8]}, }) - async def install(source: str, skill_id: str, workspace: Path) -> dict[str, Any]: + async def install( + source: str, + skill_id: str, + workspace: Path, + *, + provider: str, + version: str, + ) -> dict[str, Any]: assert source == "acme/agent-skills" assert skill_id == "react-testing" assert workspace == tmp_path + assert provider == "skills_sh" + assert version == "" skill_dir = workspace / "skills" / skill_id skill_dir.mkdir(parents=True) (skill_dir / "SKILL.md").write_text( @@ -711,7 +720,7 @@ async def test_webui_skills_marketplace_routes_search_and_install( ) assert search_response.status_code == 200 assert search_response.json()["skills"][0]["skill_id"] == "react-testing" - search.assert_awaited_once_with("react", tmp_path) + search.assert_awaited_once_with("react", tmp_path, provider="all") trending_response = await _http_get( f"http://127.0.0.1:{port}/api/webui/skills/trending", @@ -719,7 +728,7 @@ async def test_webui_skills_marketplace_routes_search_and_install( ) assert trending_response.status_code == 200 assert trending_response.json()["period"] == "24h" - trending.assert_awaited_once_with(tmp_path) + trending.assert_awaited_once_with(tmp_path, provider="all") trends_response = await _http_get( f"http://127.0.0.1:{port}/api/webui/skills/trends" diff --git a/nanobot/webui/skills_marketplace.py b/nanobot/webui/skills_marketplace.py index 9425a5051..020e36058 100644 --- a/nanobot/webui/skills_marketplace.py +++ b/nanobot/webui/skills_marketplace.py @@ -1,23 +1,37 @@ -"""Search and install skills from the skills.sh catalog.""" +"""Search and install skills from public Agent Skills catalogs.""" from __future__ import annotations import asyncio +import hashlib import os import re import shutil +import stat +import tempfile import time -from pathlib import Path +import zipfile +from pathlib import Path, PurePosixPath from typing import Any +from urllib.parse import quote, urlparse import httpx from nanobot.agent.skills import SkillsLoader from nanobot.security.network import PinnedDNSAsyncTransport +_PROVIDER_ALL = "all" +_PROVIDER_SKILLS_SH = "skills_sh" +_PROVIDER_SKILLHUB = "skillhub" +_PROVIDERS = {_PROVIDER_ALL, _PROVIDER_SKILLS_SH, _PROVIDER_SKILLHUB} _SEARCH_URL = "https://skills.sh/api/search" _TRENDING_URL = "https://skills.sh/api/skills/trending/0" _SKILL_PAGE_BASE_URL = "https://www.skills.sh" +_SKILLHUB_API_BASE_URL = "https://api.skillhub.cn" +_SKILLHUB_SEARCH_URL = f"{_SKILLHUB_API_BASE_URL}/api/v1/search" +_SKILLHUB_TRENDING_URL = f"{_SKILLHUB_API_BASE_URL}/api/v1/showcase/trending" +_SKILLHUB_DOWNLOAD_URL = f"{_SKILLHUB_API_BASE_URL}/api/v1/download" +_SKILLHUB_PAGE_BASE_URL = "https://skillhub.cn" _ALL_TIME_URLS = ( "https://skills.sh/api/skills/all-time/0", "https://skills.sh/api/skills/all-time/1", @@ -28,9 +42,13 @@ _SOURCE_RE = re.compile( r"[A-Za-z0-9](?:[A-Za-z0-9_.-]{0,98}[A-Za-z0-9])?$" ) _SKILL_RE = re.compile(r"^[a-z0-9]+(?:-[a-z0-9]+)*$") +_VERSION_RE = re.compile(r"^[A-Za-z0-9](?:[A-Za-z0-9._+-]{0,63})$") _ANSI_RE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") _INSTALL_TIMEOUT_SECONDS = 120 _WEEKLY_CACHE_TTL_SECONDS = 300 +_SKILLHUB_MAX_DOWNLOAD_BYTES = 25 * 1024 * 1024 +_SKILLHUB_MAX_UNPACKED_BYTES = 100 * 1024 * 1024 +_SKILLHUB_MAX_FILES = 1_000 # The skills CLI's OpenClaw adapter copies into /skills, nanobot's layout too. _CLI_AGENT = "openclaw" _weekly_cache: dict[tuple[str, str], list[int]] = {} @@ -55,6 +73,43 @@ async def trending_marketplace_skills( workspace_path: Path, *, limit: int = 8, + provider: str = _PROVIDER_ALL, +) -> dict[str, Any]: + """Return provider-aware marketplace rankings without mixing metric semantics.""" + selected = _valid_provider(provider) + if selected == _PROVIDER_SKILLHUB: + return await _trending_skillhub_skills(workspace_path, limit=limit) + if selected == _PROVIDER_SKILLS_SH: + return await _trending_skills_sh_skills(workspace_path, limit=limit) + + results = await asyncio.gather( + _trending_skills_sh_skills(workspace_path, limit=limit), + _trending_skillhub_skills(workspace_path, limit=limit), + return_exceptions=True, + ) + payloads = [result for result in results if isinstance(result, dict)] + if not payloads: + raise SkillsMarketplaceError( + "skill marketplaces are temporarily unavailable", + status=502, + ) + return { + "skills": [ + skill + for payload in payloads + for skill in payload.get("skills", []) + if isinstance(skill, dict) + ], + "period": "mixed", + "provider": _PROVIDER_ALL, + "install_supported": any(bool(payload.get("install_supported")) for payload in payloads), + } + + +async def _trending_skills_sh_skills( + workspace_path: Path, + *, + limit: int, ) -> dict[str, Any]: """Return a source-diverse snapshot of skills.sh's real 24-hour leaderboard.""" try: @@ -89,6 +144,7 @@ async def trending_marketplace_skills( return { "skills": skills, "period": "24h", + "provider": _PROVIDER_SKILLS_SH, "install_supported": skills_install_supported(), } @@ -98,14 +154,51 @@ async def search_marketplace_skills( workspace_path: Path, *, limit: int = 20, + provider: str = _PROVIDER_ALL, ) -> dict[str, Any]: - """Search skills.sh and annotate results already installed in this workspace.""" + """Search one or all catalogs and annotate locally installed results.""" normalized = " ".join(query.split()) if len(normalized) < 2: raise SkillsMarketplaceError("search query must contain at least 2 characters") if len(normalized) > 100: raise SkillsMarketplaceError("search query is too long") + selected = _valid_provider(provider) + if selected == _PROVIDER_SKILLHUB: + return await _search_skillhub_skills(normalized, workspace_path, limit=limit) + if selected == _PROVIDER_SKILLS_SH: + return await _search_skills_sh_skills(normalized, workspace_path, limit=limit) + + results = await asyncio.gather( + _search_skills_sh_skills(normalized, workspace_path, limit=limit), + _search_skillhub_skills(normalized, workspace_path, limit=limit), + return_exceptions=True, + ) + payloads = [result for result in results if isinstance(result, dict)] + if not payloads: + raise SkillsMarketplaceError( + "skill marketplaces are temporarily unavailable", + status=502, + ) + return { + "query": normalized, + "skills": [ + skill + for payload in payloads + for skill in payload.get("skills", []) + if isinstance(skill, dict) + ], + "provider": _PROVIDER_ALL, + "install_supported": any(bool(payload.get("install_supported")) for payload in payloads), + } + + +async def _search_skills_sh_skills( + normalized: str, + workspace_path: Path, + *, + limit: int, +) -> dict[str, Any]: try: async with _skills_client() as client: response = await client.get( @@ -133,10 +226,82 @@ async def search_marketplace_skills( return { "query": normalized, "skills": skills, + "provider": _PROVIDER_SKILLS_SH, "install_supported": skills_install_supported(), } +async def _search_skillhub_skills( + normalized: str, + workspace_path: Path, + *, + limit: int, +) -> dict[str, Any]: + try: + async with _skillhub_client() as client: + response = await client.get( + _SKILLHUB_SEARCH_URL, + params={"q": normalized, "limit": min(max(limit, 1), 50)}, + ) + response.raise_for_status() + payload = response.json() + except (httpx.HTTPError, ValueError) as exc: + raise SkillsMarketplaceError( + "SkillHub search is temporarily unavailable", + status=502, + ) from exc + + installed = _installed_skill_names(workspace_path) + rows = payload.get("results", []) if isinstance(payload, dict) else [] + skills = [ + skill + for row in rows + if isinstance(row, dict) + if (skill := _skillhub_skill(row, installed)) is not None + ] + return { + "query": normalized, + "skills": skills, + "provider": _PROVIDER_SKILLHUB, + "install_supported": True, + } + + +async def _trending_skillhub_skills( + workspace_path: Path, + *, + limit: int, +) -> dict[str, Any]: + try: + async with _skillhub_client() as client: + response = await client.get(_SKILLHUB_TRENDING_URL) + response.raise_for_status() + payload = response.json() + except (httpx.HTTPError, ValueError) as exc: + raise SkillsMarketplaceError( + "SkillHub trending skills are temporarily unavailable", + status=502, + ) from exc + + installed = _installed_skill_names(workspace_path) + rows = payload.get("skills", []) if isinstance(payload, dict) else [] + skills: list[dict[str, Any]] = [] + for rank, row in enumerate(rows, start=1): + if not isinstance(row, dict): + continue + skill = _skillhub_skill(row, installed, rank=rank) + if skill is not None: + skills.append(skill) + if len(skills) >= min(max(limit, 1), 20): + break + return { + "skills": skills, + "period": "trending", + "provider": _PROVIDER_SKILLHUB, + "install_supported": True, + } + + async def marketplace_skill_trends( skill_ids: list[str] | None = None, ) -> dict[str, dict[str, list[int]]]: @@ -162,18 +327,29 @@ async def install_marketplace_skill( source: str, skill_id: str, workspace_path: Path, + *, + provider: str = _PROVIDER_SKILLS_SH, + version: str = "", +) -> dict[str, Any]: + """Install one normalized marketplace result into ``/skills``.""" + selected = _valid_provider(provider, allow_all=False) + if selected == _PROVIDER_SKILLHUB: + return await _install_skillhub_skill(skill_id, version, workspace_path) + return await _install_skills_sh_skill(source, skill_id, workspace_path) + + +async def _install_skills_sh_skill( + source: str, + skill_id: str, + workspace_path: Path, ) -> dict[str, Any]: - """Install one skills.sh result into ``/skills``.""" if not _SOURCE_RE.fullmatch(source): raise SkillsMarketplaceError("invalid skill source") if not _valid_skill_id(skill_id): raise SkillsMarketplaceError("invalid skill name") loader = SkillsLoader(workspace_path) - existing = { - entry["name"]: entry - for entry in loader.list_skills(filter_unavailable=False) - } + existing = {entry["name"]: entry for entry in loader.list_skills(filter_unavailable=False)} if skill_id in existing: return {"installed": True, "already_installed": True, "name": skill_id} @@ -242,6 +418,287 @@ async def install_marketplace_skill( return {"installed": True, "already_installed": False, "name": skill_id} +async def _install_skillhub_skill( + skill_id: str, + requested_version: str, + workspace_path: Path, +) -> dict[str, Any]: + if not _valid_skill_id(skill_id): + raise SkillsMarketplaceError("invalid SkillHub skill name") + if requested_version and _VERSION_RE.fullmatch(requested_version) is None: + raise SkillsMarketplaceError("invalid SkillHub skill version") + + loader = SkillsLoader(workspace_path) + existing = {entry["name"]: entry for entry in loader.list_skills(filter_unavailable=False)} + if skill_id in existing: + return { + "installed": True, + "already_installed": True, + "name": skill_id, + "provider": _PROVIDER_SKILLHUB, + } + + workspace = workspace_path.expanduser().resolve() + skills_root = workspace / "skills" + skills_root.mkdir(parents=True, exist_ok=True) + target = skills_root / skill_id + + try: + async with _skillhub_client() as client: + version = requested_version or await _skillhub_latest_version(client, skill_id) + signature = await _skillhub_signature(client, skill_id, version) + expected_hash = signature.get("content_hash") + if not isinstance(expected_hash, str) or not re.fullmatch( + r"[0-9a-fA-F]{64}", + expected_hash, + ): + raise SkillsMarketplaceError( + "SkillHub did not provide a valid package fingerprint", + status=502, + ) + + with tempfile.TemporaryDirectory( + prefix=".skillhub-install-", + dir=skills_root, + ) as temporary: + temporary_path = Path(temporary) + archive_path = temporary_path / f"{skill_id}.zip" + stage_path = temporary_path / "stage" + await _download_skillhub_archive( + client, + skill_id, + version, + archive_path, + ) + actual_hash = _validate_skillhub_archive(archive_path) + if actual_hash.lower() != expected_hash.lower(): + raise SkillsMarketplaceError( + "SkillHub package fingerprint did not match", + status=502, + ) + _extract_skillhub_archive(archive_path, stage_path) + if target.exists(): + return { + "installed": True, + "already_installed": True, + "name": skill_id, + "provider": _PROVIDER_SKILLHUB, + } + os.replace(stage_path, target) + except SkillsMarketplaceError: + raise + except (httpx.HTTPError, OSError, zipfile.BadZipFile) as exc: + raise SkillsMarketplaceError( + "SkillHub skill installation failed", + status=502, + ) from exc + + installed = next( + ( + entry + for entry in loader.list_skills(filter_unavailable=False) + if entry["source"] == "workspace" and entry["name"] == skill_id + ), + None, + ) + if installed is None: + raise SkillsMarketplaceError( + "installer completed but the skill was not found in this workspace", + status=502, + ) + return { + "installed": True, + "already_installed": False, + "name": skill_id, + "provider": _PROVIDER_SKILLHUB, + "version": version, + "verified": bool(signature.get("signed")), + } + + +async def _skillhub_latest_version(client: httpx.AsyncClient, skill_id: str) -> str: + response = await client.get( + f"{_SKILLHUB_API_BASE_URL}/api/v1/skills/{quote(skill_id, safe='')}" + ) + response.raise_for_status() + payload = response.json() + latest = payload.get("latestVersion", {}) if isinstance(payload, dict) else {} + version = latest.get("version") if isinstance(latest, dict) else None + if not isinstance(version, str) or _VERSION_RE.fullmatch(version) is None: + raise SkillsMarketplaceError( + "SkillHub did not return a valid skill version", + status=502, + ) + return version + + +async def _skillhub_signature( + client: httpx.AsyncClient, + skill_id: str, + version: str, +) -> dict[str, Any]: + response = await client.get( + f"{_SKILLHUB_API_BASE_URL}/api/v1/open/skills/" + f"{quote(skill_id, safe='')}/versions/{quote(version, safe='')}/signature" + ) + response.raise_for_status() + payload = response.json() + if not isinstance(payload, dict): + raise SkillsMarketplaceError( + "SkillHub returned an invalid package fingerprint", + status=502, + ) + return payload + + +async def _download_skillhub_archive( + client: httpx.AsyncClient, + skill_id: str, + version: str, + destination: Path, +) -> None: + redirect = await client.get( + _SKILLHUB_DOWNLOAD_URL, + params={"slug": skill_id, "version": version}, + ) + if redirect.status_code not in {301, 302, 303, 307, 308}: + redirect.raise_for_status() + raise SkillsMarketplaceError( + "SkillHub returned an unexpected download response", + status=502, + ) + location = redirect.headers.get("location", "") + if not _valid_skillhub_download_url(location): + raise SkillsMarketplaceError( + "SkillHub returned an unsafe download location", + status=502, + ) + + received = 0 + async with client.stream( + "GET", + location, + headers={"Accept": "application/zip,application/octet-stream"}, + ) as response: + response.raise_for_status() + declared = response.headers.get("content-length") + if declared and declared.isdigit() and int(declared) > _SKILLHUB_MAX_DOWNLOAD_BYTES: + raise SkillsMarketplaceError("SkillHub package is too large", status=413) + with destination.open("wb") as output: + async for chunk in response.aiter_bytes(): + received += len(chunk) + if received > _SKILLHUB_MAX_DOWNLOAD_BYTES: + raise SkillsMarketplaceError("SkillHub package is too large", status=413) + output.write(chunk) + + +def _valid_skillhub_download_url(value: str) -> bool: + try: + parsed = urlparse(value) + hostname = (parsed.hostname or "").lower() + port = parsed.port + except ValueError: + return False + return ( + parsed.scheme == "https" + and parsed.username is None + and parsed.password is None + and port in {None, 443} + and hostname.endswith(".myqcloud.com") + ) + + +def _validated_skillhub_entries( + archive: zipfile.ZipFile, +) -> list[tuple[zipfile.ZipInfo, str]]: + entries: list[tuple[zipfile.ZipInfo, str]] = [] + seen: set[str] = set() + unpacked = 0 + for info in archive.infolist(): + raw_name = info.filename.replace("\\", "/") + path = PurePosixPath(raw_name) + normalized = path.as_posix() + mode = info.external_attr >> 16 + kind = stat.S_IFMT(mode) + if ( + not normalized + or "\x00" in normalized + or path.is_absolute() + or ".." in path.parts + or (path.parts and ":" in path.parts[0]) + or kind == stat.S_IFLNK + or kind not in {0, stat.S_IFREG, stat.S_IFDIR} + ): + raise SkillsMarketplaceError( + f"SkillHub package contains an unsafe path: {raw_name}", + status=422, + ) + if info.is_dir(): + continue + if normalized in seen: + raise SkillsMarketplaceError( + f"SkillHub package contains a duplicate path: {normalized}", + status=422, + ) + seen.add(normalized) + unpacked += info.file_size + if len(entries) >= _SKILLHUB_MAX_FILES: + raise SkillsMarketplaceError("SkillHub package contains too many files", status=413) + if unpacked > _SKILLHUB_MAX_UNPACKED_BYTES: + raise SkillsMarketplaceError( + "SkillHub package expands beyond the size limit", status=413 + ) + entries.append((info, normalized)) + if "SKILL.md" not in seen: + raise SkillsMarketplaceError( + "SkillHub package does not contain a root SKILL.md", + status=422, + ) + return entries + + +def _validate_skillhub_archive(archive_path: Path) -> str: + hashed: list[tuple[str, str]] = [] + with zipfile.ZipFile(archive_path, "r") as archive: + for info, normalized in _validated_skillhub_entries(archive): + if _skillhub_hash_ignored(normalized): + continue + digest = hashlib.sha256() + with archive.open(info, "r") as source: + for chunk in iter(lambda: source.read(1024 * 1024), b""): + digest.update(chunk) + hashed.append((normalized, digest.hexdigest())) + combined = hashlib.sha256() + for normalized, digest in sorted(hashed): + combined.update(f"{normalized}:{digest}\n".encode()) + return combined.hexdigest() + + +def _skillhub_hash_ignored(path: str) -> bool: + parts = PurePosixPath(path).parts + basename = parts[-1] if parts else "" + return ( + path == "_meta.json" + or "__MACOSX" in parts + or basename == ".DS_Store" + or basename.startswith("._") + or basename.lower() == "thumbs.db" + ) + + +def _extract_skillhub_archive(archive_path: Path, destination: Path) -> None: + destination.mkdir() + with zipfile.ZipFile(archive_path, "r") as archive: + for info, normalized in _validated_skillhub_entries(archive): + target = destination.joinpath(*PurePosixPath(normalized).parts) + target.parent.mkdir(parents=True, exist_ok=True) + with archive.open(info, "r") as source, target.open("wb") as output: + shutil.copyfileobj(source, output) + mode = (info.external_attr >> 16) & 0o777 + if mode: + target.chmod(mode & 0o755) + + def _skills_client() -> httpx.AsyncClient: return httpx.AsyncClient( transport=PinnedDNSAsyncTransport(), @@ -250,6 +707,14 @@ def _skills_client() -> httpx.AsyncClient: ) +def _skillhub_client() -> httpx.AsyncClient: + return httpx.AsyncClient( + transport=PinnedDNSAsyncTransport(), + timeout=httpx.Timeout(30.0, connect=10.0), + follow_redirects=False, + ) + + def _installed_skill_names(workspace_path: Path) -> set[str]: return { entry["name"] @@ -278,9 +743,66 @@ def _marketplace_skill( "skill_id": skill_id, "name": display_name.strip(), "source": source, + "provider": _PROVIDER_SKILLS_SH, "installs": installs if isinstance(installs, int) and installs >= 0 else 0, "url": f"https://skills.sh/{source}/{skill_id}", "installed": skill_id in installed, + "install_supported": skills_install_supported(), + "metric": "installs_24h" if rank is not None else "installs_total", + } + if rank is not None: + skill["rank"] = rank + return skill + + +def _skillhub_skill( + row: dict[str, Any], + installed: set[str], + *, + rank: int | None = None, +) -> dict[str, Any] | None: + skill_id = row.get("slug") + if not isinstance(skill_id, str) or not _valid_skill_id(skill_id): + return None + display_name = row.get("displayName") or row.get("name") or skill_id + if not isinstance(display_name, str) or not display_name.strip(): + display_name = skill_id + + namespace = row.get("namespace") + handle = namespace.get("handle") if isinstance(namespace, dict) else None + if not isinstance(handle, str) or not handle.strip(): + owner = row.get("owner_name") or row.get("ownerName") + handle = owner if isinstance(owner, str) and owner.strip() else "community" + source = f"@{handle.strip()}/{skill_id}" + + installs = row.get("installs") + downloads = row.get("downloads") + publisher = row.get("publisher") + verified = bool(isinstance(publisher, dict) and publisher.get("verified") is True) + labels = row.get("labels") + requires_api_key = bool( + isinstance(labels, dict) and str(labels.get("requires_api_key", "")).lower() == "true" + ) + version = row.get("version") + if not isinstance(version, str) or _VERSION_RE.fullmatch(version) is None: + version = "" + + skill: dict[str, Any] = { + "id": f"{_PROVIDER_SKILLHUB}:{skill_id}", + "skill_id": skill_id, + "name": display_name.strip(), + "source": source, + "provider": _PROVIDER_SKILLHUB, + "installs": installs if isinstance(installs, int) and installs >= 0 else 0, + "downloads": downloads if isinstance(downloads, int) and downloads >= 0 else 0, + "url": f"{_SKILLHUB_PAGE_BASE_URL}/{quote(handle.strip(), safe='')}/" + f"{quote(skill_id, safe='')}", + "installed": skill_id in installed, + "install_supported": True, + "metric": "installs_total", + "version": version, + "verified": verified, + "requires_api_key": requires_api_key, } if rank is not None: skill["rank"] = rank @@ -318,11 +840,7 @@ async def _load_weekly_installs( source = row.get("source") skill_id = row.get("skillId") values = row.get("weeklyInstalls") - if ( - isinstance(source, str) - and isinstance(skill_id, str) - and isinstance(values, list) - ): + if isinstance(source, str) and isinstance(skill_id, str) and isinstance(values, list): clean = [ value for value in values @@ -344,11 +862,7 @@ def _valid_skill_refs(skill_ids: list[str]) -> list[tuple[str, str]]: continue source, skill_id = value.rsplit("/", 1) ref = (source, skill_id) - if ( - _SOURCE_RE.fullmatch(source) - and _valid_skill_id(skill_id) - and ref not in refs - ): + if _SOURCE_RE.fullmatch(source) and _valid_skill_id(skill_id) and ref not in refs: refs.append(ref) return refs @@ -363,9 +877,7 @@ async def _load_skill_page_trends( source, skill_id = ref try: async with semaphore: - response = await client.get( - f"{_SKILL_PAGE_BASE_URL}/{source}/{skill_id}" - ) + response = await client.get(f"{_SKILL_PAGE_BASE_URL}/{source}/{skill_id}") response.raise_for_status() except httpx.HTTPError: return ref, [] @@ -373,11 +885,7 @@ async def _load_skill_page_trends( match = _TREND_VALUES_RE.search(response.text) if match is None: return ref, [] - values = [ - int(value) - for value in match.group(1).split(",") - if value.strip() - ] + values = [int(value) for value in match.group(1).split(",") if value.strip()] return ref, values if len(values) >= 2 else [] return dict(await asyncio.gather(*(fetch(ref) for ref in refs))) @@ -387,6 +895,14 @@ def _valid_skill_id(value: str) -> bool: return len(value) <= 64 and _SKILL_RE.fullmatch(value) is not None +def _valid_provider(value: str, *, allow_all: bool = True) -> str: + normalized = value.strip().lower() or _PROVIDER_ALL + allowed = _PROVIDERS if allow_all else _PROVIDERS - {_PROVIDER_ALL} + if normalized not in allowed: + raise SkillsMarketplaceError("invalid skill marketplace provider") + return normalized + + def _safe_output_tail(output: bytes | None) -> str: if not output: return "" diff --git a/nanobot/webui/ws_http.py b/nanobot/webui/ws_http.py index 1a98cf965..d5ba320c4 100644 --- a/nanobot/webui/ws_http.py +++ b/nanobot/webui/ws_http.py @@ -860,26 +860,36 @@ class GatewayHTTPHandler: async def _handle_webui_skills_search(self, request: WsRequest) -> Response: if not self.check_api_token(request): return _http_error(401, "Unauthorized") - query = _query_first(_parse_query(request.path), "q") or "" + params = _parse_query(request.path) + query = _query_first(params, "q") or "" + provider = _query_first(params, "provider") or "all" try: - payload = await search_marketplace_skills(query, self.skills_workspace_path) + payload = await search_marketplace_skills( + query, + self.skills_workspace_path, + provider=provider, + ) except SkillsMarketplaceError as exc: return _http_error(exc.status, exc.message) except Exception: - self._log.exception("skills.sh search failed") - return _http_error(500, "skills.sh search failed") + self._log.exception("skills marketplace search failed") + return _http_error(500, "skills marketplace search failed") return _http_json_response(payload) async def _handle_webui_skills_trending(self, request: WsRequest) -> Response: if not self.check_api_token(request): return _http_error(401, "Unauthorized") + provider = _query_first(_parse_query(request.path), "provider") or "all" try: - payload = await trending_marketplace_skills(self.skills_workspace_path) + payload = await trending_marketplace_skills( + self.skills_workspace_path, + provider=provider, + ) except SkillsMarketplaceError as exc: return _http_error(exc.status, exc.message) except Exception: - self._log.exception("skills.sh trending lookup failed") - return _http_error(500, "skills.sh trending lookup failed") + self._log.exception("skills marketplace trending lookup failed") + return _http_error(500, "skills marketplace trending lookup failed") return _http_json_response(payload) async def _handle_webui_skill_trends(self, request: WsRequest) -> Response: @@ -904,13 +914,17 @@ class GatewayHTTPHandler: return _http_error(403, "remote skill installation is disabled") query = _parse_query(request.path) + provider = _query_first(query, "provider") or "skills_sh" source = _query_first(query, "source") or "" skill_id = _query_first(query, "skill") or "" + version = _query_first(query, "version") or "" try: action = await install_marketplace_skill( source, skill_id, self.skills_workspace_path, + provider=provider, + version=version, ) except SkillsMarketplaceError as exc: return _http_error(exc.status, exc.message) diff --git a/tests/webui/test_skills_marketplace.py b/tests/webui/test_skills_marketplace.py index 1d10e1161..8318e3f15 100644 --- a/tests/webui/test_skills_marketplace.py +++ b/tests/webui/test_skills_marketplace.py @@ -1,3 +1,6 @@ +import hashlib +import io +import zipfile from pathlib import Path from typing import Any @@ -6,6 +9,8 @@ import pytest from nanobot.webui.skills_marketplace import ( SkillsMarketplaceError, + _valid_skillhub_download_url, + _validated_skillhub_entries, install_marketplace_skill, marketplace_skill_trends, search_marketplace_skills, @@ -60,7 +65,11 @@ async def test_search_marketplace_skills_filters_and_marks_installed( "nanobot.webui.skills_marketplace.skills_install_supported", lambda: True, ) - payload = await search_marketplace_skills(" react testing ", tmp_path) + payload = await search_marketplace_skills( + " react testing ", + tmp_path, + provider="skills_sh", + ) assert seen == { "url": "https://skills.sh/api/search", @@ -68,6 +77,7 @@ async def test_search_marketplace_skills_filters_and_marks_installed( } assert payload == { "query": "react testing", + "provider": "skills_sh", "install_supported": True, "skills": [ { @@ -75,14 +85,92 @@ async def test_search_marketplace_skills_filters_and_marks_installed( "skill_id": "react-testing", "name": "React Testing", "source": "acme/agent-skills", + "provider": "skills_sh", "installs": 42, "url": "https://skills.sh/acme/agent-skills/react-testing", "installed": True, + "install_supported": True, + "metric": "installs_total", } ], } +@pytest.mark.asyncio +async def test_search_skillhub_skills_normalizes_provider_metadata( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + seen: dict[str, Any] = {} + + class FakeResponse: + def raise_for_status(self) -> None: + pass + + def json(self) -> dict[str, Any]: + return { + "results": [ + { + "slug": "ima-skills", + "name": "ima-skills", + "namespace": {"handle": "tencent-adm"}, + "source": "enterprise", + "version": "1.1.8", + "installs": 11831, + "downloads": 142525, + "publisher": {"verified": True}, + "labels": {"requires_api_key": "true"}, + } + ] + } + + class FakeClient: + async def __aenter__(self) -> "FakeClient": + return self + + async def __aexit__(self, *_args: object) -> None: + pass + + async def get(self, url: str, *, params: dict[str, object]) -> FakeResponse: + seen.update(url=url, params=params) + return FakeResponse() + + monkeypatch.setattr( + "nanobot.webui.skills_marketplace.httpx.AsyncClient", + lambda **_kwargs: FakeClient(), + ) + + payload = await search_marketplace_skills( + " ima ", + tmp_path, + provider="skillhub", + ) + + assert seen == { + "url": "https://api.skillhub.cn/api/v1/search", + "params": {"q": "ima", "limit": 20}, + } + assert payload["provider"] == "skillhub" + assert payload["skills"] == [ + { + "id": "skillhub:ima-skills", + "skill_id": "ima-skills", + "name": "ima-skills", + "source": "@tencent-adm/ima-skills", + "provider": "skillhub", + "installs": 11831, + "downloads": 142525, + "url": "https://skillhub.cn/tencent-adm/ima-skills", + "installed": False, + "install_supported": True, + "metric": "installs_total", + "version": "1.1.8", + "verified": True, + "requires_api_key": True, + } + ] + + @pytest.mark.asyncio async def test_trending_marketplace_skills_diversifies_sources_and_keeps_rank( tmp_path: Path, @@ -131,7 +219,7 @@ async def test_trending_marketplace_skills_diversifies_sources_and_keeps_rank( "nanobot.webui.skills_marketplace.httpx.AsyncClient", lambda **_kwargs: FakeClient(), ) - payload = await trending_marketplace_skills(tmp_path) + payload = await trending_marketplace_skills(tmp_path, provider="skills_sh") assert payload["period"] == "24h" assert [(skill["name"], skill["rank"]) for skill in payload["skills"]] == [ @@ -145,7 +233,7 @@ async def test_marketplace_skill_trends_returns_history_separately( monkeypatch: pytest.MonkeyPatch, ) -> None: class FakeResponse: - text = r'' + text = r"" def raise_for_status(self) -> None: pass @@ -175,11 +263,13 @@ async def test_marketplace_skill_trends_returns_history_separately( weekly_installs, ) - assert await marketplace_skill_trends([ - "acme/skills/first", - "other/skills/second", - "invalid", - ]) == { + assert await marketplace_skill_trends( + [ + "acme/skills/first", + "other/skills/second", + "invalid", + ] + ) == { "trends": { "acme/skills/first": [2, 4, 3, 8], "other/skills/second": [3, 5, 8, 13], @@ -208,7 +298,7 @@ async def test_search_marketplace_skills_returns_safe_upstream_error( ) with pytest.raises(SkillsMarketplaceError) as exc_info: - await search_marketplace_skills("react", tmp_path) + await search_marketplace_skills("react", tmp_path, provider="skills_sh") assert exc_info.value.status == 502 assert exc_info.value.message == "skills.sh search is temporarily unavailable" @@ -277,6 +367,152 @@ async def test_install_marketplace_skill_uses_official_cli_and_workspace( assert seen["env"]["DISABLE_TELEMETRY"] == "1" +@pytest.mark.asyncio +async def test_install_skillhub_skill_checks_fingerprint_and_extracts_safely( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + archive_buffer = io.BytesIO() + skill_content = b"---\nname: ima-skills\ndescription: Tencent knowledge skill.\n---\n" + with zipfile.ZipFile(archive_buffer, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr("SKILL.md", skill_content) + archive.writestr("_meta.json", b'{"version":"1.1.8"}') + archive_bytes = archive_buffer.getvalue() + file_hash = hashlib.sha256(skill_content).hexdigest() + content_hash = hashlib.sha256(f"SKILL.md:{file_hash}\n".encode()).hexdigest() + + class FakeResponse: + def __init__( + self, + *, + payload: dict[str, Any] | None = None, + status_code: int = 200, + headers: dict[str, str] | None = None, + content: bytes = b"", + ) -> None: + self.payload = payload or {} + self.status_code = status_code + self.headers = headers or {} + self.content = content + + def raise_for_status(self) -> None: + if self.status_code >= 400: + raise httpx.HTTPStatusError( + "failed", + request=httpx.Request("GET", "https://example.com"), + response=httpx.Response(self.status_code), + ) + + def json(self) -> dict[str, Any]: + return self.payload + + async def __aenter__(self) -> "FakeResponse": + return self + + async def __aexit__(self, *_args: object) -> None: + pass + + async def aiter_bytes(self): + yield self.content[:12] + yield self.content[12:] + + class FakeClient: + async def __aenter__(self) -> "FakeClient": + return self + + async def __aexit__(self, *_args: object) -> None: + pass + + async def get( + self, + url: str, + *, + params: dict[str, str] | None = None, + ) -> FakeResponse: + if url.endswith("/signature"): + return FakeResponse(payload={"signed": True, "content_hash": content_hash}) + assert url == "https://api.skillhub.cn/api/v1/download" + assert params == {"slug": "ima-skills", "version": "1.1.8"} + return FakeResponse( + status_code=302, + headers={ + "location": ( + "https://skillhub-1388575217.cos.accelerate.myqcloud.com/" + "skills/ima-skills.zip" + ) + }, + ) + + def stream( + self, + method: str, + url: str, + *, + headers: dict[str, str], + ) -> FakeResponse: + assert method == "GET" + assert url.endswith("/skills/ima-skills.zip") + assert "application/zip" in headers["Accept"] + return FakeResponse( + headers={"content-length": str(len(archive_bytes))}, + content=archive_bytes, + ) + + monkeypatch.setattr( + "nanobot.webui.skills_marketplace.httpx.AsyncClient", + lambda **_kwargs: FakeClient(), + ) + + result = await install_marketplace_skill( + "", + "ima-skills", + tmp_path, + provider="skillhub", + version="1.1.8", + ) + + assert result == { + "installed": True, + "already_installed": False, + "name": "ima-skills", + "provider": "skillhub", + "version": "1.1.8", + "verified": True, + } + assert (tmp_path / "skills" / "ima-skills" / "SKILL.md").read_bytes() == skill_content + + +@pytest.mark.parametrize( + ("url", "valid"), + [ + ("https://skillhub.cos.myqcloud.com/skills/example.zip", True), + ("https://skillhub.cos.myqcloud.com:443/skills/example.zip", True), + ("http://skillhub.cos.myqcloud.com/skills/example.zip", False), + ("https://myqcloud.com/skills/example.zip", False), + ("https://skillhub.cos.myqcloud.com.evil.example/skill.zip", False), + ("https://user@skillhub.cos.myqcloud.com/skill.zip", False), + ("https://skillhub.cos.myqcloud.com:not-a-port/skill.zip", False), + ], +) +def test_skillhub_download_url_allows_only_pinned_cloud_hosts( + url: str, + valid: bool, +) -> None: + assert _valid_skillhub_download_url(url) is valid + + +def test_skillhub_archive_rejects_path_traversal() -> None: + archive_buffer = io.BytesIO() + with zipfile.ZipFile(archive_buffer, "w") as archive: + archive.writestr("SKILL.md", "---\nname: safe\n---\n") + archive.writestr("../outside.sh", "#!/bin/sh\n") + archive_buffer.seek(0) + + with zipfile.ZipFile(archive_buffer) as archive: + with pytest.raises(SkillsMarketplaceError, match="unsafe path"): + _validated_skillhub_entries(archive) + + @pytest.mark.asyncio async def test_install_marketplace_skill_is_idempotent( tmp_path: Path, diff --git a/webui/src/components/settings/SkillsMarketplace.tsx b/webui/src/components/settings/SkillsMarketplace.tsx index 1977768eb..5ab84a174 100644 --- a/webui/src/components/settings/SkillsMarketplace.tsx +++ b/webui/src/components/settings/SkillsMarketplace.tsx @@ -28,7 +28,11 @@ import { searchMarketplaceSkills, } from "@/lib/api"; import { notifySkillsChanged } from "@/lib/skill-events"; -import type { MarketplaceSkillSummary, SkillSummary } from "@/lib/types"; +import type { + MarketplaceProvider, + MarketplaceSkillSummary, + SkillSummary, +} from "@/lib/types"; import { cn } from "@/lib/utils"; import { useClient } from "@/providers/ClientProvider"; @@ -42,7 +46,7 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS const [loading, setLoading] = useState(false); const [trendingLoading, setTrendingLoading] = useState(true); const [error, setError] = useState(""); - const [installSupported, setInstallSupported] = useState(null); + const [provider, setProvider] = useState("all"); const [selected, setSelected] = useState(null); const [installing, setInstalling] = useState(""); const installedNames = useMemo( @@ -53,11 +57,10 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS useEffect(() => { let cancelled = false; setTrendingLoading(true); - fetchTrendingMarketplaceSkills(token) + fetchTrendingMarketplaceSkills(token, provider) .then((payload) => { if (cancelled) return; setTrending(payload.skills); - setInstallSupported(payload.install_supported); }) .catch(() => { if (!cancelled) setTrending([]); @@ -68,11 +71,13 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS return () => { cancelled = true; }; - }, [token]); + }, [provider, token]); useEffect(() => { const skills = query.trim().length < 2 ? trending : results; - const unresolved = skills.filter((skill) => !(skill.id in trends)); + const unresolved = skills.filter( + (skill) => skill.provider === "skills_sh" && !(skill.id in trends), + ); if (!unresolved.length) return; let cancelled = false; @@ -101,11 +106,10 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS const timer = window.setTimeout(() => { setLoading(true); setError(""); - searchMarketplaceSkills(token, normalized) + searchMarketplaceSkills(token, normalized, provider) .then((payload) => { if (cancelled) return; setResults(payload.skills); - setInstallSupported(payload.install_supported); }) .catch((reason: unknown) => { if (cancelled) return; @@ -127,14 +131,20 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS cancelled = true; window.clearTimeout(timer); }; - }, [query, t, token]); + }, [provider, query, t, token]); const install = async (skill: MarketplaceSkillSummary) => { setSelected(null); - setInstalling(skill.skill_id); + setInstalling(skill.id); setError(""); try { - const payload = await installMarketplaceSkill(token, skill.source, skill.skill_id); + const payload = await installMarketplaceSkill( + token, + skill.provider, + skill.source, + skill.skill_id, + skill.version, + ); notifySkillsChanged(payload); setResults((current) => current.map((item) => @@ -161,33 +171,36 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS return (
-
- - setQuery(event.target.value)} - placeholder={t("settings.skills.marketplaceSearchPlaceholder", { - defaultValue: "Search skills.sh", - })} - aria-label={t("settings.skills.marketplaceSearchLabel", { - defaultValue: "Search skills.sh", - })} - className="h-11 rounded-[14px] bg-settings-surface pl-9" - /> - {loading ? ( - +
+ + setQuery(event.target.value)} + placeholder={t("settings.skills.marketplaceSearchPlaceholder", { + defaultValue: "Search skills", })} - > - - - ) : null} + aria-label={t("settings.skills.marketplaceSearchLabel", { + defaultValue: "Search skills", + })} + className="h-11 rounded-[14px] bg-settings-surface pl-9" + /> + {loading ? ( + + + + ) : null} +
+
{error ? ( @@ -198,22 +211,22 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS {query.trim().length < 2 ? (
-
-
-

- {t("settings.skills.marketplaceTrendingTitle", { - defaultValue: "Trending today", - })} -

-

- {t("settings.skills.marketplaceTrendingDescription", { - defaultValue: - "Most installed across sources in 24h · curves show the 8-week trend", - })} -

-
+
+
+

+ {t("settings.skills.marketplaceTrendingTitle", { + defaultValue: "Trending by marketplace", + })} +

+

+ {t("settings.skills.marketplaceTrendingDescription", { + defaultValue: "Each marketplace keeps its own ranking and install metrics.", + })} +

+
+ {provider !== "all" ? ( + ) : null} +
+ {trendingLoading ? ( + + ) : trending.length ? ( + + ) : ( +
+ {t("settings.skills.marketplaceTrendingUnavailable", { + defaultValue: "Trending skills are temporarily unavailable.", + })}
- {trendingLoading ? ( - - ) : trending.length ? ( - - ) : ( -
- {t("settings.skills.marketplaceTrendingUnavailable", { - defaultValue: "Trending skills are temporarily unavailable.", - })} -
- )} + )}
) : !loading && results.length === 0 && !error ? (
@@ -251,13 +264,12 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS
) : (
-
@@ -284,13 +296,16 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS {t("settings.skills.marketplaceConfirmDescription", { source: selected?.source ?? "", + provider: selected ? providerLabel(selected.provider) : "", defaultValue: - "This third-party skill comes from {{source}} and may include instructions or executable scripts.", + "This third-party skill comes from {{provider}} ({{source}}) and may include instructions or executable scripts.", })} - - {selected?.source} - + + {selected ? : null} + {selected?.source} + {selected?.version ? v{selected.version} : null} + @@ -313,20 +328,118 @@ export function SkillsMarketplace({ installedSkills }: { installedSkills: SkillS ); } +function ProviderFilter({ + value, + onChange, +}: { + value: MarketplaceProvider; + onChange: (provider: MarketplaceProvider) => void; +}) { + const { t } = useTranslation(); + const providers: MarketplaceProvider[] = ["all", "skills_sh", "skillhub"]; + return ( +
+ {providers.map((provider) => ( + + ))} +
+ ); +} + +function MarketplaceSkillGroups({ + skills, + installedNames, + installing, + trends, + grouped, + onSelect, +}: { + skills: MarketplaceSkillSummary[]; + installedNames: Set; + installing: string; + trends: Record; + grouped: boolean; + onSelect: (skill: MarketplaceSkillSummary) => void; +}) { + const providers: Array> = [ + "skills_sh", + "skillhub", + ]; + if (!grouped) { + return ( + + ); + } + return ( +
+ {providers.map((provider) => { + const providerSkills = skills.filter((skill) => skill.provider === provider); + if (!providerSkills.length) return null; + return ( +
+
+ + + + +
+ +
+ ); + })} +
+ ); +} + function MarketplaceSkillList({ skills, installedNames, installing, - installSupported, - metric, trends, onSelect, }: { skills: MarketplaceSkillSummary[]; installedNames: Set; installing: string; - installSupported: boolean | null; - metric: "total" | "24h"; trends: Record; onSelect: (skill: MarketplaceSkillSummary) => void; }) { @@ -337,10 +450,8 @@ function MarketplaceSkillList({ key={skill.id} skill={skill} installed={skill.installed || installedNames.has(skill.skill_id)} - isInstalling={installing === skill.skill_id} + isInstalling={installing === skill.id} installBusy={Boolean(installing)} - installSupported={installSupported} - metric={metric} trend={trends[skill.id]} onSelect={onSelect} /> @@ -354,8 +465,6 @@ function MarketplaceSkillRow({ installed, isInstalling, installBusy, - installSupported, - metric, trend, onSelect, }: { @@ -363,8 +472,6 @@ function MarketplaceSkillRow({ installed: boolean; isInstalling: boolean; installBusy: boolean; - installSupported: boolean | null; - metric: "total" | "24h"; trend?: number[]; onSelect: (skill: MarketplaceSkillSummary) => void; }) { @@ -388,17 +495,20 @@ function MarketplaceSkillRow({ rel="noreferrer" aria-label={t("settings.skills.marketplaceOpen", { name: skill.name, - defaultValue: "Open {{name}} on skills.sh", + provider: providerLabel(skill.provider), + defaultValue: "Open {{name}} on {{provider}}", })} className="inline-flex h-7 w-7 shrink-0 items-center justify-center rounded-full text-muted-foreground transition-colors hover:bg-muted hover:text-foreground" > -

+

+ {skill.source} - · - {metric === "24h" + {skill.version ? · v{skill.version} : null} + · + {skill.metric === "installs_24h" ? t("settings.skills.marketplaceInstalls24h", { count: skill.installs, formattedCount: skill.installs.toLocaleString(), @@ -409,21 +519,28 @@ function MarketplaceSkillRow({ formattedCount: skill.installs.toLocaleString(), defaultValue: "{{formattedCount}} installs", })} -

+
- + {skill.provider === "skills_sh" ? : null}