Files
hyperguild/ingestion/internal/mcp/tools_notes.go
mathiasandClaude Opus 5.5 07ea5b01ec
CI / Lint / Test / Vet (pull_request) Successful in 1m9s
CI / Mirror to GitHub (pull_request) Skipped
fix(notes): the edit lock excludes other processes; pin OpenLock and the rollback (#192)
Review round 1 on PR #194:
- The edit lock did not exclude other processes (also on main): unlock
  removes the lock file, so a waiter could wake holding a flock on the
  orphaned inode while the next caller locked a fresh file. flockFile now
  re-checks after flock that the locked file is still the one at the path
  (brainfs LockIsCurrent) and retries otherwise; OpenLock reports a lock
  replaced mid-open as ErrLockChanged, which is retried instead of
  failing the edit. New cross-process test (3 re-executed processes x 400
  rounds, O_EXCL marker under the lock): 0 overlaps; red without the
  re-check.
- OpenLock unit test: relative dangling link (only the Lstat pre-check
  stops it; os.Root refuses absolute links on its own), live link, FIFO,
  directory, behind a symlinked dir, unclean path, positive control.
- Rollback test: a symlink swapped in for the note before the rollback
  (test seam beforeStampHook) is refused, target untouched in and outside
  the brain.
- The in-brain dangling-lock case uses a relative link and asserts the
  refusal names the lock. OpenLock reuses #189's RealParent; RemoveLock is
  dropped for brainfs.Remove. The doc comment names the swap residual.

Rebased onto main after #189.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 23:36:56 +02:00

448 lines
20 KiB
Go

package mcp
import (
"context"
"encoding/json"
"errors"
"fmt"
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"log/slog"
"strings"
"time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
"github.com/mathiasbq/hyperguild/ingestion/internal/capture"
"github.com/mathiasbq/hyperguild/ingestion/internal/claim"
"github.com/mathiasbq/hyperguild/ingestion/internal/classification"
"github.com/mathiasbq/hyperguild/ingestion/internal/frontmatter"
"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"),
}),
},
}
}
// --- hyperguild#97: governance-rewrite observability -----------------------
// govNotifier raises an out-of-band alert. Declared here rather than importing
// internal/audit so the MCP package keeps depending on capture alone.
type govNotifier interface {
Notify(ctx context.Context, msg string) error
}
// WithGovernanceObserver attaches the audit sink and notifier that observe a
// successful brain_edit_root. Both are optional; either may be nil.
func (s *Server) WithGovernanceObserver(sink capture.AuditSink, n govNotifier) {
s.govAudit, s.govNotify = sink, n
}
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.
//
// Deliberately NOT audited (#97). A note edit is already fully reconstructible
// from git history, which carries the same before/after bytes plus a diff.
// Governance files get the extra trail because they change what later agents
// obey — the asymmetry is about instruction-carrying content, not about how
// hard the change is to reconstruct. Auditing every note edit would also bury
// the rare governance record under routine traffic, which is how an alert
// stops being read.
func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawMessage, error) {
// Snapshot before editing: if the edited note cannot be stamped and
// verified, the edit is rolled back rather than left on disk unattributed
// (or carrying whatever provenance the edit wrote).
var a brainEditArgs
if err := json.Unmarshal(args, &a); err != nil {
return nil, fmt.Errorf("parse args: %w", err)
}
target, err := notes.Resolve(s.brainDir, a.Path, notes.ScopeNotes)
if err != nil {
return nil, err
}
// No snapshot, no edit: without one a failed stamp could not be undone.
// Read through the brain root, regular files only (hyperguild#192): a
// FIFO held open by a writer hung os.ReadFile.
bfs, err := brainfs.Open(s.brainDir)
if err != nil {
return nil, fmt.Errorf("snapshot before edit: %w", err)
}
defer func() { _ = bfs.Close() }()
prior, err := bfs.ReadRegular(target.Rel)
if err != nil {
return nil, fmt.Errorf("snapshot before edit: %w", err)
}
res, out, err := s.editWithScope(args, notes.ScopeNotes)
if err != nil {
return out, err
}
// A byte-level replace can rewrite any line, including provenance, and
// whoever edits a note is the author of what a reader now sees: re-stamp
// to the editor after every edit (PR #147 review, escape E1).
origin, writtenBy := provenanceFrom(ctx)
if beforeStampHook != nil {
beforeStampHook()
}
if serr := api.StampProvenanceFile(s.brainDir, res.Path, origin, writtenBy); serr != nil {
if werr := bfs.WriteRegular(target.Rel, prior); werr != nil {
// Say exactly what state the note is in: edited, unverified.
return nil, fmt.Errorf("stamp provenance failed (%v) AND the edit could NOT be rolled back: %w; "+
"%s carries the edit under its previous attribution", serr, werr, res.Path)
}
return nil, fmt.Errorf("edit rolled back, stamp provenance: %w", serr)
}
return out, nil
}
// beforeStampHook is a test seam (nil in production): it runs between the
// edit and the provenance stamp, so a test can swap the note before the
// rollback path runs.
var beforeStampHook func()
// 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")
}
res, out, err := s.editWithScope(args, notes.ScopeRoot)
if err != nil {
// A refusal is not a rewrite. Recording or alerting on one would cry
// wolf, and the gate already logs its own denials.
return out, err
}
// A durable instruction rewrite should be loud, not silent.
slog.Warn("brain_edit_root applied", "principal", info.principal, "result", string(out))
s.observeGovernanceEdit(ctx, info.principal, res)
return out, err
}
// observeGovernanceEdit records and alerts on a rewrite that already landed.
//
// Best-effort by construction: the bytes are written and brain-sync will commit
// them within the quarter hour, so failing the call here would report a
// non-rewrite to the caller while the rewrite sits on disk. A failure to
// observe is therefore logged at ERROR — the one case where the trail itself is
// what broke, and the only remaining signal.
func (s *Server) observeGovernanceEdit(ctx context.Context, principal string, res notes.Result) {
// Path and hashes only. The notification leaves the cluster for a phone
// via a third-party topic; a hash identifies the rewrite, the bytes would
// leak whatever was written into an instruction file.
msg := fmt.Sprintf("brain_edit_root applied\npath=%s\nprincipal=%s\nbefore=%s\nafter=%s",
res.Path, principal, res.PrevContentHash, res.ContentHash)
if s.govNotify != nil {
if err := s.govNotify.Notify(ctx, msg); err != nil {
slog.Error("brain_edit_root notify failed", "path", res.Path, "err", err)
}
}
if s.govAudit == nil {
return
}
// Internal, not Confidential: a governance file is not client data, and
// Confidential would make DegradingSink REFUSE to record while loki is
// down rather than buffer — losing exactly the record that matters most.
outcome, err := s.govAudit.Reserve(ctx, classification.Internal)
if err != nil {
slog.Error("brain_edit_root audit reserve failed", "path", res.Path, "err", err)
return
}
entry := capture.AuditEntry{
Timestamp: time.Now().UTC(),
Principal: principal,
Actor: "brain_edit_root",
Harness: "mcp",
EffectiveClassification: classification.Internal.String(),
Items: []string{res.Path},
SecurityEvents: []string{fmt.Sprintf(
"governance file rewritten: %s before=%s after=%s",
res.Path, res.PrevContentHash, res.ContentHash)},
}
if err := s.govAudit.Record(ctx, entry, outcome); err != nil {
slog.Error("brain_edit_root audit record failed", "path", res.Path, "err", err)
}
}
func (s *Server) editWithScope(args json.RawMessage, scope notes.Scope) (notes.Result, json.RawMessage, error) {
var a brainEditArgs
if err := json.Unmarshal(args, &a); err != nil {
return notes.Result{}, nil, fmt.Errorf("parse args: %w", err)
}
// hyperguild#123: brain_write/brain_update both validate the claim
// envelope before writing (hyperguild#111: "same rules when a note
// changes as when it's born"); brain_edit didn't — a byte-level string
// replace has no frontmatter awareness at all, so it could rewrite an
// existing half_life:/applies:/claim_provenance: line straight past the
// validator entirely. Scoped to notes only (governance files have no
// claim-envelope concept).
if scope == notes.ScopeNotes {
if verr := checkClaimEnvelope(s.brainDir, a.Path, a.OldStr, a.NewStr); verr != nil {
return notes.Result{}, nil, fmt.Errorf("claim envelope: %w", verr)
}
}
res, err := notes.Edit(s.brainDir, a.Path, a.OldStr, a.NewStr, scope)
if err != nil {
if errors.Is(err, notes.ErrDegenerate) && a.OldStr == "" {
// hyperguild#115: "old_str is empty" alone doesn't say whether the
// caller sent it empty or a hop between the caller and here lost
// it. Naming the raw argument byte count makes that distinguishable
// without a second investigation.
return notes.Result{}, nil, fmt.Errorf("%w (raw arguments were %d bytes)", err, len(args))
}
return notes.Result{}, 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()
}
}
out, merr := json.Marshal(res)
return res, out, merr
}
// 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
}
// checkClaimEnvelope previews what old_str->new_str would produce — the
// exact strings.Replace(before, oldStr, newStr, 1) formula notes.Edit
// itself uses internally — and validates the resulting frontmatter's claim
// envelope before the real edit is allowed to run. Fail-closed: an invalid
// result blocks the edit entirely, the same posture writeHallNote/UpdateNote
// already apply on create/supersede.
//
// Gated on old_str appearing within the note's frontmatter BLOCK, not on
// old_str/new_str mentioning a claim-envelope field NAME — found in review:
// a field-name-substring gate misses a value-only edit like
// "asserted"->"guessed" on a claim_provenance line, which never mentions
// the field name at all. That is exactly the shape of edit a minimal,
// natural diff produces, so a field-name gate was closing the issue in
// name only. Checking block containment instead means any edit that
// couldn't possibly touch the frontmatter (its target text only exists in
// the body) is still skipped cheaply, while nothing inside the fence can
// slip through unvalidated regardless of which token it targets.
//
// This does not replicate notes.Edit's uniqueness/CRLF/conflict-marker
// checks — if old_str is absent or ambiguous, this preview may not match
// what Edit ultimately does, but Edit independently and correctly rejects
// those cases at write time regardless of what this function decides, so
// the only thing riding on this preview's accuracy is the envelope check
// itself, not write safety.
// checkClaimEnvelope previews what old_str->new_str would produce and
// validates the resulting claim envelope before the real edit runs
// (hyperguild#123). Seven review rounds closed every plausible bypass in the
// fence-detection family (field-name substring, block-containment,
// fence-stripping, CRLF, UTF-8 BOM) and the first instance of a
// value-extraction gap (a folded YAML block scalar hiding a value behind an
// empty inline colon).
//
// Known, accepted v1 gap: frontmatterScalars's block-scalar fold is a
// best-effort approximation, not a YAML parser — a blank line interrupting a
// folded continuation, or other contrived multi-line YAML shapes not yet
// probed (block-scalar indicators `|`/`>`, inline comments on a key line,
// quoted multi-line values), can still hide an invalid value from
// ValidateEnvelope. Accepted deliberately rather than chased indefinitely:
// every bypass found so far requires the caller to already hold brain_edit
// write access and to deliberately construct a specific malformed-YAML shape
// with knowledge of this exact gap — a narrow, low-likelihood threat next to
// the diminishing returns of another incremental patch round. The real fix
// (a shared, tested, encoding-normalized frontmatter parser used by all
// three independent implementations in this codebase) is tracked separately
// as hyperguild#138, not bolted on here piece by piece.
func checkClaimEnvelope(brainDir, path, oldStr, newStr string) error {
if oldStr == "" {
// hyperguild#115: strings.Replace treats "" as matching position 0,
// which would make an empty old_str look like an edit that inserts
// text before the fence — a degenerate-input case that belongs to
// notes.Edit's own "old_str is empty" refusal, not to the envelope
// check.
return nil
}
before, err := notes.Read(brainDir, path)
if err != nil {
// Let notes.Edit itself produce the real error (missing file, etc.)
// — this function's job is the envelope, not read failures.
return nil
}
// No gate on where old_str falls relative to the frontmatter fence:
// round 2 found that requiring old_str to be fully contained in the
// extracted block was itself bypassable by an old_str disambiguated
// across the closing "---" into the body. Simpler and bypass-proof:
// always compute what the edit would produce and validate that
// resulting state's envelope. A body-only edit leaves the frontmatter
// (and thus the validation outcome) unchanged, so this is a no-op for
// the common case and fail-closed for every case that touches the
// envelope, regardless of how old_str is phrased.
_, hadFence := frontmatterBlock(string(before))
after := strings.Replace(string(before), oldStr, newStr, 1)
_, hasFence := frontmatterBlock(after)
if hadFence && !hasFence {
// Round 3: an edit that drops the closing fence degrades `after`
// to "unfenced" — frontmatterScalars then sees an empty map and
// ValidateEnvelope({}) is nil by design (absent fields are valid),
// so an invalid value would otherwise land with zero error while
// also corrupting the note's structure. A previously-fenced note
// must stay fenced, full stop.
return fmt.Errorf("edit would remove the frontmatter fence")
}
return claim.ValidateEnvelope(frontmatterScalars(after))
}
// frontmatterBlock returns the raw text between doc's opening and closing
// `---` fences, or ok=false if doc has no (properly closed) fence.
func frontmatterBlock(doc string) (block string, ok bool) {
// hyperguild#138: fence detection itself is frontmatter.Split, shared
// with internal/api and internal/frontmatter — the CRLF/BOM guards that
// used to live here (added piecemeal across rounds 4 and 5 of #123's
// review) are now the shared parser's job, so every caller inherits a
// fix instead of needing bug-for-bug parity edits.
b, _, ok := frontmatter.Split([]byte(doc))
if !ok {
return "", false
}
// Split's block keeps the "\n" immediately before the closing fence
// (by design — see its own doc comment); this function never included
// it. Split guarantees ok=true only when "\n---\n" matched, so this is
// always exactly one byte, never ambiguous.
return strings.TrimSuffix(string(b), "\n"), true
}
// frontmatterScalars extracts top-level "key: value" scalar pairs from
// doc's leading fence block, for the claim-envelope check only — not a
// general frontmatter parser. Indented lines (YAML block-list items,
// wrapped values) are skipped rather than misparsed as keys, same fix as
// hyperguild#126/#128 in the sibling packages this mirrors.
func frontmatterScalars(doc string) map[string]string {
out := map[string]string{}
block, ok := frontmatterBlock(doc)
if !ok {
return out
}
lines := strings.Split(block, "\n")
for i := 0; i < len(lines); i++ {
line := lines[i]
if line != "" && (line[0] == ' ' || line[0] == '\t') {
continue
}
key, val, ok := strings.Cut(line, ":")
if !ok {
continue
}
val = strings.TrimSpace(val)
if val == "" {
// Round 6: a colon with nothing after it is ambiguous — it can
// be a genuinely empty/omitted value, or the start of a YAML
// block scalar whose real value lives on the indented
// continuation line(s) that follow (e.g. "half_life:\n
// durable"). Treating the latter as absent let an edit smuggle
// an invalid value straight past ValidateEnvelope: the field
// read as unset while the disk actually carried a value. Fold
// the continuation in as a best-effort join so oneOf() still
// sees it. A genuinely empty value (no indented line follows)
// still folds to "", preserving present()'s existing
// placeholder-not-assertion contract.
var cont []string
for i+1 < len(lines) {
next := lines[i+1]
if next == "" || (next[0] != ' ' && next[0] != '\t') {
break
}
cont = append(cont, strings.TrimSpace(next))
i++
}
val = strings.Join(cont, " ")
}
out[strings.TrimSpace(key)] = val
}
return out
}