fix(brainfs,session): clean paths only; session logs open through the brain root (#173)

Review round 1 on PR #187:

- realDir checked the cleaned parent while Lstat/OpenFile used the raw
  path; os.Root resolves ".." after following a symlink, so a ".." placed
  after a symlinked directory passed the check and redirected the read or
  write. ReadRegular, ReadRegularMax and WriteRegular now refuse a path
  that is not clean and local (ErrNotRegular). No current entry point
  reached it (callers clean first); the primitive no longer relies on that.
- session.Append checked the sessions dir with Lstat, then opened by plain
  path: a symlink swapped in between created logs outside the brain
  (29k in the reviewer's RENAME_EXCHANGE race). It now opens through an
  os.Root on the sessions dir's parent, so a swap can only redirect inside
  the brain (#167), and, since os.Root follows an in-root final symlink
  even with O_NOFOLLOW, writes only after confirming the path is the
  regular file it opened (Lstat + os.SameFile).
- Endpoint tests assert ErrorIs(brainfs.ErrNotRegular) and have positive
  controls.

golang.org/x/sys moves from indirect to direct (unix.Renameat2 in the
session race test); it was already in the module graph.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ingestion/go.mod

@@ -68,7 +68,7 @@ require (
 	github.com/segmentio/asm v1.2.0 // indirect
 	golang.org/x/crypto v0.51.0 // indirect
 	golang.org/x/sync v0.21.0 // indirect
-	golang.org/x/sys v0.45.0 // indirect
+	golang.org/x/sys v0.45.0
 	golang.org/x/text v0.39.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1
 )

ingestion/internal/api/update_redirect_test.go

@@ -0,0 +1,36 @@
+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"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
+)
+
+// hyperguild#173: a directory symlink inside wiki/ that points elsewhere in
+// the brain redirected brain_update's write (os.Root follows in-root
+// intermediate symlinks). Every directory above the note must be real.
+func TestUpdateNoteRefusesANoteBehindASymlinkedDirectory(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")))
+
+	_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/alias/v.md", Content: "replaced"})
+	assert.ErrorIs(t, err, brainfs.ErrNotRegular)
+	got, _ := os.ReadFile(victim)
+	assert.Contains(t, string(got), "untouched")
+
+	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---\nold\n"), 0o644))
+	_, _, _, err = api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/r.md", Content: "new"})
+	require.NoError(t, err, "positive control: a note in a real directory is updated")
+}

ingestion/internal/brain/detect_root_test.go

@@ -1,125 +0,0 @@
-package brain_test
-
-import (
-	"fmt"
-	"os"
-	"path/filepath"
-	"syscall"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
-	"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
-	"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
-)
-
-// hyperguild#174 / #157 slice 5b: DetectTunnels walked wiki/ and opened
-// every note with a blocking os.Open, so a FIFO named *.md anywhere under
-// wiki/ hung every brain_write and brain_update (wikiUpkeep -> AutoTunnel),
-// and a symlinked note's title became a tunnel candidate.
-func TestDetectTunnelsReturnsPromptlyWithAFIFOUnderWikiWithAndWithoutAWriter(t *testing.T) {
-	for _, withWriter := range []bool{false, true} {
-		t.Run(fmt.Sprintf("writer=%v", withWriter), func(t *testing.T) {
-			b := t.TempDir()
-			facts := filepath.Join(b, "wiki", "w", "facts")
-			require.NoError(t, os.MkdirAll(facts, 0o755))
-			require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: Zqinside Phrase\n---\n"), 0o644))
-			fifo := filepath.Join(facts, "f.md")
-			require.NoError(t, syscall.Mkfifo(fifo, 0o644))
-			if withWriter {
-				// A writer held open: a reader that opens the FIFO gets no
-				// EOF and parks, unless the FIFO is never opened.
-				w, err := os.OpenFile(fifo, os.O_RDWR, 0)
-				require.NoError(t, err)
-				t.Cleanup(func() { _ = w.Close() })
-			}
-			done := make(chan []brain.TunnelCandidate, 1)
-			go func() {
-				cs, _ := brain.DetectTunnels(b, "text mentioning zqinside phrase here")
-				done <- cs
-			}()
-			select {
-			case cs := <-done:
-				assert.Contains(t, fmt.Sprintf("%+v", cs), "Zqinside Phrase", "positive control")
-			case <-time.After(3 * time.Second):
-				if r, err := os.OpenFile(fifo, os.O_WRONLY|syscall.O_NONBLOCK, 0); err == nil {
-					_ = r.Close()
-				}
-				t.Fatal("DetectTunnels blocked on a FIFO")
-			}
-		})
-	}
-}
-
-func TestDetectTunnelsRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
-	b := t.TempDir()
-	facts := filepath.Join(b, "wiki", "w", "facts")
-	require.NoError(t, os.MkdirAll(facts, 0o755))
-	// The static note's title differs from the raced note's, so the
-	// harness's "inside was read" floor can only be met by reading r.md.
-	require.NoError(t, os.WriteFile(filepath.Join(facts, "ok.md"), []byte("---\ntitle: Zqstatic Words\n---\n"), 0o644))
-	require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "out.md")))
-	inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(b, "raw", "s.md"))
-	require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "in.md")))
-	content := "zqstatic words, zqinside phrase and " + brainfstest.Sentinel
-
-	cs, err := brain.DetectTunnels(b, content)
-	require.NoError(t, err)
-	got := fmt.Sprintf("%+v", cs)
-	assert.Contains(t, got, "Zqstatic Words", "positive control")
-	assert.NotContains(t, got, brainfstest.Sentinel)
-
-	p := filepath.Join(facts, "r.md")
-	read := func() string {
-		cs, _ := brain.DetectTunnels(b, content)
-		return fmt.Sprintf("%+v", cs)
-	}
-	brainfstest.AssertNoLeak(t, p, "---\ntitle: Zqinside Phrase\n---\n", 1500*time.Millisecond, read)
-	brainfstest.AssertNoLeakFrom(t, p, "---\ntitle: Zqinside Phrase\n---\n", inBrain, 1500*time.Millisecond, read)
-}
-
-// A wiki/ that is not a real directory yields no candidates and no error:
-// AutoTunnel runs inside brain_write.
-func TestDetectTunnelsSkipsAWikiThatIsNotARealDirectory(t *testing.T) {
-	b := t.TempDir()
-	require.NoError(t, os.MkdirAll(filepath.Join(b, "raw", "w", "facts"), 0o755))
-	require.NoError(t, os.WriteFile(filepath.Join(b, "raw", "w", "facts", "r.md"), []byte("---\ntitle: Zqrawonly Phrase\n---\n"), 0o644))
-	require.NoError(t, os.Symlink("raw", filepath.Join(b, "wiki")))
-	cs, err := brain.DetectTunnels(b, "zqrawonly phrase")
-	require.NoError(t, err)
-	assert.Empty(t, cs)
-}
-
-// An unreadable directory under wiki/ is a walk error and DetectTunnels
-// reports it, as filepath.WalkDir did; it is not silently dropped.
-func TestDetectTunnelsReportsAnUnreadableDirectoryUnderWiki(t *testing.T) {
-	if os.Geteuid() == 0 {
-		t.Skip("root ignores directory permissions")
-	}
-	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", "ok.md"), []byte("---\ntitle: Zqinside Phrase\n---\n"), 0o644))
-	locked := filepath.Join(b, "wiki", "other", "locked")
-	require.NoError(t, os.MkdirAll(locked, 0o755))
-	require.NoError(t, os.Chmod(locked, 0o000))
-	t.Cleanup(func() { _ = os.Chmod(locked, 0o755) })
-	_, err := brain.DetectTunnels(b, "zqinside phrase")
-	assert.ErrorIs(t, err, os.ErrPermission)
-}
-
-// _index.md files are generated, not notes: their title never becomes a
-// candidate.
-func TestDetectTunnelsSkipsIndexFiles(t *testing.T) {
-	b := t.TempDir()
-	w := filepath.Join(b, "wiki", "w")
-	require.NoError(t, os.MkdirAll(filepath.Join(w, "facts"), 0o755))
-	require.NoError(t, os.WriteFile(filepath.Join(w, "_index.md"), []byte("---\ntitle: Zqindex Words\n---\n"), 0o644))
-	require.NoError(t, os.WriteFile(filepath.Join(w, "facts", "ok.md"), []byte("---\ntitle: Zqinside Phrase\n---\n"), 0o644))
-	cs, err := brain.DetectTunnels(b, "zqindex words and zqinside phrase")
-	require.NoError(t, err)
-	got := fmt.Sprintf("%+v", cs)
-	assert.Contains(t, got, "Zqinside Phrase", "positive control")
-	assert.NotContains(t, got, "Zqindex")
-}

