fix(compile): inline symlinks as files outside node_modules dir and warn for directories (#19285)

If a symlink within the `node_modules` directory lies outside that
directory, it will now warn and inline the file. For directories, it
will just warn for now.

Probably fixes #19251 (I'm still unable to reproduce).

6 files changed, 193 insertions, 39 deletions

diff --git a/cli/standalone/virtual_fs.rs b/cli/standalone/virtual_fs.rs
index e94758bd9..42416554d 100644
--- a/cli/standalone/virtual_fs.rs
+++ b/cli/standalone/virtual_fs.rs
@@ -24,9 +24,19 @@ use deno_runtime::deno_io::fs::FsResult;
 use deno_runtime::deno_io::fs::FsStat;
 use serde::Deserialize;
 use serde::Serialize;
+use thiserror::Error;
 
 use crate::util;
 
+#[derive(Error, Debug)]
+#[error(
+  "Failed to strip prefix '{}' from '{}'", root_path.display(), target.display()
+)]
+pub struct StripRootError {
+  root_path: PathBuf,
+  target: PathBuf,
+}
+
 pub struct VfsBuilder {
   root_path: PathBuf,
   root_dir: VirtualDirectory,
@@ -37,6 +47,7 @@ pub struct VfsBuilder {
 
 impl VfsBuilder {
   pub fn new(root_path: PathBuf) -> Self {
+    log::debug!("Building vfs with root '{}'", root_path.display());
     Self {
       root_dir: VirtualDirectory {
         name: root_path
@@ -58,7 +69,7 @@ impl VfsBuilder {
   }
 
   pub fn add_dir_recursive(&mut self, path: &Path) -> Result<(), AnyError> {
-    self.add_dir(path);
+    self.add_dir(path)?;
     let read_dir = std::fs::read_dir(path)
       .with_context(|| format!("Reading {}", path.display()))?;
 
@@ -72,19 +83,44 @@ impl VfsBuilder {
       } else if file_type.is_file() {
         let file_bytes = std::fs::read(&path)
           .with_context(|| format!("Reading {}", path.display()))?;
-        self.add_file(&path, file_bytes);
+        self.add_file(&path, file_bytes)?;
       } else if file_type.is_symlink() {
-        let target = std::fs::read_link(&path)
+        let target = util::fs::canonicalize_path(&path)
           .with_context(|| format!("Reading symlink {}", path.display()))?;
-        self.add_symlink(&path, &target);
+        if let Err(StripRootError { .. }) = self.add_symlink(&path, &target) {
+          if target.is_file() {
+            // this may change behavior, so warn the user about it
+            log::warn!(
+              "Symlink target is outside '{}'. Inlining symlink at '{}' to '{}' as file.",
+              self.root_path.display(),
+              path.display(),
+              target.display(),
+            );
+            // inline the symlink and make the target file
+            let file_bytes = std::fs::read(&target)
+              .with_context(|| format!("Reading {}", path.display()))?;
+            self.add_file(&path, file_bytes)?;
+          } else {
+            log::warn!(
+              "Symlink target is outside '{}'. Excluding symlink at '{}' with target '{}'.",
+              self.root_path.display(),
+              path.display(),
+              target.display(),
+            );
+          }
+        }
       }
     }
 
     Ok(())
   }
 
-  pub fn add_dir(&mut self, path: &Path) -> &mut VirtualDirectory {
-    let path = self.path_relative_root(path);
+  pub fn add_dir(
+    &mut self,
+    path: &Path,
+  ) -> Result<&mut VirtualDirectory, StripRootError> {
+    log::debug!("Ensuring directory '{}'", path.display());
+    let path = self.path_relative_root(path)?;
     let mut current_dir = &mut self.root_dir;
 
     for component in path.components() {
@@ -113,10 +149,15 @@ impl VfsBuilder {
       };
     }
 
-    current_dir
+    Ok(current_dir)
   }
 
-  pub fn add_file(&mut self, path: &Path, data: Vec<u8>) {
+  pub fn add_file(
+    &mut self,
+    path: &Path,
+    data: Vec<u8>,
+  ) -> Result<(), AnyError> {
+    log::debug!("Adding file '{}'", path.display());
     let checksum = util::checksum::gen(&[&data]);
     let offset = if let Some(offset) = self.file_offsets.get(&checksum) {
       // duplicate file, reuse an old offset
@@ -126,7 +167,7 @@ impl VfsBuilder {
       self.current_offset
     };
 
-    let dir = self.add_dir(path.parent().unwrap());
+    let dir = self.add_dir(path.parent().unwrap())?;
     let name = path.file_name().unwrap().to_string_lossy();
     let data_len = data.len();
     match dir.entries.binary_search_by(|e| e.name().cmp(&name)) {
@@ -148,11 +189,22 @@ impl VfsBuilder {
       self.files.push(data);
       self.current_offset += data_len as u64;
     }
+
+    Ok(())
   }
 
-  pub fn add_symlink(&mut self, path: &Path, target: &Path) {
-    let dest = self.path_relative_root(target);
-    let dir = self.add_dir(path.parent().unwrap());
+  pub fn add_symlink(
+    &mut self,
+    path: &Path,
+    target: &Path,
+  ) -> Result<(), StripRootError> {
+    log::debug!(
+      "Adding symlink '{}' to '{}'",
+      path.display(),
+      target.display()
+    );
+    let dest = self.path_relative_root(target)?;
+    let dir = self.add_dir(path.parent().unwrap())?;
     let name = path.file_name().unwrap().to_string_lossy();
     match dir.entries.binary_search_by(|e| e.name().cmp(&name)) {
       Ok(_) => unreachable!(),
@@ -169,23 +221,20 @@ impl VfsBuilder {
         );
       }
     }
+    Ok(())
   }
 
   pub fn into_dir_and_files(self) -> (VirtualDirectory, Vec<Vec<u8>>) {
     (self.root_dir, self.files)
   }
 
-  fn path_relative_root(&self, path: &Path) -> PathBuf {
+  fn path_relative_root(&self, path: &Path) -> Result<PathBuf, StripRootError> {
     match path.strip_prefix(&self.root_path) {
-      Ok(p) => p.to_path_buf(),
-      Err(err) => {
-        panic!(
-          "Failed to strip prefix '{}' from '{}': {:#}",
-          self.root_path.display(),
-          path.display(),
-          err
-        )
-      }
+      Ok(p) => Ok(p.to_path_buf()),
+      Err(_) => Err(StripRootError {
+        root_path: self.root_path.clone(),
+        target: path.to_path_buf(),
+      }),
     }
   }
 }
@@ -457,7 +506,11 @@ impl FileBackedVfsFile {
       }
       SeekFrom::End(offset) => {
         if offset < 0 && -offset as u64 > self.file.len {
-          Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, "An attempt was made to move the file pointer before the beginning of the file.").into())
+          let msg = "An attempt was made to move the file pointer before the beginning of the file.";
+          Err(
+            std::io::Error::new(std::io::ErrorKind::PermissionDenied, msg)
+              .into(),
+          )
         } else {
           let mut current_pos = self.pos.lock();
           *current_pos = if offset >= 0 {
@@ -790,16 +843,28 @@ mod test {
     let temp_dir = TempDir::new();
     let src_path = temp_dir.path().join("src");
     let mut builder = VfsBuilder::new(src_path.clone());
-    builder.add_file(&src_path.join("a.txt"), "data".into());
-    builder.add_file(&src_path.join("b.txt"), "data".into());
+    builder
+      .add_file(&src_path.join("a.txt"), "data".into())
+      .unwrap();
+    builder
+      .add_file(&src_path.join("b.txt"), "data".into())
+      .unwrap();
     assert_eq!(builder.files.len(), 1); // because duplicate data
-    builder.add_file(&src_path.join("c.txt"), "c".into());
-    builder.add_file(&src_path.join("sub_dir").join("d.txt"), "d".into());
-    builder.add_file(&src_path.join("e.txt"), "e".into());
-    builder.add_symlink(
-      &src_path.join("sub_dir").join("e.txt"),
-      &src_path.join("e.txt"),
-    );
+    builder
+      .add_file(&src_path.join("c.txt"), "c".into())
+      .unwrap();
+    builder
+      .add_file(&src_path.join("sub_dir").join("d.txt"), "d".into())
+      .unwrap();
+    builder
+      .add_file(&src_path.join("e.txt"), "e".into())
+      .unwrap();
+    builder
+      .add_symlink(
+        &src_path.join("sub_dir").join("e.txt"),
+        &src_path.join("e.txt"),
+      )
+      .unwrap();
 
     // get the virtual fs
     let (dest_path, virtual_fs) = into_virtual_fs(builder, &temp_dir);
@@ -923,9 +988,15 @@ mod test {
     let temp_dir = TempDir::new();
     let src_path = temp_dir.path().join("src");
     let mut builder = VfsBuilder::new(src_path.clone());
-    builder.add_symlink(&src_path.join("a.txt"), &src_path.join("b.txt"));
-    builder.add_symlink(&src_path.join("b.txt"), &src_path.join("c.txt"));
-    builder.add_symlink(&src_path.join("c.txt"), &src_path.join("a.txt"));
+    builder
+      .add_symlink(&src_path.join("a.txt"), &src_path.join("b.txt"))
+      .unwrap();
+    builder
+      .add_symlink(&src_path.join("b.txt"), &src_path.join("c.txt"))
+      .unwrap();
+    builder
+      .add_symlink(&src_path.join("c.txt"), &src_path.join("a.txt"))
+      .unwrap();
     let (dest_path, virtual_fs) = into_virtual_fs(builder, &temp_dir);
     assert_eq!(
       virtual_fs
@@ -950,10 +1021,12 @@ mod test {
     let temp_dir = TempDir::new();
     let temp_path = temp_dir.path();
     let mut builder = VfsBuilder::new(temp_path.to_path_buf());
-    builder.add_file(
-      &temp_path.join("a.txt"),
-      "0123456789".to_string().into_bytes(),
-    );
+    builder
+      .add_file(
+        &temp_path.join("a.txt"),
+        "0123456789".to_string().into_bytes(),
+      )
+      .unwrap();
     let (dest_path, virtual_fs) = into_virtual_fs(builder, &temp_dir);
     let virtual_fs = Arc::new(virtual_fs);
     let file = virtual_fs.open_file(&dest_path.join("a.txt")).unwrap();
diff --git a/cli/tests/integration/compile_tests.rs b/cli/tests/integration/compile_tests.rs
index 4938b36cd..f202fc87e 100644
--- a/cli/tests/integration/compile_tests.rs
+++ b/cli/tests/integration/compile_tests.rs
@@ -1082,3 +1082,68 @@ fn run_npm_bin_compile_test(opts: RunNpmBinCompileOptions) {
   output.assert_matches_file(opts.output_file);
   output.assert_exit_code(opts.exit_code);
 }
+
+#[test]
+fn compile_node_modules_symlink_outside() {
+  let context = TestContextBuilder::for_npm()
+    .use_sync_npm_download()
+    .use_copy_temp_dir("compile/node_modules_symlink_outside")
+    .cwd("compile/node_modules_symlink_outside")
+    .build();
+
+  let temp_dir = context.temp_dir();
+  let project_dir = temp_dir
+    .path()
+    .join("compile")
+    .join("node_modules_symlink_outside");
+  temp_dir.create_dir_all(project_dir.join("node_modules"));
+  temp_dir.create_dir_all(project_dir.join("some_folder"));
+  temp_dir.write(project_dir.join("test.txt"), "5");
+
+  // create a symlink in the node_modules directory that points to a folder in the cwd
+  temp_dir.symlink_dir(
+    project_dir.join("some_folder"),
+    project_dir.join("node_modules").join("some_folder"),
+  );
+  // compile folder
+  let output = context
+    .new_command()
+    .args("compile --allow-read --node-modules-dir --output bin main.ts")
+    .run();
+  output.assert_exit_code(0);
+  output.assert_matches_file(
+    "compile/node_modules_symlink_outside/main_compile_folder.out",
+  );
+  assert!(project_dir.join("node_modules/some_folder").exists());
+
+  // Cleanup and remove the folder. The folder test is done separately from
+  // the file symlink test because different systems would traverse
+  // the directory items in different order.
+  temp_dir.remove_dir_all(project_dir.join("node_modules/some_folder"));
+
+  // create a symlink in the node_modules directory that points to a file in the cwd
+  temp_dir.symlink_file(
+    project_dir.join("test.txt"),
+    project_dir.join("node_modules").join("test.txt"),
+  );
+  assert!(project_dir.join("node_modules/test.txt").exists());
+
+  // compile
+  let output = context
+    .new_command()
+    .args("compile --allow-read --node-modules-dir --output bin main.ts")
+    .run();
+  output.assert_exit_code(0);
+  output.assert_matches_file(
+    "compile/node_modules_symlink_outside/main_compile_file.out",
+  );
+
+  // run
+  let binary_path =
+    project_dir.join(if cfg!(windows) { "bin.exe" } else { "bin" });
+  let output = context
+    .new_command()
+    .command_name(binary_path.to_string_lossy())
+    .run();
+  output.assert_matches_file("compile/node_modules_symlink_outside/main.out");
+}
diff --git a/cli/tests/testdata/compile/node_modules_symlink_outside/main.out b/cli/tests/testdata/compile/node_modules_symlink_outside/main.out
new file mode 100644
index 000000000..61c83cba4
--- /dev/null
+++ b/cli/tests/testdata/compile/node_modules_symlink_outside/main.out
@@ -0,0 +1,2 @@
+4
+5
diff --git a/cli/tests/testdata/compile/node_modules_symlink_outside/main.ts b/cli/tests/testdata/compile/node_modules_symlink_outside/main.ts
new file mode 100644
index 000000000..45f681f7c
--- /dev/null
+++ b/cli/tests/testdata/compile/node_modules_symlink_outside/main.ts
@@ -0,0 +1,6 @@
+import { getValue, setValue } from "npm:@denotest/esm-basic";
+
+setValue(4);
+
+console.log(getValue());
+console.log(Deno.readTextFileSync("./node_modules/test.txt"));
diff --git a/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out
new file mode 100644
index 000000000..7602a4002
--- /dev/null
+++ b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out
@@ -0,0 +1,2 @@
+Compile file:///[WILDCARD]/node_modules_symlink_outside/main.ts to [WILDCARD]
+Symlink target is outside '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules'. Inlining symlink at '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules[WILDCARD]test.txt' to '[WILDCARD]node_modules_symlink_outside[WILDCARD]test.txt' as file.
diff --git a/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out
new file mode 100644
index 000000000..883a3f262
--- /dev/null
+++ b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out
@@ -0,0 +1,6 @@
+Download http://localhost:4545/npm/registry/@denotest/esm-basic
+Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz
+Initialize @denotest/esm-basic@1.0.0
+Check file:///[WILDCARD]/node_modules_symlink_outside/main.ts
+Compile file:///[WILDCARD]/node_modules_symlink_outside/main.ts to [WILDCARD]
+Symlink target is outside '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules'. Excluding symlink at '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules[WILDCARD]some_folder' with target '[WILDCARD]node_modules_symlink_outside[WILDCARD]some_folder'.