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 <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"syscall"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
@@ -63,3 +64,13 @@ func TestStartupSymlinkCheckRefusesAnUnlistableBrainRoot(t *testing.T) {
|
|||||||
t.Cleanup(func() { _ = os.Chmod(brain, 0o755) })
|
t.Cleanup(func() { _ = os.Chmod(brain, 0o755) })
|
||||||
assert.Error(t, startupSymlinkCheck(brain))
|
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")
|
||||||
|
}
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
@@ -46,10 +46,16 @@ type Notifier interface {
|
|||||||
type Result struct {
|
type Result struct {
|
||||||
Symlinks []string
|
Symlinks []string
|
||||||
Unreadable []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.
|
// 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
|
// String describes the findings for a log line or alert. Paths are quoted, so
|
||||||
// a name containing a newline cannot blur two findings together.
|
// a name containing a newline cannot blur two findings together.
|
||||||
@@ -59,6 +65,9 @@ func (r Result) String() string {
|
|||||||
if len(r.Symlinks) > 0 {
|
if len(r.Symlinks) > 0 {
|
||||||
parts = append(parts, fmt.Sprintf("%d symlink(s): %s", len(r.Symlinks), q(r.Symlinks)))
|
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 {
|
if len(r.Unreadable) > 0 {
|
||||||
parts = append(parts, fmt.Sprintf("%d unreadable dir(s): %s", len(r.Unreadable), q(r.Unreadable)))
|
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 {
|
if d.IsDir() && p == rootGit {
|
||||||
return filepath.SkipDir
|
return filepath.SkipDir
|
||||||
}
|
}
|
||||||
if d.Type()&os.ModeSymlink != 0 {
|
switch {
|
||||||
|
case d.Type()&os.ModeSymlink != 0:
|
||||||
r.Symlinks = append(r.Symlinks, rel)
|
r.Symlinks = append(r.Symlinks, rel)
|
||||||
|
case !d.IsDir() && !d.Type().IsRegular():
|
||||||
|
r.Special = append(r.Special, rel)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
})
|
})
|
||||||
sort.Strings(r.Symlinks)
|
sort.Strings(r.Symlinks)
|
||||||
|
sort.Strings(r.Special)
|
||||||
sort.Strings(r.Unreadable)
|
sort.Strings(r.Unreadable)
|
||||||
if walkErr != nil {
|
if walkErr != nil {
|
||||||
// Only the root's own error reaches here (deeper errors are recorded
|
// 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",
|
q.push(fmt.Sprintf("brain symlink tripwire (hyperguild#157): removed %d symlink(s) from the brain dir: %s",
|
||||||
len(removed), Result{}.quoted(removed)))
|
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 {
|
if key != reported {
|
||||||
reported = key
|
reported = key
|
||||||
@@ -322,6 +341,38 @@ func permanent(err error) bool {
|
|||||||
return errors.As(err, &p) && p.Permanent()
|
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
|
// 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
|
// 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.
|
// never touches its target. It returns what was removed and what was not.
|
||||||
|
|||||||
Reference in New Issue
Block a user