Compare commits
1
Commits
main
...
47354dc4fc
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
47354dc4fc |
@@ -136,6 +136,49 @@ func (b *Brain) ReadDirDirs(relDir string) ([]string, error) {
|
||||
return names, nil
|
||||
}
|
||||
|
||||
// OpenLock opens (creating if absent) a regular lock file at a clean
|
||||
// brain-relative path for flock, through the root (hyperguild#192): every
|
||||
// directory above it must be real, an existing lock must be a regular file,
|
||||
// and the opened handle must be the file at the path (os.Root follows an
|
||||
// in-root final symlink even with O_NOFOLLOW). A dangling or swapped-in
|
||||
// symlink is refused; nothing is created at its target.
|
||||
func (b *Brain) OpenLock(rel string) (*os.File, error) {
|
||||
if err := cleanRel(rel); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if err := b.realDir(path.Dir(rel)); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if fi, err := b.root.Lstat(rel); err == nil && !fi.Mode().IsRegular() {
|
||||
return nil, fmt.Errorf("%s: %w", rel, ErrNotRegular)
|
||||
} else if err != nil && !errors.Is(err, fs.ErrNotExist) {
|
||||
return nil, err
|
||||
}
|
||||
f, err := b.root.OpenFile(rel, os.O_RDWR|os.O_CREATE|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
hfi, herr := f.Stat()
|
||||
lfi, lerr := b.root.Lstat(rel)
|
||||
if herr != nil || lerr != nil || !lfi.Mode().IsRegular() || !os.SameFile(lfi, hfi) {
|
||||
_ = f.Close()
|
||||
return nil, fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular)
|
||||
}
|
||||
return f, nil
|
||||
}
|
||||
|
||||
// RemoveLock removes the lock file at rel through the root (a symlink there
|
||||
// is removed itself, never its target).
|
||||
func (b *Brain) RemoveLock(rel string) error {
|
||||
if err := cleanRel(rel); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := b.realDir(path.Dir(rel)); err != nil {
|
||||
return err
|
||||
}
|
||||
return b.root.Remove(rel)
|
||||
}
|
||||
|
||||
// ReadRegular reads a brain-relative path, refusing anything that is not a
|
||||
// regular file, both before the open and on the opened handle.
|
||||
func (b *Brain) ReadRegular(rel string) ([]byte, error) {
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
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/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") },
|
||||
"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")))
|
||||
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"})
|
||||
_, err := os.Lstat(tgt)
|
||||
assert.True(t, os.IsNotExist(err), "a file was created at the lock symlink's target")
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -6,8 +6,8 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
|
||||
"log/slog"
|
||||
"os"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -118,7 +118,14 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM
|
||||
return nil, err
|
||||
}
|
||||
// No snapshot, no edit: without one a failed stamp could not be undone.
|
||||
prior, err := os.ReadFile(target.Abs)
|
||||
// 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)
|
||||
}
|
||||
@@ -131,7 +138,7 @@ func (s *Server) brainEdit(ctx context.Context, args json.RawMessage) (json.RawM
|
||||
// to the editor after every edit (PR #147 review, escape E1).
|
||||
origin, writtenBy := provenanceFrom(ctx)
|
||||
if serr := api.StampProvenanceFile(s.brainDir, res.Path, origin, writtenBy); serr != nil {
|
||||
if werr := os.WriteFile(target.Abs, prior, 0o644); werr != 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)
|
||||
|
||||
@@ -127,7 +127,7 @@ func Edit(brainDir, path, oldStr, newStr string, scope Scope) (Result, error) {
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
|
||||
unlock, err := flockFile(t.Abs)
|
||||
unlock, err := flockFile(t)
|
||||
if err != nil {
|
||||
return Result{}, err
|
||||
}
|
||||
@@ -220,20 +220,32 @@ func lockFor(abs string) *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)
|
||||
// hostPath — during a RollingUpdate two ingestion pods do. The lock file is
|
||||
// opened and removed through the brain root, never through a symlink
|
||||
// (hyperguild#192): a planted dangling "<note>.lock" link made a
|
||||
// successful edit create a file at its target.
|
||||
func flockFile(t Target) (func(), error) {
|
||||
lockRel := t.Rel + ".lock"
|
||||
bfs, err := brainfs.Open(t.Brain)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("notes: lock %s: %w", filepath.Base(abs), err)
|
||||
return nil, fmt.Errorf("notes: lock %s: %w", lockRel, err)
|
||||
}
|
||||
f, err := bfs.OpenLock(lockRel)
|
||||
_ = bfs.Close()
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("notes: lock %s: %w", lockRel, 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 nil, fmt.Errorf("notes: flock %s: %w", lockRel, err)
|
||||
}
|
||||
return func() {
|
||||
_ = syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
|
||||
_ = f.Close()
|
||||
_ = os.Remove(abs + ".lock")
|
||||
if b, err := brainfs.Open(t.Brain); err == nil {
|
||||
_ = b.RemoveLock(lockRel)
|
||||
_ = b.Close()
|
||||
}
|
||||
}, nil
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user