From 796d226497ac7b01daf2793649307db0f9dcd346 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 17:04:41 +0200 Subject: [PATCH 1/3] feat(ingestion): symlink tripwire for the Syncthing-shared brain dir The brain dir is a Syncthing sendreceive folder and Syncthing syncs symlinks; search, vectorstore, context and graph readers follow one out of the brain and would index or return what it points to (reproduced with search.Query). Decision (#157): a tripwire now, brain-wide os.Root readers next. - symlinkguard.Scan lists every symlink inside the brain dir (skipping .git; the brain dir itself may be a mount symlink). - At startup the server refuses to start while any exists, naming them. - symlinkguard.Watch rescans every BRAIN_SYMLINK_SCAN_INTERVAL seconds (default 300) and sends one ntfy alert per new set of symlinks. The live brain dir had 0 symlinks on 2026-10-03, checked before this change, so deploying it does not trip. Refs #157 Co-Authored-By: Claude Opus 5.5 --- ingestion/cmd/server/main.go | 7 ++ ingestion/cmd/server/symlink_check.go | 24 +++++ ingestion/cmd/server/symlink_check_test.go | 26 ++++++ .../internal/symlinkguard/symlinkguard.go | 91 +++++++++++++++++++ .../symlinkguard/symlinkguard_test.go | 87 ++++++++++++++++++ 5 files changed, 235 insertions(+) create mode 100644 ingestion/cmd/server/symlink_check.go create mode 100644 ingestion/cmd/server/symlink_check_test.go create mode 100644 ingestion/internal/symlinkguard/symlinkguard.go create mode 100644 ingestion/internal/symlinkguard/symlinkguard_test.go diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index 59def19..81b28c1 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -33,6 +33,7 @@ import ( "github.com/mathiasbq/hyperguild/ingestion/internal/reranker" "github.com/mathiasbq/hyperguild/ingestion/internal/search" "github.com/mathiasbq/hyperguild/ingestion/internal/startupretry" + "github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard" "github.com/mathiasbq/hyperguild/ingestion/internal/vectorstore" "github.com/mathiasbq/hyperguild/ingestion/internal/watcher" "github.com/mathiasbq/hyperguild/ingestion/internal/webhook" @@ -239,6 +240,10 @@ func main() { } brainDir := envOr("INGEST_BRAIN_DIR", "../brain") + if err := startupSymlinkCheck(brainDir); err != nil { + logger.Error("brain symlink tripwire", "err", err) + os.Exit(1) + } port := envOr("INGEST_PORT", "3300") llmURL := envOr("INGEST_LLM_URL", "http://iguana:4000/v1") @@ -360,6 +365,8 @@ func main() { } ctx := context.Background() + // hyperguild#157: report a symlink that appears in the brain while running. + go symlinkguard.Watch(ctx, brainDir, time.Duration(envInt("BRAIN_SYMLINK_SCAN_INTERVAL", 300))*time.Second, buildNotifier()) // Built before the watcher, the graph backfill and session ingestion, and // retried rather than fatal (#83). Two separate reasons: diff --git a/ingestion/cmd/server/symlink_check.go b/ingestion/cmd/server/symlink_check.go new file mode 100644 index 0000000..4439e1a --- /dev/null +++ b/ingestion/cmd/server/symlink_check.go @@ -0,0 +1,24 @@ +package main + +import ( + "fmt" + "strings" + + "github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard" +) + +// startupSymlinkCheck is the hyperguild#157 tripwire at boot: a symlink in the +// brain dir would let brain readers serve a file from outside the brain, so +// the server does not start until it is removed. A scan that cannot run (e.g. +// a missing brain dir on first boot) is not treated as an incident. +func startupSymlinkCheck(brainDir string) error { + found, err := symlinkguard.Scan(brainDir) + if err != nil { + return nil + } + if len(found) > 0 { + return fmt.Errorf("refusing to start: %d symlink(s) in the brain dir %s (hyperguild#157); remove them: %s", + len(found), brainDir, strings.Join(found, ", ")) + } + return nil +} diff --git a/ingestion/cmd/server/symlink_check_test.go b/ingestion/cmd/server/symlink_check_test.go new file mode 100644 index 0000000..e8efc03 --- /dev/null +++ b/ingestion/cmd/server/symlink_check_test.go @@ -0,0 +1,26 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// hyperguild#157: the server refuses to start while a symlink exists in the +// brain dir (Mathias's chosen tripwire), naming it; a clean brain starts. +func TestStartupSymlinkCheckRefusesABrainWithASymlink(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "planted"))) + err := startupSymlinkCheck(brain) + require.Error(t, err) + assert.Contains(t, err.Error(), "planted") +} + +func TestStartupSymlinkCheckPassesACleanBrain(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(brain, "n.md"), []byte("x"), 0o644)) + assert.NoError(t, startupSymlinkCheck(brain)) +} diff --git a/ingestion/internal/symlinkguard/symlinkguard.go b/ingestion/internal/symlinkguard/symlinkguard.go new file mode 100644 index 0000000..ecf68ab --- /dev/null +++ b/ingestion/internal/symlinkguard/symlinkguard.go @@ -0,0 +1,91 @@ +// Package symlinkguard is the hyperguild#157 tripwire. The brain directory is +// a Syncthing sendreceive folder and Syncthing syncs symlinks; several brain +// readers (search, vectorstore, context, graph) follow a symlink out of the +// brain and would index or return whatever it points to. Until every reader +// reads through an os.Root, any symlink inside the brain is treated as an +// incident: the server refuses to start on one (main), and Watch reports one +// that appears while it runs. +package symlinkguard + +import ( + "context" + "fmt" + "io/fs" + "log/slog" + "os" + "path/filepath" + "sort" + "strings" + "time" +) + +// Notifier is the out-of-band alert sink (audit.Notifier satisfies it). +type Notifier interface { + Notify(ctx context.Context, msg string) error +} + +// Scan returns the brain-relative paths of every symlink inside brainDir, +// skipping .git. brainDir itself may be a symlink (a volume mount); it is +// resolved first, and only entries inside it are checked. WalkDir never +// descends a symlinked directory, so each link is reported, not followed. +func Scan(brainDir string) ([]string, error) { + root, err := filepath.EvalSymlinks(brainDir) + if err != nil { + return nil, fmt.Errorf("resolve brain dir: %w", err) + } + var found []string + err = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() && d.Name() == ".git" { + return filepath.SkipDir + } + if d.Type()&os.ModeSymlink != 0 { + rel, _ := filepath.Rel(root, p) + found = append(found, filepath.ToSlash(rel)) + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("scan brain dir: %w", err) + } + sort.Strings(found) + return found, nil +} + +// Watch rescans every interval and notifies once per distinct set of +// symlinks found, until ctx ends. A scan error is logged, not fatal. +func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notifier) { + t := time.NewTicker(interval) + defer t.Stop() + last := "" + for { + select { + case <-ctx.Done(): + return + case <-t.C: + } + found, err := Scan(brainDir) + if err != nil { + slog.Warn("symlink tripwire scan failed", "err", err) + continue + } + key := strings.Join(found, "\n") + if key == last { + continue + } + last = key + if len(found) == 0 { + continue + } + msg := fmt.Sprintf("brain symlink tripwire (hyperguild#157): %d symlink(s) in the brain dir; "+ + "readers may follow them out of the brain. Remove them: %s", len(found), strings.Join(found, ", ")) + slog.Error("brain symlink tripwire", "count", len(found), "paths", found) + if n != nil { + if err := n.Notify(ctx, msg); err != nil { + slog.Warn("symlink tripwire notify failed", "err", err) + } + } + } +} diff --git a/ingestion/internal/symlinkguard/symlinkguard_test.go b/ingestion/internal/symlinkguard/symlinkguard_test.go new file mode 100644 index 0000000..d9405b4 --- /dev/null +++ b/ingestion/internal/symlinkguard/symlinkguard_test.go @@ -0,0 +1,87 @@ +package symlinkguard + +import ( + "context" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// hyperguild#157: the brain dir is a Syncthing sendreceive folder and +// Syncthing syncs symlinks; search, vectorstore, context and graph readers +// follow them out of the brain. Until every reader goes through an os.Root, +// a symlink anywhere in the brain is a tripwire: the server refuses to start +// on one, and reports one that appears while it runs. + +func TestScanFindsSymlinksAtAnyDepthAndSkipsGit(t *testing.T) { + brain := t.TempDir() + outside := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(brain, ".git", "objects"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(brain, "wiki", "w", "facts", "ok.md"), []byte("x"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join(outside, "f"), filepath.Join(brain, "wiki", "w", "facts", "leak.md"))) + require.NoError(t, os.Symlink(outside, filepath.Join(brain, "wiki", "dirlink"))) + require.NoError(t, os.Symlink(outside, filepath.Join(brain, ".git", "objects", "alt"))) + + found, err := Scan(brain) + require.NoError(t, err) + assert.ElementsMatch(t, []string{"wiki/w/facts/leak.md", "wiki/dirlink"}, found) +} + +func TestScanOfACleanBrainFindsNothing(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "n.md"), []byte("x"), 0o644)) + found, err := Scan(brain) + require.NoError(t, err) + assert.Empty(t, found) +} + +// The brain dir itself being a symlink is fine (the volume mount may be one); +// only entries inside it are checked. +func TestScanFollowsTheRootButNotEntries(t *testing.T) { + real := t.TempDir() + link := filepath.Join(t.TempDir(), "brain") + require.NoError(t, os.Symlink(real, link)) + require.NoError(t, os.WriteFile(filepath.Join(real, "n.md"), []byte("x"), 0o644)) + found, err := Scan(link) + require.NoError(t, err) + assert.Empty(t, found) +} + +type recNotifier struct { + mu sync.Mutex + msgs []string +} + +func (r *recNotifier) Notify(_ context.Context, m string) error { + r.mu.Lock() + defer r.mu.Unlock() + r.msgs = append(r.msgs, m) + return nil +} + +func (r *recNotifier) count() int { r.mu.Lock(); defer r.mu.Unlock(); return len(r.msgs) } + +// Watch reports a symlink that appears while running, once per distinct set. +func TestWatchNotifiesOncePerNewSymlinkSet(t *testing.T) { + brain := t.TempDir() + n := &recNotifier{} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go Watch(ctx, brain, 10*time.Millisecond, n) + + time.Sleep(50 * time.Millisecond) + assert.Zero(t, n.count(), "a clean brain raises nothing") + + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "planted"))) + require.Eventually(t, func() bool { return n.count() == 1 }, 2*time.Second, 10*time.Millisecond) + time.Sleep(60 * time.Millisecond) + assert.Equal(t, 1, n.count(), "the same symlink set is reported once, not every tick") + assert.Contains(t, n.msgs[0], "planted") +} -- 2.54.0 From 956ad8b9fbbe88526634edfe0cd43d75811adcc7 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 17:11:45 +0200 Subject: [PATCH 2/3] fix(ingestion): close the PR #159 review findings on the symlink tripwire - Startup passed on ANY scan error: WalkDir stopped at the first unreadable dir and Scan discarded what it had found, while search kept walking and served a planted symlink. Scan now records unreadable dirs and keeps going; startup refuses on symlinks or unreadable dirs and only lets a missing brain dir pass. - Watch went silent while a scan error persisted, and marked a set as reported before Notify, so an ntfy outage lost the alert. It now alerts on unreadable dirs and scan errors too, and records a set as reported only after Notify succeeds (retried each tick). - Only the root .git is skipped; a nested .git is scanned. - A non-positive BRAIN_SYMLINK_SCAN_INTERVAL fell into time.NewTicker and crashlooped the server; it now falls back to the default. - Findings are quoted in alerts, so a newline in a name cannot merge two sets; alertingWarnings names the tripwire when ntfy is unset. - Test names match what they assert (two-set dedupe, symlinked entries inside a symlinked root, nested .git). Flipped: startup ignoring unreadable dirs, .git skipped at any depth, and no retry each turn a test red. Watch tests stable over -count=10 and under -race. Refs #157 Co-Authored-By: Claude Opus 5.5 --- ingestion/cmd/server/main.go | 5 +- ingestion/cmd/server/symlink_check.go | 31 +++-- ingestion/cmd/server/symlink_check_test.go | 33 ++++- .../internal/symlinkguard/symlinkguard.go | 130 +++++++++++++----- .../symlinkguard/symlinkguard_test.go | 118 ++++++++++++---- 5 files changed, 242 insertions(+), 75 deletions(-) diff --git a/ingestion/cmd/server/main.go b/ingestion/cmd/server/main.go index 81b28c1..f0fc95d 100644 --- a/ingestion/cmd/server/main.go +++ b/ingestion/cmd/server/main.go @@ -174,7 +174,8 @@ func alertingWarnings(getenv func(string) string) []string { var out []string if getenv("BRAIN_NTFY_URL") == "" { out = append(out, "alerting DISABLED: BRAIN_NTFY_URL unset — "+ - "brain_edit_root governance rewrites and capture audit degradation will not notify") + "brain_edit_root governance rewrites, capture audit degradation and the "+ + "brain symlink tripwire (hyperguild#157) will not notify") } return out } @@ -366,7 +367,7 @@ func main() { ctx := context.Background() // hyperguild#157: report a symlink that appears in the brain while running. - go symlinkguard.Watch(ctx, brainDir, time.Duration(envInt("BRAIN_SYMLINK_SCAN_INTERVAL", 300))*time.Second, buildNotifier()) + go symlinkguard.Watch(ctx, brainDir, symlinkScanInterval(os.Getenv), buildNotifier()) // Built before the watcher, the graph backfill and session ingestion, and // retried rather than fatal (#83). Two separate reasons: diff --git a/ingestion/cmd/server/symlink_check.go b/ingestion/cmd/server/symlink_check.go index 4439e1a..f1b987e 100644 --- a/ingestion/cmd/server/symlink_check.go +++ b/ingestion/cmd/server/symlink_check.go @@ -1,24 +1,39 @@ package main import ( + "errors" "fmt" - "strings" + "os" + "strconv" + "time" "github.com/mathiasbq/hyperguild/ingestion/internal/symlinkguard" ) // startupSymlinkCheck is the hyperguild#157 tripwire at boot: a symlink in the // brain dir would let brain readers serve a file from outside the brain, so -// the server does not start until it is removed. A scan that cannot run (e.g. -// a missing brain dir on first boot) is not treated as an incident. +// the server does not start until it is removed. A directory the scan cannot +// read could hide one, so it refuses on that too. A missing brain dir (first +// boot) is the only case it lets pass. func startupSymlinkCheck(brainDir string) error { - found, err := symlinkguard.Scan(brainDir) + r, err := symlinkguard.Scan(brainDir) if err != nil { - return nil + if errors.Is(err, os.ErrNotExist) { + return nil + } + return fmt.Errorf("refusing to start: cannot scan the brain dir %s (hyperguild#157): %w", brainDir, err) } - if len(found) > 0 { - return fmt.Errorf("refusing to start: %d symlink(s) in the brain dir %s (hyperguild#157); remove them: %s", - len(found), brainDir, strings.Join(found, ", ")) + if !r.Clean() { + return fmt.Errorf("refusing to start: brain dir %s has %s (hyperguild#157); fix them first", brainDir, r) } return nil } + +// symlinkScanInterval reads BRAIN_SYMLINK_SCAN_INTERVAL (seconds), falling +// back to the default on an unset, non-numeric or non-positive value. +func symlinkScanInterval(getenv func(string) string) time.Duration { + if s, err := strconv.Atoi(getenv("BRAIN_SYMLINK_SCAN_INTERVAL")); err == nil && s > 0 { + return time.Duration(s) * time.Second + } + return symlinkguard.DefaultInterval +} diff --git a/ingestion/cmd/server/symlink_check_test.go b/ingestion/cmd/server/symlink_check_test.go index e8efc03..0efcb5e 100644 --- a/ingestion/cmd/server/symlink_check_test.go +++ b/ingestion/cmd/server/symlink_check_test.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -10,7 +11,8 @@ import ( ) // hyperguild#157: the server refuses to start while a symlink exists in the -// brain dir (Mathias's chosen tripwire), naming it; a clean brain starts. +// brain dir, or while any part of it cannot be scanned (which could hide +// one); a missing brain dir is the only thing it lets pass. func TestStartupSymlinkCheckRefusesABrainWithASymlink(t *testing.T) { brain := t.TempDir() require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "planted"))) @@ -19,8 +21,35 @@ func TestStartupSymlinkCheckRefusesABrainWithASymlink(t *testing.T) { assert.Contains(t, err.Error(), "planted") } -func TestStartupSymlinkCheckPassesACleanBrain(t *testing.T) { +func TestStartupSymlinkCheckRefusesABrainWithAnUnreadableDir(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root reads mode-000 directories") + } + brain := t.TempDir() + locked := filepath.Join(brain, "aaa") + require.NoError(t, os.MkdirAll(locked, 0o755)) + require.NoError(t, os.Chmod(locked, 0o000)) + t.Cleanup(func() { _ = os.Chmod(locked, 0o755) }) + err := startupSymlinkCheck(brain) + require.Error(t, err) + assert.Contains(t, err.Error(), "aaa") +} + +func TestStartupSymlinkCheckPassesACleanOrMissingBrain(t *testing.T) { brain := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(brain, "n.md"), []byte("x"), 0o644)) assert.NoError(t, startupSymlinkCheck(brain)) + assert.NoError(t, startupSymlinkCheck(filepath.Join(brain, "missing"))) +} + +func TestSymlinkScanIntervalFallsBackOnNonPositive(t *testing.T) { + for _, v := range []string{"0", "-5", "", "junk"} { + got := symlinkScanInterval(func(string) string { return v }) + assert.Positive(t, got, "env %q", v) + } +} + +func TestAlertingWarningsNamesTheSymlinkTripwire(t *testing.T) { + w := strings.Join(alertingWarnings(func(string) string { return "" }), " ") + assert.Contains(t, w, "symlink tripwire") } diff --git a/ingestion/internal/symlinkguard/symlinkguard.go b/ingestion/internal/symlinkguard/symlinkguard.go index ecf68ab..e3fa3b2 100644 --- a/ingestion/internal/symlinkguard/symlinkguard.go +++ b/ingestion/internal/symlinkguard/symlinkguard.go @@ -2,9 +2,9 @@ // a Syncthing sendreceive folder and Syncthing syncs symlinks; several brain // readers (search, vectorstore, context, graph) follow a symlink out of the // brain and would index or return whatever it points to. Until every reader -// reads through an os.Root, any symlink inside the brain is treated as an -// incident: the server refuses to start on one (main), and Watch reports one -// that appears while it runs. +// reads through an os.Root, a symlink inside the brain, or a directory the +// scan cannot read (which could hide one), is an incident: the server refuses +// to start on one (main), and Watch reports one that appears while it runs. package symlinkguard import ( @@ -15,77 +15,131 @@ import ( "os" "path/filepath" "sort" + "strconv" "strings" "time" ) +// DefaultInterval is used when Watch is given a non-positive interval. +const DefaultInterval = 5 * time.Minute + // Notifier is the out-of-band alert sink (audit.Notifier satisfies it). type Notifier interface { Notify(ctx context.Context, msg string) error } -// Scan returns the brain-relative paths of every symlink inside brainDir, -// skipping .git. brainDir itself may be a symlink (a volume mount); it is -// resolved first, and only entries inside it are checked. WalkDir never -// descends a symlinked directory, so each link is reported, not followed. -func Scan(brainDir string) ([]string, error) { +// Result is what a scan found: symlinks, and directories it could not read +// (each of which could hide a symlink). Paths are brain-relative. +type Result struct { + Symlinks []string + Unreadable []string +} + +// Clean reports whether the scan found nothing to act on. +func (r Result) Clean() bool { return len(r.Symlinks) == 0 && len(r.Unreadable) == 0 } + +// String describes the findings for a log line or alert. Paths are quoted, so +// a name containing a newline cannot blur two findings together. +func (r Result) String() string { + q := func(xs []string) string { + out := make([]string, len(xs)) + for i, x := range xs { + out[i] = strconv.Quote(x) + } + return strings.Join(out, ", ") + } + var parts []string + if len(r.Symlinks) > 0 { + parts = append(parts, fmt.Sprintf("%d symlink(s): %s", len(r.Symlinks), q(r.Symlinks))) + } + if len(r.Unreadable) > 0 { + parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable))) + } + return strings.Join(parts, "; ") +} + +// Scan walks the brain dir and reports every symlink in it and every +// directory it could not read, continuing past both. Only the ROOT .git is +// skipped (it is brain-sync's, and no reader walks it); a nested .git is +// scanned like any other directory. brainDir itself may be a symlink (a +// volume mount); it is resolved first. The error is non-nil only when the +// root cannot be resolved, wrapping os.ErrNotExist when it is missing. +func Scan(brainDir string) (Result, error) { root, err := filepath.EvalSymlinks(brainDir) if err != nil { - return nil, fmt.Errorf("resolve brain dir: %w", err) + return Result{}, fmt.Errorf("resolve brain dir: %w", err) } - var found []string - err = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + var r Result + rootGit := filepath.Join(root, ".git") + _ = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + rel, _ := filepath.Rel(root, p) + rel = filepath.ToSlash(rel) if err != nil { - return err + if p == root { + return err + } + r.Unreadable = append(r.Unreadable, rel) + if d != nil && d.IsDir() { + return filepath.SkipDir + } + return nil } - if d.IsDir() && d.Name() == ".git" { + if d.IsDir() && p == rootGit { return filepath.SkipDir } if d.Type()&os.ModeSymlink != 0 { - rel, _ := filepath.Rel(root, p) - found = append(found, filepath.ToSlash(rel)) + r.Symlinks = append(r.Symlinks, rel) } return nil }) - if err != nil { - return nil, fmt.Errorf("scan brain dir: %w", err) - } - sort.Strings(found) - return found, nil + sort.Strings(r.Symlinks) + sort.Strings(r.Unreadable) + return r, nil } -// Watch rescans every interval and notifies once per distinct set of -// symlinks found, until ctx ends. A scan error is logged, not fatal. +// Watch rescans every interval (DefaultInterval if non-positive) until ctx +// ends, and notifies once per distinct non-clean result. A result counts as +// reported only after Notify succeeds, so an alert lost to a notifier outage +// is retried on the next tick. func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notifier) { + if interval <= 0 { + interval = DefaultInterval + } t := time.NewTicker(interval) defer t.Stop() - last := "" + reported := "" for { select { case <-ctx.Done(): return case <-t.C: } - found, err := Scan(brainDir) - if err != nil { - slog.Warn("symlink tripwire scan failed", "err", err) + r, err := Scan(brainDir) + var key, msg string + switch { + case err != nil: + key = "error: " + err.Error() + msg = "brain symlink tripwire (hyperguild#157): cannot scan the brain dir: " + err.Error() + case r.Clean(): + reported = "" + continue + default: + key = r.String() + msg = "brain symlink tripwire (hyperguild#157): " + key + + ". Readers may follow symlinks out of the brain; remove them." + } + if key == reported { continue } - key := strings.Join(found, "\n") - if key == last { + slog.Error("brain symlink tripwire", "finding", key) + if n == nil { + reported = key continue } - last = key - if len(found) == 0 { + if err := n.Notify(ctx, msg); err != nil { + slog.Warn("symlink tripwire notify failed; will retry", "err", err) continue } - msg := fmt.Sprintf("brain symlink tripwire (hyperguild#157): %d symlink(s) in the brain dir; "+ - "readers may follow them out of the brain. Remove them: %s", len(found), strings.Join(found, ", ")) - slog.Error("brain symlink tripwire", "count", len(found), "paths", found) - if n != nil { - if err := n.Notify(ctx, msg); err != nil { - slog.Warn("symlink tripwire notify failed", "err", err) - } - } + reported = key } } diff --git a/ingestion/internal/symlinkguard/symlinkguard_test.go b/ingestion/internal/symlinkguard/symlinkguard_test.go index d9405b4..4dbbf67 100644 --- a/ingestion/internal/symlinkguard/symlinkguard_test.go +++ b/ingestion/internal/symlinkguard/symlinkguard_test.go @@ -2,6 +2,7 @@ package symlinkguard import ( "context" + "errors" "os" "path/filepath" "sync" @@ -13,63 +14,84 @@ import ( ) // hyperguild#157: the brain dir is a Syncthing sendreceive folder and -// Syncthing syncs symlinks; search, vectorstore, context and graph readers -// follow them out of the brain. Until every reader goes through an os.Root, -// a symlink anywhere in the brain is a tripwire: the server refuses to start -// on one, and reports one that appears while it runs. +// Syncthing syncs symlinks; several readers follow them out of the brain. +// Until every reader goes through an os.Root, a symlink anywhere in the brain, +// or a directory the scan cannot read (which could hide one), is an incident. -func TestScanFindsSymlinksAtAnyDepthAndSkipsGit(t *testing.T) { +func TestScanReportsSymlinksAtEveryDepthIncludingNestedGit(t *testing.T) { brain := t.TempDir() outside := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w", "facts"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki", "w", "facts", ".git"), 0o755)) require.NoError(t, os.MkdirAll(filepath.Join(brain, ".git", "objects"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(brain, "wiki", "w", "facts", "ok.md"), []byte("x"), 0o644)) require.NoError(t, os.Symlink(filepath.Join(outside, "f"), filepath.Join(brain, "wiki", "w", "facts", "leak.md"))) require.NoError(t, os.Symlink(outside, filepath.Join(brain, "wiki", "dirlink"))) + // PR #159 review: only the ROOT .git is skipped; a nested .git is not + // special to the brain readers, which walk into it. + require.NoError(t, os.Symlink(filepath.Join(outside, "g"), filepath.Join(brain, "wiki", "w", "facts", ".git", "leak.md"))) require.NoError(t, os.Symlink(outside, filepath.Join(brain, ".git", "objects", "alt"))) - found, err := Scan(brain) + r, err := Scan(brain) require.NoError(t, err) - assert.ElementsMatch(t, []string{"wiki/w/facts/leak.md", "wiki/dirlink"}, found) + assert.ElementsMatch(t, []string{"wiki/w/facts/leak.md", "wiki/dirlink", "wiki/w/facts/.git/leak.md"}, r.Symlinks) + assert.Empty(t, r.Unreadable) } -func TestScanOfACleanBrainFindsNothing(t *testing.T) { +// PR #159 review: WalkDir stopped at the first unreadable directory and the +// startup check passed, while search (which keeps walking) served a planted +// symlink. Scan now records the unreadable dir and keeps going. +func TestScanReportsUnreadableDirsAndKeepsWalkingPastThem(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root reads mode-000 directories") + } brain := t.TempDir() - require.NoError(t, os.MkdirAll(filepath.Join(brain, "raw"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(brain, "raw", "n.md"), []byte("x"), 0o644)) - found, err := Scan(brain) + locked := filepath.Join(brain, "aaa") + require.NoError(t, os.MkdirAll(locked, 0o755)) + require.NoError(t, os.Chmod(locked, 0o000)) + t.Cleanup(func() { _ = os.Chmod(locked, 0o755) }) + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "zzz-planted"))) + + r, err := Scan(brain) require.NoError(t, err) - assert.Empty(t, found) + assert.Equal(t, []string{"aaa"}, r.Unreadable) + assert.Equal(t, []string{"zzz-planted"}, r.Symlinks, "the walk continues past the unreadable dir") } -// The brain dir itself being a symlink is fine (the volume mount may be one); -// only entries inside it are checked. -func TestScanFollowsTheRootButNotEntries(t *testing.T) { +func TestScanResolvesASymlinkedRootAndReportsSymlinkedEntriesInsideIt(t *testing.T) { real := t.TempDir() link := filepath.Join(t.TempDir(), "brain") require.NoError(t, os.Symlink(real, link)) require.NoError(t, os.WriteFile(filepath.Join(real, "n.md"), []byte("x"), 0o644)) - found, err := Scan(link) + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(real, "inner"))) + r, err := Scan(link) require.NoError(t, err) - assert.Empty(t, found) + assert.Equal(t, []string{"inner"}, r.Symlinks) +} + +func TestScanOfAMissingRootIsErrNotExist(t *testing.T) { + _, err := Scan(filepath.Join(t.TempDir(), "nope")) + assert.True(t, errors.Is(err, os.ErrNotExist), "got %v", err) } type recNotifier struct { mu sync.Mutex msgs []string + fail int // fail the first N calls } func (r *recNotifier) Notify(_ context.Context, m string) error { r.mu.Lock() defer r.mu.Unlock() r.msgs = append(r.msgs, m) + if r.fail > 0 { + r.fail-- + return errors.New("ntfy down") + } return nil } func (r *recNotifier) count() int { r.mu.Lock(); defer r.mu.Unlock(); return len(r.msgs) } -// Watch reports a symlink that appears while running, once per distinct set. -func TestWatchNotifiesOncePerNewSymlinkSet(t *testing.T) { +func TestWatchNotifiesOncePerDistinctSetAndAgainForANewSet(t *testing.T) { brain := t.TempDir() n := &recNotifier{} ctx, cancel := context.WithCancel(context.Background()) @@ -78,10 +100,56 @@ func TestWatchNotifiesOncePerNewSymlinkSet(t *testing.T) { time.Sleep(50 * time.Millisecond) assert.Zero(t, n.count(), "a clean brain raises nothing") - - require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "planted"))) + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "first"))) require.Eventually(t, func() bool { return n.count() == 1 }, 2*time.Second, 10*time.Millisecond) time.Sleep(60 * time.Millisecond) - assert.Equal(t, 1, n.count(), "the same symlink set is reported once, not every tick") - assert.Contains(t, n.msgs[0], "planted") + assert.Equal(t, 1, n.count(), "the same set is reported once, not every tick") + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "second"))) + require.Eventually(t, func() bool { return n.count() == 2 }, 2*time.Second, 10*time.Millisecond) +} + +// PR #159 review: last was recorded before Notify, so an alert lost to an +// ntfy outage was never retried while the symlink stayed. +func TestWatchRetriesAnAlertThatFailedToSend(t *testing.T) { + brain := t.TempDir() + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "planted"))) + n := &recNotifier{fail: 2} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go Watch(ctx, brain, 10*time.Millisecond, n) + require.Eventually(t, func() bool { return n.count() == 3 }, 2*time.Second, 10*time.Millisecond) + time.Sleep(60 * time.Millisecond) + assert.Equal(t, 3, n.count(), "after one successful send, no more") +} + +func TestWatchAlertsOnAnUnreadableDir(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root reads mode-000 directories") + } + brain := t.TempDir() + locked := filepath.Join(brain, "locked") + require.NoError(t, os.MkdirAll(locked, 0o755)) + require.NoError(t, os.Chmod(locked, 0o000)) + t.Cleanup(func() { _ = os.Chmod(locked, 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) + assert.Contains(t, n.msgs[0], "locked") +} + +// PR #159 review: a non-positive interval panicked time.NewTicker inside the +// goroutine and crashlooped the server. +func TestWatchToleratesANonPositiveInterval(t *testing.T) { + brain := t.TempDir() + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { Watch(ctx, brain, 0, nil); close(done) }() + cancel() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("Watch did not return on cancel") + } } -- 2.54.0 From cda18e6db83d7134bef6208045a9328394c4cff8 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 3 Oct 2026 17:14:46 +0200 Subject: [PATCH 3/3] fix(ingestion): an unlistable brain root is a scan error, not clean PR #159 round 2: with the brain root at mode 0100 (enterable, not listable) WalkDir failed at the root, Scan discarded the error and reported clean, and startup passed while search, which goes straight into wiki/, served a planted symlink. Scan now returns the root's walk error, so startup refuses on anything but a missing root. Also pins that a cleared set is reported again when it reappears. Refs #157 Co-Authored-By: Claude Opus 5.5 --- ingestion/cmd/server/symlink_check_test.go | 10 ++++++ .../internal/symlinkguard/symlinkguard.go | 14 ++++++-- .../symlinkguard/symlinkguard_test.go | 34 +++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/ingestion/cmd/server/symlink_check_test.go b/ingestion/cmd/server/symlink_check_test.go index 0efcb5e..0f6a357 100644 --- a/ingestion/cmd/server/symlink_check_test.go +++ b/ingestion/cmd/server/symlink_check_test.go @@ -53,3 +53,13 @@ func TestAlertingWarningsNamesTheSymlinkTripwire(t *testing.T) { w := strings.Join(alertingWarnings(func(string) string { return "" }), " ") assert.Contains(t, w, "symlink tripwire") } + +func TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root lists mode-0100 directories") + } + brain := t.TempDir() + require.NoError(t, os.Chmod(brain, 0o100)) + t.Cleanup(func() { _ = os.Chmod(brain, 0o755) }) + assert.Error(t, startupSymlinkCheck(brain)) +} diff --git a/ingestion/internal/symlinkguard/symlinkguard.go b/ingestion/internal/symlinkguard/symlinkguard.go index e3fa3b2..2dc11f0 100644 --- a/ingestion/internal/symlinkguard/symlinkguard.go +++ b/ingestion/internal/symlinkguard/symlinkguard.go @@ -62,8 +62,9 @@ func (r Result) String() string { // directory it could not read, continuing past both. Only the ROOT .git is // skipped (it is brain-sync's, and no reader walks it); a nested .git is // scanned like any other directory. brainDir itself may be a symlink (a -// volume mount); it is resolved first. The error is non-nil only when the -// root cannot be resolved, wrapping os.ErrNotExist when it is missing. +// volume mount); it is resolved first. The error is non-nil when the root +// cannot be resolved (wrapping os.ErrNotExist when it is missing) or cannot be +// listed. func Scan(brainDir string) (Result, error) { root, err := filepath.EvalSymlinks(brainDir) if err != nil { @@ -71,7 +72,7 @@ func Scan(brainDir string) (Result, error) { } var r Result rootGit := filepath.Join(root, ".git") - _ = filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + walkErr := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { rel, _ := filepath.Rel(root, p) rel = filepath.ToSlash(rel) if err != nil { @@ -94,6 +95,13 @@ func Scan(brainDir string) (Result, error) { }) sort.Strings(r.Symlinks) sort.Strings(r.Unreadable) + if walkErr != nil { + // Only the root's own error reaches here (deeper errors are recorded + // as Unreadable). A root that cannot be listed cannot be shown clean + // (PR #159 round 2: discarding this reported clean while search + // served a planted symlink). + return r, fmt.Errorf("scan brain dir: %w", walkErr) + } return r, nil } diff --git a/ingestion/internal/symlinkguard/symlinkguard_test.go b/ingestion/internal/symlinkguard/symlinkguard_test.go index 4dbbf67..6b46112 100644 --- a/ingestion/internal/symlinkguard/symlinkguard_test.go +++ b/ingestion/internal/symlinkguard/symlinkguard_test.go @@ -153,3 +153,37 @@ func TestWatchToleratesANonPositiveInterval(t *testing.T) { t.Fatal("Watch did not return on cancel") } } + +// PR #159 round 2: a brain root that can be entered but not listed (mode +// 0100) made WalkDir fail at the root; the error was discarded and Scan +// reported clean, while search (which goes straight into wiki/) still served +// a planted symlink. A root that cannot be listed is now an error. +func TestScanOfAnUnlistableRootIsAnError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root lists mode-0100 directories") + } + brain := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(brain, "wiki"), 0o755)) + require.NoError(t, os.Symlink(t.TempDir(), filepath.Join(brain, "wiki", "leak.md"))) + require.NoError(t, os.Chmod(brain, 0o100)) + t.Cleanup(func() { _ = os.Chmod(brain, 0o755) }) + _, err := Scan(brain) + require.Error(t, err) + assert.False(t, errors.Is(err, os.ErrNotExist), "an unlistable root is not a missing root") +} + +// The same set that disappears and comes back is reported again. +func TestWatchReportsASetAgainAfterItWasCleared(t *testing.T) { + brain := t.TempDir() + link := filepath.Join(brain, "planted") + n := &recNotifier{} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go Watch(ctx, brain, 10*time.Millisecond, n) + require.NoError(t, os.Symlink(t.TempDir(), link)) + require.Eventually(t, func() bool { return n.count() == 1 }, 2*time.Second, 10*time.Millisecond) + require.NoError(t, os.Remove(link)) + time.Sleep(60 * time.Millisecond) + require.NoError(t, os.Symlink(t.TempDir(), link)) + require.Eventually(t, func() bool { return n.count() == 2 }, 2*time.Second, 10*time.Millisecond) +} -- 2.54.0