diff --git a/cli/lib/npm/permission_checker.rs b/cli/lib/npm/permission_checker.rs index ebed1270f3..8db62dbc6c 100644 --- a/cli/lib/npm/permission_checker.rs +++ b/cli/lib/npm/permission_checker.rs @@ -61,7 +61,7 @@ impl NpmRegistryReadPermissionChecker { Ok(Cow::Borrowed(path)) } else { permissions - .check_read_path(path) + .check_read_path(Cow::Borrowed(path)) .map_err(JsErrorBox::from_err) } } @@ -112,7 +112,7 @@ impl NpmRegistryReadPermissionChecker { } permissions - .check_read_path(path) + .check_read_path(Cow::Borrowed(path)) .map_err(JsErrorBox::from_err) } } diff --git a/ext/fetch/fs_fetch_handler.rs b/ext/fetch/fs_fetch_handler.rs index fd15414d14..5b1886258c 100644 --- a/ext/fetch/fs_fetch_handler.rs +++ b/ext/fetch/fs_fetch_handler.rs @@ -25,8 +25,8 @@ 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) + move |path, _options, _resolve| { + permissions.check_read(path, api_name, _resolve) } } diff --git a/ext/fetch/lib.rs b/ext/fetch/lib.rs index 550733a6fa..41adc7c7f5 100644 --- a/ext/fetch/lib.rs +++ b/ext/fetch/lib.rs @@ -47,6 +47,7 @@ use deno_core::RcRef; use deno_core::Resource; use deno_core::ResourceId; use deno_error::JsErrorBox; +use deno_fs::CheckedPath; pub use deno_fs::FsError; use deno_path_util::PathToUrlError; use deno_permissions::PermissionCheckError; @@ -402,10 +403,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, + path: Cow<'a, Path>, api_name: &str, - ) -> Result, FsError>; + get_path: &'a dyn deno_fs::GetPath, + ) -> Result, FsError>; } impl FetchPermissions for deno_permissions::PermissionsContainer { @@ -421,23 +422,39 @@ impl FetchPermissions for deno_permissions::PermissionsContainer { #[inline(always)] fn check_read<'a>( &mut self, - resolved: bool, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, - ) -> Result, FsError> { - if resolved { - self - .check_special_file(path, api_name) - .map_err(FsError::NotCapable)?; - return Ok(Cow::Borrowed(path)); + get_path: &'a dyn deno_fs::GetPath, + ) -> Result, FsError> { + if self.allows_all() { + return Ok(deno_fs::CheckedPath::Unresolved(path)); } - deno_permissions::PermissionsContainer::check_read_path( + let (needs_canonicalize, normalized_path) = get_path.normalized(path)?; + + let path = deno_permissions::PermissionsContainer::check_read_path( self, - path, + normalized_path, Some(api_name), ) - .map_err(|_| FsError::NotCapable("read")) + .map_err(|_| FsError::NotCapable("read"))?; + + let path = if needs_canonicalize { + let path = get_path.resolved(&path)?; + + Cow::Owned(path) + } else { + path + }; + self + .check_special_file(&path, api_name) + .map_err(FsError::NotCapable)?; + + if needs_canonicalize { + Ok(CheckedPath::Resolved(path)) + } else { + Ok(CheckedPath::Unresolved(path)) + } } } diff --git a/ext/fs/interface.rs b/ext/fs/interface.rs index 0aae2b998f..969324d031 100644 --- a/ext/fs/interface.rs +++ b/ext/fs/interface.rs @@ -84,21 +84,26 @@ pub type FileSystemRc = crate::sync::MaybeArc; pub trait AccessCheckFn: for<'a> FnMut( - bool, - &'a Path, + Cow<'a, Path>, &'a OpenOptions, -) -> FsResult> + &'a dyn crate::GetPath, +) -> FsResult> { } impl AccessCheckFn for T where T: for<'a> FnMut( - bool, - &'a Path, + Cow<'a, Path>, &'a OpenOptions, - ) -> FsResult> + &'a dyn crate::GetPath, + ) -> FsResult> { } +pub enum CheckedPath<'a> { + Resolved(Cow<'a, Path>), + Unresolved(Cow<'a, Path>), +} + pub type AccessCheckCb<'a> = &'a mut (dyn AccessCheckFn + 'a); #[async_trait::async_trait(?Send)] diff --git a/ext/fs/lib.rs b/ext/fs/lib.rs index cb1373bbbd..74a631cc80 100644 --- a/ext/fs/lib.rs +++ b/ext/fs/lib.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; pub use deno_io::fs::FsError; use deno_permissions::PermissionCheckError; +pub use interface::CheckedPath; pub use crate::interface::AccessCheckCb; pub use crate::interface::AccessCheckFn; @@ -31,12 +32,12 @@ pub use crate::sync::MaybeSync; pub trait FsPermissions { fn check_open<'a>( &mut self, - resolved: bool, read: bool, write: bool, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, - ) -> Result, FsError>; + get_path: &'a dyn GetPath, + ) -> Result, FsError>; #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn check_read( &mut self, @@ -46,7 +47,7 @@ pub trait FsPermissions { #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn check_read_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError>; fn check_read_all( @@ -68,7 +69,7 @@ pub trait FsPermissions { #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn check_write_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError>; #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] @@ -90,56 +91,69 @@ pub trait FsPermissions { fn check<'a>( &mut self, - resolved: bool, open_options: &OpenOptions, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, - ) -> Result, FsError> { + get_path: &'a dyn GetPath, + ) -> Result, FsError> { self.check_open( - resolved, open_options.read, open_options.write || open_options.append, path, api_name, + get_path, ) } + + fn allows_all(&self) -> bool { + false + } } impl FsPermissions for deno_permissions::PermissionsContainer { fn check_open<'a>( &mut self, - resolved: bool, read: bool, write: bool, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, - ) -> Result, FsError> { - if resolved { - self - .check_special_file(path, api_name) - .map_err(FsError::NotCapable)?; - return Ok(Cow::Borrowed(path)); + get_path: &'a dyn GetPath, + ) -> Result, FsError> { + if self.allows_all() { + return Ok(CheckedPath::Unresolved(path)); } + let (needs_canonicalize, path) = get_path.normalized(path)?; // If somehow read or write aren't specified, use read let read = read || !write; - let mut path: Cow<'a, Path> = Cow::Borrowed(path); - if read { - let resolved_path = FsPermissions::check_read_path(self, &path, api_name) - .map_err(|_| FsError::NotCapable("read"))?; - if let Cow::Owned(resolved_path) = resolved_path { - path = Cow::Owned(resolved_path); - } + let path = if read { + FsPermissions::check_read_path(self, path, api_name) + .map_err(|_| FsError::NotCapable("read"))? + } else { + path + }; + let path = if write { + FsPermissions::check_write_path(self, path.clone(), api_name) + .map_err(|_| FsError::NotCapable("write"))? + } else { + path + }; + + let resolved_path = if needs_canonicalize { + Cow::Owned(get_path.resolved(&path)?) + } else { + path + }; + + self + .check_special_file(&resolved_path, api_name) + .map_err(FsError::NotCapable)?; + + if needs_canonicalize { + Ok(CheckedPath::Resolved(resolved_path)) + } else { + Ok(CheckedPath::Unresolved(resolved_path)) } - if write { - let resolved_path = - FsPermissions::check_write_path(self, &path, api_name) - .map_err(|_| FsError::NotCapable("write"))?; - if let Cow::Owned(resolved_path) = resolved_path { - path = Cow::Owned(resolved_path); - } - } - Ok(path) } fn check_read( @@ -152,7 +166,7 @@ impl FsPermissions for deno_permissions::PermissionsContainer { fn check_read_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError> { deno_permissions::PermissionsContainer::check_read_path( @@ -182,7 +196,7 @@ impl FsPermissions for deno_permissions::PermissionsContainer { fn check_write_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError> { deno_permissions::PermissionsContainer::check_write_path( @@ -224,6 +238,10 @@ impl FsPermissions for deno_permissions::PermissionsContainer { ) -> Result<(), PermissionCheckError> { deno_permissions::PermissionsContainer::check_write_all(self, api_name) } + + fn allows_all(&self) -> bool { + self.allows_all() + } } pub const UNSTABLE_FEATURE_NAME: &str = "fs"; @@ -305,3 +323,11 @@ deno_core::extension!(deno_fs, state.put(options.fs); }, ); + +pub trait GetPath { + fn normalized<'a>( + &self, + path: Cow<'a, Path>, + ) -> Result<(bool, Cow<'a, Path>), FsError>; + fn resolved(&self, path: &Path) -> Result; +} diff --git a/ext/fs/ops.rs b/ext/fs/ops.rs index c5c2b25631..bfd1c26400 100644 --- a/ext/fs/ops.rs +++ b/ext/fs/ops.rs @@ -119,19 +119,19 @@ fn sync_permission_check<'a, P: FsPermissions + 'static>( permissions: &'a mut P, api_name: &'static str, ) -> impl AccessCheckFn + 'a { - move |resolved, path, options| { - permissions.check(resolved, options, path, api_name) + move |path, options, resolve| { + permissions.check(options, path, api_name, resolve) } } fn async_permission_check( state: Rc>, api_name: &'static str, -) -> impl AccessCheckFn { - move |resolved, path, options| { +) -> impl AccessCheckFn + 'static { + move |path, options, resolve| { let mut state = state.borrow_mut(); let permissions = state.borrow_mut::

(); - permissions.check(resolved, options, path, api_name) + permissions.check(options, path, api_name, resolve) } } diff --git a/ext/fs/std_fs.rs b/ext/fs/std_fs.rs index 208612a48e..f8f606809a 100644 --- a/ext/fs/std_fs.rs +++ b/ext/fs/std_fs.rs @@ -21,9 +21,11 @@ use deno_io::StdFileResourceInner; use deno_path_util::normalize_path; use crate::interface::AccessCheckCb; +use crate::interface::CheckedPath; use crate::interface::FsDirEntry; use crate::interface::FsFileType; use crate::FileSystem; +use crate::GetPath; use crate::OpenOptions; #[derive(Debug, Default, Clone)] @@ -1079,43 +1081,15 @@ pub fn open_options_with_access_check( access_check: Option, ) -> 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) - && path_bytes.starts_with(br"\\.\") - && !path_bytes.contains(&b':'); - let path = if is_windows_device_path { - // On Windows, normalize_path doesn't work with device-prefix-style - // paths. We pass these through. - path.to_owned() - } else if path.is_absolute() { - normalize_path(path) - } else { - let cwd = current_dir()?; - normalize_path(cwd.join(path)) + let path = Cow::Borrowed(path); + let maybe_resolved = (*access_check)(path, &options, &StdGetPath)?; + + let path = maybe_resolved; + + let (_resolved, path) = match path { + CheckedPath::Resolved(path) => (true, path), + CheckedPath::Unresolved(path) => (false, path), }; - (*access_check)(false, &path, &options)?; - // On Linux, /proc may contain magic links that we don't want to resolve - let is_linux_special_path = cfg!(target_os = "linux") - && (path.starts_with("/proc") || path.starts_with("/dev")); - let needs_canonicalization = - !is_windows_device_path && !is_linux_special_path; - let path = if needs_canonicalization { - match path.canonicalize() { - Ok(path) => path, - Err(_) => { - if let (Some(parent), Some(filename)) = - (path.parent(), path.file_name()) - { - parent.canonicalize()?.join(filename) - } else { - return Err(std::io::ErrorKind::NotFound.into()); - } - } - } - } else { - path - }; - (*access_check)(true, &path, &options)?; let mut opts: fs::OpenOptions = open_options(options); #[cfg(windows)] @@ -1131,12 +1105,12 @@ pub fn open_options_with_access_check( // with the exception of /proc/ which is too special, and /dev/std* which might point to // proc. use std::os::unix::fs::OpenOptionsExt; - if needs_canonicalization { + if _resolved { opts.custom_flags(libc::O_NOFOLLOW); } } - Ok((path, opts)) + Ok((path.into_owned(), opts)) } else { // for unix #[allow(unused_mut)] @@ -1150,3 +1124,48 @@ pub fn open_options_with_access_check( Ok((path.to_path_buf(), opts)) } } + +struct StdGetPath; + +impl GetPath for StdGetPath { + fn normalized<'a>( + &self, + path: Cow<'a, Path>, + ) -> FsResult<(bool, Cow<'a, Path>)> { + let path_bytes = path.as_os_str().as_encoded_bytes(); + let is_windows_device_path = cfg!(windows) + && path_bytes.starts_with(br"\\.\") + && !path_bytes.contains(&b':'); + let path = if is_windows_device_path { + // On Windows, normalize_path doesn't work with device-prefix-style + // paths. We pass these through. + path + } else if path.is_absolute() { + Cow::Owned(normalize_path(path)) + } else { + let cwd = current_dir()?; + Cow::Owned(normalize_path(cwd.join(path))) + }; + // On Linux, /proc may contain magic links that we don't want to resolve + let is_linux_special_path = cfg!(target_os = "linux") + && (path.starts_with("/proc") || path.starts_with("/dev")); + let needs_canonicalization = + !is_windows_device_path && !is_linux_special_path; + Ok((needs_canonicalization, path)) + } + + fn resolved(&self, path: &Path) -> Result { + match path.canonicalize() { + Ok(path) => Ok(path), + Err(_) => { + if let (Some(parent), Some(filename)) = + (path.parent(), path.file_name()) + { + Ok(parent.canonicalize()?.join(filename)) + } else { + Err(std::io::ErrorKind::NotFound.into()) + } + } + } + } +} diff --git a/ext/kv/sqlite.rs b/ext/kv/sqlite.rs index 863fddddeb..526e86caad 100644 --- a/ext/kv/sqlite.rs +++ b/ext/kv/sqlite.rs @@ -46,7 +46,7 @@ pub trait SqliteDbHandlerPermissions { #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn check_write<'a>( &mut self, - p: &'a Path, + p: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError>; } @@ -64,7 +64,7 @@ impl SqliteDbHandlerPermissions for deno_permissions::PermissionsContainer { #[inline(always)] fn check_write<'a>( &mut self, - p: &'a Path, + p: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError> { deno_permissions::PermissionsContainer::check_write_path(self, p, api_name) @@ -131,7 +131,7 @@ impl DatabaseHandler for SqliteDbHandler

