From e90f42cb2ca626dbbbd9d0fc5b782435283ae46a Mon Sep 17 00:00:00 2001 From: Colin Ihrig Date: Wed, 14 Sep 2022 11:59:20 -0400 Subject: [PATCH] fix(ops): add node.js env variable allowlist (#15893) This commit allows the Node compatibility layer to skip environment variable permission checks when --unstable is passed and the variable name is one that Node uses. Fixes: https://github.com/denoland/deno/issues/15890 --- cli/tests/integration/run_tests.rs | 12 ++++++++++++ cli/tests/testdata/node_env_var_allowlist.ts | 2 ++ .../node_env_var_allowlist_with_unstable_flag.ts.out | 5 +++++ ...de_env_var_allowlist_without_unstable_flag.ts.out | 4 ++++ ext/node/lib.rs | 10 ++++++++++ runtime/ops/os.rs | 10 +++++++++- 6 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 cli/tests/testdata/node_env_var_allowlist.ts create mode 100644 cli/tests/testdata/node_env_var_allowlist_with_unstable_flag.ts.out create mode 100644 cli/tests/testdata/node_env_var_allowlist_without_unstable_flag.ts.out diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 118623760c..d4dfbb32bc 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2839,3 +2839,15 @@ itest!(nested_error { output: "nested_error.ts.out", exit_code: 1, }); + +itest!(node_env_var_allowlist_with_unstable_flag { + args: "run --unstable --no-prompt node_env_var_allowlist.ts", + output: "node_env_var_allowlist_with_unstable_flag.ts.out", + exit_code: 1, +}); + +itest!(node_env_var_allowlist_without_unstable_flag { + args: "run --no-prompt node_env_var_allowlist.ts", + output: "node_env_var_allowlist_without_unstable_flag.ts.out", + exit_code: 1, +}); diff --git a/cli/tests/testdata/node_env_var_allowlist.ts b/cli/tests/testdata/node_env_var_allowlist.ts new file mode 100644 index 0000000000..95da38c240 --- /dev/null +++ b/cli/tests/testdata/node_env_var_allowlist.ts @@ -0,0 +1,2 @@ +console.log(Deno.env.get("NODE_DEBUG") ?? "ok"); +Deno.env.get("NOT_NODE_DEBUG"); diff --git a/cli/tests/testdata/node_env_var_allowlist_with_unstable_flag.ts.out b/cli/tests/testdata/node_env_var_allowlist_with_unstable_flag.ts.out new file mode 100644 index 0000000000..62f335c0f3 --- /dev/null +++ b/cli/tests/testdata/node_env_var_allowlist_with_unstable_flag.ts.out @@ -0,0 +1,5 @@ +ok +[WILDCARD]error: Uncaught PermissionDenied: Requires env access to "NOT_NODE_DEBUG", run again with the --allow-env flag +Deno.env.get("NOT_NODE_DEBUG"); + ^ + at [WILDCARD] diff --git a/cli/tests/testdata/node_env_var_allowlist_without_unstable_flag.ts.out b/cli/tests/testdata/node_env_var_allowlist_without_unstable_flag.ts.out new file mode 100644 index 0000000000..ac92cdb6b7 --- /dev/null +++ b/cli/tests/testdata/node_env_var_allowlist_without_unstable_flag.ts.out @@ -0,0 +1,4 @@ +[WILDCARD]error: Uncaught PermissionDenied: Requires env access to "NODE_DEBUG", run again with the --allow-env flag +console.log(Deno.env.get("NODE_DEBUG") ?? "ok"); + ^ + at [WILDCARD] diff --git a/ext/node/lib.rs b/ext/node/lib.rs index da8ca30035..42348915ed 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -8,6 +8,7 @@ use deno_core::url::Url; use deno_core::Extension; use deno_core::OpState; use once_cell::sync::Lazy; +use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -59,6 +60,15 @@ pub static NODE_GLOBAL_THIS_NAME: Lazy = Lazy::new(|| { format!("__DENO_NODE_GLOBAL_THIS_{}__", seconds) }); +pub static NODE_ENV_VAR_ALLOWLIST: Lazy> = 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 +}); + struct Unstable(pub bool); pub fn init( diff --git a/runtime/ops/os.rs b/runtime/ops/os.rs index 5d275a8366..21a94b0fbd 100644 --- a/runtime/ops/os.rs +++ b/runtime/ops/os.rs @@ -8,6 +8,7 @@ use deno_core::url::Url; use deno_core::Extension; use deno_core::OpState; use deno_core::{op, ExtensionBuilder}; +use deno_node::NODE_ENV_VAR_ALLOWLIST; use serde::Serialize; use std::collections::HashMap; use std::env; @@ -99,7 +100,14 @@ fn op_get_env( state: &mut OpState, key: String, ) -> Result, AnyError> { - state.borrow_mut::().env.check(&key)?; + let skip_permission_check = + state.borrow::().unstable + && NODE_ENV_VAR_ALLOWLIST.contains(&key); + + if !skip_permission_check { + state.borrow_mut::().env.check(&key)?; + } + if key.is_empty() || key.contains(&['=', '\0'] as &[char]) { return Err(type_error("Key contains invalid characters.")); }