fix(packaging): make skill references bundle-safe (#125)

* fix(packaging): make skill references bundle-safe * test(packaging): normalize scanner paths portably * fix(packaging): bundle required runtime helpers
2026-08-19 20:31:56 -07:00
parent 0487290d5a
commit b132fa5d7a
8 changed files with 178 additions and 24 deletions
@@ -60,6 +60,88 @@ def main() -> int:
        if errors:
            raise AssertionError(f"non-reference links should be ignored: {errors}")

+        scripts = skill / "scripts"
+        assets = skill / "assets"
+        scripts.mkdir()
+        assets.mkdir()
+        required_files = sorted(verify.REQUIRED_PACKAGED_RUNTIME_FILES)
+        for target in required_files:
+            packaged_file = skill / target
+            packaged_file.parent.mkdir(parents=True, exist_ok=True)
+            packaged_file.write_text("# packaged\n", encoding="utf-8")
+        (assets / "example.html").write_text("<!doctype html>\n", encoding="utf-8")
+        required_mentions = ", ".join(f"`{target}`" for target in required_files)
+        packaged_markdown = f"""Use [the reference](references/present.md#section),
+{required_mentions}, and `assets/example.html`.
+From a repository checkout, run `python3 <repo-root>/scripts/verify-geometry.py <file>`.
+"""
+        extracted = verify.scanner_visible_support_references(packaged_markdown)
+        expected = sorted(
+            {"assets/example.html", "references/present.md", *required_files}
+        )
+        if extracted != expected:
+            raise AssertionError(f"strict-bundler references drifted: {extracted}")
+        errors = []
+        verify.check_packaged_support_references(errors, packaged_markdown, skill)
+        if errors:
+            raise AssertionError(f"valid packaged support references failed: {errors}")
+
+        for phantom in ("references/type-*.md", "references/type-<name>.md"):
+            errors = []
+            verify.check_packaged_support_references(
+                errors,
+                packaged_markdown + f"Load `{phantom}` before drawing.\n",
+                skill,
+            )
+            if len(errors) != 1 or "strict skill bundlers will abort installation" not in errors[0]:
+                raise AssertionError(
+                    f"scanner-visible placeholder was not rejected: {phantom!r}: {errors}"
+                )
+
+        errors = []
+        verify.check_packaged_support_references(
+            errors,
+            packaged_markdown + "See [unsafe](references/%2e%2e/secrets.md).\n",
+            skill,
+        )
+        expected = "SKILL.md exposes unsafe packaged support path 'references/../secrets.md'"
+        if errors != [expected]:
+            raise AssertionError(f"unsafe packaged reference was not rejected: {errors}")
+
+        errors = []
+        verify.check_packaged_support_references(
+            errors,
+            packaged_markdown + "See [unsafe](references/%2e%2e%5csecrets.md).\n",
+            skill,
+        )
+        if len(errors) != 1 or "unsafe packaged support path" not in errors[0]:
+            raise AssertionError(f"encoded Windows traversal was not rejected: {errors}")
+
+        actual_skill = verify.SKILL.read_text(encoding="utf-8")
+        actual_references = set(
+            verify.scanner_visible_support_references(actual_skill)
+        )
+        missing_runtime = verify.REQUIRED_PACKAGED_RUNTIME_FILES - actual_references
+        if missing_runtime:
+            raise AssertionError(
+                "actual SKILL.md omits required packaged runtime files: "
+                f"{sorted(missing_runtime)}"
+            )
+
+        missing_one = required_files[0]
+        errors = []
+        verify.check_packaged_support_references(
+            errors,
+            packaged_markdown.replace(f"`{missing_one}`", f"`{Path(missing_one).name}`"),
+            skill,
+        )
+        expected = (
+            f"SKILL.md does not expose required packaged runtime file {missing_one!r}; "
+            "strict skill bundlers will omit it"
+        )
+        if errors != [expected]:
+            raise AssertionError(f"omitted runtime helper was not reported: {errors}")
+
        root = Path(temp_dir) / "repo"
        profile_reference = root / "skills/diagram-design/references/profiles.md"
        command = root / "commands/profile.md"
@@ -181,7 +263,8 @@ diagram-design/
            raise AssertionError(f"missing Factory native path was not reported: {errors}")

    print(
-        "PASS: docs sync checks references, profile surfaces, and Factory install contract"
+        "PASS: docs sync checks references, strict-bundler packaging, profile surfaces, "
+        "and Factory install contract"
    )
    return 0

@@ -1,7 +1,7 @@
 #!/usr/bin/env python3
 """Verify that routing and browsing surfaces stay in sync with the skill.

-Seven drift classes, each of which has shipped before:
+Eight drift classes, each of which has shipped before:

 1. The SKILL.md frontmatter description is the only text an agent sees before
   deciding to load the skill — every visual type in the selection table must
@@ -17,6 +17,8 @@ Seven drift classes, each of which has shipped before:
   drift, because they are four separate copies of one sentence.
 7. Factory Droid's README install commands and native manifest path must agree
   with the package metadata instead of becoming a second hand-maintained API.
+8. Every support path a strict skill bundler can extract from SKILL.md must be
+   a literal file shipped inside the skill package.
 """

 from __future__ import annotations
@@ -24,7 +26,8 @@ from __future__ import annotations
 import json
 import re
 import sys
-from pathlib import Path
+from pathlib import Path, PurePosixPath
+from urllib.parse import unquote, urlsplit

 ROOT = Path(__file__).resolve().parent.parent
 SKILL = ROOT / "skills/diagram-design/SKILL.md"
@@ -44,6 +47,29 @@ PROFILE_SURFACES = (
 )
 FACTORY_MANIFEST = Path(".factory-plugin/plugin.json")
 FACTORY_MARKETPLACE = Path(".factory-plugin/marketplace.json")
+SUPPORT_DIRECTORIES = frozenset(
+    {"references", "templates", "scripts", "assets", "examples"}
+)
+# Mirrors Hermes Agent's support-file scanner. It intentionally sees Markdown
+# links, code spans, and path-like prose because strict bundlers may require
+# every extracted path before they install any part of the skill.
+SCANNER_VISIBLE_SUPPORT_REFERENCE = re.compile(
+    r"(?:\]\(|`|(?:^|[\s\"']))"
+    r"((?:references|templates|scripts|assets|examples)/[^\s)`\"'<>]+)",
+    re.MULTILINE,
+)
+REQUIRED_PACKAGED_RUNTIME_FILES = frozenset(
+    {
+        "scripts/self_check.py",
+        "scripts/drawio_extract.py",
+        "scripts/mermaid_extract.py",
+        "assets/template.html",
+        "assets/template-dark.html",
+        "assets/template-full.html",
+        "assets/template-motion.html",
+        "assets/template-terminal.html",
+    }
+)


 def normalized(text: str) -> str:
@@ -145,6 +171,45 @@ def check_skill_reference_links(
            errors.append(f"SKILL.md links to missing reference {target!r}")


+def scanner_visible_support_references(markdown: str) -> list[str]:
+    """Return the local support paths a strict skill bundler will request."""
+    normalized_markdown = markdown.replace("\\", "/")
+    references: set[str] = set()
+    for match in SCANNER_VISIBLE_SUPPORT_REFERENCE.finditer(normalized_markdown):
+        raw = match.group(1).rstrip(".,;:")
+        references.add(unquote(urlsplit(raw).path))
+    return sorted(references)
+
+
+def check_packaged_support_references(
+    errors: list[str], markdown: str, skill_directory: Path
+) -> None:
+    """Require every scanner-visible path to be a safe, packaged file."""
+    scanner_references = scanner_visible_support_references(markdown)
+    for target in scanner_references:
+        normalized_target = target.replace("\\", "/")
+        path = PurePosixPath(normalized_target)
+        parts = [part for part in path.parts if part not in {"", "."}]
+        if (
+            not parts
+            or parts[0] not in SUPPORT_DIRECTORIES
+            or normalized_target.startswith("/")
+            or path.is_absolute()
+            or any(part == ".." or ":" in part for part in parts)
+        ):
+            errors.append(f"SKILL.md exposes unsafe packaged support path {target!r}")
+        elif not (skill_directory / "/".join(parts)).is_file():
+            errors.append(
+                f"SKILL.md exposes missing packaged support file {target!r}; "
+                "strict skill bundlers will abort installation"
+            )
+    for target in sorted(REQUIRED_PACKAGED_RUNTIME_FILES - set(scanner_references)):
+        errors.append(
+            f"SKILL.md does not expose required packaged runtime file {target!r}; "
+            "strict skill bundlers will omit it"
+        )
+
+
 def check_profile_surfaces(errors: list[str], root: Path) -> None:
    reference = root / "skills/diagram-design/references/profiles.md"
    if not reference.is_file():
@@ -270,6 +335,11 @@ def main() -> int:
        SKILL.read_text(encoding="utf-8"),
        SKILL.parent,
    )
+    check_packaged_support_references(
+        errors,
+        SKILL.read_text(encoding="utf-8"),
+        SKILL.parent,
+    )
    check_profile_surfaces(errors, ROOT)
    if errors:
        print("FAIL docs sync")
@@ -278,7 +348,8 @@ def main() -> int:
        return 1
    print(
        "OK docs sync: description hooks, gallery reachability, README tree, "
-        "reference links, profile surfaces, manifest descriptions, Factory install contract"
+        "reference links, packaged support files, profile surfaces, manifest descriptions, "
+        "Factory install contract"
    )
    return 0