{ .check_read(&path, "Deno.openKv") .map_err(JsErrorBox::from_err)?; let path = permissions - .check_write(&path, "Deno.openKv") + .check_write(Cow::Owned(path), "Deno.openKv") .map_err(JsErrorBox::from_err)?; Ok(Some(path.to_string_lossy().to_string())) } diff --git a/ext/net/lib.rs b/ext/net/lib.rs index 9ed2518d9d..56bc5c8181 100644 --- a/ext/net/lib.rs +++ b/ext/net/lib.rs @@ -45,7 +45,7 @@ pub trait NetPermissions { #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn check_write_path<'a>( &mut self, - p: &'a Path, + p: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError>; #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] @@ -88,7 +88,7 @@ impl NetPermissions for deno_permissions::PermissionsContainer { #[inline(always)] fn check_write_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError> { deno_permissions::PermissionsContainer::check_write_path( diff --git a/ext/net/ops.rs b/ext/net/ops.rs index f1f9690ad3..1b5595a7da 100644 --- a/ext/net/ops.rs +++ b/ext/net/ops.rs @@ -1334,10 +1334,10 @@ mod tests { fn check_write_path<'a>( &mut self, - p: &'a Path, + p: Cow<'a, Path>, _api_name: &str, ) -> Result, PermissionCheckError> { - Ok(Cow::Borrowed(p)) + Ok(p) } fn check_vsock( diff --git a/ext/net/ops_unix.rs b/ext/net/ops_unix.rs index b347b81dc7..3cb336e3c6 100644 --- a/ext/net/ops_unix.rs +++ b/ext/net/ops_unix.rs @@ -102,11 +102,11 @@ where .borrow_mut::() .check_read(&address_path, "Deno.connect()") .map_err(NetError::Permission)?; - _ = state_ + + state_ .borrow_mut::() - .check_write_path(&address_path, "Deno.connect()") - .map_err(NetError::Permission)?; - address_path + .check_write_path(Cow::Owned(address_path), "Deno.connect()") + .map_err(NetError::Permission)? }; let unix_stream = UnixStream::connect(&address_path).await?; let local_addr = unix_stream.local_addr()?; @@ -188,8 +188,8 @@ where let address_path = permissions .check_read(&address_path, &api_call_expr) .map_err(NetError::Permission)?; - _ = permissions - .check_write_path(&address_path, &api_call_expr) + let address_path = permissions + .check_write_path(Cow::Owned(address_path), &api_call_expr) .map_err(NetError::Permission)?; let listener = UnixListener::bind(address_path)?; let local_addr = listener.local_addr()?; @@ -210,8 +210,8 @@ where let address_path = permissions .check_read(&address_path, "Deno.listenDatagram()") .map_err(NetError::Permission)?; - _ = permissions - .check_write_path(&address_path, "Deno.listenDatagram()") + let address_path = permissions + .check_write_path(Cow::Owned(address_path), "Deno.listenDatagram()") .map_err(NetError::Permission)?; let socket = UnixDatagram::bind(address_path)?; let local_addr = socket.local_addr()?; diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 358154a80f..dffd4c9bdb 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -77,7 +77,7 @@ pub trait NodePermissions { #[must_use = "the resolved return value to mitigate time-of-check to time-of-use issues"] fn check_read_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, ) -> Result, PermissionCheckError>; fn query_read_all(&mut self) -> bool; fn check_sys( @@ -124,7 +124,7 @@ impl NodePermissions for deno_permissions::PermissionsContainer { fn check_read_path<'a>( &mut self, - path: &'a Path, + path: Cow<'a, Path>, ) -> Result, PermissionCheckError> { deno_permissions::PermissionsContainer::check_read_path(self, path, None) } diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index b91f9f7c9f..ce93947fab 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -2610,13 +2610,13 @@ impl PermissionsContainer { #[inline(always)] pub fn check_read_path<'a>( &self, - path: &'a Path, + path: Cow<'a, Path>, api_name: Option<&str>, ) -> Result, PermissionCheckError> { let mut inner = self.inner.lock(); let inner = &mut inner.read; if inner.is_allow_all() { - Ok(Cow::Borrowed(path)) + Ok(path) } else { let desc = PathQueryDescriptor { requested: path.to_string_lossy().into_owned(), @@ -2698,13 +2698,13 @@ impl PermissionsContainer { #[inline(always)] pub fn check_write_path<'a>( &self, - path: &'a Path, + path: Cow<'a, Path>, api_name: &str, ) -> Result, PermissionCheckError> { let mut inner = self.inner.lock(); let inner = &mut inner.write; if inner.is_allow_all() { - Ok(Cow::Borrowed(path)) + Ok(path) } else { let desc = PathQueryDescriptor { requested: path.to_string_lossy().into_owned(), @@ -3396,6 +3396,10 @@ impl PermissionsContainer { ), ) } + + pub fn allows_all(&self) -> bool { + matches!(self.inner.lock().all.state, PermissionState::Granted) + } } const fn unit_permission_from_flag_bools( diff --git a/runtime/snapshot_info.rs b/runtime/snapshot_info.rs index 73435282ae..5da571027e 100644 --- a/runtime/snapshot_info.rs +++ b/runtime/snapshot_info.rs @@ -44,10 +44,10 @@ impl deno_fetch::FetchPermissions for Permissions { fn check_read<'a>( &mut self, - _resolved: bool, - _p: &'a Path, + _p: Cow<'a, Path>, _api_name: &str, - ) -> Result, FsError> { + _get_path: &'a dyn deno_fs::GetPath, + ) -> Result, FsError> { unreachable!("snapshotting!") } } @@ -88,7 +88,7 @@ impl deno_node::NodePermissions for Permissions { } fn check_read_path<'a>( &mut self, - _path: &'a Path, + _path: Cow<'a, Path>, ) -> Result, PermissionCheckError> { unreachable!("snapshotting!") } @@ -145,7 +145,7 @@ impl deno_net::NetPermissions for Permissions { fn check_write_path<'a>( &mut self, - _p: &'a Path, + _p: Cow<'a, Path>, _api_name: &str, ) -> Result, PermissionCheckError> { unreachable!("snapshotting!") @@ -164,12 +164,12 @@ impl deno_net::NetPermissions for Permissions { impl deno_fs::FsPermissions for Permissions { fn check_open<'a>( &mut self, - _resolved: bool, _read: bool, _write: bool, - _path: &'a Path, + _path: Cow<'a, Path>, _api_name: &str, - ) -> Result, FsError> { + _get_path: &'a dyn deno_fs::GetPath, + ) -> Result, FsError> { unreachable!("snapshotting!") } @@ -231,7 +231,7 @@ impl deno_fs::FsPermissions for Permissions { fn check_read_path<'a>( &mut self, - _path: &'a Path, + _path: Cow<'a, Path>, _api_name: &str, ) -> Result, PermissionCheckError> { unreachable!("snapshotting!") @@ -239,7 +239,7 @@ impl deno_fs::FsPermissions for Permissions { fn check_write_path<'a>( &mut self, - _path: &'a Path, + _path: Cow<'a, Path>, _api_name: &str, ) -> Result, PermissionCheckError> { unreachable!("snapshotting!") @@ -257,7 +257,7 @@ impl deno_kv::sqlite::SqliteDbHandlerPermissions for Permissions { fn check_write<'a>( &mut self, - _path: &'a Path, + _path: Cow<'a, Path>, _api_name: &str, ) -> Result, PermissionCheckError> { unreachable!("snapshotting!")