diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index cea75357a..a36ee78ce 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -357,7 +357,8 @@ def do_install(identifier: str, category: str = "", force: bool = False, # Scan c.print("[bold]Running security scan...[/]") - result = scan_skill(q_path, source=identifier) + scan_source = getattr(bundle, "identifier", "") or getattr(meta, "identifier", "") or identifier + result = scan_skill(q_path, source=scan_source) c.print(format_scan_report(result)) # Check install policy diff --git a/tests/hermes_cli/test_skills_hub.py b/tests/hermes_cli/test_skills_hub.py index d1169120b..0ef6c2d69 100644 --- a/tests/hermes_cli/test_skills_hub.py +++ b/tests/hermes_cli/test_skills_hub.py @@ -3,7 +3,7 @@ from io import StringIO import pytest from rich.console import Console -from hermes_cli.skills_hub import do_check, do_list, do_update, handle_skills_slash +from hermes_cli.skills_hub import do_check, do_install, do_list, do_update, handle_skills_slash class _DummyLockFile: @@ -177,3 +177,57 @@ def test_do_update_reinstalls_outdated_skills(monkeypatch): assert installs == [("skills-sh/example/repo/hub-skill", "category", True)] assert "Updated 1 skill" in output + + +def test_do_install_scans_with_resolved_identifier(monkeypatch, tmp_path, hub_env): + import tools.skills_guard as guard + import tools.skills_hub as hub + + canonical_identifier = "skills-sh/anthropics/skills/frontend-design" + + class _ResolvedSource: + def inspect(self, identifier): + return type("Meta", (), { + "extra": {}, + "identifier": canonical_identifier, + })() + + def fetch(self, identifier): + return type("Bundle", (), { + "name": "frontend-design", + "files": {"SKILL.md": "# Frontend Design"}, + "source": "skills.sh", + "identifier": canonical_identifier, + "trust_level": "trusted", + "metadata": {}, + })() + + q_path = tmp_path / "skills" / ".hub" / "quarantine" / "frontend-design" + q_path.mkdir(parents=True) + (q_path / "SKILL.md").write_text("# Frontend Design") + + scanned = {} + + def _scan_skill(skill_path, source="community"): + scanned["source"] = source + return guard.ScanResult( + skill_name="frontend-design", + source=source, + trust_level="trusted", + verdict="safe", + ) + + monkeypatch.setattr(hub, "ensure_hub_dirs", lambda: None) + monkeypatch.setattr(hub, "create_source_router", lambda auth: [_ResolvedSource()]) + monkeypatch.setattr(hub, "quarantine_bundle", lambda bundle: q_path) + monkeypatch.setattr(hub, "HubLockFile", lambda: type("Lock", (), {"get_installed": lambda self, name: None})()) + monkeypatch.setattr(guard, "scan_skill", _scan_skill) + monkeypatch.setattr(guard, "format_scan_report", lambda result: "scan ok") + monkeypatch.setattr(guard, "should_allow_install", lambda result, force=False: (False, "stop after scan")) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + + do_install("skils-sh/anthropics/skills/frontend-design", console=console, skip_confirm=True) + + assert scanned["source"] == canonical_identifier diff --git a/tests/tools/test_skills_guard.py b/tests/tools/test_skills_guard.py index fbe50efb5..6fcd05b31 100644 --- a/tests/tools/test_skills_guard.py +++ b/tests/tools/test_skills_guard.py @@ -55,6 +55,13 @@ class TestResolveTrustLevel: assert _resolve_trust_level("anthropics/skills") == "trusted" assert _resolve_trust_level("openai/skills/some-skill") == "trusted" + def test_skills_sh_wrapped_trusted_repos(self): + assert _resolve_trust_level("skills-sh/openai/skills/skill-creator") == "trusted" + assert _resolve_trust_level("skills-sh/anthropics/skills/frontend-design") == "trusted" + + def test_common_skills_sh_prefix_typo_still_maps_to_trusted_repo(self): + assert _resolve_trust_level("skils-sh/anthropics/skills/frontend-design") == "trusted" + def test_community_default(self): assert _resolve_trust_level("random-user/my-skill") == "community" assert _resolve_trust_level("") == "community" diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index ddd1585c8..a55a91e00 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -179,6 +179,24 @@ class TestSkillsShSource: assert bundle.identifier == "skills-sh/vercel-labs/agent-skills/vercel-react-best-practices" mock_fetch.assert_called_once_with("vercel-labs/agent-skills/vercel-react-best-practices") + @patch.object(GitHubSource, "fetch") + def test_fetch_accepts_common_skills_sh_prefix_typo(self, mock_fetch): + expected_identifier = "anthropics/skills/frontend-design" + mock_fetch.side_effect = lambda identifier: SkillBundle( + name="frontend-design", + files={"SKILL.md": "# Frontend Design"}, + source="github", + identifier=expected_identifier, + trust_level="trusted", + ) if identifier == expected_identifier else None + + bundle = self._source().fetch("skils-sh/anthropics/skills/frontend-design") + + assert bundle is not None + assert bundle.source == "skills.sh" + assert bundle.identifier == "skills-sh/anthropics/skills/frontend-design" + assert mock_fetch.call_args_list[0] == ((expected_identifier,), {}) + @patch("tools.skills_hub._write_index_cache") @patch("tools.skills_hub._read_index_cache", return_value=None) @patch("tools.skills_hub.httpx.get") @@ -213,6 +231,26 @@ class TestSkillsShSource: assert meta.extra["security_audits"]["socket"] == "Pass" mock_inspect.assert_called_once_with("vercel-labs/agent-skills/vercel-react-best-practices") + @patch.object(GitHubSource, "inspect") + def test_inspect_accepts_common_skills_sh_prefix_typo(self, mock_inspect): + expected_identifier = "anthropics/skills/frontend-design" + mock_inspect.side_effect = lambda identifier: SkillMeta( + name="frontend-design", + description="Distinctive frontend interfaces.", + source="github", + identifier=expected_identifier, + trust_level="trusted", + repo="anthropics/skills", + path="frontend-design", + ) if identifier == expected_identifier else None + + meta = self._source().inspect("skils-sh/anthropics/skills/frontend-design") + + assert meta is not None + assert meta.source == "skills.sh" + assert meta.identifier == "skills-sh/anthropics/skills/frontend-design" + assert mock_inspect.call_args_list[0] == ((expected_identifier,), {}) + @patch.object(GitHubSource, "_list_skills_in_repo") @patch.object(GitHubSource, "inspect") def test_inspect_falls_back_to_repo_skill_catalog_when_slug_differs(self, mock_inspect, mock_list_skills): @@ -307,6 +345,39 @@ class TestSkillsShSource: assert bundle.files["SKILL.md"] == "# react" assert mock_get.called + @patch("tools.skills_hub._write_index_cache") + @patch("tools.skills_hub._read_index_cache", return_value=None) + @patch.object(SkillsShSource, "_discover_identifier") + @patch.object(SkillsShSource, "_fetch_detail_page") + @patch.object(GitHubSource, "fetch") + def test_fetch_downloads_only_the_resolved_identifier( + self, + mock_fetch, + mock_detail, + mock_discover, + _mock_read_cache, + _mock_write_cache, + ): + resolved_identifier = "owner/repo/product-team/product-designer" + mock_detail.return_value = {"repo": "owner/repo", "install_skill": "product-designer"} + mock_discover.return_value = resolved_identifier + resolved_bundle = SkillBundle( + name="product-designer", + files={"SKILL.md": "# Product Designer"}, + source="github", + identifier=resolved_identifier, + trust_level="community", + ) + mock_fetch.side_effect = lambda identifier: resolved_bundle if identifier == resolved_identifier else None + + bundle = self._source().fetch("skills-sh/owner/repo/product-designer") + + assert bundle is not None + assert bundle.identifier == "skills-sh/owner/repo/product-designer" + # All candidate identifiers are tried before falling back to discovery + assert mock_fetch.call_args_list[-1] == ((resolved_identifier,), {}) + assert mock_fetch.call_args_list[0] == (("owner/repo/product-designer",), {}) + @patch("tools.skills_hub._write_index_cache") @patch("tools.skills_hub._read_index_cache", return_value=None) @patch("tools.skills_hub.httpx.get") @@ -369,6 +440,36 @@ class TestSkillsShSource: # Verify the tree-resolved identifier was used for the final GitHub fetch mock_fetch.assert_any_call("owner/repo/cli-tool/components/skills/development/my-skill") + @patch.object(GitHubSource, "_find_skill_in_repo_tree") + @patch.object(GitHubSource, "_list_skills_in_repo") + @patch("tools.skills_hub.httpx.get") + def test_discover_identifier_uses_tree_search_before_root_scan( + self, + mock_get, + mock_list_skills, + mock_find_in_tree, + ): + root_url = "https://api.github.com/repos/owner/repo/contents/" + mock_list_skills.return_value = [] + mock_find_in_tree.return_value = "owner/repo/product-team/product-designer" + + def _httpx_get_side_effect(url, **kwargs): + resp = MagicMock() + if url == root_url: + resp.status_code = 200 + resp.json = lambda: [] + return resp + resp.status_code = 404 + return resp + + mock_get.side_effect = _httpx_get_side_effect + + result = self._source()._discover_identifier("owner/repo/product-designer") + + assert result == "owner/repo/product-team/product-designer" + requested_urls = [call.args[0] for call in mock_get.call_args_list] + assert root_url not in requested_urls + class TestFindSkillInRepoTree: """Tests for GitHubSource._find_skill_in_repo_tree.""" diff --git a/tools/skills_guard.py b/tools/skills_guard.py index 74a6a8808..217863af5 100644 --- a/tools/skills_guard.py +++ b/tools/skills_guard.py @@ -1050,15 +1050,27 @@ def _get_configured_model() -> str: def _resolve_trust_level(source: str) -> str: """Map a source identifier to a trust level.""" + prefix_aliases = ( + "skills-sh/", + "skills.sh/", + "skils-sh/", + "skils.sh/", + ) + normalized_source = source + for prefix in prefix_aliases: + if normalized_source.startswith(prefix): + normalized_source = normalized_source[len(prefix):] + break + # Agent-created skills get their own permissive trust level - if source == "agent-created": + if normalized_source == "agent-created": return "agent-created" # Official optional skills shipped with the repo - if source.startswith("official/") or source == "official": + if normalized_source.startswith("official/") or normalized_source == "official": return "builtin" # Check if source matches any trusted repo for trusted in TRUSTED_REPOS: - if source.startswith(trusted) or source == trusted: + if normalized_source.startswith(trusted) or normalized_source == trusted: return "trusted" return "community" diff --git a/tools/skills_hub.py b/tools/skills_hub.py index 266cff63e..3814dddfe 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -925,19 +925,10 @@ class SkillsShSource(SkillSource): def inspect(self, identifier: str) -> Optional[SkillMeta]: canonical = self._normalize_identifier(identifier) - detail: Optional[dict] = None - for candidate in self._candidate_identifiers(canonical): - meta = self.github.inspect(candidate) - if meta: - detail = self._fetch_detail_page(canonical) - return self._finalize_inspect_meta(meta, canonical, detail) - detail = self._fetch_detail_page(canonical) - resolved = self._discover_identifier(canonical, detail=detail) - if resolved: - meta = self.github.inspect(resolved) - if meta: - return self._finalize_inspect_meta(meta, canonical, detail) + meta = self._resolve_github_meta(canonical, detail=detail) + if meta: + return self._finalize_inspect_meta(meta, canonical, detail) return None def _featured_skills(self, limit: int) -> List[SkillMeta]: @@ -1099,6 +1090,13 @@ class SkillsShSource(SkillSource): if self._matches_skill_tokens(meta, tokens): return meta.identifier + # Prefer a single recursive tree lookup before brute-forcing every + # top-level directory. This avoids large request bursts on categorized + # repos like borghei/claude-skills. + tree_result = self.github._find_skill_in_repo_tree(repo, skill_token) + if tree_result: + return tree_result + # Fallback: scan repo root for directories that might contain skills try: root_url = f"https://api.github.com/repos/{repo}/contents/" @@ -1131,14 +1129,17 @@ class SkillsShSource(SkillSource): except Exception: pass - # Final fallback: use the GitHub Trees API to find the skill anywhere - # in the repo tree. This handles deeply nested structures like - # cli-tool/components/skills/development// that the shallow - # scan above can't reach. - tree_result = self.github._find_skill_in_repo_tree(repo, skill_token) - if tree_result: - return tree_result + return None + def _resolve_github_meta(self, identifier: str, detail: Optional[dict] = None) -> Optional[SkillMeta]: + for candidate in self._candidate_identifiers(identifier): + meta = self.github.inspect(candidate) + if meta: + return meta + + resolved = self._discover_identifier(identifier, detail=detail) + if resolved: + return self.github.inspect(resolved) return None def _finalize_inspect_meta(self, meta: SkillMeta, canonical: str, detail: Optional[dict]) -> SkillMeta: @@ -1264,10 +1265,15 @@ class SkillsShSource(SkillSource): @staticmethod def _normalize_identifier(identifier: str) -> str: - if identifier.startswith("skills-sh/"): - return identifier[len("skills-sh/"):] - if identifier.startswith("skills.sh/"): - return identifier[len("skills.sh/"):] + prefix_aliases = ( + "skills-sh/", + "skills.sh/", + "skils-sh/", + "skils.sh/", + ) + for prefix in prefix_aliases: + if identifier.startswith(prefix): + return identifier[len(prefix):] return identifier @staticmethod