feat(mcp): add brain_read + brain_edit, contained to the sync source (brain#4)
brain_read returns a note verbatim; brain_edit is str_replace with an exactly-once match. Both operate on INGEST_BRAIN_DIR, which is the pod's mount of /var/lib/supervisor/brain — the tree brain-sync commits from, so an edit reaches gitea on the next cycle rather than landing somewhere the sync cannot see it (brain#2). An adversarial design review before implementation changed three things, each of which would otherwise have shipped: 1. Governance files are NOT in the model-facing tool. The original plan allowed root *.md so CLAUDE.md had an MCP path at all — the absence of one is what pushed a session to edit via the gitea REST API on 2026-06-16 and wedge the sync. But brain content is ingested from the outside world, reaches a model's context via brain_query, and that model would hold the editor: one poisoned page could rewrite the CLAUDE.md a .dispatch-allow agent later loads as instructions. Worse, brain-sync merges with --strategy-option=ours, so the live tree wins against a human's corrective push on exactly the conflicting lines. CLAUDE.md now goes through brain_edit_root: de-surfaced from tools/list (the brain_index precedent, ADR-0012 rule 4) and gated on the local static principal, so a claude.ai-origin session is refused. The two scopes are disjoint — the privileged door has the smaller reach. 2. Reads needed the same allowlist, and brain_get already lacked it. api.ReadNote resolves any brain-relative path and parseFrontmatter returns the whole file when there is no fence, so brain_get reads verbatim session transcripts under archive/ (up to 500 KB, never BM25-indexed, so nobody thinks of them as reachable). A test proves the exposure was live; the same Resolve now gates brain_get. 3. Canonicalise before allowlisting. A prefix check on the raw argument accepts knowledge/../.git/hooks/post-commit, which Clean collapses to a git hook that brain-sync executes inside a pod holding the Gitea push credentials. Traversal, symlink escape, dotfiles, .git/, archive/, sessions/ and raw/ are refused by a static backstop that does not depend on the allowlist being right. There were no traversal tests in this repo before. Also from the review: overlapping match counting (bytes.Count says "aa" occurs once in "aaa", so a Count-based uniqueness check accepts an ambiguous edit); UTF-8 validation on both sides, because encoding/json substitutes U+FFFD and would hand back bytes the file does not contain, making every derived old_str unmatchable; refusal while .git/MERGE_HEAD or index.lock exists, since an edit racing brain-sync's merge is either hard-reset away or committed as a wrongly resolved conflict; flock plus a per-path mutex with the read taken inside the lock, because two pods share the hostPath during a RollingUpdate and uniqueness evaluated against a stale read loses an update while reporting success; same-dir *.tmp + rename + chmod 0644 + fsync of file and parent; and reindex only when the edit touches frontmatter, since BM25 walks live and embeddings are mtime-driven. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BeAp5LTscnz2W7Ubt4eJ6m
This commit is contained in:
Claude Opus 5
@@ -0,0 +1,256 @@
|
||||
package notes
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func seed(t *testing.T, dir, rel, content string) string {
|
||||
t.Helper()
|
||||
abs := filepath.Join(dir, rel)
|
||||
if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(abs, []byte(content), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return abs
|
||||
}
|
||||
|
||||
func read(t *testing.T, abs string) string {
|
||||
t.Helper()
|
||||
b, err := os.ReadFile(abs)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return string(b)
|
||||
}
|
||||
|
||||
func TestRead_ReturnsVerbatimBytes(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
const body = "---\ntitle: 'x'\n---\n\nbody with double spaces\r\nand a CRLF line\nno trailing newline"
|
||||
seed(t, dir, "knowledge/verbatim.md", body)
|
||||
|
||||
got, err := Read(dir, "knowledge/verbatim.md")
|
||||
if err != nil {
|
||||
t.Fatalf("Read: %v", err)
|
||||
}
|
||||
if string(got) != body {
|
||||
t.Errorf("Read returned %q, want the file byte-for-byte", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRead_RefusesNonUTF8RatherThanCorruptingIt(t *testing.T) {
|
||||
// The response is marshalled into a JSON string, and encoding/json
|
||||
// replaces invalid UTF-8 with U+FFFD. Returning it would mean every
|
||||
// old_str the caller derives from that region can never match, and
|
||||
// pasting it back would write the corruption to disk permanently.
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/bad.md", "ok \xff\xfe broken\n")
|
||||
|
||||
if _, err := Read(dir, "knowledge/bad.md"); err == nil {
|
||||
t.Fatal("Read accepted invalid UTF-8; it must refuse rather than silently substitute")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_ReplacesExactlyOnceAndLeavesEveryOtherByteAlone(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
const body = "---\ntitle: 'x'\n---\n\nalpha\nbeta\ngamma\n"
|
||||
abs := seed(t, dir, "knowledge/n.md", body)
|
||||
|
||||
res, err := Edit(dir, "knowledge/n.md", "beta", "BETA", ScopeNotes)
|
||||
if err != nil {
|
||||
t.Fatalf("Edit: %v", err)
|
||||
}
|
||||
want := strings.Replace(body, "beta", "BETA", 1)
|
||||
if got := read(t, abs); got != want {
|
||||
t.Errorf("file = %q, want %q", got, want)
|
||||
}
|
||||
if res.ContentHash == "" {
|
||||
t.Error("result should carry a content hash for read-after-write confirmation")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_RefusesAmbiguousAndAbsentMatchesWithoutWriting(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
const body = "alpha\nalpha\n"
|
||||
abs := seed(t, dir, "knowledge/n.md", body)
|
||||
|
||||
cases := []struct{ name, old, want string }{
|
||||
{"two matches", "alpha", "2"},
|
||||
{"no match", "delta", "0"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
_, err := Edit(dir, "knowledge/n.md", tc.old, "x", ScopeNotes)
|
||||
if err == nil {
|
||||
t.Fatal("want a loud refusal")
|
||||
}
|
||||
if !strings.Contains(err.Error(), tc.want) {
|
||||
t.Errorf("error must state the match count, got %v", err)
|
||||
}
|
||||
if got := read(t, abs); got != body {
|
||||
t.Errorf("file changed on a refused edit: %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_CountsOverlappingMatches(t *testing.T) {
|
||||
// bytes.Count counts NON-overlapping occurrences: "aa" in "aaa" is 1, so
|
||||
// a Count-based uniqueness check calls an ambiguous edit unique.
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "aaa\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "aa", "b", ScopeNotes); err == nil {
|
||||
t.Fatal("overlapping matches must be reported as ambiguous")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_RejectsDegenerateArguments(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "alpha\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "", "x", ScopeNotes); err == nil {
|
||||
t.Error("empty old_str must be rejected")
|
||||
}
|
||||
if _, err := Edit(dir, "knowledge/n.md", "alpha", "alpha", ScopeNotes); err == nil {
|
||||
t.Error("a no-op edit must be rejected rather than burning a sync cycle")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_DeletionIsLegal(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
abs := seed(t, dir, "knowledge/n.md", "keep\ndrop\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "drop\n", "", ScopeNotes); err != nil {
|
||||
t.Fatalf("Edit: %v", err)
|
||||
}
|
||||
if got := read(t, abs); got != "keep\n" {
|
||||
t.Errorf("file = %q, want %q", got, "keep\n")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_ReportsWhenOnlyLineEndingsDiffer(t *testing.T) {
|
||||
// The likeliest real failure: the caller's old_str lost its \r. Never
|
||||
// normalise silently — that breaks byte-identity — but say so, because an
|
||||
// opaque "no match" is unfixable from the caller's side.
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "alpha\r\nbeta\r\n")
|
||||
|
||||
_, err := Edit(dir, "knowledge/n.md", "alpha\nbeta", "x", ScopeNotes)
|
||||
if err == nil {
|
||||
t.Fatal("want a refusal")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "line ending") {
|
||||
t.Errorf("error should point at CRLF, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_RefusesWhileGitIsMidOperation(t *testing.T) {
|
||||
// brain-sync runs `git merge -X ours origin/main`, and aborts on conflict
|
||||
// with a hard reset. An edit landing inside that window is either erased
|
||||
// or committed as a wrongly-resolved conflict, and the caller was told it
|
||||
// succeeded.
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "alpha\n")
|
||||
|
||||
for _, marker := range []string{"MERGE_HEAD", "index.lock", "REBASE_HEAD"} {
|
||||
t.Run(marker, func(t *testing.T) {
|
||||
p := filepath.Join(dir, ".git", marker)
|
||||
if err := os.WriteFile(p, []byte("x"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer func() { _ = os.Remove(p) }()
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "alpha", "beta", ScopeNotes); err == nil {
|
||||
t.Errorf("edit accepted while %s exists", marker)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_RefusesAFileHoldingConflictMarkers(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "<<<<<<< HEAD\nours\n=======\ntheirs\n>>>>>>> origin/main\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "ours", "x", ScopeNotes); err == nil {
|
||||
t.Fatal("editing one side of an unresolved conflict must be refused")
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_ConcurrentEditsBothApply(t *testing.T) {
|
||||
// Read-modify-write with no lock loses one update AND reports success for
|
||||
// both. old_str uniqueness does not protect against it: uniqueness is
|
||||
// evaluated against the stale read.
|
||||
dir := newBrain(t)
|
||||
abs := seed(t, dir, "knowledge/n.md", "alpha\nbeta\n")
|
||||
|
||||
var wg sync.WaitGroup
|
||||
errs := make([]error, 2)
|
||||
for i, pair := range [][2]string{{"alpha", "ALPHA"}, {"beta", "BETA"}} {
|
||||
wg.Add(1)
|
||||
go func(i int, old, new string) {
|
||||
defer wg.Done()
|
||||
_, errs[i] = Edit(dir, "knowledge/n.md", old, new, ScopeNotes)
|
||||
}(i, pair[0], pair[1])
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
for i, err := range errs {
|
||||
if err != nil {
|
||||
t.Fatalf("edit %d failed: %v", i, err)
|
||||
}
|
||||
}
|
||||
if got := read(t, abs); got != "ALPHA\nBETA\n" {
|
||||
t.Errorf("file = %q, want both edits applied", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_LeavesNoTemporaryFileBehind(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "alpha\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "alpha", "beta", ScopeNotes); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
entries, err := os.ReadDir(filepath.Join(dir, "knowledge"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, e := range entries {
|
||||
if e.Name() != "note.md" && e.Name() != "n.md" {
|
||||
t.Errorf("stray file left in the tree: %q (git add -A would commit it)", e.Name())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_PreservesReadableMode(t *testing.T) {
|
||||
// Other uids write this tree (brain-fix, brain-enrich). os.CreateTemp
|
||||
// yields 0600 and rename preserves it, which would break them silently.
|
||||
dir := newBrain(t)
|
||||
abs := seed(t, dir, "knowledge/n.md", "alpha\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "alpha", "beta", ScopeNotes); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
fi, err := os.Stat(abs)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if fi.Mode().Perm() != 0o644 {
|
||||
t.Errorf("mode = %v, want 0644", fi.Mode().Perm())
|
||||
}
|
||||
}
|
||||
|
||||
func TestEdit_RefusesNonUTF8Replacement(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
seed(t, dir, "knowledge/n.md", "alpha\n")
|
||||
|
||||
if _, err := Edit(dir, "knowledge/n.md", "alpha", "bad \xff", ScopeNotes); err == nil {
|
||||
t.Fatal("new_str with invalid UTF-8 must be refused, not written")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user