Commit 879e920
authored
Refuse to spawn .bat/.cmd CLI scripts on Windows to prevent command injection (#1127)
## 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.1 parent 2d4ef94 commit 879e920
2 files changed
Lines changed: 709 additions & 10 deletions
Lines changed: 203 additions & 10 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
33 | 41 | | |
34 | 42 | | |
35 | 43 | | |
| |||
147 | 155 | | |
148 | 156 | | |
149 | 157 | | |
| 158 | + | |
150 | 159 | | |
151 | | - | |
152 | | - | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
161 | 201 | | |
162 | 202 | | |
163 | 203 | | |
164 | 204 | | |
165 | 205 | | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
166 | 227 | | |
167 | 228 | | |
168 | 229 | | |
| |||
187 | 248 | | |
188 | 249 | | |
189 | 250 | | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
190 | 367 | | |
191 | 368 | | |
192 | 369 | | |
| |||
355 | 532 | | |
356 | 533 | | |
357 | 534 | | |
| 535 | + | |
358 | 536 | | |
359 | 537 | | |
360 | 538 | | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
361 | 542 | | |
362 | 543 | | |
363 | 544 | | |
| |||
431 | 612 | | |
432 | 613 | | |
433 | 614 | | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
434 | 623 | | |
435 | 624 | | |
436 | 625 | | |
| |||
483 | 672 | | |
484 | 673 | | |
485 | 674 | | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
486 | 679 | | |
487 | 680 | | |
488 | 681 | | |
| |||
0 commit comments