Process: fix Windows .cmd spawn EINVAL by icesword0760 · Pull Request #29759 · openclaw/openclaw

icesword0760 · 2026-02-28T13:03:05Z

Summary

Fix Windows spawn of .cmd/.bat (npm/pnpm) by running via cmd.exe to avoid EINVAL.
Add conservative cmd.exe argument hardening when the wrapper is required.
Avoid token-like PAT strings in diagnostics tests to prevent accidental secret scanning.

Test plan

pnpm test exec.test.ts

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d09f5aa8b5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-02-28T13:06:10Z

src/process/exec.ts

+  if (WINDOWS_UNSAFE_CMD_CHARS_RE.test(arg)) {
+    throw new Error(
+      `Unsafe Windows cmd.exe argument detected: ${JSON.stringify(arg)}. ` +


Permit literal % in cmd.exe-wrapped arguments

escapeForCmdExe now throws on any % (and &) when invoking .cmd/.bat via cmd.exe, but some existing Windows call paths pass user text as a single JSON argument (for example docsSearchCommand builds --args <payload> from the query in src/commands/docs.ts:174-178 and then runs via pnpm.cmd/npx.cmd). Queries like "100%" or "A & B" become hard failures (Unsafe Windows cmd.exe argument detected) before the subprocess starts, which is a functional regression from prior behavior for legitimate inputs.

Useful? React with 👍 / 👎.

greptile-apps · 2026-02-28T13:08:51Z

Greptile Summary

Fixes Windows EINVAL errors when spawning .cmd/.bat files (like npm.cmd, pnpm.cmd) by wrapping execution via cmd.exe with conservative argument hardening.

Detects Windows batch commands and routes them through cmd.exe with /d /s /c flags
Implements escapeForCmdExe() to safely quote arguments and reject dangerous cmd.exe metacharacters (&|<>^%\r\n)
Uses "" to escape quotes inside quoted strings (cmd.exe convention)
Applies fix to both runExec() and runCommandWithTimeout()

Issue found: runExec() is missing windowsVerbatimArguments: true when using the cmd.exe wrapper, which could cause Node.js to apply conflicting quote escaping on top of the cmd.exe-style escaping

Confidence Score: 2/5

This PR has a critical bug in runExec() that needs fixing before merge
The Windows cmd.exe wrapper logic is well-designed with conservative security hardening, but runExec() is missing windowsVerbatimArguments: true, which causes Node.js to apply conflicting quote escaping. This will result in malformed command lines and potential runtime failures. The fix in runCommandWithTimeout() is correct, but the bug in runExec() is critical enough to block merging
src/process/exec.ts needs the windowsVerbatimArguments: true fix in the runExec() function

_{Last reviewed commit: d09f5aa}

greptile-apps

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-28T13:08:55Z

src/process/exec.ts

+    const { stdout, stderr } = isWindowsBatchCommand(resolvedCommand)
+      ? await execFileAsync(
+          process.env.ComSpec ?? "cmd.exe",
+          ["/d", "/s", "/c", buildCmdExeCommandLine(resolvedCommand, args)],
+          options,
+        )


missing windowsVerbatimArguments: true when spawning via cmd.exe wrapper

without this, Node.js will apply its own Windows quoting (using \" escapes) on top of the cmd.exe-style quoting (using "" escapes) from buildCmdExeCommandLine, causing malformed command lines

Suggested change

const { stdout, stderr } = isWindowsBatchCommand(resolvedCommand)

? await execFileAsync(

process.env.ComSpec ?? "cmd.exe",

["/d", "/s", "/c", buildCmdExeCommandLine(resolvedCommand, args)],

options,

)

const { stdout, stderr } = isWindowsBatchCommand(resolvedCommand)

? await execFileAsync(

process.env.ComSpec ?? "cmd.exe",

["/d", "/s", "/c", buildCmdExeCommandLine(resolvedCommand, args)],

{ ...options, windowsVerbatimArguments: true },

)

Prompt To Fix With AI

This is a comment left during a code review. Path: src/process/exec.ts Line: 91-96 Comment: missing `windowsVerbatimArguments: true` when spawning via cmd.exe wrapper without this, Node.js will apply its own Windows quoting (using `\"` escapes) on top of the cmd.exe-style quoting (using `""` escapes) from `buildCmdExeCommandLine`, causing malformed command lines ```suggestion const { stdout, stderr } = isWindowsBatchCommand(resolvedCommand) ? await execFileAsync( process.env.ComSpec ?? "cmd.exe", ["/d", "/s", "/c", buildCmdExeCommandLine(resolvedCommand, args)], { ...options, windowsVerbatimArguments: true }, ) ``` How can I resolve this? If you propose a fix, please make it concise.

Takhoffman · 2026-03-02T13:50:08Z

PR #29759 - Process: fix Windows .cmd spawn EINVAL (#29759)

Merged via squash.

Merge commit: 6e008e9
Verified: pnpm vitest run src/process/exec.test.ts src/process/exec.windows.test.ts
Changes made:
- Kept the .cmd/.bat cmd.exe wrapper approach in src/process/exec.ts while preserving current main behavior that routes npm/npx through node + npm-cli.js.
- Added windowsVerbatimArguments: true for the cmd wrapper path in runExec to avoid quote mangling.
- Added targeted Windows wrapper tests in src/process/exec.windows.test.ts and aligned them with current npm shim behavior.
Why these changes were made:
- The PR hit a clean-merge conflict with newer Windows spawn logic on main; the resolved version keeps the intended EINVAL fix without regressing recent npm/npx handling.
- The extra test coverage closes the previous gap around wrapper invocation and argument handling.
Changelog: CHANGELOG.md updated=false required=false opt_out=true (workstream canonicalization run)

Thanks @icesword0760!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de82564fd0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-02T13:55:08Z

src/process/exec.ts

+
+function escapeForCmdExe(arg: string): string {
+  // Reject cmd metacharacters to avoid injection when we must pass a single command line.
+  if (WINDOWS_UNSAFE_CMD_CHARS_RE.test(arg)) {


Allow % and & in wrapped cmd.exe arguments

escapeForCmdExe now throws for any argument containing % or &, but several Windows call paths pass user text through argv (for example docsSearchCommand serializes the raw query into the --args JSON payload before invoking pnpm.cmd). Inputs like 100% or A & B therefore fail before the subprocess even starts, which is a functional regression for legitimate queries and command arguments; these characters need escaping in the wrapper path rather than blanket rejection.

Useful? React with 👍 / 👎.

…ord0760