From 472a37064071c66cd1311cdea2e78de8d2bc0641 Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Fri, 19 Apr 2024 18:12:03 -0600 Subject: [PATCH] feat(runtime): Allow embedders to perform additional access checks on file open (#23208) Embedders may have special requirements around file opening, so we add a new `check_open` permission check that is called as part of the file open process. --- cli/node.rs | 2 +- cli/resolver.rs | 2 +- cli/standalone/file_system.rs | 12 ++- cli/util/gitignore.rs | 2 +- ext/fs/in_memory_fs.rs | 32 +++++-- ext/fs/interface.rs | 67 ++++++++++++--- ext/fs/lib.rs | 34 +++++--- ext/fs/ops.rs | 153 ++++++++++++++++++++++----------- ext/fs/std_fs.rs | 134 ++++++++++++++++++++++++----- ext/io/fs.rs | 15 ++++ ext/node/ops/require.rs | 2 +- ext/node/package_json.rs | 2 +- runtime/permissions.rs | 30 +++++++ runtime/permissions/lib.rs | 87 ++++++++++++++++++- runtime/snapshot.rs | 12 +++ tests/integration/run_tests.rs | 11 ++- tests/unit/read_file_test.ts | 2 +- 17 files changed, 477 insertions(+), 122 deletions(-) diff --git a/cli/node.rs b/cli/node.rs index 5f0ecc65332b0e..aa62e65b28a395 100644 --- a/cli/node.rs +++ b/cli/node.rs @@ -112,7 +112,7 @@ impl CjsCodeAnalyzer for CliCjsCodeAnalyzer { Some(source) => source, None => self .fs - .read_text_file_sync(&specifier.to_file_path().unwrap())?, + .read_text_file_sync(&specifier.to_file_path().unwrap(), None)?, }; let analysis = self.inner_cjs_analysis(specifier, &source)?; match analysis { diff --git a/cli/resolver.rs b/cli/resolver.rs index ea12a6687a305d..dfee9a7048d17d 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -305,7 +305,7 @@ impl NpmModuleLoader { let file_path = specifier.to_file_path().unwrap(); let code = self .fs - .read_text_file_sync(&file_path) + .read_text_file_sync(&file_path, None) .map_err(AnyError::from) .with_context(|| { if file_path.is_dir() { diff --git a/cli/standalone/file_system.rs b/cli/standalone/file_system.rs index f1ea570b54b74b..843c7db557e951 100644 --- a/cli/standalone/file_system.rs +++ b/cli/standalone/file_system.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use std::rc::Rc; use std::sync::Arc; +use deno_runtime::deno_fs::AccessCheckCb; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_fs::FsDirEntry; use deno_runtime::deno_fs::FsFileType; @@ -47,6 +48,7 @@ impl DenoCompileFileSystem { create_new: false, mode: None, }, + None, &old_file_bytes, ) } @@ -75,22 +77,24 @@ impl FileSystem for DenoCompileFileSystem { &self, path: &Path, options: OpenOptions, + access_check: Option, ) -> FsResult> { if self.0.is_path_within(path) { Ok(self.0.open_file(path)?) } else { - RealFs.open_sync(path, options) + RealFs.open_sync(path, options, access_check) } } - async fn open_async( - &self, + async fn open_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, ) -> FsResult> { if self.0.is_path_within(&path) { Ok(self.0.open_file(&path)?) } else { - RealFs.open_async(path, options).await + RealFs.open_async(path, options, access_check).await } } diff --git a/cli/util/gitignore.rs b/cli/util/gitignore.rs index 5601b5db981e6e..12a450d64e019f 100644 --- a/cli/util/gitignore.rs +++ b/cli/util/gitignore.rs @@ -105,7 +105,7 @@ impl GitIgnoreTree { }); let current = self .fs - .read_text_file_sync(&dir_path.join(".gitignore")) + .read_text_file_sync(&dir_path.join(".gitignore"), None) .ok() .and_then(|text| { let mut builder = ignore::gitignore::GitignoreBuilder::new(dir_path); diff --git a/ext/fs/in_memory_fs.rs b/ext/fs/in_memory_fs.rs index fdd0ad7e734f98..153327ff64e5f6 100644 --- a/ext/fs/in_memory_fs.rs +++ b/ext/fs/in_memory_fs.rs @@ -19,6 +19,7 @@ use deno_io::fs::FsError; use deno_io::fs::FsResult; use deno_io::fs::FsStat; +use crate::interface::AccessCheckCb; use crate::interface::FsDirEntry; use crate::interface::FsFileType; use crate::FileSystem; @@ -48,6 +49,7 @@ impl InMemoryFs { .write_file_sync( &path, OpenOptions::write(true, false, false, None), + None, &text.into_bytes(), ) .unwrap(); @@ -82,15 +84,17 @@ impl FileSystem for InMemoryFs { &self, _path: &Path, _options: OpenOptions, + _access_check: Option, ) -> FsResult> { Err(FsError::NotSupported) } - async fn open_async( - &self, + async fn open_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, ) -> FsResult> { - self.open_sync(&path, options) + self.open_sync(&path, options, access_check) } fn mkdir_sync( @@ -350,6 +354,7 @@ impl FileSystem for InMemoryFs { &self, path: &Path, options: OpenOptions, + _access_check: Option, data: &[u8], ) -> FsResult<()> { let path = normalize_path(path); @@ -397,16 +402,21 @@ impl FileSystem for InMemoryFs { } } - async fn write_file_async( - &self, + async fn write_file_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, data: Vec, ) -> FsResult<()> { - self.write_file_sync(&path, options, &data) + self.write_file_sync(&path, options, access_check, &data) } - fn read_file_sync(&self, path: &Path) -> FsResult> { + fn read_file_sync( + &self, + path: &Path, + _access_check: Option, + ) -> FsResult> { let entry = self.get_entry(path); match entry { Some(entry) => match &*entry { @@ -419,7 +429,11 @@ impl FileSystem for InMemoryFs { None => Err(FsError::Io(Error::new(ErrorKind::NotFound, "Not found"))), } } - async fn read_file_async(&self, path: PathBuf) -> FsResult> { - self.read_file_sync(&path) + async fn read_file_async<'a>( + &'a self, + path: PathBuf, + access_check: Option>, + ) -> FsResult> { + self.read_file_sync(&path, access_check) } } diff --git a/ext/fs/interface.rs b/ext/fs/interface.rs index c5a348eb11b3ba..70f9fdf636a17f 100644 --- a/ext/fs/interface.rs +++ b/ext/fs/interface.rs @@ -80,6 +80,25 @@ pub struct FsDirEntry { #[allow(clippy::disallowed_types)] pub type FileSystemRc = crate::sync::MaybeArc; +pub trait AccessCheckFn: + for<'a> FnMut( + bool, + &'a Path, + &'a OpenOptions, +) -> FsResult> +{ +} +impl AccessCheckFn for T where + T: for<'a> FnMut( + bool, + &'a Path, + &'a OpenOptions, + ) -> FsResult> +{ +} + +pub type AccessCheckCb<'a> = &'a mut (dyn AccessCheckFn + 'a); + #[async_trait::async_trait(?Send)] pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync { fn cwd(&self) -> FsResult; @@ -91,11 +110,13 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync { &self, path: &Path, options: OpenOptions, + access_check: Option, ) -> FsResult>; - async fn open_async( - &self, + async fn open_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, ) -> FsResult>; fn mkdir_sync(&self, path: &Path, recursive: bool, mode: u32) @@ -202,22 +223,24 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync { &self, path: &Path, options: OpenOptions, + access_check: Option, data: &[u8], ) -> FsResult<()> { - let file = self.open_sync(path, options)?; + let file = self.open_sync(path, options, access_check)?; if let Some(mode) = options.mode { file.clone().chmod_sync(mode)?; } file.write_all_sync(data)?; Ok(()) } - async fn write_file_async( - &self, + async fn write_file_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, data: Vec, ) -> FsResult<()> { - let file = self.open_async(path, options).await?; + let file = self.open_async(path, options, access_check).await?; if let Some(mode) = options.mode { file.clone().chmod_async(mode).await?; } @@ -225,15 +248,23 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync { Ok(()) } - fn read_file_sync(&self, path: &Path) -> FsResult> { + fn read_file_sync( + &self, + path: &Path, + access_check: Option, + ) -> FsResult> { let options = OpenOptions::read(); - let file = self.open_sync(path, options)?; + let file = self.open_sync(path, options, access_check)?; let buf = file.read_all_sync()?; Ok(buf) } - async fn read_file_async(&self, path: PathBuf) -> FsResult> { + async fn read_file_async<'a>( + &'a self, + path: PathBuf, + access_check: Option>, + ) -> FsResult> { let options = OpenOptions::read(); - let file = self.open_async(path, options).await?; + let file = self.open_async(path, options, access_check).await?; let buf = file.read_all_async().await?; Ok(buf) } @@ -253,14 +284,22 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync { self.stat_sync(path).is_ok() } - fn read_text_file_sync(&self, path: &Path) -> FsResult { - let buf = self.read_file_sync(path)?; + fn read_text_file_sync( + &self, + path: &Path, + access_check: Option, + ) -> FsResult { + let buf = self.read_file_sync(path, access_check)?; String::from_utf8(buf).map_err(|err| { std::io::Error::new(std::io::ErrorKind::InvalidData, err).into() }) } - async fn read_text_file_async(&self, path: PathBuf) -> FsResult { - let buf = self.read_file_async(path).await?; + async fn read_text_file_async<'a>( + &'a self, + path: PathBuf, + access_check: Option>, + ) -> FsResult { + let buf = self.read_file_async(path, access_check).await?; String::from_utf8(buf).map_err(|err| { std::io::Error::new(std::io::ErrorKind::InvalidData, err).into() }) diff --git a/ext/fs/lib.rs b/ext/fs/lib.rs index 05b119e2e1d9f3..d4e79b75f7c1ab 100644 --- a/ext/fs/lib.rs +++ b/ext/fs/lib.rs @@ -7,6 +7,8 @@ mod std_fs; pub mod sync; pub use crate::in_memory_fs::InMemoryFs; +pub use crate::interface::AccessCheckCb; +pub use crate::interface::AccessCheckFn; pub use crate::interface::FileSystem; pub use crate::interface::FileSystemRc; pub use crate::interface::FsDirEntry; @@ -20,9 +22,18 @@ use crate::ops::*; use deno_core::error::AnyError; use deno_core::OpState; +use deno_io::fs::FsError; use std::path::Path; -pub trait FsPermissions { +pub trait FsPermissions: Send + Sync { + fn check_open<'a>( + &mut self, + resolved: bool, + read: bool, + write: bool, + path: &'a Path, + api_name: &str, + ) -> Result, FsError>; fn check_read(&mut self, path: &Path, api_name: &str) -> Result<(), AnyError>; fn check_read_all(&mut self, api_name: &str) -> Result<(), AnyError>; @@ -50,19 +61,20 @@ pub trait FsPermissions { api_name: &str, ) -> Result<(), AnyError>; - fn check( + fn check<'a>( &mut self, + resolved: bool, open_options: &OpenOptions, - path: &Path, + path: &'a Path, api_name: &str, - ) -> Result<(), AnyError> { - if open_options.read { - self.check_read(path, api_name)?; - } - if open_options.write || open_options.append { - self.check_write(path, api_name)?; - } - Ok(()) + ) -> Result, FsError> { + self.check_open( + resolved, + open_options.read, + open_options.write || open_options.append, + path, + api_name, + ) } } diff --git a/ext/fs/ops.rs b/ext/fs/ops.rs index d688f619eeec68..3ff32bd8f22123 100644 --- a/ext/fs/ops.rs +++ b/ext/fs/ops.rs @@ -28,12 +28,55 @@ use rand::Rng; use serde::Serialize; use crate::check_unstable; +use crate::interface::AccessCheckFn; use crate::interface::FileSystemRc; use crate::interface::FsDirEntry; use crate::interface::FsFileType; use crate::FsPermissions; use crate::OpenOptions; +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) + } +} + +fn async_permission_check( + state: Rc>, + api_name: &'static str, +) -> impl AccessCheckFn { + move |resolved, path, options| { + let mut state = state.borrow_mut(); + let permissions = state.borrow_mut::

(); + permissions.check(resolved, options, path, api_name) + } +} + +fn map_permission_error( + operation: &'static str, + error: FsError, + path: &Path, +) -> AnyError { + match error { + FsError::PermissionDenied(err) => { + let path = format!("{path:?}"); + let (path, truncated) = if path.len() > 1024 { + (&path[0..1024], "...(truncated)") + } else { + (path.as_str(), "") + }; + custom_error("PermissionDenied", format!("Requires {err} access to {path}{truncated}, run again with the --allow-{err} flag")) + } + err => Err::<(), _>(err) + .context_path(operation, path) + .err() + .unwrap(), + } +} + #[op2] #[string] pub fn op_fs_cwd

(state: &mut OpState) -> Result @@ -89,12 +132,14 @@ where let path = PathBuf::from(path); let options = options.unwrap_or_else(OpenOptions::read); - let permissions = state.borrow_mut::

(); - permissions.check(&options, &path, "Deno.openSync()")?; - - let fs = state.borrow::(); - let file = fs.open_sync(&path, options).context_path("open", &path)?; + let fs = state.borrow::().clone(); + let mut access_check = + sync_permission_check::

(state.borrow_mut(), "Deno.openSync()"); + let file = fs + .open_sync(&path, options, Some(&mut access_check)) + .map_err(|error| map_permission_error("open", error, &path))?; + drop(access_check); let rid = state .resource_table .add(FileResource::new(file, "fsFile".to_string())); @@ -114,16 +159,13 @@ where let path = PathBuf::from(path); let options = options.unwrap_or_else(OpenOptions::read); - let fs = { - let mut state = state.borrow_mut(); - let permissions = state.borrow_mut::

(); - permissions.check(&options, &path, "Deno.open()")?; - state.borrow::().clone() - }; + let mut access_check = + async_permission_check::

(state.clone(), "Deno.open()"); + let fs = state.borrow().borrow::().clone(); let file = fs - .open_async(path.clone(), options) + .open_async(path.clone(), options, Some(&mut access_check)) .await - .context_path("open", &path)?; + .map_err(|error| map_permission_error("open", error, &path))?; let rid = state .borrow_mut() @@ -961,11 +1003,10 @@ where }; let mut rng = thread_rng(); - const MAX_TRIES: u32 = 10; for _ in 0..MAX_TRIES { let path = tmp_name(&mut rng, &dir, prefix.as_deref(), suffix.as_deref())?; - match fs.open_sync(&path, open_opts) { + match fs.open_sync(&path, open_opts, None) { Ok(_) => return path_into_string(path.into_os_string()), Err(FsError::Io(ref e)) if e.kind() == io::ErrorKind::AlreadyExists => { continue; @@ -1007,7 +1048,7 @@ where const MAX_TRIES: u32 = 10; for _ in 0..MAX_TRIES { let path = tmp_name(&mut rng, &dir, prefix.as_deref(), suffix.as_deref())?; - match fs.clone().open_async(path.clone(), open_opts).await { + match fs.clone().open_async(path.clone(), open_opts, None).await { Ok(_) => return path_into_string(path.into_os_string()), Err(FsError::Io(ref e)) if e.kind() == io::ErrorKind::AlreadyExists => { continue; @@ -1150,14 +1191,13 @@ where { let path = PathBuf::from(path); - let permissions = state.borrow_mut::

(); let options = OpenOptions::write(create, append, create_new, mode); - permissions.check(&options, &path, "Deno.writeFileSync()")?; - - let fs = state.borrow::(); + let fs = state.borrow::().clone(); + let mut access_check = + sync_permission_check::

(state.borrow_mut(), "Deno.writeFileSync()"); - fs.write_file_sync(&path, options, &data) - .context_path("writefile", &path)?; + fs.write_file_sync(&path, options, Some(&mut access_check), &data) + .map_err(|error| map_permission_error("writefile", error, &path))?; Ok(()) } @@ -1181,16 +1221,21 @@ where let options = OpenOptions::write(create, append, create_new, mode); + let mut access_check = + async_permission_check::

(state.clone(), "Deno.writeFile()"); let (fs, cancel_handle) = { - let mut state = state.borrow_mut(); - let permissions = state.borrow_mut::

(); - permissions.check(&options, &path, "Deno.writeFile()")?; + let state = state.borrow_mut(); let cancel_handle = cancel_rid .and_then(|rid| state.resource_table.get::(rid).ok()); (state.borrow::().clone(), cancel_handle) }; - let fut = fs.write_file_async(path.clone(), options, data.to_vec()); + let fut = fs.write_file_async( + path.clone(), + options, + Some(&mut access_check), + data.to_vec(), + ); if let Some(cancel_handle) = cancel_handle { let res = fut.or_cancel(cancel_handle).await; @@ -1201,9 +1246,11 @@ where } }; - res?.context_path("writefile", &path)?; + res?.map_err(|error| map_permission_error("writefile", error, &path))?; } else { - fut.await.context_path("writefile", &path)?; + fut + .await + .map_err(|error| map_permission_error("writefile", error, &path))?; } Ok(()) @@ -1220,11 +1267,12 @@ where { let path = PathBuf::from(path); - let permissions = state.borrow_mut::

(); - permissions.check_read(&path, "Deno.readFileSync()")?; - - let fs = state.borrow::(); - let buf = fs.read_file_sync(&path).context_path("readfile", &path)?; + let fs = state.borrow::().clone(); + let mut access_check = + sync_permission_check::

(state.borrow_mut(), "Deno.readFileSync()"); + let buf = fs + .read_file_sync(&path, Some(&mut access_check)) + .map_err(|error| map_permission_error("readfile", error, &path))?; Ok(buf.into()) } @@ -1241,16 +1289,16 @@ where { let path = PathBuf::from(path); + let mut access_check = + async_permission_check::

(state.clone(), "Deno.readFile()"); let (fs, cancel_handle) = { - let mut state = state.borrow_mut(); - let permissions = state.borrow_mut::

(); - permissions.check_read(&path, "Deno.readFile()")?; + let state = state.borrow(); let cancel_handle = cancel_rid .and_then(|rid| state.resource_table.get::(rid).ok()); (state.borrow::().clone(), cancel_handle) }; - let fut = fs.read_file_async(path.clone()); + let fut = fs.read_file_async(path.clone(), Some(&mut access_check)); let buf = if let Some(cancel_handle) = cancel_handle { let res = fut.or_cancel(cancel_handle).await; @@ -1261,9 +1309,11 @@ where } }; - res?.context_path("readfile", &path)? + res?.map_err(|error| map_permission_error("readfile", error, &path))? } else { - fut.await.context_path("readfile", &path)? + fut + .await + .map_err(|error| map_permission_error("readfile", error, &path))? }; Ok(buf.into()) @@ -1280,11 +1330,12 @@ where { let path = PathBuf::from(path); - let permissions = state.borrow_mut::

(); - permissions.check_read(&path, "Deno.readFileSync()")?; - - let fs = state.borrow::(); - let buf = fs.read_file_sync(&path).context_path("readfile", &path)?; + let fs = state.borrow::().clone(); + let mut access_check = + sync_permission_check::

(state.borrow_mut(), "Deno.readFileSync()"); + let buf = fs + .read_file_sync(&path, Some(&mut access_check)) + .map_err(|error| map_permission_error("readfile", error, &path))?; Ok(string_from_utf8_lossy(buf)) } @@ -1301,16 +1352,16 @@ where { let path = PathBuf::from(path); + let mut access_check = + async_permission_check::

(state.clone(), "Deno.readFile()"); let (fs, cancel_handle) = { - let mut state = state.borrow_mut(); - let permissions = state.borrow_mut::

(); - permissions.check_read(&path, "Deno.readFile()")?; + let state = state.borrow_mut(); let cancel_handle = cancel_rid .and_then(|rid| state.resource_table.get::(rid).ok()); (state.borrow::().clone(), cancel_handle) }; - let fut = fs.read_file_async(path.clone()); + let fut = fs.read_file_async(path.clone(), Some(&mut access_check)); let buf = if let Some(cancel_handle) = cancel_handle { let res = fut.or_cancel(cancel_handle).await; @@ -1321,9 +1372,11 @@ where } }; - res?.context_path("readfile", &path)? + res?.map_err(|error| map_permission_error("readfile", error, &path))? } else { - fut.await.context_path("readfile", &path)? + fut + .await + .map_err(|error| map_permission_error("readfile", error, &path))? }; Ok(string_from_utf8_lossy(buf)) diff --git a/ext/fs/std_fs.rs b/ext/fs/std_fs.rs index 332866e457c0c6..1176697de858e1 100644 --- a/ext/fs/std_fs.rs +++ b/ext/fs/std_fs.rs @@ -2,27 +2,29 @@ #![allow(clippy::disallowed_methods)] +use std::env::current_dir; use std::fs; use std::io; +use std::io::Read; use std::io::Write; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; +use deno_core::normalize_path; use deno_core::unsync::spawn_blocking; use deno_io::fs::File; +use deno_io::fs::FsError; use deno_io::fs::FsResult; use deno_io::fs::FsStat; use deno_io::StdFileResourceInner; +use crate::interface::AccessCheckCb; use crate::interface::FsDirEntry; use crate::interface::FsFileType; use crate::FileSystem; use crate::OpenOptions; -#[cfg(not(unix))] -use deno_io::fs::FsError; - #[derive(Debug, Clone)] pub struct RealFs; @@ -80,18 +82,18 @@ impl FileSystem for RealFs { &self, path: &Path, options: OpenOptions, + access_check: Option, ) -> FsResult> { - let opts = open_options(options); - let std_file = opts.open(path)?; + let std_file = open_with_access_check(options, path, access_check)?; Ok(Rc::new(StdFileResourceInner::file(std_file))) } - async fn open_async( - &self, + async fn open_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, ) -> FsResult> { - let opts = open_options(options); - let std_file = spawn_blocking(move || opts.open(path)).await??; + let std_file = open_with_access_check(options, &path, access_check)?; Ok(Rc::new(StdFileResourceInner::file(std_file))) } @@ -276,10 +278,10 @@ impl FileSystem for RealFs { &self, path: &Path, options: OpenOptions, + access_check: Option, data: &[u8], ) -> FsResult<()> { - let opts = open_options(options); - let mut file = opts.open(path)?; + let mut file = open_with_access_check(options, path, access_check)?; #[cfg(unix)] if let Some(mode) = options.mode { use std::os::unix::fs::PermissionsExt; @@ -289,15 +291,15 @@ impl FileSystem for RealFs { Ok(()) } - async fn write_file_async( - &self, + async fn write_file_async<'a>( + &'a self, path: PathBuf, options: OpenOptions, + access_check: Option>, data: Vec, ) -> FsResult<()> { + let mut file = open_with_access_check(options, &path, access_check)?; spawn_blocking(move || { - let opts = open_options(options); - let mut file = opts.open(path)?; #[cfg(unix)] if let Some(mode) = options.mode { use std::os::unix::fs::PermissionsExt; @@ -309,13 +311,43 @@ impl FileSystem for RealFs { .await? } - fn read_file_sync(&self, path: &Path) -> FsResult> { - fs::read(path).map_err(Into::into) - } - async fn read_file_async(&self, path: PathBuf) -> FsResult> { - spawn_blocking(move || fs::read(path)) - .await? - .map_err(Into::into) + fn read_file_sync( + &self, + path: &Path, + access_check: Option, + ) -> FsResult> { + let mut file = open_with_access_check( + OpenOptions { + read: true, + ..Default::default() + }, + path, + access_check, + )?; + let mut buf = Vec::new(); + file.read_to_end(&mut buf)?; + Ok(buf) + } + async fn read_file_async<'a>( + &'a self, + path: PathBuf, + access_check: Option>, + ) -> FsResult> { + let mut file = open_with_access_check( + OpenOptions { + read: true, + ..Default::default() + }, + &path, + access_check, + )?; + spawn_blocking(move || { + let mut buf = Vec::new(); + file.read_to_end(&mut buf)?; + Ok::<_, FsError>(buf) + }) + .await? + .map_err(Into::into) } } @@ -410,7 +442,6 @@ fn copy_file(from: &Path, to: &Path) -> FsResult<()> { use libc::stat; use libc::unlink; use std::ffi::CString; - use std::io::Read; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::fs::PermissionsExt; @@ -845,3 +876,60 @@ fn open_options(options: OpenOptions) -> fs::OpenOptions { open_options.create_new(options.create_new); open_options } + +#[inline(always)] +fn open_with_access_check( + options: OpenOptions, + path: &Path, + access_check: Option, +) -> FsResult { + if let Some(access_check) = access_check { + let path = if path.is_absolute() { + normalize_path(path) + } else { + let cwd = current_dir()?; + normalize_path(cwd.join(path)) + }; + (*access_check)(false, &path, &options)?; + // On Linux, /proc may contain magic links that we don't want to resolve + let needs_canonicalization = + !cfg!(target_os = "linux") || path.starts_with("/proc"); + 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)?; + + // For windows + #[allow(unused_mut)] + let mut opts: fs::OpenOptions = open_options(options); + #[cfg(unix)] + { + // Don't follow symlinks on open -- we must always pass fully-resolved files + // 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 { + opts.custom_flags(libc::O_NOFOLLOW); + } + } + + Ok(opts.open(&path)?) + } else { + let opts = open_options(options); + Ok(opts.open(path)?) + } +} diff --git a/ext/io/fs.rs b/ext/io/fs.rs index d8f39355651da4..88e4eee4742404 100644 --- a/ext/io/fs.rs +++ b/ext/io/fs.rs @@ -6,6 +6,7 @@ use std::rc::Rc; use std::time::SystemTime; use std::time::UNIX_EPOCH; +use deno_core::error::custom_error; use deno_core::error::not_supported; use deno_core::error::resource_unavailable; use deno_core::error::AnyError; @@ -21,6 +22,7 @@ pub enum FsError { Io(io::Error), FileBusy, NotSupported, + PermissionDenied(&'static str), } impl FsError { @@ -29,6 +31,7 @@ impl FsError { Self::Io(err) => err.kind(), Self::FileBusy => io::ErrorKind::Other, Self::NotSupported => io::ErrorKind::Other, + Self::PermissionDenied(_) => io::ErrorKind::PermissionDenied, } } @@ -37,6 +40,9 @@ impl FsError { FsError::Io(err) => err, FsError::FileBusy => io::Error::new(self.kind(), "file busy"), FsError::NotSupported => io::Error::new(self.kind(), "not supported"), + FsError::PermissionDenied(err) => { + io::Error::new(self.kind(), format!("requires {err} access")) + } } } } @@ -47,12 +53,21 @@ impl From for FsError { } } +impl From for FsError { + fn from(err: io::ErrorKind) -> Self { + Self::Io(err.into()) + } +} + impl From for AnyError { fn from(err: FsError) -> Self { match err { FsError::Io(err) => AnyError::from(err), FsError::FileBusy => resource_unavailable(), FsError::NotSupported => not_supported(), + FsError::PermissionDenied(err) => { + custom_error("PermissionDenied", format!("permission denied: {err}")) + } } } } diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index 426c419956961b..176d64e56ba37b 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -452,7 +452,7 @@ where let file_path = PathBuf::from(file_path); ensure_read_permission::

(state, &file_path)?; let fs = state.borrow::(); - Ok(fs.read_text_file_sync(&file_path)?) + Ok(fs.read_text_file_sync(&file_path, None)?) } #[op2] diff --git a/ext/node/package_json.rs b/ext/node/package_json.rs index 77352ae1df2b19..d4ffc80d6876b0 100644 --- a/ext/node/package_json.rs +++ b/ext/node/package_json.rs @@ -82,7 +82,7 @@ impl PackageJson { return Ok(CACHE.with(|cache| cache.borrow()[&path].clone())); } - let source = match fs.read_text_file_sync(&path) { + let source = match fs.read_text_file_sync(&path, None) { Ok(source) => source, Err(err) if err.kind() == ErrorKind::NotFound => { return Ok(Rc::new(PackageJson::empty(path))); diff --git a/runtime/permissions.rs b/runtime/permissions.rs index ccd0d3254b4930..edd03e1d5276d8 100644 --- a/runtime/permissions.rs +++ b/runtime/permissions.rs @@ -1,9 +1,11 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::borrow::Cow; use std::path::Path; use deno_core::error::AnyError; use deno_core::url::Url; +pub use deno_io::fs::FsError; pub use deno_permissions::create_child_permissions; pub use deno_permissions::parse_sys_kind; pub use deno_permissions::set_prompt_callbacks; @@ -142,6 +144,34 @@ impl deno_websocket::WebSocketPermissions for PermissionsContainer { } impl deno_fs::FsPermissions for PermissionsContainer { + fn check_open<'a>( + &mut self, + resolved: bool, + read: bool, + write: bool, + path: &'a Path, + api_name: &str, + ) -> Result, FsError> { + if resolved { + self.check_special_file(path, api_name).map_err(|_| { + std::io::Error::from(std::io::ErrorKind::PermissionDenied) + })?; + return Ok(Cow::Borrowed(path)); + } + + // If somehow read or write aren't specified, use read + let read = read || !write; + if read { + deno_fs::FsPermissions::check_read(self, path, api_name) + .map_err(|_| FsError::PermissionDenied("read"))?; + } + if write { + deno_fs::FsPermissions::check_write(self, path, api_name) + .map_err(|_| FsError::PermissionDenied("write"))?; + } + Ok(Cow::Borrowed(path)) + } + fn check_read( &mut self, path: &Path, diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 1b00dc2c777246..1ac8779afe60fc 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -21,6 +21,7 @@ use fqdn::FQDN; use once_cell::sync::Lazy; use std::borrow::Cow; use std::collections::HashSet; +use std::ffi::OsStr; use std::fmt; use std::fmt::Debug; use std::hash::Hash; @@ -1641,6 +1642,91 @@ impl PermissionsContainer { self.0.lock().env.check_all() } + #[inline(always)] + pub fn check_sys_all(&mut self) -> Result<(), AnyError> { + self.0.lock().sys.check_all() + } + + #[inline(always)] + pub fn check_ffi_all(&mut self) -> Result<(), AnyError> { + self.0.lock().ffi.check_all() + } + + /// This checks to see if the allow-all flag was passed, not whether all + /// permissions are enabled! + #[inline(always)] + pub fn check_was_allow_all_flag_passed(&mut self) -> Result<(), AnyError> { + self.0.lock().all.check() + } + + /// Checks special file access, returning the failed permission type if + /// not successful. + pub fn check_special_file( + &mut self, + path: &Path, + _api_name: &str, + ) -> Result<(), &'static str> { + let error_all = |_| "all"; + + // Safe files with no major additional side-effects. While there's a small risk of someone + // draining system entropy by just reading one of these files constantly, that's not really + // something we worry about as they already have --allow-read to /dev. + if cfg!(unix) + && (path == OsStr::new("/dev/random") + || path == OsStr::new("/dev/urandom") + || path == OsStr::new("/dev/zero") + || path == OsStr::new("/dev/null")) + { + return Ok(()); + } + + if cfg!(target_os = "linux") { + if path.starts_with("/dev") + || path.starts_with("/proc") + || path.starts_with("/sys") + { + if path.ends_with("/environ") { + self.check_env_all().map_err(|_| "env")?; + } else { + self.check_was_allow_all_flag_passed().map_err(error_all)?; + } + } + if path.starts_with("/etc") { + self.check_was_allow_all_flag_passed().map_err(error_all)?; + } + } else if cfg!(unix) { + if path.starts_with("/dev") { + self.check_was_allow_all_flag_passed().map_err(error_all)?; + } + if path.starts_with("/etc") { + self.check_was_allow_all_flag_passed().map_err(error_all)?; + } + if path.starts_with("/private/etc") { + self.check_was_allow_all_flag_passed().map_err(error_all)?; + } + } else if cfg!(target_os = "windows") { + fn is_normalized_windows_drive_path(path: &Path) -> bool { + let s = path.as_os_str().as_encoded_bytes(); + // \\?\X:\ + if s.len() < 7 { + false + } else if s.starts_with(br#"\\?\"#) { + s[4].is_ascii_alphabetic() && s[5] == b':' && s[6] == b'\\' + } else { + false + } + } + + // If this is a normalized drive path, accept it + if !is_normalized_windows_drive_path(path) { + self.check_was_allow_all_flag_passed().map_err(error_all)?; + } + } else { + unimplemented!() + } + Ok(()) + } + #[inline(always)] pub fn check_net_url( &mut self, @@ -2795,7 +2881,6 @@ mod tests { fn test_check_fail() { set_prompter(Box::new(TestPrompter)); let mut perms = Permissions::none_with_prompt(); - let prompt_value = PERMISSION_PROMPT_STUB_VALUE_SETTER.lock(); prompt_value.set(false); diff --git a/runtime/snapshot.rs b/runtime/snapshot.rs index 54652e1f1fc05c..d5122af84fcc98 100644 --- a/runtime/snapshot.rs +++ b/runtime/snapshot.rs @@ -10,6 +10,7 @@ use deno_core::snapshot::*; use deno_core::v8; use deno_core::Extension; use deno_http::DefaultHttpPropertyExtractor; +use deno_io::fs::FsError; use std::io::Write; use std::path::Path; use std::path::PathBuf; @@ -129,6 +130,17 @@ 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, + _api_name: &str, + ) -> Result, FsError> { + unreachable!("snapshotting!") + } + fn check_read( &mut self, _path: &Path, diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 7b0bc78edca064..dfbebeaf1b9aea 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -704,7 +704,7 @@ fn permission_request_long() { .args_vec(["run", "--quiet", "run/permission_request_long.ts"]) .with_pty(|mut console| { console.expect(concat!( - "❌ Permission prompt length (100017 bytes) was larger than the configured maximum length (10240 bytes): denying request.\r\n", + "was larger than the configured maximum length (10240 bytes): denying request.\r\n", "❌ WARNING: This may indicate that code is trying to bypass or hide permission check requests.\r\n", "❌ Run again with --allow-read to bypass this check if this is really what you want to do.\r\n", )); @@ -4309,7 +4309,10 @@ fn fsfile_set_raw_should_not_panic_on_no_tty() { .unwrap(); assert!(!output.status.success()); let stderr = std::str::from_utf8(&output.stderr).unwrap().trim(); - assert!(stderr.contains("BadResource")); + assert!( + stderr.contains("BadResource"), + "stderr did not contain BadResource: {stderr}" + ); } #[test] @@ -4674,7 +4677,7 @@ fn stdio_streams_are_locked_in_permission_prompt() { console.write_line(r#"new Worker(URL.createObjectURL(new Blob(["setInterval(() => console.log('**malicious**'), 10)"])), { type: "module" });"#); // The worker is now spamming console.expect(malicious_output); - console.write_line(r#"Deno.readTextFileSync('Cargo.toml');"#); + console.write_line(r#"Deno.readTextFileSync('../Cargo.toml');"#); // We will get a permission prompt console.expect("Allow? [y/n/A] (y = yes, allow; n = no, deny; A = allow all read permissions) > "); // The worker is blocked, so nothing else should get written here @@ -4690,7 +4693,7 @@ fn stdio_streams_are_locked_in_permission_prompt() { console.human_delay(); console.write_line_raw("y"); // We ensure that nothing gets written here between the permission prompt and this text, despire the delay - console.expect_raw_next(format!("y{newline}\x1b[4A\x1b[0J✅ Granted read access to \"Cargo.toml\".")); + console.expect_raw_next(format!("y{newline}\x1b[4A\x1b[0J✅ Granted read access to \"")); // Back to spamming! console.expect(malicious_output); diff --git a/tests/unit/read_file_test.ts b/tests/unit/read_file_test.ts index bfb3b508512887..1b7f2a5513bdcb 100644 --- a/tests/unit/read_file_test.ts +++ b/tests/unit/read_file_test.ts @@ -173,7 +173,7 @@ Deno.test( await Deno.readFile("tests/testdata/assets/"); } catch (e) { if (Deno.build.os === "windows") { - assertEquals(e.code, "ENOENT"); + assertEquals(e.code, "EPERM"); } else { assertEquals(e.code, "EISDIR"); }