From 41b7105b313a2984de3ef578b1213ed998795f2a Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:35:19 +0200 Subject: [PATCH] fix(brainfs,api): PutRegular/AppendRegular; brain_write creates through the brain root (#188) brain_write created a new note with os.MkdirAll + os.WriteFile by path, so a symlinked wing or hall directory redirected the note elsewhere in the brain or outside it. New brainfs primitives for creating writes: - PutRegular: create or replace. Missing parents are created one component at a time through the root and every existing component must be a real directory; an existing file is replaced only via WriteRegular's checks; a new one is created with O_EXCL and confirmed (Lstat + SameFile) before writing. - AppendRegular: create-or-append with the same parent rule, writing only after confirming the path is the regular file it opened. - RealParent: the clean-path + real-directory check, for writers that do their own atomic rename. writeHallNote now uses PutRegular. Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/api/handler.go | 19 +++- ingestion/internal/api/write_redirect_test.go | 43 +++++++ ingestion/internal/brainfs/brainfs.go | 105 ++++++++++++++++++ ingestion/internal/brainfs/brainfs_test.go | 77 +++++++++++++ 4 files changed, 239 insertions(+), 5 deletions(-) create mode 100644 ingestion/internal/api/write_redirect_test.go diff --git a/ingestion/internal/api/handler.go b/ingestion/internal/api/handler.go index 3921e89..313fba4 100644 --- a/ingestion/internal/api/handler.go +++ b/ingestion/internal/api/handler.go @@ -12,6 +12,7 @@ import ( "time" "github.com/mathiasbq/hyperguild/ingestion/internal/brain" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "github.com/mathiasbq/hyperguild/ingestion/internal/claim" "github.com/mathiasbq/hyperguild/ingestion/internal/pipeline" "github.com/mathiasbq/hyperguild/ingestion/internal/search" @@ -227,14 +228,22 @@ func writeHallNote(brainDir string, opts WriteNoteOptions) (string, error) { // The directory is created only once the note is known to be writable: // a refused write leaves nothing behind, not even an empty hall dir // (brain#21; provenance refusal joins the envelope refusal here). - if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { - return "", fmt.Errorf("create hall dir: %w", err) + // Created through the brain root, never through a symlinked wing or + // hall directory (hyperguild#188). + rel, err := filepath.Rel(brainDir, dest) + if err != nil { + return "", fmt.Errorf("note path: %w", err) } - if err := os.WriteFile(dest, []byte(final), 0o644); err != nil { + rel = filepath.ToSlash(rel) + bfs, err := brainfs.Open(brainDir) + if err != nil { + return "", fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + if err := bfs.PutRegular(rel, []byte(final)); err != nil { return "", fmt.Errorf("write: %w", err) } - rel, _ := filepath.Rel(brainDir, dest) - return filepath.ToSlash(rel), nil + return rel, nil } // scalarValues flattens frontmatter fields to key -> scalar value for diff --git a/ingestion/internal/api/write_redirect_test.go b/ingestion/internal/api/write_redirect_test.go new file mode 100644 index 0000000..1a1ddbc --- /dev/null +++ b/ingestion/internal/api/write_redirect_test.go @@ -0,0 +1,43 @@ +package api_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/api" +) + +// hyperguild#188: brain_write created a new note with os.MkdirAll + +// os.WriteFile by path, so a symlinked wing directory redirected the note +// elsewhere in the brain or outside it. +func TestWriteNoteRefusesASymlinkedWing(t *testing.T) { + for name, target := range map[string]func(t *testing.T, brain string) string{ + "in-brain": func(t *testing.T, brain string) string { + require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) + return filepath.Join("..", "raw") + }, + "outside": func(t *testing.T, _ string) string { return t.TempDir() }, + } { + t.Run(name, func(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755)) + link := target(t, brain) + require.NoError(t, os.Symlink(link, filepath.Join(brain, "wiki", "lw"))) + _, err := api.WriteNote(brain, api.WriteNoteOptions{Wing: "lw", Hall: "facts", Filename: "n", Content: "learning"}) + assert.Error(t, err) + resolved := link + if !filepath.IsAbs(resolved) { + resolved = filepath.Join(brain, "wiki", link) + } + _, serr := os.Stat(filepath.Join(resolved, "facts")) + assert.True(t, os.IsNotExist(serr), "a hall was created at the link's target") + + _, err = api.WriteNote(brain, api.WriteNoteOptions{Wing: "w", Hall: "facts", Filename: "n", Content: "learning"}) + require.NoError(t, err, "positive control: a real wing is written") + }) + } +} diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index 97bf8aa..eec5f8e 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -136,6 +136,111 @@ func (b *Brain) ReadDirDirs(relDir string) ([]string, error) { return names, nil } +// PutRegular creates or replaces a regular file at a brain-relative path, +// for creating writes (hyperguild#188, #158). Missing parent directories are +// created one component at a time through the root, and every existing +// component must be a real directory, so a symlinked directory cannot +// redirect the write elsewhere in the brain or outside it. An existing file +// is replaced only if it is regular (WriteRegular's checks); a new file is +// created with O_EXCL and confirmed to be the file that was opened. A +// directory swapped in after the checks is the #167 residual. +func (b *Brain) PutRegular(rel string, data []byte) error { + if err := cleanRel(rel); err != nil { + return err + } + if err := b.mkdirReal(path.Dir(rel)); err != nil { + return err + } + if _, err := b.root.Lstat(rel); err == nil { + return b.WriteRegular(rel, data) + } else if !errors.Is(err, fs.ErrNotExist) { + return err + } + f, err := b.root.OpenFile(rel, os.O_WRONLY|os.O_CREATE|os.O_EXCL|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644) + if err != nil { + return err + } + defer func() { _ = f.Close() }() + hfi, herr := f.Stat() + lfi, lerr := b.root.Lstat(rel) + if herr != nil || lerr != nil || !lfi.Mode().IsRegular() || !os.SameFile(lfi, hfi) { + return fmt.Errorf("%s: changed while creating: %w", rel, ErrNotRegular) + } + if _, err := f.Write(data); err != nil { + return err + } + return f.Close() +} + +// AppendRegular appends data to a regular file at a brain-relative path, +// creating it (and missing parent directories, as PutRegular does) if it is +// absent. It writes only after confirming the path is the regular file it +// opened, so a final symlink or special file, or one swapped in, is refused. +func (b *Brain) AppendRegular(rel string, data []byte) error { + if err := cleanRel(rel); err != nil { + return err + } + if err := b.mkdirReal(path.Dir(rel)); err != nil { + return err + } + if fi, err := b.root.Lstat(rel); err == nil && !fi.Mode().IsRegular() { + return fmt.Errorf("%s: %w", rel, ErrNotRegular) + } else if err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + f, err := b.root.OpenFile(rel, os.O_WRONLY|os.O_APPEND|os.O_CREATE|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644) + if err != nil { + return err + } + defer func() { _ = f.Close() }() + hfi, herr := f.Stat() + lfi, lerr := b.root.Lstat(rel) + if herr != nil || lerr != nil || !lfi.Mode().IsRegular() || !os.SameFile(lfi, hfi) { + return fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular) + } + if _, err := f.Write(data); err != nil { + return err + } + return f.Close() +} + +// RealParent returns ErrNotRegular unless rel is clean and every directory +// above it is a real directory, for writers that do their own I/O (an atomic +// rename) but must not be redirected by a symlinked directory. +func (b *Brain) RealParent(rel string) error { + if err := cleanRel(rel); err != nil { + return err + } + return b.realDir(path.Dir(rel)) +} + +// mkdirReal makes sure every component of rel exists as a real directory, +// creating missing ones through the root; an existing component that is +// not a real directory is ErrNotRegular. +func (b *Brain) mkdirReal(rel string) error { + if rel == "." { + return nil + } + prefix := "" + for _, part := range strings.Split(rel, "/") { + prefix = path.Join(prefix, part) + fi, err := b.root.Lstat(prefix) + if errors.Is(err, fs.ErrNotExist) { + if err := b.root.Mkdir(prefix, 0o755); err != nil && !errors.Is(err, fs.ErrExist) { + return err + } + fi, err = b.root.Lstat(prefix) + } + if err != nil { + return err + } + if !fi.IsDir() { + return fmt.Errorf("%s: component %s is not a directory: %w", rel, prefix, ErrNotRegular) + } + } + return nil +} + // 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) { diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index b7a6fe1..eb3b9aa 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -376,3 +376,80 @@ func TestReadAndWriteRegularRefuseAPathThatIsNotClean(t *testing.T) { got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md")) assert.Equal(t, "untouched", string(got)) } + +// hyperguild#188: PutRegular creates or replaces a regular file through the +// root. Missing parent directories are created; an existing component that +// is not a real directory, a final symlink or special file, and an unclean +// path are refused, and nothing is created at a link's target. +func TestPutRegularCreatesOrReplacesOnlyThroughRealDirectories(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(dir, "wiki", "inwing"))) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "wiki", "outwing"))) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("untouched"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki", "w"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "v.md"), filepath.Join(dir, "wiki", "w", "link.md"))) + require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "wiki", "w", "f.md"), 0o644)) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + for _, rel := range []string{"wiki/inwing/facts/n.md", "wiki/outwing/facts/n.md", "wiki/w/link.md", "wiki/w/f.md", "wiki/w/../../raw/v.md", "/abs.md"} { + assert.ErrorIs(t, b.PutRegular(rel, []byte("x")), ErrNotRegular, rel) + } + ents, _ := os.ReadDir(outside) + assert.Empty(t, ents, "nothing created outside the brain") + _, err = os.Stat(filepath.Join(dir, "raw", "facts")) + assert.True(t, os.IsNotExist(err), "nothing created through the in-brain link") + got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md")) + assert.Equal(t, "untouched", string(got)) + + require.NoError(t, b.PutRegular("wiki/new/facts/n.md", []byte("one")), "positive control: create with missing parents") + got, _ = os.ReadFile(filepath.Join(dir, "wiki", "new", "facts", "n.md")) + assert.Equal(t, "one", string(got)) + require.NoError(t, b.PutRegular("wiki/new/facts/n.md", []byte("two")), "positive control: replace") + got, _ = os.ReadFile(filepath.Join(dir, "wiki", "new", "facts", "n.md")) + assert.Equal(t, "two", string(got)) +} + +func TestAppendRegularCreatesOrAppendsOnlyThroughRealDirectories(t *testing.T) { + dir := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("untouched"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "outdir"))) + require.NoError(t, os.Symlink("raw", filepath.Join(dir, "indir"))) + require.NoError(t, os.Symlink(filepath.Join("raw", "v.md"), filepath.Join(dir, "log.md"))) + require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "f.md"), 0o644)) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + for _, rel := range []string{"outdir/l.md", "indir/v.md", "log.md", "f.md", "raw/../log.md"} { + assert.ErrorIs(t, b.AppendRegular(rel, []byte("x")), ErrNotRegular, rel) + } + ents, _ := os.ReadDir(outside) + assert.Empty(t, ents) + got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md")) + assert.Equal(t, "untouched", string(got)) + + require.NoError(t, b.AppendRegular("logs/a.md", []byte("one\n")), "positive control: create") + require.NoError(t, b.AppendRegular("logs/a.md", []byte("two\n")), "positive control: append") + got, _ = os.ReadFile(filepath.Join(dir, "logs", "a.md")) + assert.Equal(t, "one\ntwo\n", string(got)) +} + +func TestRealParentRefusesASymlinkedDirectoryOrUncleanPath(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"), 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() }() + assert.ErrorIs(t, b.RealParent("wiki/alias/x.md"), ErrNotRegular) + assert.ErrorIs(t, b.RealParent("wiki/../raw/x.md"), ErrNotRegular) + assert.NoError(t, b.RealParent("wiki/x.md"), "positive control") +}