fix(ingestion): bound the tripwire's alerts; test removal guards; deflake race tests
PR #160 review: - The planter chooses the paths, and one alert listed them all, while the queue retried oldest-first forever: a message ntfy refuses (too large) blocked every later alert (2000 links: 0 alerts delivered), and an outage queued one message per removal, unbounded. Alerts now name at most 10 paths and count the rest; the queue holds at most 5, further alerts become one suppression summary; a message failing 5 times is dropped so it cannot block the rest. - removeSymlinks' Lstat re-check had no test: a direct test passes a regular file, a path through an in-brain dir link, a ../ escape and a missing path, and asserts all fail with nothing deleted. A Watch test covers dir and in-brain targets. root.Remove (not RemoveAll) is commented: it refuses a dir swapped in after the Lstat. - Every queued alert is delivered after an outage (truncating the queue after the first send is now red). - hyperguild#162: race tests did not wait for their swapper goroutines before t.TempDir cleanup ("directory not empty" in CI run 8912). Each swapper is now stopped and awaited in a t.Cleanup. Fixes #162 Refs #157 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -23,6 +23,14 @@ import (
|
||||
// DefaultInterval is used when Watch is given a non-positive interval.
|
||||
const DefaultInterval = 30 * time.Second
|
||||
|
||||
// Alert bounds (PR #160 review): the planter chooses the paths, so neither an
|
||||
// alert's size nor the queue may grow with what was planted.
|
||||
const (
|
||||
maxPathsPerAlert = 10 // further paths are counted, not listed
|
||||
maxPending = 5 // queued alerts; the rest become one summary
|
||||
maxAttempts = 5 // a message failing this often is dropped
|
||||
)
|
||||
|
||||
// Notifier is the out-of-band alert sink (audit.Notifier satisfies it).
|
||||
type Notifier interface {
|
||||
Notify(ctx context.Context, msg string) error
|
||||
@@ -52,13 +60,22 @@ func (r Result) String() string {
|
||||
return strings.Join(parts, "; ")
|
||||
}
|
||||
|
||||
// quoted joins paths with each one strconv-quoted.
|
||||
// quoted joins at most maxPathsPerAlert paths, each strconv-quoted, and
|
||||
// counts the rest.
|
||||
func (Result) quoted(xs []string) string {
|
||||
out := make([]string, len(xs))
|
||||
for i, x := range xs {
|
||||
shown := xs
|
||||
if len(shown) > maxPathsPerAlert {
|
||||
shown = shown[:maxPathsPerAlert]
|
||||
}
|
||||
out := make([]string, len(shown))
|
||||
for i, x := range shown {
|
||||
out[i] = strconv.Quote(x)
|
||||
}
|
||||
return strings.Join(out, ", ")
|
||||
s := strings.Join(out, ", ")
|
||||
if extra := len(xs) - len(shown); extra > 0 {
|
||||
s += fmt.Sprintf(" and %d more", extra)
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
// Scan walks the brain dir and reports every symlink in it and every
|
||||
@@ -121,7 +138,7 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
|
||||
}
|
||||
t := time.NewTicker(interval)
|
||||
defer t.Stop()
|
||||
var pending []string
|
||||
q := &alertQueue{}
|
||||
reported := ""
|
||||
for {
|
||||
select {
|
||||
@@ -136,10 +153,9 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
|
||||
} else {
|
||||
removed, failed := removeSymlinks(brainDir, r.Symlinks)
|
||||
if len(removed) > 0 {
|
||||
msg := fmt.Sprintf("brain symlink tripwire (hyperguild#157): removed %d symlink(s) from the brain dir: %s",
|
||||
len(removed), Result{Symlinks: removed}.quoted(removed))
|
||||
slog.Error("brain symlink tripwire removed symlinks", "paths", removed)
|
||||
pending = append(pending, msg)
|
||||
slog.Error("brain symlink tripwire removed symlinks", "count", len(removed))
|
||||
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()
|
||||
}
|
||||
@@ -147,21 +163,59 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
|
||||
reported = key
|
||||
if key != "" {
|
||||
slog.Error("brain symlink tripwire", "finding", key)
|
||||
pending = append(pending, "brain symlink tripwire (hyperguild#157): "+key+
|
||||
q.push("brain symlink tripwire (hyperguild#157): " + key +
|
||||
". Readers may follow symlinks out of the brain; fix them.")
|
||||
}
|
||||
}
|
||||
if n == nil {
|
||||
pending = nil
|
||||
continue
|
||||
}
|
||||
for len(pending) > 0 {
|
||||
if err := n.Notify(ctx, pending[0]); err != nil {
|
||||
slog.Warn("symlink tripwire notify failed; will retry", "err", err)
|
||||
break
|
||||
q.flush(ctx, n)
|
||||
}
|
||||
}
|
||||
|
||||
// alertQueue holds alerts until Notify accepts them, bounded: at most
|
||||
// maxPending queued, further ones counted into one summary, and a message
|
||||
// that fails maxAttempts times dropped so it cannot block the rest.
|
||||
type alertQueue struct {
|
||||
items []queued
|
||||
suppressed int
|
||||
}
|
||||
|
||||
type queued struct {
|
||||
msg string
|
||||
attempts int
|
||||
}
|
||||
|
||||
func (q *alertQueue) push(msg string) {
|
||||
if len(q.items) >= maxPending {
|
||||
q.suppressed++
|
||||
return
|
||||
}
|
||||
q.items = append(q.items, queued{msg: msg})
|
||||
}
|
||||
|
||||
func (q *alertQueue) flush(ctx context.Context, n Notifier) {
|
||||
if n == nil {
|
||||
q.items, q.suppressed = nil, 0
|
||||
return
|
||||
}
|
||||
for len(q.items) > 0 {
|
||||
head := &q.items[0]
|
||||
if err := n.Notify(ctx, head.msg); err != nil {
|
||||
head.attempts++
|
||||
if head.attempts >= maxAttempts {
|
||||
slog.Error("symlink tripwire alert dropped after repeated failures", "attempts", head.attempts, "err", err)
|
||||
q.items = q.items[1:]
|
||||
continue
|
||||
}
|
||||
pending = pending[1:]
|
||||
slog.Warn("symlink tripwire notify failed; will retry", "err", err)
|
||||
return
|
||||
}
|
||||
q.items = q.items[1:]
|
||||
}
|
||||
if q.suppressed > 0 {
|
||||
q.items = append(q.items, queued{msg: fmt.Sprintf(
|
||||
"brain symlink tripwire (hyperguild#157): %d further alert(s) were suppressed while the notifier was failing; check the ingestion logs", q.suppressed)})
|
||||
q.suppressed = 0
|
||||
q.flush(ctx, n)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -187,6 +241,9 @@ func removeSymlinks(brainDir string, rels []string) (removed, failed []string) {
|
||||
failed = append(failed, rel)
|
||||
continue
|
||||
}
|
||||
// root.Remove, not os.RemoveAll: if the link is swapped for a
|
||||
// non-empty directory after the Lstat, Remove refuses rather than
|
||||
// deleting it recursively, and it cannot act outside the root.
|
||||
if err := root.Remove(rel); err != nil {
|
||||
failed = append(failed, rel)
|
||||
continue
|
||||
|
||||
Reference in New Issue
Block a user