From 41b7105b313a2984de3ef578b1213ed998795f2a Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:35:19 +0200 Subject: [PATCH 1/4] 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") +} -- 2.54.0 From 138d6e2545384140fab4e41ac164368c07e3b2cc Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:39:41 +0200 Subject: [PATCH 2/4] fix: generated and promoted writes go through the brain root (#188, #158) Writers that created or rewrote brain files by path followed symlinks: - promote/reject: os.MkdirAll + os.WriteFile of the destination, then os.Remove of the raw source. Now PutRegular + brainfs.Remove; the source is kept when the write is refused. - StampProvenanceFile (brain_edit's re-stamp): provenance.ApplyFile read and rewrote by path. Now ReadRegular + WriteRegular; ApplyFile, now unused, is removed. - BuildWingIndex (_index.md) and wiki.RebuildIndex (wiki/index.md): now PutRegular. - wiki.AppendLog (log.md) and AutoTunnel's fuzzy-candidate log (raw/tunnel-candidates-*.md): now AppendRegular; tunnel's fileExists, now unused, is removed. - New brainfs.Remove: clean path, real parent, removes a final symlink itself, never its target. brain_edit's own atomic rename was already refused behind a symlinked directory by #187's read check; a guard test pins it. Pipeline writes, BackfillRefs, the watcher's raw/ markers and the session archive in cmd/server follow in a separate PR (#158). Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/api/promote.go | 20 +++--- .../internal/api/promote_redirect_test.go | 60 ++++++++++++++++++ ingestion/internal/api/provenance.go | 61 ++++++++++++++++++- ingestion/internal/brain/index.go | 7 ++- ingestion/internal/brain/tunnel.go | 37 ++++------- .../internal/brain/writers_redirect_test.go | 52 ++++++++++++++++ ingestion/internal/brainfs/brainfs.go | 10 +++ .../internal/notes/edit_redirect_test.go | 25 ++++++++ ingestion/internal/provenance/provenance.go | 19 ------ ingestion/internal/wiki/index.go | 6 +- ingestion/internal/wiki/log.go | 15 +++-- .../internal/wiki/writers_redirect_test.go | 35 +++++++++++ 12 files changed, 273 insertions(+), 74 deletions(-) create mode 100644 ingestion/internal/api/promote_redirect_test.go create mode 100644 ingestion/internal/brain/writers_redirect_test.go create mode 100644 ingestion/internal/notes/edit_redirect_test.go create mode 100644 ingestion/internal/wiki/writers_redirect_test.go diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index c6ec563..4f21bb0 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -127,18 +127,16 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { } fm.set("promoted_at", now) - if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { - return "", fmt.Errorf("create wing dir: %w", err) - } - // Write-then-delete: the source survives any write failure. + // Write-then-delete: the source survives any write failure. Both go + // through the brain root (hyperguild#188). promoted, err := StampProvenance(fm.render()+body, opts.Origin, opts.WrittenBy) if err != nil { return "", err } - if err := os.WriteFile(dest, []byte(promoted), 0o644); err != nil { + if _, err := putBrainFile(brainDir, dest, []byte(promoted)); err != nil { return "", fmt.Errorf("write promoted note: %w", err) } - if err := os.Remove(src); err != nil { + if err := removeBrainFile(brainDir, src); err != nil { return "", fmt.Errorf("promoted note written but source removal failed: %w", err) } @@ -196,14 +194,12 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) { fm.set("reject_reason", opts.Reason) } - if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { - return "", fmt.Errorf("create rejected dir: %w", err) - } - // Write-then-delete: the source survives any write failure. - if err := os.WriteFile(dest, []byte(fm.render()+body), 0o644); err != nil { + // Write-then-delete: the source survives any write failure. Both go + // through the brain root (hyperguild#188). + if _, err := putBrainFile(brainDir, dest, []byte(fm.render()+body)); err != nil { return "", fmt.Errorf("write rejected note: %w", err) } - if err := os.Remove(src); err != nil { + if err := removeBrainFile(brainDir, src); err != nil { return "", fmt.Errorf("rejected note written but source removal failed: %w", err) } diff --git a/ingestion/internal/api/promote_redirect_test.go b/ingestion/internal/api/promote_redirect_test.go new file mode 100644 index 0000000..49a21f2 --- /dev/null +++ b/ingestion/internal/api/promote_redirect_test.go @@ -0,0 +1,60 @@ +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: promote and reject wrote the destination with +// os.MkdirAll + os.WriteFile by path, so a symlinked wing (promote) or a +// symlinked raw/rejected (reject) redirected the note; the raw source was +// then removed. +func TestPromoteAndRejectRefuseASymlinkedDestinationDirectory(t *testing.T) { + brain := t.TempDir() + outside := t.TempDir() + raw := filepath.Join(brain, "raw") + require.NoError(t, os.MkdirAll(raw, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(raw, "2026-10-05-n.md"), []byte("---\ntitle: n\n---\nbody\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755)) + require.NoError(t, os.Symlink(outside, filepath.Join(brain, "wiki", "lw"))) + require.NoError(t, os.Symlink(outside, filepath.Join(raw, "rejected"))) + + _, err := api.PromoteNote(brain, api.PromoteOptions{Filename: "2026-10-05-n.md", Wing: "lw", Hall: "facts"}) + assert.Error(t, err) + _, err = api.RejectNote(brain, api.RejectOptions{Filename: "2026-10-05-n.md"}) + assert.Error(t, err) + ents, _ := os.ReadDir(outside) + assert.Empty(t, ents, "a note was written outside the brain") + _, err = os.Stat(filepath.Join(raw, "2026-10-05-n.md")) + assert.NoError(t, err, "the raw source is kept when the write is refused") + + require.NoError(t, os.Remove(filepath.Join(raw, "rejected"))) + _, err = api.PromoteNote(brain, api.PromoteOptions{Filename: "2026-10-05-n.md", Wing: "w", Hall: "facts"}) + require.NoError(t, err, "positive control: promote into a real wing") +} + +// StampProvenanceFile read and rewrote the note by path (provenance.ApplyFile). +func TestStampProvenanceFileRefusesANoteBehindASymlinkedDirectory(t *testing.T) { + brain := t.TempDir() + victim := filepath.Join(brain, "raw", "facts", "v.md") + require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755)) + require.NoError(t, os.WriteFile(victim, []byte("---\ntitle: v\n---\nuntouched\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "facts"), filepath.Join(brain, "wiki", "w", "alias"))) + assert.Error(t, api.StampProvenanceFile(brain, "wiki/w/alias/v.md", "static", "x")) + got, _ := os.ReadFile(victim) + assert.Equal(t, "---\ntitle: v\n---\nuntouched\n", string(got)) + + real := filepath.Join(brain, "wiki", "w", "facts", "r.md") + require.NoError(t, os.MkdirAll(filepath.Dir(real), 0o755)) + require.NoError(t, os.WriteFile(real, []byte("---\ntitle: r\n---\nbody\n"), 0o644)) + require.NoError(t, api.StampProvenanceFile(brain, "wiki/w/facts/r.md", "static", "x"), "positive control") + got, _ = os.ReadFile(real) + assert.Contains(t, string(got), "written_by: x") +} diff --git a/ingestion/internal/api/provenance.go b/ingestion/internal/api/provenance.go index 7a6a776..e2cafb5 100644 --- a/ingestion/internal/api/provenance.go +++ b/ingestion/internal/api/provenance.go @@ -1,6 +1,10 @@ package api import ( + "fmt" + "path/filepath" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "github.com/mathiasbq/hyperguild/ingestion/internal/provenance" ) @@ -11,11 +15,62 @@ func StampProvenance(content, origin, writtenBy string) (string, error) { return provenance.Apply(content, provenance.Stamp{Origin: origin, WrittenBy: writtenBy}) } -// StampProvenanceFile applies StampProvenance to a brain-relative note path. +// StampProvenanceFile applies StampProvenance to a brain-relative note path, +// read and rewritten through the brain root (hyperguild#158). func StampProvenanceFile(brainDir, relPath, origin, writtenBy string) error { - _, abs, err := resolveWithin(brainDir, relPath) + rel, _, err := resolveWithin(brainDir, relPath) if err != nil { return err } - return provenance.ApplyFile(abs, provenance.Stamp{Origin: origin, WrittenBy: writtenBy}) + bfs, err := brainfs.Open(brainDir) + if err != nil { + return fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + b, err := bfs.ReadRegular(rel) + if err != nil { + return fmt.Errorf("read for provenance: %w", err) + } + out, err := provenance.Apply(string(b), provenance.Stamp{Origin: origin, WrittenBy: writtenBy}) + if err != nil { + return err + } + if out == string(b) { + return nil + } + if err := bfs.WriteRegular(rel, []byte(out)); err != nil { + return fmt.Errorf("write provenance: %w", err) + } + return nil +} + +// putBrainFile creates or replaces the file at abs (inside brainDir) through +// the brain root, never through a symlinked directory or file (#188). +func putBrainFile(brainDir, abs string, data []byte) (string, error) { + rel, err := filepath.Rel(brainDir, abs) + if err != nil { + return "", err + } + rel = filepath.ToSlash(rel) + bfs, err := brainfs.Open(brainDir) + if err != nil { + return "", fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + return rel, bfs.PutRegular(rel, data) +} + +// removeBrainFile removes the file at abs (inside brainDir) through the +// brain root. +func removeBrainFile(brainDir, abs string) error { + rel, err := filepath.Rel(brainDir, abs) + if err != nil { + return err + } + bfs, err := brainfs.Open(brainDir) + if err != nil { + return fmt.Errorf("open brain: %w", err) + } + defer func() { _ = bfs.Close() }() + return bfs.Remove(filepath.ToSlash(rel)) } diff --git a/ingestion/internal/brain/index.go b/ingestion/internal/brain/index.go index d700b74..51373c0 100644 --- a/ingestion/internal/brain/index.go +++ b/ingestion/internal/brain/index.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "io/fs" - "os" "path/filepath" "sort" "strings" @@ -63,8 +62,10 @@ func BuildWingIndex(brainDir, wing string) error { fmt.Fprintf(&b, "| %s | [%s](%s/%s.md) | %s |\n", e.Hall, e.Title, e.Hall, e.Slug, e.Created) } - dest := filepath.Join(wingDir, "_index.md") - return os.WriteFile(dest, []byte(b.String()), 0o644) + // Written through the brain root, never through a symlinked _index.md + // (hyperguild#158). + _ = wingDir + return bfs.PutRegular("wiki/"+w+"/_index.md", []byte(b.String())) } // BuildAllWingIndexes regenerates _index.md for every wing under brain/wiki/. diff --git a/ingestion/internal/brain/tunnel.go b/ingestion/internal/brain/tunnel.go index db3be1b..ffc635d 100644 --- a/ingestion/internal/brain/tunnel.go +++ b/ingestion/internal/brain/tunnel.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io/fs" - "os" "path" "path/filepath" "strings" @@ -176,35 +175,23 @@ func logFuzzyCandidates(brainDir, sourcePath string, cs []TunnelCandidate) error if len(cs) == 0 { return nil } - rawDir := filepath.Join(brainDir, "raw") - if err := os.MkdirAll(rawDir, 0o755); err != nil { - return err - } - stamp := time.Now().UTC().Format("2006-01-02") - path := filepath.Join(rawDir, "tunnel-candidates-"+stamp+".md") - existed := fileExists(path) - f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + // Appended through the brain root, never through a symlinked raw/ or + // log file (hyperguild#158). + bfs, err := brainfs.Open(brainDir) if err != nil { - return err + return fmt.Errorf("open brain: %w", err) } - defer func() { _ = f.Close() }() - if !existed { - if _, err := f.WriteString("# Tunnel candidates " + stamp + "\n\nFuzzy cross-wing matches surfaced by AutoTunnel. Review and promote to a tunnel via `brain_tunnel` if relevant.\n\n"); err != nil { - return err - } + defer func() { _ = bfs.Close() }() + stamp := time.Now().UTC().Format("2006-01-02") + rel := "raw/tunnel-candidates-" + stamp + ".md" + var buf strings.Builder + if _, err := bfs.Lstat(rel); errors.Is(err, fs.ErrNotExist) { + buf.WriteString("# Tunnel candidates " + stamp + "\n\nFuzzy cross-wing matches surfaced by AutoTunnel. Review and promote to a tunnel via `brain_tunnel` if relevant.\n\n") } for _, c := range cs { - line := fmt.Sprintf("- `%s` ↔ `%s` (term: %q)\n", sourcePath, c.TargetPath, c.MatchedTerm) - if _, err := f.WriteString(line); err != nil { - return err - } + fmt.Fprintf(&buf, "- `%s` ↔ `%s` (term: %q)\n", sourcePath, c.TargetPath, c.MatchedTerm) } - return nil -} - -func fileExists(p string) bool { - _, err := os.Stat(p) - return err == nil + return bfs.AppendRegular(rel, []byte(buf.String())) } // WriteTunnel appends a bidirectional wikilink between sourcePath and diff --git a/ingestion/internal/brain/writers_redirect_test.go b/ingestion/internal/brain/writers_redirect_test.go new file mode 100644 index 0000000..66f7d2b --- /dev/null +++ b/ingestion/internal/brain/writers_redirect_test.go @@ -0,0 +1,52 @@ +package brain_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/mathiasbq/hyperguild/ingestion/internal/brain" +) + +// hyperguild#158: BuildWingIndex wrote wiki//_index.md with +// os.WriteFile, so a symlinked _index.md was written through. +func TestBuildWingIndexDoesNotWriteThroughASymlinkedIndex(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "w", "facts", "n.md"), []byte("---\ntitle: zqn\n---\n"), 0o644)) + victim := filepath.Join(b, "raw", "v.md") + require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755)) + require.NoError(t, os.WriteFile(victim, []byte("untouched"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "v.md"), filepath.Join(b, "wiki", "w", "_index.md"))) + assert.Error(t, brain.BuildWingIndex(b, "w")) + got, _ := os.ReadFile(victim) + assert.Equal(t, "untouched", string(got)) + + require.NoError(t, os.Remove(filepath.Join(b, "wiki", "w", "_index.md"))) + require.NoError(t, brain.BuildWingIndex(b, "w"), "positive control") + idx, _ := os.ReadFile(filepath.Join(b, "wiki", "w", "_index.md")) + assert.Contains(t, string(idx), "zqn") +} + +// AutoTunnel's fuzzy-candidate log appended to raw/tunnel-candidates-*.md +// with os.OpenFile, so a symlinked raw/ (or log file) was written through. +func TestAutoTunnelDoesNotAppendTheCandidateLogThroughASymlinkedRaw(t *testing.T) { + b := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "a", "facts"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "b", "facts"), 0o755)) + // "zq widget" matches as a substring of "zq widgets", a fuzzy candidate. + require.NoError(t, os.WriteFile(filepath.Join(b, "wiki", "b", "facts", "t.md"), []byte("---\ntitle: Zq Widget\n---\n"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(b, "raw"))) + _ = brain.AutoTunnel(b, "wiki/a/facts/s.md", "about zq widgets") + ents, _ := os.ReadDir(outside) + assert.Empty(t, ents, "the candidate log was written outside the brain") + + require.NoError(t, os.Remove(filepath.Join(b, "raw"))) + require.NoError(t, brain.AutoTunnel(b, "wiki/a/facts/s.md", "about zq widgets")) + logs, _ := filepath.Glob(filepath.Join(b, "raw", "tunnel-candidates-*.md")) + assert.Len(t, logs, 1, "positive control: the fuzzy candidate is logged in the real raw/") +} diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index eec5f8e..414d9a2 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -204,6 +204,16 @@ func (b *Brain) AppendRegular(rel string, data []byte) error { return f.Close() } +// Remove removes the file at a clean brain-relative path through the root, +// refusing a path behind a symlinked directory. A final symlink is removed +// itself, never its target. +func (b *Brain) Remove(rel string) error { + if err := b.RealParent(rel); err != nil { + return err + } + return b.root.Remove(rel) +} + // 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. diff --git a/ingestion/internal/notes/edit_redirect_test.go b/ingestion/internal/notes/edit_redirect_test.go new file mode 100644 index 0000000..7a75399 --- /dev/null +++ b/ingestion/internal/notes/edit_redirect_test.go @@ -0,0 +1,25 @@ +package notes + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// hyperguild#188/#173: brain_edit's atomic rename must not be redirected by +// a symlinked directory inside the brain. +func TestEditRefusesANoteBehindASymlinkedDirectory(t *testing.T) { + b := t.TempDir() + victim := filepath.Join(b, "raw", "facts", "v.md") + require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755)) + require.NoError(t, os.WriteFile(victim, []byte("old text\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "w"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "facts"), filepath.Join(b, "wiki", "w", "alias"))) + _, err := Edit(b, "wiki/w/alias/v.md", "old text", "new text", ScopeNotes) + assert.Error(t, err) + got, _ := os.ReadFile(victim) + assert.Equal(t, "old text\n", string(got)) +} diff --git a/ingestion/internal/provenance/provenance.go b/ingestion/internal/provenance/provenance.go index 1343e0d..d48807f 100644 --- a/ingestion/internal/provenance/provenance.go +++ b/ingestion/internal/provenance/provenance.go @@ -28,7 +28,6 @@ import ( "context" "errors" "fmt" - "os" "regexp" "strings" @@ -102,24 +101,6 @@ func Apply(content string, s Stamp) (string, error) { return out, nil } -// ApplyFile applies the stamp to the note at absPath in place. -func ApplyFile(absPath string, s Stamp) error { - b, err := os.ReadFile(absPath) - if err != nil { - return fmt.Errorf("read for provenance: %w", err) - } - out, err := Apply(string(b), s) - if err != nil { - return err - } - if out == string(b) { - return nil - } - if err := os.WriteFile(absPath, []byte(out), 0o644); err != nil { - return fmt.Errorf("write provenance: %w", err) - } - return nil -} // plainGroups keeps the provably plain key groups of a frontmatter block. func plainGroups(block string) string { diff --git a/ingestion/internal/wiki/index.go b/ingestion/internal/wiki/index.go index 0bab797..aa1b5d5 100644 --- a/ingestion/internal/wiki/index.go +++ b/ingestion/internal/wiki/index.go @@ -3,8 +3,6 @@ package wiki import ( "fmt" - "os" - "path/filepath" "strings" "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" @@ -50,8 +48,8 @@ func RebuildIndex(brainDir, date string) error { sb.WriteString("\n") } - dest := filepath.Join(brainDir, "wiki", "index.md") - return os.WriteFile(dest, []byte(sb.String()), 0o644) + // Written through the brain root, never through a symlink (#158). + return bfs.PutRegular("wiki/index.md", []byte(sb.String())) } // pageFirstSentence reads the page through the brain root, regular files diff --git a/ingestion/internal/wiki/log.go b/ingestion/internal/wiki/log.go index 1b19ae8..a904da5 100644 --- a/ingestion/internal/wiki/log.go +++ b/ingestion/internal/wiki/log.go @@ -3,8 +3,7 @@ package wiki import ( "fmt" - "os" - "path/filepath" + "github.com/mathiasbq/hyperguild/ingestion/internal/brainfs" "strings" ) @@ -27,14 +26,14 @@ func AppendLog(brainDir, source string, pages, warnings []string, date string) e } sb.WriteString("\n") - logPath := filepath.Join(brainDir, "log.md") - f, err := os.OpenFile(logPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + // Appended through the brain root, never through a symlink (#158). + bfs, err := brainfs.Open(brainDir) if err != nil { - return fmt.Errorf("open log: %w", err) + return fmt.Errorf("open brain: %w", err) } - if _, err = f.WriteString(sb.String()); err != nil { - f.Close() //nolint:errcheck + defer func() { _ = bfs.Close() }() + if err := bfs.AppendRegular("log.md", []byte(sb.String())); err != nil { return fmt.Errorf("write log: %w", err) } - return f.Close() + return nil } diff --git a/ingestion/internal/wiki/writers_redirect_test.go b/ingestion/internal/wiki/writers_redirect_test.go new file mode 100644 index 0000000..f47267e --- /dev/null +++ b/ingestion/internal/wiki/writers_redirect_test.go @@ -0,0 +1,35 @@ +package wiki + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// hyperguild#158: RebuildIndex wrote wiki/index.md and AppendLog appended +// log.md with os.WriteFile / os.OpenFile, so a symlinked file was written +// through. +func TestRebuildIndexAndAppendLogDoNotWriteThroughSymlinks(t *testing.T) { + b := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "concepts"), 0o755)) + victim := filepath.Join(b, "raw", "v.md") + require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755)) + require.NoError(t, os.WriteFile(victim, []byte("untouched"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw", "v.md"), filepath.Join(b, "wiki", "index.md"))) + require.NoError(t, os.Symlink(filepath.Join("raw", "v.md"), filepath.Join(b, "log.md"))) + + assert.Error(t, RebuildIndex(b, "2026-10-05")) + assert.Error(t, AppendLog(b, "src", []string{"p"}, nil, "2026-10-05")) + got, _ := os.ReadFile(victim) + assert.Equal(t, "untouched", string(got)) + + require.NoError(t, os.Remove(filepath.Join(b, "wiki", "index.md"))) + require.NoError(t, os.Remove(filepath.Join(b, "log.md"))) + require.NoError(t, RebuildIndex(b, "2026-10-05"), "positive control") + require.NoError(t, AppendLog(b, "src", []string{"p"}, nil, "2026-10-05"), "positive control") + l, _ := os.ReadFile(filepath.Join(b, "log.md")) + assert.Contains(t, string(l), "src") +} -- 2.54.0 From db18785bfc2b2f224b37e51e4478f457f83febbf Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:57:26 +0200 Subject: [PATCH 3/4] test: pin the edit symlink refusal, AppendRegular's clean check and Remove (#188) Review round 1 on PR #189 (three tests stayed green with their guard removed): - TestEditRefusesANoteBehindASymlinkedDirectory had no .git, so Edit refused "not the sync source" before any path check. It now has one, plus a positive control. - AppendRegular's unclean-path case was refused by a final symlink, not cleanRel. New test where ".." after a symlinked dir resolves to a real file, for AppendRegular and PutRegular. - brainfs.Remove had no test; new one (refuses behind a symlinked dir, removes a final link but never its target, positive control). Also: promote/reject create with O_EXCL (new brainfs.CreateRegular), so a note that appears after the collision check is never overwritten; gofmt fixes in provenance.go and wiki/log.go; dead wingDir and putBrainFile removed. Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/api/promote.go | 6 ++- ingestion/internal/api/provenance.go | 13 +++--- ingestion/internal/brain/index.go | 3 -- ingestion/internal/brainfs/brainfs.go | 19 +++++++++ ingestion/internal/brainfs/brainfs_test.go | 40 +++++++++++++++++++ .../internal/notes/edit_redirect_test.go | 11 +++++ ingestion/internal/provenance/provenance.go | 1 - 7 files changed, 80 insertions(+), 13 deletions(-) diff --git a/ingestion/internal/api/promote.go b/ingestion/internal/api/promote.go index 4f21bb0..cfeae31 100644 --- a/ingestion/internal/api/promote.go +++ b/ingestion/internal/api/promote.go @@ -133,7 +133,9 @@ func PromoteNote(brainDir string, opts PromoteOptions) (string, error) { if err != nil { return "", err } - if _, err := putBrainFile(brainDir, dest, []byte(promoted)); err != nil { + // Created with O_EXCL: a note that appeared after the collision check + // above is never overwritten. + if err := createBrainFile(brainDir, dest, []byte(promoted)); err != nil { return "", fmt.Errorf("write promoted note: %w", err) } if err := removeBrainFile(brainDir, src); err != nil { @@ -196,7 +198,7 @@ func RejectNote(brainDir string, opts RejectOptions) (string, error) { // Write-then-delete: the source survives any write failure. Both go // through the brain root (hyperguild#188). - if _, err := putBrainFile(brainDir, dest, []byte(fm.render()+body)); err != nil { + if err := createBrainFile(brainDir, dest, []byte(fm.render()+body)); err != nil { return "", fmt.Errorf("write rejected note: %w", err) } if err := removeBrainFile(brainDir, src); err != nil { diff --git a/ingestion/internal/api/provenance.go b/ingestion/internal/api/provenance.go index e2cafb5..0cd107c 100644 --- a/ingestion/internal/api/provenance.go +++ b/ingestion/internal/api/provenance.go @@ -44,20 +44,19 @@ func StampProvenanceFile(brainDir, relPath, origin, writtenBy string) error { return nil } -// putBrainFile creates or replaces the file at abs (inside brainDir) through -// the brain root, never through a symlinked directory or file (#188). -func putBrainFile(brainDir, abs string, data []byte) (string, error) { +// createBrainFile creates the file at abs (inside brainDir) through the +// brain root and fails if anything is already there. +func createBrainFile(brainDir, abs string, data []byte) error { rel, err := filepath.Rel(brainDir, abs) if err != nil { - return "", err + return err } - rel = filepath.ToSlash(rel) bfs, err := brainfs.Open(brainDir) if err != nil { - return "", fmt.Errorf("open brain: %w", err) + return fmt.Errorf("open brain: %w", err) } defer func() { _ = bfs.Close() }() - return rel, bfs.PutRegular(rel, data) + return bfs.CreateRegular(filepath.ToSlash(rel), data) } // removeBrainFile removes the file at abs (inside brainDir) through the diff --git a/ingestion/internal/brain/index.go b/ingestion/internal/brain/index.go index 51373c0..86dc3e3 100644 --- a/ingestion/internal/brain/index.go +++ b/ingestion/internal/brain/index.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "io/fs" - "path/filepath" "sort" "strings" "time" @@ -31,7 +30,6 @@ func BuildWingIndex(brainDir, wing string) error { if w == "" { return fmt.Errorf("invalid wing %q", wing) } - wingDir := filepath.Join(brainDir, "wiki", w) // Listed and read through the brain root (hyperguild#157): a wing or // hall that is a symlink is not followed, and notes are regular files. bfs, err := brainfs.Open(brainDir) @@ -64,7 +62,6 @@ func BuildWingIndex(brainDir, wing string) error { // Written through the brain root, never through a symlinked _index.md // (hyperguild#158). - _ = wingDir return bfs.PutRegular("wiki/"+w+"/_index.md", []byte(b.String())) } diff --git a/ingestion/internal/brainfs/brainfs.go b/ingestion/internal/brainfs/brainfs.go index 414d9a2..f7f423d 100644 --- a/ingestion/internal/brainfs/brainfs.go +++ b/ingestion/internal/brainfs/brainfs.go @@ -156,6 +156,25 @@ func (b *Brain) PutRegular(rel string, data []byte) error { } else if !errors.Is(err, fs.ErrNotExist) { return err } + return b.createNew(rel, data) +} + +// CreateRegular creates a new regular file at a brain-relative path, with +// PutRegular's parent rules, and fails with fs.ErrExist if anything is +// already there: for writers that must never overwrite (promote, reject). +func (b *Brain) CreateRegular(rel string, data []byte) error { + if err := cleanRel(rel); err != nil { + return err + } + if err := b.mkdirReal(path.Dir(rel)); err != nil { + return err + } + return b.createNew(rel, data) +} + +// createNew creates rel with O_EXCL and writes data only after confirming +// the path is the regular file it created. +func (b *Brain) createNew(rel string, data []byte) error { 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 diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index eb3b9aa..11b8b7d 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -453,3 +453,43 @@ func TestRealParentRefusesASymlinkedDirectoryOrUncleanPath(t *testing.T) { assert.ErrorIs(t, b.RealParent("wiki/../raw/x.md"), ErrNotRegular) assert.NoError(t, b.RealParent("wiki/x.md"), "positive control") } + +// cleanRel must refuse an unclean path on its own: here the ".." resolves, +// after a symlinked directory, to a real regular file, so only the clean +// check stands between the call and that file. +func TestAppendRegularRefusesAnUncleanPathThatResolvesToARealFile(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw", "keep"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("untouched"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("raw", "keep"), filepath.Join(dir, "indir"))) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + assert.ErrorIs(t, b.AppendRegular("indir/../v.md", []byte("x")), ErrNotRegular) + assert.ErrorIs(t, b.PutRegular("indir/../v.md", []byte("x")), ErrNotRegular) + got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md")) + assert.Equal(t, "untouched", string(got)) +} + +func TestRemoveRefusesAPathBehindASymlinkedDirectoryAndRemovesOnlyTheLink(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("keep"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "raw"), filepath.Join(dir, "wiki", "alias"))) + require.NoError(t, os.Symlink(filepath.Join("..", "raw", "v.md"), filepath.Join(dir, "wiki", "link.md"))) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki", "n.md"), []byte("n"), 0o644)) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + assert.ErrorIs(t, b.Remove("wiki/alias/v.md"), ErrNotRegular) + _, err = os.Stat(filepath.Join(dir, "raw", "v.md")) + assert.NoError(t, err, "the file behind the symlinked directory is kept") + require.NoError(t, b.Remove("wiki/link.md")) + _, err = os.Stat(filepath.Join(dir, "raw", "v.md")) + assert.NoError(t, err, "removing a final symlink never removes its target") + require.NoError(t, b.Remove("wiki/n.md"), "positive control") + _, err = os.Stat(filepath.Join(dir, "wiki", "n.md")) + assert.True(t, os.IsNotExist(err)) +} diff --git a/ingestion/internal/notes/edit_redirect_test.go b/ingestion/internal/notes/edit_redirect_test.go index 7a75399..dc52575 100644 --- a/ingestion/internal/notes/edit_redirect_test.go +++ b/ingestion/internal/notes/edit_redirect_test.go @@ -13,6 +13,9 @@ import ( // a symlinked directory inside the brain. func TestEditRefusesANoteBehindASymlinkedDirectory(t *testing.T) { b := t.TempDir() + // Edit refuses a tree that is not the sync source before any path + // check, so the test needs a .git to reach the symlink refusal. + require.NoError(t, os.MkdirAll(filepath.Join(b, ".git"), 0o755)) victim := filepath.Join(b, "raw", "facts", "v.md") require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755)) require.NoError(t, os.WriteFile(victim, []byte("old text\n"), 0o644)) @@ -22,4 +25,12 @@ func TestEditRefusesANoteBehindASymlinkedDirectory(t *testing.T) { assert.Error(t, err) got, _ := os.ReadFile(victim) assert.Equal(t, "old text\n", string(got)) + + real := filepath.Join(b, "wiki", "w", "facts", "r.md") + require.NoError(t, os.MkdirAll(filepath.Dir(real), 0o755)) + require.NoError(t, os.WriteFile(real, []byte("old text\n"), 0o644)) + _, err = Edit(b, "wiki/w/facts/r.md", "old text", "new text", ScopeNotes) + require.NoError(t, err, "positive control: a note in a real directory is edited") + got, _ = os.ReadFile(real) + assert.Equal(t, "new text\n", string(got)) } diff --git a/ingestion/internal/provenance/provenance.go b/ingestion/internal/provenance/provenance.go index d48807f..040f4fc 100644 --- a/ingestion/internal/provenance/provenance.go +++ b/ingestion/internal/provenance/provenance.go @@ -101,7 +101,6 @@ func Apply(content string, s Stamp) (string, error) { return out, nil } - // plainGroups keeps the provably plain key groups of a frontmatter block. func plainGroups(block string) string { lines := strings.Split(strings.TrimSuffix(block, "\n"), "\n") -- 2.54.0 From ac84ddf5a954b4202c6546d7ad1e9e8a2dcdfe4f Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 23:19:02 +0200 Subject: [PATCH 4/4] test: pin CreateRegular directly and promote/reject with in-brain targets (#188) Review round 2 on PR #189: CreateRegular (promote's and reject's only write path) had no test; replacing mkdirReal with root.MkdirAll, dropping cleanRel or calling PutRegular all stayed green, and the promote/reject test planted only outside targets, which os.Root refuses on its own. - TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories: in-brain symlinked parent, unclean path, existing file and dangling link (fs.ErrExist, nothing changed), positive control with missing parents. - TestPromoteAndRejectRefuseAnInBrainSymlinkedDestinationDirectory. All three flips are red now. Co-Authored-By: Claude Opus 5.5 --- .../internal/api/promote_redirect_test.go | 27 ++++++++++++++ ingestion/internal/brainfs/brainfs_test.go | 36 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/ingestion/internal/api/promote_redirect_test.go b/ingestion/internal/api/promote_redirect_test.go index 49a21f2..4e91c1e 100644 --- a/ingestion/internal/api/promote_redirect_test.go +++ b/ingestion/internal/api/promote_redirect_test.go @@ -58,3 +58,30 @@ func TestStampProvenanceFileRefusesANoteBehindASymlinkedDirectory(t *testing.T) got, _ = os.ReadFile(real) assert.Contains(t, string(got), "written_by: x") } + +// The same through an in-brain symlinked wing and raw/rejected, which +// os.Root alone would follow. +func TestPromoteAndRejectRefuseAnInBrainSymlinkedDestinationDirectory(t *testing.T) { + brain := t.TempDir() + raw := filepath.Join(brain, "raw") + require.NoError(t, os.MkdirAll(raw, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(raw, "2026-10-05-n.md"), []byte("---\ntitle: n\n---\nbody\n"), 0o644)) + target := filepath.Join(brain, "archive") + require.NoError(t, os.MkdirAll(target, 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755)) + require.NoError(t, os.Symlink(filepath.Join("..", "archive"), filepath.Join(brain, "wiki", "lw"))) + require.NoError(t, os.Symlink(filepath.Join("..", "archive"), filepath.Join(raw, "rejected"))) + + _, err := api.PromoteNote(brain, api.PromoteOptions{Filename: "2026-10-05-n.md", Wing: "lw", Hall: "facts"}) + assert.Error(t, err) + _, err = api.RejectNote(brain, api.RejectOptions{Filename: "2026-10-05-n.md"}) + assert.Error(t, err) + ents, _ := os.ReadDir(target) + assert.Empty(t, ents, "a note was written at the in-brain link's target") + _, err = os.Stat(filepath.Join(raw, "2026-10-05-n.md")) + assert.NoError(t, err, "the raw source is kept") + + require.NoError(t, os.Remove(filepath.Join(raw, "rejected"))) + _, err = api.RejectNote(brain, api.RejectOptions{Filename: "2026-10-05-n.md"}) + require.NoError(t, err, "positive control: reject into a real raw/rejected") +} diff --git a/ingestion/internal/brainfs/brainfs_test.go b/ingestion/internal/brainfs/brainfs_test.go index 11b8b7d..0f5570b 100644 --- a/ingestion/internal/brainfs/brainfs_test.go +++ b/ingestion/internal/brainfs/brainfs_test.go @@ -493,3 +493,39 @@ func TestRemoveRefusesAPathBehindASymlinkedDirectoryAndRemovesOnlyTheLink(t *tes _, err = os.Stat(filepath.Join(dir, "wiki", "n.md")) assert.True(t, os.IsNotExist(err)) } + +// CreateRegular is the only write path for promote and reject: it creates +// missing parents through the root, refuses an in-brain symlinked parent and +// an unclean path, and never replaces anything already there (fs.ErrExist, +// content unchanged), including a dangling link. +func TestCreateRegularCreatesOnlyNewFilesThroughRealDirectories(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw", "keep"), 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.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("untouched"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join("raw", "keep"), filepath.Join(dir, "indir"))) + require.NoError(t, os.WriteFile(filepath.Join(dir, "wiki", "exists.md"), []byte("original"), 0o644)) + require.NoError(t, os.Symlink("nowhere.md", filepath.Join(dir, "wiki", "dangling.md"))) + b, err := Open(dir) + require.NoError(t, err) + defer func() { _ = b.Close() }() + + assert.ErrorIs(t, b.CreateRegular("wiki/inwing/facts/n.md", []byte("x")), ErrNotRegular) + _, err = os.Stat(filepath.Join(dir, "raw", "facts")) + assert.True(t, os.IsNotExist(err), "nothing created through the in-brain link") + assert.ErrorIs(t, b.CreateRegular("indir/../v.md", []byte("x")), ErrNotRegular) + got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md")) + assert.Equal(t, "untouched", string(got)) + + assert.ErrorIs(t, b.CreateRegular("wiki/exists.md", []byte("x")), fs.ErrExist) + got, _ = os.ReadFile(filepath.Join(dir, "wiki", "exists.md")) + assert.Equal(t, "original", string(got)) + assert.ErrorIs(t, b.CreateRegular("wiki/dangling.md", []byte("x")), fs.ErrExist) + _, err = os.Stat(filepath.Join(dir, "wiki", "nowhere.md")) + assert.True(t, os.IsNotExist(err), "nothing created at the dangling link's target") + + require.NoError(t, b.CreateRegular("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)) +} -- 2.54.0