refactor(tools/release): inline npm publish functionality into publishToNpm function

[anupamme]

Summary

Harden input handling in tools/release.cjs (flagged by semgrep).

Vulnerability

Field	Value
ID	javascript.lang.security.detect-child-process.detect-child-process
Severity	HIGH
Scanner	semgrep
Rule	javascript.lang.security.detect-child-process.detect-child-process
File	tools/release.cjs:481
Assessment	Defensive hardening

Description: Detected calls to child_process from a function argument file. This could lead to a command injection if the input is user controllable. Try to avoid calls to child_process, and if it is needed ensure user input is correctly sanitized or sandboxed.

Threat Model Context

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

Changes

• tools/release.cjs

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

[Copilot]

Pull request overview

Hardens the release script’s npm publishing subprocess invocation.

Changes:

• Restricts spawned commands to an allowlist.
• Explicitly disables shell execution.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[marcoscaceres]

if toSpawnPromise is only ever called with npm, the best way to secure it would be to rename toSpawnPromise to spawnNpm(args). That way, there's no way for anything to call it. Also, toSpawnPromise("npm") is hard coded, so this isn't preventing much as the call site itself doesn't allow input.

[anupamme]

Thanks, that's a fair point; the allowlist is vacuous since the only call site already hardcodes "npm".

I've revised the PR to make the security boundary explicit at the API level instead: renamed toSpawnPromise to spawnNpm(args) and removed the file parameter entirely. That way the API itself prevents callers from introducing a different executable, rather than validating a value that was already hardcoded.

Also removed ALLOWED_SPAWN_COMMANDS since it's no longer needed, and updated the PR description; the current code isn't directly exploitable; this is defensive hardening / API tightening.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[marcoscaceres]

Lol. sorry to be a pain... but maybe this is AI? ... but also just realized that if only spawnNpm is called, and it's only ever called with [publish], then that can be inlined too 🤦 ... So publishToNpm() would be the right method name here. Everything inlined and hardcoded in. No chance of screwing anything up.

[anupamme]

There is a human in the loop.

Good point; since the args were also always fixed, I went one step further and inlined everything into publishToNpm(). No parameters at all now; the executable and the subcommand are both hardcoded inside. This makes the trust boundary as tight as it can be without removing the abstraction entirely.

[marcoscaceres]

The CI failures seem unrelated. Will check what's going on.