Skip to content

Commit e72f36a

Browse files
committed
fix(watch): skip unreadable directories instead of failing the watch
Signed-off-by: Endika Iglesias <endika2@gmail.com>
1 parent 2fa7a29 commit e72f36a

2 files changed

Lines changed: 50 additions & 0 deletions

File tree

pkg/watch/watcher_naive.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ func (d *naiveNotify) watchRecursively(dir string) error {
113113

114114
return filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error {
115115
if err != nil {
116+
// A directory we are not allowed to read is not a reason to abandon the
117+
// whole watch: we simply cannot see inside it, so skip it and carry on.
118+
if os.IsPermission(err) {
119+
logrus.Debugf("Not watching %s: %v", path, err)
120+
return filepath.SkipDir
121+
}
116122
return err
117123
}
118124

@@ -130,6 +136,10 @@ func (d *naiveNotify) watchRecursively(dir string) error {
130136
if os.IsNotExist(err) {
131137
return nil
132138
}
139+
if os.IsPermission(err) {
140+
logrus.Debugf("Not watching %s: %v", path, err)
141+
return filepath.SkipDir
142+
}
133143
return fmt.Errorf("watcher.Add(%q): %w", path, err)
134144
}
135145
return nil
@@ -180,6 +190,10 @@ func (d *naiveNotify) loop() { //nolint:gocyclo
180190
// TODO(dbentley): if there's a delete should we call d.watcher.Remove to prevent leaking?
181191
err := filepath.WalkDir(e.Name, func(path string, info fs.DirEntry, err error) error {
182192
if err != nil {
193+
if os.IsPermission(err) {
194+
logrus.Debugf("Not watching %s: %v", path, err)
195+
return filepath.SkipDir
196+
}
183197
return err
184198
}
185199

pkg/watch/watcher_naive_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,39 @@ func TestDontRecurseWhenWatchingParentsOfNonExistentFiles(t *testing.T) {
157157
t.Fatalf("watching more than 5 files: %d", n)
158158
}
159159
}
160+
161+
// A directory the current user cannot read costs us visibility into that
162+
// subtree, but it must not prevent the rest of the tree from being watched.
163+
func TestWatchRecursivelySkipsUnreadableDir(t *testing.T) {
164+
if runtime.GOOS == "windows" {
165+
t.Skip("permission semantics differ on windows")
166+
}
167+
if os.Geteuid() == 0 {
168+
t.Skip("root bypasses the permission bit this test relies on")
169+
}
170+
171+
root := t.TempDir()
172+
unreadable := filepath.Join(root, "unreadable")
173+
if err := os.MkdirAll(filepath.Join(unreadable, "inner"), 0o755); err != nil {
174+
t.Fatal(err)
175+
}
176+
if err := os.WriteFile(filepath.Join(root, "watched.txt"), []byte("x"), 0o600); err != nil {
177+
t.Fatal(err)
178+
}
179+
if err := os.Chmod(unreadable, 0o000); err != nil {
180+
t.Fatal(err)
181+
}
182+
t.Cleanup(func() { _ = os.Chmod(unreadable, 0o755) })
183+
184+
if _, err := os.ReadDir(unreadable); err == nil {
185+
t.Skip("could not make the directory unreadable in this environment")
186+
}
187+
188+
notify, err := NewWatcher([]string{root})
189+
assert.NilError(t, err)
190+
t.Cleanup(func() { _ = notify.Close() })
191+
192+
if err := notify.Start(); err != nil {
193+
t.Fatalf("Start() must not fail because of an unreadable directory: %v", err)
194+
}
195+
}

0 commit comments

Comments
 (0)