Compare commits

...
Author SHA1 Message Date
mathiasandClaude Opus 5.5 cd61239451 perf(tools): linear duplicate-key scan keyed on SimpleFold orbit
PR #66 review round 2: the pairwise EqualFold loop was quadratic, 7s of
CPU for ~90k keys under the 1 MiB body cap. foldKey maps each rune to
the smallest member of its unicode.SimpleFold orbit, equal exactly when
strings.EqualFold is, so the scan is a map lookup again. The scaling
test fails on the previous code (7.1s).

Refs #65

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 07:05:00 +02:00
mathiasandClaude Opus 5.5 31a3dd3e0e fix(tools): fail closed when the key scan cannot read the input
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>
2026-10-05 07:02:27 +02:00
mathiasandClaude Opus 5.5 ae50501ba3 fix(tools): reject case-variant arg keys and ref-shaped branch names
parseArgs now refuses, before any request: two keys equal under
strings.EqualFold in any object (exact duplicates included), and a
top-level key that differs from a schema field only in case. Without
this, encoding/json's case-insensitive last-wins match let
{"branch":"feat/x","Branch":"main"} write to main while a client-side
hook read feat/x.

file_write_branch (branch) and tbd_ship (branch and base) validate the
branch name: ^[A-Za-z0-9][A-Za-z0-9._/-]*$ plus no refs/ prefix, HEAD,
"..", "//", "/.", trailing "/" or ".", or ".lock". refs/heads/main
otherwise created refs/heads/refs/heads/main and wedged API writes to
main. tbd_ship's base is the protection-probe key, so a refs/-prefixed
base would also have read as unprotected.

