fix(skill-creator): sort .skill entries deterministically

Fixes #37748.

Sort skill package archive entries by relative POSIX archive name so generated `.skill` bundles are reproducible regardless of filesystem traversal order.

Verification:
- `PYTHONDONTWRITEBYTECODE=1 python3 skills/skill-creator/scripts/test_package_skill.py`
- `git diff --check origin/main...HEAD`
- GitHub CI run 26690938925 on `43a0fdf7175f33a5c74bc7ff92723ebf5efc4df9`: all checks passed except repeated unrelated no-output timeouts in `checks-node-agentic-commands-doctor` and `checks-node-core-runtime-infra-state` after visible tests passed.
This commit is contained in:
Coder
2026-05-30 14:42:55 -04:00
committed by GitHub
parent dfbed5053a
commit 878e433d81
2 changed files with 45 additions and 2 deletions

View File

@@ -77,8 +77,13 @@ def package_skill(skill_path, output_dir=None):
# Create the .skill file (zip format) # Create the .skill file (zip format)
try: try:
with zipfile.ZipFile(skill_filename, "w", zipfile.ZIP_DEFLATED) as zipf: with zipfile.ZipFile(skill_filename, "w", zipfile.ZIP_DEFLATED) as zipf:
# Walk through the skill directory # Walk in a deterministic order. Sort by the archive-relative POSIX
for file_path in skill_path.rglob("*"): # entry name (not Path object order, which is filesystem/OS-flavour
# dependent) so written .skill entries are byte-stable everywhere.
for file_path in sorted(
skill_path.rglob("*"),
key=lambda path: path.relative_to(skill_path).as_posix(),
):
# Security: never follow or package symlinks. # Security: never follow or package symlinks.
if file_path.is_symlink(): if file_path.is_symlink():
print(f"[WARN] Skipping symlink: {file_path}") print(f"[WARN] Skipping symlink: {file_path}")

View File

@@ -156,6 +156,44 @@ class TestPackageSkillSecurity(TestCase):
self.assertIn("self-output-skill/script.py", names) self.assertIn("self-output-skill/script.py", names)
self.assertNotIn("self-output-skill/self-output-skill.skill", names) self.assertNotIn("self-output-skill/self-output-skill.skill", names)
def test_archive_entry_order_is_deterministic(self):
skill_dir = self.create_skill("order-skill")
# Files across multiple levels, created in non-sorted order, so the
# filesystem/rglob enumeration order differs from a lexicographic sort.
(skill_dir / "zeta.md").write_text("z\n")
(skill_dir / "yankee.txt").write_text("y\n")
alpha = skill_dir / "alpha"
alpha.mkdir()
(alpha / "delta.txt").write_text("d\n")
(alpha / "bravo.txt").write_text("b\n")
nested = skill_dir / "zlib"
nested.mkdir()
(nested / "november.txt").write_text("n\n")
# "alpha-x.txt" discriminates entry-name ordering from Path-object
# ordering: "-" (0x2d) sorts before "/" (0x2f) in the archive entry
# name, but Path part-tuple ordering places it after the "alpha/" dir.
(skill_dir / "alpha-x.txt").write_text("x\n")
out_dir = self.temp_dir / "out"
out_dir.mkdir()
result = package_skill(str(skill_dir), str(out_dir))
self.assertIsNotNone(result)
skill_file = out_dir / "order-skill.skill"
with zipfile.ZipFile(skill_file, "r") as archive:
names = [name for name in archive.namelist() if not name.endswith("/")]
# Entries must be ordered by their archive entry name, regardless of
# filesystem enumeration or OS path-flavour, so archives are reproducible.
self.assertEqual(names, sorted(names))
# Lock the entry-name contract: "alpha-x.txt" precedes "alpha/bravo.txt"
# (Path-object sorting would invert these).
self.assertLess(
names.index("order-skill/alpha-x.txt"),
names.index("order-skill/alpha/bravo.txt"),
)
# Ensure the fixture actually spans multiple directories/files.
self.assertIn("order-skill/zlib/november.txt", names)
if __name__ == "__main__": if __name__ == "__main__":
main() main()