fix(skills): never change a skill's source registry on update

`hermes skills update` could silently replace a same-named skill from a
DIFFERENT registry — deleting the user's files and rewriting the lockfile's
recorded provenance. Two defects on main:

- tools/skills_hub.py: check_for_skill_updates fell back to *all* sources
  (`... or sources`) when no adapter matched the recorded source, so any
  registry with a same-named skill could satisfy the fetch and be reported
  as an update — silently reassigning provenance. Now reports the entry as
  `unavailable` instead of cross-registry fallback.
- hermes_cli/skills_hub.py: do_update called do_install with a bare, slash-
  less identifier and no source constraint, letting _resolve_short_name fuzzy-
  match a same-named skill in another registry. do_install now takes an
  optional source_id pin that ABORTS (rather than falls back) when no adapter
  matches, and do_update forwards the lockfile's recorded source as that pin.

Includes regression tests reproducing the cross-registry hijack.

Salvaged from PR #72216 onto current main; re-attributed to the human
contributor.

Co-authored-by: menhguin <menhguin@users.noreply.github.com>

contributors/emails/menhguin@users.noreply.github.com

@@ -0,0 +1 @@
+menhguin

hermes_cli/skills_hub.py

@@ -502,7 +502,8 @@ 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,
-               name_override: str = "") -> None:
+               name_override: str = "",
+               source_id: Optional[str] = None) -> None:
     """Fetch, quarantine, scan, confirm, and install a skill.
 
     ``name_override`` lets non-interactive callers (slash commands, gateway,
@@ -511,10 +512,18 @@ def do_install(identifier: str, category: str = "", force: bool = False,
     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).
+
+    ``source_id`` pins resolution to a single source adapter (e.g. ``clawhub``).
+    Callers that already know a skill's provenance -- notably ``do_update``,
+    which reads it from the lockfile -- should pass it so a bare, slash-less
+    identifier cannot be fuzzy-resolved to a same-named skill in a different
+    registry. Skill names are not namespaced across registries, so an
+    unconstrained resolve can silently change a skill's provenance.
     """
     from tools.skills_hub import (
         GitHubAuth, create_source_router, ensure_hub_dirs,
         quarantine_bundle, install_from_quarantine, HubLockFile,
+        _source_matches,
     )
     from tools.skills_guard import scan_skill_cached, should_allow_install, format_scan_report
 
