From d73660d193cfcc971d00b4b60efaf5889d028b7e Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 7 Jul 2026 06:41:15 +0500 Subject: [PATCH] fix(integrations): escape multiline/control chars in SKILL.md frontmatter the hand-built SKILL.md frontmatter in the skills setup path (base.py, and the hermes twin) wrapped values in a double-quoted yaml scalar but only escaped backslash and quote. a multiline (block-scalar) description was then emitted with raw newlines inside the quoted scalar and reparsed with those newlines collapsed to spaces; a control character would break yaml loading outright. the description comes from the template frontmatter, so this is reachable for any command with a block-scalar description. add a shared specify_cli/_yaml_string.quote_yaml_double helper that emits a valid double-quoted scalar (newline/cr/tab short escapes, other control chars as \xXX) and route both renderers through it. added a regression test that round-trips a multiline description through the yaml parser. --- src/specify_cli/_yaml_string.py | 43 +++++++++++++++++++ src/specify_cli/integrations/base.py | 11 +++-- .../integrations/hermes/__init__.py | 9 ++-- .../test_integration_base_skills.py | 34 +++++++++++++++ 4 files changed, 89 insertions(+), 8 deletions(-) create mode 100644 src/specify_cli/_yaml_string.py diff --git a/src/specify_cli/_yaml_string.py b/src/specify_cli/_yaml_string.py new file mode 100644 index 0000000000..1ae8afda51 --- /dev/null +++ b/src/specify_cli/_yaml_string.py @@ -0,0 +1,43 @@ +"""Shared YAML double-quoted scalar escaping. + +Several skill generators build ``SKILL.md`` frontmatter by hand (to match the +release packaging script's byte-for-byte output) instead of going through +``yaml.safe_dump``. They wrap values in a double-quoted scalar but historically +only escaped the backslash and the quote, which corrupts any value containing a +newline (it becomes a raw line break inside the quoted scalar, reparsing as a +space) and outright breaks YAML loading for control characters (``U+0000``– +``U+001F`` and ``U+007F``), which YAML forbids literally in every scalar form. + +``quote_yaml_double`` renders a value as a valid YAML double-quoted scalar, +using the C-style escapes YAML defines so the value round-trips exactly. +""" +from __future__ import annotations + + +def quote_yaml_double(value: str) -> str: + """Return *value* as a YAML double-quoted scalar that round-trips exactly. + + Escapes backslash and the double quote, maps newline/CR/tab to their YAML + short escapes, and emits any other control character (``U+0000``–``U+001F`` + and ``U+007F``) as a ``\\xXX`` sequence. YAML double-quoted scalars are the + only YAML string form that can carry these characters, so this is always + safe to load back. + """ + out: list[str] = [] + for ch in value: + code = ord(ch) + if ch == "\\": + out.append("\\\\") + elif ch == '"': + out.append('\\"') + elif ch == "\n": + out.append("\\n") + elif ch == "\r": + out.append("\\r") + elif ch == "\t": + out.append("\\t") + elif code < 0x20 or code == 0x7F: + out.append(f"\\x{code:02x}") + else: + out.append(ch) + return '"' + "".join(out) + '"' diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 37d3cdf965..30ab6c742a 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -25,6 +25,8 @@ import yaml +from .._yaml_string import quote_yaml_double + if TYPE_CHECKING: from .manifest import IntegrationManifest @@ -1464,10 +1466,11 @@ def setup( # Build SKILL.md with manually formatted frontmatter to match # the release packaging script output exactly (double-quoted - # values, no yaml.safe_dump quoting differences). - def _quote(v: str) -> str: - escaped = v.replace("\\", "\\\\").replace('"', '\\"') - return f'"{escaped}"' + # values, no yaml.safe_dump quoting differences). Escaping goes + # through the shared helper so a multiline description (block + # scalar) or a control character can't corrupt or break the + # generated YAML. + _quote = quote_yaml_double skill_content = ( f"---\n" diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 5d1f3a261b..a0f5502709 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -18,6 +18,7 @@ import yaml +from ..._yaml_string import quote_yaml_double from ..base import IntegrationOption, SkillsIntegration from ..manifest import IntegrationManifest @@ -153,10 +154,10 @@ def setup( if not description: description = f"Spec Kit: {command_name} workflow" - # Build SKILL.md with manually formatted frontmatter - def _quote(v: str) -> str: - escaped = v.replace("\\", "\\\\").replace('"', '\\"') - return f'"{escaped}"' + # Build SKILL.md with manually formatted frontmatter. Escaping goes + # through the shared helper so a multiline description or a control + # character can't corrupt or break the generated YAML. + _quote = quote_yaml_double skill_content = ( f"---\n" diff --git a/tests/integrations/test_integration_base_skills.py b/tests/integrations/test_integration_base_skills.py index d88b786757..4137bc8b93 100644 --- a/tests/integrations/test_integration_base_skills.py +++ b/tests/integrations/test_integration_base_skills.py @@ -141,6 +141,40 @@ def test_skill_uses_template_descriptions(self, tmp_path): assert isinstance(fm["description"], str) assert len(fm["description"]) > 0, f"{f} has empty description" + def test_skill_frontmatter_preserves_multiline_description( + self, tmp_path, monkeypatch + ): + """A multiline (block-scalar) description must round-trip exactly. + + The hand-built SKILL.md frontmatter used to only escape backslash and + quote, so a block-scalar description was emitted with raw newlines inside + a double-quoted scalar and reparsed with those newlines collapsed to + spaces. The description must survive byte-for-byte.""" + i = get_integration(self.KEY) + template = tmp_path / "sample.md" + template.write_text( + "---\n" + "description: |\n" + " first line\n" + " second line\n" + "scripts:\n" + " sh: scripts/bash/x.sh\n" + "---\n" + "Body\n", + encoding="utf-8", + ) + monkeypatch.setattr(i, "list_command_templates", lambda: [template]) + + m = IntegrationManifest(self.KEY, tmp_path) + created = i.setup(tmp_path, m) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + + content = skill_files[0].read_text(encoding="utf-8") + fm = yaml.safe_load(content.split("---", 2)[1]) + assert "\n" in fm["description"] + assert fm["description"] == "first line\nsecond line\n" + def test_templates_are_processed(self, tmp_path): """Skill body must have placeholders replaced, not raw templates.""" i = get_integration(self.KEY)