Robustness: rework isConstructorOrProto

[shadowspawn]

The isConstructorOrProto fix does not look right for the v0.2.x branch. I looked at various commits and suspect the back port of the fix itself went somewhat awry. This PR makes the code match the mainline.

Here are the relevant lines of code from the main branch and from the original commit and adapted commit adding isConstructorOrProto.

Main line

• minimist/index.js

  Lines 19 to 21 in 6901ee2

  	function isConstructorOrProto(obj, key) {
  	return (key === 'constructor' && typeof obj[key] === 'function') || key === '__proto__';
  	}

• minimist/index.js

  Line 85 in 6901ee2

  if (isConstructorOrProto(o, key)) { return; }

• minimist/index.js

  Line 99 in 6901ee2

  if (isConstructorOrProto(o, lastKey)) { return; }

Original change

• minimist/index.js

  Lines 247 to 249 in c2b9819

  	function isConstructorOrProto (obj, key) {
  	return key === 'constructor' && typeof obj[key] === 'function' || key === '__proto__';
  	}

• minimist/index.js

  Line 73 in c2b9819

  if (isConstructorOrProto(o, key)) return;

• minimist/index.js

  Line 82 in c2b9819

  if (isConstructorOrProto(o, key)) return;

Adapted change (suspect)

• minimist/index.js

  Lines 9 to 11 in ef9153f

  	function isConstructorOrProto(obj, key) {
  	return key === 'constructor' && (typeof obj[key] === 'function' || key === '__proto__');
  	}

• minimist/index.js

  Lines 28 to 30 in ef9153f

  	if (key === '__proto__' || isConstructorOrProto(o, key)) {
  	return;
  	}

• minimist/index.js

  Line 44 in ef9153f

  if (key === '__proto__') { return; }

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.55%. Comparing base (c0b2661) to head (3dbebff).
Report is 4 commits behind head on v0.2.x.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##           v0.2.x      #24   +/-   ##
=======================================
  Coverage   98.55%   98.55%           
=======================================
  Files           1        1           
  Lines         138      138           
  Branches       60       60           
=======================================
  Hits          136      136           
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ljharb]

Since this fails to handle constructor properly, could we add a failing test that would have caught this? Presumably the default branch lacks the same test (it'll get pulled in when i merge in the backport).

[ljharb]

woof, good catch

[ljharb]

at least this wasn't wrong for "proto", but now it's better :-)

[shadowspawn]

I'll look at adding tests for constructor, and the last key. (It took me a while to untangle the code, but I hopefully learnt enough on the way to write the tests now!)

[shadowspawn]

There are separate checks against the leading keys in a dotted option name than against the last key. If the checks for __proto__ or constructor are wrong in the checks of the last key then it won't lead to prototype pollution as such, but will lead to less consistent behaviour between --a.constructor.prototype.b and --a.constructor.

The pollution checks on the v0.2.x branch missed the constructor check on the last key in the dotted option name. So again this does not allow prototype pollution, but does mean there is different behaviour between the main branch and the v0.2.x branch.

This PR adds a test which fails if the constructor check is missing for the last key. (And a matching test for __proto__.)

The tests were run against the previous code in #25 to show the failure.

[shadowspawn]

To be clear, after testing I do not think this PR is a security fix. Just a tidy-up to make the dotted option key handling more consistent.