From 7f80f9db3f4c3b064b230adfec7ff958fc195da6 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 20 Jan 2020 14:45:44 +0000 Subject: [PATCH] refactor: Improve path handling in permission checks (#3714) --- cli/flags.rs | 6 +-- cli/fs.rs | 20 ++++---- cli/global_state.rs | 7 +-- cli/ops/files.rs | 11 +++-- cli/ops/fs.rs | 95 +++++++++++++++++-------------------- cli/ops/permissions.rs | 20 ++++---- cli/ops/plugins.rs | 5 +- cli/ops/tls.rs | 7 +-- cli/permissions.rs | 104 +++++++++++++++++++++-------------------- cli/state.rs | 15 +++--- 10 files changed, 144 insertions(+), 146 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 4766fbe5fb..42a876947d 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -7,6 +7,7 @@ use clap::ArgMatches; use clap::SubCommand; use log::Level; use std::collections::HashSet; +use std::path::Path; /// Creates vector of strings, Vec macro_rules! svec { @@ -397,14 +398,13 @@ fn resolve_fs_whitelist(whitelist: &[String]) -> Vec { whitelist .iter() .map(|raw_path| { - resolve_from_cwd(&raw_path) + resolve_from_cwd(Path::new(&raw_path)) .unwrap() - .0 .to_str() .unwrap() .to_owned() }) - .collect::>() + .collect() } // Shared between the run and test subcommands. They both take similar options. diff --git a/cli/fs.rs b/cli/fs.rs index cdbc88d801..272eaf84c5 100644 --- a/cli/fs.rs +++ b/cli/fs.rs @@ -129,11 +129,9 @@ pub fn chown(_path: &str, _uid: u32, _gid: u32) -> Result<(), ErrBox> { Err(crate::deno_error::op_not_implemented()) } -pub fn resolve_from_cwd(path: &str) -> Result<(PathBuf, String), ErrBox> { - let candidate_path = Path::new(path); - - let resolved_path = if candidate_path.is_absolute() { - candidate_path.to_owned() +pub fn resolve_from_cwd(path: &Path) -> Result { + let resolved_path = if path.is_absolute() { + path.to_owned() } else { let cwd = std::env::current_dir().unwrap(); cwd.join(path) @@ -155,9 +153,7 @@ pub fn resolve_from_cwd(path: &str) -> Result<(PathBuf, String), ErrBox> { .to_file_path() .expect("URL from a path should contain a valid path"); - let path_string = normalized_path.to_str().unwrap().to_string(); - - Ok((normalized_path, path_string)) + Ok(normalized_path) } #[cfg(test)] @@ -167,19 +163,19 @@ mod tests { #[test] fn resolve_from_cwd_child() { let cwd = std::env::current_dir().unwrap(); - assert_eq!(resolve_from_cwd("a").unwrap().0, cwd.join("a")); + assert_eq!(resolve_from_cwd(Path::new("a")).unwrap(), cwd.join("a")); } #[test] fn resolve_from_cwd_dot() { let cwd = std::env::current_dir().unwrap(); - assert_eq!(resolve_from_cwd(".").unwrap().0, cwd); + assert_eq!(resolve_from_cwd(Path::new(".")).unwrap(), cwd); } #[test] fn resolve_from_cwd_parent() { let cwd = std::env::current_dir().unwrap(); - assert_eq!(resolve_from_cwd("a/..").unwrap().0, cwd); + assert_eq!(resolve_from_cwd(Path::new("a/..")).unwrap(), cwd); } // TODO: Get a good expected value here for Windows. @@ -187,6 +183,6 @@ mod tests { #[test] fn resolve_from_cwd_absolute() { let expected = Path::new("/a"); - assert_eq!(resolve_from_cwd("/a").unwrap().0, expected); + assert_eq!(resolve_from_cwd(expected).unwrap(), expected); } } diff --git a/cli/global_state.rs b/cli/global_state.rs index d42a50a763..1709a3429b 100644 --- a/cli/global_state.rs +++ b/cli/global_state.rs @@ -19,6 +19,7 @@ use std; use std::env; use std::future::Future; use std::ops::Deref; +use std::path::Path; use std::str; use std::sync::Arc; use std::sync::Mutex; @@ -175,12 +176,12 @@ impl ThreadSafeGlobalState { } #[inline] - pub fn check_read(&self, filename: &str) -> Result<(), ErrBox> { + pub fn check_read(&self, filename: &Path) -> Result<(), ErrBox> { self.permissions.check_read(filename) } #[inline] - pub fn check_write(&self, filename: &str) -> Result<(), ErrBox> { + pub fn check_write(&self, filename: &Path) -> Result<(), ErrBox> { self.permissions.check_write(filename) } @@ -221,7 +222,7 @@ impl ThreadSafeGlobalState { .into_os_string() .into_string() .unwrap(); - self.check_read(&filename)?; + self.check_read(Path::new(&filename))?; Ok(()) } _ => Err(permission_denied()), diff --git a/cli/ops/files.rs b/cli/ops/files.rs index 5a86940192..8bb3c8acb9 100644 --- a/cli/ops/files.rs +++ b/cli/ops/files.rs @@ -12,6 +12,7 @@ use futures::future::FutureExt; use std; use std::convert::From; use std::io::SeekFrom; +use std::path::Path; use tokio; pub fn init(i: &mut Isolate, s: &ThreadSafeState) { @@ -34,7 +35,7 @@ fn op_open( _zero_copy: Option, ) -> Result { let args: OpenArgs = serde_json::from_value(args)?; - let (filename, filename_) = deno_fs::resolve_from_cwd(&args.filename)?; + let filename = deno_fs::resolve_from_cwd(Path::new(&args.filename))?; let mode = args.mode.as_ref(); let state_ = state.clone(); let mut open_options = tokio::fs::OpenOptions::new(); @@ -75,14 +76,14 @@ fn op_open( match mode { "r" => { - state.check_read(&filename_)?; + state.check_read(&filename)?; } "w" | "a" | "x" => { - state.check_write(&filename_)?; + state.check_write(&filename)?; } &_ => { - state.check_read(&filename_)?; - state.check_write(&filename_)?; + state.check_read(&filename)?; + state.check_write(&filename)?; } } diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs index c6830ddd2a..80972aef14 100644 --- a/cli/ops/fs.rs +++ b/cli/ops/fs.rs @@ -10,7 +10,7 @@ use deno_core::*; use remove_dir_all::remove_dir_all; use std::convert::From; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::UNIX_EPOCH; #[cfg(unix)] @@ -71,13 +71,13 @@ fn op_mkdir( _zero_copy: Option, ) -> Result { let args: MkdirArgs = serde_json::from_value(args)?; - let (path, path_) = deno_fs::resolve_from_cwd(args.path.as_ref())?; + let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?; - state.check_write(&path_)?; + state.check_write(&path)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { - debug!("op_mkdir {}", path_); + debug!("op_mkdir {}", path.display()); deno_fs::mkdir(&path, args.mode, args.recursive)?; Ok(json!({})) }) @@ -98,13 +98,13 @@ fn op_chmod( _zero_copy: Option, ) -> Result { let args: ChmodArgs = serde_json::from_value(args)?; - let (path, path_) = deno_fs::resolve_from_cwd(args.path.as_ref())?; + let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?; - state.check_write(&path_)?; + state.check_write(&path)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { - debug!("op_chmod {}", &path_); + debug!("op_chmod {}", path.display()); // Still check file/dir exists on windows let _metadata = fs::metadata(&path)?; #[cfg(any(unix))] @@ -132,12 +132,13 @@ fn op_chown( _zero_copy: Option, ) -> Result { let args: ChownArgs = serde_json::from_value(args)?; + let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?; - state.check_write(&args.path)?; + state.check_write(&path)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { - debug!("op_chown {}", &args.path); + debug!("op_chown {}", path.display()); match deno_fs::chown(args.path.as_ref(), args.uid, args.gid) { Ok(_) => Ok(json!({})), Err(e) => Err(e), @@ -159,10 +160,10 @@ fn op_remove( _zero_copy: Option, ) -> Result { let args: RemoveArgs = serde_json::from_value(args)?; - let (path, path_) = deno_fs::resolve_from_cwd(args.path.as_ref())?; + let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?; let recursive = args.recursive; - state.check_write(&path_)?; + state.check_write(&path)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { @@ -193,12 +194,11 @@ fn op_copy_file( _zero_copy: Option, ) -> Result { let args: CopyFileArgs = serde_json::from_value(args)?; + let from = deno_fs::resolve_from_cwd(Path::new(&args.from))?; + let to = deno_fs::resolve_from_cwd(Path::new(&args.to))?; - let (from, from_) = deno_fs::resolve_from_cwd(args.from.as_ref())?; - let (to, to_) = deno_fs::resolve_from_cwd(args.to.as_ref())?; - - state.check_read(&from_)?; - state.check_write(&to_)?; + state.check_read(&from)?; + state.check_write(&to)?; debug!("op_copy_file {} {}", from.display(), to.display()); let is_sync = args.promise_id.is_none(); @@ -293,12 +293,10 @@ fn op_stat( _zero_copy: Option, ) -> Result { let args: StatArgs = serde_json::from_value(args)?; - - let (filename, filename_) = - deno_fs::resolve_from_cwd(args.filename.as_ref())?; + let filename = deno_fs::resolve_from_cwd(Path::new(&args.filename))?; let lstat = args.lstat; - state.check_read(&filename_)?; + state.check_read(&filename)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { @@ -325,12 +323,13 @@ fn op_realpath( _zero_copy: Option, ) -> Result { let args: RealpathArgs = serde_json::from_value(args)?; - let (_, path_) = deno_fs::resolve_from_cwd(args.path.as_ref())?; - state.check_read(&path_)?; - let path = args.path.clone(); + let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?; + + state.check_read(&path)?; + let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { - debug!("op_realpath {}", &path); + debug!("op_realpath {}", path.display()); // corresponds to the realpath on Unix and // CreateFile and GetFinalPathNameByHandle on Windows let realpath = fs::canonicalize(&path)?; @@ -356,14 +355,13 @@ fn op_read_dir( _zero_copy: Option, ) -> Result { let args: ReadDirArgs = serde_json::from_value(args)?; - let (path, path_) = deno_fs::resolve_from_cwd(args.path.as_ref())?; + let path = deno_fs::resolve_from_cwd(Path::new(&args.path))?; - state.check_read(&path_)?; + state.check_read(&path)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { debug!("op_read_dir {}", path.display()); - let entries: Vec<_> = fs::read_dir(path)? .map(|entry| { let entry = entry.unwrap(); @@ -394,13 +392,12 @@ fn op_rename( _zero_copy: Option, ) -> Result { let args: RenameArgs = serde_json::from_value(args)?; + let oldpath = deno_fs::resolve_from_cwd(Path::new(&args.oldpath))?; + let newpath = deno_fs::resolve_from_cwd(Path::new(&args.newpath))?; - let (oldpath, oldpath_) = deno_fs::resolve_from_cwd(args.oldpath.as_ref())?; - let (newpath, newpath_) = deno_fs::resolve_from_cwd(args.newpath.as_ref())?; - - state.check_read(&oldpath_)?; - state.check_write(&oldpath_)?; - state.check_write(&newpath_)?; + state.check_read(&oldpath)?; + state.check_write(&oldpath)?; + state.check_write(&newpath)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { @@ -424,12 +421,11 @@ fn op_link( _zero_copy: Option, ) -> Result { let args: LinkArgs = serde_json::from_value(args)?; + let oldname = deno_fs::resolve_from_cwd(Path::new(&args.oldname))?; + let newname = deno_fs::resolve_from_cwd(Path::new(&args.newname))?; - let (oldname, oldname_) = deno_fs::resolve_from_cwd(args.oldname.as_ref())?; - let (newname, newname_) = deno_fs::resolve_from_cwd(args.newname.as_ref())?; - - state.check_read(&oldname_)?; - state.check_write(&newname_)?; + state.check_read(&oldname)?; + state.check_write(&newname)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { @@ -453,11 +449,10 @@ fn op_symlink( _zero_copy: Option, ) -> Result { let args: SymlinkArgs = serde_json::from_value(args)?; + let oldname = deno_fs::resolve_from_cwd(Path::new(&args.oldname))?; + let newname = deno_fs::resolve_from_cwd(Path::new(&args.newname))?; - let (oldname, _oldname_) = deno_fs::resolve_from_cwd(args.oldname.as_ref())?; - let (newname, newname_) = deno_fs::resolve_from_cwd(args.newname.as_ref())?; - - state.check_write(&newname_)?; + state.check_write(&newname)?; // TODO Use type for Windows. if cfg!(windows) { return Err( @@ -486,10 +481,9 @@ fn op_read_link( _zero_copy: Option, ) -> Result { let args: ReadLinkArgs = serde_json::from_value(args)?; + let name = deno_fs::resolve_from_cwd(Path::new(&args.name))?; - let (name, name_) = deno_fs::resolve_from_cwd(args.name.as_ref())?; - - state.check_read(&name_)?; + state.check_read(&name)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { @@ -515,15 +509,14 @@ fn op_truncate( _zero_copy: Option, ) -> Result { let args: TruncateArgs = serde_json::from_value(args)?; - - let (filename, filename_) = deno_fs::resolve_from_cwd(args.name.as_ref())?; + let filename = deno_fs::resolve_from_cwd(Path::new(&args.name))?; let len = args.len; - state.check_write(&filename_)?; + state.check_write(&filename)?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { - debug!("op_truncate {} {}", filename_, len); + debug!("op_truncate {} {}", filename.display(), len); let f = fs::OpenOptions::new().write(true).open(&filename)?; f.set_len(len)?; Ok(json!({})) @@ -547,7 +540,7 @@ fn op_make_temp_dir( let args: MakeTempDirArgs = serde_json::from_value(args)?; // FIXME - state.check_write("make_temp")?; + state.check_write(Path::new("make_temp"))?; let dir = args.dir.map(PathBuf::from); let prefix = args.prefix.map(String::from); @@ -585,7 +578,7 @@ fn op_utime( _zero_copy: Option, ) -> Result { let args: Utime = serde_json::from_value(args)?; - state.check_write(&args.filename)?; + state.check_write(Path::new(&args.filename))?; let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { debug!("op_utimes {} {} {}", args.filename, args.atime, args.mtime); diff --git a/cli/ops/permissions.rs b/cli/ops/permissions.rs index 6a0186acc9..172ce97b15 100644 --- a/cli/ops/permissions.rs +++ b/cli/ops/permissions.rs @@ -5,6 +5,7 @@ use crate::fs as deno_fs; use crate::ops::json_op; use crate::state::ThreadSafeState; use deno_core::*; +use std::path::Path; pub fn init(i: &mut Isolate, s: &ThreadSafeState) { i.register_op( @@ -29,9 +30,8 @@ struct PermissionArgs { } fn resolve_path(path: &str) -> String { - deno_fs::resolve_from_cwd(path) + deno_fs::resolve_from_cwd(Path::new(path)) .unwrap() - .0 .to_str() .unwrap() .to_string() @@ -48,7 +48,7 @@ pub fn op_query_permission( let perm = permissions.get_permission_state( &args.name, &args.url.as_ref().map(String::as_str), - &resolved_path.as_ref().map(String::as_str), + &resolved_path.as_ref().map(String::as_str).map(Path::new), )?; Ok(JsonOp::Sync(json!({ "state": perm.to_string() }))) } @@ -74,7 +74,7 @@ pub fn op_revoke_permission( let perm = permissions.get_permission_state( &args.name, &args.url.as_ref().map(String::as_str), - &resolved_path.as_ref().map(String::as_str), + &resolved_path.as_ref().map(String::as_str).map(Path::new), )?; Ok(JsonOp::Sync(json!({ "state": perm.to_string() }))) } @@ -89,12 +89,12 @@ pub fn op_request_permission( let resolved_path = args.path.as_ref().map(String::as_str).map(resolve_path); let perm = match args.name.as_ref() { "run" => Ok(permissions.request_run()), - "read" => { - Ok(permissions.request_read(&resolved_path.as_ref().map(String::as_str))) - } - "write" => { - Ok(permissions.request_write(&resolved_path.as_ref().map(String::as_str))) - } + "read" => Ok(permissions.request_read( + &resolved_path.as_ref().map(String::as_str).map(Path::new), + )), + "write" => Ok(permissions.request_write( + &resolved_path.as_ref().map(String::as_str).map(Path::new), + )), "net" => permissions.request_net(&args.url.as_ref().map(String::as_str)), "env" => Ok(permissions.request_env()), "plugin" => Ok(permissions.request_plugin()), diff --git a/cli/ops/plugins.rs b/cli/ops/plugins.rs index 747ea859d7..e31040a418 100644 --- a/cli/ops/plugins.rs +++ b/cli/ops/plugins.rs @@ -6,6 +6,7 @@ use deno_core::*; use dlopen::symbor::Library; use std::collections::HashMap; use std::ffi::OsStr; +use std::path::Path; use std::sync::Arc; pub fn init( @@ -62,9 +63,9 @@ pub fn op_open_plugin( _zero_copy: Option, ) -> Result { let args: OpenPluginArgs = serde_json::from_value(args)?; - let (filename, filename_) = deno_fs::resolve_from_cwd(&args.filename)?; + let filename = deno_fs::resolve_from_cwd(Path::new(&args.filename))?; - state.check_plugin(&filename_)?; + state.check_plugin(&filename)?; let lib = open_plugin(filename)?; let plugin_resource = PluginResource { diff --git a/cli/ops/tls.rs b/cli/ops/tls.rs index cb0ae2ecbf..87a067a9e1 100644 --- a/cli/ops/tls.rs +++ b/cli/ops/tls.rs @@ -16,6 +16,7 @@ use std::fs::File; use std::future::Future; use std::io::BufReader; use std::net::SocketAddr; +use std::path::Path; use std::pin::Pin; use std::sync::Arc; use std::task::Context; @@ -69,7 +70,7 @@ pub fn op_connect_tls( let state_ = state.clone(); state.check_net(&args.hostname, args.port)?; if let Some(path) = cert_file.clone() { - state.check_read(&path)?; + state.check_read(Path::new(&path))?; } let mut domain = args.hostname.clone(); @@ -254,8 +255,8 @@ fn op_listen_tls( let key_file = args.key_file; state.check_net(&args.hostname, args.port)?; - state.check_read(&cert_file)?; - state.check_read(&key_file)?; + state.check_read(Path::new(&cert_file))?; + state.check_read(Path::new(&key_file))?; let mut config = ServerConfig::new(NoClientAuth::new()); config diff --git a/cli/permissions.rs b/cli/permissions.rs index 735a90f74a..4f94fafc60 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -10,7 +10,7 @@ use std::collections::HashSet; use std::fmt; #[cfg(not(test))] use std::io; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; #[cfg(test)] use std::sync::atomic::AtomicBool; #[cfg(test)] @@ -135,30 +135,30 @@ impl DenoPermissions { ) } - fn get_state_read(&self, filename: &Option<&str>) -> PermissionState { - if check_path_white_list(filename, &self.read_whitelist) { + fn get_state_read(&self, path: &Option<&Path>) -> PermissionState { + if path.map_or(false, |f| check_path_white_list(f, &self.read_whitelist)) { return PermissionState::Allow; } self.allow_read } - pub fn check_read(&self, filename: &str) -> Result<(), ErrBox> { - self.get_state_read(&Some(filename)).check( - &format!("read access to \"{}\"", filename), + pub fn check_read(&self, path: &Path) -> Result<(), ErrBox> { + self.get_state_read(&Some(path)).check( + &format!("read access to \"{}\"", path.display()), "run again with the --allow-read flag", ) } - fn get_state_write(&self, filename: &Option<&str>) -> PermissionState { - if check_path_white_list(filename, &self.write_whitelist) { + fn get_state_write(&self, path: &Option<&Path>) -> PermissionState { + if path.map_or(false, |f| check_path_white_list(f, &self.write_whitelist)) { return PermissionState::Allow; } self.allow_write } - pub fn check_write(&self, filename: &str) -> Result<(), ErrBox> { - self.get_state_write(&Some(filename)).check( - &format!("write access to \"{}\"", filename), + pub fn check_write(&self, path: &Path) -> Result<(), ErrBox> { + self.get_state_write(&Some(path)).check( + &format!("write access to \"{}\"", path.display()), "run again with the --allow-write flag", ) } @@ -209,9 +209,9 @@ impl DenoPermissions { ) } - pub fn check_plugin(&self, filename: &str) -> Result<(), ErrBox> { + pub fn check_plugin(&self, path: &Path) -> Result<(), ErrBox> { self.allow_plugin.check( - &format!("access to open a plugin: {}", filename), + &format!("access to open a plugin: {}", path.display()), "run again with the --allow-plugin flag", ) } @@ -222,23 +222,27 @@ impl DenoPermissions { .request("Deno requests to access to run a subprocess.") } - pub fn request_read(&mut self, path: &Option<&str>) -> PermissionState { - if check_path_white_list(path, &self.read_whitelist) { + pub fn request_read(&mut self, path: &Option<&Path>) -> PermissionState { + if path.map_or(false, |f| check_path_white_list(f, &self.read_whitelist)) { return PermissionState::Allow; }; self.allow_read.request(&match path { None => "Deno requests read access.".to_string(), - Some(path) => format!("Deno requests read access to \"{}\".", path), + Some(path) => { + format!("Deno requests read access to \"{}\".", path.display()) + } }) } - pub fn request_write(&mut self, path: &Option<&str>) -> PermissionState { - if check_path_white_list(path, &self.write_whitelist) { + pub fn request_write(&mut self, path: &Option<&Path>) -> PermissionState { + if path.map_or(false, |f| check_path_white_list(f, &self.write_whitelist)) { return PermissionState::Allow; }; self.allow_write.request(&match path { None => "Deno requests write access.".to_string(), - Some(path) => format!("Deno requests write access to \"{}\".", path), + Some(path) => { + format!("Deno requests write access to \"{}\".", path.display()) + } }) } @@ -275,7 +279,7 @@ impl DenoPermissions { &self, name: &str, url: &Option<&str>, - path: &Option<&str>, + path: &Option<&Path>, ) -> Result { match name { "run" => Ok(self.allow_run), @@ -350,14 +354,8 @@ fn log_perm_access(message: &str) { } } -fn check_path_white_list( - filename: &Option<&str>, - white_list: &HashSet, -) -> bool { - if filename.is_none() { - return false; - } - let mut path_buf = PathBuf::from(filename.unwrap()); +fn check_path_white_list(path: &Path, white_list: &HashSet) -> bool { + let mut path_buf = PathBuf::from(path); loop { if white_list.contains(path_buf.to_str().unwrap()) { return true; @@ -399,36 +397,42 @@ mod tests { }); // Inside of /a/specific and /a/specific/dir/name - assert!(perms.check_read("/a/specific/dir/name").is_ok()); - assert!(perms.check_write("/a/specific/dir/name").is_ok()); + assert!(perms.check_read(Path::new("/a/specific/dir/name")).is_ok()); + assert!(perms.check_write(Path::new("/a/specific/dir/name")).is_ok()); // Inside of /a/specific but outside of /a/specific/dir/name - assert!(perms.check_read("/a/specific/dir").is_ok()); - assert!(perms.check_write("/a/specific/dir").is_ok()); + assert!(perms.check_read(Path::new("/a/specific/dir")).is_ok()); + assert!(perms.check_write(Path::new("/a/specific/dir")).is_ok()); // Inside of /a/specific and /a/specific/dir/name - assert!(perms.check_read("/a/specific/dir/name/inner").is_ok()); - assert!(perms.check_write("/a/specific/dir/name/inner").is_ok()); + assert!(perms + .check_read(Path::new("/a/specific/dir/name/inner")) + .is_ok()); + assert!(perms + .check_write(Path::new("/a/specific/dir/name/inner")) + .is_ok()); // Inside of /a/specific but outside of /a/specific/dir/name - assert!(perms.check_read("/a/specific/other/dir").is_ok()); - assert!(perms.check_write("/a/specific/other/dir").is_ok()); + assert!(perms.check_read(Path::new("/a/specific/other/dir")).is_ok()); + assert!(perms + .check_write(Path::new("/a/specific/other/dir")) + .is_ok()); // Exact match with /b/c - assert!(perms.check_read("/b/c").is_ok()); - assert!(perms.check_write("/b/c").is_ok()); + assert!(perms.check_read(Path::new("/b/c")).is_ok()); + assert!(perms.check_write(Path::new("/b/c")).is_ok()); // Sub path within /b/c - assert!(perms.check_read("/b/c/sub/path").is_ok()); - assert!(perms.check_write("/b/c/sub/path").is_ok()); + assert!(perms.check_read(Path::new("/b/c/sub/path")).is_ok()); + assert!(perms.check_write(Path::new("/b/c/sub/path")).is_ok()); // Inside of /b but outside of /b/c - assert!(perms.check_read("/b/e").is_err()); - assert!(perms.check_write("/b/e").is_err()); + assert!(perms.check_read(Path::new("/b/e")).is_err()); + assert!(perms.check_write(Path::new("/b/e")).is_err()); // Inside of /a but outside of /a/specific - assert!(perms.check_read("/a/b").is_err()); - assert!(perms.check_write("/a/b").is_err()); + assert!(perms.check_read(Path::new("/a/b")).is_err()); + assert!(perms.check_write(Path::new("/a/b")).is_err()); } #[test] @@ -540,7 +544,7 @@ mod tests { // If the whitelist contains the path, then the result is `allow` // regardless of prompt result assert_eq!( - perms0.request_read(&Some("/foo/bar")), + perms0.request_read(&Some(Path::new("/foo/bar"))), PermissionState::Allow ); @@ -550,7 +554,7 @@ mod tests { }); set_prompt_result(true); assert_eq!( - perms1.request_read(&Some("/foo/baz")), + perms1.request_read(&Some(Path::new("/foo/baz"))), PermissionState::Allow ); @@ -560,7 +564,7 @@ mod tests { }); set_prompt_result(false); assert_eq!( - perms2.request_read(&Some("/foo/baz")), + perms2.request_read(&Some(Path::new("/foo/baz"))), PermissionState::Deny ); } @@ -576,7 +580,7 @@ mod tests { // If the whitelist contains the path, then the result is `allow` // regardless of prompt result assert_eq!( - perms0.request_write(&Some("/foo/bar")), + perms0.request_write(&Some(Path::new("/foo/bar"))), PermissionState::Allow ); @@ -586,7 +590,7 @@ mod tests { }); set_prompt_result(true); assert_eq!( - perms1.request_write(&Some("/foo/baz")), + perms1.request_write(&Some(Path::new("/foo/baz"))), PermissionState::Allow ); @@ -596,7 +600,7 @@ mod tests { }); set_prompt_result(false); assert_eq!( - perms2.request_write(&Some("/foo/baz")), + perms2.request_write(&Some(Path::new("/foo/baz"))), PermissionState::Deny ); } diff --git a/cli/state.rs b/cli/state.rs index ed7b8e438d..acd661f251 100644 --- a/cli/state.rs +++ b/cli/state.rs @@ -26,6 +26,7 @@ use serde_json::Value; use std; use std::collections::HashMap; use std::ops::Deref; +use std::path::Path; use std::pin::Pin; use std::str; use std::sync::atomic::AtomicUsize; @@ -268,13 +269,13 @@ impl ThreadSafeState { } #[inline] - pub fn check_read(&self, filename: &str) -> Result<(), ErrBox> { - self.permissions.lock().unwrap().check_read(filename) + pub fn check_read(&self, path: &Path) -> Result<(), ErrBox> { + self.permissions.lock().unwrap().check_read(path) } #[inline] - pub fn check_write(&self, filename: &str) -> Result<(), ErrBox> { - self.permissions.lock().unwrap().check_write(filename) + pub fn check_write(&self, path: &Path) -> Result<(), ErrBox> { + self.permissions.lock().unwrap().check_write(path) } #[inline] @@ -298,7 +299,7 @@ impl ThreadSafeState { } #[inline] - pub fn check_plugin(&self, filename: &str) -> Result<(), ErrBox> { + pub fn check_plugin(&self, filename: &Path) -> Result<(), ErrBox> { self.permissions.lock().unwrap().check_plugin(filename) } @@ -313,13 +314,13 @@ impl ThreadSafeState { Ok(()) } "file" => { - let filename = u + let path = u .to_file_path() .unwrap() .into_os_string() .into_string() .unwrap(); - self.check_read(&filename)?; + self.check_read(Path::new(&path))?; Ok(()) } _ => Err(permission_denied()),