Bump notify from 5.1.0 to 6.0.1 in /src/agent (#3195)

dependabot[bot] · Porges · web-flow · commit b9911206ace4 · 2023-07-19T23:57:46.000Z
* Bump notify from 5.1.0 to 6.0.1 in /src/agent Bumps [notify](https://github.com/notify-rs/notify) from 5.1.0 to 6.0.1. - [Release notes](https://github.com/notify-rs/notify/releases) - [Changelog](https://github.com/notify-rs/notify/blob/main/CHANGELOG.md) - [Commits](notify-rs/notify@notify-5.1.0...notify-6.0.1) --- updated-dependencies: - dependency-name: notify dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Update for new notify API which has renames * Add tests for expected behaviour * Disable crossbeam support as recommended for use with Tokio * Fix Windows behaviour * Pre-filter events * Log when ignoring unknown rename event --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: George Pollard <gpollard@microsoft.com>
diff --git a/src/agent/Cargo.lock b/src/agent/Cargo.lock
diff --git a/src/agent/debugger/Cargo.toml b/src/agent/debugger/Cargo.toml
@@ -27,6 +27,7 @@ features = [
     "processthreadsapi",
     "securitybaseapi",
     "shellapi",
+    "synchapi",
     "werapi",
     "winbase",
     "winerror"
diff --git a/src/agent/onefuzz/Cargo.toml b/src/agent/onefuzz/Cargo.toml
@@ -18,7 +18,7 @@ futures-util = "0.3"
 hex = "0.4"
 lazy_static = "1.4"
 log = "0.4"
-notify = "5.1.0"
+notify = { version = "6.0.1", default-features = false }
 regex = "1.9.1"
 reqwest = { version = "0.11", features = [
     "json",
diff --git a/src/agent/onefuzz/src/monitor.rs b/src/agent/onefuzz/src/monitor.rs
@@ -1,13 +1,13 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-use std::{
-    io::ErrorKind,
-    path::{Path, PathBuf},
-};
+use std::path::{Path, PathBuf};
 
-use anyhow::{format_err, Result};
-use notify::{Event, EventKind, Watcher};
+use anyhow::{format_err, Context, Result};
+use notify::{
+    event::{CreateKind, ModifyKind, RenameMode},
+    Event, EventKind, Watcher,
+};
 use tokio::{
     fs,
     sync::mpsc::{unbounded_channel, UnboundedReceiver},
@@ -41,12 +41,37 @@ impl DirectoryMonitor {
         }
 
         let (sender, notify_events) = unbounded_channel();
-        let event_handler = move |event_or_err| {
-            // A send error only occurs when the channel is closed. No remedial
-            // action is needed (or possible), so ignore it.
-            let _ = sender.send(event_or_err);
-        };
-        let mut watcher = notify::recommended_watcher(event_handler)?;
+        let mut watcher =
+            notify::recommended_watcher(move |event_or_err: notify::Result<Event>| {
+                // pre-filter the events here
+                let result = match event_or_err {
+                    Ok(ev) => match ev.kind {
+                        // we are interested in:
+                        // - create
+                        // - remove
+                        // - modify name
+                        EventKind::Create(_)
+                        | EventKind::Remove(_)
+                        | EventKind::Modify(ModifyKind::Name(_)) => Some(Ok(ev)),
+                        // we are not interested in:
+                        // - access
+                        // - modify something else (data, metadata)
+                        // - any other events
+                        EventKind::Access(_)
+                        | EventKind::Modify(_)
+                        | EventKind::Any
+                        | EventKind::Other => None,
+                    },
+                    Err(err) => Some(Err(err)),
+                };
+
+                if let Some(to_send) = result {
+                    // A send error only occurs when the channel is closed. No remedial
+                    // action is needed (or possible), so ignore it.
+                    let _ = sender.send(to_send);
+                }
+            })?;
+
         watcher.watch(&dir, RecursiveMode::NonRecursive)?;
 
         Ok(Self {
@@ -89,56 +114,87 @@ impl DirectoryMonitor {
                 }
             };
 
+            let mut paths = event.paths.into_iter();
+
             match event.kind {
-                EventKind::Create(..) => {
-                    let path = event
-                        .paths
-                        .get(0)
-                        .ok_or_else(|| format_err!("missing path for file create event"))?
-                        .clone();
-
-                    if self.report_directories {
-                        return Ok(Some(path));
+                EventKind::Create(create_kind) => {
+                    let path = paths
+                        .next()
+                        .ok_or_else(|| format_err!("missing path for file create event"))?;
+
+                    match create_kind {
+                        CreateKind::File => {
+                            return Ok(Some(path));
+                        }
+                        CreateKind::Folder => {
+                            if self.report_directories {
+                                return Ok(Some(path));
+                            }
+                        }
+                        CreateKind::Any | CreateKind::Other => {
+                            if self.report_directories {
+                                return Ok(Some(path));
+                            }
+
+                            // check if it is a file
+                            let metadata = fs::metadata(&path)
+                                .await
+                                .context("checking metadata for file")?;
+
+                            if metadata.is_file() {
+                                return Ok(Some(path));
+                            }
+                        }
                     }
+                }
+                EventKind::Modify(ModifyKind::Name(rename_mode)) => {
+                    match rename_mode {
+                        RenameMode::To => {
+                            let path = paths.next().ok_or_else(|| {
+                                format_err!("missing 'to' path for file rename-to event")
+                            })?;
 
-                    match fs::metadata(&path).await {
-                        Ok(metadata) if metadata.is_file() => {
                             return Ok(Some(path));
                         }
-                        Ok(_) => {
-                            // Ignore directories.
-                            continue;
+                        RenameMode::Both => {
+                            let _from = paths.next().ok_or_else(|| {
+                                format_err!("missing 'from' path for file rename event")
+                            })?;
+
+                            let to = paths.next().ok_or_else(|| {
+                                format_err!("missing 'to' path for file rename event")
+                            })?;
+
+                            return Ok(Some(to));
                         }
-                        Err(err) if err.kind() == ErrorKind::NotFound => {
-                            // Ignore if deleted.
-                            continue;
+                        RenameMode::From => {
+                            // ignore rename-from
                         }
-                        Err(err) => {
-                            warn!(
-                                "error checking metadata for file. path = {}, error = {}",
-                                path.display(),
-                                err
+                        RenameMode::Any | RenameMode::Other => {
+                            // something unusual, ignore
+                            info!(
+                                "unknown rename event: ignoring {:?} for path {:?}",
+                                rename_mode,
+                                paths.next()
                             );
-                            continue;
                         }
                     }
                 }
                 EventKind::Remove(..) => {
-                    let path = event
-                        .paths
-                        .get(0)
+                    let path = paths
+                        .next()
                         .ok_or_else(|| format_err!("missing path for file remove event"))?;
 
-                    if path == &self.dir {
+                    if path == self.dir {
                         // The directory we were watching was removed; we're done.
                         let _ = self.stop();
                         return Ok(None);
                     } else {
                         // Some file _inside_ the watched directory was removed. Ignore.
                     }
                 }
-                _event_kind => {
-                    // Other filesystem event. Ignore.
+                EventKind::Access(_) | EventKind::Modify(_) | EventKind::Other | EventKind::Any => {
+                    unreachable!() // these events have already been filtered out
                 }
             }
         }
diff --git a/src/agent/onefuzz/src/monitor/tests.rs b/src/agent/onefuzz/src/monitor/tests.rs
@@ -155,3 +155,76 @@ timed_test!(test_monitor_set_report_directories, async move {
 
     Ok(())
 });
+
+timed_test!(test_rename_into_dir, async move {
+    use std::fs::canonicalize;
+
+    let dir1 = tempdir().unwrap();
+    let dir2 = tempdir().unwrap();
+
+    // create file
+    let file1 = dir1.path().join("testfile");
+    fs::write(&file1, &"xxx").await?;
+
+    // start watching
+    let mut monitor = DirectoryMonitor::new(dir2.path()).await?;
+
+    // move into watched dir
+    let file2 = dir2.path().join("testfile");
+    fs::rename(&file1, &file2).await?;
+
+    // should get notification
+    assert_eq!(monitor.next_file().await?, Some(canonicalize(file2)?));
+
+    Ok(())
+});
+
+timed_test!(test_rename_inside_dir, async move {
+    use std::fs::canonicalize;
+
+    // create file
+    let dir = tempdir().unwrap();
+    let file1 = dir.path().join("testfile");
+    fs::write(&file1, &"xxx").await?;
+
+    // start watching
+    let mut monitor = DirectoryMonitor::new(dir.path()).await?;
+
+    // rename inside watched dir
+    let file2 = dir.path().join("testfile_2");
+    fs::rename(&file1, &file2).await?;
+
+    // should get notification
+    assert_eq!(monitor.next_file().await?, Some(canonicalize(&file2)?));
+
+    Ok(())
+});
+
+timed_test!(test_rename_out_of_dir, async move {
+    let dir1 = tempdir().unwrap();
+    let dir2 = tempdir().unwrap();
+
+    // create file
+    let file1 = dir1.path().join("testfile");
+    fs::write(&file1, &"xxx").await?;
+
+    // start watching
+    let mut monitor = DirectoryMonitor::new(dir1.path()).await?;
+
+    // move _out_ of watched dir
+    let file2 = dir2.path().join("testfile");
+    fs::rename(&file1, &file2).await?;
+
+    // TODO: on Windows, `notify` doesn't provide an event for the removal of a
+    // watched directory, so we can't proactively close our channel.
+    #[cfg(not(target_os = "windows"))]
+    {
+        dir1.close()?;
+        // shouldn't get any notification
+        assert_eq!(monitor.next_file().await?, None);
+    }
+
+    let _ = monitor.stop();
+
+    Ok(())
+});
