fix(skills): escape quotes and backslashes in YAML colon fixer, escape all 5 XML special chars

Fix two bugs in the AgentSkills plugin discovered during adversarial testing:

1. _fix_yaml_colons now escapes backslashes and double quotes before
   wrapping values in double quotes. Previously, values like
   'description: Use when: user says "hello"' would produce
   invalid YAML because the quotes were not escaped.

2. XML generation now escapes all 5 XML special characters
   (&, <, >, ", ') instead of only the 3 covered by the default
   xml.sax.saxutils.escape (which omits " and ').

Author: agent-of-mkmeral

src/strands/vended_plugins/skills/agent_skills.py

@@ -10,7 +10,7 @@
 import logging
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, TypeAlias
-from xml.sax.saxutils import escape
+from xml.sax.saxutils import escape as _xml_escape
 
 from ...hooks.events import BeforeInvocationEvent
 from ...plugins import Plugin, hook
@@ -27,6 +27,24 @@
 _RESOURCE_DIRS = ("scripts", "references", "assets")
 _DEFAULT_MAX_RESOURCE_FILES = 20
 
+_XML_EXTRA_ENTITIES = {'"': """, "'": "'"}
+
+
+def _escape_xml(text: str) -> str:
+    """Escape all 5 XML special characters in text.
+
+    Extends the standard library's ``xml.sax.saxutils.escape`` to also
+    escape ``"`` and ``'``, which are not covered by default.
+
+    Args:
+        text: The text to escape.
+
+    Returns:
+        The escaped text.
+    """
+    return _xml_escape(text, _XML_EXTRA_ENTITIES)
+
+
 SkillSource: TypeAlias = str | Path | Skill
 """A single skill source: path string, Path object, or Skill instance."""
 
@@ -271,10 +289,10 @@ def _generate_skills_xml(self) -> str:
 
         for skill in self._skills.values():
             lines.append("<skill>")
-            lines.append(f"<name>{escape(skill.name)}</name>")
-            lines.append(f"<description>{escape(skill.description)}</description>")
+            lines.append(f"<name>{_escape_xml(skill.name)}</name>")
+            lines.append(f"<description>{_escape_xml(skill.description)}</description>")
             if skill.path is not None:
-                lines.append(f"<location>{escape(str(skill.path / 'SKILL.md'))}</location>")
+                lines.append(f"<location>{_escape_xml(str(skill.path / 'SKILL.md'))}</location>")
             lines.append("</skill>")
 
         lines.append("</available_skills>")

src/strands/vended_plugins/skills/skill.py

@@ -104,7 +104,8 @@ def _fix_yaml_colons(yaml_str: str) -> str:
             key, value = match.group(1), match.group(2)
             # If value contains a colon and isn't already quoted
             if ":" in value and not (value.startswith('"') or value.startswith("'")):
-                line = f'{key}: "{value}"'
+                escaped = value.replace("\\", "\\\\").replace('"', '\\"')
+                line = f'{key}: "{escaped}"'
         lines.append(line)
     return "\n".join(lines)
 

tests/strands/vended_plugins/skills/test_agent_skills.py

@@ -689,3 +689,43 @@ def test_skills_plugin_isinstance_check(self):
 
         plugin = AgentSkills(skills=[])
         assert isinstance(plugin, Plugin)
+
+
+class TestSkillsXmlEscaping:
+    """Tests for XML escaping of all 5 special characters."""
+
+    def test_escapes_double_quotes(self):
+        """Test that double quotes in descriptions are escaped."""
+        skill = _make_skill(description='Use "quotes" in text')
+        plugin = AgentSkills(skills=[skill])
+        xml = plugin._generate_skills_xml()
+
+        assert """ in xml
+        assert '"quotes"' not in xml.split("<description>")[1].split("</description>")[0]
+
+    def test_escapes_single_quotes(self):
+        """Test that single quotes (apostrophes) in descriptions are escaped."""
+        skill = _make_skill(description="It's a skill")
+        plugin = AgentSkills(skills=[skill])
+        xml = plugin._generate_skills_xml()
+
+        # Should escape ' to &apos; or &#x27; or similar
+        desc_content = xml.split("<description>")[1].split("</description>")[0]
+        assert "'" not in desc_content
+
+    def test_escapes_all_five_xml_special_chars(self):
+        """Test that all 5 XML special characters are escaped."""
+        skill = _make_skill(
+            name="test-skill",
+            description="Has & < > \" ' chars",
+        )
+        plugin = AgentSkills(skills=[skill])
+        xml = plugin._generate_skills_xml()
+
+        desc_content = xml.split("<description>")[1].split("</description>")[0]
+        assert "&amp;" in desc_content
+        assert "&lt;" in desc_content
+        assert "&gt;" in desc_content
+        assert "&quot;" in desc_content
+        # ' should be escaped as &apos; or similar entity
+        assert "'" not in desc_content

tests/strands/vended_plugins/skills/test_skill.py

@@ -559,3 +559,47 @@ def test_skill_classmethods_exist(self):
         assert callable(getattr(Skill, "from_file", None))
         assert callable(getattr(Skill, "from_content", None))
         assert callable(getattr(Skill, "from_directory", None))
+
+
+class TestFixYamlColonsEscaping:
+    """Tests for _fix_yaml_colons escaping edge cases (Bug fix)."""
+
+    def test_escapes_double_quotes_in_value(self):
+        """Test that double quotes in values are escaped before wrapping."""
+        raw = 'description: Use when: user says "hello" and "goodbye"'
+        fixed = _fix_yaml_colons(raw)
+        # Must escape quotes: description: "Use when: user says \"hello\" and \"goodbye\""
+        assert r"\"hello\"" in fixed
+        assert r"\"goodbye\"" in fixed
+        # Verify it produces valid YAML
+        import yaml
+
+        result = yaml.safe_load(fixed)
+        assert result["description"] == 'Use when: user says "hello" and "goodbye"'
+
+    def test_escapes_backslashes_in_value(self):
+        r"""Test that backslashes in values are escaped before wrapping."""
+        raw = r"description: Path C:\Users\test: with colons"
+        fixed = _fix_yaml_colons(raw)
+        # Must escape backslashes before wrapping
+        import yaml
+
+        result = yaml.safe_load(fixed)
+        assert r"C:\Users\test" in result["description"]
+
+    def test_escapes_mixed_quotes_and_backslashes(self):
+        r"""Test that both quotes and backslashes are properly escaped."""
+        raw = r'description: Run "cmd": C:\path\to: file'
+        fixed = _fix_yaml_colons(raw)
+        import yaml
+
+        result = yaml.safe_load(fixed)
+        assert '"cmd"' in result["description"]
+        assert "C:\\path\\to" in result["description"]
+
+    def test_frontmatter_with_quotes_in_colon_value(self):
+        """Test end-to-end: frontmatter with quotes and colons parses correctly."""
+        content = '---\nname: my-skill\ndescription: Use when: user says "hello"\n---\nBody.'
+        frontmatter, body = _parse_frontmatter(content)
+        assert frontmatter["name"] == "my-skill"
+        assert '"hello"' in frontmatter["description"]