fix: do special file permission check for check_read_path (#27989)

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

Cargo.lock

@@ -1785,6 +1785,7 @@ dependencies = [
  "data-url",
  "deno_core",
  "deno_error",
+ "deno_fs",
  "deno_path_util",
  "deno_permissions",
  "deno_tls",

ext/fetch/Cargo.toml

@@ -19,6 +19,7 @@ bytes.workspace = true
 data-url.workspace = true
 deno_core.workspace = true
 deno_error.workspace = true
+deno_fs.workspace = true
 deno_path_util.workspace = true
 deno_permissions.workspace = true
 deno_tls.workspace = true

ext/fetch/fs_fetch_handler.rs

@@ -9,6 +9,9 @@ use deno_core::url::Url;
 use deno_core::CancelFuture;
 use deno_core::OpState;
 use deno_error::JsErrorBox;
+use deno_fs::open_options_with_access_check;
+use deno_fs::OpenOptions;
+use deno_permissions::PermissionsContainer;
 use http::StatusCode;
 use http_body_util::BodyExt;
 use tokio_util::io::ReaderStream;
@@ -16,6 +19,16 @@ use tokio_util::io::ReaderStream;
 use crate::CancelHandle;
 use crate::CancelableResponseFuture;
 use crate::FetchHandler;
+use crate::FetchPermissions;
+
+fn sync_permission_check<'a, P: FetchPermissions + 'static>(
+  permissions: &'a mut P,
+  api_name: &'static str,
+) -> impl deno_fs::AccessCheckFn + 'a {
+  move |resolved, path, _options| {
+    permissions.check_read(resolved, path, api_name)
+  }
+}
 
 /// An implementation which tries to read file URLs from the file system via
 /// tokio::fs.
@@ -25,25 +38,52 @@ pub struct FsFetchHandler;
 impl FetchHandler for FsFetchHandler {
   fn fetch_file(
     &self,
-    _state: &mut OpState,
+    state: &mut OpState,
     url: &Url,
   ) -> (CancelableResponseFuture, Option<Rc<CancelHandle>>) {
+    let mut access_check = sync_permission_check::<PermissionsContainer>(
+      state.borrow_mut(),
+      "fetch()",
+    );
     let cancel_handle = CancelHandle::new_rc();
-    let path_result = url.to_file_path();
+    let path = match url.to_file_path() {
+      Ok(path) => path,
+      Err(_) => {
+        let fut = async move { Err::<_, _>(()) };
+        return (
+          fut
+            .map_err(move |_| super::FetchError::NetworkError)
+            .or_cancel(&cancel_handle)
+            .boxed_local(),
+          Some(cancel_handle),
+        );
+      }
+    };
+    let path_and_opts_result = open_options_with_access_check(
+      OpenOptions {
+        read: true,
+        ..Default::default()
+      },
+      &path,
+      Some(&mut access_check),
+    );
     let response_fut = async move {
-      let path = path_result?;
-      let file = tokio::fs::File::open(path).map_err(|_| ()).await?;
+      let (path, opts) = path_and_opts_result?;
+      let file = tokio::fs::OpenOptions::from(opts)
+        .open(path)
+        .await
+        .map_err(|_| super::FetchError::NetworkError)?;
       let stream = ReaderStream::new(file)
         .map_ok(hyper::body::Frame::data)
         .map_err(JsErrorBox::from_err);
+
       let body = http_body_util::StreamBody::new(stream).boxed();
       let response = http::Response::builder()
         .status(StatusCode::OK)
         .body(body)
-        .map_err(|_| ())?;
-      Ok::<_, ()>(response)
+        .map_err(move |_| super::FetchError::NetworkError)?;
+      Ok::<_, _>(response)
     }
-    .map_err(move |_| super::FetchError::NetworkError)
     .or_cancel(&cancel_handle)
     .boxed_local();
 

ext/fetch/lib.rs

@@ -47,7 +47,7 @@ use deno_core::RcRef;
 use deno_core::Resource;
 use deno_core::ResourceId;
 use deno_error::JsErrorBox;
-use deno_path_util::url_from_file_path;
+use deno_fs::FsError;
 use deno_path_util::PathToUrlError;
 use deno_permissions::PermissionCheckError;
 use deno_tls::rustls::RootCertStore;
@@ -227,6 +227,20 @@ pub enum FetchError {
   #[class(generic)]
   #[error(transparent)]
   Dns(hickory_resolver::ResolveError),
+  #[class("NotCapable")]
+  #[error("requires {0} access")]
+  NotCapable(&'static str),
+}
+
+impl From<deno_fs::FsError> for FetchError {
+  fn from(value: deno_fs::FsError) -> Self {
+    match value {
+      deno_fs::FsError::Io(_)
+      | deno_fs::FsError::FileBusy
+      | deno_fs::FsError::NotSupported => FetchError::NetworkError,
+      deno_fs::FsError::NotCapable(err) => FetchError::NotCapable(err),
+    }
+  }
 }
 
 pub type CancelableResponseFuture =
@@ -390,9 +404,10 @@ pub trait FetchPermissions {
   #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"]
   fn check_read<'a>(
     &mut self,
+    resolved: bool,
     p: &'a Path,
     api_name: &str,
-  ) -> Result<Cow<'a, Path>, PermissionCheckError>;
+  ) -> Result<Cow<'a, Path>, FsError>;
 }
 
 impl FetchPermissions for deno_permissions::PermissionsContainer {
@@ -408,14 +423,23 @@ impl FetchPermissions for deno_permissions::PermissionsContainer {
   #[inline(always)]
   fn check_read<'a>(
     &mut self,
+    resolved: bool,
     path: &'a Path,
     api_name: &str,
-  ) -> Result<Cow<'a, Path>, PermissionCheckError> {
+  ) -> Result<Cow<'a, Path>, FsError> {
+    if resolved {
+      self
+        .check_special_file(path, api_name)
+        .map_err(FsError::NotCapable)?;
+      return Ok(Cow::Borrowed(path));
+    }
+
     deno_permissions::PermissionsContainer::check_read_path(
       self,
       path,
       Some(api_name),
     )
+    .map_err(|_| FsError::NotCapable("read"))
   }
 }
 
@@ -449,18 +473,9 @@ where
   let scheme = url.scheme();
   let (request_rid, cancel_handle_rid) = match scheme {
     "file" => {
-      let path = url.to_file_path().map_err(|_| FetchError::NetworkError)?;
-      let permissions = state.borrow_mut::<FP>();
-      let path = permissions.check_read(&path, "fetch()")?;
-      let url = match path {
-        Cow::Owned(path) => url_from_file_path(&path)?,
-        Cow::Borrowed(_) => url,
-      };
-
       if method != Method::GET {
         return Err(FetchError::FsNotGet(method));
       }
-
       let Options {
         file_fetch_handler, ..
       } = state.borrow_mut::<Options>();

ext/fs/lib.rs

@@ -9,7 +9,7 @@ use std::borrow::Cow;
 use std::path::Path;
 use std::path::PathBuf;
 
-use deno_io::fs::FsError;
+pub use deno_io::fs::FsError;
 use deno_permissions::PermissionCheckError;
 
 pub use crate::interface::AccessCheckCb;
@@ -23,6 +23,7 @@ pub use crate::ops::FsOpsError;
 pub use crate::ops::FsOpsErrorKind;
 pub use crate::ops::OperationError;
 use crate::ops::*;
+pub use crate::std_fs::open_options_with_access_check;
 pub use crate::std_fs::RealFs;
 pub use crate::sync::MaybeSend;
 pub use crate::sync::MaybeSync;

ext/fs/std_fs.rs

@@ -1063,11 +1063,22 @@ fn open_options(options: OpenOptions) -> fs::OpenOptions {
 }
 
 #[inline(always)]
-fn open_with_access_check(
+pub fn open_with_access_check(
   options: OpenOptions,
   path: &Path,
   access_check: Option<AccessCheckCb>,
 ) -> FsResult<std::fs::File> {
+  let (path, opts) =
+    open_options_with_access_check(options, path, access_check)?;
+  Ok(opts.open(path)?)
+}
+
+#[inline(always)]
+pub fn open_options_with_access_check(
+  options: OpenOptions,
+  path: &Path,
+  access_check: Option<AccessCheckCb>,
+) -> FsResult<(PathBuf, fs::OpenOptions)> {
   if let Some(access_check) = access_check {
     let path_bytes = path.as_os_str().as_encoded_bytes();
     let is_windows_device_path = cfg!(windows)
@@ -1126,7 +1137,7 @@ fn open_with_access_check(
       }
     }
 
-    Ok(opts.open(&path)?)
+    Ok((path, opts))
   } else {
     // for unix
     #[allow(unused_mut)]
@@ -1137,6 +1148,6 @@ fn open_with_access_check(
       use std::os::windows::fs::OpenOptionsExt;
       opts.custom_flags(winapi::um::winbase::FILE_FLAG_BACKUP_SEMANTICS);
     }
-    Ok(opts.open(path)?)
+    Ok((path.to_path_buf(), opts))
   }
 }

runtime/permissions/lib.rs

@@ -2353,6 +2353,9 @@ pub enum PermissionCheckError {
   #[class(uri)]
   #[error(transparent)]
   HostParse(#[from] HostParseError),
+  #[class("NotCapable")]
+  #[error("Permission denied {0}")]
+  NotCapable(&'static str),
 }
 
 /// Wrapper struct for `Permissions` that can be shared across threads.
@@ -2589,6 +2592,7 @@ impl PermissionsContainer {
       }
       .into_read();
       inner.check(&desc, api_name)?;
+
       Ok(Cow::Owned(desc.0.resolved))
     }
   }
@@ -2597,7 +2601,7 @@ impl PermissionsContainer {
   /// by replacing it with the given `display`.
   #[inline(always)]
   pub fn check_read_blind(
-    &mut self,
+    &self,
     path: &Path,
     display: &str,
     api_name: &str,
@@ -2714,7 +2718,7 @@ impl PermissionsContainer {
 
   #[inline(always)]
   pub fn check_write_partial(
-    &mut self,
+    &self,
     path: &str,
     api_name: &str,
   ) -> Result<PathBuf, PermissionCheckError> {
@@ -2731,7 +2735,7 @@ impl PermissionsContainer {
 
   #[inline(always)]
   pub fn check_run(
-    &mut self,
+    &self,
     cmd: &RunQueryDescriptor,
     api_name: &str,
   ) -> Result<(), PermissionCheckError> {
@@ -2767,25 +2771,25 @@ impl PermissionsContainer {
   }
 
   #[inline(always)]
-  pub fn check_env(&mut self, var: &str) -> Result<(), PermissionCheckError> {
+  pub fn check_env(&self, var: &str) -> Result<(), PermissionCheckError> {
     self.inner.lock().env.check(var, None)?;
     Ok(())
   }
 
   #[inline(always)]
-  pub fn check_env_all(&mut self) -> Result<(), PermissionCheckError> {
+  pub fn check_env_all(&self) -> Result<(), PermissionCheckError> {
     self.inner.lock().env.check_all()?;
     Ok(())
   }
 
   #[inline(always)]
-  pub fn check_sys_all(&mut self) -> Result<(), PermissionCheckError> {
+  pub fn check_sys_all(&self) -> Result<(), PermissionCheckError> {
     self.inner.lock().sys.check_all()?;
     Ok(())
   }
 
   #[inline(always)]
-  pub fn check_ffi_all(&mut self) -> Result<(), PermissionCheckError> {
+  pub fn check_ffi_all(&self) -> Result<(), PermissionCheckError> {
     self.inner.lock().ffi.check_all()?;
     Ok(())
   }
@@ -2794,7 +2798,7 @@ impl PermissionsContainer {
   /// permissions are enabled!
   #[inline(always)]
   pub fn check_was_allow_all_flag_passed(
-    &mut self,
+    &self,
   ) -> Result<(), PermissionCheckError> {
     self.inner.lock().all.check()?;
     Ok(())
@@ -2803,7 +2807,7 @@ impl PermissionsContainer {
   /// Checks special file access, returning the failed permission type if
   /// not successful.
   pub fn check_special_file(
-    &mut self,
+    &self,
     path: &Path,
     _api_name: &str,
   ) -> Result<(), &'static str> {

runtime/snapshot_info.rs

@@ -45,9 +45,10 @@ impl deno_fetch::FetchPermissions for Permissions {
 
   fn check_read<'a>(
     &mut self,
+    _resolved: bool,
     _p: &'a Path,
     _api_name: &str,
-  ) -> Result<Cow<'a, Path>, PermissionCheckError> {
+  ) -> Result<Cow<'a, Path>, FsError> {
     unreachable!("snapshotting!")
   }
 }

tests/unit/fetch_test.ts

@@ -1692,6 +1692,15 @@ Deno.test({ permissions: { read: false } }, async function fetchFilePerm() {
   }, Deno.errors.NotCapable);
 });
 
+Deno.test(
+  { permissions: { read: true }, ignore: Deno.build.os !== "linux" },
+  async function fetchSpecialFilePerm() {
+    await assertRejects(async () => {
+      await fetch("file:///proc/self/environ");
+    }, Deno.errors.NotCapable);
+  },
+);
+
 Deno.test(
   { permissions: { read: false } },
   async function fetchFilePermDoesNotExist() {