Review round 2 on PR #194: with the hook made a no-op the test still passed (no swap, the rollback wrote the real note, victim trivially untouched). It now asserts the error says the edit could NOT be rolled back and that the note path is still the swapped-in symlink; red when the hook does not fire. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
125 lines
5.5 KiB
Go
125 lines
5.5 KiB
Go
package mcp_test
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"syscall"
|
|
"testing"
|
|
"time"
|
|
|
|
"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"
|
|
)
|
|
|
|
// hyperguild#192: brain_edit snapshotted the note with os.ReadFile by path,
|
|
// which blocks on a FIFO held open by a writer, and hung the call.
|
|
func TestBrainEditReturnsPromptlyOnAFIFONoteWithAWriter(t *testing.T) {
|
|
dir := newBrainRepo(t)
|
|
fifo := filepath.Join(dir, "wiki", "homelab", "facts", "f.md")
|
|
require.NoError(t, syscall.Mkfifo(fifo, 0o644))
|
|
w, err := os.OpenFile(fifo, os.O_RDWR, 0)
|
|
require.NoError(t, err)
|
|
t.Cleanup(func() { _ = w.Close() })
|
|
srv := mcp.NewServer(dir, nil, nil, nil)
|
|
|
|
done := make(chan map[string]any, 1)
|
|
go func() {
|
|
done <- call(t, srv, "brain_edit", map[string]any{"path": "wiki/homelab/facts/f.md", "old_str": "a", "new_str": "b"})
|
|
}()
|
|
select {
|
|
case resp := <-done:
|
|
assert.Contains(t, resp, "error", "a FIFO note is refused")
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("brain_edit hung on a FIFO note")
|
|
}
|
|
|
|
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", "positive control: a regular note is edited: %v", resp["error"])
|
|
}
|
|
|
|
// hyperguild#192: the edit lock "<note>.lock" was opened by path with
|
|
// O_CREATE, so a planted dangling .lock symlink made a successful edit create
|
|
// a file at the link's target, outside the brain or elsewhere in it.
|
|
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)
|
|
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)
|
|
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"])
|
|
// The swap happened and the rollback refused it: the error says so,
|
|
// and the note path is still the planted symlink.
|
|
assert.Contains(t, resp["error"].(map[string]any)["message"], "could NOT be rolled back")
|
|
fi, err := os.Lstat(note)
|
|
require.NoError(t, err)
|
|
assert.NotZero(t, fi.Mode()&os.ModeSymlink, "the note path is still the swapped-in symlink")
|
|
got, _ := os.ReadFile(victim)
|
|
assert.Equal(t, "untouched", string(got), "the rollback wrote through the swapped-in symlink")
|
|
})
|
|
}
|
|
}
|