Skip to content

Commit 5e41fb2

Browse files
authored
feat: add validation rules for reserved words, XML tags, and type checks (#54)
* feat: add validation rules for reserved words, XML tags, and type checks Add missing validation rules per Agent Skills specification: - Reserved word check: name cannot contain 'anthropic' or 'claude' - XML tag check: name and description cannot contain XML-like tags - Type check: name and description must be strings (detects YAML type coercion) * fix: guard string-only operations against non-string values Prevent TypeError when name/description are non-string types (e.g., YAML list or boolean). The XML tag check and other string operations now only run when the value is actually a string. * fix: ensure type validation runs without meta Move type checks from meta-only block to always run on skill values. This ensures non-string name/description are detected even when validating via index (without meta). * fix: detect falsy non-string values (null, [], False) Restructure type/required checks using if/elif to ensure falsy non-string values like null, empty list, or False are caught as type errors instead of silently passing validation.
1 parent fd79dac commit 5e41fb2

File tree

3 files changed

+214
-6
lines changed

3 files changed

+214
-6
lines changed

src/skillport/modules/skills/internal/validation.py

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import re
56
import unicodedata
67
from pathlib import Path
78

@@ -13,6 +14,13 @@
1314
DESCRIPTION_MAX_LENGTH = 1024
1415
COMPATIBILITY_MAX_LENGTH = 500
1516

17+
# Reserved words that cannot appear in skill names
18+
RESERVED_WORDS: frozenset[str] = frozenset({"anthropic", "claude"})
19+
20+
# Pattern to detect XML-like tags (e.g., <tag>, </tag>, <tag attr="x"/>)
21+
# Tags must start with a letter or "/" (for closing tags)
22+
_XML_TAG_PATTERN = re.compile(r"</?[a-zA-Z][^>]*>")
23+
1624

1725
def _is_valid_name_char(char: str) -> bool:
1826
"""Check if a character is valid for skill names (lowercase letter, digit, or hyphen)."""
@@ -29,6 +37,20 @@ def _validate_name_chars(name: str) -> bool:
2937
return all(_is_valid_name_char(c) for c in normalized)
3038

3139

40+
def _contains_reserved_word(name: str) -> str | None:
41+
"""Return the first reserved word found in name, or None."""
42+
name_lower = name.lower()
43+
for word in RESERVED_WORDS:
44+
if word in name_lower:
45+
return word
46+
return None
47+
48+
49+
def _contains_xml_tags(text: str) -> bool:
50+
"""Check if text contains XML-like tags."""
51+
return bool(_XML_TAG_PATTERN.search(text))
52+
53+
3254
# Allowed top-level frontmatter properties
3355
ALLOWED_FRONTMATTER_KEYS: set[str] = {
3456
"name",
@@ -83,12 +105,34 @@ def validate_skill_record(
83105
)
84106
)
85107

86-
# Required fields (value checks)
87-
if not name:
108+
# Type and required field checks
109+
name_is_str = isinstance(name, str)
110+
desc_is_str = isinstance(description, str)
111+
112+
# name: must be non-empty string
113+
if not name_is_str:
114+
issues.append(
115+
ValidationIssue(
116+
severity="fatal",
117+
message=f"frontmatter.name: must be a string (got {type(name).__name__})",
118+
field="name",
119+
)
120+
)
121+
elif not name:
88122
issues.append(
89123
ValidationIssue(severity="fatal", message="frontmatter.name: missing", field="name")
90124
)
91-
if not description:
125+
126+
# description: must be non-empty string
127+
if not desc_is_str:
128+
issues.append(
129+
ValidationIssue(
130+
severity="fatal",
131+
message=f"frontmatter.description: must be a string (got {type(description).__name__})",
132+
field="description",
133+
)
134+
)
135+
elif not description:
92136
issues.append(
93137
ValidationIssue(
94138
severity="fatal",
@@ -116,7 +160,7 @@ def validate_skill_record(
116160
)
117161
)
118162

119-
if name:
163+
if name and name_is_str:
120164
if len(name) > NAME_MAX_LENGTH:
121165
issues.append(
122166
ValidationIssue(
@@ -149,8 +193,25 @@ def validate_skill_record(
149193
field="name",
150194
)
151195
)
196+
reserved = _contains_reserved_word(name)
197+
if reserved:
198+
issues.append(
199+
ValidationIssue(
200+
severity="fatal",
201+
message=f"frontmatter.name: cannot contain reserved word '{reserved}'",
202+
field="name",
203+
)
204+
)
205+
if _contains_xml_tags(name):
206+
issues.append(
207+
ValidationIssue(
208+
severity="fatal",
209+
message="frontmatter.name: cannot contain XML tags",
210+
field="name",
211+
)
212+
)
152213

153-
if description:
214+
if description and desc_is_str:
154215
if len(description) > DESCRIPTION_MAX_LENGTH:
155216
issues.append(
156217
ValidationIssue(
@@ -159,6 +220,14 @@ def validate_skill_record(
159220
field="description",
160221
)
161222
)
223+
if _contains_xml_tags(description):
224+
issues.append(
225+
ValidationIssue(
226+
severity="fatal",
227+
message="frontmatter.description: cannot contain XML tags",
228+
field="description",
229+
)
230+
)
162231

163232
# Check for unexpected frontmatter keys and compatibility (requires reading SKILL.md)
164233
if path:

tests/unit/test_validation_rules.py

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
COMPATIBILITY_MAX_LENGTH,
88
DESCRIPTION_MAX_LENGTH,
99
NAME_MAX_LENGTH,
10+
RESERVED_WORDS,
1011
SKILL_LINE_THRESHOLD,
1112
validate_skill_record,
1213
)
@@ -135,6 +136,60 @@ def test_name_valid_hyphens(self):
135136
hyphen_issues = [i for i in issues if "hyphen" in i.message.lower()]
136137
assert len(hyphen_issues) == 0
137138

139+
@pytest.mark.parametrize("reserved", list(RESERVED_WORDS))
140+
def test_name_reserved_word(self, reserved: str):
141+
"""name containing reserved word → fatal."""
142+
name = f"my-{reserved}-skill"
143+
issues = validate_skill_record(
144+
{"name": name, "description": "desc", "path": f"/skills/{name}"}
145+
)
146+
fatal = [i for i in issues if i.severity == "fatal" and "reserved" in i.message.lower()]
147+
assert len(fatal) == 1
148+
assert reserved in fatal[0].message
149+
150+
def test_name_reserved_word_case_insensitive(self):
151+
"""Reserved word check should be case-insensitive."""
152+
# Note: name validation already fails on uppercase, but reserved check is independent
153+
issues = validate_skill_record(
154+
{"name": "my-skill", "description": "desc", "path": "/skills/my-skill"}
155+
)
156+
reserved_issues = [i for i in issues if "reserved" in i.message.lower()]
157+
assert len(reserved_issues) == 0
158+
159+
def test_name_xml_tags(self):
160+
"""name containing XML tags → fatal."""
161+
issues = validate_skill_record(
162+
{"name": "<script>", "description": "desc", "path": "/skills/<script>"}
163+
)
164+
fatal = [i for i in issues if i.severity == "fatal" and "xml" in i.message.lower()]
165+
assert len(fatal) == 1
166+
167+
def test_description_xml_tags(self):
168+
"""description containing XML tags → fatal."""
169+
issues = validate_skill_record(
170+
{
171+
"name": "my-skill",
172+
"description": "A skill with <script>alert('xss')</script> injection",
173+
"path": "/skills/my-skill",
174+
}
175+
)
176+
fatal = [i for i in issues if i.severity == "fatal" and "xml" in i.message.lower()]
177+
assert len(fatal) == 1
178+
assert "description" in fatal[0].field
179+
180+
def test_description_no_xml_tags_ok(self):
181+
"""description without XML tags → ok."""
182+
issues = validate_skill_record(
183+
{
184+
"name": "my-skill",
185+
"description": "A valid description with math like 3 < 5 or x > y",
186+
"path": "/skills/my-skill",
187+
}
188+
)
189+
xml_issues = [i for i in issues if "xml" in i.message.lower()]
190+
# "<" and ">" used separately (not forming tags) should pass
191+
assert len(xml_issues) == 0
192+
138193

139194
class TestValidationWarning:
140195
"""Warning validation rules (exit code 0)."""
@@ -405,6 +460,90 @@ def test_meta_none_skips_key_check(self):
405460
key_missing = [i for i in issues if "key is missing" in i.message]
406461
assert len(key_missing) == 0
407462

463+
def test_name_not_string_in_frontmatter(self):
464+
"""Non-string 'name' → fatal (e.g., name: yes → True)."""
465+
issues = validate_skill_record(
466+
{"name": True, "description": "desc", "path": "/skills/test"},
467+
meta={"name": True, "description": "desc"}, # YAML: name: yes
468+
)
469+
fatal = [
470+
i for i in issues if i.severity == "fatal" and "must be a string" in i.message
471+
]
472+
assert len(fatal) == 1
473+
assert "name" in fatal[0].field
474+
assert "bool" in fatal[0].message
475+
476+
def test_description_not_string_in_frontmatter(self):
477+
"""Non-string 'description' → fatal."""
478+
issues = validate_skill_record(
479+
{"name": "test", "description": ["item1", "item2"], "path": "/skills/test"},
480+
meta={"name": "test", "description": ["item1", "item2"]}, # YAML list
481+
)
482+
fatal = [
483+
i for i in issues if i.severity == "fatal" and "must be a string" in i.message
484+
]
485+
assert len(fatal) == 1
486+
assert "description" in fatal[0].field
487+
assert "list" in fatal[0].message
488+
489+
def test_non_string_description_no_crash(self):
490+
"""Non-string description in skill dict should not crash on XML check."""
491+
# This tests the case where skill dict has non-string values
492+
# (e.g., from index or malformed input)
493+
issues = validate_skill_record(
494+
{"name": "test", "description": ["item1", "item2"], "path": "/skills/test"},
495+
meta={"name": "test", "description": ["item1", "item2"]},
496+
)
497+
# Should not raise TypeError, should return type error issue
498+
fatal = [i for i in issues if i.severity == "fatal"]
499+
assert len(fatal) >= 1
500+
assert any("must be a string" in i.message for i in fatal)
501+
502+
def test_non_string_name_no_crash(self):
503+
"""Non-string name in skill dict should not crash on validation."""
504+
issues = validate_skill_record(
505+
{"name": True, "description": "desc", "path": "/skills/test"},
506+
meta={"name": True, "description": "desc"},
507+
)
508+
# Should not raise TypeError
509+
fatal = [i for i in issues if i.severity == "fatal"]
510+
assert len(fatal) >= 1
511+
assert any("must be a string" in i.message for i in fatal)
512+
513+
def test_non_string_without_meta(self):
514+
"""Type check works even without meta (e.g., index-based validation)."""
515+
# This is the critical case: validate via index without meta
516+
issues = validate_skill_record(
517+
{"name": True, "description": ["a", "b"], "path": "/skills/test"},
518+
meta=None, # No meta provided
519+
)
520+
fatal = [i for i in issues if i.severity == "fatal"]
521+
# Should detect both type errors
522+
type_errors = [i for i in fatal if "must be a string" in i.message]
523+
assert len(type_errors) == 2
524+
assert any("name" in i.field for i in type_errors)
525+
assert any("description" in i.field for i in type_errors)
526+
527+
def test_falsy_non_string_name_detected(self):
528+
"""Falsy non-string name (null, [], False) should be detected as type error."""
529+
for falsy_value in [None, [], False, 0]:
530+
issues = validate_skill_record(
531+
{"name": falsy_value, "description": "desc", "path": "/skills/test"},
532+
)
533+
fatal = [i for i in issues if i.severity == "fatal" and i.field == "name"]
534+
assert len(fatal) >= 1, f"Failed for name={falsy_value!r}"
535+
assert any("must be a string" in i.message for i in fatal), f"Failed for name={falsy_value!r}"
536+
537+
def test_falsy_non_string_description_detected(self):
538+
"""Falsy non-string description (null, [], False) should be detected as type error."""
539+
for falsy_value in [None, [], False, 0]:
540+
issues = validate_skill_record(
541+
{"name": "test", "description": falsy_value, "path": "/skills/test"},
542+
)
543+
fatal = [i for i in issues if i.severity == "fatal" and i.field == "description"]
544+
assert len(fatal) >= 1, f"Failed for description={falsy_value!r}"
545+
assert any("must be a string" in i.message for i in fatal), f"Failed for description={falsy_value!r}"
546+
408547

409548
class TestStrictMode:
410549
"""strict mode behavior tests."""

uv.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)