vercel/next.js · #97825

Fix Turbopack resolution through chained symlinks

marcoshernanz · merged Aug 25, 20268 files · 148 + / 14
test/production/app-dir/symbolic-links/README.md2 + / 1
@@ -5,4 +5,5 @@ like when run under a build orchestrator, such as Bazel, where its sandbox sets up source files as symlinks to their original source.  The `/directory-symlink` route covers resolving a module through a directory-symlink under `src`.+symlink under `src`. The `/directory-symlink-chain` route covers the same+resolution through two consecutive directory symlinks.
test/production/app-dir/symbolic-links/src/app/directory-symlink-chain/page.tsxadded5 + / 0
@@ -0,0 +1,5 @@+import message from '../../symlinked-package-chain/serialization'++export default function Page() {+  return <p>{message}</p>+}
test/production/app-dir/symbolic-links/src/symlinked-package-chainadded1 + / 0
@@ -0,0 +1 @@+symlinked-package\ No newline at end of file
test/production/app-dir/symbolic-links/symbolic-links.test.ts5 + / 0
@@ -16,4 +16,9 @@ describe('symbolic-links', () => {     const html = await next.render('/directory-symlink')     expect(html).toContain('hello from a directory symlink')   })++  it('should render a route that imports through chained directory symlinks', async () => {+    const html = await next.render('/directory-symlink-chain')+    expect(html).toContain('hello from a directory symlink')+  }) })
turbopack/crates/turbo-tasks-fs/src/content.rs16 + / 1
@@ -20,7 +20,7 @@ use turbo_tasks_hash::{ };  use crate::{-    FileSystemEntryType, FileSystemPath,+    FileSystemEntryType, FileSystemPath, RealPathErrorType,     json::UnparsableJson,     retry::retry_blocking,     rope::{Rope, RopeReader},@@ -200,6 +200,21 @@ impl LinkTarget {     pub async fn target_type(&self) -> Result<FileSystemEntryType> {         Ok(*self.file_system_path().get_type().await?)     }++    /// The type of the file this link ultimately points at.+    ///+    /// This follows a chain of links. A dangling link returns+    /// [`FileSystemEntryType::NotFound`], while any other unresolvable link returns+    /// [`FileSystemEntryType::Error`].+    pub async fn resolved_type(&self) -> Result<FileSystemEntryType> {+        match self.file_system_path().realpath().await? {+            Ok(path) => Ok(*path.get_type().await?),+            Err(error) => Ok(match error.kind() {+                RealPathErrorType::NotFound => FileSystemEntryType::NotFound,+                _ => FileSystemEntryType::Error,+            }),+        }+    } }  /// The contents of a symbolic link, as read from a filesystem. On Windows, this may be a junction
turbopack/crates/turbo-tasks-fs/src/disk.rs61 + / 0
@@ -1958,6 +1958,67 @@ mod tests {             tt.stop_and_wait().await;         } +        #[cfg(unix)]+        #[tokio::test(flavor = "multi_thread", worker_threads = 2)]+        async fn test_link_target_resolved_type_through_chain() {+            use std::os::unix::fs::symlink;++            let scratch = tempfile::tempdir().unwrap();+            let path = scratch.path().to_owned();+            create_dir_all(path.join("target-dir")).unwrap();+            File::create_new(path.join("target-file")).unwrap();+            symlink("target-dir", path.join("dir-inner")).unwrap();+            symlink("dir-inner", path.join("dir-outer")).unwrap();+            symlink("target-file", path.join("file-inner")).unwrap();+            symlink("file-inner", path.join("file-outer")).unwrap();+            symlink("../outside", path.join("invalid-inner")).unwrap();+            symlink("invalid-inner", path.join("invalid-outer")).unwrap();++            let root = canonicalize_to_rcstr(&path).unwrap();++            #[turbo_tasks::function(operation, root)]+            async fn assert_operation(+                fs: ResolvedVc<DiskFileSystem>,+                root_path: FileSystemPath,+            ) -> anyhow::Result<()> {+                for (input_path, expected_output) in [+                    ("dir-outer", FileSystemEntryType::Directory),+                    ("file-outer", FileSystemEntryType::File),+                    ("invalid-outer", FileSystemEntryType::Error),+                ] {+                    let link = fs.read_link(root_path.join(input_path)?).await?;+                    let LinkContent::Link { target } = &*link else {+                        anyhow::bail!("expected a valid link, got {link:?}");+                    };+                    assert_eq!(target.resolved_type().await?, expected_output);+                }++                Ok(())+            }++            let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(+                BackendOptions::default(),+                noop_backing_storage(),+            ));++            tt.run_once(async move {+                let fs = disk_file_system_operation(root)+                    .resolve()+                    .strongly_consistent()+                    .await?;++                assert_operation(fs, disk_file_system_root(fs))+                    .read_strongly_consistent()+                    .await?;++                anyhow::Ok(())+            })+            .await+            .unwrap();++            tt.stop_and_wait().await;+        }+         /// A relative target must stay inside the filesystem root at every step, not just at the         /// end. Both of these step above the root; one comes back into it and one doesn't, but         /// neither can be resolved against a root-relative [`FileSystemPath`], so `read_link`
turbopack/crates/turbo-tasks-fs/src/read_glob.rs51 + / 7
@@ -89,10 +89,15 @@ async fn read_glob_internal(                         // Skip links that leave the filesystem root.                         let link_content = path.read_link().await?;                         if let LinkContent::Link { target } = &*link_content {-                            if matches!(target.target_type().await?, FileSystemEntryType::Directory)+                            let Ok(realpath) = target.file_system_path().realpath().await? else {+                                // Preserve unresolvable symlinks that match the glob.+                                handle_file(&mut result, &entry_path, segment, entry);+                                continue;+                            };+                            if matches!(*realpath.get_type().await?, FileSystemEntryType::Directory)                             {-                                // Ensure that there are no infinite link loops, but don't resolve-                                resolve_symlink_safely(entry.clone()).await?;+                                // Reject links that point to an ancestor before recursing.+                                check_symlink_directory_recursion(path, &realpath)?;                                  // Add the directory to `results` if it is a whole match of the glob                                 handle_file(&mut result, &entry_path, segment, entry);@@ -124,14 +129,32 @@ async fn resolve_symlink_safely(entry: DirectoryEntry) -> Result<DirectoryEntry>         // Recursion can only occur if the symlink is a directory and points to an         // ancestor of the current path, which can be detected via a simple prefix         // match.-        let source_path = entry.path().unwrap();-        if source_path.is_inside_or_equal(&resolved_entry.clone().path().unwrap()) {-            bail!("'{source_path}' is a symlink causes that causes an infinite loop!",)-        }+        check_symlink_directory_recursion(+            &entry.path().unwrap(),+            &resolved_entry.clone().path().unwrap(),+        )?;     }     Ok(resolved_entry) } +fn check_symlink_directory_recursion(+    source_path: &FileSystemPath,+    realpath: &FileSystemPath,+) -> Result<()> {+    // We followed a symlink to a directory+    // To prevent an infinite loop, which in the case of turbo-tasks would simply+    // exhaust RAM or go into an infinite loop with the GC we need to check for a+    // recursive symlink, we need to check for recursion.++    // Recursion can only occur if the symlink is a directory and points to an+    // ancestor of the current path, which can be detected via a simple prefix+    // match.+    if source_path.is_inside_or_equal(realpath) {+        bail!("'{source_path}' is a symlink causes that causes an infinite loop!",)+    }+    Ok(())+}+ /// Traverses all directories that match the given `glob`. /// /// This ensures that the calling task will be invalidated@@ -361,6 +384,24 @@ pub mod tests {         );         assert_eq!(inner_sub_dir.inner.len(), 0); +        // A folder behind a symlink-to-symlink chain+        let read_dir = root+            .read_glob(Glob::new(rcstr!("sub/dir-chain/*"), GlobOptions::default()))+            .await+            .unwrap();+        assert_eq!(read_dir.results.len(), 0);+        let inner_sub = &*read_dir.inner.get("sub").unwrap().await?;+        assert_eq!(inner_sub.results.len(), 0);+        let inner_sub_dir = &*inner_sub.inner.get("dir-chain").unwrap().await?;+        assert_eq!(+            inner_sub_dir.results,+            HashMap::from_iter([(+                "index.js".into(),+                DirectoryEntry::File(root.join("sub/dir-chain/index.js")?),+            )])+        );+        assert_eq!(inner_sub_dir.inner.len(), 0);+         Ok(())     } @@ -450,6 +491,9 @@ pub mod tests {                 .write_all(b"dir index")                 .unwrap();             symlink(&dir, path.join("sub/dir")).unwrap();+            let dir_link = path.join("dir-link");+            symlink(&dir, &dir_link).unwrap();+            symlink(dir_link, path.join("sub/dir-chain")).unwrap();         }         let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new(             BackendOptions::default(),
turbopack/crates/turbopack-core/src/resolve/pattern.rs7 + / 5
@@ -1610,8 +1610,10 @@ pub async fn read_matches(                                 continue;                             };                             let path = concat(&prefix, str).into();-                            if matches!(target.target_type().await?, FileSystemEntryType::Directory)-                            {+                            if matches!(+                                target.resolved_type().await?,+                                FileSystemEntryType::Directory+                            ) {                                 results.push((index, PatternMatch::Directory(path, fs_path)));                             } else {                                 results.push((index, PatternMatch::File(path, fs_path)))@@ -1799,7 +1801,7 @@ pub async fn read_matches(                                         &*fs_path.read_link().await?                                     {                                         if matches!(-                                            target.target_type().await?,+                                            target.resolved_type().await?,                                             FileSystemEntryType::Directory                                         ) {                                             results.push((@@ -1823,7 +1825,7 @@ pub async fn read_matches(                                     if let LinkContent::Link { target } =                                         &*fs_path.read_link().await?                                         && matches!(-                                            target.target_type().await?,+                                            target.resolved_type().await?,                                             FileSystemEntryType::Directory                                         )                                     {@@ -1838,7 +1840,7 @@ pub async fn read_matches(                                     if let LinkContent::Link { target } =                                         &*fs_path.read_link().await?                                         && matches!(-                                            target.target_type().await?,+                                            target.resolved_type().await?,                                             FileSystemEntryType::Directory                                         )                                     {