Skip to content

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

Closed
agent-of-mkmeral wants to merge 1 commit intostrands-agents:mainfrom
agent-of-mkmeral:fix/skills-yaml-escape-and-xml-escape
Closed

fix(skills): escape quotes/backslashes in YAML colon fixer, escape all 5 XML special chars#2126
agent-of-mkmeral wants to merge 1 commit intostrands-agents:mainfrom
agent-of-mkmeral:fix/skills-yaml-escape-and-xml-escape

Conversation

@agent-of-mkmeral
Copy link
Copy Markdown
Contributor

Motivation

During adversarial testing of the TypeScript skills PR (sdk-typescript#807), two bugs were discovered in the Python AgentSkills plugin. The TS implementation already handles these correctly — this PR brings Python to parity.

Changes

Bug 1: _fix_yaml_colons doesn't escape quotes or backslashes (Medium)

File: src/strands/vended_plugins/skills/skill.py

The YAML colon-quoting fallback wraps values in double quotes but didn't escape existing " or \ characters in the value first, producing invalid YAML:

# Before (broken)
line = f'{key}: "{value}"'
# Input:  description: Use when: user says "hello"
# Output: description: "Use when: user says "hello""  ← YAML parse error

# After (fixed)
escaped = value.replace("\\", "\\\\").replace('"', '\\"')
line = f'{key}: "{escaped}"'
# Output: description: "Use when: user says \"hello\""  ← valid YAML

Also fails with Windows-style paths containing backslashes (C:\Users\test:) which YAML interprets as escape sequences.

Bug 2: XML escaping only covers 3 of 5 special characters (Low)

File: src/strands/vended_plugins/skills/agent_skills.py

The _generate_skills_xml method used xml.sax.saxutils.escape() which only escapes &, <, > — but not " and '. While these are technically valid in XML element content, escaping them is safer and prevents edge cases with nested XML or attribute contexts. Added a helper _escape_xml() that passes extra entities to cover all 5 characters.

Testing

  • 7 new tests added (4 for YAML escaping, 3 for XML escaping)
  • All 7 tests fail before the fix, pass after (TDD red→green)
  • All 125 existing tests continue to pass (0 regressions)
  • 132 total tests passing

cc @mkmeral

…e 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 ').
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants