From 9453c6ca8cb46b5481fb7e9bc1f692b594dd1f0d Mon Sep 17 00:00:00 2001 From: ManManavadaria Date: Fri, 24 Apr 2026 19:07:21 +0000 Subject: [PATCH 1/2] fix: handle recursive watcher ignore paths correctly Signed-off-by: ManManavadaria --- pkg/compose/watch.go | 12 +++++++++--- pkg/watch/notify.go | 4 ++-- pkg/watch/notify_test.go | 2 +- pkg/watch/watcher_darwin.go | 2 +- pkg/watch/watcher_naive.go | 34 ++++++++++++++++++++++++++++++++-- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/pkg/compose/watch.go b/pkg/compose/watch.go index f63c51683ab..1e22c9479a6 100644 --- a/pkg/compose/watch.go +++ b/pkg/compose/watch.go @@ -198,8 +198,9 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti eg, ctx := errgroup.WithContext(ctx) var ( - rules []watchRule - paths []string + rules []watchRule + paths []string + ignoreMatchers []watch.PathMatcher ) for serviceName, service := range project.Services { config, err := loadDevelopmentConfig(service, project) @@ -254,6 +255,11 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti } } } + ignore, err := watch.NewDockerPatternMatcher(trigger.Path, trigger.Ignore) + if err != nil { + return nil, err + } + ignoreMatchers = append(ignoreMatchers, ignore) paths = append(paths, trigger.Path) } @@ -268,7 +274,7 @@ func (s *composeService) watch(ctx context.Context, project *types.Project, opti return nil, fmt.Errorf("none of the selected services is configured for watch, consider setting a 'develop' section") } - watcher, err := watch.NewWatcher(paths) + watcher, err := watch.NewWatcher(paths, watch.NewCompositeMatcher(ignoreMatchers...)) if err != nil { return nil, err } diff --git a/pkg/watch/notify.go b/pkg/watch/notify.go index d63f5caf28b..71dcaf1156f 100644 --- a/pkg/watch/notify.go +++ b/pkg/watch/notify.go @@ -80,8 +80,8 @@ func (EmptyMatcher) MatchesEntireDir(f string) (bool, error) { return false, nil var _ PathMatcher = EmptyMatcher{} -func NewWatcher(paths []string) (Notify, error) { - return newWatcher(paths) +func NewWatcher(paths []string, ignore PathMatcher) (Notify, error) { + return newWatcher(paths, ignore) } const WindowsBufferSizeEnvVar = "COMPOSE_WATCH_WINDOWS_BUFFER_SIZE" diff --git a/pkg/watch/notify_test.go b/pkg/watch/notify_test.go index b1ec9b9644d..0e9a7bc63bf 100644 --- a/pkg/watch/notify_test.go +++ b/pkg/watch/notify_test.go @@ -526,7 +526,7 @@ func (f *notifyFixture) rebuildWatcher() { } // create a new watcher - notify, err := NewWatcher(f.paths) + notify, err := NewWatcher(f.paths, EmptyMatcher{}) if err != nil { f.T().Fatal(err) } diff --git a/pkg/watch/watcher_darwin.go b/pkg/watch/watcher_darwin.go index a63612aedff..9486b9af375 100644 --- a/pkg/watch/watcher_darwin.go +++ b/pkg/watch/watcher_darwin.go @@ -115,7 +115,7 @@ func (d *fseventNotify) Errors() chan error { return d.errors } -func newWatcher(paths []string) (Notify, error) { +func newWatcher(paths []string, _ PathMatcher) (Notify, error) { dw := &fseventNotify{ stream: &fsevents.EventStream{ Latency: 50 * time.Millisecond, diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 7798025e8bc..135e058ce35 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -45,6 +45,7 @@ type naiveNotify struct { // the notify list. It might be better to store this in a tree // structure, so we can filter the list quickly. notifyList map[string]bool + ignore PathMatcher isWatcherRecursive bool watcher *fsnotify.Watcher @@ -113,6 +114,10 @@ func (d *naiveNotify) watchRecursively(dir string) error { return filepath.WalkDir(dir, func(path string, info fs.DirEntry, err error) error { if err != nil { + if os.IsPermission(err) && d.shouldIgnore(path) { + logrus.Debugf("Ignoring path: %s", path) + return nil + } return err } @@ -240,6 +245,11 @@ func (d *naiveNotify) shouldSkipDir(path string) bool { return false } + // If path is present in the ignore list then we should ignore it + if d.shouldIgnore(path) { + return true + } + // Suppose we're watching // /src/.tiltignore // but the .tiltignore file doesn't exist. @@ -260,6 +270,26 @@ func (d *naiveNotify) shouldSkipDir(path string) bool { return true } +func (d *naiveNotify) shouldIgnore(path string) bool { + if d.ignore == nil { + return false + } + matches, err := d.ignore.MatchesEntireDir(path) + if err != nil { + logrus.Debugf("error checking ignored directory %q: %v", path, err) + return false + } + if matches { + return true + } + matches, err = d.ignore.Matches(path) + if err != nil { + logrus.Debugf("error checking ignored path %q: %v", path, err) + return false + } + return matches +} + func (d *naiveNotify) add(path string) error { err := d.watcher.Add(path) if err != nil { @@ -270,7 +300,7 @@ func (d *naiveNotify) add(path string) error { return nil } -func newWatcher(paths []string) (Notify, error) { +func newWatcher(paths []string, ignore PathMatcher) (Notify, error) { fsw, err := fsnotify.NewWatcher() if err != nil { if strings.Contains(err.Error(), "too many open files") && runtime.GOOS == "linux" { @@ -297,9 +327,9 @@ func newWatcher(paths []string) (Notify, error) { } notifyList[path] = true } - wmw := &naiveNotify{ notifyList: notifyList, + ignore: ignore, watcher: fsw, events: fsw.Events, wrappedEvents: wrappedEvents, From bba54a8e77854a37dbe875541c3de3a16b551174 Mon Sep 17 00:00:00 2001 From: ManManavadaria Date: Wed, 6 May 2026 19:09:44 +0000 Subject: [PATCH 2/2] fix: protect nested watch-root traversal from cross-trigger ignores and apply review fixes Signed-off-by: ManManavadaria --- pkg/watch/watcher_darwin.go | 2 +- pkg/watch/watcher_naive.go | 39 ++++++++++++++++++------ pkg/watch/watcher_naive_test.go | 54 +++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/pkg/watch/watcher_darwin.go b/pkg/watch/watcher_darwin.go index 9486b9af375..e4b6552eba5 100644 --- a/pkg/watch/watcher_darwin.go +++ b/pkg/watch/watcher_darwin.go @@ -115,7 +115,7 @@ func (d *fseventNotify) Errors() chan error { return d.errors } -func newWatcher(paths []string, _ PathMatcher) (Notify, error) { +func newWatcher(paths []string, _ ...PathMatcher) (Notify, error) { dw := &fseventNotify{ stream: &fsevents.EventStream{ Latency: 50 * time.Millisecond, diff --git a/pkg/watch/watcher_naive.go b/pkg/watch/watcher_naive.go index 135e058ce35..0ccd1f33cec 100644 --- a/pkg/watch/watcher_naive.go +++ b/pkg/watch/watcher_naive.go @@ -116,7 +116,7 @@ func (d *naiveNotify) watchRecursively(dir string) error { if err != nil { if os.IsPermission(err) && d.shouldIgnore(path) { logrus.Debugf("Ignoring path: %s", path) - return nil + return filepath.SkipDir } return err } @@ -185,6 +185,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) && d.shouldIgnore(path) { + logrus.Debugf("Ignoring path: %s", path) + return filepath.SkipDir + } return err } @@ -245,11 +249,6 @@ func (d *naiveNotify) shouldSkipDir(path string) bool { return false } - // If path is present in the ignore list then we should ignore it - if d.shouldIgnore(path) { - return true - } - // Suppose we're watching // /src/.tiltignore // but the .tiltignore file doesn't exist. @@ -262,15 +261,28 @@ func (d *naiveNotify) shouldSkipDir(path string) bool { // - A child of a directory that's in our notify list, or // - A parent of a directory that's in our notify list // (i.e., to cover the "path doesn't exist" case). + // + // We prioritize "parent of watched path" checks before ignore checks so + // one trigger's ignore rules can't hide another trigger's nested watch root. + isChildOfWatchedDir := false for root := range d.notifyList { - if pathutil.IsChild(root, path) || pathutil.IsChild(path, root) { + if pathutil.IsChild(path, root) { return false } + if pathutil.IsChild(root, path) { + isChildOfWatchedDir = true + } } - return true + + // skip the dir if ignore rules match this full subtree. + if d.shouldIgnoreEntireDir(path) { + return true + } + + return !isChildOfWatchedDir } -func (d *naiveNotify) shouldIgnore(path string) bool { +func (d *naiveNotify) shouldIgnoreEntireDir(path string) bool { if d.ignore == nil { return false } @@ -282,7 +294,14 @@ func (d *naiveNotify) shouldIgnore(path string) bool { if matches { return true } - matches, err = d.ignore.Matches(path) + return false +} + +func (d *naiveNotify) shouldIgnore(path string) bool { + if d.ignore == nil { + return false + } + matches, err := d.ignore.Matches(path) if err != nil { logrus.Debugf("error checking ignored path %q: %v", path, err) return false diff --git a/pkg/watch/watcher_naive_test.go b/pkg/watch/watcher_naive_test.go index b188de93903..ede4c0d7ab9 100644 --- a/pkg/watch/watcher_naive_test.go +++ b/pkg/watch/watcher_naive_test.go @@ -157,3 +157,57 @@ func TestDontRecurseWhenWatchingParentsOfNonExistentFiles(t *testing.T) { t.Fatalf("watching more than 5 files: %d", n) } } + +func TestShouldSkipDirWithNegatedChildException(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n!bazel-bin/app-binary\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: ignore, + notifyList: map[string]bool{repoRoot: true}, + } + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, !d.shouldSkipDir(bazelBin), "expected bazel-bin to remain traversable for negated child patterns") +} + +func TestShouldIgnorePathStillMatchesDirectoryPattern(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n!bazel-bin/app-binary\n") + assert.NilError(t, err) + + d := &naiveNotify{ignore: ignore} + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, d.shouldIgnore(bazelBin), "expected bazel-bin path to match ignore pattern") +} + +func TestShouldSkipDirForIgnoredSubtreeWithoutException(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "bazel-bin/\n") + assert.NilError(t, err) + + d := &naiveNotify{ + ignore: ignore, + notifyList: map[string]bool{repoRoot: true}, + } + + bazelBin := filepath.Join(repoRoot, "bazel-bin") + assert.Assert(t, d.shouldSkipDir(bazelBin), "expected fully ignored directory subtree to be skipped") +} + +func TestShouldSkipDirDoesNotSkipAncestorOfWatchedPath(t *testing.T) { + repoRoot := t.TempDir() + ignore, err := DockerIgnoreTesterFromContents(repoRoot, "parent/\n") + assert.NilError(t, err) + + watchedPath := filepath.Join(repoRoot, "parent", "child", "non-existent") + d := &naiveNotify{ + ignore: ignore, + notifyList: map[string]bool{watchedPath: true}, + } + + parent := filepath.Join(repoRoot, "parent") + assert.Assert(t, !d.shouldSkipDir(parent), "expected parent directory to remain traversable when it contains a watched path") +}