Files
hyperguild/ingestion/internal/notes/edit.go
mathiasandClaude Opus 5.5 27a01dd2af
CI / Lint / Test / Vet (push) Successful in 46s
CI / Mirror to GitHub (push) Successful in 4s
fix(brain): context, brain_get and brain_read read through brainfs (#157, slice 2) (#165)
readExcerpt, ReadNote and ReadScoped now read through brainfs (os.Root, regular files only, type checked on the opened handle), so a symlink swapped in after a check is not followed and a FIFO does not hang the reader. ReadRegularMax bounds the read itself with io.LimitReader(max+1). notes.Target carries the brain root.

Refs #157. Adversarial review round 2: PASS (round 1 BLOCKED: unbounded read, rule-9 test names).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Mathias <mthbqv@gmail.com>
2026-10-04 05:59:55 +00:00

288 lines
9.4 KiB
Go

package notes
import (
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"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"`
// PrevContentHash is the sha256 of the file as it was before this edit.
// Paired with ContentHash it identifies a rewrite in an audit trail or an
// alert without carrying any of the bytes (hyperguild#97).
PrevContentHash string `json:"prev_content_hash"`
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) {
// Through the brain root, regular files only, size checked on the opened
// handle (hyperguild#157): Resolve's Lstat could be raced by a swap, and
// os.ReadFile blocked on a FIFO.
bfs, err := brainfs.Open(t.Brain)
if err != nil {
return nil, fmt.Errorf("notes: %w", err)
}
defer func() { _ = bfs.Close() }()
b, err := bfs.ReadRegularMax(t.Rel, maxNoteBytes)
if errors.Is(err, brainfs.ErrTooLarge) {
return nil, fmt.Errorf("%w: %s (limit %d)", ErrTooLarge, t.Rel, maxNoteBytes)
}
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))
prev := sha256.Sum256(before)
return Result{
Path: t.Rel,
PrevContentHash: hex.EncodeToString(prev[:]),
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
}