Compare commits

..
Author SHA1 Message Date
mathiasandClaude Opus 5.5 1cf05fdf15 test(search): in-brain Query race queries the inside note too
CI / Lint / Test / Vet (pull_request) Successful in 1m11s
CI / Mirror to GitHub (pull_request) Skipped
After main's #177 floor (AssertNoLeakFrom needs at least one read that
returns the inside note), this test went red on merge: the reader
queried only the sentinel, so the regular note could never be a hit
and the race had been passing vacuously. Query both terms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 16:20:41 +02:00
mathias 51183d64d8 Merge remote-tracking branch 'origin/main' into merge-check 2026-10-05 16:17:54 +02:00
mathiasandClaude Opus 5.5 f969d97c16 test(notes): in-brain symlink target for ReadScoped, the sixth slice 1-3 caller
CI / Lint / Test / Vet (pull_request) Successful in 54s
CI / Mirror to GitHub (pull_request) Skipped
Review of #172 (finding 1): notes.ReadScoped (brain_read, slice 2)
had only outside-brain targets, which os.Root refuses on its own, so a
reader bypassing brainfs's Lstat/SameFile stayed green. Adds a static
in-brain link, a positive control and an in-brain swap race.
Flip: replacing ReadRegularMax with os.OpenRoot().ReadFile turns it
red 3/3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 07:20:59 +02:00
mathiasandClaude Opus 5.5 a83f0cb381 test(search): name the in-brain swap-race tests after what they sample
CI / Lint / Test / Vet (pull_request) Successful in 56s
CI / Mirror to GitHub (pull_request) Skipped
Review of #172 (rule 9): "LeaksNothing" claims more than a 1.5s
sampled race can prove -- the same overclaim this PR removes from the
"Never" names. Renamed to DoesNotReturn...DuringASampledSwapRace,
and the brainfstest doc now says the target may be inside the brain
(#171) and that observation is over a sampled window.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 07:19:27 +02:00
mathiasandClaude Opus 5.5 bf3646c0c7 test(brainfs): per-caller in-brain symlink targets for slices 1-3 (#171)
CI / Lint / Test / Vet (pull_request) Successful in 53s
CI / Mirror to GitHub (pull_request) Skipped
os.Root refuses links that leave the brain on its own, so the slice 1-3
callers' outside-target tests stayed green when a caller read without
brainfs's Lstat/SameFile checks (PR #168 round-2 review). Adds a static
in-brain link (wiki/w/facts/inlink.md -> ../../../raw/s.md) and an
AssertNoLeakFrom swap race with an in-brain target for search.Query,
hydrate, vectorstore.Sync, readExcerpt and api.ReadNote.

Flip-verified: replacing each caller's bfs.ReadRegular with an
os.OpenRoot(brainDir).ReadFile turns every one red on the new assertions
(Query, hydrate, Sync, readExcerpt, ReadNote).

Also from that review: rename TestReadRegularNever... to
...DoesNotReturn... (rule 9: a sampled race), and AssertNoLeakFrom's
failure message names the symlink target instead of 'outside file'.

Closes #171

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 07:02:41 +02:00
14 changed files with 135 additions and 532 deletions
@@ -23,6 +23,13 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
_, body, _, err := api.ReadNote(brain, "wiki/w/facts/leak.md")
assert.Error(t, err)
assert.NotContains(t, body, brainfstest.Sentinel)
// A link to a file elsewhere in the brain: os.Root allows it, only the
// regular-file check refuses it (hyperguild#171).
brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
_, body, _, err = api.ReadNote(brain, "wiki/w/facts/inlink.md")
assert.Error(t, err)
assert.NotContains(t, body, brainfstest.Sentinel)
p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
@@ -39,5 +46,7 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
return s
}
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, read)
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
brainfstest.AssertNoLeakFrom(t, p, "---\nx: y\n---\ninside", inBrain, 1500*time.Millisecond, read)
brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
}
+5 -17
View File
@@ -3,18 +3,15 @@ package api
import (
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"io/fs"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
"os"
"path/filepath"
"strings"
"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/notes"
)
// ContentHash returns the lowercase hex sha256 of b. It is the note's
@@ -94,23 +91,14 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
rel = filepath.ToSlash(rel)
}
// The path is caller-supplied (brain_update): resolve it as brain_edit
// does, knowledge/ and wiki/ notes only, and read and rewrite it through
// the brain root, never through a symlink (hyperguild#169).
t, err := notes.Resolve(brainDir, rel, notes.ScopeNotes)
rel, abs, err := resolveWithin(brainDir, rel)
if err != nil {
return "", "", "", err
}
rel = t.Rel
bfs, err := brainfs.Open(brainDir)
if err != nil {
return "", "", "", fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
prior, err := bfs.ReadRegular(rel)
prior, err := os.ReadFile(abs)
if err != nil {
if errors.Is(err, fs.ErrNotExist) {
if os.IsNotExist(err) {
return "", "", "", fmt.Errorf("note %q does not exist: use brain_write to create", rel)
}
return "", "", "", fmt.Errorf("read target: %w", err)
@@ -176,7 +164,7 @@ func UpdateNote(brainDir string, opts UpdateNoteOptions) (relPath, contentHash,
return "", "", "", err
}
out := []byte(stamped)
if err := bfs.WriteRegular(rel, out); err != nil {
if err := os.WriteFile(abs, out, 0o644); err != nil {
return "", "", "", fmt.Errorf("write: %w", err)
}
return rel, ContentHash(out), priorHash, nil
-112
View File
@@ -1,112 +0,0 @@
package api_test
import (
"os"
"path/filepath"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
)
// brain_update passed the caller's path through resolveWithin, a lexical
// containment check only: any file in the brain could be superseded
// (classification.yaml, raw/, sessions/), and a final symlink was written
// through. It now resolves like brain_edit: knowledge/ and wiki/ notes only,
// read and rewritten through the brain root.
func TestUpdateNoteRefusesPathsOutsideKnowledgeAndWikiNotes(t *testing.T) {
brain := t.TempDir()
files := map[string]string{
"classification.yaml": "levels: {}\n",
"raw/r.md": "raw\n",
"sessions/s.jsonl": "{}\n",
"wiki/w/facts/ok.md": "---\ntitle: ok\n---\nold\n",
"knowledge/k.md": "---\ntitle: k\n---\nold\n",
"wiki/w/facts/x.txt": "txt\n",
}
for rel, body := range files {
p := filepath.Join(brain, filepath.FromSlash(rel))
require.NoError(t, os.MkdirAll(filepath.Dir(p), 0o755))
require.NoError(t, os.WriteFile(p, []byte(body), 0o644))
}
for _, rel := range []string{"classification.yaml", "raw/r.md", "sessions/s.jsonl", "wiki/w/facts/x.txt", "wiki/w/../../raw/r.md"} {
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: rel, Content: "replaced"})
assert.Error(t, err, rel)
}
for rel, body := range files {
if rel == "wiki/w/facts/ok.md" || rel == "knowledge/k.md" {
continue
}
got, err := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
require.NoError(t, err)
assert.Equal(t, body, string(got), rel)
}
for _, rel := range []string{"wiki/w/facts/ok.md", "knowledge/k.md"} {
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: rel, Content: "new body"})
require.NoError(t, err, "positive control: %s", rel)
got, _ := os.ReadFile(filepath.Join(brain, filepath.FromSlash(rel)))
assert.Contains(t, string(got), "new body")
}
}
func TestUpdateNoteDoesNotWriteThroughASymlinkedNote(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
victim := filepath.Join(brain, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), filepath.Join(facts, "l.md")))
_, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/l.md", Content: "replaced"})
assert.Error(t, err)
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
}
// The note is swapped between a regular file and an in-brain symlink to a
// raw/ file while UpdateNote runs: the read and the write each refuse a
// symlink, so the raw/ file is never rewritten.
func TestUpdateNoteNeverRewritesASymlinkSwappedInForTheNote(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
victim := filepath.Join(brain, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
note := filepath.Join(facts, "n.md")
body := "---\ntitle: n\n---\nold\n"
require.NoError(t, os.WriteFile(note, []byte(body), 0o644))
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := note + ".swap"
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
if i%2 == 0 {
_ = os.WriteFile(tmp, []byte(body), 0o644)
} else {
_ = os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), tmp)
}
_ = os.Rename(tmp, note)
}
}()
ok := 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
if _, _, _, err := api.UpdateNote(brain, api.UpdateNoteOptions{Path: "wiki/w/facts/n.md", Content: "new"}); err == nil {
ok++
}
}
stop.Store(true)
wg.Wait()
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
assert.Positive(t, ok, "positive control: some updates reach the regular note")
}
+20 -54
View File
@@ -4,12 +4,9 @@ import (
"bufio"
"fmt"
"os"
"path"
"path/filepath"
"strings"
"time"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs"
)
// seeAlsoHeader is the markdown heading used to group cross-wing links.
@@ -200,12 +197,6 @@ func fileExists(p string) bool {
// section headers. Rejects same-wing pairs (a tunnel is by definition
// cross-wing) and missing notes.
func WriteTunnel(brainDir, sourcePath, targetPath string) error {
if err := checkNotePath(sourcePath); err != nil {
return fmt.Errorf("source: %w", err)
}
if err := checkNotePath(targetPath); err != nil {
return fmt.Errorf("target: %w", err)
}
srcWing, err := wingOf(sourcePath)
if err != nil {
return fmt.Errorf("source: %w", err)
@@ -218,50 +209,20 @@ func WriteTunnel(brainDir, sourcePath, targetPath string) error {
return fmt.Errorf("tunnel must cross wings; got both in %q", srcWing)
}
// Both notes are read and rewritten through the brain root, regular
// files only: brain_tunnel passes caller-supplied paths, and the brain is
// a Syncthing folder that can hold symlinks.
bfs, err := brainfs.Open(brainDir)
if err != nil {
return fmt.Errorf("open brain: %w", err)
}
defer func() { _ = bfs.Close() }()
srcContent, err := bfs.ReadRegular(sourcePath)
if err != nil {
srcFull := filepath.Join(brainDir, filepath.FromSlash(sourcePath))
tgtFull := filepath.Join(brainDir, filepath.FromSlash(targetPath))
if _, err := os.Stat(srcFull); err != nil {
return fmt.Errorf("source note: %w", err)
}
tgtContent, err := bfs.ReadRegular(targetPath)
if err != nil {
if _, err := os.Stat(tgtFull); err != nil {
return fmt.Errorf("target note: %w", err)
}
if out, changed, err := withSeeAlso(srcContent, wikilinkOf(targetPath)); err != nil {
if err := appendSeeAlso(srcFull, wikilinkOf(targetPath)); err != nil {
return fmt.Errorf("update source: %w", err)
} else if changed {
if err := bfs.WriteRegular(sourcePath, out); err != nil {
return fmt.Errorf("update source: %w", err)
}
}
if out, changed, err := withSeeAlso(tgtContent, wikilinkOf(sourcePath)); err != nil {
if err := appendSeeAlso(tgtFull, wikilinkOf(sourcePath)); err != nil {
return fmt.Errorf("update target: %w", err)
} else if changed {
if err := bfs.WriteRegular(targetPath, out); err != nil {
return fmt.Errorf("update target: %w", err)
}
}
return nil
}
// checkNotePath accepts only a clean, local wiki/<wing>/<hall>/.../<slug>.md
// path: no "." or ".." segments, no empty segments, no leading slash.
func checkNotePath(p string) error {
if p == "" || path.Clean(p) != p || !filepath.IsLocal(p) || !strings.HasSuffix(p, ".md") {
return fmt.Errorf("not a clean wiki note path: %q", p)
}
for _, part := range strings.Split(p, "/") {
if part == "" || part == "." || part == ".." {
return fmt.Errorf("not a clean wiki note path: %q", p)
}
}
return nil
}
@@ -287,13 +248,16 @@ func wingOf(relPath string) (string, error) {
return parts[1], nil
}
// withSeeAlso returns content with `- [[link]]` under its See also section,
// creating the section if absent. changed is false when the link is already
// present.
func withSeeAlso(content []byte, link string) (out []byte, changed bool, err error) {
// appendSeeAlso inserts `- [[link]]` under the file's See also section,
// creating the section if absent. No-op when the link is already present.
func appendSeeAlso(filePath, link string) error {
content, err := os.ReadFile(filePath)
if err != nil {
return err
}
wikilink := "[[" + link + "]]"
if strings.Contains(string(content), wikilink) {
return content, false, nil
return nil
}
bullet := "- " + wikilink
@@ -302,7 +266,8 @@ func withSeeAlso(content []byte, link string) (out []byte, changed bool, err err
// No section yet — append a fresh one. Always emit a trailing
// newline so subsequent appends don't merge into the previous line.
trimmed := strings.TrimRight(string(content), "\n")
return []byte(trimmed + "\n\n" + seeAlsoHeader + "\n\n" + bullet + "\n"), true, nil
out := trimmed + "\n\n" + seeAlsoHeader + "\n\n" + bullet + "\n"
return os.WriteFile(filePath, []byte(out), 0o644)
}
// Section exists — splice the bullet in just before the next `## `
@@ -328,11 +293,12 @@ func withSeeAlso(content []byte, link string) (out []byte, changed bool, err err
b.WriteByte('\n')
}
if err := scanner.Err(); err != nil {
return nil, false, err
return err
}
if !inserted {
// section was the last thing in the file — just append bullet
return []byte(strings.TrimRight(b.String(), "\n") + "\n" + bullet + "\n"), true, nil
out := strings.TrimRight(b.String(), "\n") + "\n" + bullet + "\n"
return os.WriteFile(filePath, []byte(out), 0o644)
}
return []byte(b.String()), true, nil
return os.WriteFile(filePath, []byte(b.String()), 0o644)
}
@@ -1,141 +0,0 @@
package brain_test
import (
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brain"
)
// brain_tunnel passes caller-supplied source and target paths to WriteTunnel,
// which checked only "wiki/<wing>/..." and then joined them onto the brain
// dir: ".." segments resolved outside the brain and appendSeeAlso rewrote the
// file there.
func TestWriteTunnelRefusesATargetThatTraversesOutOfTheBrain(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("---\ntitle: S\n---\nbody\n"), 0o644))
victim := filepath.Join(t.TempDir(), "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
up := strings.Repeat("../", strings.Count(filepath.Clean(brain0), "/")+3)
target := "wiki/b/x/" + up + strings.TrimPrefix(victim, "/")
err := brain.WriteTunnel(brain0, "wiki/a/facts/s.md", target)
assert.Error(t, err)
got, rerr := os.ReadFile(victim)
require.NoError(t, rerr)
assert.Equal(t, "untouched\n", string(got), "a file outside the brain was modified")
}
func TestWriteTunnelRefusesUncleanOrNonWikiPaths(t *testing.T) {
brain0 := t.TempDir()
for _, p := range []string{"wiki/a/facts/s.md", "wiki/b/facts/t.md", "wiki/b/facts/t.txt", "raw/r.md"} {
require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(brain0, p)), 0o755))
require.NoError(t, os.WriteFile(filepath.Join(brain0, p), []byte("x\n"), 0o644))
}
for _, target := range []string{
"wiki/b/facts/../../../raw/r.md",
"wiki/b/./facts/t.md",
"wiki/b//facts/t.md",
"/wiki/b/facts/t.md",
"wiki/b/facts/t.txt",
"wiki/b/facts/",
} {
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", target), target)
}
for _, p := range []string{"raw/r.md", "wiki/b/facts/t.txt"} {
got, err := os.ReadFile(filepath.Join(brain0, p))
require.NoError(t, err)
assert.Equal(t, "x\n", string(got), p)
}
require.NoError(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md"), "positive control")
got, err := os.ReadFile(filepath.Join(brain0, "wiki", "b", "facts", "t.md"))
require.NoError(t, err)
assert.Contains(t, string(got), "[[a/facts/s]]")
}
func TestWriteTunnelDoesNotWriteThroughASymlinkedNote(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
tdir := filepath.Join(brain0, "wiki", "b", "facts")
require.NoError(t, os.MkdirAll(tdir, 0o755))
victim := filepath.Join(t.TempDir(), "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(victim, filepath.Join(tdir, "t.md")))
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md"))
got, err := os.ReadFile(victim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got))
// The same with a link to a file elsewhere in the brain, which os.Root
// would follow.
inVictim := filepath.Join(brain0, "raw", "v.md")
require.NoError(t, os.MkdirAll(filepath.Dir(inVictim), 0o755))
require.NoError(t, os.WriteFile(inVictim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "v.md"), filepath.Join(tdir, "u.md")))
assert.Error(t, brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/u.md"))
got, err = os.ReadFile(inVictim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got))
got, err = os.ReadFile(src)
require.NoError(t, err)
assert.Equal(t, "x\n", string(got), "the source is not touched when the target is refused")
}
// The target is swapped between a regular note and a symlink to an in-brain
// file outside wiki/ while WriteTunnel runs: the read and the write each
// refuse a symlink, so the other file is never rewritten.
func TestWriteTunnelNeverRewritesASymlinkSwappedInForTheTarget(t *testing.T) {
brain0 := t.TempDir()
src := filepath.Join(brain0, "wiki", "a", "facts", "s.md")
require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755))
require.NoError(t, os.WriteFile(src, []byte("x\n"), 0o644))
tdir := filepath.Join(brain0, "wiki", "b", "facts")
require.NoError(t, os.MkdirAll(tdir, 0o755))
victim := filepath.Join(brain0, "raw", "victim.md")
require.NoError(t, os.MkdirAll(filepath.Dir(victim), 0o755))
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
tgt := filepath.Join(tdir, "t.md")
require.NoError(t, os.WriteFile(tgt, []byte("t\n"), 0o644))
var stop atomic.Bool
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
tmp := tgt + ".swap"
for i := 0; !stop.Load(); i++ {
_ = os.Remove(tmp)
if i%2 == 0 {
_ = os.WriteFile(tmp, []byte("t\n"), 0o644)
} else {
_ = os.Symlink(filepath.Join("..", "..", "..", "raw", "victim.md"), tmp)
}
_ = os.Rename(tmp, tgt)
}
}()
ok := 0
for deadline := time.Now().Add(1500 * time.Millisecond); time.Now().Before(deadline); {
if brain.WriteTunnel(brain0, "wiki/a/facts/s.md", "wiki/b/facts/t.md") == nil {
ok++
}
_ = os.WriteFile(src, []byte("x\n"), 0o644)
}
stop.Store(true)
wg.Wait()
got, err := os.ReadFile(victim)
require.NoError(t, err)
assert.Equal(t, "untouched\n", string(got), "the swapped-in symlink's target was rewritten")
assert.Positive(t, ok, "positive control: some tunnels were written to the regular target")
}
-38
View File
@@ -249,41 +249,3 @@ func (b *Brain) WalkRegular(relDir string, fn func(rel string) error, onErr func
return fn(p)
})
}
// WriteRegular replaces the contents of an existing regular file at a
// brain-relative path. It never creates a file and never writes through a
// final symlink: os.Root.OpenFile follows an in-root final symlink even with
// 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).
func (b *Brain) WriteRegular(rel string, data []byte) error {
rel = filepath.ToSlash(rel)
lfi, err := b.root.Lstat(rel)
if err != nil {
return err
}
if !lfi.Mode().IsRegular() {
return fmt.Errorf("%s: %w", rel, ErrNotRegular)
}
f, err := b.root.OpenFile(rel, os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0)
if err != nil {
return err
}
defer func() { _ = f.Close() }()
fi, err := f.Stat()
if err != nil {
return err
}
if !fi.Mode().IsRegular() || !os.SameFile(lfi, fi) {
return fmt.Errorf("%s: changed while opening: %w", rel, ErrNotRegular)
}
if err := f.Truncate(0); err != nil {
return err
}
if _, err := f.Write(data); err != nil {
return err
}
return f.Close()
}
+1 -47
View File
@@ -218,7 +218,7 @@ func TestReadDirRegularListsOnlyRegularFilesAndRefusesSymlinkedComponents(t *tes
// A final component swapped for a symlink to another regular file in the
// brain between the Lstat and the open: os.Root follows in-root links, so
// only comparing the two (os.SameFile) refuses it.
func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.T) {
func TestReadRegularDoesNotReturnAnInBrainFileSwappedInBehindASymlink(t *testing.T) {
dir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(dir, "s.md"), []byte("zqsamefilesentinel"), 0o644))
p := filepath.Join(dir, "r.md")
@@ -263,52 +263,6 @@ func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.
assert.Positive(t, ok, "positive control: the regular file is read")
}
func TestWriteRegularRefusesFinalSymlinksFIFOsEscapesAndMissingFilesAndRewritesRegularFiles(t *testing.T) {
dir := t.TempDir()
outside := filepath.Join(t.TempDir(), "o.md")
require.NoError(t, os.WriteFile(outside, []byte("untouched"), 0o644))
require.NoError(t, os.Symlink(outside, filepath.Join(dir, "l.md")))
require.NoError(t, syscall.Mkfifo(filepath.Join(dir, "f.md"), 0o644))
require.NoError(t, os.WriteFile(filepath.Join(dir, "n.md"), []byte("old content"), 0o644))
b, err := Open(dir)
require.NoError(t, err)
defer func() { _ = b.Close() }()
assert.Error(t, b.WriteRegular("l.md", []byte("x")))
got, _ := os.ReadFile(outside)
assert.Equal(t, "untouched", string(got))
// A symlink to another file inside the brain: os.Root would allow it,
// only O_NOFOLLOW refuses it.
require.NoError(t, os.WriteFile(filepath.Join(dir, "in.md"), []byte("inside"), 0o644))
require.NoError(t, os.Symlink("in.md", filepath.Join(dir, "il.md")))
assert.Error(t, b.WriteRegular("il.md", []byte("x")))
got, _ = os.ReadFile(filepath.Join(dir, "in.md"))
assert.Equal(t, "inside", string(got))
// A reader held open, so a non-blocking write open would succeed. The
// Lstat type check refuses the FIFO first; the handle check and
// O_NONBLOCK matter only if it is swapped in after the Lstat.
r, err := os.OpenFile(filepath.Join(dir, "f.md"), os.O_RDONLY|syscall.O_NONBLOCK, 0)
require.NoError(t, err)
defer func() { _ = r.Close() }()
done := make(chan error, 1)
go func() { done <- b.WriteRegular("f.md", []byte("x")) }()
select {
case err := <-done:
assert.Error(t, err)
case <-time.After(3 * time.Second):
t.Fatal("WriteRegular hung on a FIFO")
}
assert.Error(t, b.WriteRegular("../"+filepath.Base(filepath.Dir(outside))+"/o.md", []byte("x")))
assert.Error(t, b.WriteRegular("missing.md", []byte("x")), "does not create files")
require.NoError(t, b.WriteRegular("n.md", []byte("new")))
got, _ = os.ReadFile(filepath.Join(dir, "n.md"))
assert.Equal(t, "new", string(got), "truncated and rewritten")
}
func TestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponents(t *testing.T) {
dir := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(dir, "a", "real"), 0o755))
@@ -1,8 +1,10 @@
// Package brainfstest is the shared harness for hyperguild#157's swap-race
// tests: it keeps swapping a regular note, a symlink to an outside file, or a
// FIFO (with a writer held open) into a path while a reader under test loops.
// It observes two things: no sentinel byte from the outside file reaches the
// reader's output, and the reader returns within a timeout. It does not cover
// tests: it keeps swapping a regular note, a symlink to a sentinel target file
// (outside the brain, or inside it for hyperguild#171), or a FIFO (with a
// writer held open) into a path while a reader under test loops.
// It observes two things, over a sampled window only: no sentinel byte from
// the target reaches the reader's output, and the reader returns within a
// timeout. It does not cover
// hard links, other paths into the reader, or races it does not generate.
package brainfstest
@@ -18,7 +20,7 @@ import (
"time"
)
// Sentinel marks the outside file's contents.
// Sentinel marks the target file's contents.
const Sentinel = "zqbrainfssentinelonefiftyseven"
// outsideContent carries Sentinel in every field a reader might extract: the
@@ -141,7 +143,7 @@ func AssertNoLeakFrom(t testing.TB, path, inside, target string, d time.Duration
}
}
if leaks > 0 {
t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters)
t.Fatalf("symlink target %s leaked %d times in %d iterations", target, leaks, iters)
}
if n := swaps.Load(); n == 0 {
t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters)
@@ -21,6 +21,11 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
require.NoError(t, os.MkdirAll(facts, 0o755))
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md")))
assert.NotContains(t, readExcerpt(brain, "wiki/w/facts/leak.md", 500), brainfstest.Sentinel)
// A link to a file elsewhere in the brain: os.Root allows it, only the
// regular-file check refuses it (hyperguild#171).
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
assert.NotContains(t, readExcerpt(brain, "wiki/w/facts/inlink.md", 500), brainfstest.Sentinel)
p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
@@ -28,6 +33,9 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, func() string {
return readExcerpt(brain, "wiki/w/facts/r.md", 500)
})
brainfstest.AssertNoLeakFrom(t, p, "---\nx: y\n---\ninside", inBrain, 1500*time.Millisecond, func() string {
return readExcerpt(brain, "wiki/w/facts/r.md", 500)
})
brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() {
_ = readExcerpt(brain, "wiki/w/facts/r.md", 500)
})
@@ -38,3 +38,31 @@ func TestReadScopedRefusesANoteOverMaxNoteBytes(t *testing.T) {
_, err := ReadScoped(brain, "wiki/w/facts/big.md", ScopeNotes)
assert.ErrorIs(t, err, ErrTooLarge)
}
// hyperguild#171 (review of #172, finding 1): os.Root alone refuses links that
// leave the brain, so the outside-target race above stays green for a reader
// that skips brainfs's Lstat and SameFile checks. An in-brain target (raw/,
// outside ScopeNotes) is what tells them apart.
func TestReadScopedDoesNotReturnAnInBrainSymlinkTargetStaticallyOrDuringASampledSwapRace(t *testing.T) {
brain := t.TempDir()
facts := filepath.Join(brain, "wiki", "w", "facts")
require.NoError(t, os.MkdirAll(facts, 0o755))
target := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
b, err := ReadScoped(brain, "wiki/w/facts/inlink.md", ScopeNotes)
assert.Error(t, err, "static in-brain link")
assert.NotContains(t, string(b), brainfstest.Sentinel, "static in-brain link")
p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("inside"), 0o644))
b, err = ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes)
require.NoError(t, err)
assert.Equal(t, "inside", string(b), "positive control: a regular note reads")
read := func() string {
b, _ := ReadScoped(brain, "wiki/w/facts/r.md", ScopeNotes)
return string(b)
}
brainfstest.AssertNoLeakFrom(t, p, "inside", target, 1500*time.Millisecond, read)
}
@@ -12,6 +12,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mathiasbq/hyperguild/ingestion/internal/brainfs/brainfstest"
)
// hyperguild#157 option 1: search walked and read the brain by path, so a
@@ -189,3 +191,49 @@ func TestHydrateRefusesASymlink(t *testing.T) {
assert.Error(t, err)
assert.NotContains(t, r.Excerpt, sentinel157)
}
// hyperguild#171: os.Root refuses links that leave the brain on its own, so
// the outside-target cases above would stay green for a caller that read
// without brainfs's Lstat and SameFile checks. These targets are inside the
// brain (raw/, which is not a search root).
func inBrainTarget(t *testing.T, brain string) string {
t.Helper()
return brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
}
func TestQueryDoesNotReturnAnInBrainSymlinkTargetDuringASampledSwapRace(t *testing.T) {
brain, facts, _ := searchBrain(t)
target := inBrainTarget(t, brain)
read := func() string {
// Both terms, so the regular note is a possible hit: a reader that
// never returns the inside note would pass vacuously (#177 floor).
rs, _ := Query(brain, QueryOptions{Query: brainfstest.Sentinel + " zqinside", Limit: 10})
s := ""
for _, r := range rs {
s += r.Title + r.Excerpt
}
return s
}
brainfstest.AssertNoLeakFrom(t, filepath.Join(facts, "r.md"), "# r\n\nzqinside", target, 1500*time.Millisecond, read)
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
assert.NotContains(t, read(), brainfstest.Sentinel, "static in-brain link")
}
func TestHydrateRefusesAnInBrainSymlinkAndDoesNotReturnItsTargetDuringASampledSwapRace(t *testing.T) {
brain, facts, _ := searchBrain(t)
target := inBrainTarget(t, brain)
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(facts, "inlink.md")))
r, err := hydrate(brain, "wiki/w/facts/inlink.md")
assert.Error(t, err)
assert.NotContains(t, r.Excerpt+r.Title, brainfstest.Sentinel)
p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("# r\n\nzqinside"), 0o644))
r, err = hydrate(brain, "wiki/w/facts/r.md")
require.NoError(t, err)
assert.Contains(t, r.Excerpt, "zqinside", "positive control: a regular note hydrates")
brainfstest.AssertNoLeakFrom(t, p, "# r\n\nzqinside", target, 1500*time.Millisecond, func() string {
r, _ := hydrate(brain, "wiki/w/facts/r.md")
return r.Title + r.Excerpt
})
}
-92
View File
@@ -1,92 +0,0 @@
package session
import (
"errors"
"os"
"path/filepath"
"syscall"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// session_log passed the caller's session_id straight into
// filepath.Join(sessionsDir, id+".jsonl") and opened it O_CREATE|O_APPEND:
// path segments in the id created or appended files anywhere the pod could
// write. The id is now a plain name, and the open does not follow a final
// symlink.
func TestAppendRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
for _, id := range []string{"../x", "a/b", "/abs", ".", "..", ".hidden", "", "a\\b", "a\x00b"} {
assert.Error(t, Append(dir, id, Entry{SessionID: id}), "%q", id)
}
ents, _ := os.ReadDir(root)
for _, e := range ents {
assert.Equal(t, "sessions", e.Name(), "nothing created beside the sessions dir")
}
for _, id := range []string{"1557b2e1-bf0d-5c4f-bbe3-0754d087e356", "_routing", "2026-05-05-infra.plan", "0"} {
require.NoError(t, Append(dir, id, Entry{SessionID: id}), "positive control: %q", id)
}
}
func TestAppendDoesNotWriteThroughASymlinkedLog(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
victim := filepath.Join(root, "victim.md")
require.NoError(t, os.WriteFile(victim, []byte("untouched\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("..", "victim.md"), filepath.Join(dir, "s.jsonl")))
assert.Error(t, Append(dir, "s", Entry{SessionID: "s"}))
got, _ := os.ReadFile(victim)
assert.Equal(t, "untouched\n", string(got))
}
func TestReadRefusesSessionIDsThatAreNotPlainNames(t *testing.T) {
root := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(root, "x.jsonl"), []byte(`{"session_id":"x"}`+"\n"), 0o644))
_, err := Read(filepath.Join(root, "sessions"), "../x")
assert.Error(t, err)
}
// A name that starts plainly and then climbs out with ".." is exactly what the
// ban on "/" in the id's tail stops: filepath.Join cleans "a/../../x" to a
// path beside the sessions dir without needing sessions/a to exist. The
// assertion is on the refusal itself (ErrInvalidID), not on any error.
func TestAppendRefusesAnIDThatClimbsOutAfterAPlainPrefix(t *testing.T) {
root := t.TempDir()
dir := filepath.Join(root, "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
for _, id := range []string{"a/../../escape", "a/../../../escape", "a/../b"} {
err := Append(dir, id, Entry{SessionID: id})
assert.ErrorIs(t, err, ErrInvalidID, "%q", id)
}
_, err := os.Lstat(filepath.Join(root, "escape.jsonl"))
assert.True(t, errors.Is(err, os.ErrNotExist), "nothing created beside the sessions dir")
_, err = os.Lstat(filepath.Join(dir, "b.jsonl"))
assert.True(t, errors.Is(err, os.ErrNotExist), "no in-dir alias created either")
}
// Append does no Lstat type check, so O_NONBLOCK is the only thing that keeps
// a FIFO named <id>.jsonl with no reader from blocking session_log forever.
func TestAppendReturnsPromptlyOnAFIFOLogWithNoReader(t *testing.T) {
dir := filepath.Join(t.TempDir(), "sessions")
require.NoError(t, os.MkdirAll(dir, 0o755))
fifo := filepath.Join(dir, "s.jsonl")
require.NoError(t, syscall.Mkfifo(fifo, 0o644))
done := make(chan error, 1)
go func() { done <- Append(dir, "s", Entry{SessionID: "s"}) }()
select {
case err := <-done:
assert.Error(t, err, "a FIFO with no reader is refused, not written")
case <-time.After(3 * time.Second):
// Unblock the stuck open so the goroutine and TempDir cleanup finish.
if r, err := os.OpenFile(fifo, os.O_RDONLY|syscall.O_NONBLOCK, 0); err == nil {
defer r.Close() //nolint:errcheck
}
<-done
t.Fatal("Append blocked on a FIFO log with no reader")
}
}
+1 -25
View File
@@ -9,8 +9,6 @@ import (
"io/fs"
"os"
"path/filepath"
"regexp"
"syscall"
"time"
)
@@ -44,32 +42,13 @@ type Attempt struct {
RunnerOutput string `json:"runner_output,omitempty"`
}
// idPattern is a plain file name: session_log takes the id from the caller,
// and a path segment in it created or appended files anywhere the pod could
// write (hyperguild#169).
var idPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$`)
// ErrInvalidID is returned when a session id is not a plain file name.
var ErrInvalidID = errors.New("session id is not a plain name")
func checkID(sessionID string) error {
if !idPattern.MatchString(sessionID) {
return fmt.Errorf("%w: %q", ErrInvalidID, sessionID)
}
return nil
}
// Append writes entry as a single JSON line to sessionsDir/{sessionID}.jsonl.
// The open does not follow a final symlink.
func Append(sessionsDir, sessionID string, entry Entry) error {
if err := checkID(sessionID); err != nil {
return err
}
if err := os.MkdirAll(sessionsDir, 0o755); err != nil {
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)
f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644)
if err != nil {
return fmt.Errorf("open session log: %w", err)
}
@@ -91,9 +70,6 @@ func Append(sessionsDir, sessionID string, entry Entry) error {
// Read returns all entries for sessionID. Returns empty slice if no log exists.
func Read(sessionsDir, sessionID string) ([]Entry, error) {
if err := checkID(sessionID); err != nil {
return nil, err
}
path := filepath.Join(sessionsDir, sessionID+".jsonl")
f, err := os.Open(path)
if errors.Is(err, fs.ErrNotExist) {
@@ -42,6 +42,11 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir()
writeNote(t, brain, "wiki/w/facts/ok.md", "inside note")
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(brain, "wiki", "w", "facts", "leak.md")))
// A link to a file elsewhere in the brain: os.Root allows it, only the
// regular-file checks refuse it (hyperguild#171). raw/ is not a scan root,
// so its sentinel can only arrive through the link.
inBrain := brainfstest.WriteSentinelAt(t, filepath.Join(brain, "raw", "s.md"))
require.NoError(t, os.Symlink(filepath.Join("..", "..", "..", "raw", "s.md"), filepath.Join(brain, "wiki", "w", "facts", "inlink.md")))
emb := &recordingEmbedder{}
store := &stubStore{}
@@ -51,6 +56,7 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
assert.NotContains(t, emb.all(), brainfstest.Sentinel)
for p := range store.upserts {
assert.NotContains(t, p, "leak.md")
assert.NotContains(t, p, "inlink.md")
}
p := filepath.Join(brain, "knowledge", "r.md")
@@ -61,6 +67,7 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
return e.all()
}
brainfstest.AssertNoLeak(t, p, "inside", 1500*time.Millisecond, sync)
brainfstest.AssertNoLeakFrom(t, p, "inside", inBrain, 1500*time.Millisecond, sync)
brainfstest.AssertNoHang(t, p, "inside", 1500*time.Millisecond, 6*time.Second, func() { _ = sync() })
}