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 <noreply@anthropic.com>
This commit is contained in:
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user