Repository: denoland/deno
Pull request: feat(permissions): allow `Deno.env.toObject()` for partial env permission

Allow calling `Deno.env.toObject()` or getting the entire `process.env` when only having partial permissions.

For example, you can now do `--allow-env --deny-env=SOMETHING` and the object will have all the env vars except `SOMETHING`

We discussed just never throwing here, but we think it would be a bad idea because someone might get confused why their program isn't working whereas now they get alerted about env permissions ("I set this environment variable, but it's not working!").

Extracted out of https://github.com/denoland/deno/pull/31187

Changed files:
filename: ext/node/lib.rs
patch: @@ -5,7 +5,6 @@
 #![allow(clippy::too_many_arguments)]
 
 use std::borrow::Cow;
-use std::collections::HashSet;
 use std::path::Path;
 
 use deno_core::FastString;
@@ -25,7 +24,6 @@ use node_resolver::IsBuiltInNodeModuleChecker;
 use node_resolver::NpmPackageFolderResolver;
 use node_resolver::PackageJsonResolverRc;
 use node_resolver::errors::PackageJsonLoadError;
-use once_cell::sync::Lazy;
 
 extern crate libz_sys as zlib;
 
@@ -162,15 +160,6 @@ pub fn default_resolve_require_node_module_paths(from: &Path) -> Vec<String> {
   paths
 }
 
