From f1f975d281a90879db71fa110e0a442bd8bf5af8 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:42:44 +0200 Subject: [PATCH 1/3] fix(symlinkguard): report and remove FIFOs, sockets and devices too (#179) The tripwire looked only for symlinks. A FIFO in the brain passed it: at classification.yaml it made startup exit (classification.Load refuses a non-regular file, #176), and at a note path it could hang a reader that skips the handle check. - Scan reports special files (anything not a regular file, directory or symlink) in Result.Special; Clean() and String() include them, so the startup check refuses with a clear message naming the path. - Watch removes them as it removes symlinks, through an os.Root, after re-checking each is still special (a regular file, directory or symlink swapped in is left alone and reported). The live brain has no special files (checked before this change), so the startup check does not newly refuse. Co-Authored-By: Claude Opus 5.5 --- ingestion/cmd/server/symlink_check_test.go | 11 +++ .../internal/symlinkguard/special_test.go | 74 +++++++++++++++++++ .../internal/symlinkguard/symlinkguard.go | 57 +++++++++++++- 3 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 ingestion/internal/symlinkguard/special_test.go diff --git a/ingestion/cmd/server/symlink_check_test.go b/ingestion/cmd/server/symlink_check_test.go index 0f6a357..4dcf12c 100644 --- a/ingestion/cmd/server/symlink_check_test.go +++ b/ingestion/cmd/server/symlink_check_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" "github.com/stretchr/testify/assert" @@ -63,3 +64,13 @@ func TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot(t *testing.T) { t.Cleanup(func() { _ = os.Chmod(brain, 0o755) }) 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") +} diff --git a/ingestion/internal/symlinkguard/special_test.go b/ingestion/internal/symlinkguard/special_test.go new file mode 100644 index 0000000..e16c61a --- /dev/null +++ b/ingestion/internal/symlinkguard/special_test.go @@ -0,0 +1,74 @@ +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 special files as it removes symlinks, and leaves regular +// files alone. +func TestWatchRemovesSpecialFilesButNotRegularFiles(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 now a regular file or a +// directory is not removed. +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) +} diff --git a/ingestion/internal/symlinkguard/symlinkguard.go b/ingestion/internal/symlinkguard/symlinkguard.go index b5bd5da..f848264 100644 --- a/ingestion/internal/symlinkguard/symlinkguard.go +++ b/ingestion/internal/symlinkguard/symlinkguard.go @@ -46,10 +46,16 @@ type Notifier interface { type Result struct { Symlinks []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. -func (r Result) Clean() bool { return len(r.Symlinks) == 0 && len(r.Unreadable) == 0 } +func (r Result) Clean() bool { + 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 // a name containing a newline cannot blur two findings together. @@ -59,6 +65,9 @@ func (r Result) String() string { if len(r.Symlinks) > 0 { 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): %s", len(r.Special), q(r.Special))) + } if len(r.Unreadable) > 0 { parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable))) } @@ -113,12 +122,16 @@ func Scan(brainDir string) (Result, error) { if d.IsDir() && p == rootGit { return filepath.SkipDir } - if d.Type()&os.ModeSymlink != 0 { + switch { + case d.Type()&os.ModeSymlink != 0: r.Symlinks = append(r.Symlinks, rel) + case !d.IsDir() && !d.Type().IsRegular(): + r.Special = append(r.Special, rel) } return nil }) sort.Strings(r.Symlinks) + sort.Strings(r.Special) sort.Strings(r.Unreadable) if walkErr != nil { // Only the root's own error reaches here (deeper errors are recorded @@ -182,7 +195,13 @@ 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", len(removed), Result{}.quoted(removed))) } - key = Result{Symlinks: failed, Unreadable: r.Unreadable}.String() + sRemoved, sFailed := removeSpecial(brainDir, r.Special) + 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 { reported = key @@ -322,6 +341,38 @@ func permanent(err error) bool { 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 now a regular file, a directory or a symlink is left alone +// and reported as failed. +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 // 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. -- 2.54.0 From edcfe9e9bcd7eae0eeca3fa531aa96540434d91e Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:51:19 +0200 Subject: [PATCH 2/3] test(symlinkguard): pin the alert for a special file Watch cannot remove (#179) Review round 1 on PR #190: - B1: dropping the unremovable special files from Watch's persistent finding left every test green. New test: a FIFO in a read-only dir stays and the alert names it; red under that flip. - The removeSpecial doc and test comment no longer claim a regular file swapped in is always left alone: the re-check narrows, but cannot close, the window before the unlink (as in removeSymlinks). - Messages: the special-file finding says what one is and to delete it; the persistent alert no longer talks only about symlinks. - Rule 9: the Watch removal test is named for what it observes. Co-Authored-By: Claude Opus 5.5 --- .../internal/symlinkguard/special_test.go | 37 ++++++++++++++++--- .../internal/symlinkguard/symlinkguard.go | 12 +++--- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/ingestion/internal/symlinkguard/special_test.go b/ingestion/internal/symlinkguard/special_test.go index e16c61a..f592ff4 100644 --- a/ingestion/internal/symlinkguard/special_test.go +++ b/ingestion/internal/symlinkguard/special_test.go @@ -34,9 +34,9 @@ func TestScanReportsFIFOsAndSocketsAsSpecialFiles(t *testing.T) { assert.Contains(t, r.String(), "special file") } -// Watch removes special files as it removes symlinks, and leaves regular -// files alone. -func TestWatchRemovesSpecialFilesButNotRegularFiles(t *testing.T) { +// 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)) @@ -59,8 +59,10 @@ func TestWatchRemovesSpecialFilesButNotRegularFiles(t *testing.T) { assert.Contains(t, n.msgs[0], "f.md") } -// removeSpecial re-checks the type: a path that is now a regular file or a -// directory is not removed. +// 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)) @@ -72,3 +74,28 @@ func TestRemoveSpecialRefusesRegularFilesAndDirectories(t *testing.T) { _, 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") +} diff --git a/ingestion/internal/symlinkguard/symlinkguard.go b/ingestion/internal/symlinkguard/symlinkguard.go index f848264..5086b8a 100644 --- a/ingestion/internal/symlinkguard/symlinkguard.go +++ b/ingestion/internal/symlinkguard/symlinkguard.go @@ -66,7 +66,7 @@ func (r Result) String() string { 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): %s", len(r.Special), q(r.Special))) + 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 { parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable))) @@ -207,8 +207,8 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif reported = key if key != "" { slog.Error("brain symlink tripwire", "finding", key) - q.push("brain symlink tripwire (hyperguild#157): " + key + - ". Readers may follow symlinks out of the brain; fix them.") + q.push("brain symlink tripwire (hyperguild#157, #179): " + key + + ". Symlinks can lead readers out of the brain and special files can hang them; remove them.") } } } @@ -343,8 +343,10 @@ func permanent(err error) bool { // 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 now a regular file, a directory or a symlink is left alone -// and reported as failed. +// 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 -- 2.54.0 From e614d39e0653a1838d873146ede47f2172736c4f Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:55:36 +0200 Subject: [PATCH 3/3] test(symlinkguard): an unremovable special file must not be reported as removed (#179) Review round 2 on PR #190: swallowing root.Remove's error in removeSpecial left every test green (the false 'removed' alert also named the file). The test now asserts no message says 'removed'; red under that flip. Co-Authored-By: Claude Opus 5.5 --- ingestion/internal/symlinkguard/special_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ingestion/internal/symlinkguard/special_test.go b/ingestion/internal/symlinkguard/special_test.go index f592ff4..24ad902 100644 --- a/ingestion/internal/symlinkguard/special_test.go +++ b/ingestion/internal/symlinkguard/special_test.go @@ -98,4 +98,7 @@ func TestWatchReportsASpecialFileItCannotRemove(t *testing.T) { 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") + } } -- 2.54.0