From f183646ed20e75df2d8d9588704827803757cc44 Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Tue, 7 Jul 2026 22:11:48 +0200 Subject: [PATCH] fix(integrations): escape control characters in SKILL.md frontmatter yaml.safe_dump with default_style='"' replaces the hand-rolled quote helpers in base.py and hermes, so newlines and control characters in template descriptions round-trip instead of producing unparseable YAML. Fixes #3391 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/integrations/base.py | 32 +++-- .../integrations/hermes/__init__.py | 20 ++-- .../test_skill_frontmatter_quoting.py | 111 ++++++++++++++++++ 3 files changed, 140 insertions(+), 23 deletions(-) create mode 100644 tests/integrations/test_skill_frontmatter_quoting.py diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 706a3cb5d2..11265375a6 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -51,6 +51,18 @@ } +def yaml_quote(value: str) -> str: + """Emit *value* as a double-quoted YAML scalar on a single line. + + A hand-rolled quote cannot carry raw newlines (YAML folds them to + spaces) or control characters (the reader rejects them), so let the + YAML emitter produce the escapes. + """ + return yaml.safe_dump( + str(value), default_style='"', allow_unicode=True, width=sys.maxsize + ).strip() + + # --------------------------------------------------------------------------- # IntegrationOption # --------------------------------------------------------------------------- @@ -1475,21 +1487,17 @@ def setup( if not description: description = f"Spec Kit: {command_name} workflow" - # 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}"' - + # Build SKILL.md with manually formatted frontmatter (stable + # double-quoted values). yaml_quote escapes newlines and control + # characters that a plain quoted f-string cannot carry. skill_content = ( f"---\n" - f"name: {_quote(skill_name)}\n" - f"description: {_quote(description)}\n" - f"compatibility: {_quote('Requires spec-kit project structure with .specify/ directory')}\n" + f"name: {yaml_quote(skill_name)}\n" + f"description: {yaml_quote(description)}\n" + f"compatibility: {yaml_quote('Requires spec-kit project structure with .specify/ directory')}\n" f"metadata:\n" - f" author: {_quote('github-spec-kit')}\n" - f" source: {_quote('templates/commands/' + src_file.name)}\n" + f" author: {yaml_quote('github-spec-kit')}\n" + f" source: {yaml_quote('templates/commands/' + src_file.name)}\n" f"---\n" f"{processed_body}" ) diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 5d1f3a261b..63ea5f9986 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -18,7 +18,7 @@ import yaml -from ..base import IntegrationOption, SkillsIntegration +from ..base import IntegrationOption, SkillsIntegration, yaml_quote from ..manifest import IntegrationManifest @@ -153,20 +153,18 @@ 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. yaml_quote + # escapes newlines and control characters that a plain quoted + # f-string cannot carry. skill_content = ( f"---\n" - f"name: {_quote(skill_name)}\n" - f"description: {_quote(description)}\n" + f"name: {yaml_quote(skill_name)}\n" + f"description: {yaml_quote(description)}\n" f"compatibility: " - f"{_quote('Requires spec-kit project structure with .specify/ directory')}\n" + f"{yaml_quote('Requires spec-kit project structure with .specify/ directory')}\n" f"metadata:\n" - f" author: {_quote('github-spec-kit')}\n" - f" source: {_quote('templates/commands/' + src_file.name)}\n" + f" author: {yaml_quote('github-spec-kit')}\n" + f" source: {yaml_quote('templates/commands/' + src_file.name)}\n" f"---\n" f"{processed_body}" ) diff --git a/tests/integrations/test_skill_frontmatter_quoting.py b/tests/integrations/test_skill_frontmatter_quoting.py new file mode 100644 index 0000000000..739039ea3f --- /dev/null +++ b/tests/integrations/test_skill_frontmatter_quoting.py @@ -0,0 +1,111 @@ +"""Regression tests for SKILL.md frontmatter quoting (#3391). + +The skills setup path builds SKILL.md frontmatter by hand with +double-quoted values. A double-quoted YAML scalar cannot carry a raw +newline (the parser folds it to a space) or a control character (the +reader rejects the document), so descriptions taken from template +frontmatter must be escaped by the YAML emitter. +""" + +from pathlib import Path + +import yaml + +from specify_cli.integrations import get_integration +from specify_cli.integrations.base import yaml_quote +from specify_cli.integrations.manifest import IntegrationManifest + +MULTILINE = "first line\nsecond line\n" +CONTROL = "ding\aling" + +HOSTILE_TEMPLATE = """--- +description: | + first line + second line +--- + +Body of the command. +""" + +CONTROL_TEMPLATE = """--- +description: "ding\\aling" +--- + +Body of the command. +""" + + +def _parse_frontmatter(skill_file: Path) -> dict: + content = skill_file.read_text(encoding="utf-8") + assert content.startswith("---\n") + return yaml.safe_load(content.split("---", 2)[1]) + + +def _fake_templates(tmp_path: Path, body: str) -> Path: + templates = tmp_path / "templates" + templates.mkdir(exist_ok=True) + (templates / "plan.md").write_text(body, encoding="utf-8") + return templates + + +class TestYamlQuote: + def test_simple_value_keeps_plain_double_quoted_form(self): + assert yaml_quote("speckit-plan") == '"speckit-plan"' + assert yaml_quote('say "hi"') == '"say \\"hi\\""' + assert yaml_quote("back\\slash") == '"back\\\\slash"' + + def test_multiline_value_round_trips(self): + quoted = yaml_quote(MULTILINE) + assert "\n" not in quoted + assert yaml.safe_load(quoted) == MULTILINE + + def test_control_character_round_trips(self): + quoted = yaml_quote(CONTROL) + assert "\a" not in quoted + assert yaml.safe_load(quoted) == CONTROL + + +class TestSkillFrontmatterQuoting: + def _generate(self, tmp_path, monkeypatch, template: str) -> Path: + integration = get_integration("agy") + monkeypatch.setattr( + integration, + "shared_commands_dir", + lambda: _fake_templates(tmp_path, template), + ) + manifest = IntegrationManifest("agy", tmp_path) + created = integration.setup(tmp_path, manifest) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + return skill_files[0] + + def test_multiline_description_survives(self, tmp_path, monkeypatch): + skill_file = self._generate(tmp_path, monkeypatch, HOSTILE_TEMPLATE) + fm = _parse_frontmatter(skill_file) + assert fm["description"] == MULTILINE + + def test_control_character_description_parses(self, tmp_path, monkeypatch): + skill_file = self._generate(tmp_path, monkeypatch, CONTROL_TEMPLATE) + fm = _parse_frontmatter(skill_file) + assert fm["description"] == CONTROL + + +class TestHermesSkillFrontmatterQuoting: + def test_multiline_description_survives(self, tmp_path, monkeypatch): + home = tmp_path / "home" + home.mkdir(exist_ok=True) + monkeypatch.setattr(Path, "home", lambda: home) + + integration = get_integration("hermes") + monkeypatch.setattr( + integration, + "shared_commands_dir", + lambda: _fake_templates(tmp_path, HOSTILE_TEMPLATE), + ) + manifest = IntegrationManifest("hermes", tmp_path) + created = integration.setup(tmp_path, manifest) + skill_files = [f for f in created if f.name == "SKILL.md"] + assert len(skill_files) == 1 + + fm = _parse_frontmatter(skill_files[0]) + assert fm["description"] == MULTILINE