Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions src/strands/vended_plugins/skills/agent_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."""

Expand Down Expand Up @@ -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>")
Expand Down
3 changes: 2 additions & 1 deletion src/strands/vended_plugins/skills/skill.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
40 changes: 40 additions & 0 deletions tests/strands/vended_plugins/skills/test_agent_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "&quot;" 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
44 changes: 44 additions & 0 deletions tests/strands/vended_plugins/skills/test_skill.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]