fix(parallels): send guest prep scripts over stdin

[saariuslystoned]

Problem

prlctl exec does not preserve argv boundaries. It joins its arguments into one string and re-parses that string with a shell inside the guest. EnsureGuestReady and InstallSSHKey both handed the whole generated POSIX script over as a single argv element after /bin/sh -lc, so the guest effectively ran /bin/sh -lc set with -eu as positional arguments — consuming line 1 — and executed every remaining line in its outer shell, with set -eu never in effect.

Guest preparation therefore ran with abort-on-error disabled, defeating guards the scripts are written to rely on, such as [ -x "$kickstart" ] in the macOS --desktop branch.

Fixes #2396.

What this does

Deliver both scripts on stdin, which prlctl exec preserves verbatim — the shape BootstrapMacOSOverSSH already uses (sudo -n /bin/sh -s).

prlctlWithStdin carries the reader through both routes. prlctl is now a thin wrapper over it, so host selection, key handling, the remote PATH= prefix and the password-filtered child environment are untouched on the remote/proxy route; ssh forwards stdin to prlctl on the Parallels host.

Stdin delivery imposes a rule on the scripts themselves: any child that reads stdin swallows the remainder of the script while the shell still exits 0. #2387 established </dev/null for the macOS Node children; this extends it to every child in both scripts. Three groups matter most:

• The Linux branch, which reaches stdin for the first time here — apt-get update, both apt-get install calls, systemctl daemon-reload / enable --now.
• The readiness gate, /usr/local/bin/crabbox-ready </dev/null >/tmp/crabbox-ready.log 2>&1, which runs mid-script: anything it consumed would be lost.
• The probes guarding that gate — the desktop VNC checks (nc -z 127.0.0.1 5900, systemctl is-active) and, after the rebase, the macOS SSH-listener check from fix(parallels): verify the macOS SSH listener during guest preparation #2399.

Commands that are intentionally non-fatal keep their || true.

Commits

bc6ea357	failing regression: both scripts must arrive on stdin and not in argv, plus the stdin-consumption rule
47459e3e	the fix

Rebased onto current main (2ad31a5a); the rebase was clean, and on it go vet ./... is clean and go test -race ./internal/cli -run 'TestParallels|TestManagedMacOSNodeBaseline|TestMacOSNode' and go test -race ./internal/providers/parallels/ both pass. Previously rebased onto f7bed55d after #2399 merged. That PR added the macOS SSH-listener probe crabbox_ssh_listening, which runs mid-script inside the readiness gate, so its nc -z 127.0.0.1 "$port" is now covered by the same </dev/null rule and asserted by the regression test. The nc inside the generated crabbox-ready helper needs no guard: the prep script already invokes that helper with stdin on /dev/null.

The regression commit fails on its parent, and only through its own two tests — the rest of TestParallels is green there.

The runner is mocked, so the tests cannot observe guest-side reconstruction; they pin the transport crabbox chooses, which is the part that is wrong. Native evidence below covers the guest side.

Verification

In tree

go vet ./internal/cli ./internal/providers/parallels/...                                                  clean
go test -race -timeout=20m ./internal/cli -run 'TestParallels'                                            ok  10.9s
go test -race -timeout=20m ./internal/cli -run 'TestParallels|TestManagedMacOSNodeBaseline|TestMacOSNode'  ok  18.1s
go test -race -timeout=20m ./internal/providers/parallels/...                                             ok   5.9s
gofmt                                                                                                     clean

Native, on a real Apple-silicon Parallels macOS 26.5.2 guest

Run twice: once against the pre-rebase head (the transport probes below), and again against the rebased head e29afd08, so the combination with #2399 listener probe is covered rather than assumed.

Disposable full clone of the host's macOS template, deleted afterwards; template left stopped with all 7 snapshots and its current snapshot unchanged.

The mechanism, both shapes, same guest. Old argv shape — set -eu is swallowed, the rest leaks, exit 0:

$ prlctl exec $V /bin/sh -lc "$(printf 'set -eu\nfalse\necho SHOULD_NOT_PRINT\n')"
...
BASH_EXECUTION_STRING=set
...
SHOULD_NOT_PRINT
exit=0

