Skip to content

Commit

Permalink
fix: support relative path for whitelisting (denoland#2317)
Browse files Browse the repository at this point in the history
Using `std::fs::canonicalize` to expand path to full existing path, such that
later attempt to loop-pop and compare path segment would work.
  • Loading branch information
kevinkassimo authored and ry committed May 9, 2019
1 parent 2edee33 commit d9cdc67
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 15 deletions.
8 changes: 8 additions & 0 deletions cli/deno_dir.rs
Expand Up @@ -896,6 +896,14 @@ pub fn resolve_file_url(
Ok(j)
}

pub fn resolve_path(path: &str) -> Result<(PathBuf, String), DenoError> {
let url = resolve_file_url(path.to_string(), ".".to_string())
.map_err(DenoError::from)?;
let path = url.to_file_path().unwrap();
let path_string = path.to_str().unwrap().to_string();
Ok((path, path_string))
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
50 changes: 44 additions & 6 deletions cli/flags.rs
@@ -1,5 +1,6 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
use clap::{App, AppSettings, Arg, ArgMatches, SubCommand};
use crate::deno_dir;

// Creates vector of strings, Vec<String>
macro_rules! svec {
Expand Down Expand Up @@ -282,6 +283,28 @@ This command has implicit access to all permissions (equivalent to deno run --al
).arg(Arg::with_name("code").takes_value(true).required(true)),
)
}

/// Convert paths supplied into full path.
/// If a path is invalid, we print out a warning
/// and ignore this path in the output.
fn resolve_paths(paths: Vec<String>) -> Vec<String> {
let mut out: Vec<String> = vec![];
for pathstr in paths.iter() {
let result = deno_dir::resolve_path(pathstr);
if result.is_err() {
eprintln!("Unrecognized path to whitelist: {}", pathstr);
continue;
}
let mut full_path = result.unwrap().1;
// Remove trailing slash.
if full_path.len() > 1 && full_path.ends_with('/') {
full_path.pop();
}
out.push(full_path);
}
out
}

/// Parse ArgMatches into internal DenoFlags structure.
/// This method should not make any side effects.
#[cfg_attr(feature = "cargo-clippy", allow(stutter))]
Expand Down Expand Up @@ -318,17 +341,21 @@ 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 =
let raw_read_whitelist: Vec<String> =
read_wl.map(std::string::ToString::to_string).collect();
flags.read_whitelist = resolve_paths(raw_read_whitelist);
debug!("read whitelist: {:#?}", &flags.read_whitelist);
} else {
flags.allow_read = true;
}
}
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 =
let raw_write_whitelist =
write_wl.map(std::string::ToString::to_string).collect();
flags.write_whitelist = resolve_paths(raw_write_whitelist);
debug!("write whitelist: {:#?}", &flags.write_whitelist);
} else {
flags.allow_write = true;
}
Expand All @@ -338,6 +365,7 @@ pub fn parse_flags(matches: ArgMatches) -> DenoFlags {
let net_wl = run_matches.values_of("allow-net").unwrap();
flags.net_whitelist =
net_wl.map(std::string::ToString::to_string).collect();
debug!("net whitelist: {:#?}", &flags.net_whitelist);
} else {
flags.allow_net = true;
}
Expand Down Expand Up @@ -814,17 +842,22 @@ mod tests {
}
#[test]
fn test_flags_from_vec_19() {
use tempfile::TempDir;
let temp_dir = TempDir::new().expect("tempdir fail");
let (_, temp_dir_path) =
deno_dir::resolve_path(temp_dir.path().to_str().unwrap()).unwrap();

let (flags, subcommand, argv) = flags_from_vec(svec![
"deno",
"run",
"--allow-read=/some/test/dir",
format!("--allow-read={}", &temp_dir_path),
"script.ts"
]);
assert_eq!(
flags,
DenoFlags {
allow_read: false,
read_whitelist: svec!["/some/test/dir"],
read_whitelist: svec![&temp_dir_path],
..DenoFlags::default()
}
);
Expand All @@ -833,17 +866,22 @@ mod tests {
}
#[test]
fn test_flags_from_vec_20() {
use tempfile::TempDir;
let temp_dir = TempDir::new().expect("tempdir fail");
let (_, temp_dir_path) =
deno_dir::resolve_path(temp_dir.path().to_str().unwrap()).unwrap();

let (flags, subcommand, argv) = flags_from_vec(svec![
"deno",
"run",
"--allow-write=/some/test/dir",
format!("--allow-write={}", &temp_dir_path),
"script.ts"
]);
assert_eq!(
flags,
DenoFlags {
allow_write: false,
write_whitelist: svec!["/some/test/dir"],
write_whitelist: svec![&temp_dir_path],
..DenoFlags::default()
}
);
Expand Down
10 changes: 1 addition & 9 deletions cli/ops.rs
Expand Up @@ -2,7 +2,7 @@
use atty;
use crate::ansi;
use crate::compiler::get_compiler_config;
use crate::deno_dir;
use crate::deno_dir::resolve_path;
use crate::dispatch_minimal::dispatch_minimal;
use crate::dispatch_minimal::parse_min_record;
use crate::errors;
Expand Down Expand Up @@ -241,14 +241,6 @@ pub fn op_selector_std(inner_type: msg::Any) -> Option<OpCreator> {
}
}

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)?;
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
// since the start time of the deno runtime.
// If the High precision flag is not set, the
Expand Down
28 changes: 28 additions & 0 deletions tools/complex_permissions_test.py
Expand Up @@ -92,6 +92,10 @@ def test(self):
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 + "_relative", self.test_relative,
test_type)
wrap_test(test_name_base + "_no_prefix", self.test_no_prefix,
test_type)
wrap_test(test_name_base + "_allow_localhost_4545",
self.test_allow_localhost_4545)
wrap_test(test_name_base + "_allow_deno_land",
Expand Down Expand Up @@ -179,6 +183,30 @@ def test_allow_localhost(self):
assert not PROMPT_PATTERN in stderr
assert not PERMISSION_DENIED_PATTERN in stderr

def test_relative(self, test_type):
# Save and restore curdir
saved_curdir = os.getcwd()
os.chdir(root_path)
code, _stdout, stderr = self.run(
["--no-prompt", "--allow-" + test_type + "=" + "./tests"],
[test_type, "tests/subdir/config.json"], b'')
assert code == 0
assert not PROMPT_PATTERN in stderr
assert not PERMISSION_DENIED_PATTERN in stderr
os.chdir(saved_curdir)

def test_no_prefix(self, test_type):
# Save and restore curdir
saved_curdir = os.getcwd()
os.chdir(root_path)
code, _stdout, stderr = self.run(
["--no-prompt", "--allow-" + test_type + "=" + "tests"],
[test_type, "tests/subdir/config.json"], b'')
assert code == 0
assert not PROMPT_PATTERN in stderr
assert not PERMISSION_DENIED_PATTERN in stderr
os.chdir(saved_curdir)


def complex_permissions_test(deno_exe):
p = Prompt(deno_exe, ["read", "write", "net"])
Expand Down

0 comments on commit d9cdc67

Please sign in to comment.