diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index 0f5570b..21d8ee1 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -529,3 +529,49 @@ func TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories(t *testing.T) { got, _ = os.ReadFile(filepath.Join(dir, "wiki", "new", "facts", "n.md")) assert.Equal(t, "one", string(got)) } + +// hyperguild#192: OpenLock creates or reuses a regular lock file through the +// root and refuses every other shape without creating anything at a link's +// target or hanging. +func TestOpenLockOpensOnlyARegularLockThroughRealDirectories(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki", "w"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("untouched"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "created"), filepath.Join(dir, "wiki", "w", "dangling.lock"))) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "v.md"), filepath.Join(dir, "wiki", "w", "live.lock"))) + require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "wiki", "w", "fifo.lock"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki", "w", "dir.lock"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(dir, "wiki", "alias"))) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + for _, rel := range []string{"wiki/w/dangling.lock", "wiki/w/live.lock", "wiki/w/fifo.lock", "wiki/w/dir.lock", "wiki/alias/x.lock", "wiki/w/../w/x.lock"} { + done := make(chan error, 1) + go func() { + f, err := b.OpenLock(rel) + if f != nil { + _ = f.Close() + } + done <- err + }() + select { + case err := <-done: + assert.ErrorIs(t, err, ErrNotRegular, rel) + case <-time.After(3 * time.Second): + t.Fatalf("OpenLock hung on %s", rel) + } + } + _, err = os.Lstat(filepath.Join(dir, "raw", "created")) + assert.True(t, os.IsNotExist(err), "nothing created at the dangling link's target") + got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md")) + assert.Equal(t, "untouched", string(got)) + + f, err := b.OpenLock("wiki/w/n.md.lock") + require.NoError(t, err, "positive control: creates a lock") + assert.True(t, b.LockIsCurrent("wiki/w/n.md.lock", f)) + require.NoError(t, b.Remove("wiki/w/n.md.lock")) + assert.False(t, b.LockIsCurrent("wiki/w/n.md.lock", f), "a removed lock is no longer current") + _ = f.Close() +} diff --git a/ingestion/internal/mcp/edit_pathio_test.go b/ingestion/internal/mcp/edit_pathio_test.go index 21ee56f..1fb2f45 100644 --- a/ingestion/internal/mcp/edit_pathio_test.go +++ b/ingestion/internal/mcp/edit_pathio_test.go @@ -10,6 +10,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mathiasbq/hyperguild/ingestion/internal/audit" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainstore" + "github.com/mathiasbq/hyperguild/ingestion/internal/capture" + "github.com/mathiasbq/hyperguild/ingestion/internal/capturehttp" + "github.com/mathiasbq/hyperguild/ingestion/internal/classification" "github.com/mathiasbq/hyperguild/ingestion/internal/mcp" ) @@ -45,17 +50,69 @@ func TestBrainEditReturnsPromptlyOnAFIFONoteWithAWriter(t *testing.T) { func TestBrainEditDoesNotCreateAFileThroughADanglingLockSymlink(t *testing.T) { for name, target := range map[string]func(t *testing.T, dir string) string{ "outside": func(t *testing.T, _ string) string { return filepath.Join(t.TempDir(), "created") }, + // Relative, as Syncthing carries it: os.Root refuses an absolute + // link on its own, so only OpenLock's pre-check stops this one. "in-brain": func(t *testing.T, dir string) string { return filepath.Join(dir, "raw", "created") }, } { t.Run(name, func(t *testing.T) { dir := newBrainRepo(t) require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) tgt := target(t, dir) - require.NoError(t, os.Symlink(tgt, filepath.Join(dir, "wiki", "homelab", "facts", "n.md.lock"))) + lock := filepath.Join(dir, "wiki", "homelab", "facts", "n.md.lock") + linkTo := tgt + if name == "in-brain" { + rel, err := filepath.Rel(filepath.Dir(lock), tgt) + require.NoError(t, err) + linkTo = rel + } + require.NoError(t, os.Symlink(linkTo, lock)) srv := mcp.NewServer(dir, nil, nil, nil) - _ = call(t, srv, "brain_edit", map[string]any{"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA"}) + resp := call(t, srv, "brain_edit", map[string]any{"path": "wiki/homelab/facts/n.md", "old_str": "beta", "new_str": "BETA"}) + require.NotNil(t, resp["error"], "the edit is refused") + assert.Contains(t, resp["error"].(map[string]any)["message"], "lock", "refused because of the lock file") _, err := os.Lstat(tgt) assert.True(t, os.IsNotExist(err), "a file was created at the lock symlink's target") }) } } + +// The rollback (when the stamp cannot be verified) writes the prior bytes +// through the brain root: a symlink swapped in for the note before the +// rollback is refused, and its target, in the brain or outside, is +// untouched. +func TestBrainEditRollbackDoesNotWriteThroughASymlinkSwappedInForTheNote(t *testing.T) { + for name, mk := range map[string]func(t *testing.T, dir string) (link, victim string){ + "outside": func(t *testing.T, _ string) (string, string) { + v := filepath.Join(t.TempDir(), "victim.md") + return v, v + }, + "in-brain": func(t *testing.T, dir string) (string, string) { + return filepath.Join("..", "..", "..", "raw", "victim.md"), filepath.Join(dir, "raw", "victim.md") + }, + } { + t.Run(name, func(t *testing.T) { + dir := newBrainRepo(t) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + cfg, err := classification.Load(dir) + require.NoError(t, err) + svc := capture.NewService(brainstore.New(dir), capFakeTracker{}, cfg, audit.NewSlogSink(nil)) + srv := mcp.NewServer(dir, nil, nil, nil) + srv.WithCapture(svc, capFakeValidator{subject: "bad\xff"}, capStaticTok, "local-cli", capturehttp.NewOriginResolver(nil)) + out := resultText(t, authCall(t, srv, "Bearer "+capStaticTok, "brain_write", map[string]any{ + "content": "# R\n\nstatic body", "wing": "testing", "hall": "facts", "filename": "rb", + })) + path := out["path"].(string) + note := filepath.Join(dir, path) + link, victim := mk(t, dir) + require.NoError(t, os.WriteFile(victim, []byte("untouched"), 0o644)) + defer mcp.SetBeforeStampHook(func() { + require.NoError(t, os.Remove(note)) + require.NoError(t, os.Symlink(link, note)) + })() + resp := authCall(t, srv, jwtAuthz, "brain_edit", map[string]any{"path": path, "old_str": "static body", "new_str": "injected"}) + require.NotNil(t, resp["error"]) + got, _ := os.ReadFile(victim) + assert.Equal(t, "untouched", string(got), "the rollback wrote through the swapped-in symlink") + }) + } +} diff --git a/ingestion/internal/mcp/export_test.go b/ingestion/internal/mcp/export_test.go new file mode 100644 index 0000000..5afc51e --- /dev/null +++ b/ingestion/internal/mcp/export_test.go @@ -0,0 +1,9 @@ +package mcp + +// SetBeforeStampHook installs beforeStampHook for a test and returns a +// restore func. +func SetBeforeStampHook(f func()) func() { + old := beforeStampHook + beforeStampHook = f + return func() { beforeStampHook = old } +} diff --git a/ingestion/internal/mcp/tools_notes.go b/ingestion/internal/mcp/tools_notes.go index e906308..0707045 100644 --- a/ingestion/internal/mcp/tools_notes.go +++ b/ingestion/internal/mcp/tools_notes.go @@ -137,6 +137,9 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM // 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. @@ -148,6 +151,11 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM 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 diff --git a/ingestion/internal/notes/lock_xproc_test.go b/ingestion/internal/notes/lock_xproc_test.go new file mode 100644 index 0000000..ce3f2ae --- /dev/null +++ b/ingestion/internal/notes/lock_xproc_test.go @@ -0,0 +1,82 @@ +package notes + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// lockChild runs inside a re-executed test binary: it takes the note's edit +// lock N times and, while holding it, creates an O_EXCL marker. Finding the +// marker already there means another process holds the "same" lock: mutual +// exclusion failed. It prints the overlap count. +func lockChild(brainDir string, rounds int) { + t, err := Resolve(brainDir, "wiki/w/facts/n.md", ScopeNotes) + if err != nil { + fmt.Println("resolve:", err) + os.Exit(2) + } + marker := filepath.Join(brainDir, "marker") + overlaps := 0 + for i := 0; i < rounds; i++ { + unlock, err := flockFile(t) + if err != nil { + fmt.Println("lock:", err) + os.Exit(2) + } + f, err := os.OpenFile(marker, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) + if errors.Is(err, os.ErrExist) { + overlaps++ + } else if err == nil { + _ = f.Close() + _ = os.Remove(marker) + } + unlock() + } + fmt.Println(overlaps) + os.Exit(0) +} + +func TestMain(m *testing.M) { + if dir := os.Getenv("NOTES_LOCK_CHILD"); dir != "" { + n, _ := strconv.Atoi(os.Getenv("NOTES_LOCK_ROUNDS")) + lockChild(dir, n) + } + os.Exit(m.Run()) +} + +// hyperguild#192: the edit lock must exclude other processes (two ingestion +// pods share the hostPath brain during a RollingUpdate). Unlock removes the +// lock file, so without re-checking that the locked file is still the one at +// the path, a waiter and the next caller each held a lock (measured on main: +// hundreds of overlaps in 2000 rounds). +func TestEditLockExcludesOtherProcesses(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(brain, "wiki", "w", "facts", "n.md"), []byte("x"), 0o644)) + const procs, rounds = 3, 400 + cmds := make([]*exec.Cmd, procs) + outs := make([]*strings.Builder, procs) + for i := range cmds { + outs[i] = &strings.Builder{} + cmds[i] = exec.Command(os.Args[0], "-test.run=^$") + cmds[i].Env = append(os.Environ(), "NOTES_LOCK_CHILD="+brain, "NOTES_LOCK_ROUNDS="+strconv.Itoa(rounds)) + cmds[i].Stdout, cmds[i].Stderr = outs[i], outs[i] + require.NoError(t, cmds[i].Start()) + } + total := 0 + for i, c := range cmds { + require.NoError(t, c.Wait(), "child %d: %s", i, outs[i].String()) + n, err := strconv.Atoi(strings.TrimSpace(outs[i].String())) + require.NoError(t, err, "child %d output: %q", i, outs[i].String()) + total += n + } + require.Zero(t, total, "processes held the edit lock at the same time") +}