New stdin shape — set -eu is in effect, nothing leaks:

$ printf 'set -eu\nfalse\necho SHOULD_NOT_PRINT\n' | prlctl exec $V /bin/sh -s
exit=1

Argv flattening is inherent to prlctl exec and is not fixed by moving the script to stdin — it is simply no longer in the way. Positional arguments still split:

$ printf 'for a in "$@"; do printf "[%s]\n" "$a"; done\n' | prlctl exec $V /bin/sh -s 'a b' c
[a]
[b]
[c]

Exit status propagates (exit 7 → exit=7), and a 60 KB script over stdin arrives whole.

The real generated scripts, dumped from this branch and piped to prlctl exec <vm> /bin/sh -s on a fresh clone: install-key exit 0, ensure-ready exit 0.

End to end, this branch's binary (GOOS=darwin GOARCH=arm64):

run summary lease=30.769s bootstrap=318ms sync=0s command=394ms total=893ms end_to_end=31.985s exit=0
run summary lease=31.256s bootstrap=324ms sync=0s command=422ms total=933ms end_to_end=32.512s exit=0

After the rebase, on a fresh full clone, the generated scripts from e29afd08 piped to prlctl exec <vm> /bin/sh -s:

-- install-key over stdin                       exit=0
-- ensure-ready over stdin                      exit=0      (includes the #2399 listener probe)
-- guest-side state
   Connection to 127.0.0.1 port 22 succeeded!
   SSHD_LISTENING
   READY_OK                                                 (/usr/local/bin/crabbox-ready passes as root)
-- set -eu really in effect on this guest shell  exit=1      (no leak)

Template left stopped with all 7 snapshots and its current snapshot unchanged; every clone deleted.

An unrelated flake I hit, with a control

Some runs on that host fail at prlctl exec with PrlJob_GetRetCode: Invalid argument / PrlJob_GetResult: Invalid argument, at whichever exec comes first. I ran origin/main built the same way as a control, alternating. Pre-rebase: this branch 2 pass / 2 fail, origin/main 1 pass / 2 fail. Post-rebase against f7bed55d: both binaries passed round 1 and both failed round 2, on the same round. It is pre-existing and not introduced here, but worth knowing if CI or another native run trips on it.

Native, remote-host route (--parallels-host)

Earlier revisions of this body listed the remote route as unexercised; this section covers it. Crabbox ran on an operator Mac and drove a remote Apple-silicon Parallels host (macOS 26.5.2, prlctl 27.0.1) over --parallels-host, cloning a macOS template with --parallels-clone-mode full. The branch is rebased onto current main (2ad31a5a). The control is an unmodified main binary built the same way.

crabbox run --provider parallels --target macos \
  --parallels-host <host> --parallels-host-user <user> \
  --parallels-source <macOS template> --parallels-clone-mode full \
  --parallels-user <guest user> --no-sync -- /bin/sh -c 'echo GUEST_OK; sw_vers -productVersion; ls -ld /var/lib/crabbox/bootstrapped'

While each lease ran, a sampler on the Parallels host recorded the argv of every prlctl exec process, so the transport is observed on the host rather than inferred.

Fault injection without touching the template. CRABBOX_WORK_ROOT=/var/lib/crabbox/ssh.username is a path that InstallSSHKey creates as a regular root-owned file just before guest prep. In EnsureGuestReady, mkdir -p "$work_root" then fails mid-script. The final crabbox-ready still passes, because test -w on a file is true for root. The fault can only be caught if set -eu is really in effect.

Run	Binary	Fault	Guest-prep argv seen on the host	Result
happy	this PR	none	prlctl exec <vm> /bin/sh -s	lease ready, GUEST_OK, exit 0
fault	this PR	work root is a file	prlctl exec <vm> /bin/sh -s	parallels guest prep: exit status 1: mkdir: /var/lib/crabbox/ssh.username: File exists
fault	main	same	prlctl exec <vm> /bin/sh -lc set -eu\012user=…\012work_root=… (whole script in argv)	prep passes, lease reports provisioned, then fails later as create remote workdir: exit status 1

Happy path, this PR:

provisioned lease=cbx_5080a8f918bb vm=5933f005-… ip=10.211.55.86
GUEST_OK
26.5.2
-rw-rw-rw-  1 root  wheel  0 Sep 20 23:27 /var/lib/crabbox/bootstrapped
run summary lease=41.471s bootstrap=526ms sync=0s command=1.14s total=2.171s end_to_end=44.262s sync_skipped=true exit=0
lease cleanup stopped=true policy=auto lease=cbx_5080a8f918bb

Fault, this PR. It fails at the step that failed, and crabbox deletes the clone:

provisioning provider=parallels lease=cbx_c26822d8cad9 ... clone_mode=full keep=false
parallels guest prep: exit status 1: mkdir: /var/lib/crabbox/ssh.username: File exists

Fault, main. The failure is swallowed and surfaces later under the wrong name:

provisioned lease=cbx_75c5a7d9bc84 vm=c4e8b504-… ip=10.211.55.88
  workdir=/var/lib/crabbox/ssh.username/cbx_75c5a7d9bc84/crabbox
create remote workdir: exit status 1

So ssh does forward stdin to the remote prlctl intact. Both scripts reached the guest whole: the happy path provisioned and ran, and set -eu aborted the fault run at the right line.

One PR fault attempt died with the pre-existing PrlJob_GetRetCode: Invalid argument flake described above, while another VM was running on the host. The retry is the run shown.

Cleanup. Every clone was deleted, with a readback of prlctl list --all after each run. The template was left stopped, with the same snapshot count and the same current snapshot as before, compared against a pre-run readback.

Linux: there is no Linux Parallels template on this host either, so the Linux apt-get path still has no native coverage. That limit is unchanged, and accepting it is the maintainer's call.

Scope limits

Please read the native evidence as covering exactly this and no more:

• Exercised: the local (host-resident prlctl) route and the remote route (--parallels-host, section above), both on a macOS guest, on one Apple-silicon host and one template.
• Reasoned about, not exercised: Linux guests. There is no Linux Parallels template on that host, so the apt-get stdin-consumption path — the single largest behavioral surface this change opens — has no native coverage at all. It is asserted in tree by string match only. Dropping -l also means a Linux guest now runs the script under /bin/sh proper (dash on Debian/Ubuntu) rather than whatever outer shell the flattened form landed in; TestParallelsEnsureReadyScriptParsesUnderPOSIXShell covers that syntactically, and on a Linux CI runner that check is dash.
• Behavior change worth a maintainer's eye: set -eu genuinely taking effect is the point of the fix, but it means preparation can now abort where it previously ran on. Anything meant to stay non-fatal must carry || true; I audited both scripts and left the existing ones alone, but a guest-side failure that used to be silently survivable will now surface as a failed prep.

Contributor PR — no CHANGELOG.md edit, per AGENTS.md.

🤖 Generated with Claude Code

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 21, 2026, 3:56 AM ET / 07:56 UTC (Revision 5).

ClawSweeper review

What this changes

The PR streams Parallels guest preparation and SSH-key installation scripts through stdin, protects that input from child commands, and adds regression tests and documentation.

Merge readiness

⛔ Blocked before merge - 3 items remain

The fix remains necessary, and no introduced correctness defect was found. The latest documentation establishes fail-fast intent; Linux compatibility coverage and the merge conflict remain unresolved.

Priority: P2
Reviewed head: 560d0741a29e38e9260b3e173e685115b429f927
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with strong native macOS evidence and no concrete patch defect; Linux compatibility remains the material validation limit.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): Contributor terminal traces exercise the real local and remote macOS preparation paths, showing successful leases and correct failure propagation under an injected fault. Production code is unchanged since that evidence; Linux compatibility remains a separate, explicitly disclosed coverage decision.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): Contributor terminal traces exercise the real local and remote macOS preparation paths, showing successful leases and correct failure propagation under an injected fault. Production code is unchanged since that evidence; Linux compatibility remains a separate, explicitly disclosed coverage decision.
Evidence reviewed	9 items	Introduced patch and review continuity: The pinned merge-base-to-head diff changes four files. Both POSIX entrypoints now pass scripts through stdin, and the transport forwards the reader on local and remote routes. Comparing the previous reviewed head with this head shows only seven added documentation/changelog lines; production code and tests are unchanged. Current main still uses argv: Current main still passes both generated scripts as arguments after /bin/sh -lc. The merged Node-baseline and SSH-listener fixes address adjacent behavior and do not implement this transport repair. Latest release remains affected: GitHub identifies v0.63.0 as the latest release. Its source also sends both preparation scripts through /bin/sh -lc argv. Local release-blob inspection failed, so the released source was read through the GitHub contents API.
Findings	None	None.
Security	None	None.

