From fb543842d8429747defe8dcd7fdd4124f054e5ef Mon Sep 17 00:00:00 2001 From: Endika Iglesias Date: Fri, 31 Jul 2026 19:01:11 +0200 Subject: [PATCH] fix(watch): skip unreadable directories instead of failing the watch Signed-off-by: Endika Iglesias --- pkg/watch/watcher_naive.go | 14 +++++++++++++ pkg/watch/watcher_naive_test.go | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 7798025e8..d9f4ec111 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -113,6 +113,12 @@ func (d *naiveNotify) watchRecursively(dir string) error { return filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { if err != nil { + // A directory we are not allowed to read is not a reason to abandon the + // whole watch: we simply cannot see inside it, so skip it and carry on. + if os.IsPermission(err) { + logrus.Debugf("Not watching %s: %v", path, err) + return filepath.SkipDir + } return err } @@ -130,6 +136,10 @@ func (d *naiveNotify) watchRecursively(dir string) error { if os.IsNotExist(err) { return nil } + if os.IsPermission(err) { + logrus.Debugf("Not watching %s: %v", path, err) + return filepath.SkipDir + } return fmt.Errorf("watcher.Add(%q): %w", path, err) } return nil @@ -180,6 +190,10 @@ func (d *naiveNotify) loop() { //nolint:gocyclo // TODO(dbentley): if there's a delete should we call d.watcher.Remove to prevent leaking? err := filepath.WalkDir(e.Name, func(path string, info fs.DirEntry, err error) error { if err != nil { + if os.IsPermission(err) { + logrus.Debugf("Not watching %s: %v", path, err) + return filepath.SkipDir + } return err } diff --git a/pkg/watch/watcher_naive_test.go b/pkg/watch/watcher_naive_test.go index b188de939..f6b456c43 100644 --- a/pkg/watch/watcher_naive_test.go +++ b/pkg/watch/watcher_naive_test.go @@ -157,3 +157,39 @@ func TestDontRecurseWhenWatchingParentsOfNonExistentFiles(t *testing.T) { t.Fatalf("watching more than 5 files: %d", n) } } + +// A directory the current user cannot read costs us visibility into that +// subtree, but it must not prevent the rest of the tree from being watched. +func TestWatchRecursivelySkipsUnreadableDir(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("permission semantics differ on windows") + } + if os.Geteuid() == 0 { + t.Skip("root bypasses the permission bit this test relies on") + } + + root := t.TempDir() + unreadable := filepath.Join(root, "unreadable") + if err := os.MkdirAll(filepath.Join(unreadable, "inner"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "watched.txt"), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chmod(unreadable, 0o000); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(unreadable, 0o755) }) + + if _, err := os.ReadDir(unreadable); err == nil { + t.Skip("could not make the directory unreadable in this environment") + } + + notify, err := NewWatcher([]string{root}) + assert.NilError(t, err) + t.Cleanup(func() { _ = notify.Close() }) + + if err := notify.Start(); err != nil { + t.Fatalf("Start() must not fail because of an unreadable directory: %v", err) + } +}