Closes #69.
craft.shell.spawn(id, cmd, args) has advertised an arguments array since it was written. The SDK's spawn(command, args?, options?) says the same. Native declared no args field, so ignore_unknown_fields parsed the array and dropped it without a word:
craft.shell.spawn('p', 'git', ['status']) // ran `git` bare, reported successWhy the arguments had nowhere to go
spawn ran /bin/sh -c <command>. A shell takes one string, not an argv — so there was no slot to put them in.
The issue offers "append them to the argv", but the argv was {"/bin/sh", "-c", command}. Appending to that means appending to the command string, which means quoting every argument correctly, forever, on two platforms, against input supplied by a web page — in order to avoid handing that page a shell.
So the default stops using one:
| before | after | |
|---|---|---|
| what runs | /bin/sh -c "git" | execve("git", ["git", "status"]) |
args | parsed, discarded | passed |
; in an argument | shell syntax | a semicolon the program receives |
| injection surface | a blocklist between a page and sh -c | none — no shell exists |
shell: true — and it was already documented
The SDK's SpawnOptions has declared shell?: boolean all along, saying "The Craft bridge translates this automatically". It was as dead as args — native had no field for it either. It's real now.
That path keeps the metacharacter blocklist, because it's the only path that needs one. Passing args alongside it is refused, not half-honoured — silently ignoring arguments is the exact bug being fixed, and the compatibility path must not reintroduce it.
The compatibility break, stated plainly
spawn('p', 'git status') used to work, because sh split the string. It now looks for a program named "git status".
That is the same break Node made deliberately, and the fix is spawn('p', 'git', ['status']) — what the signature asked for in the first place. Worth noting how little the shell was contributing: validateCommand already rejected ;, |, &, >, <, \n, $( and backticks, so it provided word-splitting and essentially nothing else. Callers who need more can pass shell: true.
Tests
bridge_shell.zig had no tests and no test artifact — which is how it shipped parsing an array and throwing it away. The argv decision is now a pure function with 10 tests, wired into zig build test.
Controlled: reintroducing the original behaviour — parsing the arguments and dropping them, compiling cleanly — fails 7 tests by name:
error: 'bridge_shell.test.arguments reach the program instead of being dropped' failed
error: 'bridge_shell.test.no shell is involved, so no shell metacharacters are interpreted' failed
error: 'bridge_shell.test.a program with no arguments is just the program' failed
...(My first attempt at that control failed to compile rather than failing an assertion, which proves nothing about the test. Redone so the broken version builds.)
Also covered: an older payload with no args key still spawns the same program, empty-string arguments survive, argument order is pinned, and the JSON→argv seam is tested end to end with the struct the bridge actually decodes with.
Noted, not fixed
The SDK's ChildProcess.start() posts to process.spawn, and process is not a routed bridge type anywhere in the dispatcher — that path answers nothing at all. Separate from this issue; flagging rather than widening scope into it.
Verified
zig build · zig build test · zig build test-js · x86_64-windows cross-build · zig fmt --check · tsc --noEmit · 511 bun tests · pickier 38 warnings, unchanged from main.