How this fits together

Crabbox prepares cloned Parallels VMs before using them for SSH-based execution. These scripts install the lease key and prepare the guest through local prlctl or SSH to a remote Parallels host.

flowchart TD
  A[Clone and lease settings] --> B[Generate guest scripts]
  B --> C{Parallels host location}
  C --> D[Local prlctl]
  C --> E[SSH to remote prlctl]
  D --> F[Guest shell reads stdin]
  E --> F
  F --> G{Preparation succeeds}
  G --> H[SSH readiness or lease cleanup]

Loading

Decision needed

Question	Recommendation
Is the macOS native evidence plus Linux source and syntax coverage sufficient to accept the untested Linux preparation and upgrade path?	Establish Linux compatibility: Require native evidence from fresh and previously prepared Linux templates before landing.

Why: The intended fail-fast behavior is now documented, but the previous Linux compatibility requirement remains unanswered and needs an explicit evidence-risk decision.

Before merge

• Resolve merge risk (P1) - Fresh and previously prepared Linux templates lack native compatibility evidence: effective set -eu can now stop leases on package-installation or desktop-setup errors that previously went unnoticed. The documented fail-fast contract does not explicitly accept this coverage gap.
• Complete next step (P2) - Resolve the reported merge conflicts while preserving current-main changes and the integration changelog entry, then validate the resulting head.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +54/-36 (net +18); tests +161/-8 (net +153)	Production growth supports stdin transport and child-input isolation, with regression coverage for both routes and both scripts.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #2396
Summary: This PR is the candidate fix for the open argv-flattening issue; the related merged preparation fixes address separate behavior.

