fix(symlinkguard): report and remove FIFOs, sockets and devices too (#179) #190
@@ -34,9 +34,9 @@ func TestScanReportsFIFOsAndSocketsAsSpecialFiles(t *testing.T) {
|
|||||||
assert.Contains(t, r.String(), "special file")
|
assert.Contains(t, r.String(), "special file")
|
||||||
}
|
}
|
||||||
|
|
||||||
// Watch removes special files as it removes symlinks, and leaves regular
|
// Watch removes a FIFO as it removes symlinks, and leaves a regular file
|
||||||
// files alone.
|
// (which Scan never reports) alone.
|
||||||
func TestWatchRemovesSpecialFilesButNotRegularFiles(t *testing.T) {
|
func TestWatchRemovesAFIFOAndLeavesAnUnreportedRegularFile(t *testing.T) {
|
||||||
brain := t.TempDir()
|
brain := t.TempDir()
|
||||||
fifo := filepath.Join(brain, "f.md")
|
fifo := filepath.Join(brain, "f.md")
|
||||||
require.NoError(t, syscall.Mkfifo(fifo, 0o644))
|
require.NoError(t, syscall.Mkfifo(fifo, 0o644))
|
||||||
@@ -59,8 +59,10 @@ func TestWatchRemovesSpecialFilesButNotRegularFiles(t *testing.T) {
|
|||||||
assert.Contains(t, n.msgs[0], "f.md")
|
assert.Contains(t, n.msgs[0], "f.md")
|
||||||
}
|
}
|
||||||
|
|
||||||
// removeSpecial re-checks the type: a path that is now a regular file or a
|
// removeSpecial re-checks the type: a path that is a regular file or a
|
||||||
// directory is not removed.
|
// 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) {
|
func TestRemoveSpecialRefusesRegularFilesAndDirectories(t *testing.T) {
|
||||||
brain := t.TempDir()
|
brain := t.TempDir()
|
||||||
require.NoError(t, os.WriteFile(filepath.Join(brain, "n.md"), []byte("n"), 0o644))
|
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"))
|
_, err := os.Stat(filepath.Join(brain, "n.md"))
|
||||||
assert.NoError(t, err)
|
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")
|
||||||
|
}
|
||||||
|
|||||||
@@ -66,7 +66,7 @@ func (r Result) String() string {
|
|||||||
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 {
|
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 {
|
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)))
|
||||||
@@ -207,8 +207,8 @@ func Watch(ctx context.Context, brainDir string, interval time.Duration, n Notif
|
|||||||
reported = key
|
reported = key
|
||||||
if key != "" {
|
if key != "" {
|
||||||
slog.Error("brain symlink tripwire", "finding", key)
|
slog.Error("brain symlink tripwire", "finding", key)
|
||||||
q.push("brain symlink tripwire (hyperguild#157): " + key +
|
q.push("brain symlink tripwire (hyperguild#157, #179): " + key +
|
||||||
". Readers may follow symlinks out of the brain; fix them.")
|
". 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
|
// 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
|
// 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
|
// path that is a regular file, a directory or a symlink at the re-check is
|
||||||
// and reported as failed.
|
// 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) {
|
func removeSpecial(brainDir string, rels []string) (removed, failed []string) {
|
||||||
if len(rels) == 0 {
|
if len(rels) == 0 {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
|
|||||||
Reference in New Issue
Block a user