Refuse to spawn .bat/.cmd CLI scripts on Windows to prevent command injection

[qing-ant]

Summary

Follow-up to the argv-injection fix in #1123 (HackerOne #3870101). On Windows installs that have no bundled claude.exe — sdist / source installs and platforms without a bundled wheel, notably Windows ARM64 — CLI discovery falls back to shutil.which("claude"), which resolves npm's claude.cmd batch shim. CreateProcess runs .bat/.cmd files by rewriting the spawn into cmd.exe /c ..., and cmd.exe re-parses the whole command line. subprocess.list2cmdline quotes for the MSVCRT argv rules only (it adds quotes only around whitespace, and cmd.exe does not honor \" escaping), so cmd.exe metacharacters inside an argument value — a --resume session title, the --mcp-config JSON, a system prompt — reach cmd.exe unescaped and can execute injected commands before the CLI even starts. The --flag=value equals form from #1123 does not help on this path: once cmd.exe re-parses the string there is no argv boundary left to protect.

This is the "BatBadBut" vulnerability class (CVE-2024-27980).

The fix

Refuse to spawn a .bat/.cmd script as the CLI on Windows. There is no reliable escaping for cmd.exe (%VAR% expands even inside double quotes), so refusing is the only robust remediation — the same one Node.js shipped for CVE-2024-27980.

The check runs once in connect(), immediately after the CLI path is resolved and before anything is spawned with it, so it covers every route to the executable path:

• the shutil.which("claude") fallback that finds npm's claude.cmd,
• an explicit ClaudeAgentOptions(cli_path=...) pointing at a .bat/.cmd,
• the version probe, which spawns the same path before the main process.

The extension test normalizes the path the way Win32 does before checking it — trailing dots and spaces stripped, ./.. and repeated separators collapsed, NTFS alternate-data-stream specs (claude.cmd:stream, claude:evil.cmd) covered in both directions, drive-relative C:claude.cmd, and a bare .cmd treated as a batch extension (as PathFindExtension treats it). This is the same normalization Rust applied for CVE-2024-24576. It is deliberately plain string logic rather than pathlib, so it behaves identically on the POSIX CI hosts and on Windows.