Members:

• canonical: parallels: prlctl exec flattens argv, silently dropping set -eu from guest prep scripts #2396 - Reports the exact transport defect addressed by both changed entrypoints.
• adjacent_distinct: fix(parallels): install the macOS Node baseline on the Parallels path #2387 - Merged Node-baseline preparation and earlier child-stdin protections without replacing the prlctl argv transport.
• adjacent_distinct: fix(parallels): verify the macOS SSH listener during guest preparation #2399 - Merged SSH-listener verification, whose probe receives stdin isolation here.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Verify Linux template compatibility (recommended)
   Provide fresh and previously prepared Linux guest results covering preparation, readiness and required-step failure propagation.
2. Accept bounded platform coverage
   Record explicit acceptance of the missing Linux runtime coverage while retaining the documented fail-fast contract.

Technical review

Best possible solution:

Retain the shared stdin transport and explicit best-effort exceptions, with Linux fresh-template and upgrade compatibility demonstrated or its coverage limit explicitly accepted.

Do we have a high-confidence way to reproduce the issue?

Yes: current main retains the argv transport, and the supplied native control demonstrates swallowed shell failures. This review inspected that path without independently executing it.

Is this the best way to solve the issue?

Yes: stdin delivery follows the existing SSH bootstrap pattern and directly avoids argv reconstruction; protecting child input addresses the resulting script-consumption hazard.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 64cf0ed6adc3.

Labels

Label justifications:

