child_process: check array readability in spawn

[jazelly]

With previous example issues caused by prototype pollution, I don't think we should add a test, as this PR fixes it from hard crash, but does not guarantee a defined behaviour. For example,

const {exec} = require('node:child_process');

Object.defineProperty(Array.prototype, "2", {
  set: function () {},
});

(async function () {
  exec('pwd', (err, stdout, stderr) => {
    console.log(stdout);
  });
})();

gives

node:internal/util/inspect:2063
      totalLength += output[i].length;
                               ^

TypeError: Cannot read properties of undefined (reading 'length')
    at isBelowBreakLength (node:internal/util/inspect:2063:32)
    at reduceToSingleString (node:internal/util/inspect:2106:13)
    at formatRaw (node:internal/util/inspect:1138:15)
    at formatValue (node:internal/util/inspect:883:10)
    at Object.inspect (node:internal/util/inspect:386:10)
    at TypeError.<anonymous> (node:internal/errors:1441:45)
    at getMessage (node:internal/errors:598:12)
    at new NodeError (node:internal/errors:470:20)
    at stdioStringToArray (node:internal/child_process:253:13)
    at getValidStdio (node:internal/child_process:997:13)

Node.js v24.0.0-pre

In case we does check if array is readable in some other modules, the error would likely be different.

Fixes: #56531

[codecov]

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 89.20%. Comparing base (9230f22) to head (eed306c).
Report is 66 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/child_process.js	62.50%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #56625      +/-   ##
==========================================
+ Coverage   89.17%   89.20%   +0.03%     
==========================================
  Files         662      662              
  Lines      191761   191901     +140     
  Branches    36905    36937      +32     
==========================================
+ Hits       171006   171194     +188     
+ Misses      13620    13546      -74     
- Partials     7135     7161      +26     

Files with missing lines	Coverage Δ
lib/internal/child_process.js	94.75% <62.50%> (-0.23%)	⬇️

... and 78 files with indirect coverage changes

[lpinca]

Can you add a comment for future readers? FWIW, I think we should not "fix" this. There are probably other crashes like the one this is fixing if built-in prototypes are modified.

[jazelly]

yeah, I had the same thought. Sure I will add a comment.

Edit: leaving this unresolved for more input.

[targos]

I think a less brittle and easier to understand fix would be to initialize the array with let options; and then use options = [a, b, c] instead of ArrayPrototypePush

[github-actions]

Failed to start CI

   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/12894525741