diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index dd0aae4686..8fdb09e715 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -737,7 +737,7 @@ def do_install(identifier: str, category: str = "", force: bool = False, bundle.trust_level, "invalid_path", str(exc)) return from tools.skills_hub import SKILLS_DIR - c.print(f"[bold green]Installed:[/] {install_dir.relative_to(SKILLS_DIR)}") + c.print(f"[bold green]Installed:[/] {install_dir.resolve().relative_to(Path(SKILLS_DIR).resolve()).as_posix()}") c.print(f"[dim]Files: {', '.join(bundle.files.keys())}[/]\n") # Blueprint detection: if the installed skill declares a diff --git a/tests/tools/test_skill_bundle_provenance.py b/tests/tools/test_skill_bundle_provenance.py index 4b2960318f..379a24b708 100644 --- a/tests/tools/test_skill_bundle_provenance.py +++ b/tests/tools/test_skill_bundle_provenance.py @@ -2,6 +2,7 @@ import json import subprocess +import sys import threading from functools import partial from http.server import SimpleHTTPRequestHandler, ThreadingHTTPServer @@ -165,6 +166,69 @@ def test_real_temp_repo_and_home_install_e2e(served_repo, monkeypatch, tmp_path) assert "Scan provenance: fresh" in sink.getvalue() +def _make_skills_redirect(link: Path, target: Path) -> bool: + """Make *link* a directory redirect (Windows junction or POSIX symlink) + pointing at *target*. Junctions need no admin rights, unlike symlinks.""" + if sys.platform == "win32": + try: + subprocess.run( + ["cmd", "/c", "mklink", "/J", str(link), str(target)], + check=True, + capture_output=True, + ) + return True + except (subprocess.CalledProcessError, OSError): + return False + try: + link.symlink_to(target, target_is_directory=True) + return True + except OSError: + return False + + +def test_install_with_junctioned_skills_dir(served_repo, monkeypatch, tmp_path): + """#86971: install must not mix resolved and unresolved paths when the + skills directory is a junction/symlink redirect. + + install_dir is resolved by _resolve_lock_install_path (following the + redirect), so relative_to() must receive the resolved skills root or it + raises ValueError after the files have already been moved, leaving a lock + entry without a content_hash (which then poisons 'hermes skills check'). + """ + from hermes_cli.skills_hub import do_install + import tools.skills_hub as hub + + _repo, url = served_repo + home = tmp_path / "home" + home.mkdir() + real_skills = tmp_path / "real-skills" + real_skills.mkdir() + skills_link = home / "skills" + if not _make_skills_redirect(skills_link, real_skills): + pytest.skip("Cannot create a junction/symlink in this environment") + + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.setattr("tools.skills_hub.is_safe_url", lambda _url: True) + monkeypatch.setattr("tools.skills_hub.check_website_access", lambda _url: None) + monkeypatch.setattr(hub, "create_source_router", lambda auth=None: [UrlSource()]) + + sink = StringIO() + do_install(url, console=Console(file=sink, force_terminal=False), skip_confirm=True) + + # Files landed in the real target, reached through the junction. + installed = real_skills / "demo-bundle" + assert (installed / "SKILL.md").is_file() + assert (installed / "references" / "guide.md").read_text() == "safe guide\n" + # Lock entry got a valid relative install_path AND the content hash — the + # record_install call the pre-fix ValueError used to skip. + entry = json.loads((home / "skills" / ".hub" / "lock.json").read_text())["installed"]["demo-bundle"] + assert entry["install_path"] == "demo-bundle" + assert entry["content_hash"].startswith("sha256:") + # The post-install "Installed:" line (relative_to on the display path) + # renders instead of raising. + assert "Installed:" in sink.getvalue() + + def test_bundled_optional_source_still_includes_support_files(tmp_path, monkeypatch): from tools.skills_hub import OptionalSkillSource diff --git a/tools/skills_hub.py b/tools/skills_hub.py index 51340e6479..ebd00d7334 100644 --- a/tools/skills_hub.py +++ b/tools/skills_hub.py @@ -3394,7 +3394,7 @@ class OptionalSkillSource(SkillSource): name=name, files=files, source="official", - identifier=f"official/{skill_dir.relative_to(self._optional_dir)}", + identifier=f"official/{skill_dir.resolve().relative_to(self._optional_dir.resolve()).as_posix()}", trust_level="builtin", ) @@ -3999,7 +3999,7 @@ def install_from_quarantine( trust_level=bundle.trust_level, scan_verdict=scan_result.verdict, skill_hash=content_hash(install_dir), - install_path=str(install_dir.relative_to(_skills_dir())), + install_path=install_dir.resolve().relative_to(_skills_dir().resolve()).as_posix(), files=list(bundle.files.keys()), metadata=bundle.metadata, scan_provenance=scan_provenance or getattr(scan_result, "scan_provenance", None),