fix(brainstorm): launch BRAINSTORM_OPEN_CMD without a shell

[aznikline]

What

Hardens the BRAINSTORM_OPEN_CMD browser-launch path in skills/brainstorming/scripts/server.cjs so it no longer goes through a shell. Closes #1957.

Why

The security audit in #1957 flagged that maybeOpenBrowser() ran the operator-provided BRAINSTORM_OPEN_CMD via cp.exec(cmd + ' ' + JSON.stringify(url)), i.e. through sh -c. The env var is opt-in, loopback-only, trusted operator input, so the practical risk is bounded — but the shell-exec shape is what the audit (and downstream org security reviews) block adoption on. The issue's proposed resolution was to match the platform launchers two lines below, which already use execFile with the URL as a single argv element.

How

• Replaced the cp.exec branch with cp.execFile(bin, args.concat(url), …) — no shell, the URL is a single argv element.
• Added a quote-aware tokenizer shellArgv() that splits the operator's command string into an argv honoring single/double quotes (so a command like node "/path/with space/capture.cjs" "/marker/path" still resolves to the right tokens, matching how the existing test fixture builds BRAINSTORM_OPEN_CMD).
• A command with unbalanced quotes returns null and is rejected — it is never handed to a shell as a fallback. The previous try/catch best-effort semantics are preserved on the actual spawn.

This mirrors the existing browserLauncherForPlatform pattern (bin + args + execFile), so the two launcher paths now share the same no-shell shape.

Tests

• Existing lifecycle.test.js tests for BRAINSTORM_OPEN_CMD still pass — their fixture (node "script" "marker") tokenizes cleanly to [node, script, marker], with the URL appended as the final element.
• Added a regression test: BRAINSTORM_OPEN_CMD launches without a shell and keeps the URL as one argv element. It captures the launched argv and asserts the server URL (which carries a ?key= query) arrives as exactly one token with no shell-splitting — the direct proof the injection shape is gone.

Behavior change

Operators who set BRAINSTORM_OPEN_CMD to a command relying on shell features (pipes, &&, env expansion, globbing) will need to wrap it in a script — the command is now tokenized and execFile'd, not shell-interpreted. This matches how the platform launchers already behave and is the intended hardening. I called this out in the env var's comment.

Checklist

• Change is scoped to the flagged line and its helper
• No new shell surface introduced
• Existing tests green; added a regression test for the no-shell contract

[muunkky]

Independently took a run at this before I noticed #1964 — and landed on the same core fix (quote-aware tokenizer → execFile, no shell). Nice to see it converge; good sign it's the right shape.

Two bits from my version that might be worth folding into #1964, if useful:

• Operator docs — a BRAINSTORM_OPEN_CMD block in visual-companion.md (accepted shape, trust posture, the no-shell note). The issue title mentions docs, and fix(brainstorm): launch BRAINSTORM_OPEN_CMD without a shell #1964 is code-only: https://github.com/muunkky/superpowers/blob/showcase/1957/skills/brainstorming/visual-companion.md
• A direct injection test — set BRAINSTORM_OPEN_CMD to node "<capture>" "<marker>" ; touch "<pwned>" and assert <pwned> is never created — i.e. prove the metacharacters don't execute (complements your URL-integrity assertion): https://github.com/muunkky/superpowers/blob/showcase/1957/tests/brainstorm-server/lifecycle.test.js

Also minor: #1964 targets main; but I pushed mine to dev as per contribution rules.

Full write-up on my fork if it's worth a skim — PRD → design → ADR: PRD · design · ADR. Happy to send the docs + test as a follow-up to #1964 rather than open a competing PR.

Disclosure (per contribution rules): produced with an autonomous development harness — gitban (muunkky.github.io/gitban-site) — on Claude Code 2.1.207 / Claude Opus 4.8; reasoned from the issue + direct code inspection.

[aznikline]

@muunkky — really nice writeup, and a strong signal to see two independent passes converge on the same tokenizer → execFile shape.

I folded both of your bits straight into #1964 (appreciate you offering them as a follow-up rather than competing):

• Operator docs — added a BRAINSTORM_OPEN_CMD section to visual-companion.md covering the accepted shapes (bare command / command+args / quoted argv), the trust posture (env var = invoking user's authority), and the no-shell note (the launcher tokenizes and runs without a shell, so ;/& in the value are literal). The issue title does call out docs, and fix(brainstorm): launch BRAINSTORM_OPEN_CMD without a shell #1964 was code-only — good catch.
• Direct injection test — added alongside the URL-integrity regression: BRAINSTORM_OPEN_CMD set to a capture command with a ; touch <pwned> tail, asserting <pwned> is never created. Complements the existing assertion that the URL's &/? stay a single argv element.

Also retargeted the PR base main → dev per the contribution rules (good flag — I'd missed that). Branch is rebased onto dev, mergeable/CLEAN. Full brainstorm-server suite green (136 tests: 125 node + 11 bash, including both the URL-integrity and the new injection case).

@obra — ready for review when you have a moment. Closes #1957.