From 891ee925d4f359aac91aae1b3b7ea3663d3f255b Mon Sep 17 00:00:00 2001 From: afinch7 Date: Tue, 16 Apr 2019 13:52:39 -0400 Subject: [PATCH 01/17] permissions whitelist --- cli/flags.rs | 27 +++++++++++ cli/permissions.rs | 113 ++++++++++++++++++++++++++++++++------------- 2 files changed, 109 insertions(+), 31 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index 3fd3f91fbc86c..86b585b3d5740 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -15,8 +15,11 @@ pub struct DenoFlags { pub version: bool, pub reload: bool, pub allow_read: bool, + pub read_whitelist: Vec, pub allow_write: bool, + pub write_whitelist: Vec, pub allow_net: bool, + pub net_whitelist: Vec, pub allow_env: bool, pub allow_run: bool, pub allow_high_precision: bool, @@ -44,12 +47,21 @@ impl<'a> From> for DenoFlags { if matches.is_present("allow-read") { flags.allow_read = true; } + if let Some(read_wl) = matches.values_of("read-wl") { + flags.read_whitelist = read_wl.map(|s| s.to_string()).collect(); + } if matches.is_present("allow-write") { flags.allow_write = true; } + if let Some(write_wl) = matches.values_of("write-wl") { + flags.write_whitelist = write_wl.map(|s| s.to_string()).collect(); + } if matches.is_present("allow-net") { flags.allow_net = true; } + if let Some(net_wl) = matches.values_of("net-wl") { + flags.net_whitelist = net_wl.map(|s| s.to_string()).collect(); + } if matches.is_present("allow-env") { flags.allow_env = true; } @@ -112,14 +124,29 @@ fn create_cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("allow-read") .long("allow-read") .help("Allow file system read access"), + ).arg( + Arg::with_name("read-wl") + .long("read-wl") + .takes_value(true) + .help("Allow file system read access to specific file or directory"), ).arg( Arg::with_name("allow-write") .long("allow-write") .help("Allow file system write access"), + ).arg( + Arg::with_name("write-wl") + .long("write-wl") + .takes_value(true) + .help("Allow file system write access to specific file or directory"), ).arg( Arg::with_name("allow-net") .long("allow-net") .help("Allow network access"), + ).arg( + Arg::with_name("net-wl") + .long("net-wl") + .takes_value(true) + .help("Allow file system net access to specific domain"), ).arg( Arg::with_name("allow-env") .long("allow-env") diff --git a/cli/permissions.rs b/cli/permissions.rs index 84c2f0e178aca..ec54a235a9896 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -6,6 +6,7 @@ use crate::flags::DenoFlags; use ansi_term::Style; use crate::errors::permission_denied; use crate::errors::DenoResult; +use std::collections::HashSet; use std::fmt; use std::io; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -127,8 +128,11 @@ impl Default for PermissionAccessor { pub struct DenoPermissions { // Keep in sync with src/permissions.ts pub allow_read: PermissionAccessor, + pub read_whitelist: Arc>, pub allow_write: PermissionAccessor, + pub write_whitelist: Arc>, pub allow_net: PermissionAccessor, + pub net_whitelist: Arc>, pub allow_env: PermissionAccessor, pub allow_run: PermissionAccessor, pub allow_high_precision: PermissionAccessor, @@ -139,9 +143,14 @@ impl DenoPermissions { pub fn from_flags(flags: &DenoFlags) -> Self { Self { allow_read: PermissionAccessor::from(flags.allow_read), + read_whitelist: Arc::new(flags.read_whitelist.iter().cloned().collect()), allow_write: PermissionAccessor::from(flags.allow_write), - allow_env: PermissionAccessor::from(flags.allow_env), + write_whitelist: Arc::new( + flags.write_whitelist.iter().cloned().collect(), + ), allow_net: PermissionAccessor::from(flags.allow_net), + net_whitelist: Arc::new(flags.net_whitelist.iter().cloned().collect()), + allow_env: PermissionAccessor::from(flags.allow_env), allow_run: PermissionAccessor::from(flags.allow_run), allow_high_precision: PermissionAccessor::from( flags.allow_high_precision, @@ -170,51 +179,93 @@ impl DenoPermissions { pub fn check_read(&self, filename: &str) -> DenoResult<()> { match self.allow_read.get_state() { PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => match self - .try_permissions_prompt(&format!("read access to \"{}\"", filename)) - { - Err(e) => Err(e), - Ok(v) => { - self.allow_read.update_with_prompt_result(&v); - v.check()?; - Ok(()) + state => { + match self.read_whitelist.contains(filename) { + true => Ok(()), + false => { + match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => { + match self.try_permissions_prompt(&format!( + "read access to \"{}\"", + filename + )) { + Err(e) => Err(e), + Ok(v) => { + self.allow_read.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + } + } + PermissionAccessorState::Deny => Err(permission_denied()), + } + } } - }, - PermissionAccessorState::Deny => Err(permission_denied()), + } } } pub fn check_write(&self, filename: &str) -> DenoResult<()> { match self.allow_write.get_state() { PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => match self - .try_permissions_prompt(&format!("write access to \"{}\"", filename)) - { - Err(e) => Err(e), - Ok(v) => { - self.allow_write.update_with_prompt_result(&v); - v.check()?; - Ok(()) + state => { + match self.write_whitelist.contains(filename) { + true => Ok(()), + false => { + match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => { + match self.try_permissions_prompt(&format!( + "write access to \"{}\"", + filename + )) { + Err(e) => Err(e), + Ok(v) => { + self.allow_write.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + } + } + PermissionAccessorState::Deny => Err(permission_denied()), + } + } } - }, - PermissionAccessorState::Deny => Err(permission_denied()), + } } } pub fn check_net(&self, domain_name: &str) -> DenoResult<()> { match self.allow_net.get_state() { PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => match self.try_permissions_prompt( - &format!("network access to \"{}\"", domain_name), - ) { - Err(e) => Err(e), - Ok(v) => { - self.allow_net.update_with_prompt_result(&v); - v.check()?; - Ok(()) + state => { + match self.net_whitelist.contains(domain_name) { + true => Ok(()), + false => { + match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => { + match self.try_permissions_prompt(&format!( + "network access to \"{}\"", + domain_name + )) { + Err(e) => Err(e), + Ok(v) => { + self.allow_net.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + } + } + PermissionAccessorState::Deny => Err(permission_denied()), + } + } } - }, - PermissionAccessorState::Deny => Err(permission_denied()), + } } } From 20d8e10d944c999f1b4ca583e46fb72704884bd4 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Sun, 21 Apr 2019 14:31:39 -0400 Subject: [PATCH 02/17] subpath matching for read/write --- cli/permissions.rs | 91 ++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 48 deletions(-) diff --git a/cli/permissions.rs b/cli/permissions.rs index ec54a235a9896..99a48f8c0a70b 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -9,6 +9,7 @@ use crate::errors::DenoResult; use std::collections::HashSet; use std::fmt; use std::io; +use std::path::Path; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::Arc; @@ -128,9 +129,9 @@ impl Default for PermissionAccessor { pub struct DenoPermissions { // Keep in sync with src/permissions.ts pub allow_read: PermissionAccessor, - pub read_whitelist: Arc>, + pub read_whitelist: Vec, pub allow_write: PermissionAccessor, - pub write_whitelist: Arc>, + pub write_whitelist: Vec, pub allow_net: PermissionAccessor, pub net_whitelist: Arc>, pub allow_env: PermissionAccessor, @@ -143,11 +144,9 @@ impl DenoPermissions { pub fn from_flags(flags: &DenoFlags) -> Self { Self { allow_read: PermissionAccessor::from(flags.allow_read), - read_whitelist: Arc::new(flags.read_whitelist.iter().cloned().collect()), + read_whitelist: flags.read_whitelist.iter().cloned().collect(), allow_write: PermissionAccessor::from(flags.allow_write), - write_whitelist: Arc::new( - flags.write_whitelist.iter().cloned().collect(), - ), + write_whitelist: flags.write_whitelist.iter().cloned().collect(), allow_net: PermissionAccessor::from(flags.allow_net), net_whitelist: Arc::new(flags.net_whitelist.iter().cloned().collect()), allow_env: PermissionAccessor::from(flags.allow_env), @@ -180,29 +179,27 @@ impl DenoPermissions { match self.allow_read.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - match self.read_whitelist.contains(filename) { - true => Ok(()), - false => { - match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => { - match self.try_permissions_prompt(&format!( - "read access to \"{}\"", - filename - )) { - Err(e) => Err(e), - Ok(v) => { - self.allow_read.update_with_prompt_result(&v); - v.check()?; - Ok(()) - } - } - } - PermissionAccessorState::Deny => Err(permission_denied()), - } + let file_path = Path::new(filename); + for path in &self.read_whitelist { + if file_path.starts_with(path) { + return Ok(()); } } + match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => match self + .try_permissions_prompt(&format!("read access to \"{}\"", filename)) + { + Err(e) => Err(e), + Ok(v) => { + self.allow_read.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + }, + PermissionAccessorState::Deny => Err(permission_denied()), + } } } } @@ -211,29 +208,27 @@ impl DenoPermissions { match self.allow_write.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - match self.write_whitelist.contains(filename) { - true => Ok(()), - false => { - match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => { - match self.try_permissions_prompt(&format!( - "write access to \"{}\"", - filename - )) { - Err(e) => Err(e), - Ok(v) => { - self.allow_write.update_with_prompt_result(&v); - v.check()?; - Ok(()) - } - } - } - PermissionAccessorState::Deny => Err(permission_denied()), - } + let file_path = Path::new(filename); + for path in &self.write_whitelist { + if file_path.starts_with(path) { + return Ok(()); } } + match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => match self.try_permissions_prompt( + &format!("write access to \"{}\"", filename), + ) { + Err(e) => Err(e), + Ok(v) => { + self.allow_write.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + }, + PermissionAccessorState::Deny => Err(permission_denied()), + } } } } From bf63ae648b9a9c6cc88aaf98bfd4b0ff7358052e Mon Sep 17 00:00:00 2001 From: afinch7 Date: Wed, 24 Apr 2019 13:29:18 -0400 Subject: [PATCH 03/17] moved path checking logic to function and refactored the path matching algorithim --- cli/permissions.rs | 96 +++++++++++++++++++++++++--------------------- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/cli/permissions.rs b/cli/permissions.rs index 99a48f8c0a70b..2f5c827c9862c 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -9,7 +9,7 @@ use crate::errors::DenoResult; use std::collections::HashSet; use std::fmt; use std::io; -use std::path::Path; +use std::path::PathBuf; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::Arc; @@ -129,9 +129,9 @@ impl Default for PermissionAccessor { pub struct DenoPermissions { // Keep in sync with src/permissions.ts pub allow_read: PermissionAccessor, - pub read_whitelist: Vec, + pub read_whitelist: Arc>, pub allow_write: PermissionAccessor, - pub write_whitelist: Vec, + pub write_whitelist: Arc>, pub allow_net: PermissionAccessor, pub net_whitelist: Arc>, pub allow_env: PermissionAccessor, @@ -144,9 +144,11 @@ impl DenoPermissions { pub fn from_flags(flags: &DenoFlags) -> Self { Self { allow_read: PermissionAccessor::from(flags.allow_read), - read_whitelist: flags.read_whitelist.iter().cloned().collect(), + read_whitelist: Arc::new(flags.read_whitelist.iter().cloned().collect()), allow_write: PermissionAccessor::from(flags.allow_write), - write_whitelist: flags.write_whitelist.iter().cloned().collect(), + write_whitelist: Arc::new( + flags.write_whitelist.iter().cloned().collect(), + ), allow_net: PermissionAccessor::from(flags.allow_net), net_whitelist: Arc::new(flags.net_whitelist.iter().cloned().collect()), allow_env: PermissionAccessor::from(flags.allow_env), @@ -179,26 +181,23 @@ impl DenoPermissions { match self.allow_read.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - let file_path = Path::new(filename); - for path in &self.read_whitelist { - if file_path.starts_with(path) { - return Ok(()); - } - } - match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => match self - .try_permissions_prompt(&format!("read access to \"{}\"", filename)) - { - Err(e) => Err(e), - Ok(v) => { - self.allow_read.update_with_prompt_result(&v); - v.check()?; - Ok(()) - } + match check_path_white_list(filename, &self.read_whitelist) { + true => Ok(()), + false => match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => match self.try_permissions_prompt( + &format!("read access to \"{}\"", filename), + ) { + Err(e) => Err(e), + Ok(v) => { + self.allow_read.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + }, + PermissionAccessorState::Deny => Err(permission_denied()), }, - PermissionAccessorState::Deny => Err(permission_denied()), } } } @@ -208,26 +207,23 @@ impl DenoPermissions { match self.allow_write.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - let file_path = Path::new(filename); - for path in &self.write_whitelist { - if file_path.starts_with(path) { - return Ok(()); - } - } - match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), - PermissionAccessorState::Ask => match self.try_permissions_prompt( - &format!("write access to \"{}\"", filename), - ) { - Err(e) => Err(e), - Ok(v) => { - self.allow_write.update_with_prompt_result(&v); - v.check()?; - Ok(()) - } + match check_path_white_list(filename, &self.write_whitelist) { + true => Ok(()), + false => match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Ask => match self.try_permissions_prompt( + &format!("write access to \"{}\"", filename), + ) { + Err(e) => Err(e), + Ok(v) => { + self.allow_write.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + }, + PermissionAccessorState::Deny => Err(permission_denied()), }, - PermissionAccessorState::Deny => Err(permission_denied()), } } } @@ -400,3 +396,17 @@ fn permission_prompt(message: &str) -> DenoResult { }; } } + +fn check_path_white_list( + filename: &str, + white_list: &Arc>, +) -> bool { + let mut path_buf = PathBuf::from(filename); + + while path_buf.pop() == true { + if white_list.contains(path_buf.to_str().unwrap()) { + return true; + } + } + return false; +} From 7a6dc701ae254d185cfbe07799c53548cafed7f3 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Tue, 30 Apr 2019 13:19:46 -0400 Subject: [PATCH 04/17] unit test and fixed some bugs --- cli/permissions.rs | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/cli/permissions.rs b/cli/permissions.rs index 2f5c827c9862c..09041ed2fffb7 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -403,6 +403,10 @@ fn check_path_white_list( ) -> bool { let mut path_buf = PathBuf::from(filename); + if white_list.contains(path_buf.to_str().unwrap()) { + return true; + } + while path_buf.pop() == true { if white_list.contains(path_buf.to_str().unwrap()) { return true; @@ -410,3 +414,42 @@ fn check_path_white_list( } return false; } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn check_paths() { + let whitelist = vec![ + "/a/specific/dir/name".to_string(), + "/a/specific".to_string(), + "/b/c".to_string(), + ].to_vec(); + + let perms = DenoPermissions::from_flags(&DenoFlags { + read_whitelist: whitelist.clone(), + write_whitelist: whitelist.clone(), + no_prompts: true, + ..Default::default() + }); + + perms.check_read("/a/specific/dir/name").unwrap(); + perms.check_write("/a/specific/dir/name").unwrap(); + + perms.check_read("/a/specific/other/dir").unwrap(); + perms.check_write("/a/specific/other/dir").unwrap(); + + perms.check_read("/b/c").unwrap(); + perms.check_write("/b/c").unwrap(); + + perms.check_read("/b/c/sub/path").unwrap(); + perms.check_write("/b/c/sub/path").unwrap(); + + perms.check_read("/b/e").unwrap_err(); + perms.check_write("/b/e").unwrap_err(); + + perms.check_read("/a/b").unwrap_err(); + perms.check_write("/a/b").unwrap_err(); + } +} From a4bf99847b624e2f4a917058e5b9dcabd34a06b1 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Wed, 1 May 2019 14:10:49 -0400 Subject: [PATCH 05/17] added net tests and parsing for urls --- cli/permissions.rs | 64 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 61 insertions(+), 3 deletions(-) diff --git a/cli/permissions.rs b/cli/permissions.rs index 09041ed2fffb7..4e5b887668cb7 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -229,11 +229,19 @@ impl DenoPermissions { } } - pub fn check_net(&self, domain_name: &str) -> DenoResult<()> { + pub fn check_net(&self, url_str: &str) -> DenoResult<()> { match self.allow_net.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - match self.net_whitelist.contains(domain_name) { + let parsed_url_result = url::Url::parse(url_str); + let whitelist_result = match parsed_url_result { + Ok(parsed_url) => match parsed_url.host() { + Some(host) => self.net_whitelist.contains(&host.to_string()), + None => false, + }, + Err(_) => false, + }; + match whitelist_result { true => Ok(()), false => { match state { @@ -242,7 +250,7 @@ impl DenoPermissions { PermissionAccessorState::Ask => { match self.try_permissions_prompt(&format!( "network access to \"{}\"", - domain_name + url_str )) { Err(e) => Err(e), Ok(v) => { @@ -434,22 +442,72 @@ mod tests { ..Default::default() }); + // Inside of /a/specific and /a/specific/dir/name perms.check_read("/a/specific/dir/name").unwrap(); perms.check_write("/a/specific/dir/name").unwrap(); + // Inside of /a/specific but outside of /a/specific/dir/name + perms.check_read("/a/specific/dir").unwrap(); + perms.check_write("/a/specific/dir").unwrap(); + + // Inside of /a/specific and /a/specific/dir/name + perms.check_read("/a/specific/dir/name/inner").unwrap(); + perms.check_write("/a/specific/dir/name/inner").unwrap(); + + // Inside of /a/specific but outside of /a/specific/dir/name perms.check_read("/a/specific/other/dir").unwrap(); perms.check_write("/a/specific/other/dir").unwrap(); + // Exact match with /b/c perms.check_read("/b/c").unwrap(); perms.check_write("/b/c").unwrap(); + // Sub path within /b/c perms.check_read("/b/c/sub/path").unwrap(); perms.check_write("/b/c/sub/path").unwrap(); + // Inside of /b but outside of /b/c perms.check_read("/b/e").unwrap_err(); perms.check_write("/b/e").unwrap_err(); + // Inside of /a but outside of /a/specific perms.check_read("/a/b").unwrap_err(); perms.check_write("/a/b").unwrap_err(); } + + #[test] + fn check_domains() { + let perms = DenoPermissions::from_flags(&DenoFlags { + net_whitelist: vec![ + "localhost".to_string(), + "deno.land".to_string(), + "127.0.0.1".to_string(), + ].to_vec(), + no_prompts: true, + ..Default::default() + }); + + // http and https for the same domain should pass + perms.check_net("http://localhost/test/url").unwrap(); + perms.check_net("https://localhost/test/url").unwrap(); + + // Correct domain should pass but typo should not + perms + .check_net("https://deno.land/std/http/file_server.ts") + .unwrap(); + perms + .check_net("https://deno.lands/std/http/file_server.ts") + .unwrap_err(); + + // Correct ipv4 address should succeed but type should not + perms.check_net("https://127.0.0.1").unwrap(); + perms.check_net("https://127.0.0.2").unwrap_err(); + // TODO(afinch7) This currently succeeds but we may want to + // change this behavior in the future. + perms.check_net("https://127.0.0.1:3000").unwrap(); + + // Completely different hosts should also fail + perms.check_net("https://somedomain/").unwrap_err(); + perms.check_net("https://192.168.0.1/").unwrap_err(); + } } From 735c0b6320d8876b74b56220353610c8f4bfbcb9 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Thu, 2 May 2019 17:23:06 -0400 Subject: [PATCH 06/17] simplified flags and added path resolution before permissions checks. --- cli/deno_dir.rs | 55 +++++++------ cli/flags.rs | 57 +++++++------ cli/ops.rs | 197 +++++++++++++++++++++++++++++++++----------- js/read_dir_test.ts | 2 +- 4 files changed, 212 insertions(+), 99 deletions(-) diff --git a/cli/deno_dir.rs b/cli/deno_dir.rs index 5dc9afed3663f..3f1f6376639f3 100644 --- a/cli/deno_dir.rs +++ b/cli/deno_dir.rs @@ -271,36 +271,14 @@ impl DenoDir { referrer: &str, ) -> Result { let specifier = self.src_file_to_url(specifier); - let mut referrer = self.src_file_to_url(referrer); + let referrer = self.src_file_to_url(referrer); debug!( "resolve_module specifier {} referrer {}", specifier, referrer ); - if referrer.starts_with('.') { - let cwd = std::env::current_dir().unwrap(); - let referrer_path = cwd.join(referrer); - referrer = referrer_path.to_str().unwrap().to_string() + "/"; - } - - let j = if is_remote(&specifier) - || (Path::new(&specifier).is_absolute() && !is_remote(&referrer)) - { - parse_local_or_remote(&specifier)? - } else if referrer.ends_with('/') { - let r = Url::from_directory_path(&referrer); - // TODO(ry) Properly handle error. - if r.is_err() { - error!("Url::from_directory_path error {}", referrer); - } - let base = r.unwrap(); - base.join(specifier.as_ref())? - } else { - let base = parse_local_or_remote(&referrer)?; - base.join(specifier.as_ref())? - }; - Ok(j) + resolve_file_url(specifier, referrer) } /// Returns (module name, local filename) @@ -853,6 +831,35 @@ fn save_source_code_headers( } } +pub fn resolve_file_url( + specifier: String, + mut referrer: String, +) -> Result { + if referrer.starts_with('.') { + let cwd = std::env::current_dir().unwrap(); + let referrer_path = cwd.join(referrer); + referrer = referrer_path.to_str().unwrap().to_string() + "/"; + } + + let j = if is_remote(&specifier) + || (Path::new(&specifier).is_absolute() && !is_remote(&referrer)) + { + parse_local_or_remote(&specifier)? + } else if referrer.ends_with('/') { + let r = Url::from_directory_path(&referrer); + // TODO(ry) Properly handle error. + if r.is_err() { + error!("Url::from_directory_path error {}", referrer); + } + let base = r.unwrap(); + base.join(specifier.as_ref())? + } else { + let base = parse_local_or_remote(&referrer)?; + base.join(specifier.as_ref())? + }; + Ok(j) +} + #[cfg(test)] mod tests { use super::*; diff --git a/cli/flags.rs b/cli/flags.rs index 7d8468b3d5825..695e270691504 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -47,30 +47,27 @@ pub fn create_cli_app<'a, 'b>() -> App<'a, 'b> { ).arg( Arg::with_name("allow-read") .long("allow-read") - .help("Allow file system read access"), - ).arg( - Arg::with_name("read-wl") - .long("read-wl") + .min_values(0) .takes_value(true) - .help("Allow file system read access to specific file or directory"), + .use_delimiter(true) + .require_equals(true) + .help("Allow file system read access"), ).arg( Arg::with_name("allow-write") .long("allow-write") - .help("Allow file system write access"), - ).arg( - Arg::with_name("write-wl") - .long("write-wl") + .min_values(0) .takes_value(true) - .help("Allow file system write access to specific file or directory"), + .use_delimiter(true) + .require_equals(true) + .help("Allow file system write access"), ).arg( Arg::with_name("allow-net") .long("allow-net") - .help("Allow network access"), - ).arg( - Arg::with_name("net-wl") - .long("net-wl") + .min_values(0) .takes_value(true) - .help("Allow file system net access to specific domain"), + .use_delimiter(true) + .require_equals(true) + .help("Allow network access"), ).arg( Arg::with_name("allow-env") .long("allow-env") @@ -166,22 +163,28 @@ pub fn parse_flags(matches: ArgMatches) -> DenoFlags { flags.reload = true; } if matches.is_present("allow-read") { - flags.allow_read = true; - } - if let Some(read_wl) = matches.values_of("read-wl") { - flags.read_whitelist = read_wl.map(|s| s.to_string()).collect(); + if matches.value_of("allow-read").is_some() { + let read_wl = matches.values_of("allow-read").unwrap(); + flags.read_whitelist = read_wl.map(|s| s.to_string()).collect(); + } else { + flags.allow_read = true; + } } if matches.is_present("allow-write") { - flags.allow_write = true; - } - if let Some(write_wl) = matches.values_of("write-wl") { - flags.write_whitelist = write_wl.map(|s| s.to_string()).collect(); + if matches.value_of("allow-write").is_some() { + let write_wl = matches.values_of("allow-write").unwrap(); + flags.write_whitelist = write_wl.map(|s| s.to_string()).collect(); + } else { + flags.allow_write = true; + } } if matches.is_present("allow-net") { - flags.allow_net = true; - } - if let Some(net_wl) = matches.values_of("net-wl") { - flags.net_whitelist = net_wl.map(|s| s.to_string()).collect(); + if matches.value_of("allow-net").is_some() { + let net_wl = matches.values_of("allow-net").unwrap(); + flags.net_whitelist = net_wl.map(|s| s.to_string()).collect(); + } else { + flags.allow_net = true; + } } if matches.is_present("allow-env") { flags.allow_env = true; diff --git a/cli/ops.rs b/cli/ops.rs index 044e0fc20aaf3..472fe21de72da 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -1,6 +1,7 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. use atty; use crate::ansi; +use crate::deno_dir; use crate::errors; use crate::errors::{DenoError, DenoResult, ErrorKind}; use crate::fs as deno_fs; @@ -745,7 +746,13 @@ fn op_mkdir( ) -> Box { assert_eq!(data.len(), 0); let inner = base.inner_as_mkdir().unwrap(); - let path = String::from(inner.path().unwrap()); + let path = match deno_dir::resolve_file_url( + inner.path().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.path().to_string(), + }; let recursive = inner.recursive(); let mode = inner.mode(); @@ -768,7 +775,13 @@ fn op_chmod( assert_eq!(data.len(), 0); let inner = base.inner_as_chmod().unwrap(); let _mode = inner.mode(); - let path = String::from(inner.path().unwrap()); + let path = match deno_dir::resolve_file_url( + inner.path().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.path().to_string(), + }; if let Err(e) = state.check_write(&path) { return odd_future(e); @@ -807,8 +820,14 @@ fn op_open( assert_eq!(data.len(), 0); let cmd_id = base.cmd_id(); let inner = base.inner_as_open().unwrap(); - let filename_str = inner.filename().unwrap(); - let filename = PathBuf::from(&filename_str); + let filename = match deno_dir::resolve_file_url( + inner.filename().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let filename_ = filename.to_str().unwrap(); let mode = inner.mode().unwrap(); let mut open_options = tokio::fs::OpenOptions::new(); @@ -849,20 +868,20 @@ fn op_open( match mode { "r" => { - if let Err(e) = state.check_read(&filename_str) { + if let Err(e) = state.check_read(filename_) { return odd_future(e); } } "w" | "a" | "x" => { - if let Err(e) = state.check_write(&filename_str) { + if let Err(e) = state.check_write(filename_) { return odd_future(e); } } &_ => { - if let Err(e) = state.check_read(&filename_str) { + if let Err(e) = state.check_read(filename_) { return odd_future(e); } - if let Err(e) = state.check_write(&filename_str) { + if let Err(e) = state.check_write(filename_) { return odd_future(e); } } @@ -1036,11 +1055,17 @@ fn op_remove( ) -> Box { assert_eq!(data.len(), 0); let inner = base.inner_as_remove().unwrap(); - let path_ = inner.path().unwrap(); - let path = PathBuf::from(path_); + let path = match deno_dir::resolve_file_url( + inner.path().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let path_ = path.to_str().unwrap(); let recursive = inner.recursive(); - if let Err(e) = state.check_write(path.to_str().unwrap()) { + if let Err(e) = state.check_write(path_) { return odd_future(e); } @@ -1065,19 +1090,31 @@ fn op_copy_file( ) -> Box { assert_eq!(data.len(), 0); let inner = base.inner_as_copy_file().unwrap(); - let from_ = inner.from().unwrap(); - let from = PathBuf::from(from_); - let to_ = inner.to().unwrap(); - let to = PathBuf::from(to_); + let from = match deno_dir::resolve_file_url( + inner.from().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let from_ = from.to_str().unwrap(); + let to = match deno_dir::resolve_file_url( + inner.to().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v, + }; + let to_ = to.path(); - if let Err(e) = state.check_read(&from_) { + if let Err(e) = state.check_read(from_) { return odd_future(e); } if let Err(e) = state.check_write(&to_) { return odd_future(e); } - debug!("op_copy_file {} {}", from.display(), to.display()); + debug!("op_copy_file {} {}", from.display(), to); blocking(base.sync(), move || { // On *nix, Rust deem non-existent path as invalid input // See https://github.com/rust-lang/rust/issues/54800 @@ -1089,7 +1126,7 @@ fn op_copy_file( )); } - fs::copy(&from, &to)?; + fs::copy(&from, &to.to_file_path().unwrap())?; Ok(empty_buf()) }) } @@ -1148,8 +1185,14 @@ fn op_stat( assert_eq!(data.len(), 0); let inner = base.inner_as_stat().unwrap(); let cmd_id = base.cmd_id(); - let filename_ = inner.filename().unwrap(); - let filename = PathBuf::from(filename_); + let filename = match deno_dir::resolve_file_url( + inner.filename().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let filename_ = filename.to_str().unwrap().to_string(); let lstat = inner.lstat(); if let Err(e) = state.check_read(&filename_) { @@ -1165,6 +1208,8 @@ fn op_stat( fs::metadata(&filename)? }; + let filename_str = builder.create_string(&filename_); + let inner = msg::StatRes::create( builder, &msg::StatResArgs { @@ -1176,6 +1221,7 @@ fn op_stat( created: to_seconds!(metadata.created()), mode: get_mode(&metadata.permissions()), has_mode: cfg!(target_family = "unix"), + path: Some(filename_str), ..Default::default() }, ); @@ -1200,16 +1246,23 @@ fn op_read_dir( assert_eq!(data.len(), 0); let inner = base.inner_as_read_dir().unwrap(); let cmd_id = base.cmd_id(); - let path = String::from(inner.path().unwrap()); + let path = match deno_dir::resolve_file_url( + inner.path().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let path_ = path.to_str().unwrap(); - if let Err(e) = state.check_read(&path) { + if let Err(e) = state.check_read(path_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_read_dir {}", path); + debug!("op_read_dir {}", path.display()); let builder = &mut FlatBufferBuilder::new(); - let entries: Vec<_> = fs::read_dir(Path::new(&path))? + let entries: Vec<_> = fs::read_dir(path)? .map(|entry| { let entry = entry.unwrap(); let metadata = entry.metadata().unwrap(); @@ -1260,15 +1313,28 @@ fn op_rename( ) -> Box { assert_eq!(data.len(), 0); let inner = base.inner_as_rename().unwrap(); - let oldpath = PathBuf::from(inner.oldpath().unwrap()); - let newpath_ = inner.newpath().unwrap(); - let newpath = PathBuf::from(newpath_); + let oldpath = match deno_dir::resolve_file_url( + inner.oldpath().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let newpath = match deno_dir::resolve_file_url( + inner.newpath().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v, + }; + let newpath_ = newpath.path(); + if let Err(e) = state.check_write(&newpath_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_rename {} {}", oldpath.display(), newpath.display()); - fs::rename(&oldpath, &newpath)?; + debug!("op_rename {} {}", oldpath.display(), newpath); + fs::rename(&oldpath, &newpath.to_file_path().unwrap())?; Ok(empty_buf()) }) } @@ -1280,17 +1346,29 @@ fn op_link( ) -> Box { assert_eq!(data.len(), 0); let inner = base.inner_as_link().unwrap(); - let oldname = PathBuf::from(inner.oldname().unwrap()); - let newname_ = inner.newname().unwrap(); - let newname = PathBuf::from(newname_); + let oldname = match deno_dir::resolve_file_url( + inner.oldname().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let newname = match deno_dir::resolve_file_url( + inner.newname().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v, + }; + let newname_ = newname.path(); if let Err(e) = state.check_write(&newname_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_link {} {}", oldname.display(), newname.display()); - std::fs::hard_link(&oldname, &newname)?; + debug!("op_link {} {}", oldname.display(), newname); + std::fs::hard_link(&oldname, &newname.to_file_path().unwrap())?; Ok(empty_buf()) }) } @@ -1302,9 +1380,21 @@ fn op_symlink( ) -> Box { assert_eq!(data.len(), 0); let inner = base.inner_as_symlink().unwrap(); - let oldname = PathBuf::from(inner.oldname().unwrap()); - let newname_ = inner.newname().unwrap(); - let newname = PathBuf::from(newname_); + let oldname = match deno_dir::resolve_file_url( + inner.oldname().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let newname = match deno_dir::resolve_file_url( + inner.newname().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v, + }; + let newname_ = newname.path(); if let Err(e) = state.check_write(&newname_) { return odd_future(e); @@ -1317,9 +1407,9 @@ fn op_symlink( )); } blocking(base.sync(), move || -> OpResult { - debug!("op_symlink {} {}", oldname.display(), newname.display()); + debug!("op_symlink {} {}", oldname.display(), newname); #[cfg(any(unix))] - std::os::unix::fs::symlink(&oldname, &newname)?; + std::os::unix::fs::symlink(&oldname, &newname.to_file_path().unwrap())?; Ok(empty_buf()) }) } @@ -1332,16 +1422,22 @@ fn op_read_link( assert_eq!(data.len(), 0); let inner = base.inner_as_readlink().unwrap(); let cmd_id = base.cmd_id(); - let name_ = inner.name().unwrap(); - let name = PathBuf::from(name_); + let name = match deno_dir::resolve_file_url( + inner.name().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v, + }; + let name_ = name.path(); - if let Err(e) = state.check_read(&name_) { + if let Err(e) = state.check_read(name_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_read_link {}", name.display()); - let path = fs::read_link(&name)?; + debug!("op_read_link {}", name); + let path = fs::read_link(&name.to_file_path().unwrap())?; let builder = &mut FlatBufferBuilder::new(); let path_off = builder.create_string(path.to_str().unwrap()); let inner = msg::ReadlinkRes::create( @@ -1437,15 +1533,22 @@ fn op_truncate( assert_eq!(data.len(), 0); let inner = base.inner_as_truncate().unwrap(); - let filename = String::from(inner.name().unwrap()); + let filename = match deno_dir::resolve_file_url( + inner.name().unwrap().to_string(), + ".".to_string(), + ) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v.to_file_path().unwrap(), + }; + let filename_ = filename.to_str().unwrap().to_string(); let len = inner.len(); - if let Err(e) = state.check_write(&filename) { + if let Err(e) = state.check_write(&filename_) { return odd_future(e); } blocking(base.sync(), move || { - debug!("op_truncate {} {}", filename, len); + debug!("op_truncate {} {}", filename_, len); let f = fs::OpenOptions::new().write(true).open(&filename)?; f.set_len(u64::from(len))?; Ok(empty_buf()) diff --git a/js/read_dir_test.ts b/js/read_dir_test.ts index c2d90642ca0ef..81b023158fd75 100644 --- a/js/read_dir_test.ts +++ b/js/read_dir_test.ts @@ -13,7 +13,7 @@ function assertSameContent(files: FileInfo[]): void { } if (file.name === "002_hello.ts") { - assertEquals(file.path, `tests/${file.name}`); + assert(file.path.endsWith(`tests/${file.name}`)); assertEquals(file.mode!, Deno.statSync(`tests/${file.name}`).mode!); counter++; } From 7af6ea4d8736ed94bdc9f82938bdeaa61f8a7fc7 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Thu, 2 May 2019 17:45:49 -0400 Subject: [PATCH 07/17] fmt --- cli/ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ops.rs b/cli/ops.rs index fd659d2e7a6e7..bddc3738050c7 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -1,8 +1,8 @@ // Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. use atty; use crate::ansi; -use crate::deno_dir; use crate::compiler::get_compiler_config; +use crate::deno_dir; use crate::errors; use crate::errors::{DenoError, DenoResult, ErrorKind}; use crate::fs as deno_fs; From ea7aa178b6cdd233c22acfa3096c5ea26377cecf Mon Sep 17 00:00:00 2001 From: afinch7 Date: Sat, 4 May 2019 12:38:05 -0400 Subject: [PATCH 08/17] added port checking for network whitelist. --- cli/permissions.rs | 72 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/cli/permissions.rs b/cli/permissions.rs index 4e5b887668cb7..e4f31c169b61f 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -185,7 +185,7 @@ impl DenoPermissions { true => Ok(()), false => match state { // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( &format!("read access to \"{}\"", filename), ) { @@ -211,7 +211,7 @@ impl DenoPermissions { true => Ok(()), false => match state { // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( &format!("write access to \"{}\"", filename), ) { @@ -235,9 +235,30 @@ impl DenoPermissions { state => { let parsed_url_result = url::Url::parse(url_str); let whitelist_result = match parsed_url_result { - Ok(parsed_url) => match parsed_url.host() { - Some(host) => self.net_whitelist.contains(&host.to_string()), - None => false, + Ok(parsed_url) => match parsed_url.origin() { + url::Origin::Tuple(_, url::Host::Domain(domain), port) => { + match self.net_whitelist.contains(&domain) { + false => { + self.net_whitelist.contains(&format!("{}:{}", domain, port)) + } + true => true, + } + } + url::Origin::Tuple(_, url::Host::Ipv4(ip), port) => match self + .net_whitelist + .contains(&format!("{}", ip)) + { + false => self.net_whitelist.contains(&format!("{}:{}", ip, port)), + true => true, + }, + url::Origin::Tuple(_, url::Host::Ipv6(ip), port) => match self + .net_whitelist + .contains(&format!("{}", ip)) + { + false => self.net_whitelist.contains(&format!("{}:{}", ip, port)), + true => true, + }, + _ => panic!("Unexpected url origin type."), }, Err(_) => false, }; @@ -246,7 +267,7 @@ impl DenoPermissions { false => { match state { // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => Ok(()), + PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => { match self.try_permissions_prompt(&format!( "network access to \"{}\"", @@ -481,7 +502,9 @@ mod tests { net_whitelist: vec![ "localhost".to_string(), "deno.land".to_string(), + "github.com:3000".to_string(), "127.0.0.1".to_string(), + "172.16.0.2:8000".to_string(), ].to_vec(), no_prompts: true, ..Default::default() @@ -491,22 +514,47 @@ mod tests { perms.check_net("http://localhost/test/url").unwrap(); perms.check_net("https://localhost/test/url").unwrap(); - // Correct domain should pass but typo should not + // Correct domain + any port should pass incorrect shouldn't perms .check_net("https://deno.land/std/http/file_server.ts") .unwrap(); + perms + .check_net("https://deno.land:3000/std/http/file_server.ts") + .unwrap(); perms .check_net("https://deno.lands/std/http/file_server.ts") .unwrap_err(); + perms + .check_net("https://deno.lands:3000/std/http/file_server.ts") + .unwrap_err(); - // Correct ipv4 address should succeed but type should not + // Correct domain + port should pass all other combinations should fail + perms + .check_net("https://github.com:3000/denoland/deno") + .unwrap(); + perms + .check_net("https://github.com/denoland/deno") + .unwrap_err(); + perms + .check_net("https://github.com:2000/denoland/deno") + .unwrap_err(); + perms + .check_net("https://github.net:3000/denoland/deno") + .unwrap_err(); + + // Correct ipv4 address + any port should pass others should not perms.check_net("https://127.0.0.1").unwrap(); - perms.check_net("https://127.0.0.2").unwrap_err(); - // TODO(afinch7) This currently succeeds but we may want to - // change this behavior in the future. perms.check_net("https://127.0.0.1:3000").unwrap(); + perms.check_net("https://127.0.0.2").unwrap_err(); + perms.check_net("https://127.0.0.2:3000").unwrap_err(); + + // Correct address + port should pass all other combinations should fail + perms.check_net("https://172.16.0.2:8000").unwrap(); + perms.check_net("https://172.16.0.2").unwrap_err(); + perms.check_net("https://172.16.0.2:6000").unwrap_err(); + perms.check_net("https://172.16.0.1:8000").unwrap_err(); - // Completely different hosts should also fail + // Just some random hosts that should fail perms.check_net("https://somedomain/").unwrap_err(); perms.check_net("https://192.168.0.1/").unwrap_err(); } From 209a875ae50cc9bd3bbc423b4f37543433ffd2e7 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Sat, 4 May 2019 13:10:40 -0400 Subject: [PATCH 09/17] fmt --- cli/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/flags.rs b/cli/flags.rs index d7ea869cf4130..dca26ffd1c2ea 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -820,7 +820,7 @@ mod tests { assert_eq!(subcommand, DenoSubcommand::Run); assert_eq!(argv, svec!["deno", "script.ts"]); } - #[test] + #[test] fn test_flags_from_vec_21() { let (flags, subcommand, argv) = flags_from_vec(svec![ "deno", From a9ccef2877b4fc426b6dce25d4c52846a44f78fd Mon Sep 17 00:00:00 2001 From: afinch7 Date: Sat, 4 May 2019 13:53:13 -0400 Subject: [PATCH 10/17] fixed clippy errors and warnings --- cli/flags.rs | 9 +++-- cli/permissions.rs | 96 ++++++++++++++++++++++------------------------ 2 files changed, 52 insertions(+), 53 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index dca26ffd1c2ea..51d172ca428af 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -307,7 +307,8 @@ pub fn parse_flags(matches: ArgMatches) -> DenoFlags { if run_matches.is_present("allow-read") { if run_matches.value_of("allow-read").is_some() { let read_wl = run_matches.values_of("allow-read").unwrap(); - flags.read_whitelist = read_wl.map(|s| s.to_string()).collect(); + flags.read_whitelist = + read_wl.map(std::string::ToString::to_string).collect(); } else { flags.allow_read = true; } @@ -315,7 +316,8 @@ pub fn parse_flags(matches: ArgMatches) -> DenoFlags { if run_matches.is_present("allow-write") { if run_matches.value_of("allow-write").is_some() { let write_wl = run_matches.values_of("allow-write").unwrap(); - flags.write_whitelist = write_wl.map(|s| s.to_string()).collect(); + flags.write_whitelist = + write_wl.map(std::string::ToString::to_string).collect(); } else { flags.allow_write = true; } @@ -323,7 +325,8 @@ pub fn parse_flags(matches: ArgMatches) -> DenoFlags { if run_matches.is_present("allow-net") { if run_matches.value_of("allow-net").is_some() { let net_wl = run_matches.values_of("allow-net").unwrap(); - flags.net_whitelist = net_wl.map(|s| s.to_string()).collect(); + flags.net_whitelist = + net_wl.map(std::string::ToString::to_string).collect(); } else { flags.allow_net = true; } diff --git a/cli/permissions.rs b/cli/permissions.rs index e4f31c169b61f..3503329cea3a1 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -181,9 +181,10 @@ impl DenoPermissions { match self.allow_read.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - match check_path_white_list(filename, &self.read_whitelist) { - true => Ok(()), - false => match state { + if check_path_white_list(filename, &self.read_whitelist) { + Ok(()) + } else { + match state { // This shouldn't be possible but I guess rust doesn't realize. PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( @@ -197,7 +198,7 @@ impl DenoPermissions { } }, PermissionAccessorState::Deny => Err(permission_denied()), - }, + } } } } @@ -207,9 +208,10 @@ impl DenoPermissions { match self.allow_write.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - match check_path_white_list(filename, &self.write_whitelist) { - true => Ok(()), - false => match state { + if check_path_white_list(filename, &self.write_whitelist) { + Ok(()) + } else { + match state { // This shouldn't be possible but I guess rust doesn't realize. PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( @@ -223,7 +225,7 @@ impl DenoPermissions { } }, PermissionAccessorState::Deny => Err(permission_denied()), - }, + } } } } @@ -237,53 +239,47 @@ impl DenoPermissions { let whitelist_result = match parsed_url_result { Ok(parsed_url) => match parsed_url.origin() { url::Origin::Tuple(_, url::Host::Domain(domain), port) => { - match self.net_whitelist.contains(&domain) { - false => { - self.net_whitelist.contains(&format!("{}:{}", domain, port)) - } - true => true, + if self.net_whitelist.contains(&domain) { + true + } else { + self.net_whitelist.contains(&format!("{}:{}", domain, port)) + } + } + url::Origin::Tuple(_, url::Host::Ipv4(ip), port) => { + if self.net_whitelist.contains(&format!("{}", ip)) { + true + } else { + self.net_whitelist.contains(&format!("{}:{}", ip, port)) + } + } + url::Origin::Tuple(_, url::Host::Ipv6(ip), port) => { + if self.net_whitelist.contains(&format!("{}", ip)) { + true + } else { + self.net_whitelist.contains(&format!("{}:{}", ip, port)) } } - url::Origin::Tuple(_, url::Host::Ipv4(ip), port) => match self - .net_whitelist - .contains(&format!("{}", ip)) - { - false => self.net_whitelist.contains(&format!("{}:{}", ip, port)), - true => true, - }, - url::Origin::Tuple(_, url::Host::Ipv6(ip), port) => match self - .net_whitelist - .contains(&format!("{}", ip)) - { - false => self.net_whitelist.contains(&format!("{}:{}", ip, port)), - true => true, - }, _ => panic!("Unexpected url origin type."), }, Err(_) => false, }; - match whitelist_result { - true => Ok(()), - false => { - match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => unreachable!(), - PermissionAccessorState::Ask => { - match self.try_permissions_prompt(&format!( - "network access to \"{}\"", - url_str - )) { - Err(e) => Err(e), - Ok(v) => { - self.allow_net.update_with_prompt_result(&v); - v.check()?; - Ok(()) - } - } - } - PermissionAccessorState::Deny => Err(permission_denied()), + if whitelist_result { + return Ok(()); + } + match state { + // This shouldn't be possible but I guess rust doesn't realize. + PermissionAccessorState::Allow => unreachable!(), + PermissionAccessorState::Ask => match self.try_permissions_prompt( + &format!("network access to \"{}\"", url_str), + ) { + Err(e) => Err(e), + Ok(v) => { + self.allow_net.update_with_prompt_result(&v); + v.check()?; + Ok(()) } - } + }, + PermissionAccessorState::Deny => Err(permission_denied()), } } } @@ -436,12 +432,12 @@ fn check_path_white_list( return true; } - while path_buf.pop() == true { + while path_buf.pop() { if white_list.contains(path_buf.to_str().unwrap()) { return true; } } - return false; + false } #[cfg(test)] From b133fb462a942a360df19d001cc515d488c774e0 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Tue, 7 May 2019 13:57:22 -0400 Subject: [PATCH 11/17] fixed windows unit test issues. --- cli/ops.rs | 18 +++++++++--------- js/read_dir_test.ts | 8 +++++++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/cli/ops.rs b/cli/ops.rs index 5e9e5bfa7a219..0a44ed63da8de 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -44,7 +44,6 @@ use std; use std::convert::From; use std::fs; use std::net::Shutdown; -use std::path::Path; use std::path::PathBuf; use std::process::Command; use std::time::{Duration, Instant, UNIX_EPOCH}; @@ -821,18 +820,19 @@ fn op_mkdir( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.path().to_string(), + Ok(v) => v.to_file_path().unwrap(), }; + let path_ = path.to_str().unwrap().to_string(); let recursive = inner.recursive(); let mode = inner.mode(); - if let Err(e) = state.check_write(&path) { + if let Err(e) = state.check_write(&path_) { return odd_future(e); } blocking(base.sync(), move || { - debug!("op_mkdir {}", path); - deno_fs::mkdir(Path::new(&path), mode, recursive)?; + debug!("op_mkdir {}", path_); + deno_fs::mkdir(&path, mode, recursive)?; Ok(empty_buf()) }) } @@ -850,16 +850,16 @@ fn op_chmod( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.path().to_string(), + Ok(v) => v.to_file_path().unwrap(), }; + let path_ = path.to_str().unwrap().to_string(); - if let Err(e) = state.check_write(&path) { + if let Err(e) = state.check_write(&path_) { return odd_future(e); } blocking(base.sync(), move || { - debug!("op_chmod {}", &path); - let path = PathBuf::from(&path); + debug!("op_chmod {}", &path_); // Still check file/dir exists on windows let _metadata = fs::metadata(&path)?; // Only work in unix diff --git a/js/read_dir_test.ts b/js/read_dir_test.ts index 49755bb079078..55badd0dba32a 100644 --- a/js/read_dir_test.ts +++ b/js/read_dir_test.ts @@ -3,6 +3,8 @@ import { testPerm, assert, assertEquals } from "./test_util.ts"; type FileInfo = Deno.FileInfo; +const isWin = Deno.build.os === "win"; + function assertSameContent(files: FileInfo[]): void { let counter = 0; @@ -13,7 +15,11 @@ function assertSameContent(files: FileInfo[]): void { } if (file.name === "002_hello.ts") { - assert(file.path.endsWith(`tests/${file.name}`)); + if (isWin) { + assert(file.path.endsWith(`tests\\${file.name}`)); + } else { + assert(file.path.endsWith(`tests/${file.name}`)); + } assertEquals(file.mode!, Deno.statSync(`tests/${file.name}`).mode!); counter++; } From 734c0f2211b6c2f3f9167c1ef21abe5ed945a10e Mon Sep 17 00:00:00 2001 From: afinch7 Date: Tue, 7 May 2019 15:45:44 -0400 Subject: [PATCH 12/17] consistent path resolution for all ops --- cli/ops.rs | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/cli/ops.rs b/cli/ops.rs index 0a44ed63da8de..52471103a73b6 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -1188,9 +1188,9 @@ fn op_copy_file( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v, + Ok(v) => v.to_file_path().unwrap(), }; - let to_ = to.path(); + let to_ = to.to_str().unwrap().to_string(); if let Err(e) = state.check_read(from_) { return odd_future(e); @@ -1199,7 +1199,7 @@ fn op_copy_file( return odd_future(e); } - debug!("op_copy_file {} {}", from.display(), to); + debug!("op_copy_file {} {}", from.display(), to.display()); blocking(base.sync(), move || { // On *nix, Rust deem non-existent path as invalid input // See https://github.com/rust-lang/rust/issues/54800 @@ -1211,7 +1211,7 @@ fn op_copy_file( )); } - fs::copy(&from, &to.to_file_path().unwrap())?; + fs::copy(&from, &to)?; Ok(empty_buf()) }) } @@ -1410,16 +1410,16 @@ fn op_rename( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v, + Ok(v) => v.to_file_path().unwrap(), }; - let newpath_ = newpath.path(); + let newpath_ = newpath.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&newpath_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_rename {} {}", oldpath.display(), newpath); - fs::rename(&oldpath, &newpath.to_file_path().unwrap())?; + debug!("op_rename {} {}", oldpath.display(), newpath.display()); + fs::rename(&oldpath, &newpath)?; Ok(empty_buf()) }) } @@ -1443,17 +1443,17 @@ fn op_link( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v, + Ok(v) => v.to_file_path().unwrap(), }; - let newname_ = newname.path(); + let newname_ = newname.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&newname_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_link {} {}", oldname.display(), newname); - std::fs::hard_link(&oldname, &newname.to_file_path().unwrap())?; + debug!("op_link {} {}", oldname.display(), newname.display()); + std::fs::hard_link(&oldname, &newname)?; Ok(empty_buf()) }) } @@ -1477,9 +1477,9 @@ fn op_symlink( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v, + Ok(v) => v.to_file_path().unwrap(), }; - let newname_ = newname.path(); + let newname_ = newname.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&newname_) { return odd_future(e); @@ -1492,7 +1492,7 @@ fn op_symlink( )); } blocking(base.sync(), move || -> OpResult { - debug!("op_symlink {} {}", oldname.display(), newname); + debug!("op_symlink {} {}", oldname.display(), newname.display()); #[cfg(any(unix))] std::os::unix::fs::symlink(&oldname, &newname.to_file_path().unwrap())?; Ok(empty_buf()) @@ -1512,17 +1512,17 @@ fn op_read_link( ".".to_string(), ) { Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v, + Ok(v) => v.to_file_path().unwrap(), }; - let name_ = name.path(); + let name_ = name.to_str().unwrap().to_string(); - if let Err(e) = state.check_read(name_) { + if let Err(e) = state.check_read(&name_) { return odd_future(e); } blocking(base.sync(), move || -> OpResult { - debug!("op_read_link {}", name); - let path = fs::read_link(&name.to_file_path().unwrap())?; + debug!("op_read_link {}", name.display()); + let path = fs::read_link(&name)?; let builder = &mut FlatBufferBuilder::new(); let path_off = builder.create_string(path.to_str().unwrap()); let inner = msg::ReadlinkRes::create( From bbc45344e1582c839f4e280f1d77b6b7fba7bd06 Mon Sep 17 00:00:00 2001 From: andy finch Date: Tue, 7 May 2019 16:00:58 -0400 Subject: [PATCH 13/17] not sure why this didn't error on windows --- cli/ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ops.rs b/cli/ops.rs index 52471103a73b6..d3e86ad01c4a0 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -1494,7 +1494,7 @@ fn op_symlink( blocking(base.sync(), move || -> OpResult { debug!("op_symlink {} {}", oldname.display(), newname.display()); #[cfg(any(unix))] - std::os::unix::fs::symlink(&oldname, &newname.to_file_path().unwrap())?; + std::os::unix::fs::symlink(&oldname, &newname)?; Ok(empty_buf()) }) } From 9ce9c2f15dd0a9281fde1af031550c9fbecde3df Mon Sep 17 00:00:00 2001 From: afinch7 Date: Wed, 8 May 2019 13:16:58 -0400 Subject: [PATCH 14/17] requested changes and added command line tests --- cli/ops.rs | 150 ++++++---------- cli/permissions.rs | 282 ++++++++++++++++++------------ tools/complex_permissions_test.py | 195 +++++++++++++++++++++ tools/complex_permissions_test.ts | 24 +++ tools/test.py | 2 + 5 files changed, 444 insertions(+), 209 deletions(-) create mode 100755 tools/complex_permissions_test.py create mode 100644 tools/complex_permissions_test.ts diff --git a/cli/ops.rs b/cli/ops.rs index d3e86ad01c4a0..4a825c7b7a91f 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -240,6 +240,12 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { } } +fn resolve_path(path: &str) -> Result { + let url = deno_dir::resolve_file_url(path.to_string(), ".".to_string()) + .map_err(|err| DenoError::from(err))?; + Ok(url.to_file_path().unwrap()) +} + // Returns a milliseconds and nanoseconds subsec // since the start time of the deno runtime. // If the High precision flag is not set, the @@ -815,12 +821,9 @@ fn op_mkdir( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_mkdir().unwrap(); - let path = match deno_dir::resolve_file_url( - inner.path().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let path = match resolve_path(inner.path().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let path_ = path.to_str().unwrap().to_string(); let recursive = inner.recursive(); @@ -845,12 +848,9 @@ fn op_chmod( assert!(data.is_none()); let inner = base.inner_as_chmod().unwrap(); let _mode = inner.mode(); - let path = match deno_dir::resolve_file_url( - inner.path().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let path = match resolve_path(inner.path().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let path_ = path.to_str().unwrap().to_string(); @@ -890,12 +890,9 @@ fn op_open( assert!(data.is_none()); let cmd_id = base.cmd_id(); let inner = base.inner_as_open().unwrap(); - let filename = match deno_dir::resolve_file_url( - inner.filename().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let filename = match resolve_path(inner.filename().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let filename_ = filename.to_str().unwrap(); let mode = inner.mode().unwrap(); @@ -1140,12 +1137,9 @@ fn op_remove( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_remove().unwrap(); - let path = match deno_dir::resolve_file_url( - inner.path().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let path = match resolve_path(inner.path().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let path_ = path.to_str().unwrap(); let recursive = inner.recursive(); @@ -1175,20 +1169,14 @@ fn op_copy_file( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_copy_file().unwrap(); - let from = match deno_dir::resolve_file_url( - inner.from().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let from = match resolve_path(inner.from().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let from_ = from.to_str().unwrap(); - let to = match deno_dir::resolve_file_url( - inner.to().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let to = match resolve_path(inner.to().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let to_ = to.to_str().unwrap().to_string(); @@ -1270,12 +1258,9 @@ fn op_stat( assert!(data.is_none()); let inner = base.inner_as_stat().unwrap(); let cmd_id = base.cmd_id(); - let filename = match deno_dir::resolve_file_url( - inner.filename().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let filename = match resolve_path(inner.filename().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let filename_ = filename.to_str().unwrap().to_string(); let lstat = inner.lstat(); @@ -1331,12 +1316,9 @@ fn op_read_dir( assert!(data.is_none()); let inner = base.inner_as_read_dir().unwrap(); let cmd_id = base.cmd_id(); - let path = match deno_dir::resolve_file_url( - inner.path().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let path = match resolve_path(inner.path().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let path_ = path.to_str().unwrap(); @@ -1398,19 +1380,13 @@ fn op_rename( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_rename().unwrap(); - let oldpath = match deno_dir::resolve_file_url( - inner.oldpath().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let oldpath = match resolve_path(inner.oldpath().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; - let newpath = match deno_dir::resolve_file_url( - inner.newpath().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let newpath = match resolve_path(inner.newpath().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let newpath_ = newpath.to_str().unwrap().to_string(); @@ -1431,19 +1407,13 @@ fn op_link( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_link().unwrap(); - let oldname = match deno_dir::resolve_file_url( - inner.oldname().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let oldname = match resolve_path(inner.oldname().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; - let newname = match deno_dir::resolve_file_url( - inner.newname().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let newname = match resolve_path(inner.newname().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let newname_ = newname.to_str().unwrap().to_string(); @@ -1465,19 +1435,13 @@ fn op_symlink( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_symlink().unwrap(); - let oldname = match deno_dir::resolve_file_url( - inner.oldname().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let oldname = match resolve_path(inner.oldname().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; - let newname = match deno_dir::resolve_file_url( - inner.newname().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let newname = match resolve_path(inner.newname().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let newname_ = newname.to_str().unwrap().to_string(); @@ -1507,12 +1471,9 @@ fn op_read_link( assert!(data.is_none()); let inner = base.inner_as_readlink().unwrap(); let cmd_id = base.cmd_id(); - let name = match deno_dir::resolve_file_url( - inner.name().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let name = match resolve_path(inner.name().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let name_ = name.to_str().unwrap().to_string(); @@ -1618,12 +1579,9 @@ fn op_truncate( assert!(data.is_none()); let inner = base.inner_as_truncate().unwrap(); - let filename = match deno_dir::resolve_file_url( - inner.name().unwrap().to_string(), - ".".to_string(), - ) { - Err(err) => return odd_future(DenoError::from(err)), - Ok(v) => v.to_file_path().unwrap(), + let filename = match resolve_path(inner.name().unwrap()) { + Err(err) => return odd_future(err), + Ok(v) => v, }; let filename_ = filename.to_str().unwrap().to_string(); let len = inner.len(); diff --git a/cli/permissions.rs b/cli/permissions.rs index 3503329cea3a1..5a0d0e69d64d1 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -185,8 +185,6 @@ impl DenoPermissions { Ok(()) } else { match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( &format!("read access to \"{}\"", filename), ) { @@ -198,6 +196,7 @@ impl DenoPermissions { } }, PermissionAccessorState::Deny => Err(permission_denied()), + _ => unreachable!(), } } } @@ -212,8 +211,6 @@ impl DenoPermissions { Ok(()) } else { match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( &format!("write access to \"{}\"", filename), ) { @@ -225,52 +222,69 @@ impl DenoPermissions { } }, PermissionAccessorState::Deny => Err(permission_denied()), + _ => unreachable!(), } } } } } - pub fn check_net(&self, url_str: &str) -> DenoResult<()> { + pub fn check_net(&self, url_or_host_and_port: &str) -> DenoResult<()> { match self.allow_net.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - let parsed_url_result = url::Url::parse(url_str); - let whitelist_result = match parsed_url_result { - Ok(parsed_url) => match parsed_url.origin() { - url::Origin::Tuple(_, url::Host::Domain(domain), port) => { - if self.net_whitelist.contains(&domain) { - true + // Try to parse as url first and do checks. + // Return None if unable parse the url or the parse result doesn't have + // a host(for strings like "localhost:1234" host is parsed as scheme). + let url_check_result = match url::Url::parse(url_or_host_and_port) { + Ok(url) => match url.host() { + Some(host) => { + if self.net_whitelist.contains(&format!("{}", host)) { + Some(true) } else { - self.net_whitelist.contains(&format!("{}:{}", domain, port)) + match url.port() { + Some(port) => Some( + self.net_whitelist.contains(&format!("{}:{}", host, port)), + ), + None => Some(false), + } } } - url::Origin::Tuple(_, url::Host::Ipv4(ip), port) => { - if self.net_whitelist.contains(&format!("{}", ip)) { - true - } else { - self.net_whitelist.contains(&format!("{}:{}", ip, port)) - } - } - url::Origin::Tuple(_, url::Host::Ipv6(ip), port) => { - if self.net_whitelist.contains(&format!("{}", ip)) { - true - } else { - self.net_whitelist.contains(&format!("{}:{}", ip, port)) - } - } - _ => panic!("Unexpected url origin type."), + None => None, }, - Err(_) => false, + Err(_) => None, }; - if whitelist_result { + + // If the first check is a None(unable to check) then try splitting by + // ":" and checking host and host + port combinations. + if match url_check_result { + None => { + let parts = url_or_host_and_port.split(":").collect::>(); + match parts.len() { + 2 => { + if self.net_whitelist.contains(&format!("{}", parts[0])) { + true + } else { + self + .net_whitelist + .contains(&format!("{}:{}", parts[0], parts[1])) + } + } + 1 => self.net_whitelist.contains(&format!("{}", parts[0])), + _ => panic!( + "Failed to parse origin string: {}", + url_or_host_and_port + ), + } + } + Some(v) => v, + } { return Ok(()); } + match state { - // This shouldn't be possible but I guess rust doesn't realize. - PermissionAccessorState::Allow => unreachable!(), PermissionAccessorState::Ask => match self.try_permissions_prompt( - &format!("network access to \"{}\"", url_str), + &format!("network access to \"{}\"", url_or_host_and_port), ) { Err(e) => Err(e), Ok(v) => { @@ -280,6 +294,7 @@ impl DenoPermissions { } }, PermissionAccessorState::Deny => Err(permission_denied()), + _ => unreachable!(), } } } @@ -428,14 +443,13 @@ fn check_path_white_list( ) -> bool { let mut path_buf = PathBuf::from(filename); - if white_list.contains(path_buf.to_str().unwrap()) { - return true; - } - - while path_buf.pop() { + loop { if white_list.contains(path_buf.to_str().unwrap()) { return true; } + if !path_buf.pop() { + break; + } } false } @@ -444,13 +458,14 @@ fn check_path_white_list( mod tests { use super::*; + // Creates vector of strings, Vec + macro_rules! svec { + ($($x:expr),*) => (vec![$($x.to_string()),*]); + } + #[test] fn check_paths() { - let whitelist = vec![ - "/a/specific/dir/name".to_string(), - "/a/specific".to_string(), - "/b/c".to_string(), - ].to_vec(); + let whitelist = svec!["/a/specific/dir/name", "/a/specific", "/b/c"]; let perms = DenoPermissions::from_flags(&DenoFlags { read_whitelist: whitelist.clone(), @@ -460,98 +475,139 @@ mod tests { }); // Inside of /a/specific and /a/specific/dir/name - perms.check_read("/a/specific/dir/name").unwrap(); - perms.check_write("/a/specific/dir/name").unwrap(); + assert!(perms.check_read("/a/specific/dir/name").is_ok()); + assert!(perms.check_write("/a/specific/dir/name").is_ok()); // Inside of /a/specific but outside of /a/specific/dir/name - perms.check_read("/a/specific/dir").unwrap(); - perms.check_write("/a/specific/dir").unwrap(); + assert!(perms.check_read("/a/specific/dir").is_ok()); + assert!(perms.check_write("/a/specific/dir").is_ok()); // Inside of /a/specific and /a/specific/dir/name - perms.check_read("/a/specific/dir/name/inner").unwrap(); - perms.check_write("/a/specific/dir/name/inner").unwrap(); + assert!(perms.check_read("/a/specific/dir/name/inner").is_ok()); + assert!(perms.check_write("/a/specific/dir/name/inner").is_ok()); // Inside of /a/specific but outside of /a/specific/dir/name - perms.check_read("/a/specific/other/dir").unwrap(); - perms.check_write("/a/specific/other/dir").unwrap(); + assert!(perms.check_read("/a/specific/other/dir").is_ok()); + assert!(perms.check_write("/a/specific/other/dir").is_ok()); // Exact match with /b/c - perms.check_read("/b/c").unwrap(); - perms.check_write("/b/c").unwrap(); + assert!(perms.check_read("/b/c").is_ok()); + assert!(perms.check_write("/b/c").is_ok()); // Sub path within /b/c - perms.check_read("/b/c/sub/path").unwrap(); - perms.check_write("/b/c/sub/path").unwrap(); + assert!(perms.check_read("/b/c/sub/path").is_ok()); + assert!(perms.check_write("/b/c/sub/path").is_ok()); // Inside of /b but outside of /b/c - perms.check_read("/b/e").unwrap_err(); - perms.check_write("/b/e").unwrap_err(); + assert!(perms.check_read("/b/e").is_err()); + assert!(perms.check_write("/b/e").is_err()); // Inside of /a but outside of /a/specific - perms.check_read("/a/b").unwrap_err(); - perms.check_write("/a/b").unwrap_err(); + assert!(perms.check_read("/a/b").is_err()); + assert!(perms.check_write("/a/b").is_err()); } #[test] - fn check_domains() { + fn check_net() { let perms = DenoPermissions::from_flags(&DenoFlags { - net_whitelist: vec![ - "localhost".to_string(), - "deno.land".to_string(), - "github.com:3000".to_string(), - "127.0.0.1".to_string(), - "172.16.0.2:8000".to_string(), - ].to_vec(), + net_whitelist: svec![ + "localhost", + "deno.land", + "github.com:3000", + "127.0.0.1", + "172.16.0.2:8000" + ], no_prompts: true, ..Default::default() }); - // http and https for the same domain should pass - perms.check_net("http://localhost/test/url").unwrap(); - perms.check_net("https://localhost/test/url").unwrap(); - - // Correct domain + any port should pass incorrect shouldn't - perms - .check_net("https://deno.land/std/http/file_server.ts") - .unwrap(); - perms - .check_net("https://deno.land:3000/std/http/file_server.ts") - .unwrap(); - perms - .check_net("https://deno.lands/std/http/file_server.ts") - .unwrap_err(); - perms - .check_net("https://deno.lands:3000/std/http/file_server.ts") - .unwrap_err(); - - // Correct domain + port should pass all other combinations should fail - perms - .check_net("https://github.com:3000/denoland/deno") - .unwrap(); - perms - .check_net("https://github.com/denoland/deno") - .unwrap_err(); - perms - .check_net("https://github.com:2000/denoland/deno") - .unwrap_err(); - perms - .check_net("https://github.net:3000/denoland/deno") - .unwrap_err(); - - // Correct ipv4 address + any port should pass others should not - perms.check_net("https://127.0.0.1").unwrap(); - perms.check_net("https://127.0.0.1:3000").unwrap(); - perms.check_net("https://127.0.0.2").unwrap_err(); - perms.check_net("https://127.0.0.2:3000").unwrap_err(); - - // Correct address + port should pass all other combinations should fail - perms.check_net("https://172.16.0.2:8000").unwrap(); - perms.check_net("https://172.16.0.2").unwrap_err(); - perms.check_net("https://172.16.0.2:6000").unwrap_err(); - perms.check_net("https://172.16.0.1:8000").unwrap_err(); - - // Just some random hosts that should fail - perms.check_net("https://somedomain/").unwrap_err(); - perms.check_net("https://192.168.0.1/").unwrap_err(); + // Any protocol + port for localhost should be ok, since we don't specify + assert!(perms.check_net("http://localhost").is_ok()); + assert!(perms.check_net("http://localhost:8080").is_ok()); + assert!(perms.check_net("https://localhost").is_ok()); + assert!(perms.check_net("https://localhost:4443").is_ok()); + assert!(perms.check_net("tcp://localhost:5000").is_ok()); + assert!(perms.check_net("udp://localhost:6000").is_ok()); + assert!(perms.check_net("localhost:1234").is_ok()); + + // Correct domain + any port and protocol should be ok incorrect shouldn't + assert!(perms.check_net("deno.land").is_ok()); + assert!( + perms + .check_net("https://deno.land/std/example/welcome.ts") + .is_ok() + ); + assert!(perms.check_net("deno.land:3000").is_ok()); + assert!( + perms + .check_net("https://deno.land:3000/std/example/welcome.ts") + .is_ok() + ); + assert!(perms.check_net("deno.lands").is_err()); + assert!( + perms + .check_net("https://deno.lands/std/example/welcome.ts") + .is_err() + ); + assert!(perms.check_net("deno.lands:3000").is_err()); + assert!( + perms + .check_net("https://deno.lands:3000/std/example/welcome.ts") + .is_err() + ); + + // Correct domain + port should be ok all other combinations should err + assert!(perms.check_net("github.com:3000").is_ok()); + assert!( + perms + .check_net("https://github.com:3000/denoland/deno") + .is_ok() + ); + assert!(perms.check_net("github.com").is_err()); + assert!(perms.check_net("https://github.com/denoland/deno").is_err()); + assert!(perms.check_net("github.com:2000").is_err()); + assert!( + perms + .check_net("https://github.com:2000/denoland/deno") + .is_err() + ); + assert!(perms.check_net("github.net:3000").is_err()); + assert!( + perms + .check_net("https://github.net:3000/denoland/deno") + .is_err() + ); + + // Correct ipv4 address + any port should be ok others should err + assert!(perms.check_net("127.0.0.1").is_ok()); + assert!(perms.check_net("tcp://127.0.0.1").is_ok()); + assert!(perms.check_net("https://127.0.0.1").is_ok()); + assert!(perms.check_net("127.0.0.1:3000").is_ok()); + assert!(perms.check_net("tcp://127.0.0.1:3000").is_ok()); + assert!(perms.check_net("https://127.0.0.1:3000").is_ok()); + assert!(perms.check_net("127.0.0.2").is_err()); + assert!(perms.check_net("tcp://127.0.0.2").is_err()); + assert!(perms.check_net("https://127.0.0.2").is_err()); + assert!(perms.check_net("127.0.0.2:3000").is_err()); + assert!(perms.check_net("tcp://127.0.0.2:3000").is_err()); + assert!(perms.check_net("https://127.0.0.2:3000").is_err()); + + // Correct address + port should be ok all other combinations should err + assert!(perms.check_net("172.16.0.2:8000").is_ok()); + assert!(perms.check_net("tcp://172.16.0.2:8000").is_ok()); + assert!(perms.check_net("https://172.16.0.2:8000").is_ok()); + assert!(perms.check_net("172.16.0.2").is_err()); + assert!(perms.check_net("tcp://172.16.0.2").is_err()); + assert!(perms.check_net("https://172.16.0.2").is_err()); + assert!(perms.check_net("172.16.0.2:6000").is_err()); + assert!(perms.check_net("tcp://172.16.0.2:6000").is_err()); + assert!(perms.check_net("https://172.16.0.2:6000").is_err()); + assert!(perms.check_net("172.16.0.1:8000").is_err()); + assert!(perms.check_net("tcp://172.16.0.1:8000").is_err()); + assert!(perms.check_net("https://172.16.0.1:8000").is_err()); + + // Just some random hosts that should err + assert!(perms.check_net("somedomain").is_err()); + assert!(perms.check_net("192.168.0.1").is_err()); } } diff --git a/tools/complex_permissions_test.py b/tools/complex_permissions_test.py new file mode 100755 index 0000000000000..98eeac013d2a1 --- /dev/null +++ b/tools/complex_permissions_test.py @@ -0,0 +1,195 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +import os +import pty +import select +import subprocess +import sys +import time + +from util import build_path, root_path, executable_suffix, green_ok, red_failed + +PERMISSIONS_PROMPT_TEST_TS = "tools/complex_permissions_test.ts" + +PROMPT_PATTERN = b'⚠️' +PERMISSION_DENIED_PATTERN = b'PermissionDenied: permission denied' + + +# This function is copied from: +# https://gist.github.com/hayd/4f46a68fc697ba8888a7b517a414583e +# https://stackoverflow.com/q/52954248/1240268 +def tty_capture(cmd, bytes_input, timeout=5): + """Capture the output of cmd with bytes_input to stdin, + with stdin, stdout and stderr as TTYs.""" + mo, so = pty.openpty() # provide tty to enable line-buffering + me, se = pty.openpty() + mi, si = pty.openpty() + fdmap = {mo: 'stdout', me: 'stderr', mi: 'stdin'} + + timeout_exact = time.time() + timeout + p = subprocess.Popen( + cmd, bufsize=1, stdin=si, stdout=so, stderr=se, close_fds=True) + os.write(mi, bytes_input) + + select_timeout = .04 #seconds + res = {'stdout': b'', 'stderr': b''} + while True: + ready, _, _ = select.select([mo, me], [], [], select_timeout) + if ready: + for fd in ready: + data = os.read(fd, 512) + if not data: + break + res[fdmap[fd]] += data + elif p.poll() is not None or time.time( + ) > timeout_exact: # select timed-out + break # p exited + for fd in [si, so, se, mi, mo, me]: + os.close(fd) # can't do it sooner: it leads to errno.EIO error + p.wait() + return p.returncode, res['stdout'], res['stderr'] + + +# Wraps a test in debug printouts +# so we have visual indicator of what test failed +def wrap_test(test_name, test_method, *argv): + sys.stdout.write(test_name + " ... ") + try: + test_method(*argv) + print green_ok() + except AssertionError: + print red_failed() + raise + + +class Prompt(object): + def __init__(self, deno_exe, test_types): + self.deno_exe = deno_exe + self.test_types = test_types + + def run(self, flags, args, bytes_input): + "Returns (return_code, stdout, stderr)." + cmd = [self.deno_exe, "run"] + flags + [PERMISSIONS_PROMPT_TEST_TS + ] + args + print " ".join(cmd) + return tty_capture(cmd, bytes_input) + + def warm_up(self): + # ignore the ts compiling message + self.run(["--allow-read"], ["read", "package.json"], b'') + + def test(self): + for test_type in ["read", "write"]: + test_name_base = "test_" + test_type + wrap_test(test_name_base + "_inside_project_dir", + self.test_inside_project_dir, test_type) + wrap_test(test_name_base + "_outside_tests_dir", + self.test_outside_test_dir, test_type) + wrap_test(test_name_base + "_inside_tests_dir", + self.test_inside_test_dir, test_type) + wrap_test(test_name_base + "_outside_tests_and_js_dir", + self.test_outside_test_and_js_dir, test_type) + wrap_test(test_name_base + "_inside_tests_and_js_dir", + self.test_inside_test_and_js_dir, test_type) + wrap_test(test_name_base + "_allow_localhost_4545", + self.test_allow_localhost_4545) + wrap_test(test_name_base + "_allow_deno_land", + self.test_allow_deno_land) + wrap_test(test_name_base + "_allow_localhost_4545_fail", + self.test_allow_localhost_4545_fail) + wrap_test(test_name_base + "_allow_localhost", + self.test_allow_localhost) + + def test_inside_project_dir(self, test_type): + code, _stdout, stderr = self.run( + ["--no-prompt", "--allow-" + test_type + "=" + root_path], + [test_type, "package.json", "tests/subdir/config.json"], b'') + assert code == 0 + assert not PROMPT_PATTERN in stderr + assert not PERMISSION_DENIED_PATTERN in stderr + + def test_outside_test_dir(self, test_type): + code, _stdout, stderr = self.run([ + "--no-prompt", + "--allow-" + test_type + "=" + os.path.join(root_path, "tests") + ], [test_type, "package.json"], b'') + assert code == 1 + assert not PROMPT_PATTERN in stderr + assert PERMISSION_DENIED_PATTERN in stderr + + def test_inside_test_dir(self, test_type): + code, _stdout, stderr = self.run([ + "--no-prompt", + "--allow-" + test_type + "=" + os.path.join(root_path, "tests") + ], [test_type, "tests/subdir/config.json"], b'') + assert code == 0 + assert not PROMPT_PATTERN in stderr + assert not PERMISSION_DENIED_PATTERN in stderr + + def test_outside_test_and_js_dir(self, test_type): + code, _stdout, stderr = self.run([ + "--no-prompt", "--allow-" + test_type + "=" + os.path.join( + root_path, "tests") + "," + os.path.join(root_path, "js") + ], [test_type, "package.json"], b'') + assert code == 1 + assert not PROMPT_PATTERN in stderr + assert PERMISSION_DENIED_PATTERN in stderr + + def test_inside_test_and_js_dir(self, test_type): + code, _stdout, stderr = self.run([ + "--no-prompt", "--allow-" + test_type + "=" + os.path.join( + root_path, "tests") + "," + os.path.join(root_path, "js") + ], [test_type, "js/dir_test.ts", "tests/subdir/config.json"], b'') + assert code == 0 + assert not PROMPT_PATTERN in stderr + assert not PERMISSION_DENIED_PATTERN in stderr + + def test_allow_localhost_4545(self): + code, _stdout, stderr = self.run( + ["--no-prompt", "--allow-net=localhost:4545"], + ["net", "http://localhost:4545"], b'') + assert code == 0 + assert not PROMPT_PATTERN in stderr + assert not PERMISSION_DENIED_PATTERN in stderr + + def test_allow_deno_land(self): + code, _stdout, stderr = self.run( + ["--no-prompt", "--allow-net=deno.land"], + ["net", "http://localhost:4545"], b'') + assert code == 1 + assert not PROMPT_PATTERN in stderr + assert PERMISSION_DENIED_PATTERN in stderr + + def test_allow_localhost_4545_fail(self): + code, _stdout, stderr = self.run( + ["--no-prompt", "--allow-net=localhost:4545"], + ["net", "http://localhost:4546"], b'') + assert code == 1 + assert not PROMPT_PATTERN in stderr + assert PERMISSION_DENIED_PATTERN in stderr + + def test_allow_localhost(self): + code, _stdout, stderr = self.run( + ["--no-prompt", "--allow-net=localhost"], [ + "net", "http://localhost:4545", "http://localhost:4546", + "http://localhost:4547" + ], b'') + assert code == 0 + assert not PROMPT_PATTERN in stderr + assert not PERMISSION_DENIED_PATTERN in stderr + + +def complex_permissions_test(deno_exe): + p = Prompt(deno_exe, ["read", "write", "net"]) + p.test() + + +def main(): + print "Permissions prompt tests" + deno_exe = os.path.join(build_path(), "deno" + executable_suffix) + complex_permissions_test(deno_exe) + + +if __name__ == "__main__": + main() diff --git a/tools/complex_permissions_test.ts b/tools/complex_permissions_test.ts new file mode 100644 index 0000000000000..72377ff93a5e3 --- /dev/null +++ b/tools/complex_permissions_test.ts @@ -0,0 +1,24 @@ +// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license. +const { args, readFileSync, writeFileSync, exit, dial } = Deno; + +const name = args[1]; +const test: (args: string[]) => void = { + read: (files: string[]): void => { + files.forEach((file): any => readFileSync(file)); + }, + write: (files: string[]): void => { + files.forEach( + (file): any => writeFileSync(file, new Uint8Array(), { append: true }) + ); + }, + net: (hosts: string[]): void => { + hosts.forEach((host): any => fetch(host)); + } +}[name]; + +if (!test) { + console.log("Unknown test:", name); + exit(1); +} + +test(args.slice(2)); diff --git a/tools/test.py b/tools/test.py index 2a59a0c87923b..0b913ea5b6120 100755 --- a/tools/test.py +++ b/tools/test.py @@ -105,7 +105,9 @@ def main(argv): if os.name != 'nt': from is_tty_test import is_tty_test from permission_prompt_test import permission_prompt_test + from complex_permissions_test import complex_permissions_test permission_prompt_test(deno_exe) + complex_permissions_test(deno_exe) is_tty_test(deno_exe) repl_tests(deno_exe) From 02dd3be90bfd69cddcec0b2554221f1d4c0a76d8 Mon Sep 17 00:00:00 2001 From: afinch7 Date: Wed, 8 May 2019 13:43:27 -0400 Subject: [PATCH 15/17] fixed clippy warnings --- cli/ops.rs | 2 +- cli/permissions.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cli/ops.rs b/cli/ops.rs index 4a825c7b7a91f..9354fb092cc1d 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -242,7 +242,7 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { fn resolve_path(path: &str) -> Result { let url = deno_dir::resolve_file_url(path.to_string(), ".".to_string()) - .map_err(|err| DenoError::from(err))?; + .map_err(DenoError::from)?; Ok(url.to_file_path().unwrap()) } diff --git a/cli/permissions.rs b/cli/permissions.rs index 5a0d0e69d64d1..549ac0e876044 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -259,10 +259,10 @@ impl DenoPermissions { // ":" and checking host and host + port combinations. if match url_check_result { None => { - let parts = url_or_host_and_port.split(":").collect::>(); + let parts = url_or_host_and_port.split(':').collect::>(); match parts.len() { 2 => { - if self.net_whitelist.contains(&format!("{}", parts[0])) { + if self.net_whitelist.contains(parts[0]) { true } else { self @@ -270,7 +270,7 @@ impl DenoPermissions { .contains(&format!("{}:{}", parts[0], parts[1])) } } - 1 => self.net_whitelist.contains(&format!("{}", parts[0])), + 1 => self.net_whitelist.contains(parts[0]), _ => panic!( "Failed to parse origin string: {}", url_or_host_and_port @@ -456,6 +456,7 @@ fn check_path_white_list( #[cfg(test)] mod tests { + #![allow(clippy::cyclomatic_complexity)] use super::*; // Creates vector of strings, Vec From d4180971c33e65166bdd5976c73be1bf2ce503bb Mon Sep 17 00:00:00 2001 From: afinch7 Date: Wed, 8 May 2019 14:27:51 -0400 Subject: [PATCH 16/17] simplified path resolution code for ops a little more --- cli/ops.rs | 65 +++++++++++++++++++++++------------------------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/cli/ops.rs b/cli/ops.rs index 9354fb092cc1d..29c3ff1e9dd36 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -240,10 +240,12 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option { } } -fn resolve_path(path: &str) -> Result { +fn resolve_path(path: &str) -> Result<(PathBuf, String), DenoError> { let url = deno_dir::resolve_file_url(path.to_string(), ".".to_string()) .map_err(DenoError::from)?; - Ok(url.to_file_path().unwrap()) + let path = url.to_file_path().unwrap(); + let path_string = path.to_str().unwrap().to_string(); + Ok((path, path_string)) } // Returns a milliseconds and nanoseconds subsec @@ -821,11 +823,10 @@ fn op_mkdir( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_mkdir().unwrap(); - let path = match resolve_path(inner.path().unwrap()) { + let (path, path_) = match resolve_path(inner.path().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let path_ = path.to_str().unwrap().to_string(); let recursive = inner.recursive(); let mode = inner.mode(); @@ -848,11 +849,10 @@ fn op_chmod( assert!(data.is_none()); let inner = base.inner_as_chmod().unwrap(); let _mode = inner.mode(); - let path = match resolve_path(inner.path().unwrap()) { + let (path, path_) = match resolve_path(inner.path().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let path_ = path.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&path_) { return odd_future(e); @@ -890,11 +890,10 @@ fn op_open( assert!(data.is_none()); let cmd_id = base.cmd_id(); let inner = base.inner_as_open().unwrap(); - let filename = match resolve_path(inner.filename().unwrap()) { + let (filename, filename_) = match resolve_path(inner.filename().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let filename_ = filename.to_str().unwrap(); let mode = inner.mode().unwrap(); let mut open_options = tokio::fs::OpenOptions::new(); @@ -935,20 +934,20 @@ fn op_open( match mode { "r" => { - if let Err(e) = state.check_read(filename_) { + if let Err(e) = state.check_read(&filename_) { return odd_future(e); } } "w" | "a" | "x" => { - if let Err(e) = state.check_write(filename_) { + if let Err(e) = state.check_write(&filename_) { return odd_future(e); } } &_ => { - if let Err(e) = state.check_read(filename_) { + if let Err(e) = state.check_read(&filename_) { return odd_future(e); } - if let Err(e) = state.check_write(filename_) { + if let Err(e) = state.check_write(&filename_) { return odd_future(e); } } @@ -1137,14 +1136,13 @@ fn op_remove( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_remove().unwrap(); - let path = match resolve_path(inner.path().unwrap()) { + let (path, path_) = match resolve_path(inner.path().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let path_ = path.to_str().unwrap(); let recursive = inner.recursive(); - if let Err(e) = state.check_write(path_) { + if let Err(e) = state.check_write(&path_) { return odd_future(e); } @@ -1169,18 +1167,16 @@ fn op_copy_file( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_copy_file().unwrap(); - let from = match resolve_path(inner.from().unwrap()) { + let (from, from_) = match resolve_path(inner.from().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let from_ = from.to_str().unwrap(); - let to = match resolve_path(inner.to().unwrap()) { + let (to, to_) = match resolve_path(inner.to().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let to_ = to.to_str().unwrap().to_string(); - if let Err(e) = state.check_read(from_) { + if let Err(e) = state.check_read(&from_) { return odd_future(e); } if let Err(e) = state.check_write(&to_) { @@ -1258,11 +1254,10 @@ fn op_stat( assert!(data.is_none()); let inner = base.inner_as_stat().unwrap(); let cmd_id = base.cmd_id(); - let filename = match resolve_path(inner.filename().unwrap()) { + let (filename, filename_) = match resolve_path(inner.filename().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let filename_ = filename.to_str().unwrap().to_string(); let lstat = inner.lstat(); if let Err(e) = state.check_read(&filename_) { @@ -1316,13 +1311,12 @@ fn op_read_dir( assert!(data.is_none()); let inner = base.inner_as_read_dir().unwrap(); let cmd_id = base.cmd_id(); - let path = match resolve_path(inner.path().unwrap()) { + let (path, path_) = match resolve_path(inner.path().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let path_ = path.to_str().unwrap(); - if let Err(e) = state.check_read(path_) { + if let Err(e) = state.check_read(&path_) { return odd_future(e); } @@ -1380,15 +1374,14 @@ fn op_rename( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_rename().unwrap(); - let oldpath = match resolve_path(inner.oldpath().unwrap()) { + let (oldpath, _) = match resolve_path(inner.oldpath().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let newpath = match resolve_path(inner.newpath().unwrap()) { + let (newpath, newpath_) = match resolve_path(inner.newpath().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let newpath_ = newpath.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&newpath_) { return odd_future(e); @@ -1407,15 +1400,14 @@ fn op_link( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_link().unwrap(); - let oldname = match resolve_path(inner.oldname().unwrap()) { + let (oldname, _) = match resolve_path(inner.oldname().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let newname = match resolve_path(inner.newname().unwrap()) { + let (newname, newname_) = match resolve_path(inner.newname().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let newname_ = newname.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&newname_) { return odd_future(e); @@ -1435,15 +1427,14 @@ fn op_symlink( ) -> Box { assert!(data.is_none()); let inner = base.inner_as_symlink().unwrap(); - let oldname = match resolve_path(inner.oldname().unwrap()) { + let (oldname, _) = match resolve_path(inner.oldname().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let newname = match resolve_path(inner.newname().unwrap()) { + let (newname, newname_) = match resolve_path(inner.newname().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let newname_ = newname.to_str().unwrap().to_string(); if let Err(e) = state.check_write(&newname_) { return odd_future(e); @@ -1471,11 +1462,10 @@ fn op_read_link( assert!(data.is_none()); let inner = base.inner_as_readlink().unwrap(); let cmd_id = base.cmd_id(); - let name = match resolve_path(inner.name().unwrap()) { + let (name, name_) = match resolve_path(inner.name().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let name_ = name.to_str().unwrap().to_string(); if let Err(e) = state.check_read(&name_) { return odd_future(e); @@ -1579,11 +1569,10 @@ fn op_truncate( assert!(data.is_none()); let inner = base.inner_as_truncate().unwrap(); - let filename = match resolve_path(inner.name().unwrap()) { + let (filename, filename_) = match resolve_path(inner.name().unwrap()) { Err(err) => return odd_future(err), Ok(v) => v, }; - let filename_ = filename.to_str().unwrap().to_string(); let len = inner.len(); if let Err(e) = state.check_write(&filename_) { From 49cc718e3645b7863526b68fdbbc8ef42428b65e Mon Sep 17 00:00:00 2001 From: afinch7 Date: Wed, 8 May 2019 17:00:56 -0400 Subject: [PATCH 17/17] add method check_net_url --- cli/ops.rs | 6 +- cli/permissions.rs | 292 ++++++++++++++++++++++++++++++--------------- cli/state.rs | 9 +- 3 files changed, 210 insertions(+), 97 deletions(-) diff --git a/cli/ops.rs b/cli/ops.rs index 29c3ff1e9dd36..e39d4cfea2ab3 100644 --- a/cli/ops.rs +++ b/cli/ops.rs @@ -704,7 +704,11 @@ fn op_fetch( } let req = maybe_req.unwrap(); - if let Err(e) = state.check_net(url) { + let url_ = match url::Url::parse(url) { + Err(err) => return odd_future(DenoError::from(err)), + Ok(v) => v, + }; + if let Err(e) = state.check_net_url(url_) { return odd_future(e); } diff --git a/cli/permissions.rs b/cli/permissions.rs index 549ac0e876044..304b6edfef55e 100644 --- a/cli/permissions.rs +++ b/cli/permissions.rs @@ -229,77 +229,79 @@ impl DenoPermissions { } } - pub fn check_net(&self, url_or_host_and_port: &str) -> DenoResult<()> { + pub fn check_net(&self, host_and_port: &str) -> DenoResult<()> { match self.allow_net.get_state() { PermissionAccessorState::Allow => Ok(()), state => { - // Try to parse as url first and do checks. - // Return None if unable parse the url or the parse result doesn't have - // a host(for strings like "localhost:1234" host is parsed as scheme). - let url_check_result = match url::Url::parse(url_or_host_and_port) { - Ok(url) => match url.host() { - Some(host) => { - if self.net_whitelist.contains(&format!("{}", host)) { - Some(true) - } else { - match url.port() { - Some(port) => Some( - self.net_whitelist.contains(&format!("{}:{}", host, port)), - ), - None => Some(false), - } - } - } - None => None, - }, - Err(_) => None, - }; - - // If the first check is a None(unable to check) then try splitting by - // ":" and checking host and host + port combinations. - if match url_check_result { - None => { - let parts = url_or_host_and_port.split(':').collect::>(); - match parts.len() { - 2 => { - if self.net_whitelist.contains(parts[0]) { - true - } else { - self - .net_whitelist - .contains(&format!("{}:{}", parts[0], parts[1])) - } - } - 1 => self.net_whitelist.contains(parts[0]), - _ => panic!( - "Failed to parse origin string: {}", - url_or_host_and_port - ), + let parts = host_and_port.split(':').collect::>(); + if match parts.len() { + 2 => { + if self.net_whitelist.contains(parts[0]) { + true + } else { + self + .net_whitelist + .contains(&format!("{}:{}", parts[0], parts[1])) } } - Some(v) => v, + 1 => self.net_whitelist.contains(parts[0]), + _ => panic!("Failed to parse origin string: {}", host_and_port), } { - return Ok(()); + Ok(()) + } else { + self.check_net_inner(state, host_and_port) } + } + } + } - match state { - PermissionAccessorState::Ask => match self.try_permissions_prompt( - &format!("network access to \"{}\"", url_or_host_and_port), - ) { - Err(e) => Err(e), - Ok(v) => { - self.allow_net.update_with_prompt_result(&v); - v.check()?; - Ok(()) + pub fn check_net_url(&self, url: url::Url) -> DenoResult<()> { + match self.allow_net.get_state() { + PermissionAccessorState::Allow => Ok(()), + state => { + let host = url.host().unwrap(); + let whitelist_result = { + if self.net_whitelist.contains(&format!("{}", host)) { + true + } else { + match url.port() { + Some(port) => { + self.net_whitelist.contains(&format!("{}:{}", host, port)) + } + None => false, } - }, - PermissionAccessorState::Deny => Err(permission_denied()), - _ => unreachable!(), + } + }; + if whitelist_result { + Ok(()) + } else { + self.check_net_inner(state, &url.to_string()) } } } } + fn check_net_inner( + &self, + state: PermissionAccessorState, + prompt_str: &str, + ) -> DenoResult<()> { + match state { + PermissionAccessorState::Ask => match self.try_permissions_prompt( + &format!("network access to \"{}\"", prompt_str), + ) { + Err(e) => Err(e), + Ok(v) => { + self.allow_net.update_with_prompt_result(&v); + v.check()?; + Ok(()) + } + }, + PermissionAccessorState::Deny => Err(permission_denied()), + _ => unreachable!(), + } + } + pub fn check_env(&self) -> DenoResult<()> { match self.allow_env.get_state() { PermissionAccessorState::Allow => Ok(()), @@ -523,89 +525,191 @@ mod tests { }); // Any protocol + port for localhost should be ok, since we don't specify - assert!(perms.check_net("http://localhost").is_ok()); - assert!(perms.check_net("http://localhost:8080").is_ok()); - assert!(perms.check_net("https://localhost").is_ok()); - assert!(perms.check_net("https://localhost:4443").is_ok()); - assert!(perms.check_net("tcp://localhost:5000").is_ok()); - assert!(perms.check_net("udp://localhost:6000").is_ok()); + assert!( + perms + .check_net_url(url::Url::parse("http://localhost").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("http://localhost:8080").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://localhost").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://localhost:4443").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("tcp://localhost:5000").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("udp://localhost:6000").unwrap()) + .is_ok() + ); assert!(perms.check_net("localhost:1234").is_ok()); // Correct domain + any port and protocol should be ok incorrect shouldn't assert!(perms.check_net("deno.land").is_ok()); assert!( perms - .check_net("https://deno.land/std/example/welcome.ts") - .is_ok() + .check_net_url( + url::Url::parse("https://deno.land/std/example/welcome.ts").unwrap() + ).is_ok() ); assert!(perms.check_net("deno.land:3000").is_ok()); assert!( perms - .check_net("https://deno.land:3000/std/example/welcome.ts") - .is_ok() + .check_net_url( + url::Url::parse("https://deno.land:3000/std/example/welcome.ts") + .unwrap() + ).is_ok() ); assert!(perms.check_net("deno.lands").is_err()); assert!( perms - .check_net("https://deno.lands/std/example/welcome.ts") - .is_err() + .check_net_url( + url::Url::parse("https://deno.lands/std/example/welcome.ts").unwrap() + ).is_err() ); assert!(perms.check_net("deno.lands:3000").is_err()); assert!( perms - .check_net("https://deno.lands:3000/std/example/welcome.ts") - .is_err() + .check_net_url( + url::Url::parse("https://deno.lands:3000/std/example/welcome.ts") + .unwrap() + ).is_err() ); // Correct domain + port should be ok all other combinations should err assert!(perms.check_net("github.com:3000").is_ok()); assert!( perms - .check_net("https://github.com:3000/denoland/deno") - .is_ok() + .check_net_url( + url::Url::parse("https://github.com:3000/denoland/deno").unwrap() + ).is_ok() ); assert!(perms.check_net("github.com").is_err()); - assert!(perms.check_net("https://github.com/denoland/deno").is_err()); + assert!( + perms + .check_net_url( + url::Url::parse("https://github.com/denoland/deno").unwrap() + ).is_err() + ); assert!(perms.check_net("github.com:2000").is_err()); assert!( perms - .check_net("https://github.com:2000/denoland/deno") - .is_err() + .check_net_url( + url::Url::parse("https://github.com:2000/denoland/deno").unwrap() + ).is_err() ); assert!(perms.check_net("github.net:3000").is_err()); assert!( perms - .check_net("https://github.net:3000/denoland/deno") - .is_err() + .check_net_url( + url::Url::parse("https://github.net:3000/denoland/deno").unwrap() + ).is_err() ); // Correct ipv4 address + any port should be ok others should err assert!(perms.check_net("127.0.0.1").is_ok()); - assert!(perms.check_net("tcp://127.0.0.1").is_ok()); - assert!(perms.check_net("https://127.0.0.1").is_ok()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://127.0.0.1").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://127.0.0.1").unwrap()) + .is_ok() + ); assert!(perms.check_net("127.0.0.1:3000").is_ok()); - assert!(perms.check_net("tcp://127.0.0.1:3000").is_ok()); - assert!(perms.check_net("https://127.0.0.1:3000").is_ok()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://127.0.0.1:3000").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://127.0.0.1:3000").unwrap()) + .is_ok() + ); assert!(perms.check_net("127.0.0.2").is_err()); - assert!(perms.check_net("tcp://127.0.0.2").is_err()); - assert!(perms.check_net("https://127.0.0.2").is_err()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://127.0.0.2").unwrap()) + .is_err() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://127.0.0.2").unwrap()) + .is_err() + ); assert!(perms.check_net("127.0.0.2:3000").is_err()); - assert!(perms.check_net("tcp://127.0.0.2:3000").is_err()); - assert!(perms.check_net("https://127.0.0.2:3000").is_err()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://127.0.0.2:3000").unwrap()) + .is_err() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://127.0.0.2:3000").unwrap()) + .is_err() + ); // Correct address + port should be ok all other combinations should err assert!(perms.check_net("172.16.0.2:8000").is_ok()); - assert!(perms.check_net("tcp://172.16.0.2:8000").is_ok()); - assert!(perms.check_net("https://172.16.0.2:8000").is_ok()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://172.16.0.2:8000").unwrap()) + .is_ok() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://172.16.0.2:8000").unwrap()) + .is_ok() + ); assert!(perms.check_net("172.16.0.2").is_err()); - assert!(perms.check_net("tcp://172.16.0.2").is_err()); - assert!(perms.check_net("https://172.16.0.2").is_err()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://172.16.0.2").unwrap()) + .is_err() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://172.16.0.2").unwrap()) + .is_err() + ); assert!(perms.check_net("172.16.0.2:6000").is_err()); - assert!(perms.check_net("tcp://172.16.0.2:6000").is_err()); - assert!(perms.check_net("https://172.16.0.2:6000").is_err()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://172.16.0.2:6000").unwrap()) + .is_err() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://172.16.0.2:6000").unwrap()) + .is_err() + ); assert!(perms.check_net("172.16.0.1:8000").is_err()); - assert!(perms.check_net("tcp://172.16.0.1:8000").is_err()); - assert!(perms.check_net("https://172.16.0.1:8000").is_err()); + assert!( + perms + .check_net_url(url::Url::parse("tcp://172.16.0.1:8000").unwrap()) + .is_err() + ); + assert!( + perms + .check_net_url(url::Url::parse("https://172.16.0.1:8000").unwrap()) + .is_err() + ); // Just some random hosts that should err assert!(perms.check_net("somedomain").is_err()); diff --git a/cli/state.rs b/cli/state.rs index 8a4f4eaee0ca3..f27aa95a4de5f 100644 --- a/cli/state.rs +++ b/cli/state.rs @@ -189,8 +189,13 @@ impl ThreadSafeState { } #[inline] - pub fn check_net(&self, filename: &str) -> DenoResult<()> { - self.permissions.check_net(filename) + pub fn check_net(&self, host_and_port: &str) -> DenoResult<()> { + self.permissions.check_net(host_and_port) + } + + #[inline] + pub fn check_net_url(&self, url: url::Url) -> DenoResult<()> { + self.permissions.check_net_url(url) } #[inline]