From 36b757fabf006b5fb512441ae04a403fb1026ffe Mon Sep 17 00:00:00 2001 From: Mathias Date: Tue, 18 Aug 2026 17:01:37 +0200 Subject: [PATCH] feat(mcp): add brain_read + brain_edit, contained to the sync source (brain#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01BeAp5LTscnz2W7Ubt4eJ6m --- ingestion/internal/mcp/brain_update_test.go | 2 +- ingestion/internal/mcp/handlers.go | 12 + ingestion/internal/mcp/server.go | 9 + ingestion/internal/mcp/server_test.go | 8 + ingestion/internal/mcp/tools_capture.go | 8 +- ingestion/internal/mcp/tools_notes.go | 145 ++++++++++ ingestion/internal/mcp/tools_notes_test.go | 127 +++++++++ ingestion/internal/notes/edit.go | 276 ++++++++++++++++++++ ingestion/internal/notes/edit_test.go | 256 ++++++++++++++++++ ingestion/internal/notes/path.go | 201 ++++++++++++++ ingestion/internal/notes/path_test.go | 146 +++++++++++ 11 files changed, 1185 insertions(+), 5 deletions(-) create mode 100644 ingestion/internal/mcp/tools_notes.go create mode 100644 ingestion/internal/mcp/tools_notes_test.go create mode 100644 ingestion/internal/notes/edit.go create mode 100644 ingestion/internal/notes/edit_test.go create mode 100644 ingestion/internal/notes/path.go create mode 100644 ingestion/internal/notes/path_test.go diff --git a/ingestion/internal/mcp/brain_update_test.go b/ingestion/internal/mcp/brain_update_test.go index 742d4cb..3777983 100644 --- a/ingestion/internal/mcp/brain_update_test.go +++ b/ingestion/internal/mcp/brain_update_test.go @@ -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", })) diff --git a/ingestion/internal/mcp/handlers.go b/ingestion/internal/mcp/handlers.go index a72b879..deb6f8b 100644 --- a/ingestion/internal/mcp/handlers.go +++ b/ingestion/internal/mcp/handlers.go @@ -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 diff --git a/ingestion/internal/mcp/server.go b/ingestion/internal/mcp/server.go index 37fb990..f5c9c37 100644 --- a/ingestion/internal/mcp/server.go +++ b/ingestion/internal/mcp/server.go @@ -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": diff --git a/ingestion/internal/mcp/server_test.go b/ingestion/internal/mcp/server_test.go index 16f493f..6637197 100644 --- a/ingestion/internal/mcp/server_test.go +++ b/ingestion/internal/mcp/server_test.go @@ -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) { diff --git a/ingestion/internal/mcp/tools_capture.go b/ingestion/internal/mcp/tools_capture.go index f92db2f..6f952fe 100644 --- a/ingestion/internal/mcp/tools_capture.go +++ b/ingestion/internal/mcp/tools_capture.go @@ -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) diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go new file mode 100644 index 0000000..008a5ec --- /dev/null +++ b/ingestion/internal/mcp/tools_notes.go @@ -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 +} diff --git a/ingestion/internal/mcp/tools_notes_test.go b/ingestion/internal/mcp/tools_notes_test.go new file mode 100644 index 0000000..af86ce7 --- /dev/null +++ b/ingestion/internal/mcp/tools_notes_test.go @@ -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 +} diff --git a/ingestion/internal/notes/edit.go b/ingestion/internal/notes/edit.go new file mode 100644 index 0000000..8c91271 --- /dev/null +++ b/ingestion/internal/notes/edit.go @@ -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 +} diff --git a/ingestion/internal/notes/edit_test.go b/ingestion/internal/notes/edit_test.go new file mode 100644 index 0000000..fb79c9b --- /dev/null +++ b/ingestion/internal/notes/edit_test.go @@ -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") + } +} diff --git a/ingestion/internal/notes/path.go b/ingestion/internal/notes/path.go new file mode 100644 index 0000000..e54a026 --- /dev/null +++ b/ingestion/internal/notes/path.go @@ -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 +} diff --git a/ingestion/internal/notes/path_test.go b/ingestion/internal/notes/path_test.go new file mode 100644 index 0000000..bb6b98c --- /dev/null +++ b/ingestion/internal/notes/path_test.go @@ -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) + } +}