-pub static NODE_ENV_VAR_ALLOWLIST: Lazy<HashSet<String>> = Lazy::new(|| {
-  // The full list of environment variables supported by Node.js is available
-  // at https://nodejs.org/api/cli.html#environment-variables
-  let mut set = HashSet::new();
-  set.insert("NODE_DEBUG".to_string());
-  set.insert("NODE_OPTIONS".to_string());
-  set
-});
-
 #[op2]
 #[string]
 fn op_node_build_os() -> String {

filename: ext/os/lib.rs
patch: @@ -2,8 +2,8 @@
 
 use std::borrow::Cow;
 use std::collections::HashMap;
-use std::collections::HashSet;
 use std::env;
+use std::ffi::OsString;
 use std::sync::Arc;
 use std::sync::atomic::AtomicI32;
 use std::sync::atomic::Ordering;
@@ -13,20 +13,19 @@ use deno_core::op2;
 use deno_core::v8;
 use deno_path_util::normalize_path;
 use deno_permissions::PermissionCheckError;
+use deno_permissions::PermissionState;
 use deno_permissions::PermissionsContainer;
-use once_cell::sync::Lazy;
 use serde::Serialize;
 
 mod ops;
 pub mod sys_info;
 
 pub use ops::signal::SignalError;
 
-pub static NODE_ENV_VAR_ALLOWLIST: Lazy<HashSet<&str>> = Lazy::new(|| {
-  // The full list of environment variables supported by Node.js is available
-  // at https://nodejs.org/api/cli.html#environment-variables
-  HashSet::from(["NODE_DEBUG", "NODE_OPTIONS", "FORCE_COLOR", "NO_COLOR"])
-});
+// The full list of environment variables supported by Node.js is available
+// at https://nodejs.org/api/cli.html#environment-variables
+const SORTED_NODE_ENV_VAR_ALLOWLIST: [&str; 4] =
+  ["FORCE_COLOR", "NODE_DEBUG", "NODE_OPTIONS", "NO_COLOR"];
 
 #[derive(Clone, Default)]
 pub struct ExitCode(Arc<AtomicI32>);
@@ -179,15 +178,41 @@ fn op_set_env(
 fn op_env(
   state: &mut OpState,
 ) -> Result<HashMap<String, String>, PermissionCheckError> {
-  state.borrow_mut::<PermissionsContainer>().check_env_all()?;
+  fn map_kv(kv: (OsString, OsString)) -> Option<(String, String)> {
+    kv.0
+      .into_string()
+      .ok()
+      .and_then(|key| kv.1.into_string().ok().map(|value| (key, value)))
+  }
 
+  let permissions_container = state.borrow::<PermissionsContainer>();
+  let grant_all = match permissions_container.check_env_all() {
+    Ok(()) => true,
+    Err(PermissionCheckError::PermissionDenied(err)) => match err.state {
+      PermissionState::Granted
+      | PermissionState::Prompt
+      | PermissionState::Denied => return Err(err.into()),
+      PermissionState::GrantedPartial | PermissionState::DeniedPartial => false,
+    },
+    Err(err) => return Err(err),
+  };
   Ok(
     env::vars_os()
-      .filter_map(|(key_os, value_os)| {
-        key_os
-          .into_string()
-          .ok()
-          .and_then(|key| value_os.into_string().ok().map(|value| (key, value)))
+      .filter_map(|kv| {
+        let (k, v) = map_kv(kv)?;
+        let state = if grant_all {
+          PermissionState::Granted
+        } else {
+          permissions_container.query_env(Some(&k))
+        };
+        match state {
+          PermissionState::Granted | PermissionState::GrantedPartial => {
+            Some((k, v))
+          }
+          PermissionState::Prompt
+          | PermissionState::Denied
+          | PermissionState::DeniedPartial => None,
+        }
       })
       .collect(),
   )
@@ -223,7 +248,8 @@ fn op_get_env(
   state: &mut OpState,
   #[string] key: &str,
 ) -> Result<Option<String>, OsError> {
-  let skip_permission_check = NODE_ENV_VAR_ALLOWLIST.contains(key);
+  let skip_permission_check =
+    SORTED_NODE_ENV_VAR_ALLOWLIST.binary_search(&key).is_ok();
 
   if !skip_permission_check {
     state.borrow_mut::<PermissionsContainer>().check_env(key)?;
@@ -235,9 +261,9 @@ fn op_get_env(
 #[op2(fast, stack_trace)]
 fn op_delete_env(
   state: &mut OpState,
-  #[string] key: String,
+  #[string] key: &str,
 ) -> Result<(), OsError> {
-  state.borrow_mut::<PermissionsContainer>().check_env(&key)?;
+  state.borrow_mut::<PermissionsContainer>().check_env(key)?;
   if key.is_empty() || key.contains(&['=', '\0'] as &[char]) {
     return Err(OsError::EnvInvalidKey(key.to_string()));
   }
@@ -701,3 +727,16 @@ fn os_uptime(state: &mut OpState) -> Result<u64, PermissionCheckError> {
 fn op_os_uptime(state: &mut OpState) -> Result<u64, PermissionCheckError> {
   os_uptime(state)
 }
+
+#[cfg(test)]
+mod test {
+  use crate::SORTED_NODE_ENV_VAR_ALLOWLIST;
+
+  #[test]
+  fn ensure_node_env_var_list_sorted() {
+    // ensure this is sorted for binary search
+    let mut items = SORTED_NODE_ENV_VAR_ALLOWLIST.to_vec();
+    items.sort();
+    assert_eq!(items, SORTED_NODE_ENV_VAR_ALLOWLIST);
+  }
+}

filename: tests/specs/run/permission_env_allow_and_deny/__test__.jsonc
patch: @@ -1,7 +1,6 @@
 {
   "args": "run --allow-env --deny-env=FOOBAR main.ts",
-  "output": "main.out",
-  "exitCode": 1,
+  "output": "",
   "envs": {
     "FOOBAR": "FOOBAR"
   }

filename: tests/specs/run/permission_env_allow_and_deny/main.out
patch: @@ -1,4 +0,0 @@
-error: Uncaught (in promise) NotCapable: Requires env access, run again with the --allow-env flag
-console.log(Deno.env.toObject());
-                     ^
-[WILDCARD]main.ts:1:22

filename: tests/specs/run/permission_env_allow_and_deny/main.ts
patch: @@ -1 +1,11 @@
-console.log(Deno.env.toObject());
+const obj = Deno.env.toObject();
+const pathKey = Object.keys(obj).find((p) => p.toLowerCase() === "path");
+if (pathKey == null) {
+  throw "FAIL CASING";
+}
+if (obj[pathKey] == null) {
+  throw "FAIL";
+}
+if ("FOOBAR" in obj) {
+  throw "FAIL2";
+}