harden: if unverified user data can reach the exec me... in...

[anupamme]

Summary

Harden input handling in Utilities/DataGenerator/convert-cli.js (flagged by semgrep).

Vulnerability

Field	Value
ID	javascript.shelljs.security.shelljs-exec-injection.shelljs-exec-injection
Severity	HIGH
Scanner	semgrep
Rule	javascript.shelljs.security.shelljs-exec-injection.shelljs-exec-injection
File	Utilities/DataGenerator/convert-cli.js:68
Assessment	Defensive hardening

Description: If unverified user data can reach the exec method it can result in Remote Code Execution

Threat Model Context

This is a Node.js library - vulnerabilities affect downstream consumers who use this package.

Changes

• Utilities/DataGenerator/convert-cli.js

Behavior Preservation

The change is scoped to 1 file on the vulnerable path; it only tightens handling of untrusted input and leaves valid inputs unaffected.

---

This patch removes an exploit primitive — a code pattern that, while not independently exploitable today, could be chained with other weaknesses by automated exploit-development tooling. Proactive removal of such primitives raises the bar against increasingly capable automated attack tools.

---

Automated security fix by OrbisAI Security

[finetjul]

Please fix commit name

[anupamme]

done. pls review.

[finetjul]

Why not extend the reasoning to shell.find ?

[anupamme]

shell.find() is worth considering because --paraview can influence the paths it receives, but I don’t think it has the same command-injection sink semantics as shell.exec(). find() performs filesystem traversal rather than passing a constructed string to a system shell. The actual injection primitive was the cmdLine.join(' ') → shell.exec() path, which is now replaced with execFileSync() and preserves argument boundaries.

That said, I’ll add a regression test covering shell metacharacters in --input/--output to make the security property explicit.

[anupamme]

Added a regression test (Utilities/DataGenerator/convert-cli.security-test.js, runnable via npm run test:convert-cli-security) that spawns convert-cli.js against a stub pvpython and asserts shell metacharacters passed via --input/--output are never shell-interpreted and reach pvpython as a literal argv element — making the shell.exec() → execFileSync() security property explicit, as promised.

While wiring this test up end-to-end, I found that the CLI's help-check condition (!options.help) was always true — commander's opts() never populates a help key, so program.outputHelp(); process.exit(0); fired on every invocation, making the execFileSync calls (including the ones this PR just patched) unreachable dead code. This predates this PR (introduced in a commander v9 migration back in 2021) but I fixed it in a separate commit so the new regression test could actually exercise the real, patched code path rather than testing the pattern in isolation.