• P2: This repairs a bounded Parallels provisioning defect that silently bypasses intended shell failure checks.
• merge-risk: 🚨 compatibility: Fail-fast preparation is documented as intentional, but Linux fresh-template and upgrade behavior remains unverified.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Contributor terminal traces exercise the real local and remote macOS preparation paths, showing successful leases and correct failure propagation under an injected fault. Production code is unchanged since that evidence; Linux compatibility remains a separate, explicitly disclosed coverage decision.
• proof: sufficient: Contributor real behavior proof is sufficient. Contributor terminal traces exercise the real local and remote macOS preparation paths, showing successful leases and correct failure propagation under an injected fault. Production code is unchanged since that evidence; Linux compatibility remains a separate, explicitly disclosed coverage decision.

Evidence

What I checked:

• Introduced patch and review continuity: The pinned merge-base-to-head diff changes four files. Both POSIX entrypoints now pass scripts through stdin, and the transport forwards the reader on local and remote routes. Comparing the previous reviewed head with this head shows only seven added documentation/changelog lines; production code and tests are unchanged. (internal/cli/parallels.go:423, 560d0741a29e)
• Current main still uses argv: Current main still passes both generated scripts as arguments after /bin/sh -lc. The merged Node-baseline and SSH-listener fixes address adjacent behavior and do not implement this transport repair. (internal/cli/parallels.go:422, 64cf0ed6adc3)
• Latest release remains affected: GitHub identifies v0.63.0 as the latest release. Its source also sends both preparation scripts through /bin/sh -lc argv. Local release-blob inspection failed, so the released source was read through the GitHub contents API. (internal/cli/parallels.go:336, 8a8b22d6fd4f)
• Native production-path proof: The captured PR body in context sourceRevision ef90f42230e82c76ed42d8bc7918eefc5edb9dd37c8d998b01bf389c2842f828 contains local and remote macOS guest traces. The remote Crabbox run reaches GUEST_OK; an injected work-root failure stops inside preparation on the branch while the main control incorrectly proceeds. Host-side argv sampling confirms /bin/sh -s. These traces exercise InstallSSHKey, EnsureGuestReady and the real transport; the latest commit changes only documentation. See fix(parallels): send guest prep scripts over stdin #2401. (internal/cli/parallels.go:533, 47459e3e3141)
• Fail-fast contract documented: The latest steipete-authored commit explicitly documents immediate failure of required preparation steps while preserving best-effort service handling, and adds the credited changelog entry. This resolves ambiguity about intended failure semantics, but does not explicitly accept the missing Linux compatibility coverage. (docs/providers/parallels.md:21, 560d0741a29e)
• Remaining Linux coverage limit: The PR explicitly reports no native Linux template coverage. Linux package installation and desktop service setup now execute with effective set -eu and protected stdin. Added tests assert transport and redirection strings; the existing POSIX-shell check establishes syntax, not fresh-template or upgrade behavior. This is the remaining substantive item from the previous review. (internal/cli/parallels.go:1001, 560d0741a29e)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• saariuslystoned: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Establish fresh and previously prepared Linux template compatibility, or record explicit acceptance of that coverage limit.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (4 earlier review cycles)

• reviewed 2026-09-20T19:42:38.076Z sha 7007df0 :: blocked before merge. :: none
• reviewed 2026-09-20T21:23:23.633Z sha e29afd0 :: blocked before merge. :: none
• reviewed 2026-09-20T21:35:26.261Z sha e29afd0 :: blocked before merge. :: none
• reviewed 2026-09-21T04:44:15.528Z sha 47459e3 :: blocked before merge. :: none

[saariuslystoned]

@clawsweeper re-review

Rebased onto current main (2ad31a5a). The PR body now has native evidence for the remote-host route (--parallels-host): a normal lease, plus a fault-injected lease that fails at the failing step on this branch and is swallowed on main. It also confirms there is no Linux template on the host, so the Linux limit stands.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/35562160004
• Updated: 2026-09-21T04:46:02.677Z

[steipete]

Landed via #2431: your commits (authorship intact) rebased onto main after the caller-context diagnostics change in #2415 conflicted with this branch, plus a docs/changelog commit thanking you. Verified with the full race suite on a remote Crabbox AWS gate and CI. Thanks @saariuslystoned!