@@ -525,6 +534,18 @@ def do_install(identifier: str, category: str = "", force: bool = False,
     auth = GitHubAuth()
     sources = create_source_router(auth)
 
+    if source_id:
+        pinned = [src for src in sources if _source_matches(src, source_id)]
+        if pinned:
+            sources = pinned
+        else:
+            c.print(
+                f"[bold red]Error:[/] no source adapter for '{source_id}'. "
+                f"Refusing to resolve '{identifier}' against other registries "
+                f"(that would change the skill's provenance).\n"
+            )
+            return
+
     # If identifier looks like a short name (no slashes), resolve it via search
     if "/" not in identifier:
         identifier = _resolve_short_name(identifier, sources, c)
@@ -1058,7 +1079,20 @@ def do_update(name: Optional[str] = None, console: Optional[Console] = None) ->
         installed = lock.get_installed(entry["name"])
         category = _derive_category_from_install_path(installed.get("install_path", "")) if installed else ""
         c.print(f"[bold]Updating:[/] {entry['name']}")
-        do_install(entry["identifier"], category=category, force=True, console=c)
+        # Pin the update to the source registry recorded in the lockfile.
+        # Without this, a bare (slash-less) identifier such as "reddit" falls
+        # through to _resolve_short_name()'s fuzzy catalog search inside
+        # do_install, which can match a same-named skill in a DIFFERENT
+        # registry and install that instead -- overwriting the user's files
+        # and rewriting the lock's `source`. An update must never change a
+        # skill's provenance.
+        do_install(
+            entry["identifier"],
+            category=category,
+            force=True,
+            console=c,
+            source_id=entry.get("source", "") or None,
+        )
 
     c.print(f"[bold green]Updated {len(updates)} skill(s).[/]\n")
 

tests/hermes_cli/test_skills_hub.py

@@ -92,7 +92,7 @@ def _capture_update(monkeypatch, results) -> tuple[str, list[tuple[str, str, boo
     monkeypatch.setattr(hub, "HubLockFile", lambda: type("L", (), {
         "get_installed": lambda self, name: {"install_path": "category/" + name}
     })())
-    monkeypatch.setattr(cli_hub, "do_install", lambda identifier, category="", force=False, console=None: installs.append((identifier, category, force)))
+    monkeypatch.setattr(cli_hub, "do_install", lambda identifier, category="", force=False, console=None, source_id=None: installs.append((identifier, category, force)))
 
     do_update(console=console)
     return sink.getvalue(), installs
@@ -249,6 +249,123 @@ def test_do_update_reinstalls_outdated_skills(monkeypatch):
     assert "Updated 1 skill" in output
 
 
+# ---------------------------------------------------------------------------
+# Cross-registry hijack regression tests
+#
+# An update must never change a skill's source registry. Skill names are not
+# namespaced across registries, so an unconstrained name resolve can install a
+# different author's same-named skill over the user's files.
+# ---------------------------------------------------------------------------
+
+
+def test_do_update_pins_install_to_locked_source(monkeypatch):
+    """do_update must forward the lockfile's `source` to do_install.
+
+    Without the pin, a bare identifier like "reddit" reaches
+    _resolve_short_name()'s fuzzy catalog search inside do_install and can
+    resolve to a same-named skill in another registry.
+    """
+    import tools.skills_hub as hub
+    import hermes_cli.skills_hub as cli_hub
+
+    sink = StringIO()
+    console = Console(file=sink, force_terminal=False, color_system=None)
+    calls = []
+
+    monkeypatch.setattr(hub, "check_for_skill_updates", lambda **_kwargs: [
+        {"name": "reddit", "identifier": "reddit", "source": "clawhub",
+         "status": "update_available"},
+    ])
+    monkeypatch.setattr(hub, "HubLockFile", lambda: type("L", (), {
+        "get_installed": lambda self, name: {"install_path": "category/" + name}
+    })())
+    monkeypatch.setattr(
+        cli_hub, "do_install",
+        lambda identifier, category="", force=False, console=None, source_id=None:
+            calls.append({"identifier": identifier, "source_id": source_id}),
+    )
+
+    do_update(console=console)
+
+    assert calls == [{"identifier": "reddit", "source_id": "clawhub"}]
+
+
+def test_do_install_refuses_unknown_pinned_source(monkeypatch, hub_env):
+    """A pinned source with no matching adapter must abort, not fall back.
+
+    Falling back to the full source router is what allowed a foreign registry
+    to satisfy the install.
+    """
+    import tools.skills_hub as hub
+
+    sink = StringIO()
+    console = Console(file=sink, force_terminal=False, color_system=None)
+    resolved = []
+
+    monkeypatch.setattr(hub, "create_source_router", lambda auth=None: [])
+    monkeypatch.setattr(hub, "GitHubAuth", lambda: object())
+    monkeypatch.setattr(
+        "hermes_cli.skills_hub._resolve_short_name",
+        lambda name, sources, c: resolved.append(name) or "",
+    )
+
+    do_install("reddit", source_id="clawhub", console=console)
+
+    assert resolved == [], "must not attempt a fuzzy resolve when the pin is unsatisfiable"
+    assert "provenance" in sink.getvalue()
+
+
+def test_check_for_skill_updates_does_not_fall_back_across_registries():
+    """An entry whose source has no adapter reports `unavailable`.
+
+    Previously `candidate_sources ... or sources` fell back to every source, so
+    a same-named skill in another registry could satisfy the fetch and be
+    reported as this entry's update -- the step that preceded the overwrite.
+    The foreign source here returns a *valid* bundle with a different hash, so
+    the old code reports `update_available` (sourced from the wrong registry)
+    while the fixed code reports `unavailable`.
+    """
+    from tools.skills_hub import check_for_skill_updates
+
+    class _ForeignBundle:
+        name = "reddit"
+        files = {"SKILL.md": "# a different author's reddit skill"}
+        source = "skills.sh"
+        identifier = "skills-sh/someone-else/reddit"
+        trust_level = "community"
+        metadata: dict = {}
+
+    class _ForeignSource:
+        """skills-sh adapter; must NOT be consulted for a clawhub-locked entry."""
+
+        def source_id(self):
+            return "skills-sh"
+
+        def fetch(self, identifier):
+            return _ForeignBundle()
+
+        def inspect(self, identifier):
+            return _ForeignBundle()
+
+    lock = _DummyLockFile([
+        {"name": "reddit", "identifier": "reddit", "source": "clawhub",
+         "content_hash": "hash-of-the-clawhub-copy"},
+    ])
+
+    results = check_for_skill_updates(
+        lock=lock,  # type: ignore[arg-type]  # duck-typed double, matches _DummyLockFile usage above
+        sources=[_ForeignSource()],  # type: ignore[list-item]
+    )
+
+    assert len(results) == 1
+    assert results[0]["source"] == "clawhub", "provenance must be preserved"
+    assert results[0]["status"] == "unavailable", (
+        "a clawhub-locked skill must not be matched against a skills-sh bundle; "
+        "reporting update_available here is the cross-registry hijack"
+    )
+    assert "bundle" not in results[0], "must not carry a foreign registry's bundle"
+
+
 def test_handle_skills_slash_search_accepts_chatconsole_without_status_errors():
     results = [type("R", (), {
         "name": "kubernetes",

tools/skills_hub.py

@@ -3742,7 +3742,22 @@ def check_for_skill_updates(
     for entry in installed:
         identifier = entry.get("identifier", "")
         source_name = entry.get("source", "")
-        candidate_sources = [src for src in sources if _source_matches(src, source_name)] or sources
+        candidate_sources = [src for src in sources if _source_matches(src, source_name)]
+        if not candidate_sources:
+            # No adapter for the recorded source (e.g. a tap was removed, or the
+            # source was renamed upstream). Previously this fell back to *all*
+            # sources, which meant a same-named skill in a DIFFERENT registry
+            # could satisfy the fetch and be reported as an update for this
+            # entry -- silently reassigning provenance. Skill names are not
+            # namespaced across registries, so that fallback is unsafe by
+            # construction. Report unavailable instead and let the user decide.
+            results.append({
+                "name": entry.get("name", ""),
+                "identifier": identifier,
+                "source": source_name,
+                "status": "unavailable",
+            })
+            continue
 
         bundle = None
         for src in candidate_sources: