From c2e642915c1d681c8a51661ff72b7db4e22fe2df Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sun, 20 Sep 2026 20:39:59 +0800 Subject: [PATCH] fix(skills): write quarantined text members verbatim, without newline translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Path.write_text(newline=None) translates LF to os.linesep on Windows, so the quarantined SKILL.md landed as CRLF while bundle_content_hash re-encodes the str as LF bytes — content_hash of the installed copy could never match and every hub skill reported update_available forever (#117181). Pass newline="" to keep the bundle's bytes verbatim, mirroring the bytes branch. --- tests/tools/test_skills_hub.py | 45 ++++++++++++++++++++++++++++++++++ tools/skills_hub_install.py | 4 ++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_skills_hub.py b/tests/tools/test_skills_hub.py index ebb85794a8..e074662ac8 100644 --- a/tests/tools/test_skills_hub.py +++ b/tests/tools/test_skills_hub.py @@ -1313,6 +1313,51 @@ class TestQuarantineBundleBinaryAssets: assert (q_path / "SKILL.md").read_text(encoding="utf-8").startswith("---") assert (q_path / "assets" / "neutts-cli" / "samples" / "jo.wav").read_bytes() == b"RIFF\x00\x01fakewav" + def test_quarantine_bundle_writes_text_files_without_newline_translation(self, tmp_path, monkeypatch): + """Text members must land on disk byte-for-byte as the bundle carries them. + + Path.write_text(newline=None) translates "\\n" to os.linesep on Windows, so a + CRLF copy became the installed content while bundle_content_hash re-encodes + the str as LF — every hub skill then reported update_available forever (#117181). + """ + import pathlib + + import tools.skills_hub as hub + from tools.skills_guard import content_hash + + def windows_text_mode_write_text(self, data, encoding=None, errors=None, newline=None): + # Emulate the platform translation newline=None performs on Windows; + # newline="" is the documented opt-out quarantine must rely on. + if newline != "": + data = data.replace("\n", "\r\n") + self.write_bytes(data.encode(encoding or "utf-8")) + + monkeypatch.setattr(pathlib.Path, "write_text", windows_text_mode_write_text) + + hub_dir = tmp_path / "skills" / ".hub" + with patch.object(hub, "SKILLS_DIR", tmp_path / "skills"), \ + patch.object(hub, "HUB_DIR", hub_dir), \ + patch.object(hub, "LOCK_FILE", hub_dir / "lock.json"), \ + patch.object(hub, "QUARANTINE_DIR", hub_dir / "quarantine"), \ + patch.object(hub, "AUDIT_LOG", hub_dir / "audit.log"), \ + patch.object(hub, "TAPS_FILE", hub_dir / "taps.json"), \ + patch.object(hub, "INDEX_CACHE_DIR", hub_dir / "index-cache"): + bundle = SkillBundle( + name="crlfskill", + files={ + "SKILL.md": "---\nname: crlfskill\n---\n\nBody line one.\nBody line two.\n", + "assets/binary.bin": b"\x00\x01raw", + }, + source="official", + identifier="official/mlops/models/crlfskill", + trust_level="builtin", + ) + + q_path = quarantine_bundle(bundle) + + assert (q_path / "SKILL.md").read_bytes() == bundle.files["SKILL.md"].encode("utf-8") + assert content_hash(q_path) == bundle_content_hash(bundle) + def test_quarantine_bundle_rejects_traversal_file_paths(self, tmp_path): import tools.skills_hub as hub diff --git a/tools/skills_hub_install.py b/tools/skills_hub_install.py index dc32cc340e..04d82a3426 100644 --- a/tools/skills_hub_install.py +++ b/tools/skills_hub_install.py @@ -74,7 +74,9 @@ def quarantine_bundle(bundle: SkillBundle) -> Path: if isinstance(file_content, bytes): file_dest.write_bytes(file_content) else: - file_dest.write_text(file_content, encoding="utf-8") + # newline="" keeps the bundle's LF bytes verbatim; the default None mode would + # translate to os.linesep on Windows and desync content_hash from bundle_content_hash. + file_dest.write_text(file_content, encoding="utf-8", newline="") return dest