PR #66 review: json.Decoder.Token() fails on a number outside float64 range (1e999), which json.Unmarshal ignores in an unknown key, so a decoy key ended the scan and the #65 repro wrote to main. The decoder now uses UseNumber, and any scan error is a validation error. The duplicate check folds with strings.EqualFold (as encoding/json does) instead of ToLower, so long s and the Kelvin sign are caught in nested objects too. Refs #65 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
252 lines
7.8 KiB
Go
252 lines
7.8 KiB
Go
package tools
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"reflect"
|
|
"regexp"
|
|
"strings"
|
|
|
|
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
|
|
"git.d-ma.be/mathias/gitea-mcp/internal/registry"
|
|
)
|
|
|
|
// Tool implements registry.Tool.
|
|
type Tool = registry.Tool
|
|
|
|
func textOK(v any) (json.RawMessage, error) {
|
|
return json.Marshal(v)
|
|
}
|
|
|
|
func parseArgs(raw json.RawMessage, dst any) error {
|
|
if len(raw) == 0 {
|
|
if err := json.Unmarshal([]byte("{}"), dst); err != nil {
|
|
return err
|
|
}
|
|
} else {
|
|
// Reject ambiguity BEFORE alias normalisation: re-marshalling through a
|
|
// map would otherwise silently collapse exact duplicates (#65).
|
|
if err := rejectAmbiguousKeys(raw, dst); err != nil {
|
|
return err
|
|
}
|
|
if err := json.Unmarshal(normalizeAliases(raw), dst); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return validateRequiredIdentifiers(dst)
|
|
}
|
|
|
|
// rejectAmbiguousKeys refuses input whose meaning depends on encoding/json's
|
|
// case-insensitive, last-key-wins matching (#65). A client-side check that reads
|
|
// `branch` must see the value the server acts on, so: no object at any depth may
|
|
// carry two keys equal under strings.EqualFold (exact duplicates included), and
|
|
// no top-level key may differ from a field of dst only in case.
|
|
func rejectAmbiguousKeys(raw json.RawMessage, dst any) error {
|
|
dec := json.NewDecoder(bytes.NewReader(raw))
|
|
// UseNumber: without it Token() fails on a number outside float64 range,
|
|
// which json.Unmarshal ignores in an unknown key — a decoy that would end
|
|
// the scan before the duplicate (PR #66 review).
|
|
dec.UseNumber()
|
|
if err := scanJSONValue(dec); err != nil {
|
|
if errors.Is(err, gitea.ErrValidation) {
|
|
return err
|
|
}
|
|
// Fail closed: input the scanner cannot read is input it cannot vouch for.
|
|
return fmt.Errorf("arguments are not valid JSON: %v: %w", err, gitea.ErrValidation)
|
|
}
|
|
var top map[string]json.RawMessage
|
|
if err := json.Unmarshal(raw, &top); err != nil {
|
|
return nil // not an object; nothing to compare against the schema
|
|
}
|
|
fields := jsonFieldNames(dst)
|
|
for k := range top {
|
|
for _, f := range fields {
|
|
if k != f && strings.EqualFold(k, f) {
|
|
return fmt.Errorf("argument %q does not match field %q exactly (keys are case-sensitive): %w", k, f, gitea.ErrValidation)
|
|
}
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// scanJSONValue consumes one JSON value from dec, failing with ErrValidation if
|
|
// any object within it has two keys that are equal under strings.EqualFold.
|
|
func scanJSONValue(dec *json.Decoder) error {
|
|
tok, err := dec.Token()
|
|
if err != nil {
|
|
return err
|
|
}
|
|
d, ok := tok.(json.Delim)
|
|
if !ok {
|
|
return nil
|
|
}
|
|
switch d {
|
|
case '{':
|
|
var seen []string
|
|
for dec.More() {
|
|
kt, err := dec.Token()
|
|
if err != nil {
|
|
return err
|
|
}
|
|
k, _ := kt.(string)
|
|
// EqualFold, not ToLower: encoding/json folds the same way, so
|
|
// long s (U+017F) matches s and the Kelvin sign matches k.
|
|
for _, prev := range seen {
|
|
if strings.EqualFold(prev, k) {
|
|
return fmt.Errorf("duplicate argument keys %q and %q (keys must be unique ignoring case): %w", prev, k, gitea.ErrValidation)
|
|
}
|
|
}
|
|
seen = append(seen, k)
|
|
if err := scanJSONValue(dec); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
case '[':
|
|
for dec.More() {
|
|
if err := scanJSONValue(dec); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
}
|
|
_, err = dec.Token() // closing delimiter
|
|
return err
|
|
}
|
|
|
|
// jsonFieldNames returns the json tag names of dst's top-level struct fields.
|
|
func jsonFieldNames(dst any) []string {
|
|
v := reflect.ValueOf(dst)
|
|
if v.Kind() != reflect.Pointer || v.IsNil() || v.Elem().Kind() != reflect.Struct {
|
|
return nil
|
|
}
|
|
t := v.Elem().Type()
|
|
names := make([]string, 0, t.NumField())
|
|
for i := 0; i < t.NumField(); i++ {
|
|
name, _, _ := strings.Cut(t.Field(i).Tag.Get("json"), ",")
|
|
if name == "" || name == "-" {
|
|
name = t.Field(i).Name
|
|
}
|
|
names = append(names, name)
|
|
}
|
|
return names
|
|
}
|
|
|
|
// branchNameRe is the shape the skills mcp-destructive-guard hook enforces.
|
|
var branchNameRe = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._/-]*$`)
|
|
|
|
// ValidateBranchName rejects branch names that Gitea would turn into a ref
|
|
// other than refs/heads/<name> as the caller meant it. `refs/heads/main` in
|
|
// particular creates refs/heads/refs/heads/main, after which every API write to
|
|
// main fails with "matches more than one" until the ref is deleted by hand
|
|
// (#65). Beyond the shape regexp it applies git's own check-ref-format rules
|
|
// that the regexp admits.
|
|
func ValidateBranchName(b string) error {
|
|
bad := !branchNameRe.MatchString(b) ||
|
|
strings.HasPrefix(b, "refs/") ||
|
|
b == "HEAD" ||
|
|
strings.Contains(b, "..") ||
|
|
strings.Contains(b, "//") ||
|
|
strings.Contains(b, "/.") ||
|
|
strings.HasSuffix(b, "/") ||
|
|
strings.HasSuffix(b, ".") ||
|
|
strings.HasSuffix(b, ".lock")
|
|
if bad {
|
|
return fmt.Errorf("invalid branch name %q (want a short name like feat/x, no refs/ prefix): %w", b, gitea.ErrValidation)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// validateRequiredIdentifiers rejects an empty repo identifier at the tool layer
|
|
// before it reaches the client, where it would build a trailing-empty path
|
|
// segment (`/api/v1/repos/{owner}/`) and leak gitea's bare 404 (#37). `repo` and
|
|
// `name` are always required where present — the per-repo identifier tools and
|
|
// the two create tools respectively; `owner` is already enforced by the
|
|
// allowlist check. Returns a typed ErrValidation naming the missing field.
|
|
func validateRequiredIdentifiers(dst any) error {
|
|
v := reflect.ValueOf(dst)
|
|
if v.Kind() != reflect.Pointer || v.IsNil() {
|
|
return nil
|
|
}
|
|
v = v.Elem()
|
|
if v.Kind() != reflect.Struct {
|
|
return nil
|
|
}
|
|
t := v.Type()
|
|
for i := 0; i < t.NumField(); i++ {
|
|
base, opts, _ := strings.Cut(t.Field(i).Tag.Get("json"), ",")
|
|
if base != "repo" && base != "name" {
|
|
continue
|
|
}
|
|
// An optional identifier (e.g. code_search's `repo`, empty => owner-wide
|
|
// fan-out) opts out by carrying `,omitempty`.
|
|
if strings.Contains(opts, "omitempty") {
|
|
continue
|
|
}
|
|
if v.Field(i).Kind() == reflect.String && v.Field(i).String() == "" {
|
|
return fmt.Errorf("%q is required: %w", base, gitea.ErrValidation)
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// normalizeAliases reconciles the two spellings of the repo identifier so a tool
|
|
// works whichever the caller sends. `repo` is the canonical, idiomatic name that
|
|
// identifier tools now advertise (#38); `name` is kept as a back-compat alias.
|
|
// The mapping is bidirectional: `name`->`repo` fills the new canonical for old
|
|
// callers, and `repo`->`name` still fills the two create tools (repo_create,
|
|
// create_project_from_template) whose `name` legitimately means "name of the new
|
|
// repo". `index`->`number` is kept for the issue tools whose canonical is
|
|
// `number`. An explicit canonical value always wins over its alias.
|
|
func normalizeAliases(raw json.RawMessage) json.RawMessage {
|
|
var m map[string]json.RawMessage
|
|
if err := json.Unmarshal(raw, &m); err != nil {
|
|
return raw // not a JSON object — leave untouched
|
|
}
|
|
changed := aliasInto(m, "name", "repo")
|
|
changed = aliasInto(m, "repo", "name") || changed
|
|
changed = aliasInto(m, "number", "index") || changed
|
|
if !changed {
|
|
return raw
|
|
}
|
|
if patched, err := json.Marshal(m); err == nil {
|
|
return patched
|
|
}
|
|
return raw
|
|
}
|
|
|
|
// aliasInto copies m[alias] into m[canonical] when canonical is absent or an
|
|
// empty/zero JSON value and alias carries a usable one. An explicit canonical
|
|
// value always wins. Returns true if it wrote canonical.
|
|
func aliasInto(m map[string]json.RawMessage, canonical, alias string) bool {
|
|
av, ok := m[alias]
|
|
if !ok || isEmptyJSON(av) {
|
|
return false
|
|
}
|
|
if cv, ok := m[canonical]; ok && !isEmptyJSON(cv) {
|
|
return false
|
|
}
|
|
m[canonical] = av
|
|
return true
|
|
}
|
|
|
|
func isEmptyJSON(v json.RawMessage) bool {
|
|
switch strings.TrimSpace(string(v)) {
|
|
case "", `""`, "null", "0":
|
|
return true
|
|
default:
|
|
return false
|
|
}
|
|
}
|
|
|
|
// capLimit returns a sane page size: 0 or negative → def, > 50 → 50.
|
|
func capLimit(in, def int) int {
|
|
if in <= 0 {
|
|
return def
|
|
}
|
|
if in > 50 {
|
|
return 50
|
|
}
|
|
return in
|
|
}
|