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) {