kqueue: don't set up watchers on unreadable files (#479)

Watching an entire directory would fail if it contains a file we can't
read due to permission errors. That's obviously not ideal, so just skip
the file.

Also add a test for trying to watch a directory where the permissions
are denied, and test the error is what's expected; this prevents
regressions like in #393.

Fixes #439

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index e926f07067e5d42cc5ff2bb68241d56f81201df6..15bae7ade114376f15477bad3528836c77eb73c7 100644 (file)
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -42,7 +42,8 @@ jobs:
           usesh: true
           prepare: pkg install -y go
           run: |
-            go test -race ./...
+            pw user add -n action -m
+            su action -c 'go test -race ./...'
 
   testOpenBSD:
     runs-on: macos-12
@@ -54,18 +55,18 @@ jobs:
         uses: vmactions/openbsd-vm@v0.0.6
         with:
           prepare: pkg_add go
+          # No -race as the VM doesn't include the comp set.
+          #
+          # TODO: should probably add this, but on my local machine the tests
+          #       time out with -race as the waits aren't long enough (OpenBSD
+          #       is kind of slow), so should probably look into that first.
+          #       Go 1.19 is supposed to have a much faster race detector, so
+          #       maybe waiting until we have that is enough.
           run: |
             # Default of 512 leads to "too many open files".
             ulimit -n 1024
-
-            # No -race as the VM doesn't include the comp set.
-            #
-            # TODO: should probably add this, but on my local machine the tests
-            #       time out with -race as the waits aren't long enough (OpenBSD
-            #       is kind of slow), so should probably look into that first.
-            #       Go 1.19 is supposed to have a much faster race detector, so
-            #       maybe waiting until we have that is enough.
-            go test ./...
+            useradd -mG wheel action
+            su action -c 'go test ./...'
 
   testNetBSD:
     runs-on: macos-12
@@ -77,9 +78,10 @@ jobs:
         uses: vmactions/netbsd-vm@v0.0.4
         with:
           prepare: pkg_add go
+          # TODO: no -race for the same reason as OpenBSD (the timing; it does run).
           run: |
-            # TODO: no -race for the same reason as OpenBSD (the timing; it does run).
-            go117 test ./...
+            useradd -mG wheel action
+            su action -c 'go117 test ./...'
 
   testillumos:
     runs-on: macos-12

