From b729bf0ad9a2711b9740b30dcf78d826fbc76349 Mon Sep 17 00:00:00 2001 From: Yazan AbdAl-Rahman Date: Thu, 21 Nov 2024 01:30:43 +0200 Subject: [PATCH] feat(permission): support suffix wildcards in `--allow-env` flag (#25255) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds support for suffix wildcard for `--allow-env` flag. Specifying flag like `--allow-env=DENO_*` will enable access to all environmental variables starting with `DENO_*`. Closes #24847 --------- Co-authored-by: Bartek Iwańczuk Co-authored-by: David Sherret --- runtime/permissions/lib.rs | 203 ++++++++++++++++-- .../process_env_permissions/__test__.jsonc | 26 +++ .../process_env_permissions/main.js | 5 + .../allow_env_wildcard_worker/__test__.jsonc | 10 + .../run/allow_env_wildcard_worker/main.js | 12 ++ .../run/allow_env_wildcard_worker/main.out | 11 + .../run/allow_env_wildcard_worker/worker.js | 3 + 7 files changed, 250 insertions(+), 20 deletions(-) create mode 100644 tests/specs/permission/process_env_permissions/__test__.jsonc create mode 100644 tests/specs/permission/process_env_permissions/main.js create mode 100644 tests/specs/run/allow_env_wildcard_worker/__test__.jsonc create mode 100644 tests/specs/run/allow_env_wildcard_worker/main.js create mode 100644 tests/specs/run/allow_env_wildcard_worker/main.out create mode 100644 tests/specs/run/allow_env_wildcard_worker/worker.js diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs index 71ef7d2289..a0b901d200 100644 --- a/runtime/permissions/lib.rs +++ b/runtime/permissions/lib.rs @@ -294,7 +294,7 @@ impl UnitPermission { /// A normalized environment variable name. On Windows this will /// be uppercase and on other platforms it will stay as-is. #[derive(Clone, Eq, PartialEq, Hash, Debug)] -struct EnvVarName { +pub struct EnvVarName { inner: String, } @@ -1114,15 +1114,37 @@ impl ImportDescriptor { pub struct EnvDescriptorParseError; #[derive(Clone, Eq, PartialEq, Hash, Debug)] -pub struct EnvDescriptor(EnvVarName); +pub enum EnvDescriptor { + Name(EnvVarName), + PrefixPattern(EnvVarName), +} impl EnvDescriptor { pub fn new(env: impl AsRef) -> Self { - Self(EnvVarName::new(env)) + if let Some(prefix_pattern) = env.as_ref().strip_suffix('*') { + Self::PrefixPattern(EnvVarName::new(prefix_pattern)) + } else { + Self::Name(EnvVarName::new(env)) + } } } -impl QueryDescriptor for EnvDescriptor { +#[derive(Clone, Eq, PartialEq, Hash, Debug)] +enum EnvQueryDescriptorInner { + Name(EnvVarName), + PrefixPattern(EnvVarName), +} + +#[derive(Clone, Eq, PartialEq, Hash, Debug)] +pub struct EnvQueryDescriptor(EnvQueryDescriptorInner); + +impl EnvQueryDescriptor { + pub fn new(env: impl AsRef) -> Self { + Self(EnvQueryDescriptorInner::Name(EnvVarName::new(env))) + } +} + +impl QueryDescriptor for EnvQueryDescriptor { type AllowDesc = EnvDescriptor; type DenyDesc = EnvDescriptor; @@ -1131,19 +1153,45 @@ impl QueryDescriptor for EnvDescriptor { } fn display_name(&self) -> Cow { - Cow::from(self.0.as_ref()) + Cow::from(match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => env_var_name.as_ref(), + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref() + } + }) } fn from_allow(allow: &Self::AllowDesc) -> Self { - allow.clone() + match allow { + Self::AllowDesc::Name(s) => { + Self(EnvQueryDescriptorInner::Name(s.clone())) + } + Self::AllowDesc::PrefixPattern(s) => { + Self(EnvQueryDescriptorInner::PrefixPattern(s.clone())) + } + } } fn as_allow(&self) -> Option { - Some(self.clone()) + Some(match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => { + Self::AllowDesc::Name(env_var_name.clone()) + } + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + Self::AllowDesc::PrefixPattern(env_var_name.clone()) + } + }) } fn as_deny(&self) -> Self::DenyDesc { - self.clone() + match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => { + Self::DenyDesc::Name(env_var_name.clone()) + } + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + Self::DenyDesc::PrefixPattern(env_var_name.clone()) + } + } } fn check_in_permission( @@ -1156,19 +1204,79 @@ impl QueryDescriptor for EnvDescriptor { } fn matches_allow(&self, other: &Self::AllowDesc) -> bool { - self == other + match other { + Self::AllowDesc::Name(n) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => n == env_var_name, + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref().starts_with(n.as_ref()) + } + }, + Self::AllowDesc::PrefixPattern(p) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => { + env_var_name.as_ref().starts_with(p.as_ref()) + } + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref().starts_with(p.as_ref()) + } + }, + } } fn matches_deny(&self, other: &Self::DenyDesc) -> bool { - self == other + match other { + Self::AllowDesc::Name(n) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => n == env_var_name, + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref().starts_with(n.as_ref()) + } + }, + Self::AllowDesc::PrefixPattern(p) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => { + env_var_name.as_ref().starts_with(p.as_ref()) + } + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + p == env_var_name + } + }, + } } fn revokes(&self, other: &Self::AllowDesc) -> bool { - self == other + match other { + Self::AllowDesc::Name(n) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => n == env_var_name, + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref().starts_with(n.as_ref()) + } + }, + Self::AllowDesc::PrefixPattern(p) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => { + env_var_name.as_ref().starts_with(p.as_ref()) + } + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + p == env_var_name + } + }, + } } fn stronger_than_deny(&self, other: &Self::DenyDesc) -> bool { - self == other + match other { + Self::AllowDesc::Name(n) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => n == env_var_name, + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref().starts_with(n.as_ref()) + } + }, + Self::AllowDesc::PrefixPattern(p) => match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => { + env_var_name.as_ref().starts_with(p.as_ref()) + } + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + p == env_var_name + } + }, + } } fn overlaps_deny(&self, _other: &Self::DenyDesc) -> bool { @@ -1176,9 +1284,14 @@ impl QueryDescriptor for EnvDescriptor { } } -impl AsRef for EnvDescriptor { +impl AsRef for EnvQueryDescriptor { fn as_ref(&self) -> &str { - self.0.as_ref() + match &self.0 { + EnvQueryDescriptorInner::Name(env_var_name) => env_var_name.as_ref(), + EnvQueryDescriptorInner::PrefixPattern(env_var_name) => { + env_var_name.as_ref() + } + } } } @@ -1749,20 +1862,20 @@ impl UnaryPermission { } } -impl UnaryPermission { +impl UnaryPermission { pub fn query(&self, env: Option<&str>) -> PermissionState { self.query_desc( - env.map(EnvDescriptor::new).as_ref(), + env.map(EnvQueryDescriptor::new).as_ref(), AllowPartial::TreatAsPartialGranted, ) } pub fn request(&mut self, env: Option<&str>) -> PermissionState { - self.request_desc(env.map(EnvDescriptor::new).as_ref()) + self.request_desc(env.map(EnvQueryDescriptor::new).as_ref()) } pub fn revoke(&mut self, env: Option<&str>) -> PermissionState { - self.revoke_desc(env.map(EnvDescriptor::new).as_ref()) + self.revoke_desc(env.map(EnvQueryDescriptor::new).as_ref()) } pub fn check( @@ -1771,7 +1884,7 @@ impl UnaryPermission { api_name: Option<&str>, ) -> Result<(), PermissionDeniedError> { skip_check_if_is_permission_fully_granted!(self); - self.check_desc(Some(&EnvDescriptor::new(env)), false, api_name) + self.check_desc(Some(&EnvQueryDescriptor::new(env)), false, api_name) } pub fn check_all(&mut self) -> Result<(), PermissionDeniedError> { @@ -1905,7 +2018,7 @@ pub struct Permissions { pub read: UnaryPermission, pub write: UnaryPermission, pub net: UnaryPermission, - pub env: UnaryPermission, + pub env: UnaryPermission, pub sys: UnaryPermission, pub run: UnaryPermission, pub ffi: UnaryPermission, @@ -4564,6 +4677,56 @@ mod tests { assert_eq!(perms.env.revoke(Some("HomE")), PermissionState::Prompt); } + #[test] + fn test_env_wildcards() { + set_prompter(Box::new(TestPrompter)); + let _prompt_value = PERMISSION_PROMPT_STUB_VALUE_SETTER.lock(); + let mut perms = Permissions::allow_all(); + perms.env = UnaryPermission { + granted_global: false, + ..Permissions::new_unary( + Some(HashSet::from([EnvDescriptor::new("HOME_*")])), + None, + false, + ) + }; + assert_eq!(perms.env.query(Some("HOME")), PermissionState::Prompt); + assert_eq!(perms.env.query(Some("HOME_")), PermissionState::Granted); + assert_eq!(perms.env.query(Some("HOME_TEST")), PermissionState::Granted); + + // assert no privilege escalation + let parser = TestPermissionDescriptorParser; + assert!(perms + .env + .create_child_permissions( + ChildUnaryPermissionArg::GrantedList(vec!["HOME_SUB".to_string()]), + |value| parser.parse_env_descriptor(value).map(Some), + ) + .is_ok()); + assert!(perms + .env + .create_child_permissions( + ChildUnaryPermissionArg::GrantedList(vec!["HOME*".to_string()]), + |value| parser.parse_env_descriptor(value).map(Some), + ) + .is_err()); + assert!(perms + .env + .create_child_permissions( + ChildUnaryPermissionArg::GrantedList(vec!["OUTSIDE".to_string()]), + |value| parser.parse_env_descriptor(value).map(Some), + ) + .is_err()); + assert!(perms + .env + .create_child_permissions( + // ok because this is a subset of HOME_* + ChildUnaryPermissionArg::GrantedList(vec!["HOME_S*".to_string()]), + |value| parser.parse_env_descriptor(value).map(Some), + ) + .is_ok()); + } + #[test] fn test_check_partial_denied() { let parser = TestPermissionDescriptorParser; diff --git a/tests/specs/permission/process_env_permissions/__test__.jsonc b/tests/specs/permission/process_env_permissions/__test__.jsonc new file mode 100644 index 0000000000..d3c756e0dc --- /dev/null +++ b/tests/specs/permission/process_env_permissions/__test__.jsonc @@ -0,0 +1,26 @@ +{ + "tempDir": true, + "tests": { + "deno_env_wildcard_tests": { + "envs": { + "MYAPP_HELLO": "Hello\tworld,", + "MYAPP_GOODBYE": "farewell", + "OTHER_VAR": "ignore" + }, + "steps": [ + { + "args": "run --allow-env=MYAPP_* main.js", + "output": "Hello\tworld,\nfarewell\ndone\n" + }, + { + "args": "run --allow-env main.js", + "output": "Hello\tworld,\nfarewell\ndone\n" + }, + { + "args": "run --allow-env=MYAPP_HELLO,MYAPP_GOODBYE,MYAPP_TEST,MYAPP_DONE main.js", + "output": "Hello\tworld,\nfarewell\ndone\n" + } + ] + } + } +} diff --git a/tests/specs/permission/process_env_permissions/main.js b/tests/specs/permission/process_env_permissions/main.js new file mode 100644 index 0000000000..7a412659c9 --- /dev/null +++ b/tests/specs/permission/process_env_permissions/main.js @@ -0,0 +1,5 @@ +console.log(Deno.env.get("MYAPP_HELLO")); +console.log(Deno.env.get("MYAPP_GOODBYE")); +Deno.env.set("MYAPP_TEST", "done"); +Deno.env.set("MYAPP_DONE", "done"); +console.log(Deno.env.get("MYAPP_DONE")); diff --git a/tests/specs/run/allow_env_wildcard_worker/__test__.jsonc b/tests/specs/run/allow_env_wildcard_worker/__test__.jsonc new file mode 100644 index 0000000000..6cfde6207f --- /dev/null +++ b/tests/specs/run/allow_env_wildcard_worker/__test__.jsonc @@ -0,0 +1,10 @@ +{ + "envs": { + "DENO_HELLO": "hello", + "DENO_BYE": "bye", + "AWS_HELLO": "aws" + }, + "args": "run --allow-env --allow-read --unstable-worker-options main.js", + "output": "main.out", + "exitCode": 1 +} diff --git a/tests/specs/run/allow_env_wildcard_worker/main.js b/tests/specs/run/allow_env_wildcard_worker/main.js new file mode 100644 index 0000000000..8d1a45fa61 --- /dev/null +++ b/tests/specs/run/allow_env_wildcard_worker/main.js @@ -0,0 +1,12 @@ +console.log("main1", Deno.env.get("DENO_HELLO")); +console.log("main2", Deno.env.get("DENO_BYE")); +console.log("main3", Deno.env.get("AWS_HELLO")); + +new Worker(import.meta.resolve("./worker.js"), { + type: "module", + deno: { + permissions: { + env: ["DENO_*"], + }, + }, +}); diff --git a/tests/specs/run/allow_env_wildcard_worker/main.out b/tests/specs/run/allow_env_wildcard_worker/main.out new file mode 100644 index 0000000000..01a7c42e39 --- /dev/null +++ b/tests/specs/run/allow_env_wildcard_worker/main.out @@ -0,0 +1,11 @@ +main1 hello +main2 bye +main3 aws +worker1 hello +worker2 bye +error: Uncaught (in worker "") (in promise) NotCapable: Requires env access to "AWS_HELLO", run again with the --allow-env flag +console.log("worker3", Deno.env.get("AWS_HELLO")); + ^ +[WILDCARD] +error: Uncaught (in promise) Error: Unhandled error in child worker. +[WILDCARD] diff --git a/tests/specs/run/allow_env_wildcard_worker/worker.js b/tests/specs/run/allow_env_wildcard_worker/worker.js new file mode 100644 index 0000000000..0238cbf6dc --- /dev/null +++ b/tests/specs/run/allow_env_wildcard_worker/worker.js @@ -0,0 +1,3 @@ +console.log("worker1", Deno.env.get("DENO_HELLO")); +console.log("worker2", Deno.env.get("DENO_BYE")); +console.log("worker3", Deno.env.get("AWS_HELLO"));