Skip to content

Commit

Permalink
feat(permissions): allow env permission to take values (denoland#9825)
Browse files Browse the repository at this point in the history
  • Loading branch information
@crowlKats
crowlKats committed Apr 13, 2021
1 parent ec1fce5 commit 8b59d9f
Show file tree
Hide file tree
Showing 6 changed files with 350 additions and 59 deletions.
1 change: 1 addition & 0 deletions cli/dts/lib.deno.ns.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2164,6 +2164,7 @@ declare namespace Deno {

export interface EnvPermissionDescriptor {
name: "env";
variable?: string;
}

export interface PluginPermissionDescriptor {
Expand Down
115 changes: 97 additions & 18 deletions cli/flags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ pub struct Flags {
pub argv: Vec<String>,
pub subcommand: DenoSubcommand,

pub allow_env: bool,
pub allow_env: Option<Vec<String>>,
pub allow_hrtime: bool,
pub allow_net: Option<Vec<String>>,
pub allow_plugin: bool,
Expand Down Expand Up @@ -207,8 +207,15 @@ impl Flags {
_ => {}
}

if self.allow_env {
args.push("--allow-env".to_string());
match &self.allow_env {
Some(env_allowlist) if env_allowlist.is_empty() => {
args.push("--allow-env".to_string());
}
Some(env_allowlist) => {
let s = format!("--allow-env={}", env_allowlist.join(","));
args.push(s);
}
_ => {}
}

match &self.allow_run {
Expand Down Expand Up @@ -527,7 +534,7 @@ fn repl_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
flags.repl = true;
flags.subcommand = DenoSubcommand::Repl;
flags.allow_net = Some(vec![]);
flags.allow_env = true;
flags.allow_env = Some(vec![]);
flags.allow_run = Some(vec![]);
flags.allow_read = Some(vec![]);
flags.allow_write = Some(vec![]);
Expand All @@ -538,7 +545,7 @@ fn repl_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
fn eval_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
runtime_args_parse(flags, matches, false, true);
flags.allow_net = Some(vec![]);
flags.allow_env = true;
flags.allow_env = Some(vec![]);
flags.allow_run = Some(vec![]);
flags.allow_read = Some(vec![]);
flags.allow_write = Some(vec![]);
Expand Down Expand Up @@ -1402,7 +1409,19 @@ fn permission_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
.arg(
Arg::with_name("allow-env")
.long("allow-env")
.help("Allow environment access"),
.min_values(0)
.takes_value(true)
.use_delimiter(true)
.require_equals(true)
.help("Allow environment access")
.validator(|keys| {
for key in keys.split(',') {
if key.is_empty() || key.contains(&['=', '\0'] as &[char]) {
return Err(format!("invalid key \"{}\"", key));
}
}
Ok(())
}),
)
.arg(
Arg::with_name("allow-run")
Expand Down Expand Up @@ -1826,15 +1845,26 @@ fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
debug!("net allowlist: {:#?}", &flags.allow_net);
}

if let Some(env_wl) = matches.values_of("allow-env") {
let env_allowlist: Vec<String> = env_wl
.map(|env: &str| {
if cfg!(windows) {
env.to_uppercase()
} else {
env.to_string()
}
})
.collect();
flags.allow_env = Some(env_allowlist);
debug!("env allowlist: {:#?}", &flags.allow_env);
}

if let Some(run_wl) = matches.values_of("allow-run") {
let run_allowlist: Vec<String> = run_wl.map(ToString::to_string).collect();
flags.allow_run = Some(run_allowlist);
debug!("run allowlist: {:#?}", &flags.allow_run);
}

if matches.is_present("allow-env") {
flags.allow_env = true;
}
if matches.is_present("allow-plugin") {
flags.allow_plugin = true;
}
Expand All @@ -1843,7 +1873,7 @@ fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
}
if matches.is_present("allow-all") {
flags.allow_read = Some(vec![]);
flags.allow_env = true;
flags.allow_env = Some(vec![]);
flags.allow_net = Some(vec![]);
flags.allow_run = Some(vec![]);
flags.allow_write = Some(vec![]);
Expand Down Expand Up @@ -2054,7 +2084,7 @@ mod tests {
script: "gist.ts".to_string(),
},
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand Down Expand Up @@ -2426,7 +2456,7 @@ mod tests {
ext: "js".to_string(),
},
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand All @@ -2449,7 +2479,7 @@ mod tests {
ext: "js".to_string(),
},
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand All @@ -2473,7 +2503,7 @@ mod tests {
ext: "ts".to_string(),
},
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand Down Expand Up @@ -2510,7 +2540,7 @@ mod tests {
seed: Some(1),
inspect: Some("127.0.0.1:9229".parse().unwrap()),
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand Down Expand Up @@ -2540,7 +2570,7 @@ mod tests {
},
argv: svec!["arg1", "arg2"],
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand All @@ -2560,7 +2590,7 @@ mod tests {
repl: true,
subcommand: DenoSubcommand::Repl,
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand Down Expand Up @@ -2594,7 +2624,7 @@ mod tests {
seed: Some(1),
inspect: Some("127.0.0.1:9229".parse().unwrap()),
allow_net: Some(vec![]),
allow_env: true,
allow_env: Some(vec![]),
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
Expand Down Expand Up @@ -2671,6 +2701,55 @@ mod tests {
);
}

#[test]
fn allow_env_allowlist() {
let r =
flags_from_vec(svec!["deno", "run", "--allow-env=HOME", "script.ts"]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run {
script: "script.ts".to_string(),
},
allow_env: Some(svec!["HOME"]),
..Flags::default()
}
);
}

#[test]
fn allow_env_allowlist_multiple() {
let r = flags_from_vec(svec![
"deno",
"run",
"--allow-env=HOME,PATH",
"script.ts"
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run {
script: "script.ts".to_string(),
},
allow_env: Some(svec!["HOME", "PATH"]),
..Flags::default()
}
);
}

#[test]
fn allow_env_allowlist_validator() {
let r =
flags_from_vec(svec!["deno", "run", "--allow-env=HOME", "script.ts"]);
assert!(r.is_ok());
let r =
flags_from_vec(svec!["deno", "run", "--allow-env=H=ME", "script.ts"]);
assert!(r.is_err());
let r =
flags_from_vec(svec!["deno", "run", "--allow-env=H\0ME", "script.ts"]);
assert!(r.is_err());
}

#[test]
fn bundle() {
let r = flags_from_vec(svec!["deno", "bundle", "source.ts"]);
Expand Down
19 changes: 10 additions & 9 deletions runtime/ops/os.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ fn op_set_env(
args: SetEnv,
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<(), AnyError> {
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check(&args.key)?;
let invalid_key =
args.key.is_empty() || args.key.contains(&['=', '\0'] as &[char]);
let invalid_value = args.value.contains('\0');
Expand All @@ -70,7 +70,7 @@ fn op_env(
_args: (),
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<HashMap<String, String>, AnyError> {
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check_all()?;
Ok(env::vars().collect())
}

Expand All @@ -79,7 +79,7 @@ fn op_get_env(
key: String,
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<Option<String>, AnyError> {
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check(&key)?;
if key.is_empty() || key.contains(&['=', '\0'] as &[char]) {
return Err(type_error("Key contains invalid characters."));
}
Expand All @@ -89,12 +89,13 @@ fn op_get_env(
};
Ok(r)
}

fn op_delete_env(
state: &mut OpState,
key: String,
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<(), AnyError> {
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check(&key)?;
if key.is_empty() || key.contains(&['=', '\0'] as &[char]) {
return Err(type_error("Key contains invalid characters."));
}
Expand All @@ -116,7 +117,7 @@ fn op_loadavg(
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<(f64, f64, f64), AnyError> {
super::check_unstable(state, "Deno.loadavg");
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check_all()?;
match sys_info::loadavg() {
Ok(loadavg) => Ok((loadavg.one, loadavg.five, loadavg.fifteen)),
Err(_) => Ok((0.0, 0.0, 0.0)),
Expand All @@ -129,7 +130,7 @@ fn op_hostname(
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<String, AnyError> {
super::check_unstable(state, "Deno.hostname");
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check_all()?;
let hostname = sys_info::hostname().unwrap_or_else(|_| "".to_string());
Ok(hostname)
}
Expand All @@ -140,7 +141,7 @@ fn op_os_release(
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<String, AnyError> {
super::check_unstable(state, "Deno.osRelease");
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check_all()?;
let release = sys_info::os_release().unwrap_or_else(|_| "".to_string());
Ok(release)
}
Expand All @@ -164,7 +165,7 @@ fn op_system_memory_info(
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<Option<MemInfo>, AnyError> {
super::check_unstable(state, "Deno.systemMemoryInfo");
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check_all()?;
match sys_info::mem_info() {
Ok(info) => Ok(Some(MemInfo {
total: info.total,
Expand All @@ -191,7 +192,7 @@ fn op_system_cpu_info(
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<CpuInfo, AnyError> {
super::check_unstable(state, "Deno.systemCpuInfo");
state.borrow_mut::<Permissions>().env.check()?;
state.borrow_mut::<Permissions>().env.check_all()?;

let cores = sys_info::cpu_num().ok();
let speed = sys_info::cpu_speed().ok();
Expand Down
7 changes: 4 additions & 3 deletions runtime/ops/permissions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub struct PermissionArgs {
name: String,
path: Option<String>,
host: Option<String>,
variable: Option<String>,
command: Option<String>,
}

Expand All @@ -41,7 +42,7 @@ pub fn op_query_permission(
}
.as_ref(),
),
"env" => permissions.env.query(),
"env" => permissions.env.query(args.variable.as_deref()),
"run" => permissions.run.query(args.command.as_deref()),
"plugin" => permissions.plugin.query(),
"hrtime" => permissions.hrtime.query(),
Expand Down Expand Up @@ -72,7 +73,7 @@ pub fn op_revoke_permission(
}
.as_ref(),
),
"env" => permissions.env.revoke(),
"env" => permissions.env.revoke(args.variable.as_deref()),
"run" => permissions.run.revoke(args.command.as_deref()),
"plugin" => permissions.plugin.revoke(),
"hrtime" => permissions.hrtime.revoke(),
Expand Down Expand Up @@ -103,7 +104,7 @@ pub fn op_request_permission(
}
.as_ref(),
),
"env" => permissions.env.request(),
"env" => permissions.env.request(args.variable.as_deref()),
"run" => permissions.run.request(args.command.as_deref()),
"plugin" => permissions.plugin.request(),
"hrtime" => permissions.hrtime.request(),
Expand Down

0 comments on commit 8b59d9f

Please sign in to comment.