Repository: denoland/deno
Pull request: fix(ext/node): checking `Symbol` in `env` should not ask for permission

Discovered on #30905, though not directly addressing the main issue

Given this code: 
```ts
console.log(globalThis.process.env)
```

Deno 2.5.4:
```bash
➜ deno index.ts 
✅ Granted env access to "Symbol(Deno.customInspect)".
✅ Granted env access to "Symbol(Deno.privateCustomInspect)".
✅ Granted env access to "Symbol(nodejs.util.inspect.custom)".
✅ Granted env access to "constructor".
✅ Granted env access to "Symbol(Symbol.iterator)".
✅ Granted env access.
[..result..]
```

This PR:
```bash
➜ deno index.ts
✅ Granted env access to "constructor".
✅ Granted env access.
[..result..]
```

Futhermore, it allows the following tests to pass:
- https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-process-env-deprecation.js
- https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-process-env-ignore-getter-setter.js

Changed files:
filename: ext/node/polyfills/_process/process.ts
patch: @@ -15,13 +15,19 @@ const {
   Proxy,
   ObjectPrototype,
   ObjectPrototypeIsPrototypeOf,
+  ReflectDefineProperty,
+  ReflectHas,
   TypeErrorPrototype,
 } = primordials;
-const { build } = core;
+const { build, createLazyLoader } = core;
 
 import { nextTick as _nextTick } from "ext:deno_node/_next_tick.ts";
 import { _exiting } from "ext:deno_node/_process/exiting.ts";
 import * as fs from "ext:deno_fs/30_fs.js";
