Skip to content

Commit

Permalink
feat(permissions): allow run permission to take values (denoland#9833)
Browse files Browse the repository at this point in the history
This commit adds allowlist support to `--allow-run` flag.

Additionally `Deno.permissions.query()` allows to query for specific
programs within allowlist.
  • Loading branch information
crowlKats committed Apr 9, 2021
1 parent 1c7217e commit e7b7129
Show file tree
Hide file tree
Showing 11 changed files with 271 additions and 51 deletions.
5 changes: 3 additions & 2 deletions cli/dts/lib.deno.ns.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1967,10 +1967,10 @@ declare namespace Deno {
*
* If `stdout` and/or `stderr` were set to `"piped"`, they must be closed
* manually before the process can exit.
*
*
* To run process to completion and collect output from both `stdout` and
* `stderr` use:
*
*
* ```ts
* const p = Deno.run({ cmd, stderr: 'piped', stdout: 'piped' });
* const [status, stdout, stderr] = await Promise.all([
Expand Down Expand Up @@ -2135,6 +2135,7 @@ declare namespace Deno {

export interface RunPermissionDescriptor {
name: "run";
command?: string;
}

export interface ReadPermissionDescriptor {
Expand Down
48 changes: 31 additions & 17 deletions cli/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ pub struct Flags {
pub allow_net: Option<Vec<String>>,
pub allow_plugin: bool,
pub allow_read: Option<Vec<PathBuf>>,
pub allow_run: bool,
pub allow_run: Option<Vec<String>>,
pub allow_write: Option<Vec<PathBuf>>,
pub location: Option<Url>,
pub cache_blocklist: Vec<String>,
Expand Down Expand Up @@ -211,8 +211,15 @@ impl Flags {
args.push("--allow-env".to_string());
}

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

if self.allow_plugin {
Expand Down Expand Up @@ -520,7 +527,7 @@ fn repl_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
flags.subcommand = DenoSubcommand::Repl;
flags.allow_net = Some(vec![]);
flags.allow_env = true;
flags.allow_run = true;
flags.allow_run = Some(vec![]);
flags.allow_read = Some(vec![]);
flags.allow_write = Some(vec![]);
flags.allow_plugin = true;
Expand All @@ -531,7 +538,7 @@ 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_run = true;
flags.allow_run = Some(vec![]);
flags.allow_read = Some(vec![]);
flags.allow_write = Some(vec![]);
flags.allow_plugin = true;
Expand Down Expand Up @@ -1399,6 +1406,10 @@ fn permission_args<'a, 'b>(app: App<'a, 'b>) -> App<'a, 'b> {
.arg(
Arg::with_name("allow-run")
.long("allow-run")
.min_values(0)
.takes_value(true)
.use_delimiter(true)
.require_equals(true)
.help("Allow running subprocesses"),
)
.arg(
Expand Down Expand Up @@ -1809,12 +1820,15 @@ fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
debug!("net allowlist: {:#?}", &flags.allow_net);
}

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-run") {
flags.allow_run = true;
}
if matches.is_present("allow-plugin") {
flags.allow_plugin = true;
}
Expand All @@ -1825,7 +1839,7 @@ fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
flags.allow_read = Some(vec![]);
flags.allow_env = true;
flags.allow_net = Some(vec![]);
flags.allow_run = true;
flags.allow_run = Some(vec![]);
flags.allow_write = Some(vec![]);
flags.allow_plugin = true;
flags.allow_hrtime = true;
Expand Down Expand Up @@ -2032,7 +2046,7 @@ mod tests {
},
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand Down Expand Up @@ -2404,7 +2418,7 @@ mod tests {
},
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand All @@ -2427,7 +2441,7 @@ mod tests {
},
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand All @@ -2451,7 +2465,7 @@ mod tests {
},
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand Down Expand Up @@ -2488,7 +2502,7 @@ mod tests {
inspect: Some("127.0.0.1:9229".parse().unwrap()),
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand Down Expand Up @@ -2518,7 +2532,7 @@ mod tests {
argv: svec!["arg1", "arg2"],
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand All @@ -2538,7 +2552,7 @@ mod tests {
subcommand: DenoSubcommand::Repl,
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand Down Expand Up @@ -2572,7 +2586,7 @@ mod tests {
inspect: Some("127.0.0.1:9229".parse().unwrap()),
allow_net: Some(vec![]),
allow_env: true,
allow_run: true,
allow_run: Some(vec![]),
allow_read: Some(vec![]),
allow_write: Some(vec![]),
allow_plugin: true,
Expand Down
13 changes: 13 additions & 0 deletions cli/tests/089_run_allow_list.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
try {
Deno.run({
cmd: ["ls"],
});
} catch (e) {
console.log(e);
}

const proc = Deno.run({
cmd: ["cat", "089_run_allow_list.ts"],
stdout: "null",
});
console.log((await proc.status()).success);
3 changes: 3 additions & 0 deletions cli/tests/089_run_allow_list.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[WILDCARD]PermissionDenied: Requires run access to "ls", run again with the --allow-run flag
[WILDCARD]
true
9 changes: 9 additions & 0 deletions cli/tests/090_run_permissions_request.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const status1 =
(await Deno.permissions.request({ name: "run", command: "ls" })).state;
const status2 =
(await Deno.permissions.query({ name: "run", command: "cat" })).state;
const status3 =
(await Deno.permissions.request({ name: "run", command: "cat" })).state;
console.log(status1);
console.log(status2);
console.log(status3);
3 changes: 3 additions & 0 deletions cli/tests/090_run_permissions_request.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[WILDCARD]granted
prompt
denied
15 changes: 15 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2846,6 +2846,21 @@ console.log("finish");
output: "088_dynamic_import_already_evaluating.ts.out",
});

itest!(_089_run_allow_list {
args: "run --allow-run=cat 089_run_allow_list.ts",
output: "089_run_allow_list.ts.out",
});

#[cfg(unix)]
#[test]
fn _090_run_permissions_request() {
let args = "run 090_run_permissions_request.ts";
let output = "090_run_permissions_request.ts.out";
let input = b"g\nd\n";

util::test_pty(args, output, input);
}

itest!(js_import_detect {
args: "run --quiet --reload js_import_detect.ts",
output: "js_import_detect.ts.out",
Expand Down
7 changes: 4 additions & 3 deletions runtime/ops/permissions.rs
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>,
command: Option<String>,
}

pub fn op_query_permission(
Expand All @@ -41,7 +42,7 @@ pub fn op_query_permission(
.as_ref(),
),
"env" => permissions.env.query(),
"run" => permissions.run.query(),
"run" => permissions.run.query(args.command.as_deref()),
"plugin" => permissions.plugin.query(),
"hrtime" => permissions.hrtime.query(),
n => {
Expand Down Expand Up @@ -72,7 +73,7 @@ pub fn op_revoke_permission(
.as_ref(),
),
"env" => permissions.env.revoke(),
"run" => permissions.run.revoke(),
"run" => permissions.run.revoke(args.command.as_deref()),
"plugin" => permissions.plugin.revoke(),
"hrtime" => permissions.hrtime.revoke(),
n => {
Expand Down Expand Up @@ -103,7 +104,7 @@ pub fn op_request_permission(
.as_ref(),
),
"env" => permissions.env.request(),
"run" => permissions.run.request(),
"run" => permissions.run.request(args.command.as_deref()),
"plugin" => permissions.plugin.request(),
"hrtime" => permissions.hrtime.request(),
n => {
Expand Down
10 changes: 2 additions & 8 deletions runtime/ops/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,8 @@ fn op_run(
run_args: RunArgs,
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<RunInfo, AnyError> {
state.borrow::<Permissions>().run.check()?;

let args = run_args.cmd;
state.borrow::<Permissions>().run.check(&args[0])?;
let env = run_args.env;
let cwd = run_args.cwd;

Expand Down Expand Up @@ -198,11 +197,6 @@ async fn op_run_status(
rid: ResourceId,
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<RunStatus, AnyError> {
{
let s = state.borrow();
s.borrow::<Permissions>().run.check()?;
}

let resource = state
.borrow_mut()
.resource_table
Expand Down Expand Up @@ -292,7 +286,7 @@ fn op_kill(
_zero_copy: Option<ZeroCopyBuf>,
) -> Result<(), AnyError> {
super::check_unstable(state, "Deno.kill");
state.borrow::<Permissions>().run.check()?;
state.borrow::<Permissions>().run.check_all()?;

kill(args.pid, args.signo)?;
Ok(())
Expand Down
43 changes: 40 additions & 3 deletions runtime/ops/worker_host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::permissions::NetDescriptor;
use crate::permissions::PermissionState;
use crate::permissions::Permissions;
use crate::permissions::ReadDescriptor;
use crate::permissions::RunDescriptor;
use crate::permissions::UnaryPermission;
use crate::permissions::UnitPermission;
use crate::permissions::WriteDescriptor;
Expand Down Expand Up @@ -189,6 +190,26 @@ fn merge_write_permission(
Ok(main)
}

fn merge_run_permission(
mut main: UnaryPermission<RunDescriptor>,
worker: Option<UnaryPermission<RunDescriptor>>,
) -> Result<UnaryPermission<RunDescriptor>, AnyError> {
if let Some(worker) = worker {
if (worker.global_state < main.global_state)
|| !worker.granted_list.iter().all(|x| main.check(&x.0).is_ok())
{
return Err(custom_error(
"PermissionDenied",
"Can't escalate parent thread permissions",
));
} else {
main.global_state = worker.global_state;
main.granted_list = worker.granted_list;
}
}
Ok(main)
}

fn create_worker_permissions(
main_perms: Permissions,
worker_perms: PermissionsArg,
Expand All @@ -199,7 +220,7 @@ fn create_worker_permissions(
net: merge_net_permission(main_perms.net, worker_perms.net)?,
plugin: merge_boolean_permission(main_perms.plugin, worker_perms.plugin)?,
read: merge_read_permission(main_perms.read, worker_perms.read)?,
run: merge_boolean_permission(main_perms.run, worker_perms.run)?,
run: merge_run_permission(main_perms.run, worker_perms.run)?,
write: merge_write_permission(main_perms.write, worker_perms.write)?,
})
}
Expand All @@ -216,8 +237,8 @@ struct PermissionsArg {
plugin: Option<PermissionState>,
#[serde(default, deserialize_with = "as_unary_read_permission")]
read: Option<UnaryPermission<ReadDescriptor>>,
#[serde(default, deserialize_with = "as_permission_state")]
run: Option<PermissionState>,
#[serde(default, deserialize_with = "as_unary_run_permission")]
run: Option<UnaryPermission<RunDescriptor>>,
#[serde(default, deserialize_with = "as_unary_write_permission")]
write: Option<UnaryPermission<WriteDescriptor>>,
}
Expand Down Expand Up @@ -349,6 +370,22 @@ where
}))
}

fn as_unary_run_permission<'de, D>(
deserializer: D,
) -> Result<Option<UnaryPermission<RunDescriptor>>, D::Error>
where
D: Deserializer<'de>,
{
let value: UnaryPermissionBase =
deserializer.deserialize_any(ParseBooleanOrStringVec)?;

Ok(Some(UnaryPermission::<RunDescriptor> {
global_state: value.global_state,
granted_list: value.paths.into_iter().map(RunDescriptor).collect(),
..Default::default()
}))
}

#[derive(Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct CreateWorkerArgs {
Expand Down
Loading

0 comments on commit e7b7129

Please sign in to comment.