From f1f975d281a90879db71fa110e0a442bd8bf5af8 Mon Sep 17 00:00:00 2001 From: Mathias Date: Mon, 5 Oct 2026 22:42:44 +0200 Subject: [PATCH] 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.