Closes #65

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 06:58:31 +02:00
mathiasandClaude Opus 5.5 9396b4daaa chore(context): resync derived adapters from canonical AGENT.md
CD / Lint / Test / Vet (push) Successful in 28s
CD / Build & Import (push) Successful in 14s
CD / Deploy via GitOps (push) Successful in 6s
Clears ContextAdapterDrift; canonical changed in local-dev 12f694b/218cf0d.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-04 22:49:57 +02:00
7 changed files with 318 additions and 4 deletions
+1 -1
View File
@@ -304,7 +304,7 @@ entries that age well are about *why*, *how to avoid*, and *what to do when*.
|---------|-------|-------| |---------|-------|-------|
| **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool | | **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool |
| **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same | | **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same |
| **Anything HTTP-only (curl, scripts)** | `POST https://brain-mcp.d-ma.be/query` with `{"query":"..."}` (auth via `BRAIN_MCP_TOKEN`) | `POST .../write` with `{"content":"...","filename":"..."}` | | **Anything HTTP-only (curl, scripts)** | Tailnet only: `POST https://koala.tail30472e.ts.net:8443/query` with `{"query":"..."}`, sending `BRAIN_MCP_TOKEN` (not yet enforced on REST, hyperguild#149). The public `brain-mcp.d-ma.be` serves `/mcp` only (infra#517). | `POST .../write` with `{"content":"...","wing":"...","hall":"..."}` |
| **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files | | **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files |
- **Scoping — the brain holds no confidential client content.** Nothing on the read path - **Scoping — the brain holds no confidential client content.** Nothing on the read path
+1 -1
View File
@@ -299,7 +299,7 @@ entries that age well are about *why*, *how to avoid*, and *what to do when*.
|---------|-------|-------| |---------|-------|-------|
| **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool | | **Claude Code, Claude Desktop** | `brain_query` (BM25), `brain_answer` (LLM-synth + sources) MCP tools | `brain_write` MCP tool |
| **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same | | **Crush, Pi, Antigravity, other MCP-capable** | same MCP server: `ingestion-brain` (via the `mcp__*_brain__*` namespace once authenticated) | same |
| **Anything HTTP-only (curl, scripts)** | `POST https://brain-mcp.d-ma.be/query` with `{"query":"..."}` (auth via `BRAIN_MCP_TOKEN`) | `POST .../write` with `{"content":"...","filename":"..."}` | | **Anything HTTP-only (curl, scripts)** | Tailnet only: `POST https://koala.tail30472e.ts.net:8443/query` with `{"query":"..."}`, sending `BRAIN_MCP_TOKEN` (not yet enforced on REST, hyperguild#149). The public `brain-mcp.d-ma.be` serves `/mcp` only (infra#517). | `POST .../write` with `{"content":"...","wing":"...","hall":"..."}` |
| **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files | | **Browser / human inspection** | `https://git.d-ma.be/mathias/hyperguild` → `knowledge/` and `wiki/` markdown files |
- **Scoping — the brain holds no confidential client content.** Nothing on the read path - **Scoping — the brain holds no confidential client content.** Nothing on the read path
+3
View File
@@ -63,6 +63,9 @@ func (t *FileWriteBranch) Call(ctx context.Context, raw json.RawMessage) (json.R
if args.Branch == "" { if args.Branch == "" {
return nil, fmt.Errorf("branch is required: %w", gitea.ErrValidation) return nil, fmt.Errorf("branch is required: %w", gitea.ErrValidation)
} }
if err := ValidateBranchName(args.Branch); err != nil {
return nil, err
}
if args.Message == "" { if args.Message == "" {
return nil, fmt.Errorf("message is required: %w", gitea.ErrValidation) return nil, fmt.Errorf("message is required: %w", gitea.ErrValidation)
} }
+42
View File
@@ -0,0 +1,42 @@
package tools
import (
"fmt"
"strings"
"testing"
"time"
)
// foldKey must agree with strings.EqualFold, which is how encoding/json matches.
func TestFoldKeyAgreesWithEqualFold(t *testing.T) {
pairs := [][2]string{{"branch", "Branch"}, {"sha", "ſha"}, {"k", "K"}, {"ss", "ß"}, {"a", "b"}, {"Σ", "ς"}}
for _, p := range pairs {
if got, want := foldKey(p[0]) == foldKey(p[1]), strings.EqualFold(p[0], p[1]); got != want {
t.Errorf("%q vs %q: foldKey equal=%v, EqualFold=%v", p[0], p[1], got, want)
}
}
}
// The duplicate scan is linear in the number of keys: 90k keys (about the
// 1 MiB body cap) must scan well under a second.
func TestRejectAmbiguousKeysScalesLinearly(t *testing.T) {
var b strings.Builder
b.WriteString("{")
for i := 0; i < 90_000; i++ {
if i > 0 {
b.WriteString(",")
}
fmt.Fprintf(&b, `"k%d":1`, i)
}
b.WriteString("}")
var dst struct {
Owner string `json:"owner"`
}
start := time.Now()
if err := rejectAmbiguousKeys([]byte(b.String()), &dst); err != nil {
t.Fatal(err)
}
if d := time.Since(start); d > 2*time.Second {
t.Errorf("scan of 90k keys took %v", d)
}
}
+116
View File
@@ -0,0 +1,116 @@
package tools_test
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"sync/atomic"
"testing"
"git.d-ma.be/mathias/gitea-mcp/internal/allowlist"
"git.d-ma.be/mathias/gitea-mcp/internal/gitea"
"git.d-ma.be/mathias/gitea-mcp/internal/tools"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// countingServer fails nothing by itself; it records how many requests reached
// it so a test can assert that a rejected call never touched Gitea.
func countingServer(t *testing.T) (*httptest.Server, *atomic.Int32) {
t.Helper()
var n atomic.Int32
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
n.Add(1)
w.WriteHeader(http.StatusTeapot)
}))
t.Cleanup(srv.Close)
return srv, &n
}
// #65 (1): encoding/json matches keys case-insensitively and the last one wins,
// so a client-side check reading `branch` could see a different value from the
// one the server acts on. Any two keys equal under strings.EqualFold, and any
// key that differs from a schema field only in case, are rejected before a
// request is made.
func TestParseArgsRejectsCaseVariantKeys(t *testing.T) {
cases := map[string]string{
"issue repro: branch + Branch": `{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","branch":"feat/y8","Branch":"main"}`,
"exact duplicate key": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","branch":"feat/y8","branch":"main"}`,
"case-only schema mismatch": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","BRANCH":"main"}`,
"nested object duplicate": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","branch":"feat/y8","extra":{"a":1,"A":2}}`,
// PR #66 review: a number the token scanner cannot parse as float64 must
// not make the scan give up and let the duplicate through.
"1e999 decoy before duplicate": `{"owner":"probe","repo":"t","name":"t","path":"case8.txt","content":"x","message":"m","z":1e999,"branch":"feat/y8","Branch":"main"}`,
"-1e309 decoy before exact dup": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","z":-1e309,"branch":"feat/y8","branch":"main"}`,
// encoding/json folds like strings.EqualFold: long s (U+017F) == s.
"nested long-s duplicate": `{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","branch":"feat/y8","extra":{"s":1,"\u017f":2}}`,
}
for name, raw := range cases {
t.Run(name, func(t *testing.T) {
srv, n := countingServer(t)
tool := tools.NewFileWriteBranch(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
_, err := tool.Call(context.Background(), json.RawMessage(raw))
require.Error(t, err)
assert.ErrorIs(t, err, gitea.ErrValidation)
assert.Zero(t, n.Load(), "no request may reach gitea")
})
}
}
// #65 (2): `refs/heads/main` as a branch makes Gitea create
// refs/heads/refs/heads/main, after which every API write to main 500s.
// Branch names that can create such refs are rejected before any request.
func TestBranchNameValidationRejectsBeforeRequest(t *testing.T) {
bad := []string{
"refs/heads/main", "refs/tags/v1", "a..b", "feat x", "feat\tx", "feat/",
"/feat", "-feat", ".feat", "feat//x", "feat/.x", "feat.lock", "feat.", "HEAD", "feat@{1}", "",
}
for _, b := range bad {
t.Run("file_write_branch "+b, func(t *testing.T) {
srv, n := countingServer(t)
tool := tools.NewFileWriteBranch(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
args, _ := json.Marshal(map[string]string{
"owner": "probe", "repo": "t", "path": "p", "content": "x", "message": "m", "branch": b,
})
_, err := tool.Call(context.Background(), args)
require.Error(t, err)
assert.ErrorIs(t, err, gitea.ErrValidation)
assert.Zero(t, n.Load(), "no request may reach gitea")
})
if b == "" {
continue // tbd_ship derives a branch when empty
}
t.Run("tbd_ship "+b, func(t *testing.T) {
srv, n := countingServer(t)
tool := tools.NewTBDShip(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
args, _ := json.Marshal(map[string]string{
"owner": "probe", "repo": "t", "path": "p", "content": "x", "message": "m", "branch": b,
})
_, err := tool.Call(context.Background(), args)
require.Error(t, err)
assert.ErrorIs(t, err, gitea.ErrValidation)
assert.Zero(t, n.Load(), "no request may reach gitea")
})
}
}
// tbd_ship's base is the branch-protection probe key: `refs/heads/main` would
// read as unprotected, so it is held to the same rule as the head branch.
func TestTBDShipRejectsRefsPrefixedBase(t *testing.T) {
srv, n := countingServer(t)
tool := tools.NewTBDShip(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"probe"}))
_, err := tool.Call(context.Background(), json.RawMessage(
`{"owner":"probe","repo":"t","path":"p","content":"x","message":"m","base":"refs/heads/main"}`))
require.Error(t, err)
assert.ErrorIs(t, err, gitea.ErrValidation)
assert.Zero(t, n.Load(), "no request may reach gitea")
}
func TestBranchNameValidationAcceptsOrdinaryNames(t *testing.T) {
for _, b := range []string{"main", "feat/y8", "tbd/fix-abc-1234abcd", "hold/mail-chat-infra-21", "v1.2_x"} {
t.Run(b, func(t *testing.T) {
assert.NoError(t, tools.ValidateBranchName(b))
})
}
}
+7
View File
@@ -160,6 +160,13 @@ func (t *TBDShip) Call(ctx context.Context, raw json.RawMessage) (json.RawMessag
if branch == "" { if branch == "" {
branch = deriveBranch(args.Message, args.Path, args.Content) branch = deriveBranch(args.Message, args.Path, args.Content)
} }
// Both names reach a ref: base is also the protection-probe key, so a
// `refs/heads/main` base would read as unprotected (#65).
for _, b := range []string{base, branch} {
if err := ValidateBranchName(b); err != nil {
return nil, err
}
}
deleteBranch := true deleteBranch := true
if args.DeleteBranch != nil { if args.DeleteBranch != nil {
deleteBranch = *args.DeleteBranch deleteBranch = *args.DeleteBranch
+148 -2
View File
@@ -1,10 +1,14 @@
package tools package tools
import ( import (
"bytes"
"encoding/json" "encoding/json"
"errors"
"fmt" "fmt"
"reflect" "reflect"
"regexp"
"strings" "strings"
"unicode"
"git.d-ma.be/mathias/gitea-mcp/internal/gitea" "git.d-ma.be/mathias/gitea-mcp/internal/gitea"
"git.d-ma.be/mathias/gitea-mcp/internal/registry" "git.d-ma.be/mathias/gitea-mcp/internal/registry"
@@ -22,12 +26,154 @@ func parseArgs(raw json.RawMessage, dst any) error {
if err := json.Unmarshal([]byte("{}"), dst); err != nil { if err := json.Unmarshal([]byte("{}"), dst); err != nil {
return err return err
} }
} else if err := json.Unmarshal(normalizeAliases(raw), dst); err != nil { } else {
return err // 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) 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 '{':
seen := map[string]string{}
for dec.More() {
kt, err := dec.Token()
if err != nil {
return err
}
k, _ := kt.(string)
fk := foldKey(k)
if prev, dup := seen[fk]; dup {
return fmt.Errorf("duplicate argument keys %q and %q (keys must be unique ignoring case): %w", prev, k, gitea.ErrValidation)
}
seen[fk] = 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
}
// foldKey maps every rune to the smallest member of its unicode.SimpleFold
// orbit, so foldKey(a) == foldKey(b) exactly when strings.EqualFold(a, b) —
// the folding encoding/json uses (long s == s, Kelvin sign == k). A map keyed
// on it keeps the duplicate scan linear (PR #66 review: a pairwise EqualFold
// loop cost 7s of CPU on ~90k keys).
func foldKey(s string) string {
var b strings.Builder
for _, r := range s {
min := r
for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) {
if f < min {
min = f
}
}
b.WriteRune(min)
}
return b.String()
}
// 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 // validateRequiredIdentifiers rejects an empty repo identifier at the tool layer
// before it reaches the client, where it would build a trailing-empty path // 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 // segment (`/api/v1/repos/{owner}/`) and leak gitea's bare 404 (#37). `repo` and