fix: use shell when running child_process.spawn

[mallachari]

What/Why/How?

CLI preview command was failing on Windows with specific Node versions.
Turns out there is a breaking change in Node API that prevents passing .cmd files to spawn function without using shell: true option. Since we're using npx.cmd shell option is now necessary.

Using shell: true brings an actual shell which introduces a risk of shell injection. This means that the arguments should be sanitized before passing. In our case those that require to be sanitized are strings not validated by yargs itself: paths and locale. For these values there are sanitize functions restricting input to the set of allowed characters, which significantly limits the possible risk (but also reduces characters that can be used in regular paths).

To limit the risk exposure shell: true is used only on windows platform.

Other options considered:

• using execFile instead of spawn - execFile doesn't spawn a shell which makes it safer, however it behaves differently and doesn't handle long running processes and user interaction like spawn does. eject use case is fine with execFile, but preview and translate won't work in this case
• escaping arguments - it would be fine if we were using single platform/shell, but in this case we need to support both unix and windows. Within windows we also have multiple shells (cmd, powershell, git-bash) each having different way of escaping commands. I didn't find any library that could cover all these cases and writing it ourselves would require shell detection on windows which might be hard to do (and not always work)
• checking if path exists - checking a path with fs.existsSync wouldn't prevent injections since a malformed path might be created (there are no limitations in characters)
• validating path format with regex - that still would need to include the whitelisted character set so it wouldn't bring additional protection

Reference

https://github.com/Redocly/redocly/issues/11284

Testing

Tested on Windows and MacOS with Node versions from before and after the change.

Screenshots (optional)

Check yourself

• Code changed? - Tested with redoc/reference-docs/workflows (internal)
• All new/updated code is covered with tests
• New package installed? - Tested in different environments (browser/node)

Security

• Security impact of change has been considered
• Code follows company security practices and guidelines

[changeset-bot]

🦋 Changeset detected

Latest commit: 3707e9d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@redocly/cli	Patch
@redocly/openapi-core	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Command	Mean [s]	Min [s]	Max [s]	Relative
redocly lint packages/core/src/benchmark/benches/rebilly.yaml	1.010 ± 0.029	0.984	1.086	1.01 ± 0.04
redocly-next lint packages/core/src/benchmark/benches/rebilly.yaml	1.000 ± 0.027	0.971	1.073	1.00

[mallachari]

Should it include @redocly/openapi-core bump too?

[tatomyr]

No as all the changes are scoped to the cli package.

[github-actions]

Coverage report

❌ An unexpected error occurred. For more details, check console

Error: The process '/opt/hostedtoolcache/node/20.18.0/x64/bin/npm' failed with exit code 1

St.❔	Category	Percentage	Covered / Total
🟡	Statements	78.59%	5022/6390
🟡	Branches	67.16%	2076/3091
🟡	Functions	73.18%	832/1137
🟡	Lines	78.87%	4738/6007

Test suite run failed

Failed tests: 0/835. Failed suites: 0/123.

Report generated by 🧪jest coverage report action from 3707e9d

[mallachari]

Converting back to draft as I'd like to add some input sanitization