diff --git a/integration_test.go b/integration_test.go
index 3367e8044e9dc98b68a3c82aae87f88e8d3ceaad..da9bb20d7ad6427639437e3df6c1d66ed501bc6c 100644 (file)
--- a/integration_test.go
+++ b/integration_test.go
@@ -4,6 +4,7 @@
 package fsnotify
 
 import (
+       "errors"
        "fmt"
        "path/filepath"
        "runtime"
@@ -11,6 +12,8 @@ import (
        "sync/atomic"
        "testing"
        "time"
+
+       "github.com/fsnotify/fsnotify/internal"
 )
 
 func TestWatch(t *testing.T) {
@@ -79,6 +82,31 @@ func TestWatch(t *testing.T) {
                                remove /sub
                                remove /file
                `},
+
+               {"file in directory is not readable", func(t *testing.T, w *Watcher, tmp string) {
+                       if runtime.GOOS == "windows" {
+                               t.Skip("attributes don't work on Windows")
+                       }
+
+                       touch(t, tmp, "file-unreadable")
+                       chmod(t, 0, tmp, "file-unreadable")
+                       touch(t, tmp, "file")
+                       addWatch(t, w, tmp)
+
+                       cat(t, "hello", tmp, "file")
+                       rm(t, tmp, "file")
+                       rm(t, tmp, "file-unreadable")
+               }, `
+                       WRITE     "/file"
+                       REMOVE    "/file"
+                       REMOVE    "/file-unreadable"
+
+                       # We never set up a watcher on the unreadable file, so we don't get
+                       # the REMOVE.
+                       kqueue:
+                WRITE    "/file"
+                REMOVE   "/file"
+               `},
        }
 
        for _, tt := range tests {
@@ -445,6 +473,38 @@ func TestClose(t *testing.T) {
        })
 }
 
+func TestAdd(t *testing.T) {
+       t.Run("permission denied", func(t *testing.T) {
+               if runtime.GOOS == "windows" {
+                       t.Skip("attributes don't work on Windows")
+               }
+
+               t.Parallel()
+
+               tmp := t.TempDir()
+               dir := filepath.Join(tmp, "dir-unreadable")
+               mkdir(t, dir)
+               touch(t, dir, "/file")
+               chmod(t, 0, dir)
+
+               w := newWatcher(t)
+               defer func() {
+                       w.Close()
+                       chmod(t, 0o755, dir) // Make TempDir() cleanup work
+               }()
+               err := w.Add(dir)
+               if err == nil {
+                       t.Fatal("error is nil")
+               }
+               if !errors.Is(err, internal.UnixEACCES) {
+                       t.Errorf("not unix.EACCESS: %T %#[1]v", err)
+               }
+               if !errors.Is(err, internal.SyscallEACCES) {
+                       t.Errorf("not syscall.EACCESS: %T %#[1]v", err)
+               }
+       })
+}
+
 // TODO: should also check internal state is correct/cleaned up; e.g. no
 //       left-over file descriptors or whatnot.
 func TestRemove(t *testing.T) {

diff --git a/internal/unix.go b/internal/unix.go
new file mode 100644 (file)
index 0000000..7c73a9d
--- /dev/null
+++ b/internal/unix.go
@@ -0,0 +1,15 @@
+//go:build !windows
+// +build !windows
+
+package internal
+
+import (
+       "syscall"
+
+       "golang.org/x/sys/unix"
+)
+
+var (
+       SyscallEACCES = syscall.EACCES
+       UnixEACCES    = unix.EACCES
+)

diff --git a/internal/windows.go b/internal/windows.go
new file mode 100644 (file)
index 0000000..6f20859
--- /dev/null
+++ b/internal/windows.go
@@ -0,0 +1,14 @@
+//go:build windows
+// +build windows
+
+package internal
+
+import (
+       "errors"
+)
+
+// Just a dummy.
+var (
+       SyscallEACCES = errors.New("dummy")
+       UnixEACCES    = errors.New("dummy")
+)

diff --git a/kqueue.go b/kqueue.go
index c63e7b6118e436a9b132754b92768665358d7f87..765e85f86752137e7ce2d60b55a74ddeda51553c 100644 (file)
--- a/kqueue.go
+++ b/kqueue.go
@@ -199,13 +199,8 @@ func (w *Watcher) addWatch(name string, flags uint32) (string, error) {
                        return "", err
                }
 
-               // Don't watch sockets.
-               if fi.Mode()&os.ModeSocket == os.ModeSocket {
-                       return "", nil
-               }
-
-               // Don't watch named pipes.
-               if fi.Mode()&os.ModeNamedPipe == os.ModeNamedPipe {
+               // Don't watch sockets or named pipes
+               if (fi.Mode()&os.ModeSocket == os.ModeSocket) || (fi.Mode()&os.ModeNamedPipe == os.ModeNamedPipe) {
                        return "", nil
                }
 
@@ -427,14 +422,24 @@ func (w *Watcher) watchDirectoryFiles(dirPath string) error {
        }
 
        for _, fileInfo := range files {
-               filePath := filepath.Join(dirPath, fileInfo.Name())
-               filePath, err = w.internalWatch(filePath, fileInfo)
+               path := filepath.Join(dirPath, fileInfo.Name())
+
+               cleanPath, err := w.internalWatch(path, fileInfo)
                if err != nil {
-                       return fmt.Errorf("%q: %w", filepath.Join(dirPath, fileInfo.Name()), err)
+                       // No permission to read the file; that's not a problem: just skip.
+                       // But do add it to w.fileExists to prevent it from being picked up
+                       // as a "new" file later (it still shows up in the directory
+                       // listing).
+                       switch {
+                       case errors.Is(err, unix.EACCES) || errors.Is(err, unix.EPERM):
+                               cleanPath = filepath.Clean(path)
+                       default:
+                               return fmt.Errorf("%q: %w", filepath.Join(dirPath, fileInfo.Name()), err)
+                       }
                }
 
                w.mu.Lock()
-               w.fileExists[filePath] = struct{}{}
+               w.fileExists[cleanPath] = struct{}{}
                w.mu.Unlock()
        }