summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNayeem Rahman <nayeemrmn99@gmail.com>2023-08-30 18:52:01 +0100
committerGitHub <noreply@github.com>2023-08-30 18:52:01 +0100
commit1cce3060227f7bc53a8d5ad938f092362cf78855 (patch)
tree9abfe9f27372291b9f04e11a4e4127bb2daf39af
parentd28384c3deec1497d28f0f6bd16cf51de832e572 (diff)
fix(runtime/permissions): Resolve executable specifiers in allowlists and queries (#14130)
Closes #14122. Adds two extensions to `--allow-run` behaviour: - When `--allow-run=foo` is specified and `foo` is found in the `PATH` at startup, `RunDescriptor::Path(which("foo"))` is added to the allowlist alongside `RunDescriptor::Name("foo")`. Currently only the latter is. - When run permission for `foo` is queried and `foo` is found in the `PATH` at runtime, either `RunDescriptor::Path(which("foo"))` or `RunDescriptor::Name("foo")` would qualify in the allowlist. Currently only the latter does.
-rw-r--r--Cargo.lock1
-rw-r--r--cli/tests/integration/run_tests.rs5
-rw-r--r--cli/tests/testdata/allow_run_allowlist_resolution.ts66
-rw-r--r--cli/tests/testdata/allow_run_allowlist_resolution.ts.out15
-rw-r--r--runtime/Cargo.toml1
-rw-r--r--runtime/permissions/mod.rs140
6 files changed, 183 insertions, 45 deletions
diff --git a/Cargo.lock b/Cargo.lock
index d565cbb23..dd12b8851 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1515,6 +1515,7 @@ dependencies = [
"tokio",
"tokio-metrics",
"uuid",
+ "which",
"winapi",
"winres",
]
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index 22096cb60..e7ff19954 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -3617,6 +3617,11 @@ itest!(followup_dyn_import_resolved {
output: "run/followup_dyn_import_resolves/main.ts.out",
});
+itest!(allow_run_allowlist_resolution {
+ args: "run --quiet --unstable -A allow_run_allowlist_resolution.ts",
+ output: "allow_run_allowlist_resolution.ts.out",
+});
+
itest!(unhandled_rejection {
args: "run --check run/unhandled_rejection.ts",
output: "run/unhandled_rejection.ts.out",
diff --git a/cli/tests/testdata/allow_run_allowlist_resolution.ts b/cli/tests/testdata/allow_run_allowlist_resolution.ts
new file mode 100644
index 000000000..c7369d928
--- /dev/null
+++ b/cli/tests/testdata/allow_run_allowlist_resolution.ts
@@ -0,0 +1,66 @@
+// Testing the following (but with `deno` instead of `echo`):
+// | `deno run --allow-run=echo` | `which path == "/usr/bin/echo"` at startup | `which path != "/usr/bin/echo"` at startup |
+// |-------------------------------------|--------------------------------------------|--------------------------------------------|
+// | **`Deno.Command("echo")`** | ✅ | ✅ |
+// | **`Deno.Command("/usr/bin/echo")`** | ✅ | ❌ |
+
+// | `deno run --allow-run=/usr/bin/echo | `which path == "/usr/bin/echo"` at runtime | `which path != "/usr/bin/echo"` at runtime |
+// |-------------------------------------|--------------------------------------------|--------------------------------------------|
+// | **`Deno.Command("echo")`** | ✅ | ❌ |
+// | **`Deno.Command("/usr/bin/echo")`** | ✅ | ✅ |
+
+const execPath = Deno.execPath();
+const execPathParent = execPath.replace(/[/\\][^/\\]+$/, "");
+
+const testUrl = `data:application/typescript;base64,${
+ btoa(`
+ console.log(await Deno.permissions.query({ name: "run", command: "deno" }));
+ console.log(await Deno.permissions.query({ name: "run", command: "${
+ execPath.replaceAll("\\", "\\\\")
+ }" }));
+ Deno.env.set("PATH", "");
+ console.log(await Deno.permissions.query({ name: "run", command: "deno" }));
+ console.log(await Deno.permissions.query({ name: "run", command: "${
+ execPath.replaceAll("\\", "\\\\")
+ }" }));
+`)
+}`;
+
+const process1 = await new Deno.Command(Deno.execPath(), {
+ args: [
+ "run",
+ "--quiet",
+ "--allow-env",
+ "--allow-run=deno",
+ testUrl,
+ ],
+ stderr: "null",
+ env: { "PATH": execPathParent },
+}).output();
+console.log(new TextDecoder().decode(process1.stdout));
+
+const process2 = await new Deno.Command(Deno.execPath(), {
+ args: [
+ "run",
+ "--quiet",
+ "--allow-env",
+ "--allow-run=deno",
+ testUrl,
+ ],
+ stderr: "null",
+ env: { "PATH": "" },
+}).output();
+console.log(new TextDecoder().decode(process2.stdout));
+
+const process3 = await new Deno.Command(Deno.execPath(), {
+ args: [
+ "run",
+ "--quiet",
+ "--allow-env",
+ `--allow-run=${execPath}`,
+ testUrl,
+ ],
+ stderr: "null",
+ env: { "PATH": execPathParent },
+}).output();
+console.log(new TextDecoder().decode(process3.stdout));
diff --git a/cli/tests/testdata/allow_run_allowlist_resolution.ts.out b/cli/tests/testdata/allow_run_allowlist_resolution.ts.out
new file mode 100644
index 000000000..16ba6754a
--- /dev/null
+++ b/cli/tests/testdata/allow_run_allowlist_resolution.ts.out
@@ -0,0 +1,15 @@
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "granted", onchange: null }
+
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "prompt", onchange: null }
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "prompt", onchange: null }
+
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "granted", onchange: null }
+PermissionStatus { state: "prompt", onchange: null }
+PermissionStatus { state: "granted", onchange: null }
+
diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml
index d4401effd..5ab50714d 100644
--- a/runtime/Cargo.toml
+++ b/runtime/Cargo.toml
@@ -109,6 +109,7 @@ termcolor = "1.1.3"
tokio.workspace = true
tokio-metrics.workspace = true
uuid.workspace = true
+which = "4.2.5"
[target.'cfg(windows)'.dependencies]
fwdansi.workspace = true
diff --git a/runtime/permissions/mod.rs b/runtime/permissions/mod.rs
index a87ca309f..77f44b813 100644
--- a/runtime/permissions/mod.rs
+++ b/runtime/permissions/mod.rs
@@ -27,6 +27,7 @@ use std::path::PathBuf;
use std::str::FromStr;
use std::string::ToString;
use std::sync::Arc;
+use which::which;
mod prompter;
use prompter::permission_prompt;
@@ -261,6 +262,9 @@ pub trait Descriptor: Eq + Clone {
fn stronger_than(&self, other: &Self) -> bool {
self == other
}
+ fn aliases(&self) -> Vec<Self> {
+ vec![]
+ }
}
#[derive(Clone, Debug, Eq, PartialEq)]
@@ -326,33 +330,43 @@ impl<T: Descriptor + Hash> UnaryPermission<T> {
desc: &Option<T>,
allow_partial: AllowPartial,
) -> PermissionState {
- if self.is_flag_denied(desc) || self.is_prompt_denied(desc) {
- PermissionState::Denied
- } else if self.is_granted(desc) {
- match allow_partial {
- AllowPartial::TreatAsGranted => PermissionState::Granted,
- AllowPartial::TreatAsDenied => {
- if self.is_partial_flag_denied(desc) {
- PermissionState::Denied
- } else {
- PermissionState::Granted
+ let aliases = desc.as_ref().map_or(vec![], T::aliases);
+ for desc in [desc]
+ .into_iter()
+ .chain(&aliases.into_iter().map(Some).collect::<Vec<_>>())
+ {
+ let state = if self.is_flag_denied(desc) || self.is_prompt_denied(desc) {
+ PermissionState::Denied
+ } else if self.is_granted(desc) {
+ match allow_partial {
+ AllowPartial::TreatAsGranted => PermissionState::Granted,
+ AllowPartial::TreatAsDenied => {
+ if self.is_partial_flag_denied(desc) {
+ PermissionState::Denied
+ } else {
+ PermissionState::Granted
+ }
}
- }
- AllowPartial::TreatAsPartialGranted => {
- if self.is_partial_flag_denied(desc) {
- PermissionState::GrantedPartial
- } else {
- PermissionState::Granted
+ AllowPartial::TreatAsPartialGranted => {
+ if self.is_partial_flag_denied(desc) {
+ PermissionState::GrantedPartial
+ } else {
+ PermissionState::Granted
+ }
}
}
+ } else if matches!(allow_partial, AllowPartial::TreatAsDenied)
+ && self.is_partial_flag_denied(desc)
+ {
+ PermissionState::Denied
+ } else {
+ PermissionState::Prompt
+ };
+ if state != PermissionState::Prompt {
+ return state;
}
- } else if matches!(allow_partial, AllowPartial::TreatAsDenied)
- && self.is_partial_flag_denied(desc)
- {
- PermissionState::Denied
- } else {
- PermissionState::Prompt
}
+ PermissionState::Prompt
}
fn request_desc(
@@ -402,7 +416,12 @@ impl<T: Descriptor + Hash> UnaryPermission<T> {
fn revoke_desc(&mut self, desc: &Option<T>) -> PermissionState {
match desc.as_ref() {
- Some(desc) => self.granted_list.retain(|v| !v.stronger_than(desc)),
+ Some(desc) => {
+ self.granted_list.retain(|v| !v.stronger_than(desc));
+ for alias in desc.aliases() {
+ self.granted_list.retain(|v| !v.stronger_than(&alias));
+ }
+ }
None => {
self.granted_global = false;
// Revoke global is a special case where the entire granted list is
@@ -469,7 +488,11 @@ impl<T: Descriptor + Hash> UnaryPermission<T> {
) {
match desc {
Some(desc) => {
+ let aliases = desc.aliases();
list.insert(desc);
+ for alias in aliases {
+ list.insert(alias);
+ }
}
None => *list_global = true,
}
@@ -580,7 +603,11 @@ impl AsRef<str> for EnvDescriptor {
#[derive(Clone, Eq, PartialEq, Hash, Debug)]
pub enum RunDescriptor {
+ /// Warning: You may want to construct with `RunDescriptor::from()` for case
+ /// handling.
Name(String),
+ /// Warning: You may want to construct with `RunDescriptor::from()` for case
+ /// handling.
Path(PathBuf),
}
@@ -592,19 +619,41 @@ impl Descriptor for RunDescriptor {
fn name(&self) -> Cow<str> {
Cow::from(self.to_string())
}
-}
-impl FromStr for RunDescriptor {
- type Err = ();
+ fn aliases(&self) -> Vec<Self> {
+ match self {
+ RunDescriptor::Name(name) => match which(name) {
+ Ok(path) => vec![RunDescriptor::Path(path)],
+ Err(_) => vec![],
+ },
+ RunDescriptor::Path(_) => vec![],
+ }
+ }
+}
- fn from_str(s: &str) -> Result<Self, Self::Err> {
+impl From<String> for RunDescriptor {
+ fn from(s: String) -> Self {
+ #[cfg(windows)]
+ let s = s.to_lowercase();
let is_path = s.contains('/');
#[cfg(windows)]
- let is_path = is_path || s.contains('\\') || Path::new(s).is_absolute();
+ let is_path = is_path || s.contains('\\') || Path::new(&s).is_absolute();
if is_path {
- Ok(Self::Path(resolve_from_cwd(Path::new(s)).unwrap()))
+ Self::Path(resolve_from_cwd(Path::new(&s)).unwrap())
} else {
- Ok(Self::Name(s.to_string()))
+ Self::Name(s)
+ }
+ }
+}
+
+impl From<PathBuf> for RunDescriptor {
+ fn from(p: PathBuf) -> Self {
+ #[cfg(windows)]
+ let p = PathBuf::from(p.to_string_lossy().to_string().to_lowercase());
+ if p.is_absolute() {
+ Self::Path(p)
+ } else {
+ Self::Path(resolve_from_cwd(&p).unwrap())
}
}
}
@@ -905,19 +954,19 @@ impl UnaryPermission<SysDescriptor> {
impl UnaryPermission<RunDescriptor> {
pub fn query(&self, cmd: Option<&str>) -> PermissionState {
self.query_desc(
- &cmd.map(|c| RunDescriptor::from_str(c).unwrap()),
+ &cmd.map(|c| RunDescriptor::from(c.to_string())),
AllowPartial::TreatAsPartialGranted,
)
}
pub fn request(&mut self, cmd: Option<&str>) -> PermissionState {
- self.request_desc(&cmd.map(|c| RunDescriptor::from_str(c).unwrap()), || {
+ self.request_desc(&cmd.map(|c| RunDescriptor::from(c.to_string())), || {
Some(cmd?.to_string())
})
}
pub fn revoke(&mut self, cmd: Option<&str>) -> PermissionState {
- self.revoke_desc(&cmd.map(|c| RunDescriptor::from_str(c).unwrap()))
+ self.revoke_desc(&cmd.map(|c| RunDescriptor::from(c.to_string())))
}
pub fn check(
@@ -926,7 +975,7 @@ impl UnaryPermission<RunDescriptor> {
api_name: Option<&str>,
) -> Result<(), AnyError> {
self.check_desc(
- &Some(RunDescriptor::from_str(cmd).unwrap()),
+ &Some(RunDescriptor::from(cmd.to_string())),
false,
api_name,
|| Some(format!("\"{}\"", cmd)),
@@ -1594,19 +1643,20 @@ fn parse_sys_list(
fn parse_run_list(
list: &Option<Vec<String>>,
) -> Result<HashSet<RunDescriptor>, AnyError> {
+ let mut result = HashSet::new();
if let Some(v) = list {
- v.iter()
- .map(|x| {
- if x.is_empty() {
- Err(AnyError::msg("Empty path is not allowed"))
- } else {
- Ok(RunDescriptor::from_str(x).unwrap())
- }
- })
- .collect()
- } else {
- Ok(HashSet::new())
+ for s in v {
+ if s.is_empty() {
+ return Err(AnyError::msg("Empty path is not allowed"));
+ } else {
+ let desc = RunDescriptor::from(s.to_string());
+ let aliases = desc.aliases();
+ result.insert(desc);
+ result.extend(aliases);
+ }
+ }
}
+ Ok(result)
}
fn escalation_error() -> AnyError {