From b946c3a7adcfd7c808663d987be96e869705591b Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Wed, 13 Apr 2022 23:08:34 +1200
Subject: [PATCH 01/10] Proof of concept of pollution protection

---
 index.js | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/index.js b/index.js
index 9e25e05..55b733e 100644
--- a/index.js
+++ b/index.js
@@ -124,11 +124,17 @@ function storeOption({
   }
 }
 
-const parseArgs = ({
-  args = getMainArgs(),
-  strict = true,
-  options = {}
-} = {}) => {
+// ToDo: move to utils.js
+function objectGetOwn(obj, prop) {
+  if (ObjectHasOwn(obj, prop))
+    return obj[prop];
+}
+
+const parseArgs = (config = {}) => {
+  const args = objectGetOwn(config, 'args') ?? getMainArgs();
+  const strict = objectGetOwn(config, 'strict') ?? true;
+  const options = objectGetOwn(config, 'options') ?? {};
+
   validateArray(args, 'args');
   validateBoolean(strict, 'strict');
   validateObject(options, 'options');
@@ -136,8 +142,7 @@ const parseArgs = ({
     ObjectEntries(options),
     ({ 0: longOption, 1: optionConfig }) => {
       validateObject(optionConfig, `options.${longOption}`);
-
-      validateUnion(optionConfig.type, `options.${longOption}.type`, ['string', 'boolean']);
+      validateUnion(objectGetOwn(optionConfig, 'type'), `options.${longOption}.type`, ['string', 'boolean']);
 
       if (ObjectHasOwn(optionConfig, 'short')) {
         const shortOption = optionConfig.short;

From fc9ff43eee8067d6042cb30463808dfd920a9b1c Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Thu, 14 Apr 2022 20:46:46 +1200
Subject: [PATCH 02/10] Update index.js

Co-authored-by: Jordan Harband <ljharb@gmail.com>
---
 index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/index.js b/index.js
index 55b733e..67cf17d 100644
--- a/index.js
+++ b/index.js
@@ -133,7 +133,7 @@ function objectGetOwn(obj, prop) {
 const parseArgs = (config = {}) => {
   const args = objectGetOwn(config, 'args') ?? getMainArgs();
   const strict = objectGetOwn(config, 'strict') ?? true;
-  const options = objectGetOwn(config, 'options') ?? {};
+  const options = objectGetOwn(config, 'options') ?? { __proto__: null };
 
   validateArray(args, 'args');
   validateBoolean(strict, 'strict');

From 6d2351a60c175b951546022d4e20fe8c07b3d6d4 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Thu, 14 Apr 2022 23:48:18 +1200
Subject: [PATCH 03/10] Tighten up index.js code against prototype pollution

---
 index.js | 121 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 57 insertions(+), 64 deletions(-)

diff --git a/index.js b/index.js
index 67cf17d..5c97121 100644
--- a/index.js
+++ b/index.js
@@ -74,67 +74,66 @@ function getMainArgs() {
   return ArrayPrototypeSlice(process.argv, 2);
 }
 
-const protoKey = '__proto__';
-
-function storeOption({
-  strict,
-  options,
-  result,
-  longOption,
-  shortOption,
-  optionValue,
-}) {
-  const hasOptionConfig = ObjectHasOwn(options, longOption);
-  const optionConfig = hasOptionConfig ? options[longOption] : {};
+// ToDo: move to utils.js
+function objectGetOwn(obj, prop) {
+  if (ObjectHasOwn(obj, prop))
+    return obj[prop];
+}
 
-  if (strict) {
-    if (!hasOptionConfig) {
-      throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOption == null ? `--${longOption}` : `-${shortOption}`);
-    }
+function optionsGetOwn(options, longOption, prop) {
+  if (ObjectHasOwn(options, longOption))
+    return objectGetOwn(options[longOption], prop);
+}
 
-    const shortOptionErr = ObjectHasOwn(optionConfig, 'short') ? `-${optionConfig.short}, ` : '';
+function checkOptionUsage(longOption, optionValue, options,
+                          shortOrLong, strict) {
+  // Strict and options are used from local context.
+  if (!strict) return;
 
-    if (options[longOption].type === 'string' && optionValue == null) {
-      throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption} <value>' argument missing`);
-    }
+  if (!ObjectHasOwn(options, longOption)) {
+    throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOrLong);
+  }
 
-    if (options[longOption].type === 'boolean' && optionValue != null) {
-      throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption}' does not take an argument`);
-    }
+  const short = optionsGetOwn(options, longOption, 'short');
+  const shortAndLong = short ? `-${short}, --${longOption}` : `--${longOption}`;
+  const type = optionsGetOwn(options, longOption, 'type');
+  if (type === 'string' && typeof optionValue !== 'string') {
+    throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong} <value>' argument missing`);
   }
+  // (Idiomatic test for undefined||null, expecting undefined.)
+  if (type === 'boolean' && optionValue != null) {
+    throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong}' does not take an argument`);
+  }
+}
 
-  if (longOption === protoKey) {
+function storeOption(longOption, optionValue, options, values) {
+  if (longOption === '__proto__') {
     return;
   }
-
-  // Values
-  const usedAsFlag = optionValue === undefined;
-  const newValue = usedAsFlag ? true : optionValue;
-  if (optionConfig.multiple) {
-    // Always store value in array, including for flags.
-    // result.values[longOption] starts out not present,
+  // We store based on the option value rather than option type,
+  // preserving the users intent for author to deal with.
+  const newValue = optionValue ?? true;
+  if (optionsGetOwn(options, longOption, 'multiple')) {
+    // Always store value in array, including for boolean.
+    // values[longOption] starts out not present,
     // first value is added as new array [newValue],
     // subsequent values are pushed to existing array.
-    if (result.values[longOption] !== undefined)
-      ArrayPrototypePush(result.values[longOption], newValue);
-    else
-      result.values[longOption] = [newValue];
+    if (ObjectHasOwn(values, longOption)) {
+      ArrayPrototypePush(values[longOption], newValue);
+    } else {
+      values[longOption] = [newValue];
+    }
   } else {
-    result.values[longOption] = newValue;
+    values[longOption] = newValue;
   }
 }
 
-// ToDo: move to utils.js
-function objectGetOwn(obj, prop) {
-  if (ObjectHasOwn(obj, prop))
-    return obj[prop];
-}
-
-const parseArgs = (config = {}) => {
+const parseArgs = (config = { __proto__: null }) => {
   const args = objectGetOwn(config, 'args') ?? getMainArgs();
   const strict = objectGetOwn(config, 'strict') ?? true;
   const options = objectGetOwn(config, 'options') ?? { __proto__: null };
 
+  // Validate input configuration.
   validateArray(args, 'args');
   validateBoolean(strict, 'strict');
   validateObject(options, 'options');
@@ -142,6 +141,8 @@ const parseArgs = (config = {}) => {
     ObjectEntries(options),
     ({ 0: longOption, 1: optionConfig }) => {
       validateObject(optionConfig, `options.${longOption}`);
+
+      // type is required
       validateUnion(objectGetOwn(optionConfig, 'type'), `options.${longOption}.type`, ['string', 'boolean']);
 
       if (ObjectHasOwn(optionConfig, 'short')) {
@@ -188,18 +189,13 @@ const parseArgs = (config = {}) => {
       const shortOption = StringPrototypeCharAt(arg, 1);
       const longOption = findLongOptionForShort(shortOption, options);
       let optionValue;
-      if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) {
+      if (optionsGetOwn(options, longOption, 'type') === 'string' &&
+          isOptionValue(nextArg)) {
         // e.g. '-f', 'bar'
         optionValue = ArrayPrototypeShift(remainingArgs);
       }
-      storeOption({
-        strict,
-        options,
-        result,
-        longOption,
-        shortOption,
-        optionValue,
-      });
+      checkOptionUsage(longOption, optionValue, options, arg, strict);
+      storeOption(longOption, optionValue, options, result.values);
       continue;
     }
 
@@ -209,7 +205,7 @@ const parseArgs = (config = {}) => {
       for (let index = 1; index < arg.length; index++) {
         const shortOption = StringPrototypeCharAt(arg, index);
         const longOption = findLongOptionForShort(shortOption, options);
-        if (options[longOption]?.type !== 'string' ||
+        if (optionsGetOwn(options, longOption, 'type') !== 'string' ||
           index === arg.length - 1) {
           // Boolean option, or last short in group. Well formed.
           ArrayPrototypePush(expanded, `-${shortOption}`);
@@ -229,14 +225,8 @@ const parseArgs = (config = {}) => {
       const shortOption = StringPrototypeCharAt(arg, 1);
       const longOption = findLongOptionForShort(shortOption, options);
       const optionValue = StringPrototypeSlice(arg, 2);
-      storeOption({
-        strict,
-        options,
-        result,
-        longOption,
-        shortOption,
-        optionValue,
-      });
+      checkOptionUsage(longOption, optionValue, options, `-${shortOption}`, strict);
+      storeOption(longOption, optionValue, options, result.values);
       continue;
     }
 
@@ -244,11 +234,13 @@ const parseArgs = (config = {}) => {
       // e.g. '--foo'
       const longOption = StringPrototypeSlice(arg, 2);
       let optionValue;
-      if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) {
+      if (optionsGetOwn(options, longOption, 'type') === 'string' &&
+          isOptionValue(nextArg)) {
         // e.g. '--foo', 'bar'
         optionValue = ArrayPrototypeShift(remainingArgs);
       }
-      storeOption({ strict, options, result, longOption, optionValue });
+      checkOptionUsage(longOption, optionValue, options, arg, strict);
+      storeOption(longOption, optionValue, options, result.values);
       continue;
     }
 
@@ -257,7 +249,8 @@ const parseArgs = (config = {}) => {
       const index = StringPrototypeIndexOf(arg, '=');
       const longOption = StringPrototypeSlice(arg, 2, index);
       const optionValue = StringPrototypeSlice(arg, index + 1);
-      storeOption({ strict, options, result, longOption, optionValue });
+      checkOptionUsage(longOption, optionValue, options, `--${longOption}`, strict);
+      storeOption(longOption, optionValue, options, result.values);
       continue;
     }
 

From 9c7d5d46ad41f6c0080691ee5794845bce1b0c43 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 09:52:20 +1200
Subject: [PATCH 04/10] Move helpers to utils

---
 index.js | 15 +++------------
 utils.js | 21 ++++++++++++++++++++-
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/index.js b/index.js
index 5c97121..efeb6f9 100644
--- a/index.js
+++ b/index.js
@@ -29,7 +29,9 @@ const {
   isLongOptionAndValue,
   isOptionValue,
   isShortOptionAndValue,
-  isShortOptionGroup
+  isShortOptionGroup,
+  objectGetOwn,
+  optionsGetOwn
 } = require('./utils');
 
 const {
@@ -74,17 +76,6 @@ function getMainArgs() {
   return ArrayPrototypeSlice(process.argv, 2);
 }
 
-// ToDo: move to utils.js
-function objectGetOwn(obj, prop) {
-  if (ObjectHasOwn(obj, prop))
-    return obj[prop];
-}
-
-function optionsGetOwn(options, longOption, prop) {
-  if (ObjectHasOwn(options, longOption))
-    return objectGetOwn(options[longOption], prop);
-}
-
 function checkOptionUsage(longOption, optionValue, options,
                           shortOrLong, strict) {
   // Strict and options are used from local context.
diff --git a/utils.js b/utils.js
index b718c01..bc55863 100644
--- a/utils.js
+++ b/utils.js
@@ -3,6 +3,7 @@
 const {
   ArrayPrototypeFind,
   ObjectEntries,
+  ObjectPrototypeHasOwnProperty: ObjectHasOwn,
   StringPrototypeCharAt,
   StringPrototypeIncludes,
   StringPrototypeSlice,
@@ -20,6 +21,22 @@ const {
 //
 // These routines are for internal use, not for export to client.
 
+/**
+ * Return the named property, but only if it is an own property.
+ */
+function objectGetOwn(obj, prop) {
+  if (ObjectHasOwn(obj, prop))
+    return obj[prop];
+}
+
+/**
+ * Return the named options property, but only if it is an own property.
+ */
+function optionsGetOwn(options, longOption, prop) {
+  if (ObjectHasOwn(options, longOption))
+    return objectGetOwn(options[longOption], prop);
+}
+
 /**
  * Determines if the argument may be used as an option value.
  * NB: We are choosing not to accept option-ish arguments.
@@ -156,5 +173,7 @@ module.exports = {
   isLongOptionAndValue,
   isOptionValue,
   isShortOptionAndValue,
-  isShortOptionGroup
+  isShortOptionGroup,
+  objectGetOwn,
+  optionsGetOwn
 };

From 54e221a49d26c2e8457b246d81c647889b56013b Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 10:06:26 +1200
Subject: [PATCH 05/10] Make utils more robust to pollution

---
 utils.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/utils.js b/utils.js
index bc55863..739f49d 100644
--- a/utils.js
+++ b/utils.js
@@ -124,7 +124,7 @@ function isShortOptionGroup(arg, options) {
 
   const firstShort = StringPrototypeCharAt(arg, 1);
   const longOption = findLongOptionForShort(firstShort, options);
-  return options[longOption]?.type !== 'string';
+  return optionsGetOwn(options, longOption, 'type') !== 'string';
 }
 
 /**
@@ -145,7 +145,7 @@ function isShortOptionAndValue(arg, options) {
 
   const shortOption = StringPrototypeCharAt(arg, 1);
   const longOption = findLongOptionForShort(shortOption, options);
-  return options[longOption]?.type === 'string';
+  return optionsGetOwn(options, longOption, 'type') === 'string';
 }
 
 /**
@@ -161,7 +161,7 @@ function findLongOptionForShort(shortOption, options) {
   validateObject(options, 'options');
   const { 0: longOption } = ArrayPrototypeFind(
     ObjectEntries(options),
-    ({ 1: optionConfig }) => optionConfig.short === shortOption
+    ({ 1: optionConfig }) => objectGetOwn(optionConfig, 'short') === shortOption
   ) || [];
   return longOption || shortOption;
 }

From 3ce5a47f1b5e1768f1b3473e004631d6f881f67a Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 11:05:12 +1200
Subject: [PATCH 06/10] Likely temporary ObjectDefineProperty paranoia

---
 index.js | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/index.js b/index.js
index efeb6f9..afcfae0 100644
--- a/index.js
+++ b/index.js
@@ -6,8 +6,9 @@ const {
   ArrayPrototypeShift,
   ArrayPrototypeSlice,
   ArrayPrototypePush,
-  ObjectPrototypeHasOwnProperty: ObjectHasOwn,
+  ObjectDefineProperty,
   ObjectEntries,
+  ObjectPrototypeHasOwnProperty: ObjectHasOwn,
   StringPrototypeCharAt,
   StringPrototypeIncludes,
   StringPrototypeIndexOf,
@@ -101,6 +102,17 @@ function storeOption(longOption, optionValue, options, values) {
   if (longOption === '__proto__') {
     return;
   }
+
+  // Can be removed when value has a null prototype
+  const safeAssignProperty = (obj, prop, value) => {
+    ObjectDefineProperty(obj, prop, {
+      value,
+      writable: true,
+      enumerable: true,
+      configurable: true
+    });
+  };
+
   // We store based on the option value rather than option type,
   // preserving the users intent for author to deal with.
   const newValue = optionValue ?? true;
@@ -112,10 +124,10 @@ function storeOption(longOption, optionValue, options, values) {
     if (ObjectHasOwn(values, longOption)) {
       ArrayPrototypePush(values[longOption], newValue);
     } else {
-      values[longOption] = [newValue];
+      safeAssignProperty(values, longOption, [newValue]);
     }
   } else {
-    values[longOption] = newValue;
+    safeAssignProperty(values, longOption, newValue);
   }
 }
 

From ec4a81da501540fd199ca21e8620bbc414a099d0 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 14:14:15 +1200
Subject: [PATCH 07/10] Add JSDoc for Check and Store

---
 index.js | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/index.js b/index.js
index afcfae0..ac5b0ad 100644
--- a/index.js
+++ b/index.js
@@ -77,6 +77,15 @@ function getMainArgs() {
   return ArrayPrototypeSlice(process.argv, 2);
 }
 
+/**
+ * In strict mode, throw for usage errors.
+ *
+ * @param {string} longOption - long option name e.g. 'foo'
+ * @param {string|undefined} optionValue - value from user args
+ * @param {Object} options - option configs, from parseArgs({ options })
+ * @param {string} shortOrLong - option used, with dashes e.g. `-l` or `--long`
+ * @param {boolean} strict - show errors, from parseArgs({ strict })
+ */
 function checkOptionUsage(longOption, optionValue, options,
                           shortOrLong, strict) {
   // Strict and options are used from local context.
@@ -98,6 +107,14 @@ function checkOptionUsage(longOption, optionValue, options,
   }
 }
 
+/**
+ * Store the option value in `values`.
+ *
+ * @param {string} longOption - long option name e.g. 'foo'
+ * @param {string|undefined} optionValue - value from user args
+ * @param {Object} options - option configs, from parseArgs({ options })
+ * @param {Object} values - option values returned in `values` by parseArgs
+ */
 function storeOption(longOption, optionValue, options, values) {
   if (longOption === '__proto__') {
     return;

From 501c63163d90e7a022f6d7613cb6b06825ef3f16 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 14:50:53 +1200
Subject: [PATCH 08/10] Add pollution tests

---
 test/pollution.js | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)
 create mode 100644 test/pollution.js

diff --git a/test/pollution.js b/test/pollution.js
new file mode 100644
index 0000000..aa1467d
--- /dev/null
+++ b/test/pollution.js
@@ -0,0 +1,76 @@
+'use strict';
+/* eslint max-len: 0 */
+
+const test = require('tape');
+const { parseArgs } = require('../index.js');
+
+// These tests are not synced upstream with node, as hacking global prototypes.
+
+function setObjectPrototype(prop, value) {
+  const oldValue = Object.prototype[prop];
+  Object.prototype[prop] = value;
+  return oldValue;
+}
+
+function restoreObjectPrototype(prop, oldValue) {
+  if (oldValue == null) {
+    delete Object.prototype[prop];
+  } else {
+    Object.prototype[prop] = oldValue;
+  }
+}
+
+test('when prototype has multiple then ignored', (t) => {
+  const args = ['--foo', '1', '--foo', '2'];
+  const options = { foo: { type: 'string' } };
+  const expectedResult = { values: { foo: '2' }, positionals: [] };
+
+  const holdValue = setObjectPrototype('multiple', true);
+  const result = parseArgs({ args, options });
+  restoreObjectPrototype('multiple', holdValue);
+  t.deepEqual(result, expectedResult);
+  t.end();
+});
+
+test('when prototype has type then ignored', (t) => {
+  const args = ['--foo', '1'];
+  const options = { foo: { } };
+
+  const holdValue = setObjectPrototype('type', 'string');
+  t.throws(() => {
+    parseArgs({ args, options });
+  });
+  restoreObjectPrototype('type', holdValue);
+  t.end();
+});
+
+test('when prototype has short then ignored', (t) => {
+  const args = ['-f', '1'];
+  const options = { foo: { type: 'string' } };
+
+  const holdValue = setObjectPrototype('short', 'f');
+  t.throws(() => {
+    parseArgs({ args, options });
+  });
+  restoreObjectPrototype('short', holdValue);
+  t.end();
+});
+
+test('when prototype has strict then ignored', (t) => {
+  const args = ['-f'];
+
+  const holdValue = setObjectPrototype('strict', false);
+  t.throws(() => {
+    parseArgs({ args });
+  });
+  restoreObjectPrototype('strict', holdValue);
+  t.end();
+});
+
+test('when prototype has args then ignored', (t) => {
+  const holdValue = setObjectPrototype('args', ['--foo']);
+  const result = parseArgs({ strict: false });
+  restoreObjectPrototype('args', holdValue);
+  t.false(result.values.foo);
+  t.end();
+});

From c6849162c2bf2e86b477544625bf7a718ce77153 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 15:06:48 +1200
Subject: [PATCH 09/10] Combined test files

---
 test/pollution.js           | 76 -------------------------------------
 test/prototype-pollution.js | 72 +++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 76 deletions(-)
 delete mode 100644 test/pollution.js

diff --git a/test/pollution.js b/test/pollution.js
deleted file mode 100644
index aa1467d..0000000
--- a/test/pollution.js
+++ /dev/null
@@ -1,76 +0,0 @@
-'use strict';
-/* eslint max-len: 0 */
-
-const test = require('tape');
-const { parseArgs } = require('../index.js');
-
-// These tests are not synced upstream with node, as hacking global prototypes.
-
-function setObjectPrototype(prop, value) {
-  const oldValue = Object.prototype[prop];
-  Object.prototype[prop] = value;
-  return oldValue;
-}
-
-function restoreObjectPrototype(prop, oldValue) {
-  if (oldValue == null) {
-    delete Object.prototype[prop];
-  } else {
-    Object.prototype[prop] = oldValue;
-  }
-}
-
-test('when prototype has multiple then ignored', (t) => {
-  const args = ['--foo', '1', '--foo', '2'];
-  const options = { foo: { type: 'string' } };
-  const expectedResult = { values: { foo: '2' }, positionals: [] };
-
-  const holdValue = setObjectPrototype('multiple', true);
-  const result = parseArgs({ args, options });
-  restoreObjectPrototype('multiple', holdValue);
-  t.deepEqual(result, expectedResult);
-  t.end();
-});
-
-test('when prototype has type then ignored', (t) => {
-  const args = ['--foo', '1'];
-  const options = { foo: { } };
-
-  const holdValue = setObjectPrototype('type', 'string');
-  t.throws(() => {
-    parseArgs({ args, options });
-  });
-  restoreObjectPrototype('type', holdValue);
-  t.end();
-});
-
-test('when prototype has short then ignored', (t) => {
-  const args = ['-f', '1'];
-  const options = { foo: { type: 'string' } };
-
-  const holdValue = setObjectPrototype('short', 'f');
-  t.throws(() => {
-    parseArgs({ args, options });
-  });
-  restoreObjectPrototype('short', holdValue);
-  t.end();
-});
-
-test('when prototype has strict then ignored', (t) => {
-  const args = ['-f'];
-
-  const holdValue = setObjectPrototype('strict', false);
-  t.throws(() => {
-    parseArgs({ args });
-  });
-  restoreObjectPrototype('strict', holdValue);
-  t.end();
-});
-
-test('when prototype has args then ignored', (t) => {
-  const holdValue = setObjectPrototype('args', ['--foo']);
-  const result = parseArgs({ strict: false });
-  restoreObjectPrototype('args', holdValue);
-  t.false(result.values.foo);
-  t.end();
-});
diff --git a/test/prototype-pollution.js b/test/prototype-pollution.js
index 3f28f59..d9e8c6a 100644
--- a/test/prototype-pollution.js
+++ b/test/prototype-pollution.js
@@ -4,6 +4,23 @@
 const test = require('tape');
 const { parseArgs } = require('../index.js');
 
+// These tests are not synced upstream with node, in case of possible side-effects.
+// See index.js for tests shared with upstream.
+
+function setObjectPrototype(prop, value) {
+  const oldValue = Object.prototype[prop];
+  Object.prototype[prop] = value;
+  return oldValue;
+}
+
+function restoreObjectPrototype(prop, oldValue) {
+  if (oldValue == null) {
+    delete Object.prototype[prop];
+  } else {
+    Object.prototype[prop] = oldValue;
+  }
+}
+
 test('should not allow __proto__ key to be set on object', (t) => {
   const passedArgs = ['--__proto__=hello'];
   const expected = { values: {}, positionals: [] };
@@ -13,3 +30,58 @@ test('should not allow __proto__ key to be set on object', (t) => {
   t.deepEqual(result, expected);
   t.end();
 });
+
+test('when prototype has multiple then ignored', (t) => {
+  const args = ['--foo', '1', '--foo', '2'];
+  const options = { foo: { type: 'string' } };
+  const expectedResult = { values: { foo: '2' }, positionals: [] };
+
+  const holdValue = setObjectPrototype('multiple', true);
+  const result = parseArgs({ args, options });
+  restoreObjectPrototype('multiple', holdValue);
+  t.deepEqual(result, expectedResult);
+  t.end();
+});
+
+test('when prototype has type then ignored', (t) => {
+  const args = ['--foo', '1'];
+  const options = { foo: { } };
+
+  const holdValue = setObjectPrototype('type', 'string');
+  t.throws(() => {
+    parseArgs({ args, options });
+  });
+  restoreObjectPrototype('type', holdValue);
+  t.end();
+});
+
+test('when prototype has short then ignored', (t) => {
+  const args = ['-f', '1'];
+  const options = { foo: { type: 'string' } };
+
+  const holdValue = setObjectPrototype('short', 'f');
+  t.throws(() => {
+    parseArgs({ args, options });
+  });
+  restoreObjectPrototype('short', holdValue);
+  t.end();
+});
+
+test('when prototype has strict then ignored', (t) => {
+  const args = ['-f'];
+
+  const holdValue = setObjectPrototype('strict', false);
+  t.throws(() => {
+    parseArgs({ args });
+  });
+  restoreObjectPrototype('strict', holdValue);
+  t.end();
+});
+
+test('when prototype has args then ignored', (t) => {
+  const holdValue = setObjectPrototype('args', ['--foo']);
+  const result = parseArgs({ strict: false });
+  restoreObjectPrototype('args', holdValue);
+  t.false(result.values.foo);
+  t.end();
+});

From 99768b543e2418ca65714eca6af53d6d0ef382c8 Mon Sep 17 00:00:00 2001
From: John Gee <john@ruru.gen.nz>
Date: Fri, 15 Apr 2022 15:35:17 +1200
Subject: [PATCH 10/10] Make set/restore prototype do more faithful restore

---
 test/prototype-pollution.js | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/test/prototype-pollution.js b/test/prototype-pollution.js
index d9e8c6a..f1c82c4 100644
--- a/test/prototype-pollution.js
+++ b/test/prototype-pollution.js
@@ -8,16 +8,16 @@ const { parseArgs } = require('../index.js');
 // See index.js for tests shared with upstream.
 
 function setObjectPrototype(prop, value) {
-  const oldValue = Object.prototype[prop];
+  const oldDescriptor = Object.getOwnPropertyDescriptor(Object.prototype, prop);
   Object.prototype[prop] = value;
-  return oldValue;
+  return oldDescriptor;
 }
 
-function restoreObjectPrototype(prop, oldValue) {
-  if (oldValue == null) {
+function restoreObjectPrototype(prop, oldDescriptor) {
+  if (oldDescriptor == null) {
     delete Object.prototype[prop];
   } else {
-    Object.prototype[prop] = oldValue;
+    Object.defineProperty(Object.prototype, prop, oldDescriptor);
   }
 }
 
@@ -36,9 +36,9 @@ test('when prototype has multiple then ignored', (t) => {
   const options = { foo: { type: 'string' } };
   const expectedResult = { values: { foo: '2' }, positionals: [] };
 
-  const holdValue = setObjectPrototype('multiple', true);
+  const holdDescriptor = setObjectPrototype('multiple', true);
   const result = parseArgs({ args, options });
-  restoreObjectPrototype('multiple', holdValue);
+  restoreObjectPrototype('multiple', holdDescriptor);
   t.deepEqual(result, expectedResult);
   t.end();
 });
@@ -47,11 +47,11 @@ test('when prototype has type then ignored', (t) => {
   const args = ['--foo', '1'];
   const options = { foo: { } };
 
-  const holdValue = setObjectPrototype('type', 'string');
+  const holdDescriptor = setObjectPrototype('type', 'string');
   t.throws(() => {
     parseArgs({ args, options });
   });
-  restoreObjectPrototype('type', holdValue);
+  restoreObjectPrototype('type', holdDescriptor);
   t.end();
 });
 
@@ -59,29 +59,29 @@ test('when prototype has short then ignored', (t) => {
   const args = ['-f', '1'];
   const options = { foo: { type: 'string' } };
 
-  const holdValue = setObjectPrototype('short', 'f');
+  const holdDescriptor = setObjectPrototype('short', 'f');
   t.throws(() => {
     parseArgs({ args, options });
   });
-  restoreObjectPrototype('short', holdValue);
+  restoreObjectPrototype('short', holdDescriptor);
   t.end();
 });
 
 test('when prototype has strict then ignored', (t) => {
   const args = ['-f'];
 
-  const holdValue = setObjectPrototype('strict', false);
+  const holdDescriptor = setObjectPrototype('strict', false);
   t.throws(() => {
     parseArgs({ args });
   });
-  restoreObjectPrototype('strict', holdValue);
+  restoreObjectPrototype('strict', holdDescriptor);
   t.end();
 });
 
 test('when prototype has args then ignored', (t) => {
-  const holdValue = setObjectPrototype('args', ['--foo']);
+  const holdDescriptor = setObjectPrototype('args', ['--foo']);
   const result = parseArgs({ strict: false });
-  restoreObjectPrototype('args', holdValue);
+  restoreObjectPrototype('args', holdDescriptor);
   t.false(result.values.foo);
   t.end();
 });