ingestion/internal/brain/index.go

@@ -116,6 +116,23 @@ func collectWingEntries(bfs *brainfs.Brain, wingRel string) ([]noteEntry, error)
 	return out, nil
 }
 
+// readTitleAndCreated reads YAML frontmatter for title + created_at; falls
+// back to slug and file mtime when absent.
+func readTitleAndCreated(path, slug string) (string, string) {
+	f, err := os.Open(path)
+	if err != nil {
+		return slug, ""
+	}
+	defer func() { _ = f.Close() }()
+	title, created := parseTitleAndCreated(f)
+	if created == "" {
+		if info, err := os.Stat(path); err == nil {
+			created = info.ModTime().UTC().Format("2006-01-02")
+		}
+	}
+	return titleOrSlug(title, slug), created
+}
+
 // readTitleAndCreatedIn is readTitleAndCreated through the brain root,
 // regular files only (hyperguild#157).
 func readTitleAndCreatedIn(bfs *brainfs.Brain, rel, slug string) (string, string) {

ingestion/internal/brain/tunnel.go

@@ -2,9 +2,7 @@ package brain
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
-	"io/fs"
 	"os"
 	"path"
 	"path/filepath"
@@ -41,29 +39,24 @@ type TunnelCandidate struct {
 // A note's title is read from YAML frontmatter `title:`; failing that,
 // the filename slug (sans `.md`, hyphens → spaces) is used.
 func DetectTunnels(brainDir, content string) ([]TunnelCandidate, error) {
-	// Walked and read through the brain root, regular files only
-	// (hyperguild#174, #157): this runs inside every brain_write, so a FIFO
-	// must not block it and a symlinked note's title must not become a
-	// candidate.
-	bfs, err := brainfs.Open(brainDir)
-	if err != nil {
-		return nil, fmt.Errorf("open brain: %w", err)
-	}
-	defer func() { _ = bfs.Close() }()
-	if _, err := bfs.Lstat("wiki"); errors.Is(err, fs.ErrNotExist) {
+	wikiDir := filepath.Join(brainDir, "wiki")
+	if _, err := os.Stat(wikiDir); os.IsNotExist(err) {
 		return nil, nil
+	} else if err != nil {
+		return nil, fmt.Errorf("stat wiki: %w", err)
 	}
 
 	lowerContent := strings.ToLower(content)
 
 	var out []TunnelCandidate
-	var walkErr error
-	err = bfs.WalkRegular("wiki", func(rel string) error {
-		name := path.Base(rel)
-		if !strings.HasSuffix(name, ".md") || name == "_index.md" {
+	err := filepath.WalkDir(wikiDir, func(path string, d os.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() || !strings.HasSuffix(path, ".md") || d.Name() == "_index.md" {
 			return nil
 		}
-		title, _ := readTitleAndCreatedIn(bfs, rel, strings.TrimSuffix(name, ".md"))
+		title, _ := readTitleAndCreated(path, strings.TrimSuffix(d.Name(), ".md"))
 		title = strings.TrimSpace(title)
 		if title == "" {
 			return nil
@@ -85,25 +78,17 @@ func DetectTunnels(brainDir, content string) ([]TunnelCandidate, error) {
 		if idx == -1 {
 			return nil
 		}
+		rel, err := filepath.Rel(brainDir, path)
+		if err != nil {
+			return err
+		}
 		out = append(out, TunnelCandidate{
-			TargetPath:  rel,
+			TargetPath:  filepath.ToSlash(rel),
 			MatchedTerm: title,
 			Exact:       isWholeWord(haystack, idx, len(needle)),
 		})
 		return nil
-	}, func(rel string, err error) {
-		// A wiki/ that is not a real directory yields no candidates, as
-		// filepath.WalkDir did; other walk errors still abort.
-		if rel == "wiki" && errors.Is(err, brainfs.ErrNotRegular) {
-			return
-		}
-		if walkErr == nil {
-			walkErr = fmt.Errorf("%s: %w", rel, err)
-		}
 	})
-	if err == nil {
-		err = walkErr
-	}
 	if err != nil {
 		return nil, err
 	}

ingestion/internal/brain/tunnel_redirect_test.go

@@ -0,0 +1,38 @@
+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"
+	"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
+)
+
+// hyperguild#173: brain_tunnel's write followed a directory symlink inside
+// wiki/ that points elsewhere in the brain.
+func TestWriteTunnelRefusesATargetBehindASymlinkedDirectory(t *testing.T) {
+	b := t.TempDir()
+	src := filepath.Join(b, "wiki", "a", "facts", "s.md")
+	require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
+	require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
+	victim := filepath.Join(b, "raw", "facts", "v.md")
+	require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
+	require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(b, "wiki", "b"), 0o755))
+	require.NoError(t, os.Symlink(filepath.Join("..", "..", "raw", "facts"), filepath.Join(b, "wiki", "b", "facts")))
+
+	assert.ErrorIs(t, brain.WriteTunnel(b, "wiki/a/facts/s.md", "wiki/b/facts/v.md"), brainfs.ErrNotRegular)
+	got, _ := os.ReadFile(victim)
+	assert.Equal(t, "untouched\n", string(got))
+	got, _ = os.ReadFile(src)
+	assert.Equal(t, "x\n", string(got), "the source is not rewritten either")
+
+	real := filepath.Join(b, "wiki", "c", "facts", "t.md")
+	require.NoError(t, os.MkdirAll(filepath.Dir(real), 0o755))
+	require.NoError(t, os.WriteFile(real, []byte("t\n"), 0o644))
+	require.NoError(t, brain.WriteTunnel(b, "wiki/a/facts/s.md", "wiki/c/facts/t.md"), "positive control: a tunnel between real directories is written")
+}

ingestion/internal/brainfs/brainfs.go

@@ -104,6 +104,17 @@ func (b *Brain) ReadDirRegular(relDir string) ([]string, error) {
 	return names, nil
 }
 
+// cleanRel refuses a path that is not clean and local. realDir checks the
+// cleaned parent, so the I/O must use that same path: os.Root resolves ".."
+// after following a symlink, and a ".." after a symlinked directory would
+// otherwise pass the check and redirect the read or write.
+func cleanRel(rel string) error {
+	if rel == "" || path.Clean(rel) != rel || !filepath.IsLocal(rel) || strings.Contains(rel, "\\") {
+		return fmt.Errorf("%q: not a clean brain-relative path: %w", rel, ErrNotRegular)
+	}
+	return nil
+}
+
 // ReadDirDirs lists the real subdirectories directly in a brain-relative
 // directory, by name. Every component of relDir must be a real directory
 // (ErrNotRegular otherwise); symlinks to directories are left out.
@@ -128,6 +139,12 @@ func (b *Brain) ReadDirDirs(relDir string) ([]string, error) {
 // 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) {
+	if err := cleanRel(rel); err != nil {
+		return nil, err
+	}
+	if err := b.realDir(path.Dir(rel)); err != nil {
+		return nil, err
+	}
 	fi, err := b.root.Lstat(rel)
 	if err != nil {
 		return nil, err
@@ -154,6 +171,12 @@ func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(
 // ReadRegularMax is ReadRegular with a size limit, checked on the opened
 // handle before reading (max < 0 means no limit).
 func (b *Brain) ReadRegularMax(rel string, max int64) ([]byte, error) {
+	if err := cleanRel(rel); err != nil {
+		return nil, err
+	}
+	if err := b.realDir(path.Dir(rel)); err != nil {
+		return nil, err
+	}
 	fi, err := b.root.Lstat(rel)
 	if err != nil {
 		return nil, err
@@ -256,10 +279,20 @@ func (b *Brain) WalkRegular(relDir string, fn func(rel string) error, onErr func
 // O_NOFOLLOW (verified on go1.27), so the path is Lstat-ed through the root
 // first and the opened handle must be that same file (os.SameFile), which
 // also catches a swap between the two. O_NONBLOCK keeps a FIFO from blocking
-// the open. Intermediate in-brain symlinks are still followed (os.Root keeps
-// the write inside the brain; hyperguild#167).
+// the open. Every directory above the file must be a real directory
+// (hyperguild#173); a directory swapped in after that check is the #167
+// residual.
 func (b *Brain) WriteRegular(rel string, data []byte) error {
-	rel = filepath.ToSlash(rel)
+	if err := cleanRel(rel); err != nil {
+		return err
+	}
+	// Every directory above the file must be real: os.Root follows an
+	// in-brain intermediate symlink, which would redirect the write
+	// elsewhere in the brain (hyperguild#173). A swap after this check is
+	// the #167 residual.
+	if err := b.realDir(path.Dir(rel)); err != nil {
+		return err
+	}
 	lfi, err := b.root.Lstat(rel)
 	if err != nil {
 		return err

ingestion/internal/brainfs/brainfs_test.go

@@ -326,3 +326,53 @@ func TestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponents(t *testing
 	_, err = b.ReadDirDirs("alias")
 	assert.ErrorIs(t, err, ErrNotRegular)
 }
+
+// hyperguild#173: a file reached through a symlinked directory inside the
+// brain is refused for reads and writes, so a caller-supplied path cannot be
+// redirected to another part of the brain.
+func TestReadAndWriteRegularRefuseAFileBehindASymlinkedDirectory(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("untouched"), 0o644))
+	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() }()
+
+	_, err = b.ReadRegular("wiki/alias/v.md")
+	assert.ErrorIs(t, err, ErrNotRegular)
+	_, err = b.ReadRegularMax("wiki/alias/v.md", 100)
+	assert.ErrorIs(t, err, ErrNotRegular)
+	assert.ErrorIs(t, b.WriteRegular("wiki/alias/v.md", []byte("x")), ErrNotRegular)
+	got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md"))
+	assert.Equal(t, "untouched", string(got))
+	got, err = b.ReadRegular("raw/v.md")
+	require.NoError(t, err, "positive control: the real path still reads")
+	assert.Equal(t, "untouched", string(got))
+}
+
+// realDir checks the cleaned parent, so the I/O must use that same clean
+// path: os.Root resolves ".." after following a symlink, and a ".." placed
+// after a symlinked directory would otherwise pass the check and redirect.
+func TestReadAndWriteRegularRefuseAPathThatIsNotClean(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "raw", "facts"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "raw", "v.md"), []byte("untouched"), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(dir, "wiki"), 0o755))
+	require.NoError(t, os.Symlink(filepath.Join("..", "raw", "facts"), filepath.Join(dir, "wiki", "alias")))
+	b, err := Open(dir)
+	require.NoError(t, err)
+	defer func() { _ = b.Close() }()
+
+	for _, rel := range []string{"wiki/alias/../v.md", "./raw/v.md", "raw//v.md", "/raw/v.md"} {
+		got, err := b.ReadRegular(rel)
+		assert.ErrorIs(t, err, ErrNotRegular, rel)
+		assert.NotContains(t, string(got), "untouched", rel)
+		_, err = b.ReadRegularMax(rel, 100)
+		assert.ErrorIs(t, err, ErrNotRegular, rel)
+		assert.ErrorIs(t, b.WriteRegular(rel, []byte("OVERWRITTEN")), ErrNotRegular, rel)
+	}
+	got, _ := os.ReadFile(filepath.Join(dir, "raw", "v.md"))
+	assert.Equal(t, "untouched", string(got))
+}

ingestion/internal/session/id_test.go

@@ -2,14 +2,17 @@ package session
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
+	"sync/atomic"
 	"syscall"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 )
 
 // session_log passed the caller's session_id straight into
@@ -90,3 +93,85 @@ func TestAppendReturnsPromptlyOnAFIFOLogWithNoReader(t *testing.T) {
 		t.Fatal("Append blocked on a FIFO log with no reader")
 	}
 }
+
+// hyperguild#173: session_log followed a symlinked sessions dir.
+func TestAppendRefusesASymlinkedSessionsDir(t *testing.T) {
+	root := t.TempDir()
+	elsewhere := filepath.Join(root, "elsewhere")
+	require.NoError(t, os.MkdirAll(elsewhere, 0o755))
+	dir := filepath.Join(root, "sessions")
+	require.NoError(t, os.Symlink("elsewhere", dir))
+	assert.Error(t, Append(dir, "s", Entry{SessionID: "s"}))
+	ents, _ := os.ReadDir(elsewhere)
+	assert.Empty(t, ents, "a log was created through the symlinked dir")
+}
+
+// The sessions dir is swapped (RENAME_EXCHANGE) with a symlink to a
+// directory outside the brain while Append runs. Append opens through an
+// os.Root on the sessions dir's parent, so a swap can redirect it only
+// inside that parent (the #167 residual), never outside it.
+func TestAppendNeverCreatesALogOutsideTheParentUnderADirSwapRace(t *testing.T) {
+	brain := t.TempDir()
+	outside := t.TempDir()
+	dir := filepath.Join(brain, "sessions")
+	other := filepath.Join(brain, "sessions.link")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	require.NoError(t, os.Symlink(outside, other))
+	var stop atomic.Bool
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		for !stop.Load() {
+			_ = unix.Renameat2(unix.AT_FDCWD, dir, unix.AT_FDCWD, other, unix.RENAME_EXCHANGE)
+		}
+	}()
+	ok := 0
+	for i := 0; i < 20000; i++ {
+		if Append(dir, fmt.Sprintf("s%d", i), Entry{SessionID: "s"}) == nil {
+			ok++
+		}
+	}
+	stop.Store(true)
+	<-done
+	ents, _ := os.ReadDir(outside)
+	assert.Empty(t, ents, "logs were created outside the brain")
+	assert.Positive(t, ok, "positive control: appends to the real dir succeed")
+}
+
+// The log file is swapped between a regular file and a symlink to another
+// file in the brain while Append runs: Append writes only after confirming
+// the path is the regular file it opened, so the other file never grows.
+func TestAppendNeverWritesThroughALogSwappedForASymlink(t *testing.T) {
+	brain := t.TempDir()
+	dir := filepath.Join(brain, "sessions")
+	require.NoError(t, os.MkdirAll(dir, 0o755))
+	victim := filepath.Join(brain, "victim.md")
+	require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
+	log := filepath.Join(dir, "s.jsonl")
+	var stop atomic.Bool
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		tmp := log + ".swap"
+		for i := 0; !stop.Load(); i++ {
+			_ = os.Remove(tmp)
+			if i%2 == 0 {
+				_ = os.WriteFile(tmp, nil, 0o644)
+			} else {
+				_ = os.Symlink(filepath.Join("..", "victim.md"), tmp)
+			}
+			_ = os.Rename(tmp, log)
+		}
+	}()
+	ok := 0
+	for i := 0; i < 20000; i++ {
+		if Append(dir, "s", Entry{SessionID: "s"}) == nil {
+			ok++
+		}
+	}
+	stop.Store(true)
+	<-done
+	got, _ := os.ReadFile(victim)
+	assert.Equal(t, "untouched\n", string(got), "a log line was written through the swapped-in symlink")
+	assert.Positive(t, ok, "positive control: appends to the regular log succeed")
+}

ingestion/internal/session/session.go

@@ -65,14 +65,42 @@ func Append(sessionsDir, sessionID string, entry Entry) error {
 	if err := checkID(sessionID); err != nil {
 		return err
 	}
-	if err := os.MkdirAll(sessionsDir, 0o755); err != nil {
+	// Opened through an os.Root on the sessions dir's parent (the brain dir),
+	// so a sessions dir swapped for a symlink after the check below can
+	// redirect the log only inside that parent, never outside it (#173; the
+	// in-brain swap is the #167 residual).
+	parentDir, name := filepath.Dir(sessionsDir), filepath.Base(sessionsDir)
+	if err := os.MkdirAll(parentDir, 0o755); err != nil {
+		return fmt.Errorf("create sessions parent: %w", err)
+	}
+	root, err := os.OpenRoot(parentDir)
+	if err != nil {
+		return fmt.Errorf("open sessions parent: %w", err)
+	}
+	defer func() { _ = root.Close() }()
+	if err := root.Mkdir(name, 0o755); err != nil && !errors.Is(err, fs.ErrExist) {
 		return fmt.Errorf("create sessions dir: %w", err)
 	}
-	path := filepath.Join(sessionsDir, sessionID+".jsonl")
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
+	// The sessions dir itself must be a real directory, not a symlink that
+	// redirects the log elsewhere (hyperguild#173).
+	if fi, err := root.Lstat(name); err != nil {
+		return fmt.Errorf("stat sessions dir: %w", err)
+	} else if !fi.IsDir() {
+		return fmt.Errorf("sessions dir %s is not a real directory", sessionsDir)
+	}
+	logRel := name + "/" + sessionID + ".jsonl"
+	f, err := root.OpenFile(logRel, os.O_CREATE|os.O_APPEND|os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
 	if err != nil {
 		return fmt.Errorf("open session log: %w", err)
 	}
+	// os.Root follows an in-root final symlink even with O_NOFOLLOW, so
+	// confirm, before writing, that the path is the regular file we opened.
+	hfi, herr := f.Stat()
+	lfi, lerr := root.Lstat(logRel)
+	if herr != nil || lerr != nil || !lfi.Mode().IsRegular() || !os.SameFile(lfi, hfi) {
+		_ = f.Close()
+		return fmt.Errorf("session log %s is not a regular file", logRel)
+	}
 
 	line, err := json.Marshal(entry)
 	if err != nil {