feat(mcp): brain_read + brain_edit, contained to the sync source (brain#4) (#96)
brain_read returns a note verbatim; brain_edit is str_replace with an exactly-once match, operating on the tree brain-sync commits from. An adversarial design review before implementation changed three things: 1. Governance files are not in the model-facing tool. Brain content is ingested from the outside world and reaches a model's context, and that model would hold the editor — one poisoned page could rewrite the CLAUDE.md a .dispatch-allow agent later loads as instructions. And brain-sync merges with --strategy-option=ours, so the live tree beats a human's corrective push on the conflicting lines. CLAUDE.md now goes through brain_edit_root: de-surfaced from tools/list and gated on the local static principal. The scopes are disjoint — the privileged door has the smaller reach. 2. Reads needed the same allowlist, and brain_get already lacked it: it returned verbatim session transcripts under archive/ by path. A test proved the exposure live before the fix. 3. Canonicalise before allowlisting — knowledge/../.git/hooks/post-commit would otherwise reach a hook that brain-sync executes in a pod holding the Gitea push credentials. Plus overlapping match counting, UTF-8 validation both ways, refusal while git is mid-operation, flock with the read inside the lock, atomic same-dir rename, and reindex only when frontmatter is touched. Refs brain#4. Co-authored-by: Mathias <mthbqv@gmail.com>
This commit was merged in pull request #96.
This commit is contained in:
@@ -165,7 +165,7 @@ func TestBrainUpdateReembedsNewContent(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
callResult(t, toolCall(t, srv, "brain_write", map[string]any{
|
||||
"content": "# Note\n\nthe OLD distinctive payload\n",
|
||||
"content": "# Note\n\nthe OLD distinctive payload\n",
|
||||
"filename": "n", "wing": "a", "hall": "facts",
|
||||
}))
|
||||
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/capture"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/extract"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/graphsync"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/notes"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/search"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/session"
|
||||
@@ -97,6 +98,8 @@ func (s *Server) tools() []map[string]any {
|
||||
"slug": str("optional target slug; defaults to filename minus date prefix"),
|
||||
}),
|
||||
},
|
||||
noteToolDescriptors()[0],
|
||||
noteToolDescriptors()[1],
|
||||
{
|
||||
"name": "brain_tunnel",
|
||||
"description": "Create an explicit bidirectional [[wikilink]] between two notes in different wings. Idempotent.",
|
||||
@@ -320,6 +323,15 @@ func (s *Server) brainGet(ctx context.Context, args json.RawMessage) (json.RawMe
|
||||
if target == "" {
|
||||
return nil, fmt.Errorf("id or path is required")
|
||||
}
|
||||
// Containment: the store resolves any brain-relative path, and
|
||||
// parseFrontmatter returns the WHOLE file as body when there is no
|
||||
// frontmatter fence — so without this, brain_get reads verbatim session
|
||||
// transcripts under archive/ and sessions/, which BM25 never indexes and
|
||||
// which nobody therefore thinks of as reachable. Same allowlist as
|
||||
// brain_read (brain#4).
|
||||
if _, err := notes.Resolve(s.brainDir, target, notes.ScopeNotes); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
note, err := s.store.Get(ctx, target)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -275,6 +275,15 @@ func (s *Server) handleCall(ctx context.Context, name string, args json.RawMessa
|
||||
return s.brainPromote(ctx, args)
|
||||
case "capture":
|
||||
return s.brainCapture(ctx, args)
|
||||
case "brain_read":
|
||||
return s.brainRead(ctx, args)
|
||||
case "brain_edit":
|
||||
return s.brainEdit(ctx, args)
|
||||
case "brain_edit_root":
|
||||
// Dispatches but is intentionally absent from tools/list — see
|
||||
// tools_notes.go for why a governance-file editor must not be
|
||||
// discoverable by a model.
|
||||
return s.brainEditRoot(ctx, args)
|
||||
case "brain_index":
|
||||
return s.brainIndex(ctx, args)
|
||||
case "brain_tunnel":
|
||||
|
||||
@@ -59,6 +59,7 @@ func TestServerToolsList(t *testing.T) {
|
||||
assert.ElementsMatch(t, []string{
|
||||
"brain_query", "brain_write", "brain_update", "brain_get",
|
||||
"brain_pending", "brain_promote",
|
||||
"brain_read", "brain_edit",
|
||||
"brain_tunnel",
|
||||
"brain_ingest_raw", "brain_ingest",
|
||||
"brain_answer", "brain_classify", "brain_graph", "brain_context",
|
||||
@@ -69,6 +70,13 @@ func TestServerToolsList(t *testing.T) {
|
||||
// a caller verb is cognitive load with no decision behind it (ADR-0012
|
||||
// rule 4, #78). The capability stays reachable — see the admin test below.
|
||||
assert.NotContains(t, names, "brain_index")
|
||||
|
||||
// brain_edit_root is de-surfaced for a sharper reason than brain_index:
|
||||
// brain content is ingested from the outside world and reaches a model's
|
||||
// context, so a listed governance-file editor would be a durable
|
||||
// prompt-injection path — one poisoned page rewriting the CLAUDE.md a
|
||||
// .dispatch-allow agent later loads as instructions.
|
||||
assert.NotContains(t, names, "brain_edit_root")
|
||||
}
|
||||
|
||||
func TestBrainIndexStaysCallableForAdminUseAfterDeSurfacing(t *testing.T) {
|
||||
|
||||
@@ -31,7 +31,7 @@ func captureToolDescriptor() map[string]any {
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"text": str("the insight body"), "wing": str("brain wing"),
|
||||
"hall": str("brain hall (facts/decisions/failures/hypotheses/sources)"),
|
||||
"hall": str("brain hall (facts/decisions/failures/hypotheses/sources)"),
|
||||
"supersede_slug": str("optional: slug of a prior note to revise in place instead of creating"),
|
||||
},
|
||||
"required": []string{"text", "wing", "hall"},
|
||||
@@ -41,7 +41,7 @@ func captureToolDescriptor() map[string]any {
|
||||
"properties": map[string]any{
|
||||
"repo": str("gitea repo (owner is always mathias)"), "action": str("create|close|comment"),
|
||||
"number": map[string]any{"type": "integer", "description": "issue number (close/comment)"},
|
||||
"title": str("issue title (create)"), "body": str("issue/comment body"),
|
||||
"title": str("issue title (create)"), "body": str("issue/comment body"),
|
||||
},
|
||||
"required": []string{"repo", "action"},
|
||||
}
|
||||
@@ -51,14 +51,14 @@ func captureToolDescriptor() map[string]any {
|
||||
"context": map[string]any{
|
||||
"type": "object",
|
||||
"properties": map[string]any{
|
||||
"harness": str("descriptive harness label (telemetry only, never a gate input)"),
|
||||
"harness": str("descriptive harness label (telemetry only, never a gate input)"),
|
||||
"session_ref": str("optional session reference"), "fidelity": str("live-capture|transcript-parse|agent-runlog"),
|
||||
"actor": str("acting user/agent"), "classification": str("caller-declared sensitivity: public|internal|confidential"),
|
||||
},
|
||||
},
|
||||
"insights": map[string]any{"type": "array", "items": insightItem},
|
||||
"tickets": map[string]any{"type": "array", "items": ticketItem},
|
||||
"dry_run": map[string]any{"type": "boolean", "description": "validate + return the would-be receipt, write nothing"},
|
||||
"dry_run": map[string]any{"type": "boolean", "description": "validate + return the would-be receipt, write nothing"},
|
||||
},
|
||||
}
|
||||
b, _ := json.Marshal(schema)
|
||||
|
||||
@@ -0,0 +1,145 @@
|
||||
package mcp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"strings"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/notes"
|
||||
)
|
||||
|
||||
// noteToolDescriptors are the model-facing halves of brain#4.
|
||||
//
|
||||
// brain_edit_root is deliberately absent: it dispatches but is not listed, the
|
||||
// same de-surfacing ADR-0012 rule 4 applies to brain_index (#78). Hiding the
|
||||
// verb matters here for a specific reason — brain content is ingested from the
|
||||
// outside world and reaches a model's context, so a listed governance-file
|
||||
// editor is a durable prompt-injection path: one poisoned page could rewrite
|
||||
// the CLAUDE.md that a .dispatch-allow agent later obeys. A human who knows
|
||||
// the tool exists can still call it; a model reading tools/list cannot
|
||||
// discover it.
|
||||
func noteToolDescriptors() []map[string]any {
|
||||
str := func(d string) map[string]any { return map[string]any{"type": "string", "description": d} }
|
||||
schema := func(required []string, props map[string]any) json.RawMessage {
|
||||
b, _ := json.Marshal(map[string]any{"type": "object", "required": required, "properties": props})
|
||||
return b
|
||||
}
|
||||
return []map[string]any{
|
||||
{
|
||||
"name": "brain_read",
|
||||
"description": "Return a note's raw markdown verbatim — no synthesis, no excerpting. " +
|
||||
"Use this to obtain the exact text before brain_edit; never build old_str from a " +
|
||||
"brain_query excerpt, which is truncated and normalised and will not match.",
|
||||
"inputSchema": schema([]string{"path"}, map[string]any{
|
||||
"path": str("brain-relative path under knowledge/ or wiki/, e.g. wiki/homelab/facts/x.md"),
|
||||
}),
|
||||
},
|
||||
{
|
||||
"name": "brain_edit",
|
||||
"description": "Replace exactly one occurrence of old_str with new_str in a note, leaving " +
|
||||
"every other byte identical. Fails loudly if old_str is absent or matches more than " +
|
||||
"once — extend old_str until it is unique. Does NOT stamp updated_at or supersedes " +
|
||||
"(brain_update does); it is a byte-level patch, not a metadata operation.",
|
||||
"inputSchema": schema([]string{"path", "old_str", "new_str"}, map[string]any{
|
||||
"path": str("brain-relative path under knowledge/ or wiki/"),
|
||||
"old_str": str("exact text to replace; must occur exactly once"),
|
||||
"new_str": str("replacement text; empty string deletes old_str"),
|
||||
}),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
type brainReadArgs struct {
|
||||
Path string `json:"path"`
|
||||
}
|
||||
|
||||
func (s *Server) brainRead(_ context.Context, args json.RawMessage) (json.RawMessage, error) {
|
||||
var a brainReadArgs
|
||||
if err := json.Unmarshal(args, &a); err != nil {
|
||||
return nil, fmt.Errorf("parse args: %w", err)
|
||||
}
|
||||
content, err := notes.Read(s.brainDir, a.Path)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return json.Marshal(map[string]any{"path": a.Path, "content": string(content)})
|
||||
}
|
||||
|
||||
type brainEditArgs struct {
|
||||
Path string `json:"path"`
|
||||
OldStr string `json:"old_str"`
|
||||
NewStr string `json:"new_str"`
|
||||
}
|
||||
|
||||
// brainEdit is the model-facing editor: notes only.
|
||||
func (s *Server) brainEdit(_ context.Context, args json.RawMessage) (json.RawMessage, error) {
|
||||
return s.editWithScope(args, notes.ScopeNotes)
|
||||
}
|
||||
|
||||
// brainEditRoot edits a governance file (CLAUDE.md). De-surfaced AND gated on
|
||||
// the authenticated principal: only the static local-CLI token may call it, so
|
||||
// a claude.ai-origin session — the one that may have just read an untrusted
|
||||
// page — cannot rewrite the instructions later agents load.
|
||||
func (s *Server) brainEditRoot(ctx context.Context, args json.RawMessage) (json.RawMessage, error) {
|
||||
info, ok := ctx.Value(principalKey).(principalInfo)
|
||||
if !ok || !info.viaStatic {
|
||||
return nil, fmt.Errorf("brain_edit_root requires the local static principal; " +
|
||||
"a governance file is not editable from a model-facing origin")
|
||||
}
|
||||
out, err := s.editWithScope(args, notes.ScopeRoot)
|
||||
if err == nil {
|
||||
// A durable instruction rewrite should be loud, not silent.
|
||||
slog.Warn("brain_edit_root applied", "principal", info.principal, "result", string(out))
|
||||
}
|
||||
return out, err
|
||||
}
|
||||
|
||||
func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (json.RawMessage, error) {
|
||||
var a brainEditArgs
|
||||
if err := json.Unmarshal(args, &a); err != nil {
|
||||
return nil, fmt.Errorf("parse args: %w", err)
|
||||
}
|
||||
res, err := notes.Edit(s.brainDir, a.Path, a.OldStr, a.NewStr, scope)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Reindex only when the edit could have changed what the index reads.
|
||||
// BuildWingIndex consumes title/created_at from the frontmatter block;
|
||||
// BM25 walks the filesystem live and embeddings are mtime-driven, so an
|
||||
// unconditional rebuild would buy nothing and dirty a second file —
|
||||
// _index.md being precisely the class brain-sync already needs a bespoke
|
||||
// conflict resolver for.
|
||||
if wing := wingOf(res.Path); wing != "" && touchesFrontmatter(a.OldStr, a.NewStr) {
|
||||
if ierr := brain.BuildWingIndex(s.brainDir, wing); ierr != nil {
|
||||
// Best-effort, matching every other index hook here: the write
|
||||
// already landed and will be committed, so failing the call would
|
||||
// make the caller retry an edit whose old_str no longer exists.
|
||||
res.ReindexError = ierr.Error()
|
||||
}
|
||||
}
|
||||
return json.Marshal(res)
|
||||
}
|
||||
|
||||
// wingOf returns the wing for a wiki path, or "" for knowledge/ and root.
|
||||
func wingOf(rel string) string {
|
||||
parts := strings.Split(rel, "/")
|
||||
if len(parts) < 3 || parts[0] != "wiki" {
|
||||
return ""
|
||||
}
|
||||
return parts[1]
|
||||
}
|
||||
|
||||
// touchesFrontmatter reports whether either side of the edit mentions a field
|
||||
// the wing index reads.
|
||||
func touchesFrontmatter(oldStr, newStr string) bool {
|
||||
for _, s := range []string{oldStr, newStr} {
|
||||
if strings.Contains(s, "title:") || strings.Contains(s, "created_at:") || strings.Contains(s, "---") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -0,0 +1,127 @@
|
||||
// ingestion/internal/mcp/tools_notes_test.go
|
||||
package mcp_test
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/mcp"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// ---- brain#4: brain_read / brain_edit through the JSON-RPC surface --------
|
||||
|
||||
func newBrainRepo(t *testing.T) string {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(dir, ".git"), 0o755))
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki/homelab/facts"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki/homelab/facts/n.md"),
|
||||
[]byte("---\ntitle: 'n'\n---\n\nalpha\nbeta\n"), 0o644))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(dir, "CLAUDE.md"), []byte("# rules\n"), 0o644))
|
||||
return dir
|
||||
}
|
||||
|
||||
func call(t *testing.T, srv *mcp.Server, name string, args map[string]any) map[string]any {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(http.MethodPost, "/mcp", body(t, map[string]any{
|
||||
"jsonrpc": "2.0", "id": 1, "method": "tools/call",
|
||||
"params": map[string]any{"name": name, "arguments": args},
|
||||
}))
|
||||
rr := httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, req)
|
||||
var resp map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
|
||||
return resp
|
||||
}
|
||||
|
||||
func TestBrainReadAndEdit_RoundTripThroughJSONRPC(t *testing.T) {
|
||||
// The Go functions are tested in internal/notes; this pins the transport,
|
||||
// because a tool that works in-process and is unreachable over JSON-RPC
|
||||
// is indistinguishable from a working one in a unit test.
|
||||
dir := newBrainRepo(t)
|
||||
srv := mcp.NewServer(dir, nil, nil, nil)
|
||||
|
||||
resp := call(t, srv, "brain_read", map[string]any{"path": "wiki/homelab/facts/n.md"})
|
||||
require.NotContains(t, resp, "error", "%v", resp["error"])
|
||||
|
||||
resp = call(t, srv, "brain_edit", map[string]any{
|
||||
"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA",
|
||||
})
|
||||
require.NotContains(t, resp, "error", "%v", resp["error"])
|
||||
|
||||
got, err := os.ReadFile(filepath.Join(dir, "wiki/homelab/facts/n.md"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "---\ntitle: 'n'\n---\n\nalpha\nBETA\n", string(got),
|
||||
"only the matched bytes may change")
|
||||
}
|
||||
|
||||
func TestBrainEdit_RefusesGovernanceFilesFromTheModelFacingTool(t *testing.T) {
|
||||
dir := newBrainRepo(t)
|
||||
srv := mcp.NewServer(dir, nil, nil, nil)
|
||||
|
||||
resp := call(t, srv, "brain_edit", map[string]any{
|
||||
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
||||
})
|
||||
require.Contains(t, resp, "error", "brain_edit must not reach CLAUDE.md")
|
||||
|
||||
got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "# rules\n", string(got))
|
||||
}
|
||||
|
||||
func TestBrainEditRoot_RefusedWithoutTheLocalStaticPrincipal(t *testing.T) {
|
||||
// No principal in context is the model-facing case (and the default in
|
||||
// this test server): the privileged door must stay shut.
|
||||
dir := newBrainRepo(t)
|
||||
srv := mcp.NewServer(dir, nil, nil, nil)
|
||||
|
||||
resp := call(t, srv, "brain_edit_root", map[string]any{
|
||||
"path": "CLAUDE.md", "old_str": "# rules", "new_str": "# rewritten",
|
||||
})
|
||||
require.Contains(t, resp, "error")
|
||||
|
||||
got, err := os.ReadFile(filepath.Join(dir, "CLAUDE.md"))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, "# rules\n", string(got), "governance file must be untouched")
|
||||
}
|
||||
|
||||
func TestBrainGet_CannotReachSessionTranscripts(t *testing.T) {
|
||||
// Pre-existing hole, not introduced here: api.ReadNote resolves any
|
||||
// brain-relative path and parseFrontmatter returns the WHOLE file as body
|
||||
// when there is no frontmatter fence. archive/ holds verbatim Claude
|
||||
// session transcripts (up to 500 KB) that BM25 never indexes, so they are
|
||||
// invisible to search and trivially readable by path — and CLAUDE.md
|
||||
// documents the layout.
|
||||
dir := newBrainRepo(t)
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(dir, "archive/claude-sessions/koala"), 0o755))
|
||||
require.NoError(t, os.WriteFile(
|
||||
filepath.Join(dir, "archive/claude-sessions/koala/session-1.md"),
|
||||
[]byte("verbatim transcript with tool output\n"), 0o644))
|
||||
|
||||
srv := mcp.NewServer(dir, nil, nil, nil)
|
||||
resp := call(t, srv, "brain_get", map[string]any{"path": "archive/claude-sessions/koala/session-1.md"})
|
||||
|
||||
require.Contains(t, resp, "error", "brain_get must not read outside knowledge/ and wiki/")
|
||||
assert.NotContains(t, string(mustJSON(t, resp)), "verbatim transcript")
|
||||
}
|
||||
|
||||
func TestBrainGet_StillReadsNotes(t *testing.T) {
|
||||
dir := newBrainRepo(t)
|
||||
srv := mcp.NewServer(dir, nil, nil, nil)
|
||||
|
||||
resp := call(t, srv, "brain_get", map[string]any{"path": "wiki/homelab/facts/n.md"})
|
||||
assert.NotContains(t, resp, "error", "%v", resp["error"])
|
||||
}
|
||||
|
||||
func mustJSON(t *testing.T, v any) []byte {
|
||||
t.Helper()
|
||||
b, err := json.Marshal(v)
|
||||
require.NoError(t, err)
|
||||
return b
|
||||
}
|
||||
Reference in New Issue
Block a user