From 9d11a7fae71e387d4b573e331f0042f8662b06aa Mon Sep 17 00:00:00 2001 From: dacharyc Date: Sat, 19 Sep 2026 20:48:21 -0400 Subject: [PATCH] Count frontmatter field lengths in characters, not bytes The name, description, and compatibility limits (64, 1024, 500) were enforced with len(), which counts UTF-8 bytes, while the spec and the error messages speak of characters. A 984-character CJK description is 2952 bytes and was rejected as "exceeds 1024 characters (2952)". Count Unicode code points instead, the same unit the skills-ref reference validator uses. The judge's sanitizeStringField cap moves to 1024 characters for the same reason: it was cutting spec-compliant multibyte descriptions to a third of their length before scoring. Fixes #94 --- judge/judge.go | 14 +++++++----- judge/judge_test.go | 18 ++++++++++----- structure/frontmatter.go | 27 +++++++++++++++------- structure/frontmatter_test.go | 42 +++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 20 deletions(-) diff --git a/judge/judge.go b/judge/judge.go index 69c3ed8..acc6cf3 100644 --- a/judge/judge.go +++ b/judge/judge.go @@ -252,14 +252,16 @@ const ( var controlCharStripper = regexp.MustCompile(`[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]`) +// maxStringFieldChars caps free-text fields that flow into or out of the +// judge. It is measured in characters (Unicode code points) so that a +// spec-compliant 1024-character description is passed through whole +// rather than cut to a fraction of its length by a byte count. +const maxStringFieldChars = 1024 + func sanitizeStringField(s string) string { s = controlCharStripper.ReplaceAllString(s, "") - if len(s) > 1024 { - s = s[:1024] - // Back off any partial UTF-8 sequence left by the byte-boundary cut. - for len(s) > 0 && !utf8.ValidString(s) { - s = s[:len(s)-1] - } + if utf8.RuneCountInString(s) > maxStringFieldChars { + s = string([]rune(s)[:maxStringFieldChars]) } return s } diff --git a/judge/judge_test.go b/judge/judge_test.go index f54e4b6..2e1c3c9 100644 --- a/judge/judge_test.go +++ b/judge/judge_test.go @@ -1038,14 +1038,20 @@ func TestSanitizeStringField(t *testing.T) { t.Errorf("truncated len = %d, want 1024", len(got)) } - // A multi-byte rune straddling the 1024-byte boundary must not be split. - straddle := strings.Repeat("x", 1023) + "é" + strings.Repeat("y", 100) - got := sanitizeStringField(straddle) + // The cap is in characters, not bytes: a spec-compliant 1024-character + // CJK description (3072 bytes) must pass through untouched. + cjk := strings.Repeat("\u6f22", 1024) + if got := sanitizeStringField(cjk); got != cjk { + t.Errorf("1024-character multibyte string was altered (len %d, want %d)", len(got), len(cjk)) + } + + // Over the cap, truncation lands on a character boundary. + got := sanitizeStringField(strings.Repeat("\u6f22", 2000)) if !utf8.ValidString(got) { - t.Errorf("truncation produced invalid UTF-8: %q", got[1015:]) + t.Errorf("truncation produced invalid UTF-8") } - if len(got) != 1023 { - t.Errorf("truncated len = %d, want 1023 (partial rune dropped)", len(got)) + if n := utf8.RuneCountInString(got); n != 1024 { + t.Errorf("truncated rune count = %d, want 1024", n) } } diff --git a/structure/frontmatter.go b/structure/frontmatter.go index 8e20eaf..a39b866 100644 --- a/structure/frontmatter.go +++ b/structure/frontmatter.go @@ -4,6 +4,7 @@ import ( "path/filepath" "regexp" "strings" + "unicode/utf8" "github.com/agent-ecosystem/skill-validator/skill" "github.com/agent-ecosystem/skill-validator/types" @@ -11,6 +12,16 @@ import ( var namePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`) +// Field length limits from the spec are in characters, which this package +// counts as Unicode code points (the same unit as the skills-ref reference +// validator). Counting bytes would reject multibyte descriptions well +// below the limit. +const ( + maxNameChars = 64 + maxDescriptionChars = 1024 + maxCompatibilityChars = 500 +) + // CheckFrontmatter validates the YAML frontmatter of a parsed skill. It checks // required fields (name, description), enforces format and length constraints, // validates optional fields, and warns about unrecognized or keyword-stuffed fields. @@ -23,8 +34,8 @@ func CheckFrontmatter(s *skill.Skill, opts Options) []types.Result { if name == "" { results = append(results, ctx.Error("name is required")) } else { - if len(name) > 64 { - results = append(results, ctx.Errorf("name exceeds 64 characters (%d)", len(name))) + if n := utf8.RuneCountInString(name); n > maxNameChars { + results = append(results, ctx.Errorf("name exceeds %d characters (%d)", maxNameChars, n)) } if !namePattern.MatchString(name) { results = append(results, ctx.Errorf("name %q must be lowercase alphanumeric with hyphens, no leading/trailing/consecutive hyphens", name)) @@ -43,12 +54,12 @@ func CheckFrontmatter(s *skill.Skill, opts Options) []types.Result { desc := s.Frontmatter.Description if desc == "" { results = append(results, ctx.Error("description is required")) - } else if len(desc) > 1024 { - results = append(results, ctx.Errorf("description exceeds 1024 characters (%d)", len(desc))) + } else if n := utf8.RuneCountInString(desc); n > maxDescriptionChars { + results = append(results, ctx.Errorf("description exceeds %d characters (%d)", maxDescriptionChars, n)) } else if strings.TrimSpace(desc) == "" { results = append(results, ctx.Error("description must not be empty/whitespace-only")) } else { - results = append(results, ctx.Passf("description: (%d chars)", len(desc))) + results = append(results, ctx.Passf("description: (%d chars)", n)) results = append(results, checkDescriptionKeywordStuffing(ctx, desc)...) } @@ -59,10 +70,10 @@ func CheckFrontmatter(s *skill.Skill, opts Options) []types.Result { // Check optional compatibility if s.Frontmatter.Compatibility != "" { - if len(s.Frontmatter.Compatibility) > 500 { - results = append(results, ctx.Errorf("compatibility exceeds 500 characters (%d)", len(s.Frontmatter.Compatibility))) + if n := utf8.RuneCountInString(s.Frontmatter.Compatibility); n > maxCompatibilityChars { + results = append(results, ctx.Errorf("compatibility exceeds %d characters (%d)", maxCompatibilityChars, n)) } else { - results = append(results, ctx.Passf("compatibility: (%d chars)", len(s.Frontmatter.Compatibility))) + results = append(results, ctx.Passf("compatibility: (%d chars)", n)) } } diff --git a/structure/frontmatter_test.go b/structure/frontmatter_test.go index d10fb76..14ac5bb 100644 --- a/structure/frontmatter_test.go +++ b/structure/frontmatter_test.go @@ -47,6 +47,15 @@ func TestCheckFrontmatter_Name(t *testing.T) { requireResult(t, results, types.Error, "name exceeds 64 characters (65)") }) + t.Run("name length counts characters, not bytes", func(t *testing.T) { + // 40 two-byte runes: 80 bytes, 40 characters. The name pattern + // rejects it, but the length check must not. + name := strings.Repeat("\u00e9", 40) + s := makeSkill("/tmp/"+name, name, "A description") + results := CheckFrontmatter(s, Options{}) + requireNoResultContaining(t, results, types.Error, "exceeds 64 characters") + }) + t.Run("name with uppercase", func(t *testing.T) { s := makeSkill("/tmp/My-Skill", "My-Skill", "A description") results := CheckFrontmatter(s, Options{}) @@ -110,6 +119,23 @@ func TestCheckFrontmatter_Description(t *testing.T) { requireResult(t, results, types.Error, "description exceeds 1024 characters (1025)") }) + t.Run("multibyte description under the limit", func(t *testing.T) { + // 342 CJK characters is 1026 UTF-8 bytes: over the limit if + // counted in bytes, well under it in characters (#94). + desc := strings.Repeat("\u6f22", 342) + s := makeSkill("/tmp/my-skill", "my-skill", desc) + results := CheckFrontmatter(s, Options{}) + requireNoResultContaining(t, results, types.Error, "description exceeds") + requireResultContaining(t, results, types.Pass, "description: (342 chars)") + }) + + t.Run("multibyte description over the limit", func(t *testing.T) { + desc := strings.Repeat("\u6f22", 1025) + s := makeSkill("/tmp/my-skill", "my-skill", desc) + results := CheckFrontmatter(s, Options{}) + requireResult(t, results, types.Error, "description exceeds 1024 characters (1025)") + }) + t.Run("whitespace-only description", func(t *testing.T) { s := makeSkill("/tmp/my-skill", "my-skill", " \t\n ") results := CheckFrontmatter(s, Options{}) @@ -368,6 +394,22 @@ func TestCheckFrontmatter_Compatibility(t *testing.T) { results := CheckFrontmatter(s, Options{}) requireResult(t, results, types.Error, "compatibility exceeds 500 characters (501)") }) + + t.Run("multibyte compatibility under the limit", func(t *testing.T) { + // 400 CJK characters is 1200 bytes but only 400 characters. + s := makeSkill("/tmp/my-skill", "my-skill", "desc") + s.Frontmatter.Compatibility = strings.Repeat("\u6f22", 400) + results := CheckFrontmatter(s, Options{}) + requireNoResultContaining(t, results, types.Error, "compatibility exceeds") + requireResultContaining(t, results, types.Pass, "compatibility: (400 chars)") + }) + + t.Run("multibyte compatibility over the limit", func(t *testing.T) { + s := makeSkill("/tmp/my-skill", "my-skill", "desc") + s.Frontmatter.Compatibility = strings.Repeat("\u6f22", 501) + results := CheckFrontmatter(s, Options{}) + requireResult(t, results, types.Error, "compatibility exceeds 500 characters (501)") + }) } func TestCheckFrontmatter_Metadata(t *testing.T) {