refactor(permissions): split up Descriptor into Allow, Deny, and Query (#25508)

This makes the permission system more versatile.

4 files changed, 198 insertions, 51 deletions

diff --git a/runtime/ops/fs_events.rs b/runtime/ops/fs_events.rs
index 58fe9d5fd..d88a32d91 100644
--- a/runtime/ops/fs_events.rs
+++ b/runtime/ops/fs_events.rs
@@ -123,10 +123,9 @@ fn op_fs_events_open(
     RecursiveMode::NonRecursive
   };
   for path in &args.paths {
-    let path = PathBuf::from(path);
-    state
+    let path = state
       .borrow_mut::<PermissionsContainer>()
-      .check_read(&path, "Deno.watchFs()")?;
+      .check_read(path, "Deno.watchFs()")?;
     watcher.watch(&path, recursive_mode)?;
   }
   let resource = FsEventsResource {
diff --git a/runtime/ops/permissions.rs b/runtime/ops/permissions.rs
index e6974efad..9b46dd019 100644
--- a/runtime/ops/permissions.rs
+++ b/runtime/ops/permissions.rs
@@ -1,7 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use ::deno_permissions::parse_sys_kind;
-use ::deno_permissions::NetDescriptor;
+use ::deno_permissions::PermissionDescriptorParser;
 use ::deno_permissions::PermissionState;
 use ::deno_permissions::PermissionsContainer;
 use deno_core::error::custom_error;
@@ -10,7 +10,7 @@ use deno_core::op2;
 use deno_core::OpState;
 use serde::Deserialize;
 use serde::Serialize;
-use std::path::Path;
+use std::sync::Arc;
 
 deno_core::extension!(
   deno_permissions,
@@ -19,6 +19,12 @@ deno_core::extension!(
     op_revoke_permission,
     op_request_permission,
   ],
+  options = {
+    permission_desc_parser: Arc<dyn PermissionDescriptorParser>,
+  },
+  state = |state, options| {
+    state.put(options.permission_desc_parser);
+  },
 );
 
 #[derive(Deserialize)]
@@ -56,15 +62,37 @@ pub fn op_query_permission(
   state: &mut OpState,
   #[serde] args: PermissionArgs,
 ) -> Result<PermissionStatus, AnyError> {
-  let permissions = state.borrow::<PermissionsContainer>().0.lock();
+  let permissions_container = state.borrow::<PermissionsContainer>();
+  // todo(dsherret): don't have this function use the properties of
+  // permission container
+  let desc_parser = &permissions_container.descriptor_parser;
+  let permissions = permissions_container.inner.lock();
   let path = args.path.as_deref();
   let perm = match args.name.as_ref() {
-    "read" => permissions.read.query(path.map(Path::new)),
-    "write" => permissions.write.query(path.map(Path::new)),
+    "read" => permissions.read.query(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_read(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
+    "write" => permissions.write.query(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_write(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
     "net" => permissions.net.query(
       match args.host.as_deref() {
         None => None,
-        Some(h) => Some(NetDescriptor::parse(h)?),
+        Some(h) => Some(desc_parser.parse_net_descriptor(h)?),
       }
       .as_ref(),
     ),
@@ -72,8 +100,24 @@ pub fn op_query_permission(
     "sys" => permissions
       .sys
       .query(args.kind.as_deref().map(parse_sys_kind).transpose()?),
-    "run" => permissions.run.query(args.command.as_deref()),
-    "ffi" => permissions.ffi.query(args.path.as_deref().map(Path::new)),
+    "run" => permissions.run.query(
+      args
+        .command
+        .as_deref()
+        .map(|request| desc_parser.parse_run_query(request))
+        .transpose()?
+        .as_ref(),
+    ),
+    "ffi" => permissions.ffi.query(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_ffi(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
     n => {
       return Err(custom_error(
         "ReferenceError",
@@ -90,15 +134,37 @@ pub fn op_revoke_permission(
   state: &mut OpState,
   #[serde] args: PermissionArgs,
 ) -> Result<PermissionStatus, AnyError> {
-  let mut permissions = state.borrow_mut::<PermissionsContainer>().0.lock();
+  // todo(dsherret): don't have this function use the properties of
+  // permission container
+  let permissions_container = state.borrow_mut::<PermissionsContainer>();
+  let desc_parser = &permissions_container.descriptor_parser;
+  let mut permissions = permissions_container.inner.lock();
   let path = args.path.as_deref();
   let perm = match args.name.as_ref() {
-    "read" => permissions.read.revoke(path.map(Path::new)),
-    "write" => permissions.write.revoke(path.map(Path::new)),
+    "read" => permissions.read.revoke(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_read(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
+    "write" => permissions.write.revoke(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_write(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
     "net" => permissions.net.revoke(
       match args.host.as_deref() {
         None => None,
-        Some(h) => Some(NetDescriptor::parse(h)?),
+        Some(h) => Some(desc_parser.parse_net_descriptor(h)?),
       }
       .as_ref(),
     ),
@@ -106,8 +172,24 @@ pub fn op_revoke_permission(
     "sys" => permissions
       .sys
       .revoke(args.kind.as_deref().map(parse_sys_kind).transpose()?),
-    "run" => permissions.run.revoke(args.command.as_deref()),
-    "ffi" => permissions.ffi.revoke(args.path.as_deref().map(Path::new)),
+    "run" => permissions.run.revoke(
+      args
+        .command
+        .as_deref()
+        .map(|request| desc_parser.parse_run_query(request))
+        .transpose()?
+        .as_ref(),
+    ),
+    "ffi" => permissions.ffi.revoke(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_ffi(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
     n => {
       return Err(custom_error(
         "ReferenceError",
@@ -124,15 +206,37 @@ pub fn op_request_permission(
   state: &mut OpState,
   #[serde] args: PermissionArgs,
 ) -> Result<PermissionStatus, AnyError> {
-  let mut permissions = state.borrow_mut::<PermissionsContainer>().0.lock();
+  // todo(dsherret): don't have this function use the properties of
+  // permission container
+  let permissions_container = state.borrow_mut::<PermissionsContainer>();
+  let desc_parser = &permissions_container.descriptor_parser;
+  let mut permissions = permissions_container.inner.lock();
   let path = args.path.as_deref();
   let perm = match args.name.as_ref() {
-    "read" => permissions.read.request(path.map(Path::new)),
-    "write" => permissions.write.request(path.map(Path::new)),
+    "read" => permissions.read.request(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_read(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
+    "write" => permissions.write.request(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_write(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
     "net" => permissions.net.request(
       match args.host.as_deref() {
         None => None,
-        Some(h) => Some(NetDescriptor::parse(h)?),
+        Some(h) => Some(desc_parser.parse_net_descriptor(h)?),
       }
       .as_ref(),
     ),
@@ -140,8 +244,24 @@ pub fn op_request_permission(
     "sys" => permissions
       .sys
       .request(args.kind.as_deref().map(parse_sys_kind).transpose()?),
-    "run" => permissions.run.request(args.command.as_deref()),
-    "ffi" => permissions.ffi.request(args.path.as_deref().map(Path::new)),
+    "run" => permissions.run.request(
+      args
+        .command
+        .as_deref()
+        .map(|request| desc_parser.parse_run_query(request))
+        .transpose()?
+        .as_ref(),
+    ),
+    "ffi" => permissions.ffi.request(
+      path
+        .map(|path| {
+          Result::<_, AnyError>::Ok(
+            desc_parser.parse_path_query(path)?.into_ffi(),
+          )
+        })
+        .transpose()?
+        .as_ref(),
+    ),
     n => {
       return Err(custom_error(
         "ReferenceError",
diff --git a/runtime/ops/process.rs b/runtime/ops/process.rs
index b7242c07f..a39bb5f04 100644
--- a/runtime/ops/process.rs
+++ b/runtime/ops/process.rs
@@ -17,12 +17,13 @@ use deno_io::ChildStderrResource;
 use deno_io::ChildStdinResource;
 use deno_io::ChildStdoutResource;
 use deno_permissions::PermissionsContainer;
-use deno_permissions::RunPathQuery;
+use deno_permissions::RunQueryDescriptor;
 use serde::Deserialize;
 use serde::Serialize;
 use std::borrow::Cow;
 use std::cell::RefCell;
 use std::collections::HashMap;
+use std::ffi::OsString;
 use std::path::Path;
 use std::path::PathBuf;
 use std::process::ExitStatus;
@@ -536,9 +537,9 @@ fn compute_run_cmd_and_check_permissions(
     .with_context(|| format!("Failed to spawn '{}'", arg_cmd))?;
   check_run_permission(
     state,
-    RunPathQuery {
-      requested: arg_cmd,
-      resolved: &cmd,
+    &RunQueryDescriptor::Path {
+      requested: arg_cmd.to_string(),
+      resolved: cmd.clone(),
     },
     &run_env,
     api_name,
@@ -547,7 +548,7 @@ fn compute_run_cmd_and_check_permissions(
 }
 
 struct RunEnv {
-  envs: HashMap<String, String>,
+  envs: HashMap<OsString, OsString>,
   cwd: PathBuf,
 }
 
@@ -567,11 +568,32 @@ fn compute_run_env(
     .map(|cwd_arg| resolve_path(cwd_arg, &cwd))
     .unwrap_or(cwd);
   let envs = if arg_clear_env {
-    arg_envs.iter().cloned().collect()
+    arg_envs
+      .iter()
+      .map(|(k, v)| (OsString::from(k), OsString::from(v)))
+      .collect()
   } else {
-    let mut envs = std::env::vars().collect::<HashMap<_, _>>();
+    let mut envs = std::env::vars_os()
+      .map(|(k, v)| {
+        (
+          if cfg!(windows) {
+            k.to_ascii_uppercase()
+          } else {
+            k
+          },
+          v,
+        )
+      })
+      .collect::<HashMap<_, _>>();
     for (key, value) in arg_envs {
-      envs.insert(key.clone(), value.clone());
+      envs.insert(
+        OsString::from(if cfg!(windows) {
+          key.to_ascii_uppercase()
+        } else {
+          key.clone()
+        }),
+        OsString::from(value.clone()),
+      );
     }
     envs
   };
@@ -585,19 +607,7 @@ fn resolve_cmd(cmd: &str, env: &RunEnv) -> Result<PathBuf, AnyError> {
   if is_path {
     Ok(resolve_path(cmd, &env.cwd))
   } else {
-    let path = env.envs.get("PATH").or_else(|| {
-      if cfg!(windows) {
-        env.envs.iter().find_map(|(k, v)| {
-          if k.to_uppercase() == "PATH" {
-            Some(v)
-          } else {
-            None
-          }
-        })
-      } else {
-        None
-      }
-    });
+    let path = env.envs.get(&OsString::from("PATH"));
     match which::which_in(cmd, path, &env.cwd) {
       Ok(cmd) => Ok(cmd),
       Err(which::Error::CannotFindBinaryPath) => {
@@ -614,7 +624,7 @@ fn resolve_path(path: &str, cwd: &Path) -> PathBuf {
 
 fn check_run_permission(
   state: &mut OpState,
-  cmd: RunPathQuery,
+  cmd: &RunQueryDescriptor,
   run_env: &RunEnv,
   api_name: &str,
 ) -> Result<(), AnyError> {
@@ -647,11 +657,22 @@ fn get_requires_allow_all_env_vars(env: &RunEnv) -> Vec<&str> {
     key.starts_with("LD_") || key.starts_with("DYLD_")
   }
 
+  fn is_empty(value: &OsString) -> bool {
+    value.is_empty()
+      || value.to_str().map(|v| v.trim().is_empty()).unwrap_or(false)
+  }
+
   let mut found_envs = env
     .envs
     .iter()
-    .filter(|(k, v)| requires_allow_all(k) && !v.trim().is_empty())
-    .map(|(k, _)| k.as_str())
+    .filter_map(|(k, v)| {
+      let key = k.to_str()?;
+      if requires_allow_all(key) && !is_empty(v) {
+        Some(key)
+      } else {
+        None
+      }
+    })
     .collect::<Vec<_>>();
   found_envs.sort();
   found_envs
diff --git a/runtime/ops/worker_host.rs b/runtime/ops/worker_host.rs
index 19475fedf..3c0035645 100644
--- a/runtime/ops/worker_host.rs
+++ b/runtime/ops/worker_host.rs
@@ -19,6 +19,7 @@ use deno_core::ModuleSpecifier;
 use deno_core::OpState;
 use deno_permissions::create_child_permissions;
 use deno_permissions::ChildPermissionsArg;
+use deno_permissions::PermissionDescriptorParser;
 use deno_permissions::PermissionsContainer;
 use deno_web::deserialize_js_transferables;
 use deno_web::JsMessageData;
@@ -153,13 +154,19 @@ fn op_create_worker(
       "Worker.deno.permissions",
     );
   }
+  let permission_desc_parser = state
+    .borrow::<Arc<dyn PermissionDescriptorParser>>()
+    .clone();
   let parent_permissions = state.borrow_mut::<PermissionsContainer>();
   let worker_permissions = if let Some(child_permissions_arg) = args.permissions
   {
-    let mut parent_permissions = parent_permissions.0.lock();
-    let perms =
-      create_child_permissions(&mut parent_permissions, child_permissions_arg)?;
-    PermissionsContainer::new(perms)
+    let mut parent_permissions = parent_permissions.inner.lock();
+    let perms = create_child_permissions(
+      permission_desc_parser.as_ref(),
+      &mut parent_permissions,
+      child_permissions_arg,
+    )?;
+    PermissionsContainer::new(permission_desc_parser, perms)
   } else {
     parent_permissions.clone()
   };