refactor: improve node permission checks (#26028)

Does less work when requesting permissions with `-A`

1 files changed, 13 insertions, 11 deletions

diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs
index 15667aae7..547336981 100644
--- a/ext/node/ops/require.rs
+++ b/ext/node/ops/require.rs
@@ -15,6 +15,7 @@ use deno_path_util::normalize_path;
 use node_resolver::NodeModuleKind;
 use node_resolver::NodeResolutionMode;
 use node_resolver::REQUIRE_CONDITIONS;
+use std::borrow::Cow;
 use std::cell::RefCell;
 use std::path::Path;
 use std::path::PathBuf;
@@ -25,10 +26,11 @@ use crate::NodeRequireResolverRc;
 use crate::NodeResolverRc;
 use crate::NpmResolverRc;
 
-fn ensure_read_permission<P>(
+#[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
+fn ensure_read_permission<'a, P>(
   state: &mut OpState,
-  file_path: &Path,
-) -> Result<(), AnyError>
+  file_path: &'a Path,
+) -> Result<Cow<'a, Path>, AnyError>
 where
   P: NodePermissions + 'static,
 {
@@ -107,7 +109,7 @@ where
     deno_path_util::normalize_path(current_dir.join(from))
   };
 
-  ensure_read_permission::<P>(state, &from)?;
+  let from = ensure_read_permission::<P>(state, &from)?;
 
   if cfg!(windows) {
     // return root node_modules when path is 'D:\\'.
@@ -129,7 +131,7 @@ where
   }
 
   let mut paths = Vec::with_capacity(from.components().count());
-  let mut current_path = from.as_path();
+  let mut current_path = from.as_ref();
   let mut maybe_parent = Some(current_path);
   while let Some(parent) = maybe_parent {
     if !parent.ends_with("node_modules") {
@@ -267,7 +269,7 @@ where
   P: NodePermissions + 'static,
 {
   let path = PathBuf::from(path);
-  ensure_read_permission::<P>(state, &path)?;
+  let path = ensure_read_permission::<P>(state, &path)?;
   let fs = state.borrow::<FileSystemRc>();
   if let Ok(metadata) = fs.stat_sync(&path) {
     if metadata.is_file {
@@ -290,7 +292,7 @@ where
   P: NodePermissions + 'static,
 {
   let path = PathBuf::from(request);
-  ensure_read_permission::<P>(state, &path)?;
+  let path = ensure_read_permission::<P>(state, &path)?;
   let fs = state.borrow::<FileSystemRc>();
   let canonicalized_path =
     deno_core::strip_unc_prefix(fs.realpath_sync(&path)?);
@@ -362,7 +364,7 @@ where
     if parent_id == "<repl>" || parent_id == "internal/preload" {
       let fs = state.borrow::<FileSystemRc>();
       if let Ok(cwd) = fs.cwd() {
-        ensure_read_permission::<P>(state, &cwd)?;
+        let cwd = ensure_read_permission::<P>(state, &cwd)?;
         return Ok(Some(cwd.to_string_lossy().into_owned()));
       }
     }
@@ -443,7 +445,7 @@ where
   P: NodePermissions + 'static,
 {
   let file_path = PathBuf::from(file_path);
-  ensure_read_permission::<P>(state, &file_path)?;
+  let file_path = ensure_read_permission::<P>(state, &file_path)?;
   let fs = state.borrow::<FileSystemRc>();
   Ok(fs.read_text_file_lossy_sync(&file_path, None)?)
 }
@@ -528,7 +530,7 @@ where
   P: NodePermissions + 'static,
 {
   let filename = PathBuf::from(filename);
-  ensure_read_permission::<P>(state, filename.parent().unwrap())?;
+  // permissions: allow reading the closest package.json files
   let node_resolver = state.borrow::<NodeResolverRc>().clone();
   node_resolver
     .get_closest_package_json_from_path(&filename)
@@ -567,7 +569,7 @@ where
   P: NodePermissions + 'static,
 {
   let referrer_path = PathBuf::from(&referrer_filename);
-  ensure_read_permission::<P>(state, &referrer_path)?;
+  let referrer_path = ensure_read_permission::<P>(state, &referrer_path)?;
   let node_resolver = state.borrow::<NodeResolverRc>();
   let Some(pkg) =
     node_resolver.get_closest_package_json_from_path(&referrer_path)?