diff --git a/pkg/skills/validator.go b/pkg/skills/validator.go index 175ab96fa0..571d641e1f 100644 --- a/pkg/skills/validator.go +++ b/pkg/skills/validator.go @@ -127,10 +127,78 @@ func collectWarnings(result *ParseResult, content []byte) []string { warnings = append(warnings, fmt.Sprintf("SKILL.md has %d lines (recommended max: %d)", lineCount, RecommendedMaxSkillMDLines)) } + if w := checkInvisibleRunes(content); w != "" { + warnings = append(warnings, w) + } return warnings } +// invisibleRuneName reports a human-readable name for runes that render as nothing +// (or reorder what follows) while still reaching a model as instruction text. An +// empty string means the rune is not of interest. +// +// A skill carrying these is not automatically malicious -- ZWJ occurs in ordinary +// emoji sequences and RTL marks occur in ordinary Arabic and Hebrew prose -- so this +// is reported as a warning for a human to look at, not as a validation error. +func invisibleRuneName(r rune) string { + switch { + case r >= 0xE0000 && r <= 0xE007F: + // Unicode tag characters: a full ASCII alphabet that most renderers draw as + // nothing at all, which makes them the usual carrier for smuggled instructions. + return "Unicode tag characters (U+E0000-E007F)" + case r == 0x200B, r == 0x200C, r == 0x200D, r == 0x2060, r == 0xFEFF: + return "zero-width characters" + case r >= 0x2061 && r <= 0x2064: + return "invisible mathematical operators (U+2061-2064)" + case r == 0x200E, r == 0x200F, r >= 0x202A && r <= 0x202E, r >= 0x2066 && r <= 0x2069: + // Bidirectional overrides let the rendered order of a line differ from the + // byte order a model reads. See CVE-2021-42574 ("Trojan Source"). + return "bidirectional override characters" + case r >= 0xFE00 && r <= 0xFE0F: + return "variation selectors (U+FE00-FE0F)" + default: + return "" + } +} + +// checkInvisibleRunes reports whether SKILL.md contains characters that are invisible +// when rendered but are still read by a model, which is how instructions get smuggled +// past human review. Returns an empty string when the content is clean. +func checkInvisibleRunes(content []byte) string { + // Preserve first-seen order so the message is stable across runs. + var kinds []string + seen := make(map[string]bool) + line := 1 + firstLine := 0 + + for _, r := range string(content) { + if r == '\n' { + line++ + continue + } + name := invisibleRuneName(r) + if name == "" { + continue + } + if !seen[name] { + seen[name] = true + kinds = append(kinds, name) + } + if firstLine == 0 { + firstLine = line + } + } + + if len(kinds) == 0 { + return "" + } + return fmt.Sprintf( + "SKILL.md contains %s, first at line %d; these are invisible when rendered but are "+ + "still read by the model, so review the file for instructions a human reviewer cannot see", + strings.Join(kinds, " and "), firstLine) +} + // ValidateSkillName checks that a skill name conforms to the Agent Skills specification. // Names must be 2-64 lowercase alphanumeric characters or hyphens, starting and ending // with alphanumeric, with no consecutive hyphens. diff --git a/pkg/skills/validator_test.go b/pkg/skills/validator_test.go index ec92fa2315..509a80a4c2 100644 --- a/pkg/skills/validator_test.go +++ b/pkg/skills/validator_test.go @@ -359,3 +359,78 @@ func containsSubstring(strs []string, substr string) bool { } return false } + +func TestCheckInvisibleRunes(t *testing.T) { + t.Parallel() + + // Unicode tag characters spelling "ignore" -- invisible in every renderer, + // but delivered to the model as text. + var tagged strings.Builder + for _, r := range "ignore" { + tagged.WriteRune(r + 0xE0000) + } + + tests := []struct { + name string + content string + want string // substring the warning must contain; "" means no warning + }{ + { + name: "clean content produces no warning", + content: "# Skill\n\nA perfectly ordinary skill description.\n", + want: "", + }, + { + name: "non-ASCII prose is not flagged", + content: "# Beceri\n\nTürkçe açıklama, Ελληνικά, 日本語, emoji 🔐.\n", + want: "", + }, + { + name: "tag characters are reported", + content: "# Skill\n\nHarmless text." + tagged.String() + "\n", + want: "Unicode tag characters", + }, + { + name: "zero-width characters are reported", + content: "# Skill\n\nHarmless​text.\n", + want: "zero-width characters", + }, + { + name: "bidi overrides are reported", + content: "# Skill\n\nallowed-tools: read‮gnitirw‬\n", + want: "bidirectional override characters", + }, + { + name: "variation selectors are reported", + content: "# Skill\n\nHarmless︁ text.\n", + want: "variation selectors", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := checkInvisibleRunes([]byte(tt.content)) + if tt.want == "" { + assert.Empty(t, got) + return + } + assert.Contains(t, got, tt.want) + }) + } +} + +func TestCheckInvisibleRunesReportsFirstLine(t *testing.T) { + t.Parallel() + + got := checkInvisibleRunes([]byte("# Skill\n\nclean\nstill clean\nhere​it is\n")) + assert.Contains(t, got, "first at line 5") +} + +func TestCollectWarningsFlagsInvisibleRunes(t *testing.T) { + t.Parallel() + + warnings := collectWarnings(&ParseResult{}, []byte("# Skill\n\nHarmless​text.\n")) + require.Len(t, warnings, 1) + assert.Contains(t, warnings[0], "zero-width characters") +}