fix(turborepo): avoid globbing directories due to ancestor truncation (…

…#5273)

Co-authored-by: Greg Soltis <Greg Soltis>
Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
Co-authored-by: Nicholas Yang <nicholas.yang@vercel.com>

crates/turborepo-ffi/src/lib.rs

@@ -6,7 +6,7 @@ mod lockfile;
 
 use std::{collections::HashMap, mem::ManuallyDrop, path::PathBuf};
 
-use globwalk::{globwalk, WalkError};
+use globwalk::globwalk;
 pub use lockfile::{patches, subgraph, transitive_closure};
 use turbopath::{AbsoluteSystemPathBuf, AnchoredSystemPathBuf};
 use turborepo_env::EnvironmentVariableMap;
@@ -483,13 +483,13 @@ pub extern "C" fn glob(buffer: Buffer) -> Buffer {
         false => globwalk::WalkType::All,
     };
 
-    let iter = match globwalk(
+    let files = match globwalk(
         &AbsoluteSystemPathBuf::new(req.base_path).expect("absolute"),
         &req.include_patterns,
         &req.exclude_patterns,
         walk_type,
     ) {
-        Ok(iter) => iter,
+        Ok(files) => files,
         Err(err) => {
             let resp = proto::GlobResp {
                 response: Some(proto::glob_resp::Response::Error(err.to_string())),
@@ -498,17 +498,8 @@ pub extern "C" fn glob(buffer: Buffer) -> Buffer {
         }
     };
 
-    let paths = match iter.collect::<Result<Vec<_>, WalkError>>() {
-        Ok(paths) => paths,
-        Err(err) => {
-            let resp = proto::GlobResp {
-                response: Some(proto::glob_resp::Response::Error(err.to_string())),
-            };
-            return resp.into();
-        }
-    };
     // TODO: is to_string_lossy the right thing to do here? We could error...
-    let files: Vec<_> = paths
+    let files: Vec<_> = files
         .into_iter()
         .map(|path| path.to_string_lossy().to_string())
         .collect();

crates/turborepo-globwalk/src/lib.rs

@@ -4,14 +4,15 @@ mod empty_glob;
 
 use std::{
     borrow::Cow,
+    collections::HashSet,
     io::ErrorKind,
     path::{Path, PathBuf},
 };
 
 use empty_glob::InclusiveEmptyAny;
 use itertools::Itertools;
 use path_slash::PathExt;
-use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf};
+use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathError};
 use wax::{Any, BuildError, Glob, Pattern};
 
 #[derive(Debug, PartialEq, Clone, Copy)]
@@ -50,6 +51,12 @@ pub enum WalkError {
     InvalidPath,
     #[error("walk error: {0}")]
     WalkError(#[from] walkdir::Error),
+    #[error(transparent)]
+    Path(#[from] PathError),
+    #[error(transparent)]
+    WaxWalk(#[from] wax::WalkError),
+    #[error("Internal error on glob {glob}: {error}")]
+    InternalError { glob: String, error: String },
 }
 
 /// Performs a glob walk, yielding paths that _are_ included in the include list
@@ -62,7 +69,7 @@ pub enum WalkError {
 ///       - collapse the path, and calculate the new base_path, which defined as
 ///         the longest common prefix of all the includes
 ///       - traversing above the root of the base_path is not allowed
-pub fn globwalk(
+pub fn _globwalk(
     base_path: &AbsoluteSystemPath,
     include: &[String],
     exclude: &[String],
@@ -277,6 +284,74 @@ fn collapse_path(path: &str) -> Option<(Cow<str>, usize)> {
     }
 }
 
+pub fn globwalk(
+    base_path: &AbsoluteSystemPath,
+    include: &[String],
+    exclude: &[String],
+    walk_type: WalkType,
+) -> Result<HashSet<AbsoluteSystemPathBuf>, WalkError> {
+    let (base_path_new, include_paths, exclude_paths) =
+        preprocess_paths_and_globs(base_path, include, exclude)?;
+    let inc_patterns = include_paths
+        .iter()
+        .map(|g| Glob::new(g.as_str()).map_err(|e| e.into()))
+        .collect::<Result<Vec<_>, WalkError>>()?;
+    let ex_patterns = exclude_paths
+        .iter()
+        .map(|g| Glob::new(g.as_str()))
+        .collect::<Result<Vec<_>, _>>()?;
+
+    let result = inc_patterns
+        .into_iter()
+        .flat_map(|glob| {
+            // Check if the glob specifies an exact filename with no meta characters.
+            if let Some(prefix) = glob.variance().path() {
+                // We expect all of our globs to be absolute paths (asserted above)
+                assert!(prefix.is_absolute(), "Found relative glob path {}", glob);
+                // We're either going to return this path or nothing. Check if it's a directory
+                // and if we want directories
+                match AbsoluteSystemPathBuf::new(prefix).and_then(|path| {
+                    let metadata = path.symlink_metadata()?;
+                    Ok((path, metadata))
+                }) {
+                    Err(e) if e.is_io_error(ErrorKind::NotFound) => {
+                        // If the file doesn't exist, it's not an error, there's just nothing to
+                        // glob
+                        vec![]
+                    }
+                    Err(e) => vec![Err(e.into())],
+                    Ok((_, md)) if walk_type == WalkType::Files && md.is_dir() => {
+                        vec![]
+                    }
+                    Ok((path, _)) => vec![Ok(path)],
+                }
+            } else {
+                glob.walk(&base_path_new)
+                    .not(ex_patterns.iter().cloned())
+                    // Per docs, only fails if exclusion list is too large, since we're using
+                    // pre-compiled globs
+                    .unwrap_or_else(|e| {
+                        panic!(
+                            "Failed to compile exclusion globs: {:?}: {}",
+                            ex_patterns, e,
+                        )
+                    })
+                    .filter_map(|entry| match entry {
+                        Ok(entry) if walk_type == WalkType::Files && entry.file_type().is_dir() => {
+                            None
+                        }
+                        Ok(entry) => {
+                            Some(AbsoluteSystemPathBuf::new(entry.path()).map_err(|e| e.into()))
+                        }
+                        Err(e) => Some(Err(e.into())),
+                    })
+                    .collect::<Vec<_>>()
+            }
+        })
+        .collect::<Result<HashSet<_>, WalkError>>()?;
+    Ok(result)
+}
+
 #[cfg(test)]
 mod test {
     use std::{collections::HashSet, path::Path};
@@ -512,7 +587,9 @@ mod test {
     #[test_case("**/*.txt", 1, 1 => matches None)]
     #[test_case("**/【*", 1, 1 => matches None)]
     // in the go implementation, broken-symlink is yielded,
-    // however in symlink mode, walkdir yields broken symlinks as errors
+    // however in symlink mode, walkdir yields broken symlinks as errors.
+    // Note that walkdir _always_ follows root symlinks. We handle this in the layer
+    // above wax.
     #[test_case("broken-symlink", 1, 1 => matches None ; "broken symlinks should be yielded")]
     // globs that match across a symlink should not follow the symlink
     #[test_case("working-symlink/c/*", 0, 0 => matches None ; "working symlink should not be followed")]
@@ -556,11 +633,10 @@ mod test {
         let dir = setup();
 
         let path = AbsoluteSystemPathBuf::new(dir.path()).unwrap();
-        let (success, _error): (Vec<AbsoluteSystemPathBuf>, Vec<_>) =
-            match super::globwalk(&path, &[pattern.into()], &[], crate::WalkType::All) {
-                Ok(e) => e.into_iter().partition_result(),
-                Err(e) => return Some(e),
-            };
+        let success = match super::globwalk(&path, &[pattern.into()], &[], crate::WalkType::All) {
+            Ok(e) => e.into_iter(),
+            Err(e) => return Some(e),
+        };
 
         assert_eq!(
             success.len(),
@@ -1128,10 +1204,7 @@ mod test {
             (crate::WalkType::Files, expected_files),
             (crate::WalkType::All, expected),
         ] {
-            let (success, _): (Vec<AbsoluteSystemPathBuf>, Vec<_>) =
-                super::globwalk(&path, &include, &exclude, walk_type)
-                    .unwrap()
-                    .partition_result();
+            let success = super::globwalk(&path, &include, &exclude, walk_type).unwrap();
 
             let success = success
                 .iter()
@@ -1220,12 +1293,11 @@ mod test {
         let child = root.join_component("child");
         let include = &["../*-file".to_string()];
         let exclude = &[];
-        let iter = globwalk(&child, include, exclude, WalkType::Files).unwrap();
+        let iter = globwalk(&child, include, exclude, WalkType::Files)
+            .unwrap()
+            .into_iter();
         let results = iter
-            .map(|entry| {
-                let entry = entry.unwrap();
-                root.anchor(entry).unwrap().to_str().unwrap().to_string()
-            })
+            .map(|entry| root.anchor(entry).unwrap().to_str().unwrap().to_string())
             .collect::<Vec<_>>();
         let expected = vec!["root-file".to_string()];
         assert_eq!(results, expected);
@@ -1250,8 +1322,8 @@ mod test {
         let exclude = &["apps/ignored".to_string(), "**/node_modules/**".to_string()];
         let iter = globwalk(&root, include, exclude, WalkType::Files).unwrap();
         let paths = iter
+            .into_iter()
             .map(|path| {
-                let path = path.unwrap();
                 let relative = root.anchor(path).unwrap();
                 relative.to_str().unwrap().to_string()
             })

crates/turborepo-lib/src/package_manager/mod.rs

@@ -394,19 +394,16 @@ impl PackageManager {
     pub fn get_package_jsons(
         &self,
         repo_root: &AbsoluteSystemPath,
-    ) -> Result<Vec<AbsoluteSystemPathBuf>, Error> {
+    ) -> Result<impl Iterator<Item = AbsoluteSystemPathBuf>, Error> {
         let globs = self.get_workspace_globs(repo_root)?;
 
-        let walker = globwalk::globwalk(
+        let files = globwalk::globwalk(
             repo_root,
             &globs.package_json_inclusions,
             &globs.raw_exclusions,
             globwalk::WalkType::Files,
         )?;
-        let items = walker
-            .map(|result| result.map_err(|e| e.into()))
-            .collect::<Result<Vec<_>, Error>>()?;
-        Ok(items)
+        Ok(files.into_iter())
     }
 }
 

crates/turborepo-scm/src/package_deps.rs

@@ -65,15 +65,15 @@ pub fn get_package_file_hashes_from_inputs<S: AsRef<str>>(
                 Either::Left([package_unix_path, raw_glob.as_ref()].join("/"))
             }
         });
-    let iter = globwalk::globwalk(
+    let files = globwalk::globwalk(
         turbo_root,
         &inclusions,
         &exclusions,
         globwalk::WalkType::Files,
     )?;
-    let to_hash = iter
+    let to_hash = files
+        .iter()
         .map(|entry| {
-            let entry = entry?;
             let path = git_root.anchor(entry)?.to_unix()?;
             Ok(path)
         })

crates/turborepo-wax/src/walk.rs

@@ -143,11 +143,15 @@ macro_rules! walk {
                 .strip_prefix(&$state.prefix)
                 .expect("path is not in tree");
             let depth = entry.depth().saturating_sub(1);
+            // Globs don't include the root token, but absolute paths do.
+            // Skip that token so that matching up components will work below.
             for candidate in path
                 .components()
+                .filter(|c| !matches!(c, Component::RootDir))
                 .skip(depth)
                 .filter_map(|component| match component {
                     Component::Normal(component) => Some(CandidatePath::from(component)),
+                    Component::Prefix(component) => Some(CandidatePath::from(component.as_os_str())),
                     _ => None,
                 })
                 .zip_longest($state.components.iter().skip(depth))
@@ -570,9 +574,22 @@ pub struct Walk<'g> {
     root: PathBuf,
     prefix: PathBuf,
     walk: walkdir::IntoIter,
+    // This is a hack to express an empty iterator
+    is_empty: bool,
 }
 
 impl<'g> Walk<'g> {
+    fn empty() -> Self {
+        Self {
+            pattern: Cow::Owned(Regex::new("").unwrap()),
+            components: vec![],
+            root: PathBuf::new(),
+            prefix: PathBuf::new(),
+            walk: walkdir::WalkDir::new(PathBuf::new()).into_iter(),
+            is_empty: true,
+        }
+    }
+
     fn compile<'t, I>(tokens: I) -> Result<Vec<Regex>, CompileError>
     where
         I: IntoIterator<Item = &'t Token<'t>>,
@@ -602,13 +619,15 @@ impl<'g> Walk<'g> {
             root,
             prefix,
             walk,
+            is_empty,
         } = self;
         Walk {
             pattern: Cow::Owned(pattern.into_owned()),
             components,
             root,
             prefix,
             walk,
+            is_empty,
         }
     }
 
@@ -704,6 +723,9 @@ impl Iterator for Walk<'_> {
     type Item = WalkItem<'static>;
 
     fn next(&mut self) -> Option<Self::Item> {
+        if self.is_empty {
+            return None;
+        }
         walk!(self => |entry| {
             return Some(entry.map(WalkEntry::into_owned));
         });
@@ -907,6 +929,24 @@ pub fn walk<'g>(
             }
         },
     );
+    if matches!(link, LinkBehavior::ReadFile) {
+        if let Ok(tail) = root.strip_prefix(directory) {
+            let found = tail
+                .components()
+                .try_fold(directory.to_path_buf(), |accum, c| {
+                    let candidate = accum.join(c);
+                    if candidate.is_symlink() {
+                        None
+                    } else {
+                        Some(candidate)
+                    }
+                })
+                .is_none();
+            if found {
+                return Walk::empty();
+            }
+        }
+    }
     let components =
         Walk::compile(glob.tree.as_ref().tokens()).expect("failed to compile glob sub-expressions");
     Walk {
@@ -921,6 +961,7 @@ pub fn walk<'g>(
             })
             .max_depth(depth)
             .into_iter(),
+        is_empty: false,
     }
 }