· 5 min read
The advice is correct and everyone repeats it: don't build shell strings, use the array form.
spawnSync('git', ['worktree', 'add', '-b', branch, path, baseRef]);No shell, so no shell injection. We verified that rather than assuming it —
generated every command our orchestrator produces, swapped in a payload that would
touch a marker file, and executed each one in a real shell:
const PAY = `x'; touch ${S}/PWNED; echo 'y`;
for (const s of generatedCommands) spawnSync('sh', ['-c', s]);Across session name, worker name, task text, repo root and base ref: no marker, ever. The quoting holds.
Then we looked at the argv itself.
git parses its own arguments
baseRef = '--git-dir=/elsewhere';
// argv: ['worktree', 'add', '-b', 'orchestrator-x', '/tmp/wt', '--git-dir=/elsewhere']The shell never sees it. git does, and git reads anything starting with - as an
option. In git worktree add -b <branch> <path> <commit-ish>, a commit-ish of
--git-dir=/elsewhere points git at a different repository. --force changes the
command's meaning.
No metacharacters involved. Nothing for a shell to interpret. The attack is that data moved into the option namespace.
Finding yours
Walk every value that reaches an argv array and ask where it came from. In our case four were already safe and one was not:
| value | how it's built | safe? |
|---|---|---|
sessionName | slugify() → [a-z0-9-]+ | yes |
branchName | literal orchestrator- prefix | yes |
worktreePath | path.join → absolute | yes |
| worker slug | slugify() | yes |
baseRef | straight from --base-ref | no |
The four safe ones are safe by accident of shape — a slug can't start with -, an
absolute path starts with /, a prefixed string starts with its prefix. Only the
one value that passed through untouched was exposed.
That is a useful audit: anything slugified, prefixed, or path-resolved is fine. Anything that reaches argv as the user typed it needs a look.
The fix
if (typeof baseRef !== 'string' || baseRef.startsWith('-')) {
throw new Error(`baseRef must be a ref name, not an option: ${JSON.stringify(baseRef)}`);
}Rejecting a leading - costs nothing here, and that is worth checking rather than
assuming: git check-ref-format forbids refs beginning with -. So no
legitimate value is lost. HEAD, main, v1.2.3, feature/x, origin/main all
still work — pinned by a test, alongside the payloads.
Where a leading dash is legitimate — a filename, say — use the end-of-options separator instead:
spawnSync('grep', ['-r', pattern, '--', userPath]);Most GNU and git commands honour --. Check the one you're calling; not all do,
and git worktree add does not take it before a commit-ish, which is why we
validate there instead.
Is this "self-inflicted"?
Partly. --base-ref is a flag the user types, so the first-order case is someone
attacking themselves.
But swarm configs are files, and files get shared — the same reasoning that makes a session bundle untrusted. A config from a teammate, a template from a gist, a repo you cloned. The moment the value can arrive from somewhere other than your own keyboard, "the user typed it" stops being a security argument.
The short version
Array-form spawn protects the shell boundary. It does nothing for the argv boundary. Those are two different boundaries and you need both.
If a user-supplied string lands in an argv array, either prove it cannot start
with -, or pass -- before it.
Fixed in ECC 2.24.8. The orchestrator and its tests are in scripts/lib.