feat(mcp): brain_read + brain_edit, contained to the sync source (brain#4) #96
@@ -83,7 +83,7 @@ func (s *Service) Capture(ctx context.Context, in CaptureInput) (CaptureReceipt,
|
||||
declared, _ = classification.ParseLevel(in.Context.Classification)
|
||||
}
|
||||
|
||||
effective, securityEvents := s.resolveClassification(declared, in)
|
||||
effective, securityEvents, escalators := s.resolveClassification(declared, in)
|
||||
|
||||
// Server-derived origin governs the I1 gate; a caller-asserted harness
|
||||
// label that names a different zone is descriptive-only and logged as a
|
||||
@@ -107,8 +107,8 @@ func (s *Service) Capture(ctx context.Context, in CaptureInput) (CaptureReceipt,
|
||||
Items: nil, // refused before any write
|
||||
SecurityEvents: append(securityEvents, "I1 refusal: confidential capture via us-nexus origin"),
|
||||
}, AuditCentral)
|
||||
return CaptureReceipt{}, fmt.Errorf("%w: effective classification confidential through %s origin",
|
||||
ErrSovereigntyRefused, in.Context.Origin)
|
||||
return CaptureReceipt{}, fmt.Errorf("%w: effective classification confidential through %s origin (%s)",
|
||||
ErrSovereigntyRefused, in.Context.Origin, escalationReason(escalators))
|
||||
}
|
||||
|
||||
receipt := CaptureReceipt{
|
||||
@@ -221,15 +221,28 @@ func (s *Service) validate(in CaptureInput) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// resolveClassification computes the effective level (stricter of
|
||||
// declared and every target's derived level) and collects a security
|
||||
// event whenever the caller under-declared relative to a target floor.
|
||||
func (s *Service) resolveClassification(declared classification.Level, in CaptureInput) (classification.Level, []string) {
|
||||
// resolveClassification computes the effective level (stricter of declared
|
||||
// and every target's derived level), collects a security event whenever the
|
||||
// caller under-declared relative to a target floor, and names the targets
|
||||
// that carry the strictest level.
|
||||
//
|
||||
// That third return is what makes an I1 refusal actionable. A caller sending
|
||||
// twenty tickets gets one refusal; without the offending target it cannot
|
||||
// tell which repo tripped the gate, and infra#265 shows what happens next —
|
||||
// the caller concluded the tickets ARRAY was broken, built a fallback of
|
||||
// twenty individual calls, and filed a bug against the wrong thing.
|
||||
func (s *Service) resolveClassification(declared classification.Level, in CaptureInput) (classification.Level, []string, []string) {
|
||||
effective := declared
|
||||
var events []string
|
||||
derivedFor := map[string]classification.Level{}
|
||||
var order []string
|
||||
consider := func(kind classification.TargetKind, name string) {
|
||||
derived := s.policy.Derive(classification.Target{Kind: kind, Name: name})
|
||||
effective = classification.Stricter(effective, derived)
|
||||
if _, seen := derivedFor[name]; !seen {
|
||||
derivedFor[name] = derived
|
||||
order = append(order, name)
|
||||
}
|
||||
if declared < derived {
|
||||
events = append(events, fmt.Sprintf("classification under-declared: declared=%s target=%s(%s) derived=%s",
|
||||
declared, name, kindString(kind), derived))
|
||||
@@ -241,7 +254,28 @@ func (s *Service) resolveClassification(declared classification.Level, in Captur
|
||||
for _, tk := range in.Tickets {
|
||||
consider(classification.RepoTarget, tk.Repo)
|
||||
}
|
||||
return effective, events
|
||||
|
||||
// Only targets that actually reach the effective level are named: at
|
||||
// twenty tickets, listing the innocent ones is noise that hides the one
|
||||
// that matters.
|
||||
var escalators []string
|
||||
for _, name := range order {
|
||||
if derivedFor[name] == effective && effective > declared {
|
||||
escalators = append(escalators, name)
|
||||
}
|
||||
}
|
||||
return effective, events, escalators
|
||||
}
|
||||
|
||||
// escalationReason renders why the effective classification landed where it
|
||||
// did, for an operator reading a refusal. Naming the caller's own declaration
|
||||
// matters as much as naming a target: "you asked for this" and "a repo you
|
||||
// touched required it" call for different fixes.
|
||||
func escalationReason(escalators []string) string {
|
||||
if len(escalators) == 0 {
|
||||
return "declared by the caller; no target required it"
|
||||
}
|
||||
return "required by: " + strings.Join(escalators, ", ")
|
||||
}
|
||||
|
||||
func (s *Service) persistInsight(ctx context.Context, ins Insight) (InsightResult, string, error) {
|
||||
|
||||
@@ -3,6 +3,7 @@ package capture
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -59,12 +60,18 @@ type fakeTracker struct {
|
||||
closed []int
|
||||
comments []int
|
||||
err error
|
||||
// failOnTitle makes CreateIssue fail for one specific title, so a
|
||||
// partial-failure receipt can be exercised without failing every item.
|
||||
failOnTitle string
|
||||
}
|
||||
|
||||
func (f *fakeTracker) CreateIssue(_ context.Context, repo, title, _ string) (IssueRef, error) {
|
||||
if f.err != nil {
|
||||
return IssueRef{}, f.err
|
||||
}
|
||||
if f.failOnTitle != "" && title == f.failOnTitle {
|
||||
return IssueRef{}, errors.New("tracker refused this one")
|
||||
}
|
||||
f.created = append(f.created, repo+":"+title)
|
||||
return IssueRef{Repo: repo, Number: 100 + len(f.created), URL: "https://git/" + repo + "/issues/x"}, nil
|
||||
}
|
||||
@@ -87,7 +94,9 @@ func (f *fakeTracker) CommentIssue(_ context.Context, repo string, number int, _
|
||||
|
||||
// fakePolicy derives from an explicit map; default Internal so tests pin
|
||||
// behaviour without depending on the real defaulting.
|
||||
type fakePolicy struct{ tags map[string]classification.Level }
|
||||
type fakePolicy struct {
|
||||
tags map[string]classification.Level
|
||||
}
|
||||
|
||||
func (p fakePolicy) Derive(t classification.Target) classification.Level {
|
||||
if lvl, ok := p.tags[t.Name]; ok {
|
||||
@@ -434,3 +443,98 @@ func TestCaptureDryRunSkipsAuditGate(t *testing.T) {
|
||||
assert.True(t, rec.DryRun)
|
||||
assert.Empty(t, au.entries)
|
||||
}
|
||||
|
||||
// ---- hyperguild#91 part 1 -------------------------------------------------
|
||||
//
|
||||
// The reported symptom was "capture with a tickets array fails" (infra#265:
|
||||
// 20 tickets errored, 5 errored, caller fell back to 20 individual
|
||||
// issue_create calls). The array was never the failing dimension — every
|
||||
// ticket targeted a client repo, and the I1 gate refuses that through a
|
||||
// us-nexus origin at ANY length. The refusal did not say which target
|
||||
// escalated the classification, so it read as a malfunction.
|
||||
|
||||
func TestCapture_TicketsArrayWorksAtLengthOne(t *testing.T) {
|
||||
b, tr, au := &fakeBrain{}, &fakeTracker{}, &fakeAudit{}
|
||||
svc := newSvc(b, tr, fakePolicy{}, au)
|
||||
|
||||
rec, err := svc.Capture(context.Background(), CaptureInput{
|
||||
Context: baseCtx(),
|
||||
Tickets: []Ticket{{Repo: "hyperguild", Action: "create", Title: "one", Body: "b"}},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rec.Tickets, 1)
|
||||
assert.True(t, rec.Tickets[0].OK)
|
||||
assert.Len(t, tr.created, 1)
|
||||
assert.Empty(t, rec.Errors)
|
||||
}
|
||||
|
||||
func TestCapture_TicketsArrayWorksAtLengthMany(t *testing.T) {
|
||||
b, tr, au := &fakeBrain{}, &fakeTracker{}, &fakeAudit{}
|
||||
svc := newSvc(b, tr, fakePolicy{}, au)
|
||||
|
||||
in := CaptureInput{Context: baseCtx()}
|
||||
for i := range 20 {
|
||||
in.Tickets = append(in.Tickets, Ticket{
|
||||
Repo: "hyperguild", Action: "create", Title: fmt.Sprintf("t%02d", i), Body: "b",
|
||||
})
|
||||
}
|
||||
|
||||
rec, err := svc.Capture(context.Background(), in)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, rec.Tickets, 20, "one receipt entry per ticket")
|
||||
for i, r := range rec.Tickets {
|
||||
assert.True(t, r.OK, "ticket %d failed: %+v", i, rec.Errors)
|
||||
}
|
||||
assert.Len(t, tr.created, 20)
|
||||
assert.Empty(t, rec.Errors)
|
||||
}
|
||||
|
||||
func TestCapture_OneFailingTicketDoesNotSinkTheRest(t *testing.T) {
|
||||
// infra#265 asked explicitly: "if ticket 3 of 5 fails, what happened to
|
||||
// 1 and 2?" They land, and the receipt says so per item.
|
||||
b, au := &fakeBrain{}, &fakeAudit{}
|
||||
tr := &fakeTracker{failOnTitle: "boom"}
|
||||
svc := newSvc(b, tr, fakePolicy{}, au)
|
||||
|
||||
rec, err := svc.Capture(context.Background(), CaptureInput{
|
||||
Context: baseCtx(),
|
||||
Tickets: []Ticket{
|
||||
{Repo: "hyperguild", Action: "create", Title: "ok-1", Body: "b"},
|
||||
{Repo: "hyperguild", Action: "create", Title: "boom", Body: "b"},
|
||||
{Repo: "hyperguild", Action: "create", Title: "ok-3", Body: "b"},
|
||||
},
|
||||
})
|
||||
require.NoError(t, err, "a per-item failure is a partial receipt, not a call error")
|
||||
require.Len(t, rec.Tickets, 3)
|
||||
assert.True(t, rec.Tickets[0].OK)
|
||||
assert.False(t, rec.Tickets[1].OK)
|
||||
assert.True(t, rec.Tickets[2].OK)
|
||||
require.Len(t, rec.Errors, 1)
|
||||
assert.Contains(t, rec.Errors[0].Item, "ticket[1]")
|
||||
}
|
||||
|
||||
func TestCapture_I1RefusalNamesWhatEscalatedTheClassification(t *testing.T) {
|
||||
// The actual defect behind infra#265. Without the offending target in
|
||||
// the message, a caller with 20 tickets cannot tell which repo tripped
|
||||
// the gate — and reasonably concludes the array itself is broken.
|
||||
b, tr, au := &fakeBrain{}, &fakeTracker{}, &fakeAudit{}
|
||||
pol := fakePolicy{tags: map[string]classification.Level{"trucker-ecommerce": classification.Confidential}}
|
||||
svc := newSvc(b, tr, pol, au)
|
||||
|
||||
ctx := baseCtx()
|
||||
ctx.Origin = ZoneUSNexus
|
||||
_, err := svc.Capture(context.Background(), CaptureInput{
|
||||
Context: ctx,
|
||||
Tickets: []Ticket{
|
||||
{Repo: "hyperguild", Action: "create", Title: "fine", Body: "b"},
|
||||
{Repo: "trucker-ecommerce", Action: "create", Title: "client work", Body: "b"},
|
||||
},
|
||||
})
|
||||
|
||||
require.ErrorIs(t, err, ErrSovereigntyRefused)
|
||||
assert.Contains(t, err.Error(), "trucker-ecommerce",
|
||||
"the refusal must name the target that escalated the classification")
|
||||
assert.NotContains(t, err.Error(), "hyperguild",
|
||||
"only the offending target, or the message is noise at 20 tickets")
|
||||
assert.Empty(t, tr.created, "refused before any write")
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -0,0 +1,276 @@
|
||||
package notes
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
"unicode/utf8"
|
||||
)
|
||||
|
||||
// maxNoteBytes caps what Read will hand back. Generous for knowledge/ and
|
||||
// wiki/ (the largest note today is far under it) and small enough that a
|
||||
// pathological file cannot flood a caller's context.
|
||||
const maxNoteBytes = 256 << 10
|
||||
|
||||
// Errors callers distinguish.
|
||||
var (
|
||||
ErrNoMatch = errors.New("notes: old_str not found")
|
||||
ErrAmbiguous = errors.New("notes: old_str matches more than once")
|
||||
ErrNotUTF8 = errors.New("notes: content is not valid UTF-8")
|
||||
ErrGitBusy = errors.New("notes: a git operation is in progress")
|
||||
ErrTooLarge = errors.New("notes: file exceeds the readable size limit")
|
||||
ErrDegenerate = errors.New("notes: edit is degenerate")
|
||||
ErrInConflict = errors.New("notes: file contains unresolved conflict markers")
|
||||
)
|
||||
|
||||
// pathLocks serialises edits per file within this process. The flock below
|
||||
// covers the cross-process case (two pods share the hostPath during a rolling
|
||||
// update, and brain-fix/brain-enrich write the same tree).
|
||||
var pathLocks sync.Map // string -> *sync.Mutex
|
||||
|
||||
// Result is what a successful edit reports.
|
||||
type Result struct {
|
||||
Path string `json:"path"`
|
||||
ContentHash string `json:"content_hash"`
|
||||
BytesBefore int `json:"bytes_before"`
|
||||
BytesAfter int `json:"bytes_after"`
|
||||
ReindexError string `json:"reindex_error,omitempty"`
|
||||
}
|
||||
|
||||
// Read returns a note's bytes verbatim.
|
||||
//
|
||||
// No frontmatter parsing, no excerpting, no normalisation: the entire purpose
|
||||
// is to give a caller an exact basis for an edit. An excerpt from brain_query
|
||||
// would be truncated and normalised, and every old_str built from one would
|
||||
// miss.
|
||||
func Read(brainDir, path string) ([]byte, error) {
|
||||
t, err := Resolve(brainDir, path, ScopeNotes)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return readContained(t)
|
||||
}
|
||||
|
||||
// ReadScoped is Read for a caller already holding the privileged scope.
|
||||
func ReadScoped(brainDir, path string, scope Scope) ([]byte, error) {
|
||||
t, err := Resolve(brainDir, path, scope)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return readContained(t)
|
||||
}
|
||||
|
||||
func readContained(t Target) ([]byte, error) {
|
||||
fi, err := os.Stat(t.Abs)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("notes: stat %s: %w", t.Rel, err)
|
||||
}
|
||||
if fi.Size() > maxNoteBytes {
|
||||
return nil, fmt.Errorf("%w: %s is %d bytes (limit %d)", ErrTooLarge, t.Rel, fi.Size(), maxNoteBytes)
|
||||
}
|
||||
b, err := os.ReadFile(t.Abs)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("notes: read %s: %w", t.Rel, err)
|
||||
}
|
||||
// The MCP response marshals this into a JSON string, and encoding/json
|
||||
// substitutes U+FFFD for invalid UTF-8 — silently returning different
|
||||
// bytes than the file holds, which defeats "verbatim" entirely.
|
||||
if !utf8.Valid(b) {
|
||||
return nil, fmt.Errorf("%w: %s", ErrNotUTF8, t.Rel)
|
||||
}
|
||||
return b, nil
|
||||
}
|
||||
|
||||
// Edit replaces exactly one occurrence of oldStr with newStr, in place.
|
||||
//
|
||||
// Fails loudly and writes nothing on: zero matches, more than one match
|
||||
// (counted with overlap), degenerate arguments, invalid UTF-8, a git operation
|
||||
// in flight, or unresolved conflict markers in the target.
|
||||
func Edit(brainDir, path, oldStr, newStr string, scope Scope) (Result, error) {
|
||||
t, err := Resolve(brainDir, path, scope)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
if oldStr == "" {
|
||||
return Result{}, fmt.Errorf("%w: old_str is empty", ErrDegenerate)
|
||||
}
|
||||
if oldStr == newStr {
|
||||
return Result{}, fmt.Errorf("%w: old_str equals new_str", ErrDegenerate)
|
||||
}
|
||||
if !utf8.ValidString(newStr) {
|
||||
return Result{}, fmt.Errorf("%w: new_str", ErrNotUTF8)
|
||||
}
|
||||
// brain#2 guard: refuse to write to a tree the sync does not commit from.
|
||||
if err := assertSyncSource(brainDir); err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
if err := assertGitIdle(brainDir); err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
|
||||
mu := lockFor(t.Abs)
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
|
||||
unlock, err := flockFile(t.Abs)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
// Read INSIDE the lock: the match count must be evaluated against the
|
||||
// bytes we are about to overwrite, or a concurrent edit is silently lost
|
||||
// while both callers are told they succeeded.
|
||||
before, err := readContained(t)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
if hasConflictMarkers(before) {
|
||||
return Result{}, fmt.Errorf("%w: %s", ErrInConflict, t.Rel)
|
||||
}
|
||||
|
||||
switch n := countOverlapping(string(before), oldStr); {
|
||||
case n == 0:
|
||||
return Result{}, fmt.Errorf("%w in %s (0 matches)%s", ErrNoMatch, t.Rel, crlfHint(string(before), oldStr))
|
||||
case n > 1:
|
||||
return Result{}, fmt.Errorf("%w in %s (%d matches) — extend old_str until it is unique",
|
||||
ErrAmbiguous, t.Rel, n)
|
||||
}
|
||||
|
||||
after := strings.Replace(string(before), oldStr, newStr, 1)
|
||||
if err := writeAtomic(t.Abs, []byte(after)); err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
|
||||
sum := sha256.Sum256([]byte(after))
|
||||
return Result{
|
||||
Path: t.Rel,
|
||||
ContentHash: hex.EncodeToString(sum[:]),
|
||||
BytesBefore: len(before),
|
||||
BytesAfter: len(after),
|
||||
}, nil
|
||||
}
|
||||
|
||||
// countOverlapping counts occurrences allowing overlap. bytes.Count does not:
|
||||
// "aa" occurs once in "aaa" by its reckoning, so a Count-based uniqueness
|
||||
// check accepts an ambiguous edit as unique.
|
||||
func countOverlapping(s, sub string) int {
|
||||
var n, from int
|
||||
for {
|
||||
i := strings.Index(s[from:], sub)
|
||||
if i < 0 {
|
||||
return n
|
||||
}
|
||||
n++
|
||||
from += i + 1
|
||||
}
|
||||
}
|
||||
|
||||
// crlfHint reports whether the caller's old_str would have matched if line
|
||||
// endings agreed. It never normalises — that would break byte-identity — but
|
||||
// an opaque "no match" is unfixable from the caller's side, and a stripped \r
|
||||
// is the likeliest cause.
|
||||
func crlfHint(content, oldStr string) string {
|
||||
if strings.Contains(content, "\r\n") && !strings.Contains(oldStr, "\r\n") {
|
||||
if strings.Contains(strings.ReplaceAll(content, "\r\n", "\n"), oldStr) {
|
||||
return " — the file uses CRLF line endings and old_str does not; include the \\r"
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func hasConflictMarkers(b []byte) bool {
|
||||
s := string(b)
|
||||
return strings.Contains(s, "\n<<<<<<< ") || strings.HasPrefix(s, "<<<<<<< ")
|
||||
}
|
||||
|
||||
// assertGitIdle refuses while brain-sync (or brain-fix, or a human) has a git
|
||||
// operation in flight. An edit landing between `git merge` and its abort is
|
||||
// either hard-reset away or committed as a wrongly resolved conflict — and the
|
||||
// caller was told it succeeded.
|
||||
func assertGitIdle(brainDir string) error {
|
||||
for _, marker := range []string{"MERGE_HEAD", "REBASE_HEAD", "CHERRY_PICK_HEAD", "index.lock"} {
|
||||
if _, err := os.Stat(filepath.Join(brainDir, ".git", marker)); err == nil {
|
||||
return fmt.Errorf("%w (.git/%s present) — retry after the sync settles", ErrGitBusy, marker)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func lockFor(abs string) *sync.Mutex {
|
||||
v, _ := pathLocks.LoadOrStore(abs, &sync.Mutex{})
|
||||
return v.(*sync.Mutex)
|
||||
}
|
||||
|
||||
// flockFile takes an advisory lock covering other processes sharing the
|
||||
// hostPath — during a RollingUpdate two ingestion pods do.
|
||||
func flockFile(abs string) (func(), error) {
|
||||
f, err := os.OpenFile(abs+".lock", os.O_CREATE|os.O_RDWR, 0o644)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("notes: lock %s: %w", filepath.Base(abs), err)
|
||||
}
|
||||
if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
|
||||
_ = f.Close()
|
||||
return nil, fmt.Errorf("notes: flock %s: %w", filepath.Base(abs), err)
|
||||
}
|
||||
return func() {
|
||||
_ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
|
||||
_ = f.Close()
|
||||
_ = os.Remove(abs + ".lock")
|
||||
}, nil
|
||||
}
|
||||
|
||||
// writeAtomic replaces the file via a same-directory temp + rename, then
|
||||
// fsyncs the directory so the rename survives a node crash.
|
||||
//
|
||||
// Same directory because a rename across the hostPath boundary fails with
|
||||
// EXDEV; the ".tmp" suffix because the tree's .gitignore already covers it, so
|
||||
// a leaked temp is not committed by `git add -A`.
|
||||
func writeAtomic(abs string, content []byte) error {
|
||||
dir := filepath.Dir(abs)
|
||||
tmp, err := os.CreateTemp(dir, filepath.Base(abs)+".*.tmp")
|
||||
if err != nil {
|
||||
return fmt.Errorf("notes: temp file: %w", err)
|
||||
}
|
||||
tmpName := tmp.Name()
|
||||
defer func() { _ = os.Remove(tmpName) }() // no-op once renamed
|
||||
|
||||
if _, err := tmp.Write(content); err != nil {
|
||||
_ = tmp.Close()
|
||||
return fmt.Errorf("notes: write temp: %w", err)
|
||||
}
|
||||
if err := tmp.Sync(); err != nil {
|
||||
_ = tmp.Close()
|
||||
return fmt.Errorf("notes: sync temp: %w", err)
|
||||
}
|
||||
if err := tmp.Close(); err != nil {
|
||||
return fmt.Errorf("notes: close temp: %w", err)
|
||||
}
|
||||
// CreateTemp makes 0600; every other writer of this tree uses 0644 and
|
||||
// other uids read it.
|
||||
if err := os.Chmod(tmpName, 0o644); err != nil {
|
||||
return fmt.Errorf("notes: chmod temp: %w", err)
|
||||
}
|
||||
if err := os.Rename(tmpName, abs); err != nil {
|
||||
return fmt.Errorf("notes: rename: %w", err)
|
||||
}
|
||||
return fsyncDir(dir)
|
||||
}
|
||||
|
||||
func fsyncDir(dir string) error {
|
||||
d, err := os.Open(dir)
|
||||
if err != nil {
|
||||
return fmt.Errorf("notes: open dir: %w", err)
|
||||
}
|
||||
defer func() { _ = d.Close() }()
|
||||
if err := d.Sync(); err != nil {
|
||||
return fmt.Errorf("notes: fsync dir: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,201 @@
|
||||
// Package notes provides containment-checked read and surgical edit of brain
|
||||
// notes on the live tree that brain-sync commits from.
|
||||
//
|
||||
// Everything here is defensive for one reason: brain content is ingested from
|
||||
// the outside world, that content reaches a model's context via brain_query /
|
||||
// brain_context, and these primitives are callable by that model. A path bug
|
||||
// is therefore not a path bug — it is an outside author choosing a file.
|
||||
package notes
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Scope is which part of the tree a caller is allowed to touch.
|
||||
type Scope int
|
||||
|
||||
const (
|
||||
// ScopeNotes is the model-facing scope: markdown under knowledge/ and
|
||||
// wiki/ only.
|
||||
ScopeNotes Scope = iota
|
||||
// ScopeRoot admits the named governance files and NOTHING else. It exists so
|
||||
// there is an MCP path to CLAUDE.md at all — the absence of one is what
|
||||
// pushed a session to edit via the gitea REST API on 2026-06-16, which
|
||||
// never reached the live tree and wedged brain-sync for hours. It is NOT
|
||||
// reachable from the model-facing toolset; see tools_notes.go.
|
||||
ScopeRoot
|
||||
)
|
||||
|
||||
// rootFiles is an exact-name allowlist, deliberately not a "root *.md" glob.
|
||||
//
|
||||
// Two reasons. log.md is gitignored, so an edit to it applies to the live tree
|
||||
// and never reaches gitea — the exact shape of brain#2, admitted by a glob. And
|
||||
// a glob auto-admits whatever governance file appears next (AGENTS.md, a
|
||||
// bootstrap doc) with nobody reviewing that decision.
|
||||
var rootFiles = map[string]bool{"CLAUDE.md": true}
|
||||
|
||||
// noteDirs are the two trees that hold notes.
|
||||
var noteDirs = []string{"knowledge/", "wiki/"}
|
||||
|
||||
// deniedPrefixes are refused in every scope, independent of the allowlist.
|
||||
//
|
||||
// A static backstop rather than a consequence of the allowlist being right:
|
||||
// .git/hooks/post-commit is executed by brain-sync inside a pod holding the
|
||||
// Gitea push credentials, and archive/ + sessions/ hold verbatim session
|
||||
// transcripts that BM25 never indexes and so nobody thinks of as reachable.
|
||||
var deniedPrefixes = []string{".git/", "archive/", "sessions/", "raw/", "training-data/", ".obsidian/"}
|
||||
|
||||
// Errors are sentinels so callers can distinguish a refusal from an I/O fault.
|
||||
var (
|
||||
ErrOutsideBrain = errors.New("notes: path escapes the brain dir")
|
||||
ErrNotAllowed = errors.New("notes: path is not editable through this tool")
|
||||
ErrNotSyncSource = errors.New("notes: brain dir is not the sync source")
|
||||
)
|
||||
|
||||
// Target is a resolved, contained path.
|
||||
type Target struct {
|
||||
Rel string // brain-relative, cleaned
|
||||
Abs string // absolute, symlink-checked
|
||||
}
|
||||
|
||||
// Resolve canonicalises path, then checks it against the scope's allowlist,
|
||||
// then re-checks containment after symlink evaluation.
|
||||
//
|
||||
// Order matters and is the point: a prefix test on the raw argument accepts
|
||||
// "knowledge/../.git/hooks/post-commit", because Clean only collapses it
|
||||
// afterwards.
|
||||
func Resolve(brainDir, path string, scope Scope) (Target, error) {
|
||||
// Note: the sync-source assertion lives on the WRITE path (Edit), not
|
||||
// here. brain#2 was an edit landing where the sync could not commit it; a
|
||||
// read from such a tree is merely a read.
|
||||
|
||||
// 1. Canonicalise. Rooting at "/" makes Clean collapse every ".." that
|
||||
// would otherwise escape, and ToSlash normalises separators first.
|
||||
clean := strings.TrimPrefix(filepath.Clean("/"+filepath.ToSlash(path)), "/")
|
||||
if clean == "" || clean == "." {
|
||||
return Target{}, fmt.Errorf("%w: empty path", ErrNotAllowed)
|
||||
}
|
||||
|
||||
// 2. Static denials, before any allowlist reasoning.
|
||||
for _, d := range deniedPrefixes {
|
||||
if strings.HasPrefix(clean+"/", d) {
|
||||
return Target{}, fmt.Errorf("%w: %q is not note content", ErrNotAllowed, clean)
|
||||
}
|
||||
}
|
||||
if strings.HasPrefix(filepath.Base(clean), ".") {
|
||||
return Target{}, fmt.Errorf("%w: dotfiles are not editable", ErrNotAllowed)
|
||||
}
|
||||
|
||||
// 3. Scope allowlist.
|
||||
if err := allow(clean, scope); err != nil {
|
||||
return Target{}, err
|
||||
}
|
||||
|
||||
// 4. Containment, lexically.
|
||||
abs := filepath.Join(brainDir, filepath.FromSlash(clean))
|
||||
if !within(brainDir, abs) {
|
||||
return Target{}, fmt.Errorf("%w: %q", ErrOutsideBrain, path)
|
||||
}
|
||||
|
||||
// 5. Containment again, after resolving symlinks. The tree is Syncthing
|
||||
// managed, so a link can appear without anyone adding one on purpose;
|
||||
// a symlinked note or directory would otherwise read and write outside
|
||||
// the brain entirely.
|
||||
if err := assertNoSymlinkEscape(brainDir, abs); err != nil {
|
||||
return Target{}, err
|
||||
}
|
||||
|
||||
return Target{Rel: clean, Abs: abs}, nil
|
||||
}
|
||||
|
||||
// allow enforces the scope's allowlist. The two scopes are DISJOINT, not
|
||||
// nested: the privileged root scope reaches governance files and nothing else,
|
||||
// so the tool that carries the higher privilege carries the smaller reach.
|
||||
func allow(clean string, scope Scope) error {
|
||||
if scope == ScopeRoot {
|
||||
if !rootFiles[clean] {
|
||||
return fmt.Errorf("%w: %q is not a governance file", ErrNotAllowed, clean)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !isNotePath(clean) {
|
||||
return fmt.Errorf("%w: %q is outside knowledge/ and wiki/", ErrNotAllowed, clean)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func isNotePath(clean string) bool {
|
||||
var inDir bool
|
||||
for _, d := range noteDirs {
|
||||
if strings.HasPrefix(clean, d) {
|
||||
inDir = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !inDir || filepath.Ext(clean) != ".md" {
|
||||
return false
|
||||
}
|
||||
// _index.md is generated per wing, and brain-sync's conflict resolver
|
||||
// deliberately resolves it in origin's favour — an edit here is reverted
|
||||
// by design, so accepting one would be a lie.
|
||||
return filepath.Base(clean) != "_index.md"
|
||||
}
|
||||
|
||||
func within(root, abs string) bool {
|
||||
rel, err := filepath.Rel(root, abs)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))
|
||||
}
|
||||
|
||||
// assertNoSymlinkEscape resolves the deepest existing ancestor of abs and
|
||||
// requires it to still be inside brainDir. The file itself need not exist
|
||||
// (a create is legal); its directory chain must be real and contained.
|
||||
func assertNoSymlinkEscape(brainDir, abs string) error {
|
||||
if fi, err := os.Lstat(abs); err == nil && fi.Mode()&os.ModeSymlink != 0 {
|
||||
return fmt.Errorf("%w: %q is a symlink", ErrNotAllowed, abs)
|
||||
}
|
||||
|
||||
realRoot, err := filepath.EvalSymlinks(brainDir)
|
||||
if err != nil {
|
||||
return fmt.Errorf("notes: resolve brain dir: %w", err)
|
||||
}
|
||||
|
||||
dir := filepath.Dir(abs)
|
||||
for {
|
||||
resolved, err := filepath.EvalSymlinks(dir)
|
||||
if err == nil {
|
||||
if !within(realRoot, resolved) {
|
||||
return fmt.Errorf("%w: %q resolves outside the brain dir", ErrOutsideBrain, dir)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !os.IsNotExist(err) {
|
||||
return fmt.Errorf("notes: resolve %q: %w", dir, err)
|
||||
}
|
||||
parent := filepath.Dir(dir)
|
||||
if parent == dir {
|
||||
return fmt.Errorf("%w: no existing ancestor for %q", ErrOutsideBrain, abs)
|
||||
}
|
||||
dir = parent
|
||||
}
|
||||
}
|
||||
|
||||
// assertSyncSource refuses a brain dir that is not a git working tree.
|
||||
//
|
||||
// brain#2 was an edit landing on a directory the sync does not commit from.
|
||||
// That cannot be prevented by review — a wrong INGEST_BRAIN_DIR in a future
|
||||
// manifest reintroduces it while every unit test still passes — so it is
|
||||
// asserted at runtime, the same check brain-sync itself makes before running.
|
||||
func assertSyncSource(brainDir string) error {
|
||||
if fi, err := os.Stat(filepath.Join(brainDir, ".git")); err != nil || !fi.IsDir() {
|
||||
return fmt.Errorf("%w: %q has no .git, so an edit there would never be committed",
|
||||
ErrNotSyncSource, brainDir)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,146 @@
|
||||
package notes
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// newBrain lays out a tree shaped like the live one: a git dir, note dirs,
|
||||
// and the non-note areas that must never be reachable.
|
||||
func newBrain(t *testing.T) string {
|
||||
t.Helper()
|
||||
dir := t.TempDir()
|
||||
for _, d := range []string{
|
||||
".git/hooks", "knowledge", "wiki/homelab/facts", "archive/claude-sessions/koala",
|
||||
"sessions", "raw", ".obsidian",
|
||||
} {
|
||||
if err := os.MkdirAll(filepath.Join(dir, d), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
write := func(p, s string) {
|
||||
t.Helper()
|
||||
if err := os.WriteFile(filepath.Join(dir, p), []byte(s), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
write("CLAUDE.md", "# instructions\n")
|
||||
write("log.md", "ignored scratch\n")
|
||||
write("knowledge/note.md", "# note\n")
|
||||
write("wiki/homelab/facts/f.md", "# fact\n")
|
||||
write("wiki/homelab/_index.md", "# index\n")
|
||||
write("archive/claude-sessions/koala/session-1.md", "verbatim transcript\n")
|
||||
write("sessions/s.jsonl", "{}\n")
|
||||
write(".git/hooks/post-commit", "#!/bin/sh\n")
|
||||
return dir
|
||||
}
|
||||
|
||||
func TestResolve_AcceptsNotePaths(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
for _, p := range []string{"knowledge/note.md", "wiki/homelab/facts/f.md"} {
|
||||
got, err := Resolve(dir, p, ScopeNotes)
|
||||
if err != nil {
|
||||
t.Errorf("Resolve(%q): %v", p, err)
|
||||
continue
|
||||
}
|
||||
if !strings.HasPrefix(got.Abs, dir) {
|
||||
t.Errorf("Resolve(%q).Abs = %q, want inside %q", p, got.Abs, dir)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolve_RefusesEscapesAndNonNoteAreas(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
|
||||
// Canonicalisation must happen BEFORE the allowlist check. A prefix test
|
||||
// on the raw argument accepts every one of these: "knowledge/..%2f" style
|
||||
// inputs collapse to .git, and .git/hooks/post-commit is executed by
|
||||
// brain-sync inside a pod holding the Gitea push credentials.
|
||||
refuse := []string{
|
||||
"knowledge/../.git/hooks/post-commit",
|
||||
"wiki/../.git/config",
|
||||
"./knowledge/../.git/config",
|
||||
"knowledge//../.git/config",
|
||||
"../../etc/passwd",
|
||||
"/etc/passwd",
|
||||
".git/config",
|
||||
"archive/claude-sessions/koala/session-1.md", // verbatim transcripts
|
||||
"sessions/s.jsonl",
|
||||
"raw/x.md",
|
||||
".obsidian/workspace.json",
|
||||
"log.md", // gitignored: an edit here would never sync (brain#2)
|
||||
"wiki/homelab/_index.md", // generated; brain-sync resolves it in origin's favour
|
||||
"knowledge/note.txt", // not markdown
|
||||
"CLAUDE.md", // root file — not reachable in the notes scope
|
||||
}
|
||||
for _, p := range refuse {
|
||||
if _, err := Resolve(dir, p, ScopeNotes); err == nil {
|
||||
t.Errorf("Resolve(%q) = nil error, want refusal", p)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolve_RootScopeAdmitsOnlyNamedFiles(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
|
||||
if _, err := Resolve(dir, "CLAUDE.md", ScopeRoot); err != nil {
|
||||
t.Errorf("Resolve(CLAUDE.md, ScopeRoot): %v", err)
|
||||
}
|
||||
// log.md is the reason this is an exact-name list and not "root *.md":
|
||||
// it is gitignored, so an edit would apply locally and never reach gitea.
|
||||
for _, p := range []string{"log.md", "AGENTS.md", "knowledge/note.md", ".git/config"} {
|
||||
if _, err := Resolve(dir, p, ScopeRoot); err == nil {
|
||||
t.Errorf("Resolve(%q, ScopeRoot) = nil error, want refusal", p)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolve_RefusesSymlinkedTargets(t *testing.T) {
|
||||
dir := newBrain(t)
|
||||
outside := filepath.Join(t.TempDir(), "secret.md")
|
||||
if err := os.WriteFile(outside, []byte("secret\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// The tree is Syncthing-managed, so a symlink can appear without anyone
|
||||
// adding one deliberately. Containment must survive it.
|
||||
if err := os.Symlink(outside, filepath.Join(dir, "knowledge/link.md")); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
if _, err := Resolve(dir, "knowledge/link.md", ScopeNotes); err == nil {
|
||||
t.Error("Resolve followed a symlink out of the brain dir")
|
||||
}
|
||||
|
||||
if err := os.Symlink(filepath.Dir(outside), filepath.Join(dir, "wiki/escape")); err != nil {
|
||||
t.Skipf("symlinks unavailable: %v", err)
|
||||
}
|
||||
if _, err := Resolve(dir, "wiki/escape/secret.md", ScopeNotes); err == nil {
|
||||
t.Error("Resolve followed a symlinked directory out of the brain dir")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolve_RefusesWhenTheTreeIsNotTheSyncSource(t *testing.T) {
|
||||
// brain#2 was "wrote to a directory the sync does not commit from". No
|
||||
// code review prevents that recurring — a wrong INGEST_BRAIN_DIR in a
|
||||
// future manifest reintroduces it and every unit test still passes. So
|
||||
// assert the property at runtime, the way brain-sync itself does.
|
||||
dir := t.TempDir()
|
||||
if err := os.MkdirAll(filepath.Join(dir, "knowledge"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(dir, "knowledge/note.md"), []byte("x"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Reads are fine anywhere; the guard belongs to writes.
|
||||
if _, err := Resolve(dir, "knowledge/note.md", ScopeNotes); err != nil {
|
||||
t.Fatalf("Resolve should not require .git for a read: %v", err)
|
||||
}
|
||||
_, err := Edit(dir, "knowledge/note.md", "x", "y", ScopeNotes)
|
||||
if err == nil {
|
||||
t.Fatal("Edit accepted a tree with no .git — the write would never be committed")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "sync source") {
|
||||
t.Errorf("error should name the cause, got %v", err)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user