Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permissions whitelist #2129

Merged
merged 20 commits into from May 8, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

moved path checking logic to function and refactored the path matchin…

…g algorithim
  • Loading branch information...
afinch7 committed Apr 24, 2019
commit bf63ae648b9a9c6cc88aaf98bfd4b0ff7358052e
@@ -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<String>,
pub read_whitelist: Arc<HashSet<String>>,
pub allow_write: PermissionAccessor,
pub write_whitelist: Vec<String>,
pub write_whitelist: Arc<HashSet<String>>,
pub allow_net: PermissionAccessor,
pub net_whitelist: Arc<HashSet<String>>,
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.
This conversation was marked as resolved by afinch7

This comment has been minimized.

Copy link
@ry

ry May 7, 2019

Collaborator

I'm not sure what this comment means?

I see. If it's allowed it would have branched above. I think the comment is superfluous and unreachable!() sufficient.

This comment has been minimized.

Copy link
@afinch7

afinch7 May 8, 2019

Author Contributor

Should I remove the type as well, and just replace with a _ => unreachable!()?

This comment has been minimized.

Copy link
@ry

ry May 8, 2019

Collaborator

Or keep the type. I just think the comment is confusing.

PermissionAccessorState::Allow => Ok(()),
This conversation was marked as resolved by afinch7

This comment has been minimized.

Copy link
@bartlomieju

bartlomieju May 3, 2019

Contributor

PermissionAccessorState::Allow => unreachable!()?

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<PromptResult> {
};
}
}

fn check_path_white_list(
filename: &str,
white_list: &Arc<HashSet<String>>,
) -> 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;
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.