process: disallow adding options to process.allowedNodeEnvironmentFlags

Make no-op direct calls of `Set` prototype methods to
`process.allowedNodeEnvironmentFlags`.

```js
const { add } = Set.prototype;
add.call(process.allowedNodeEnvironmentFlags, '--user-option`);
process.allowedNodeEnvironmentFlags.has('--user-option') === false;
```

PR-URL: #36660
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: aduh95

lib/internal/process/per_thread.js

@@ -6,21 +6,27 @@
 
 const {
   ArrayPrototypeEvery,
+  ArrayPrototypeForEach,
+  ArrayPrototypeIncludes,
   ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeSplice,
   BigUint64Array,
   Float64Array,
   NumberMAX_SAFE_INTEGER,
-  ObjectDefineProperty,
   ObjectFreeze,
   ReflectApply,
   RegExpPrototypeTest,
-  SafeSet,
+  SafeArrayIterator,
+  Set,
+  SetPrototypeEntries,
+  SetPrototypeValues,
   StringPrototypeEndsWith,
   StringPrototypeReplace,
   StringPrototypeSlice,
   StringPrototypeStartsWith,
+  Symbol,
+  SymbolIterator,
   Uint32Array,
 } = primordials;
 
@@ -41,6 +47,8 @@ const {
 } = require('internal/validators');
 const constants = internalBinding('constants').os.signals;
 
+const kInternal = Symbol('internal properties');
+
 function assert(x, msg) {
   if (!x) throw new ERR_ASSERTION(msg || 'assertion error');
 }
@@ -293,18 +301,18 @@ function buildAllowedFlags() {
 
   // Save these for comparison against flags provided to
   // process.allowedNodeEnvironmentFlags.has() which lack leading dashes.
-  const nodeFlags = new SafeSet(ArrayPrototypeMap(allowedNodeEnvironmentFlags,
-                                                  trimLeadingDashes));
-
-  class NodeEnvironmentFlagsSet extends SafeSet {
-    constructor(...args) {
-      super(...args);
-
-      // The super constructor consumes `add`, but
-      // disallow any future adds.
-      ObjectDefineProperty(this, 'add', {
-        value: () => this
-      });
+  const nodeFlags = ArrayPrototypeMap(allowedNodeEnvironmentFlags,
+                                      trimLeadingDashes);
+
+  class NodeEnvironmentFlagsSet extends Set {
+    constructor(array) {
+      super();
+      this[kInternal] = { array };
+    }
+
+    add() {
+      // No-op, `Set` API compatible
+      return this;
     }
 
     delete() {
@@ -313,7 +321,7 @@ function buildAllowedFlags() {
     }
 
     clear() {
-      // No-op
+      // No-op, `Set` API compatible
     }
 
     has(key) {
@@ -328,13 +336,39 @@ function buildAllowedFlags() {
         key = StringPrototypeReplace(key, replaceUnderscoresRegex, '-');
         if (RegExpPrototypeTest(leadingDashesRegex, key)) {
           key = StringPrototypeReplace(key, trailingValuesRegex, '');
-          return super.has(key);
+          return ArrayPrototypeIncludes(this[kInternal].array, key);
         }
-        return nodeFlags.has(key);
+        return ArrayPrototypeIncludes(nodeFlags, key);
       }
       return false;
     }
+
+    entries() {
+      this[kInternal].set ??=
+        new Set(new SafeArrayIterator(this[kInternal].array));
+      return SetPrototypeEntries(this[kInternal].set);
+    }
+
+    forEach(callback, thisArg = undefined) {
+      ArrayPrototypeForEach(
+        this[kInternal].array,
+        (v) => ReflectApply(callback, thisArg, [v, v, this])
+      );
+    }
+
+    get size() {
+      return this[kInternal].array.length;
+    }
+
+    values() {
+      this[kInternal].set ??=
+        new Set(new SafeArrayIterator(this[kInternal].array));
+      return SetPrototypeValues(this[kInternal].set);
+    }
   }
+  NodeEnvironmentFlagsSet.prototype.keys =
+  NodeEnvironmentFlagsSet.prototype[SymbolIterator] =
+    NodeEnvironmentFlagsSet.prototype.values;
 
   ObjectFreeze(NodeEnvironmentFlagsSet.prototype.constructor);
   ObjectFreeze(NodeEnvironmentFlagsSet.prototype);

test/parallel/test-process-env-allowed-flags.js

@@ -1,6 +1,6 @@
 'use strict';
 
-require('../common');
+const common = require('../common');
 const assert = require('assert');
 
 // Assert legit flags are allowed, and bogus flags are disallowed
@@ -63,14 +63,41 @@ const assert = require('assert');
 
   process.allowedNodeEnvironmentFlags.add('foo');
   assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);
-  process.allowedNodeEnvironmentFlags.forEach((flag) => {
-    assert.strictEqual(flag === 'foo', false);
-  });
+  Set.prototype.add.call(process.allowedNodeEnvironmentFlags, 'foo');
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.has('foo'), false);
 
-  process.allowedNodeEnvironmentFlags.clear();
-  assert.strictEqual(process.allowedNodeEnvironmentFlags.size > 0, true);
+  const thisArg = {};
+  process.allowedNodeEnvironmentFlags.forEach(
+    common.mustCallAtLeast(function(flag, _, set) {
+      assert.notStrictEqual(flag, 'foo');
+      assert.strictEqual(this, thisArg);
+      assert.strictEqual(set, process.allowedNodeEnvironmentFlags);
+    }),
+    thisArg
+  );
+
+  for (const flag of process.allowedNodeEnvironmentFlags.keys()) {
+    assert.notStrictEqual(flag, 'foo');
+  }
+  for (const flag of process.allowedNodeEnvironmentFlags.values()) {
+    assert.notStrictEqual(flag, 'foo');
+  }
+  for (const flag of process.allowedNodeEnvironmentFlags) {
+    assert.notStrictEqual(flag, 'foo');
+  }
+  for (const [flag] of process.allowedNodeEnvironmentFlags.entries()) {
+    assert.notStrictEqual(flag, 'foo');
+  }
 
   const size = process.allowedNodeEnvironmentFlags.size;
+
+  process.allowedNodeEnvironmentFlags.clear();
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
+  Set.prototype.clear.call(process.allowedNodeEnvironmentFlags);
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
+
   process.allowedNodeEnvironmentFlags.delete('-r');
   assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
+  Set.prototype.delete.call(process.allowedNodeEnvironmentFlags, '-r');
+  assert.strictEqual(process.allowedNodeEnvironmentFlags.size, size);
 }