+import { ERR_INVALID_OBJECT_DEFINE_PROPERTY } from "ext:deno_node/internal/errors.ts";
+
+const loadProcess = createLazyLoader<NodeJS.Process>("node:process");
+let nodeProcess: NodeJS.Process | undefined;
 
 /** Returns the operating system CPU architecture for which the Deno binary was compiled */
 export function arch(): string {
@@ -63,8 +69,9 @@ const OBJECT_PROTO_PROP_NAMES = ObjectGetOwnPropertyNames(ObjectPrototype);
  * https://nodejs.org/api/process.html#process_process_env
  * Requires env permissions
  */
-export const env: InstanceType<ObjectConstructor> & Record<string, string> =
-  new Proxy(Object(), {
+export const env:
+  & InstanceType<ObjectConstructor>
+  & Record<string | symbol, string> = new Proxy(Object(), {
     get: (target, prop) => {
       if (typeof prop === "symbol") {
         return target[prop];
@@ -93,15 +100,69 @@ export const env: InstanceType<ObjectConstructor> & Record<string, string> =
         };
       }
     },
-    set(_target, prop, value) {
+    set(target, prop, value) {
+      if (typeof prop === "symbol") {
+        target[prop] = value;
+        return true;
+      }
+
+      if (typeof value !== "string") {
+        nodeProcess ??= loadProcess();
+        nodeProcess.emitWarning(
+          "Assigning any value other than a string, number, or boolean to a " +
+            "process.env property is deprecated. Please make sure to convert the value " +
+            "to a string before setting process.env with it.",
+          "DeprecationWarning",
+          "DEP0104",
+        );
+      }
+
       Deno.env.set(String(prop), String(value));
       return true; // success
     },
-    has: (_target, prop) => typeof denoEnvGet(String(prop)) === "string",
-    deleteProperty(_target, key) {
+    has: (target, prop) => {
+      if (typeof prop === "symbol") {
+        return ReflectHas(target, prop);
+      }
+
+      return typeof denoEnvGet(prop) === "string";
+    },
+    deleteProperty(target, key) {
+      if (typeof key === "symbol") {
+        delete target[key];
+        return true;
+      }
+
       Deno.env.delete(String(key));
       return true;
     },
+    defineProperty(target, property, attributes) {
+      if (attributes?.get || attributes?.set) {
+        throw new ERR_INVALID_OBJECT_DEFINE_PROPERTY(
+          "'process.env' does not accept an " +
+            "accessor(getter/setter) descriptor",
+        );
+      }
+
+      if (
+        !attributes?.configurable || !attributes?.enumerable ||
+        !attributes?.writable
+      ) {
+        throw new ERR_INVALID_OBJECT_DEFINE_PROPERTY(
+          "'process.env' only accepts a " +
+            "configurable, writable," +
+            " and enumerable data descriptor",
+        );
+      }
+
+      if (typeof property === "symbol") {
+        ReflectDefineProperty(target, property, attributes);
+        return true;
+      }
+
+      Deno.env.set(String(property), String(attributes?.value));
+      return true;
+    },
   });
 
 /**

filename: ext/node/polyfills/internal/errors.ts
patch: @@ -1612,6 +1612,14 @@ export class ERR_INVALID_IP_ADDRESS extends NodeTypeError {
     super("ERR_INVALID_IP_ADDRESS", `Invalid IP address: ${x}`);
   }
 }
+export class ERR_INVALID_OBJECT_DEFINE_PROPERTY extends NodeTypeError {
+  constructor(message: string) {
+    super(
+      "ERR_INVALID_OBJECT_DEFINE_PROPERTY",
+      message,
+    );
+  }
+}
 export class ERR_INVALID_OPT_VALUE_ENCODING extends NodeTypeError {
   constructor(x: string) {
     super(
@@ -3028,6 +3036,7 @@ export default {
   ERR_INVALID_HTTP_TOKEN,
   ERR_INVALID_IP_ADDRESS,
   ERR_INVALID_MODULE_SPECIFIER,
+  ERR_INVALID_OBJECT_DEFINE_PROPERTY,
   ERR_INVALID_OPT_VALUE,
   ERR_INVALID_OPT_VALUE_ENCODING,
   ERR_INVALID_PACKAGE_CONFIG,

filename: tests/node_compat/config.toml
patch: @@ -878,6 +878,8 @@
 "parallel/test-process-emitwarning.js" = {}
 "parallel/test-process-env-allowed-flags.js" = {}
 "parallel/test-process-env-delete.js" = {}
+"parallel/test-process-env-deprecation.js" = {}
+"parallel/test-process-env-ignore-getter-setter.js" = {}
 "parallel/test-process-env-sideeffects.js" = {}
 "parallel/test-process-env-tz.js" = {}
 "parallel/test-process-env-windows-error-reset.js" = {}

filename: tests/unit_node/process_test.ts
patch: @@ -414,6 +414,14 @@ Deno.test({
       ),
     );
 
+    Object.defineProperty(process.env, "HELLO", {
+      value: "OTHER_WORLD",
+      configurable: true,
+      writable: true,
+      enumerable: true,
+    });
+    assertEquals(process.env.HELLO, "OTHER_WORLD");
+
     // deno-lint-ignore no-prototype-builtins
     assert(process.env.hasOwnProperty("HELLO"));
     assert("HELLO" in process.env);
@@ -475,6 +483,42 @@ Deno.test({
   },
 });
 
+Deno.test({
+  "name": "process.env: checking symbol in env should not require permission",
+  permissions: "none",
+  fn() {
+    const symbol = Symbol.for("67");
+    Reflect.has(globalThis.process.env, symbol);
+  },
+});
+
+Deno.test({
+  // NB(Tango992): Node.js does not support using symbols as env keys,
+  // thus this test should be omitted once we align with Node.js behavior.
+  name: "process.env: setting and getting a symbol key",
+  fn() {
+    const symbol = Symbol.for("foo");
+    // @ts-expect-error setting a symbol key
+    process.env[symbol] = "foo";
+    // @ts-expect-error getting a symbol key
+    assertEquals(process.env[symbol], "foo");
+    assert(Reflect.has(process.env, symbol));
+
+    // @ts-expect-error deleting a symbol key
+    delete process.env[symbol];
+    assertFalse(Reflect.has(process.env, symbol));
+
+    Object.defineProperty(process.env, symbol, {
+      value: "bar",
+      configurable: true,
+      writable: true,
+      enumerable: true,
+    });
+    // @ts-expect-error getting a symbol key
+    assertEquals(process.env[symbol], "bar");
+  },
+});
+
 Deno.test({
   name: "process.stdin",
   fn() {