diff options
author | Nayeem Rahman <nayeemrmn99@gmail.com> | 2020-05-29 16:27:43 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-05-29 17:27:43 +0200 |
commit | 8e39275429c36ff5dfa5d62a80713c5b288fe26a (patch) | |
tree | 1d6d2781682bed1c29f81366f08db65c604fef2c | |
parent | ad6d2a7734aafb4a64837abc6abd1d1d0fb20017 (diff) |
fix(cli/permissions): Fix CWD and exec path leaks (#5642)
-rw-r--r-- | cli/flags.rs | 30 | ||||
-rw-r--r-- | cli/js/lib.deno.ns.d.ts | 14 | ||||
-rw-r--r-- | cli/ops/fs.rs | 48 | ||||
-rw-r--r-- | cli/ops/os.rs | 2 | ||||
-rw-r--r-- | cli/ops/permissions.rs | 27 | ||||
-rw-r--r-- | cli/ops/plugin.rs | 5 | ||||
-rw-r--r-- | cli/permissions.rs | 110 | ||||
-rw-r--r-- | cli/state.rs | 11 | ||||
-rw-r--r-- | cli/tests/056_make_temp_file_write_perm.ts | 13 | ||||
-rw-r--r-- | cli/tests/059_fs_relative_path_perm.ts | 2 | ||||
-rw-r--r-- | cli/tests/059_fs_relative_path_perm.ts.out | 2 | ||||
-rw-r--r-- | cli/tests/integration_tests.rs | 6 | ||||
-rw-r--r-- | cli/tests/unit/dir_test.ts | 37 | ||||
-rw-r--r-- | cli/tests/unit/os_test.ts | 16 |
14 files changed, 190 insertions, 133 deletions
diff --git a/cli/flags.rs b/cli/flags.rs index 075a29b5d..1eebb8b87 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -1,5 +1,4 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -use crate::fs::resolve_from_cwd; use clap::App; use clap::AppSettings; use clap::Arg; @@ -7,7 +6,7 @@ use clap::ArgMatches; use clap::SubCommand; use log::Level; use std::net::SocketAddr; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; /// Creates vector of strings, Vec<String> macro_rules! svec { @@ -472,13 +471,6 @@ fn lock_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } } -fn resolve_fs_whitelist(whitelist: &[PathBuf]) -> Vec<PathBuf> { - whitelist - .iter() - .map(|raw_path| resolve_from_cwd(Path::new(&raw_path)).unwrap()) - .collect() -} - // Shared between the run and test subcommands. They both take similar options. fn run_test_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { reload_arg_parse(flags, matches); @@ -1235,25 +1227,22 @@ fn no_remote_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { if let Some(read_wl) = matches.values_of("allow-read") { - let raw_read_whitelist: Vec<PathBuf> = read_wl.map(PathBuf::from).collect(); + let read_whitelist: Vec<PathBuf> = read_wl.map(PathBuf::from).collect(); - if raw_read_whitelist.is_empty() { + if read_whitelist.is_empty() { flags.allow_read = true; } else { - flags.read_whitelist = resolve_fs_whitelist(&raw_read_whitelist); - debug!("read whitelist: {:#?}", &flags.read_whitelist); + flags.read_whitelist = read_whitelist; } } if let Some(write_wl) = matches.values_of("allow-write") { - let raw_write_whitelist: Vec<PathBuf> = - write_wl.map(PathBuf::from).collect(); + let write_whitelist: Vec<PathBuf> = write_wl.map(PathBuf::from).collect(); - if raw_write_whitelist.is_empty() { + if write_whitelist.is_empty() { flags.allow_write = true; } else { - flags.write_whitelist = resolve_fs_whitelist(&raw_write_whitelist); - debug!("write whitelist: {:#?}", &flags.write_whitelist); + flags.write_whitelist = write_whitelist; } } @@ -1352,7 +1341,6 @@ fn resolve_hosts(paths: Vec<String>) -> Vec<String> { #[cfg(test)] mod tests { use super::*; - use std::env::current_dir; #[test] fn upgrade() { @@ -1853,7 +1841,7 @@ mod tests { r.unwrap(), Flags { allow_read: false, - read_whitelist: vec![current_dir().unwrap(), temp_dir], + read_whitelist: vec![PathBuf::from("."), temp_dir], subcommand: DenoSubcommand::Run { script: "script.ts".to_string(), }, @@ -1877,7 +1865,7 @@ mod tests { r.unwrap(), Flags { allow_write: false, - write_whitelist: vec![current_dir().unwrap(), temp_dir], + write_whitelist: vec![PathBuf::from("."), temp_dir], subcommand: DenoSubcommand::Run { script: "script.ts".to_string(), }, diff --git a/cli/js/lib.deno.ns.d.ts b/cli/js/lib.deno.ns.d.ts index fd0c0e6d7..74f1a2e15 100644 --- a/cli/js/lib.deno.ns.d.ts +++ b/cli/js/lib.deno.ns.d.ts @@ -897,7 +897,11 @@ declare namespace Deno { export interface MakeTempOptions { /** Directory where the temporary directory should be created (defaults to - * the env variable TMPDIR, or the system's default, usually /tmp). */ + * the env variable TMPDIR, or the system's default, usually /tmp). + * + * Note that if the passed `dir` is relative, the path returned by + * makeTempFile() and makeTempDir() will also be relative. Be mindful of + * this when changing working directory. */ dir?: string; /** String that should precede the random portion of the temporary * directory's name. */ @@ -1253,7 +1257,9 @@ declare namespace Deno { * console.log(realSymLinkPath); // outputs "/home/alice/file.txt" * ``` * - * Requires `allow-read` permission. */ + * Requires `allow-read` permission for the target path. + * Also requires `allow-read` permission for the CWD if the target path is + * relative.*/ export function realPathSync(path: string): string; /** Resolves to the absolute normalized path, with symbolic links resolved. @@ -1267,7 +1273,9 @@ declare namespace Deno { * console.log(realSymLinkPath); // outputs "/home/alice/file.txt" * ``` * - * Requires `allow-read` permission. */ + * Requires `allow-read` permission for the target path. + * Also requires `allow-read` permission for the CWD if the target path is + * relative.*/ export function realPath(path: string): Promise<string>; export interface DirEntry { diff --git a/cli/ops/fs.rs b/cli/ops/fs.rs index 9d5be3077..ce7045e21 100644 --- a/cli/ops/fs.rs +++ b/cli/ops/fs.rs @@ -3,7 +3,6 @@ use super::dispatch_json::{blocking_json, Deserialize, JsonOp, Value}; use super::io::std_file_resource; use super::io::{FileMetadata, StreamResource, StreamResourceHolder}; -use crate::fs::resolve_from_cwd; use crate::op_error::OpError; use crate::ops::dispatch_json::JsonResult; use crate::state::State; @@ -75,7 +74,7 @@ fn op_open( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: OpenArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = Path::new(&args.path).to_path_buf(); let resource_table = isolate.resource_table.clone(); let mut open_options = std::fs::OpenOptions::new(); @@ -274,7 +273,7 @@ fn op_mkdir( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: MkdirArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = Path::new(&args.path).to_path_buf(); let mode = args.mode.unwrap_or(0o777) & 0o777; state.check_write(&path)?; @@ -308,7 +307,7 @@ fn op_chmod( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: ChmodArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = Path::new(&args.path).to_path_buf(); let mode = args.mode & 0o777; state.check_write(&path)?; @@ -348,7 +347,7 @@ fn op_chown( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: ChownArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = Path::new(&args.path).to_path_buf(); state.check_write(&path)?; @@ -387,7 +386,7 @@ fn op_remove( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: RemoveArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); let recursive = args.recursive; state.check_write(&path)?; @@ -438,8 +437,8 @@ fn op_copy_file( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: CopyFileArgs = serde_json::from_value(args)?; - let from = resolve_from_cwd(Path::new(&args.from))?; - let to = resolve_from_cwd(Path::new(&args.to))?; + let from = PathBuf::from(&args.from); + let to = PathBuf::from(&args.to); state.check_read(&from)?; state.check_write(&to)?; @@ -532,7 +531,7 @@ fn op_stat( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: StatArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); let lstat = args.lstat; state.check_read(&path)?; @@ -562,9 +561,12 @@ fn op_realpath( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: RealpathArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); state.check_read(&path)?; + if path.is_relative() { + state.check_read_blind(¤t_dir()?, "CWD")?; + } let is_sync = args.promise_id.is_none(); blocking_json(is_sync, move || { @@ -594,7 +596,7 @@ fn op_read_dir( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: ReadDirArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); state.check_read(&path)?; @@ -637,8 +639,8 @@ fn op_rename( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: RenameArgs = serde_json::from_value(args)?; - let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?; - let newpath = resolve_from_cwd(Path::new(&args.newpath))?; + let oldpath = PathBuf::from(&args.oldpath); + let newpath = PathBuf::from(&args.newpath); state.check_read(&oldpath)?; state.check_write(&oldpath)?; @@ -667,8 +669,8 @@ fn op_link( ) -> Result<JsonOp, OpError> { state.check_unstable("Deno.link"); let args: LinkArgs = serde_json::from_value(args)?; - let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?; - let newpath = resolve_from_cwd(Path::new(&args.newpath))?; + let oldpath = PathBuf::from(&args.oldpath); + let newpath = PathBuf::from(&args.newpath); state.check_read(&oldpath)?; state.check_write(&newpath)?; @@ -705,8 +707,8 @@ fn op_symlink( ) -> Result<JsonOp, OpError> { state.check_unstable("Deno.symlink"); let args: SymlinkArgs = serde_json::from_value(args)?; - let oldpath = resolve_from_cwd(Path::new(&args.oldpath))?; - let newpath = resolve_from_cwd(Path::new(&args.newpath))?; + let oldpath = PathBuf::from(&args.oldpath); + let newpath = PathBuf::from(&args.newpath); state.check_write(&newpath)?; @@ -764,7 +766,7 @@ fn op_read_link( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: ReadLinkArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); state.check_read(&path)?; @@ -791,7 +793,7 @@ fn op_truncate( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let args: TruncateArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); let len = args.len; state.check_write(&path)?; @@ -866,7 +868,7 @@ fn op_make_temp_dir( ) -> Result<JsonOp, OpError> { let args: MakeTempArgs = serde_json::from_value(args)?; - let dir = args.dir.map(|s| resolve_from_cwd(Path::new(&s)).unwrap()); + let dir = args.dir.map(|s| PathBuf::from(&s)); let prefix = args.prefix.map(String::from); let suffix = args.suffix.map(String::from); @@ -897,7 +899,7 @@ fn op_make_temp_file( ) -> Result<JsonOp, OpError> { let args: MakeTempArgs = serde_json::from_value(args)?; - let dir = args.dir.map(|s| resolve_from_cwd(Path::new(&s)).unwrap()); + let dir = args.dir.map(|s| PathBuf::from(&s)); let prefix = args.prefix.map(String::from); let suffix = args.suffix.map(String::from); @@ -938,7 +940,7 @@ fn op_utime( state.check_unstable("Deno.utime"); let args: UtimeArgs = serde_json::from_value(args)?; - let path = resolve_from_cwd(Path::new(&args.path))?; + let path = PathBuf::from(&args.path); state.check_write(&path)?; @@ -956,7 +958,7 @@ fn op_cwd( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let path = current_dir()?; - state.check_read(&path)?; + state.check_read_blind(&path, "CWD")?; let path_str = into_string(path.into_os_string())?; Ok(JsonOp::Sync(json!(path_str))) } diff --git a/cli/ops/os.rs b/cli/ops/os.rs index 6c1801520..1dd2ddc4f 100644 --- a/cli/ops/os.rs +++ b/cli/ops/os.rs @@ -83,7 +83,7 @@ fn op_exec_path( _zero_copy: Option<ZeroCopyBuf>, ) -> Result<JsonOp, OpError> { let current_exe = env::current_exe().unwrap(); - state.check_read(¤t_exe)?; + state.check_read_blind(¤t_exe, "exec_path")?; // Now apply URL parser to current exe to get fully resolved path, otherwise // we might get `./` and `../` bits in `exec_path` let exe_url = Url::from_file_path(current_exe).unwrap(); diff --git a/cli/ops/permissions.rs b/cli/ops/permissions.rs index 3ccff4065..13a8f3bb8 100644 --- a/cli/ops/permissions.rs +++ b/cli/ops/permissions.rs @@ -1,6 +1,5 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use super::dispatch_json::{Deserialize, JsonOp, Value}; -use crate::fs as deno_fs; use crate::op_error::OpError; use crate::state::State; use deno_core::CoreIsolate; @@ -29,14 +28,6 @@ struct PermissionArgs { path: Option<String>, } -fn resolve_path(path: &str) -> String { - deno_fs::resolve_from_cwd(Path::new(path)) - .unwrap() - .to_str() - .unwrap() - .to_string() -} - pub fn op_query_permission( state: &State, args: Value, @@ -44,11 +35,11 @@ pub fn op_query_permission( ) -> Result<JsonOp, OpError> { let args: PermissionArgs = serde_json::from_value(args)?; let state = state.borrow(); - let resolved_path = args.path.as_deref().map(resolve_path); + let path = args.path.as_deref(); let perm = state.permissions.get_permission_state( &args.name, &args.url.as_deref(), - &resolved_path.as_deref().map(Path::new), + &path.as_deref().map(Path::new), )?; Ok(JsonOp::Sync(json!({ "state": perm.to_string() }))) } @@ -71,11 +62,11 @@ pub fn op_revoke_permission( "hrtime" => permissions.allow_hrtime.revoke(), _ => {} }; - let resolved_path = args.path.as_deref().map(resolve_path); + let path = args.path.as_deref(); let perm = permissions.get_permission_state( &args.name, &args.url.as_deref(), - &resolved_path.as_deref().map(Path::new), + &path.as_deref().map(Path::new), )?; Ok(JsonOp::Sync(json!({ "state": perm.to_string() }))) } @@ -88,15 +79,11 @@ pub fn op_request_permission( let args: PermissionArgs = serde_json::from_value(args)?; let mut state = state.borrow_mut(); let permissions = &mut state.permissions; - let resolved_path = args.path.as_deref().map(resolve_path); + let path = args.path.as_deref(); let perm = match args.name.as_ref() { "run" => Ok(permissions.request_run()), - "read" => { - Ok(permissions.request_read(&resolved_path.as_deref().map(Path::new))) - } - "write" => { - Ok(permissions.request_write(&resolved_path.as_deref().map(Path::new))) - } + "read" => Ok(permissions.request_read(&path.as_deref().map(Path::new))), + "write" => Ok(permissions.request_write(&path.as_deref().map(Path::new))), "net" => permissions.request_net(&args.url.as_deref()), "env" => Ok(permissions.request_env()), "plugin" => Ok(permissions.request_plugin()), diff --git a/cli/ops/plugin.rs b/cli/ops/plugin.rs index cabb3329d..f55c91df8 100644 --- a/cli/ops/plugin.rs +++ b/cli/ops/plugin.rs @@ -1,5 +1,4 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -use crate::fs::resolve_from_cwd; use crate::op_error::OpError; use crate::ops::dispatch_json::Deserialize; use crate::ops::dispatch_json::JsonOp; @@ -14,7 +13,7 @@ use deno_core::OpId; use deno_core::ZeroCopyBuf; use dlopen::symbor::Library; use futures::prelude::*; -use std::path::Path; +use std::path::PathBuf; use std::pin::Pin; use std::rc::Rc; use std::task::Context; @@ -41,7 +40,7 @@ pub fn op_open_plugin( ) -> Result<JsonOp, OpError> { state.check_unstable("Deno.openPlugin"); let args: OpenPluginArgs = serde_json::from_value(args).unwrap(); - let filename = resolve_from_cwd(Path::new(&args.filename))?; + let filename = PathBuf::from(&args.filename); state.check_plugin(&filename)?; diff --git a/cli/permissions.rs b/cli/permissions.rs index 833dc28df..298259f6b 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -1,10 +1,12 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. use crate::colors; use crate::flags::Flags; +use crate::fs::resolve_from_cwd; use crate::op_error::OpError; use serde::de; use serde::Deserialize; use std::collections::HashSet; +use std::env::current_dir; use std::fmt; #[cfg(not(test))] use std::io; @@ -149,20 +151,20 @@ pub struct Permissions { pub allow_hrtime: PermissionState, } +fn resolve_fs_whitelist(whitelist: &[PathBuf]) -> HashSet<PathBuf> { + whitelist + .iter() + .map(|raw_path| resolve_from_cwd(Path::new(&raw_path)).unwrap()) + .collect() +} + impl Permissions { pub fn from_flags(flags: &Flags) -> Self { - // assert each whitelist path is absolute, since the cwd may change. - for path in &flags.read_whitelist { - assert!(path.has_root()); - } - for path in &flags.write_whitelist { - assert!(path.has_root()); - } Self { allow_read: PermissionState::from(flags.allow_read), - read_whitelist: flags.read_whitelist.iter().cloned().collect(), + read_whitelist: resolve_fs_whitelist(&flags.read_whitelist), allow_write: PermissionState::from(flags.allow_write), - write_whitelist: flags.write_whitelist.iter().cloned().collect(), + write_whitelist: resolve_fs_whitelist(&flags.write_whitelist), allow_net: PermissionState::from(flags.allow_net), net_whitelist: flags.net_whitelist.iter().cloned().collect(), allow_env: PermissionState::from(flags.allow_env), @@ -172,6 +174,24 @@ impl Permissions { } } + /// Arbitrary helper. Resolves the path from CWD, and also gets a path that + /// can be displayed without leaking the CWD when not allowed. + fn resolved_and_display_path(&self, path: &Path) -> (PathBuf, PathBuf) { + let resolved_path = resolve_from_cwd(path).unwrap(); + let display_path = if path.is_absolute() { + path.to_path_buf() + } else { + match self + .get_state_read(&Some(¤t_dir().unwrap())) + .check("", "") + { + Ok(_) => resolved_path.clone(), + Err(_) => path.to_path_buf(), + } + }; + (resolved_path, display_path) + } + pub fn allow_all() -> Self { Self { allow_read: PermissionState::from(true), @@ -199,12 +219,26 @@ impl Permissions { } pub fn check_read(&self, path: &Path) -> Result<(), OpError> { - self.get_state_read(&Some(path)).check( - &format!("read access to \"{}\"", path.display()), + let (resolved_path, display_path) = self.resolved_and_display_path(path); + self.get_state_read(&Some(&resolved_path)).check( + &format!("read access to \"{}\"", display_path.display()), "--allow-read", ) } + /// As `check_read()`, but permission error messages will anonymize the path + /// by replacing it with the given `display`. + pub fn check_read_blind( + &self, + path: &Path, + display: &str, + ) -> Result<(), OpError> { + let resolved_path = resolve_from_cwd(path).unwrap(); + self + .get_state_read(&Some(&resolved_path)) + .check(&format!("read access to <{}>", display), "--allow-read") + } + 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; @@ -213,8 +247,9 @@ impl Permissions { } pub fn check_write(&self, path: &Path) -> Result<(), OpError> { - self.get_state_write(&Some(path)).check( - &format!("write access to \"{}\"", path.display()), + let (resolved_path, display_path) = self.resolved_and_display_path(path); + self.get_state_write(&Some(&resolved_path)).check( + &format!("write access to \"{}\"", display_path.display()), "--allow-write", ) } @@ -264,8 +299,9 @@ impl Permissions { } pub fn check_plugin(&self, path: &Path) -> Result<(), OpError> { + let (_, display_path) = self.resolved_and_display_path(path); self.allow_plugin.check( - &format!("access to open a plugin: {}", path.display()), + &format!("access to open a plugin: {}", display_path.display()), "--allow-plugin", ) } @@ -277,26 +313,34 @@ impl Permissions { } 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; + let paths = path.map(|p| self.resolved_and_display_path(p)); + if let Some((p, _)) = paths.as_ref() { + if check_path_white_list(&p, &self.read_whitelist) { + return PermissionState::Allow; + } }; - self.allow_read.request(&match path { + self.allow_read.request(&match paths { None => "Deno requests read access".to_string(), - Some(path) => { - format!("Deno requests read access to \"{}\"", path.display()) - } + Some((_, display_path)) => format!( + "Deno requests read access to \"{}\"", + display_path.display() + ), }) } 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; + let paths = path.map(|p| self.resolved_and_display_path(p)); + if let Some((p, _)) = paths.as_ref() { + if check_path_white_list(&p, &self.write_whitelist) { + return PermissionState::Allow; + } }; - self.allow_write.request(&match path { + self.allow_write.request(&match paths { None => "Deno requests write access".to_string(), - Some(path) => { - format!("Deno requests write access to \"{}\"", path.display()) - } + Some((_, display_path)) => format!( + "Deno requests write access to \"{}\"", + display_path.display() + ), }) } @@ -335,10 +379,12 @@ impl Permissions { url: &Option<&str>, path: &Option<&Path>, ) -> Result<PermissionState, OpError> { + let path = path.map(|p| resolve_from_cwd(p).unwrap()); + let path = path.as_deref(); match name { "run" => Ok(self.allow_run), - "read" => Ok(self.get_state_read(path)), - "write" => Ok(self.get_state_write(path)), + "read" => Ok(self.get_state_read(&path)), + "write" => Ok(self.get_state_write(&path)), "net" => self.get_state_net_url(url), "env" => Ok(self.allow_env), "plugin" => Ok(self.allow_plugin), @@ -486,6 +532,14 @@ mod tests { assert!(perms.check_read(Path::new("/b/c/sub/path")).is_ok()); assert!(perms.check_write(Path::new("/b/c/sub/path")).is_ok()); + // Sub path within /b/c, needs normalizing + assert!(perms + .check_read(Path::new("/b/c/sub/path/../path/.")) + .is_ok()); + assert!(perms + .check_write(Path::new("/b/c/sub/path/../path/.")) + .is_ok()); + // Inside of /b but outside of /b/c assert!(perms.check_read(Path::new("/b/e")).is_err()); assert!(perms.check_write(Path::new("/b/e")).is_err()); diff --git a/cli/state.rs b/cli/state.rs index 00337c286..a15d113c8 100644 --- a/cli/state.rs +++ b/cli/state.rs @@ -425,6 +425,17 @@ impl State { self.borrow().permissions.check_read(path) } + /// As `check_read()`, but permission error messages will anonymize the path + /// by replacing it with the given `display`. + #[inline] + pub fn check_read_blind( + &self, + path: &Path, + display: &str, + ) -> Result<(), OpError> { + self.borrow().permissions.check_read_blind(path, display) + } + #[inline] pub fn check_write(&self, path: &Path) -> Result<(), OpError> { self.borrow().permissions.check_write(path) diff --git a/cli/tests/056_make_temp_file_write_perm.ts b/cli/tests/056_make_temp_file_write_perm.ts index 15aefaff9..c0deda8a2 100644 --- a/cli/tests/056_make_temp_file_write_perm.ts +++ b/cli/tests/056_make_temp_file_write_perm.ts @@ -1,8 +1,9 @@ -const path = await Deno.makeTempFile({ dir: "./subdir/" }); -if (path.startsWith(Deno.cwd())) { +const path = await Deno.makeTempFile({ dir: `subdir` }); +try { + if (!path.match(/^subdir[/\\][^/\\]+/)) { + throw Error("bad " + path); + } console.log("good", path); -} else { - throw Error("bad " + path); +} finally { + await Deno.remove(path); } -console.log(path); -await Deno.remove(path); diff --git a/cli/tests/059_fs_relative_path_perm.ts b/cli/tests/059_fs_relative_path_perm.ts new file mode 100644 index 000000000..26630fe1c --- /dev/null +++ b/cli/tests/059_fs_relative_path_perm.ts @@ -0,0 +1,2 @@ +// The permission error message shouldn't include the CWD. +Deno.readFileSync("non-existent"); diff --git a/cli/tests/059_fs_relative_path_perm.ts.out b/cli/tests/059_fs_relative_path_perm.ts.out new file mode 100644 index 000000000..83df41903 --- /dev/null +++ b/cli/tests/059_fs_relative_path_perm.ts.out @@ -0,0 +1,2 @@ +[WILDCARD]error: Uncaught PermissionDenied: read access to "non-existent", run again with the --allow-read flag + at [WILDCARD] diff --git a/cli/tests/integration_tests.rs b/cli/tests/integration_tests.rs index 653f5a008..0a50190d2 100644 --- a/cli/tests/integration_tests.rs +++ b/cli/tests/integration_tests.rs @@ -1326,6 +1326,12 @@ itest!(_058_tasks_microtasks_close { output: "058_tasks_microtasks_close.ts.out", }); +itest!(_059_fs_relative_path_perm { + args: "run 059_fs_relative_path_perm.ts", + output: "059_fs_relative_path_perm.ts.out", + exit_code: 1, +}); + itest!(js_import_detect { args: "run --quiet --reload js_import_detect.ts", output: "js_import_detect.ts.out", diff --git a/cli/tests/unit/dir_test.ts b/cli/tests/unit/dir_test.ts index dc05d9564..5b60b3dba 100644 --- a/cli/tests/unit/dir_test.ts +++ b/cli/tests/unit/dir_test.ts @@ -1,5 +1,5 @@ // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. -import { unitTest, assert, assertEquals } from "./test_util.ts"; +import { unitTest, assert, assertEquals, assertThrows } from "./test_util.ts"; unitTest({ perms: { read: true } }, function dirCwdNotNull(): void { assert(Deno.cwd() != null); @@ -29,32 +29,31 @@ unitTest({ perms: { read: true, write: true } }, function dirCwdError(): void { Deno.chdir(path); Deno.removeSync(path); try { - Deno.cwd(); - throw Error("current directory removed, should throw error"); - } catch (err) { - if (err instanceof Deno.errors.NotFound) { - assert(err.name === "NotFound"); - } else { - throw Error("raised different exception"); - } + assertThrows(() => { + Deno.cwd(); + }, Deno.errors.NotFound); + } finally { + Deno.chdir(initialdir); } - Deno.chdir(initialdir); } }); +unitTest({ perms: { read: false } }, function dirCwdPermError(): void { + assertThrows( + () => { + Deno.cwd(); + }, + Deno.errors.PermissionDenied, + "read access to <CWD>, run again with the --allow-read flag" + ); +}); + unitTest( { perms: { read: true, write: true } }, function dirChdirError(): void { const path = Deno.makeTempDirSync() + "test"; - try { + assertThrows(() => { Deno.chdir(path); - throw Error("directory not available, should throw error"); - } catch (err) { - if (err instanceof Deno.errors.NotFound) { - assert(err.name === "NotFound"); - } else { - throw Error("raised different exception"); - } - } + }, Deno.errors.NotFound); } ); diff --git a/cli/tests/unit/os_test.ts b/cli/tests/unit/os_test.ts index e99002534..72c88b0e1 100644 --- a/cli/tests/unit/os_test.ts +++ b/cli/tests/unit/os_test.ts @@ -277,15 +277,13 @@ unitTest({ perms: { read: true } }, function execPath(): void { }); unitTest({ perms: { read: false } }, function execPathPerm(): void { - let caughtError = false; - try { - Deno.execPath(); - } catch (err) { - caughtError = true; - assert(err instanceof Deno.errors.PermissionDenied); - assertEquals(err.name, "PermissionDenied"); - } - assert(caughtError); + assertThrows( + () => { + Deno.execPath(); + }, + Deno.errors.PermissionDenied, + "read access to <exec_path>, run again with the --allow-read flag" + ); }); unitTest({ perms: { env: true } }, function loadavgSuccess(): void { |