The error message points at the alternatives that avoid cmd.exe entirely: the native installer (irm https://claude.ai/install.ps1 | iex), an explicit claude.exe path via ClaudeAgentOptions(cli_path=...), or a wheel for a platform that bundles claude.exe.

Defense in depth

With batch-script spawning refused, cmd.exe metacharacters are harmless — list2cmdline quotes correctly for native executables. resume and session_id are nevertheless the values applications most often take from external input, so on Windows they now reject cmd.exe metacharacters (& | < > ^ % ! ") and CR/LF. That keeps them inert even if a cmd.exe hop is ever reintroduced between the SDK and the CLI. No format is imposed beyond that (resume values may be arbitrary session titles, not only UUIDs), and POSIX behavior is unchanged.

Related hardening

extra_args now emits --flag=value when the value starts with -, so a dash-leading value binds to its flag instead of parsing as a separate CLI flag — the same class of issue the --resume equals-form change in #1123 closed, applied to the remaining two-token call site.

Behavior changes on Windows

• Launching a .bat/.cmd CLI (including npm's claude.cmd shim) now raises CLIConnectionError instead of spawning it. Windows users relying on the npm shim need the native installer, an explicit claude.exe path, or a wheel that bundles the CLI.
• resume / session_id values containing cmd.exe metacharacters or newlines now raise ValueError on Windows — e.g. resume="R&D notes". POSIX is unaffected.

Recommended follow-up

Where an npm-only Windows install has no claude.exe, the SDK could resolve the .cmd shim to the underlying node.exe + cli.js and launch node directly with a list argv, restoring npm-shim support without any cmd.exe hop. Not included here to keep this change a focused refusal.

Scope

• src/claude_agent_sdk/_internal/transport/subprocess_cli.py: _reject_windows_batch_cli, _reject_windows_cmd_metacharacters, the connect() chokepoint, and the extra_args equals form.
• tests/test_transport.py: TestWindowsBatchScriptRefusal, TestExtraArgsValueBinding, TestWindowsCmdMetacharacterRejection.
• No behavior change on Linux or macOS.

Testing

• Full suite: 1268 passed, 5 skipped (pytest tests/), plus ruff check, ruff format --check, and mypy src/ clean.
• 36 new tests; 30 of them fail with the source change reverted, so they exercise the fix rather than pre-existing behavior. The refusal tests assert anyio.open_process is called zero times — the batch script is refused before the version probe, not merely before the main spawn.
• The Windows code paths are exercised by patching platform.system(); the actual cmd.exe re-parse is Windows-only behavior reasoned from CreateProcess / list2cmdline semantics rather than executed in CI.

[claude]

Beyond the inline findings, this pass also examined and ruled out: (1) the refusal aborting CLI discovery rather than skipping the .cmd candidate — intended behavior, and the error message carries the remediation; (2) cmd.exe-metacharacter bypass via extra_args-supplied resume/session-id — with batch spawning refused, list2cmdline quotes correctly for native executables, so the metachar check is defense-in-depth for the two documented options only; (3) the metacharacter ValueError escaping connect() unwrapped — it matches the existing ValueError pattern for invalid option values in _build_command.

Extended reasoning...

One inline finding is a bypass of the batch-script refusal itself (trailing dot-dot-space segment diverging from Win32 normalization), so this security PR needs that addressed before merge — no approval. This note only records the additional candidates that were investigated and refuted this run, so a later pass or the author does not re-explore them from scratch. It is informational, not a guarantee of correctness elsewhere in the diff.

[claude]

Beyond the inline findings, this run also examined and ruled out: a missing CHANGELOG.md entry (this repo updates the changelog in per-release commits, e.g. f604b8c, not per PR); a separate '.. ..' dots-and-spaces pop divergence in _is_windows_batch_cli (not a distinct issue beyond the middle-component desync already reported inline); and the shim fallthrough returning an extensionless POSIX shell script that then executes via cmd.exe (a non-.bat/.cmd file is never rewritten through cmd.exe — CreateProcess fails to spawn a non-PE image, so the spawn fails loudly rather than injecting).

Extended reasoning...

Bugs were found, so the inline comments carry the verdict and I am not approving. This note only records what else was examined and refuted this run so a later pass does not re-explore it from scratch: the changelog convention was checked against git history, the '.. ..' segment behavior was traced through the same component loop as the confirmed finding, and the extensionless-script concern was ruled out because the BatBadBut rewrite applies only to .bat/.cmd images. This is informational, not a guarantee of correctness or an instruction to skip anything.

[claude]

Beyond this run's inline nits, I re-checked the two findings from my previous review against 85da04a: the middle-dots/.. stack-desync bypass no longer reproduces — I ran _is_windows_batch_cli directly against all four bypass spellings (claude.cmd\...\.., . .\.., \ \.., .. \..) and each is now refused by the any-component batch-extension rule, while the legitimate claude.exe paths remain allowed. The drive-relative /usr/local/bin/claude planting path is also closed: the Windows fallback list is now native-only (~/.local/bin/claude.exe), locked by test_windows_fallback_skips_posix_shaped_probes.

Extended reasoning...

Both findings from the previous review round (the 🔴 component-stack desync bypass of _is_windows_batch_cli and the 🟡 POSIX-shaped fallback probes reachable on Windows) were independently re-verified against the current head (85da04a): I executed the guard with platform.system() patched to Windows and confirmed all four bypass spellings are refused and the legitimate exe corpus is unaffected, and confirmed by reading _find_cli that the Windows fallback probes only the native installer's claude.exe. This run's two remaining findings are documentation/coverage nits posted inline; the PR remains security-sensitive command-injection hardening, so it still warrants human review rather than shadow approval.

[claude]

No new findings this round — the fixes in 85da04a and 28fac00 hold up under re-review, and everything raised in earlier rounds is now addressed. Since this PR is a security fix in the CLI spawn path, it still warrants a human sign-off rather than a bot approval.

What was reviewed this round:

• The any-component batch check in _is_windows_batch_cli against the four prior normalization-desync spellings — all refused now, since claude.cmd appearing as any component triggers refusal regardless of what follows.
• _is_windows_native_exe vetting on both which() results — a PATHEXT-appended claude.exe.cmd is skipped (final segment ends .cmd after trim), and an extensionless wrapper hit falls through to the native-exe rescue instead of an early return.
• The Windows fallback list — the drive-relative and extensionless POSIX probes are gone; which_hit is returned only as the last resort, so shim-only machines still get the explanatory refusal.
• POSIX behavior — the non-Windows branch returns the which("claude") hit directly and the POSIX locations list is unchanged.

Extended reasoning...

Overview

This PR hardens SubprocessCLITransport against the BatBadBut vulnerability class (CVE-2024-27980): on Windows, CreateProcess runs .bat/.cmd files by rewriting the spawn into cmd.exe /c, and subprocess.list2cmdline quotes only for MSVCRT argv rules, so cmd.exe metacharacters in argument values (resume titles, MCP config JSON, system prompts) could execute injected commands. The fix refuses to spawn batch scripts at a single chokepoint in connect(), adds a defense-in-depth metacharacter rejection for resume/session_id on Windows, closes a dash-leading-value binding gap in extra_args, and — across four review-driven commits — reworks Windows CLI discovery to prefer a native claude.exe over npm's shim while never widening what may be spawned. Changes are confined to subprocess_cli.py plus ~500 lines of new tests.

Security risks

The PR is itself a security fix, and its review history shows why it needs careful eyes: three successive rounds found real issues — a Win32-normalization desync that let claude.cmd\...\..-style paths bypass the guard entirely (a genuine bypass of the PR's core protection, confirmed and fixed in 85da04a), a binary-planting probe via the drive-relative /usr/local/bin/claude fallback (fixed in 85da04a), and unvetted shutil.which() results on Python 3.12+ (fixed in 28fac00). I verified the final state against HEAD: the any-component refusal in _is_windows_batch_cli structurally kills the normalization-desync class (every trick must spell the batch component somewhere in the string), the Windows fallback probes only the native installer's ~/.local/bin/claude.exe, and both which() results pass _is_windows_native_exe before being preferred — while connect() still validates every returned path with _reject_windows_batch_cli, so the discovery preference never widens the spawn surface. This run's bug hunt found no new issues.

Level of scrutiny

Maximum. This is production transport code guarding against command injection, tied to a HackerOne report, with subtle Windows path-resolution semantics (trailing dot/space trimming, NTFS streams, drive-relative paths, PATHEXT behavior differences across Python versions) that this PR's own history proves are easy to get slightly wrong. It also carries a deliberate behavior break: Windows npm-shim users now get a hard CLIConnectionError instead of a working (but injectable) CLI, and resume/session_id values with cmd.exe metacharacters now raise ValueError on Windows. Whether that breakage/telemetry tradeoff is acceptable — and whether the recommended follow-up (resolving the shim to node.exe + cli.js) should land first — is a product call for a human maintainer.

Other factors

Test coverage is strong: the refusal tests assert zero anyio.open_process calls (guarding the version probe too), the author reports 30 of 36 new tests fail with the fix reverted, and each review round added regression tests confirmed failing-before/passing-after. All review threads are resolved and every raised issue has a corresponding fix commit verified in this pass. The one open nit (inverted PATHEXT-ordering claim in a comment) was fixed in the final commit's comment text. Nothing is outstanding — the only reason not to approve is the security-critical nature of the code, which per policy requires human sign-off.