Compare commits

..
Author SHA1 Message Date
mathiasandClaude Opus 5.5 b8ea201bd3 test(pipeline): the page-swap test's positive control asserts the source ref (#157)
CI / Lint / Test / Vet (pull_request) Successful in 1m0s
CI / Mirror to GitHub (pull_request) Skipped
Review round 2 on PR #176: the control checked for text that was in the
page before BackfillRefs ran, so the test stayed green when no page was
read at all. It now asserts the [[s|S]] ref BackfillRefs adds; red when
the page read always fails, still red when the read follows the link.
Branch rebased onto main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 20:48:25 +02:00
mathiasandClaude Opus 5.5 b71fc2e19a test(pipeline): pin BackfillRefs' page read with a swap race (#157)
Review round 1 on PR #176: reverting the BackfillRefs concept-page read
to os.ReadFile left every test green. The static symlinked page never
reaches that read, because LoadInventory does not list it; the read is
reachable only when a page listed as regular is swapped for a symlink
before it is read. New test does that against an in-brain target and
reads the page back with O_NOFOLLOW; red under the flip (2/2), green
3/3 on head.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 20:47:57 +02:00
mathiasandClaude Opus 5.5 aee5f66398 fix(pipeline,classification): read the brain through brainfs (#157, slice 6)
The ingest pipeline read wiki/sources/<slug>.md and schema.md (both go
into the LLM prompt), the pages it revises, and the pages it adds
source refs to, with os.ReadFile; BackfillRefs listed wiki/sources with
os.ReadDir. A symlinked file's content reached the prompt and the
written pages (1.6M leaks in the race), and a FIFO hung the ingest.
classification.Load (the I1 sovereignty levels) read
classification.yaml the same way and hung startup on a FIFO.

- pipeline: readSourcePage, loadSchema, reviseOrCreate, injectSourceRefs
  read through readBrainFile (brainfs ReadRegular); reviseOrCreate takes
  the brain dir and reads page.Path, a missing page is still created and
  a non-regular one is an error. BackfillRefs lists with ReadDirRegular
  and reads with ReadRegular.
- classification.Load: read through brainfs; a symlinked or special
  classification.yaml is a hard error (as a malformed one is); an absent
  file or brain dir still yields the default config.

Tests use in-brain targets and the [[slug|title]] link form the pipeline
actually follows (bare [[slug]] links are ignored by it, which made a
first draft of the backfill test vacuous).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-05 20:47:57 +02:00
19 changed files with 51 additions and 714 deletions
@@ -4,7 +4,6 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"strings" "strings"
"syscall"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@@ -64,13 +63,3 @@ func TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot(t *testing.T) {
t.Cleanup(func() { _ = os.Chmod(brain, 0o755) }) t.Cleanup(func() { _ = os.Chmod(brain, 0o755) })
assert.Error(t, startupSymlinkCheck(brain)) assert.Error(t, startupSymlinkCheck(brain))
} }
// hyperguild#179: a FIFO in the brain is refused at startup with a clear
// message, instead of hanging or exiting later in a reader.
func TestStartupSymlinkCheckRefusesABrainWithAFIFO(t *testing.T) {
brain := t.TempDir()
require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "classification.yaml"), 0o644))
err := startupSymlinkCheck(brain)
require.Error(t, err)
assert.Contains(t, err.Error(), "classification.yaml")
}
+1 -1
View File
@@ -68,7 +68,7 @@ require (
github.com/segmentio/asm v1.2.0 // indirect github.com/segmentio/asm v1.2.0 // indirect
golang.org/x/crypto v0.51.0 // indirect golang.org/x/crypto v0.51.0 // indirect
golang.org/x/sync v0.21.0 // indirect golang.org/x/sync v0.21.0 // indirect
golang.org/x/sys v0.45.0 golang.org/x/sys v0.45.0 // indirect
golang.org/x/text v0.39.0 // indirect golang.org/x/text v0.39.0 // indirect
gopkg.in/yaml.v3 v3.0.1 gopkg.in/yaml.v3 v3.0.1
) )
@@ -23,13 +23,6 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
_, body, _, err := api.ReadNote(brain, "wiki/w/facts/leak.md") _, body, _, err := api.ReadNote(brain, "wiki/w/facts/leak.md")
assert.Error(t, err) assert.Error(t, err)
assert.NotContains(t, body, brainfstest.Sentinel) 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") p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644)) require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
@@ -46,7 +39,5 @@ func TestReadNoteRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
return s return s
} }
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, read) 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() }) brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() { _ = read() })
} }
@@ -1,36 +0,0 @@
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")
}
@@ -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")
}
+17
View File
@@ -116,6 +116,23 @@ func collectWingEntries(bfs *brainfs.Brain, wingRel string) ([]noteEntry, error)
return out, nil 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, // readTitleAndCreatedIn is readTitleAndCreated through the brain root,
// regular files only (hyperguild#157). // regular files only (hyperguild#157).
func readTitleAndCreatedIn(bfs *brainfs.Brain, rel, slug string) (string, string) { func readTitleAndCreatedIn(bfs *brainfs.Brain, rel, slug string) (string, string) {
+15 -30
View File
@@ -2,9 +2,7 @@ package brain
import ( import (
"bufio" "bufio"
"errors"
"fmt" "fmt"
"io/fs"
"os" "os"
"path" "path"
"path/filepath" "path/filepath"
@@ -41,29 +39,24 @@ type TunnelCandidate struct {
// A note's title is read from YAML frontmatter `title:`; failing that, // A note's title is read from YAML frontmatter `title:`; failing that,
// the filename slug (sans `.md`, hyphens → spaces) is used. // the filename slug (sans `.md`, hyphens → spaces) is used.
func DetectTunnels(brainDir, content string) ([]TunnelCandidate, error) { func DetectTunnels(brainDir, content string) ([]TunnelCandidate, error) {
// Walked and read through the brain root, regular files only wikiDir := filepath.Join(brainDir, "wiki")
// (hyperguild#174, #157): this runs inside every brain_write, so a FIFO if _, err := os.Stat(wikiDir); os.IsNotExist(err) {
// 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) {
return nil, nil return nil, nil
} else if err != nil {
return nil, fmt.Errorf("stat wiki: %w", err)
} }
lowerContent := strings.ToLower(content) lowerContent := strings.ToLower(content)
var out []TunnelCandidate var out []TunnelCandidate
var walkErr error err := filepath.WalkDir(wikiDir, func(path string, d os.DirEntry, err error) error {
err = bfs.WalkRegular("wiki", func(rel string) error { if err != nil {
name := path.Base(rel) return err
if !strings.HasSuffix(name, ".md") || name == "_index.md" { }
if d.IsDir() || !strings.HasSuffix(path, ".md") || d.Name() == "_index.md" {
return nil return nil
} }
title, _ := readTitleAndCreatedIn(bfs, rel, strings.TrimSuffix(name, ".md")) title, _ := readTitleAndCreated(path, strings.TrimSuffix(d.Name(), ".md"))
title = strings.TrimSpace(title) title = strings.TrimSpace(title)
if title == "" { if title == "" {
return nil return nil
@@ -85,25 +78,17 @@ func DetectTunnels(brainDir, content string) ([]TunnelCandidate, error) {
if idx == -1 { if idx == -1 {
return nil return nil
} }
rel, err := filepath.Rel(brainDir, path)
if err != nil {
return err
}
out = append(out, TunnelCandidate{ out = append(out, TunnelCandidate{
TargetPath: rel, TargetPath: filepath.ToSlash(rel),
MatchedTerm: title, MatchedTerm: title,
Exact: isWholeWord(haystack, idx, len(needle)), Exact: isWholeWord(haystack, idx, len(needle)),
}) })
return nil 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 { if err != nil {
return nil, err return nil, err
} }
@@ -1,38 +0,0 @@
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")
}
+3 -36
View File
@@ -104,17 +104,6 @@ func (b *Brain) ReadDirRegular(relDir string) ([]string, error) {
return names, nil 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 // ReadDirDirs lists the real subdirectories directly in a brain-relative
// directory, by name. Every component of relDir must be a real directory // directory, by name. Every component of relDir must be a real directory
// (ErrNotRegular otherwise); symlinks to directories are left out. // (ErrNotRegular otherwise); symlinks to directories are left out.
@@ -139,12 +128,6 @@ func (b *Brain) ReadDirDirs(relDir string) ([]string, error) {
// ReadRegular reads a brain-relative path, refusing anything that is not a // ReadRegular reads a brain-relative path, refusing anything that is not a
// regular file, both before the open and on the opened handle. // regular file, both before the open and on the opened handle.
func (b *Brain) ReadRegular(rel string) ([]byte, error) { 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) fi, err := b.root.Lstat(rel)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -171,12 +154,6 @@ func (b *Brain) readOpened(rel string) ([]byte, error) { return b.readOpenedMax(
// ReadRegularMax is ReadRegular with a size limit, checked on the opened // ReadRegularMax is ReadRegular with a size limit, checked on the opened
// handle before reading (max < 0 means no limit). // handle before reading (max < 0 means no limit).
func (b *Brain) ReadRegularMax(rel string, max int64) ([]byte, error) { 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) fi, err := b.root.Lstat(rel)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -279,20 +256,10 @@ 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 // 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 // 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 // also catches a swap between the two. O_NONBLOCK keeps a FIFO from blocking
// the open. Every directory above the file must be a real directory // the open. Intermediate in-brain symlinks are still followed (os.Root keeps
// (hyperguild#173); a directory swapped in after that check is the #167 // the write inside the brain; hyperguild#167).
// residual.
func (b *Brain) WriteRegular(rel string, data []byte) error { func (b *Brain) WriteRegular(rel string, data []byte) error {
if err := cleanRel(rel); err != nil { rel = filepath.ToSlash(rel)
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) lfi, err := b.root.Lstat(rel)
if err != nil { if err != nil {
return err return err
+1 -51
View File
@@ -218,7 +218,7 @@ func TestReadDirRegularListsOnlyRegularFilesAndRefusesSymlinkedComponents(t *tes
// A final component swapped for a symlink to another regular file in the // 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 // brain between the Lstat and the open: os.Root follows in-root links, so
// only comparing the two (os.SameFile) refuses it. // only comparing the two (os.SameFile) refuses it.
func TestReadRegularDoesNotReturnAnInBrainFileSwappedInBehindASymlink(t *testing.T) { func TestReadRegularNeverReturnsAnInBrainFileSwappedInBehindASymlink(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(dir, "s.md"), []byte("zqsamefilesentinel"), 0o644)) require.NoError(t, os.WriteFile(filepath.Join(dir, "s.md"), []byte("zqsamefilesentinel"), 0o644))
p := filepath.Join(dir, "r.md") p := filepath.Join(dir, "r.md")
@@ -326,53 +326,3 @@ func TestReadDirDirsListsOnlyRealSubdirsAndRefusesSymlinkedComponents(t *testing
_, err = b.ReadDirDirs("alias") _, err = b.ReadDirDirs("alias")
assert.ErrorIs(t, err, ErrNotRegular) 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))
}
@@ -1,10 +1,8 @@
// Package brainfstest is the shared harness for hyperguild#157's swap-race // Package brainfstest is the shared harness for hyperguild#157's swap-race
// tests: it keeps swapping a regular note, a symlink to a sentinel target file // tests: it keeps swapping a regular note, a symlink to an outside file, or a
// (outside the brain, or inside it for hyperguild#171), or a FIFO (with a // FIFO (with a writer held open) into a path while a reader under test loops.
// 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
// It observes two things, over a sampled window only: no sentinel byte from // reader's output, and the reader returns within a timeout. It does not cover
// 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. // hard links, other paths into the reader, or races it does not generate.
package brainfstest package brainfstest
@@ -20,7 +18,7 @@ import (
"time" "time"
) )
// Sentinel marks the target file's contents. // Sentinel marks the outside file's contents.
const Sentinel = "zqbrainfssentinelonefiftyseven" const Sentinel = "zqbrainfssentinelonefiftyseven"
// outsideContent carries Sentinel in every field a reader might extract: the // outsideContent carries Sentinel in every field a reader might extract: the
@@ -143,7 +141,7 @@ func AssertNoLeakFrom(t testing.TB, path, inside, target string, d time.Duration
} }
} }
if leaks > 0 { if leaks > 0 {
t.Fatalf("symlink target %s leaked %d times in %d iterations", target, leaks, iters) t.Fatalf("outside file leaked %d times in %d iterations", leaks, iters)
} }
if n := swaps.Load(); n == 0 { if n := swaps.Load(); n == 0 {
t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters) t.Fatalf("no symlink was swapped into %s in %d iterations: the race did not run", path, iters)
@@ -21,11 +21,6 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
require.NoError(t, os.MkdirAll(facts, 0o755)) require.NoError(t, os.MkdirAll(facts, 0o755))
require.NoError(t, os.Symlink(brainfstest.Outside(t), filepath.Join(facts, "leak.md"))) 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) 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") p := filepath.Join(facts, "r.md")
require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644)) require.NoError(t, os.WriteFile(p, []byte("---\nx: y\n---\ninside"), 0o644))
@@ -33,9 +28,6 @@ func TestReadExcerptRefusesSymlinksAndSurvivesSwapRaces(t *testing.T) {
brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, func() string { brainfstest.AssertNoLeak(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, func() string {
return readExcerpt(brain, "wiki/w/facts/r.md", 500) 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() { brainfstest.AssertNoHang(t, p, "---\nx: y\n---\ninside", 1500*time.Millisecond, 6*time.Second, func() {
_ = readExcerpt(brain, "wiki/w/facts/r.md", 500) _ = readExcerpt(brain, "wiki/w/facts/r.md", 500)
}) })
@@ -38,31 +38,3 @@ func TestReadScopedRefusesANoteOverMaxNoteBytes(t *testing.T) {
_, err := ReadScoped(brain, "wiki/w/facts/big.md", ScopeNotes) _, err := ReadScoped(brain, "wiki/w/facts/big.md", ScopeNotes)
assert.ErrorIs(t, err, ErrTooLarge) 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,8 +12,6 @@ import (
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "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 // hyperguild#157 option 1: search walked and read the brain by path, so a
@@ -191,49 +189,3 @@ func TestHydrateRefusesASymlink(t *testing.T) {
assert.Error(t, err) assert.Error(t, err)
assert.NotContains(t, r.Excerpt, sentinel157) 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
})
}
-85
View File
@@ -2,17 +2,14 @@ package session
import ( import (
"errors" "errors"
"fmt"
"os" "os"
"path/filepath" "path/filepath"
"sync/atomic"
"syscall" "syscall"
"testing" "testing"
"time" "time"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/sys/unix"
) )
// session_log passed the caller's session_id straight into // session_log passed the caller's session_id straight into
@@ -93,85 +90,3 @@ func TestAppendReturnsPromptlyOnAFIFOLogWithNoReader(t *testing.T) {
t.Fatal("Append blocked on a FIFO log with no reader") 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")
}
+3 -31
View File
@@ -65,42 +65,14 @@ func Append(sessionsDir, sessionID string, entry Entry) error {
if err := checkID(sessionID); err != nil { if err := checkID(sessionID); err != nil {
return err return err
} }
// Opened through an os.Root on the sessions dir's parent (the brain dir), if err := os.MkdirAll(sessionsDir, 0o755); err != nil {
// 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) return fmt.Errorf("create sessions dir: %w", err)
} }
// The sessions dir itself must be a real directory, not a symlink that path := filepath.Join(sessionsDir, sessionID+".jsonl")
// redirects the log elsewhere (hyperguild#173). f, err := os.OpenFile(path, os.O_CREATE|os.O_APPEND|os.O_WRONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0o644)
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 { if err != nil {
return fmt.Errorf("open session log: %w", err) 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) line, err := json.Marshal(entry)
if err != nil { if err != nil {
@@ -1,104 +0,0 @@
package symlinkguard
import (
"context"
"net"
"os"
"path/filepath"
"syscall"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// hyperguild#179: the tripwire looked only for symlinks, so a FIFO (or
// socket) in the brain passed it. A FIFO at classification.yaml then made
// startup exit; at a note path it could hang readers that skip the handle
// check.
func TestScanReportsFIFOsAndSocketsAsSpecialFiles(t *testing.T) {
brain := t.TempDir()
require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w"), 0o755))
require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "classification.yaml"), 0o644))
require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "wiki", "w", "f.md"), 0o644))
l, err := net.Listen("unix", filepath.Join(brain, "s.sock"))
require.NoError(t, err)
defer func() { _ = l.Close() }()
require.NoError(t, os.WriteFile(filepath.Join(brain, "wiki", "w", "n.md"), []byte("n"), 0o644))
r, err := Scan(brain)
require.NoError(t, err)
assert.Equal(t, []string{"classification.yaml", "s.sock", "wiki/w/f.md"}, r.Special)
assert.False(t, r.Clean())
assert.Contains(t, r.String(), "special file")
}
// Watch removes a FIFO as it removes symlinks, and leaves a regular file
// (which Scan never reports) alone.
func TestWatchRemovesAFIFOAndLeavesAnUnreportedRegularFile(t *testing.T) {
brain := t.TempDir()
fifo := filepath.Join(brain, "f.md")
require.NoError(t, syscall.Mkfifo(fifo, 0o644))
regular := filepath.Join(brain, "n.md")
require.NoError(t, os.WriteFile(regular, []byte("n"), 0o644))
n := &recNotifier{}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go Watch(ctx, brain, 10*time.Millisecond, n)
require.Eventually(t, func() bool {
_, err := os.Lstat(fifo)
return os.IsNotExist(err)
}, 2*time.Second, 10*time.Millisecond, "the FIFO is removed")
require.Eventually(t, func() bool { return n.count() >= 1 }, 2*time.Second, 10*time.Millisecond)
b, err := os.ReadFile(regular)
require.NoError(t, err)
assert.Equal(t, "n", string(b))
n.mu.Lock()
defer n.mu.Unlock()
assert.Contains(t, n.msgs[0], "f.md")
}
// removeSpecial re-checks the type: a path that is a regular file or a
// directory at the re-check is not removed. (A swap between the re-check and
// the unlink can still remove one; Linux has no conditional unlink. The same
// window exists in removeSymlinks.)
func TestRemoveSpecialRefusesRegularFilesAndDirectories(t *testing.T) {
brain := t.TempDir()
require.NoError(t, os.WriteFile(filepath.Join(brain, "n.md"), []byte("n"), 0o644))
require.NoError(t, os.MkdirAll(filepath.Join(brain, "d"), 0o755))
require.NoError(t, syscall.Mkfifo(filepath.Join(brain, "f.md"), 0o644))
removed, failed := removeSpecial(brain, []string{"n.md", "d", "f.md"})
assert.Equal(t, []string{"f.md"}, removed)
assert.Equal(t, []string{"n.md", "d"}, failed)
_, err := os.Stat(filepath.Join(brain, "n.md"))
assert.NoError(t, err)
}
// A special file Watch cannot remove (its directory is read-only) stays
// reported: the persistent-finding alert names it.
func TestWatchReportsASpecialFileItCannotRemove(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("root ignores directory permissions")
}
brain := t.TempDir()
ro := filepath.Join(brain, "ro")
require.NoError(t, os.MkdirAll(ro, 0o755))
require.NoError(t, syscall.Mkfifo(filepath.Join(ro, "f.md"), 0o644))
require.NoError(t, os.Chmod(ro, 0o555))
t.Cleanup(func() { _ = os.Chmod(ro, 0o755) })
n := &recNotifier{}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
go Watch(ctx, brain, 10*time.Millisecond, n)
require.Eventually(t, func() bool { return n.count() >= 1 }, 2*time.Second, 10*time.Millisecond)
_, err := os.Lstat(filepath.Join(ro, "f.md"))
require.NoError(t, err, "the FIFO could not be removed")
n.mu.Lock()
defer n.mu.Unlock()
assert.Contains(t, n.msgs[0], "ro/f.md")
assert.Contains(t, n.msgs[0], "special file")
for _, m := range n.msgs {
assert.NotContains(t, m, "removed", "a file it could not remove was reported as removed")
}
}
@@ -46,16 +46,10 @@ type Notifier interface {
type Result struct { type Result struct {
Symlinks []string Symlinks []string
Unreadable []string Unreadable []string
// Special holds FIFOs, sockets and devices (hyperguild#179): a reader
// that opens one by path can hang, and a FIFO at classification.yaml
// stops startup.
Special []string
} }
// Clean reports whether the scan found nothing to act on. // Clean reports whether the scan found nothing to act on.
func (r Result) Clean() bool { func (r Result) Clean() bool { return len(r.Symlinks) == 0 && len(r.Unreadable) == 0 }
return len(r.Symlinks) == 0 && len(r.Unreadable) == 0 && len(r.Special) == 0
}
// String describes the findings for a log line or alert. Paths are quoted, so // String describes the findings for a log line or alert. Paths are quoted, so
// a name containing a newline cannot blur two findings together. // a name containing a newline cannot blur two findings together.
@@ -65,9 +59,6 @@ func (r Result) String() string {
if len(r.Symlinks) > 0 { if len(r.Symlinks) > 0 {
parts = append(parts, fmt.Sprintf("%d symlink(s): %s", len(r.Symlinks), q(r.Symlinks))) parts = append(parts, fmt.Sprintf("%d symlink(s): %s", len(r.Symlinks), q(r.Symlinks)))
} }
if len(r.Special) > 0 {
parts = append(parts, fmt.Sprintf("%d special file(s) (FIFO, socket or device; delete them): %s", len(r.Special), q(r.Special)))
}
if len(r.Unreadable) > 0 { if len(r.Unreadable) > 0 {
parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable))) parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable)))
} }
@@ -122,16 +113,12 @@ func Scan(brainDir string) (Result, error) {
if d.IsDir() && p == rootGit { if d.IsDir() && p == rootGit {
return filepath.SkipDir return filepath.SkipDir
} }
switch { if d.Type()&os.ModeSymlink != 0 {
case d.Type()&os.ModeSymlink != 0:
r.Symlinks = append(r.Symlinks, rel) r.Symlinks = append(r.Symlinks, rel)
case !d.IsDir() && !d.Type().IsRegular():
r.Special = append(r.Special, rel)
} }
return nil return nil
}) })
sort.Strings(r.Symlinks) sort.Strings(r.Symlinks)
sort.Strings(r.Special)
sort.Strings(r.Unreadable) sort.Strings(r.Unreadable)
if walkErr != nil { if walkErr != nil {
// Only the root's own error reaches here (deeper errors are recorded // Only the root's own error reaches here (deeper errors are recorded
@@ -195,20 +182,14 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
q.push(fmt.Sprintf("brain symlink tripwire (hyperguild#157): removed %d symlink(s) from the brain dir: %s", q.push(fmt.Sprintf("brain symlink tripwire (hyperguild#157): removed %d symlink(s) from the brain dir: %s",
len(removed), Result{}.quoted(removed))) len(removed), Result{}.quoted(removed)))
} }
sRemoved, sFailed := removeSpecial(brainDir, r.Special) key = Result{Symlinks: failed, Unreadable: r.Unreadable}.String()
if len(sRemoved) > 0 {
slog.Error("brain symlink tripwire removed special files", "count", len(sRemoved))
q.push(fmt.Sprintf("brain symlink tripwire (hyperguild#179): removed %d special file(s) (FIFO, socket or device) from the brain dir: %s",
len(sRemoved), Result{}.quoted(sRemoved)))
}
key = Result{Symlinks: failed, Unreadable: r.Unreadable, Special: sFailed}.String()
} }
if key != reported { if key != reported {
reported = key reported = key
if key != "" { if key != "" {
slog.Error("brain symlink tripwire", "finding", key) slog.Error("brain symlink tripwire", "finding", key)
q.push("brain symlink tripwire (hyperguild#157, #179): " + key + q.push("brain symlink tripwire (hyperguild#157): " + key +
". Symlinks can lead readers out of the brain and special files can hang them; remove them.") ". Readers may follow symlinks out of the brain; fix them.")
} }
} }
} }
@@ -341,40 +322,6 @@ func permanent(err error) bool {
return errors.As(err, &p) && p.Permanent() return errors.As(err, &p) && p.Permanent()
} }
// removeSpecial unlinks each brain-relative FIFO, socket or device through
// an os.Root on the brain dir, after re-checking that it is still one: a
// path that is a regular file, a directory or a symlink at the re-check is
// left alone and reported as failed. The re-check narrows, but cannot close,
// the window before the unlink (Linux has no conditional unlink); the same
// window exists in removeSymlinks, and only a local writer can use it.
func removeSpecial(brainDir string, rels []string) (removed, failed []string) {
if len(rels) == 0 {
return nil, nil
}
resolved, err := filepath.EvalSymlinks(brainDir)
if err != nil {
return nil, rels
}
root, err := os.OpenRoot(resolved)
if err != nil {
return nil, rels
}
defer func() { _ = root.Close() }()
for _, rel := range rels {
fi, err := root.Lstat(rel)
if err != nil || fi.IsDir() || fi.Mode().IsRegular() || fi.Mode()&os.ModeSymlink != 0 {
failed = append(failed, rel)
continue
}
if err := root.Remove(rel); err != nil {
failed = append(failed, rel)
continue
}
removed = append(removed, rel)
}
return removed, failed
}
// removeSymlinks unlinks each brain-relative symlink through an os.Root on // removeSymlinks unlinks each brain-relative symlink through an os.Root on
// the brain dir, after re-checking it is still a symlink. Removing a symlink // the brain dir, after re-checking it is still a symlink. Removing a symlink
// never touches its target. It returns what was removed and what was not. // never touches its target. It returns what was removed and what was not.
@@ -42,11 +42,6 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
brain := t.TempDir() brain := t.TempDir()
writeNote(t, brain, "wiki/w/facts/ok.md", "inside note") 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"))) 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{} emb := &recordingEmbedder{}
store := &stubStore{} store := &stubStore{}
@@ -56,7 +51,6 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
assert.NotContains(t, emb.all(), brainfstest.Sentinel) assert.NotContains(t, emb.all(), brainfstest.Sentinel)
for p := range store.upserts { for p := range store.upserts {
assert.NotContains(t, p, "leak.md") assert.NotContains(t, p, "leak.md")
assert.NotContains(t, p, "inlink.md")
} }
p := filepath.Join(brain, "knowledge", "r.md") p := filepath.Join(brain, "knowledge", "r.md")
@@ -67,7 +61,6 @@ func TestSyncRefusesSymlinkedNotesAndSurvivesSwapRaces(t *testing.T) {
return e.all() return e.all()
} }
brainfstest.AssertNoLeak(t, p, "inside", 1500*time.Millisecond, sync) 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() }) brainfstest.AssertNoHang(t, p, "inside", 1500*time.Millisecond, 6*time.Second, func() { _ = sync() })
} }