fix(skills): a SKILL.md link above the skill directory no longer makes the skill uninstallable
`_referenced_support_paths` returned None (bundle rejected) when a same-directory markdown link started with `..`. Such a link (`../../tools/REGISTRY.md` in a multi-skill repo) is prose: it is never fetched and never becomes a bundle path, so the refusal protected nothing while every skill that links a sibling doc failed with "files no longer exist upstream" / "Could not fetch from any source" — the reporter's five marketingskills names, which curl fetched fine. Skip the link with a warning instead; support-dir traversal (`references/../x`) is still rejected because that path IS written into the bundle.
This commit is contained in:
@@ -134,15 +134,23 @@ def test_same_dir_linked_siblings_are_fetched(served_repo, monkeypatch):
|
||||
assert "README.md" not in bundle.files
|
||||
|
||||
|
||||
def test_same_dir_traversal_link_is_rejected(monkeypatch):
|
||||
source = UrlSource()
|
||||
skill = (
|
||||
"---\nname: bad\ndescription: bad\n---\n"
|
||||
"[bad](./../outside-secret.md)\n"
|
||||
)
|
||||
monkeypatch.setattr(source, "_fetch_text", lambda _url: skill)
|
||||
def test_same_dir_link_outside_skill_dir_is_skipped_not_fatal(served_repo, monkeypatch):
|
||||
"""A repo-relative link above the skill directory is prose, not a bundle path (#115171).
|
||||
|
||||
assert source.fetch("https://example.com/bad/SKILL.md") is None
|
||||
Nothing is fetched for it, so it must not reject the bundle: the skill installs with
|
||||
SKILL.md and its real siblings, and the outside link is simply left dangling.
|
||||
"""
|
||||
repo, url = served_repo
|
||||
(repo / "DEFS.md").write_text("defs\n")
|
||||
(repo / "SKILL.md").write_text(SKILL_MD + "[registry](../../tools/REGISTRY.md) and [defs](./DEFS.md)\n")
|
||||
monkeypatch.setattr("tools.skills_hub.is_safe_url", lambda _url: True)
|
||||
monkeypatch.setattr("tools.skills_hub.check_website_access", lambda _url: None)
|
||||
|
||||
bundle = UrlSource().fetch(url)
|
||||
|
||||
assert bundle is not None
|
||||
assert "DEFS.md" in bundle.files
|
||||
assert not any(".." in name for name in bundle.files)
|
||||
|
||||
|
||||
def test_same_dir_link_without_extension_is_ignored(monkeypatch):
|
||||
|
||||
@@ -312,7 +312,13 @@ def _referenced_support_paths(skill_md: str) -> Optional[set[str]]:
|
||||
if not name or "://" in raw or raw.startswith(("mailto:", "#", "/")):
|
||||
continue
|
||||
if name.startswith(".."):
|
||||
return None
|
||||
# A repo-relative link to a doc outside the skill directory (``../../tools/REGISTRY.md``
|
||||
# in a multi-skill repo) is prose, never a bundle path: nothing is fetched or written for
|
||||
# it, so refusing the whole bundle protected nothing and made every skill that links a
|
||||
# sibling doc uninstallable with a misleading "files no longer exist upstream" (#115171).
|
||||
# The link is left dangling in the installed copy, like an absent support file.
|
||||
logger.warning("SKILL.md links outside the skill directory; installing without it: %s", raw)
|
||||
continue
|
||||
# Only unambiguous file links: an extension, no internal slash, never SKILL.md itself (casefolded —
|
||||
# a ``skill.md`` entry would collide with the bundle root on macOS/Windows; skipped, not merged).
|
||||
if ("/" in name or name.casefold() == "skill.md" or "." not in name.lstrip(".")
|
||||
|
||||
Reference in New Issue
Block a user