diff --git a/skills/skill-creator/scripts/package_skill.py b/skills/skill-creator/scripts/package_skill.py index aa4de89675a8..a5442a463937 100644 --- a/skills/skill-creator/scripts/package_skill.py +++ b/skills/skill-creator/scripts/package_skill.py @@ -77,8 +77,13 @@ def package_skill(skill_path, output_dir=None): # Create the .skill file (zip format) try: with zipfile.ZipFile(skill_filename, "w", zipfile.ZIP_DEFLATED) as zipf: - # Walk through the skill directory - for file_path in skill_path.rglob("*"): + # Walk in a deterministic order. Sort by the archive-relative POSIX + # 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. if file_path.is_symlink(): print(f"[WARN] Skipping symlink: {file_path}") diff --git a/skills/skill-creator/scripts/test_package_skill.py b/skills/skill-creator/scripts/test_package_skill.py index 588290b85839..a994a79dd1f7 100644 --- a/skills/skill-creator/scripts/test_package_skill.py +++ b/skills/skill-creator/scripts/test_package_skill.py @@ -156,6 +156,44 @@ class TestPackageSkillSecurity(TestCase): self.assertIn("self-output-skill/script.py", 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__": main()