diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f2b3be82..95b8d3d3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes +- Deliver Parallels POSIX guest preparation and SSH-key installation scripts over stdin on local and remote hosts, preserving fail-fast shell checks and preventing child commands from consuming the script. [PR 2401](https://github.com/openclaw/crabbox/pull/2401), [Issue 2396](https://github.com/openclaw/crabbox/issues/2396). Thanks @saariuslystoned. - Restore Lume guest bootstrap with named shared directories, authenticated status readiness, and cleanup with quiet partial-match `lsof` output; existing golden images need refreshed hooks. [PR 2409](https://github.com/openclaw/crabbox/pull/2409). - Lume: persist heartbeat policy across fresh reads, honor explicit idle-timeout changes, and admit owned instance-scoped leases through the public heartbeat command. [PR 2407](https://github.com/openclaw/crabbox/pull/2407). - Persist Tart heartbeat timestamps and explicit idle-timeout changes in the lease claim, preserving them across fresh status reads and cleanup without losing SSH target details. [PR 2405](https://github.com/openclaw/crabbox/pull/2405). diff --git a/docs/providers/parallels.md b/docs/providers/parallels.md index 02166bded..ef8909a15 100644 --- a/docs/providers/parallels.md +++ b/docs/providers/parallels.md @@ -18,6 +18,12 @@ The provider is local-first: by default it drives the `prlctl` on the same Mac that runs Crabbox. Set `parallels.host` to drive a Parallels Desktop install on another Mac over SSH. +Linux and macOS guest preparation streams its scripts over stdin through +`prlctl exec`, including when the Parallels host is reached over SSH. Required +preparation steps fail immediately on error; a later successful readiness check +cannot hide an earlier failure. Commands intended to be best-effort, such as +starting SSH services, retain their existing handling. + **Targets:** Linux, macOS, and Windows (`--windows-mode normal` or `--windows-mode wsl2`). diff --git a/internal/cli/parallels.go b/internal/cli/parallels.go index 4b649aa4f..b9aba5555 100644 --- a/internal/cli/parallels.go +++ b/internal/cli/parallels.go @@ -419,8 +419,8 @@ func (c *ParallelsClient) InstallSSHKey(ctx context.Context, vmID string, cfg Co if cfg.TargetOS == targetWindows { return c.runWindowsPowerShellFile(ctx, vmID, "install-ssh", WindowsBootstrapPowerShell(cfg, publicKey)) } - args := []string{"/bin/sh", "-lc", parallelsPOSIXInstallSSHKeyScript(user, publicKey)} - result, err := c.prlctl(ctx, nil, append([]string{"exec", vmID}, args...)...) + script := parallelsPOSIXInstallSSHKeyScript(user, publicKey) + result, err := c.prlctlWithStdin(ctx, strings.NewReader(script), nil, "exec", vmID, "/bin/sh", "-s") if err != nil { return commandOutputError("parallels install ssh key", result, err) } @@ -529,7 +529,8 @@ func (c *ParallelsClient) EnsureGuestReady(ctx context.Context, vmID string, cfg workRoot = baseConfig().WorkRoot } desktop := cfg.Desktop - result, err := c.prlctl(ctx, nil, "exec", vmID, "/bin/sh", "-lc", parallelsPOSIXEnsureReadyScript(user, workRoot, desktop, cfg.TargetOS == targetMacOS && cfg.Parallels.Password != "", sshPortCandidates(cfg.SSHPort, cfg.SSHFallbackPorts))) + script := parallelsPOSIXEnsureReadyScript(user, workRoot, desktop, cfg.TargetOS == targetMacOS && cfg.Parallels.Password != "", sshPortCandidates(cfg.SSHPort, cfg.SSHFallbackPorts)) + result, err := c.prlctlWithStdin(ctx, strings.NewReader(script), nil, "exec", vmID, "/bin/sh", "-s") if err != nil { return commandOutputError("parallels guest prep", result, err) } @@ -762,13 +763,19 @@ func (c *ParallelsClient) probeHostTCP(ctx context.Context, ip, port string) err return nil } +// Both preparation scripts are handed to the guest shell on stdin, never as an +// argv element: `prlctl exec` flattens argv and re-parses it guest-side, which +// silently drops the leading `set -eu`. Every child either script runs must +// therefore take its stdin from /dev/null, or it eats the rest of the script +// while the shell still exits 0. +// https://github.com/openclaw/crabbox/issues/2396 func parallelsPOSIXInstallSSHKeyScript(user, publicKey string) string { return fmt.Sprintf(`set -eu user=%s key=%s -home=$(getent passwd "$user" 2>/dev/null | cut -d: -f6 || true) +home=$(getent passwd "$user" /dev/null | cut -d: -f6 || true) if [ -z "$home" ]; then - home=$(dscl . -read "/Users/$user" NFSHomeDirectory 2>/dev/null | awk '{print $2}' || true) + home=$(dscl . -read "/Users/$user" NFSHomeDirectory /dev/null | awk '{print $2}' || true) fi if [ -z "$home" ]; then echo "user home not found: $user" >&2 @@ -787,9 +794,9 @@ printf '%%s\n' "$user" >/var/lib/crabbox/ssh.username 2>/dev/null || true func parallelsMacOSDesktopReadyTest(accountCredentials bool) string { if accountCredentials { - return "[ -f /var/db/crabbox/vnc.console ] && nc -z 127.0.0.1 5900" + return "[ -f /var/db/crabbox/vnc.console ] && nc -z 127.0.0.1 5900 &2; exit 1 ;; @@ -812,23 +819,23 @@ func parallelsMacOSDesktopSetupScript(accountCredentials bool) string { mkdir -p /etc/sudoers.d printf '%s ALL=(root) NOPASSWD: /bin/cat /var/db/crabbox/vnc.password\n' "$user" >/etc/sudoers.d/crabbox-vnc-password chmod 0440 /etc/sudoers.d/crabbox-vnc-password - /usr/sbin/visudo -cf /etc/sudoers.d/crabbox-vnc-password >/dev/null + /usr/sbin/visudo -cf /etc/sudoers.d/crabbox-vnc-password /dev/null ` - clientOptions = ` "$kickstart" -configure -clientopts -setdirlogins -dirlogins no -setvnclegacy -vnclegacy yes -setvncpw -vncpw "$vnc_password" >/dev/null 2>&1 + clientOptions = ` "$kickstart" -configure -clientopts -setdirlogins -dirlogins no -setvnclegacy -vnclegacy yes -setvncpw -vncpw "$vnc_password" /dev/null 2>&1 ` } return ` mkdir -p /var/db/crabbox ` + credentialSetup + ` kickstart=/System/Library/CoreServices/RemoteManagement/ARDAgent.app/Contents/Resources/kickstart [ -x "$kickstart" ] - /usr/bin/defaults write /Library/Preferences/com.apple.RemoteManagement VNCAlwaysStartOnConsole -bool true - "$kickstart" -activate -configure -allowAccessFor -specifiedUsers >/dev/null 2>&1 - "$kickstart" -configure -access -on -users "$user" -privs -all >/dev/null 2>&1 -` + clientOptions + ` "$kickstart" -restart -agent >/dev/null 2>&1 - /bin/launchctl enable system/com.apple.screensharing >/dev/null 2>&1 || true - /bin/launchctl kickstart -k system/com.apple.screensharing >/dev/null 2>&1 || true + /usr/bin/defaults write /Library/Preferences/com.apple.RemoteManagement VNCAlwaysStartOnConsole -bool true /dev/null 2>&1 + "$kickstart" -configure -access -on -users "$user" -privs -all /dev/null 2>&1 +` + clientOptions + ` "$kickstart" -restart -agent /dev/null 2>&1 + /bin/launchctl enable system/com.apple.screensharing /dev/null 2>&1 || true + /bin/launchctl kickstart -k system/com.apple.screensharing /dev/null 2>&1 || true vnc_ready=false for _ in $(jot 60 1); do - if nc -z 127.0.0.1 5900; then + if nc -z 127.0.0.1 5900 /dev/null 2>&1; then # Resolve it as that user through bash -lc, matching the probe exactly # rather than the user's default login shell, which reads different rc # files. Never source their login files as root, and keep stdin off these - # children -- this whole script arrives on stdin via sudo -n /bin/sh -s, so + # children -- this whole script arrives on the guest shell's stdin, so # anything reading stdin silently eats the rest of it. # # Ask node for its own execPath rather than taking what command -v returns. @@ -956,14 +963,14 @@ crabbox_ssh_listening() { return 0 fi for port in %s; do - if nc -z 127.0.0.1 "$port" >/dev/null 2>&1; then + if nc -z 127.0.0.1 "$port" /dev/null 2>&1; then return 0 fi done return 1 } %s -if [ -x /usr/local/bin/crabbox-ready ] && /usr/local/bin/crabbox-ready >/tmp/crabbox-ready.log 2>&1 && crabbox_ssh_listening; then +if [ -x /usr/local/bin/crabbox-ready ] && /usr/local/bin/crabbox-ready /tmp/crabbox-ready.log 2>&1 && crabbox_ssh_listening; then if [ "$desktop" != true ]; then exit 0 fi @@ -971,7 +978,7 @@ if [ -x /usr/local/bin/crabbox-ready ] && /usr/local/bin/crabbox-ready >/tmp/cra if %s; then exit 0 fi - elif command -v websockify >/dev/null 2>&1 && command -v x11vnc >/dev/null 2>&1 && { [ -f /usr/share/novnc/vnc.html ] || [ -f /usr/share/novnc/core/vnc.html ] || [ -f /usr/share/novnc/html/vnc.html ]; } && systemctl is-active --quiet crabbox-x11vnc.service; then + elif command -v websockify >/dev/null 2>&1 && command -v x11vnc >/dev/null 2>&1 && { [ -f /usr/share/novnc/vnc.html ] || [ -f /usr/share/novnc/core/vnc.html ] || [ -f /usr/share/novnc/html/vnc.html ]; } && systemctl is-active --quiet crabbox-x11vnc.service /var/lib/crabbox/vnc.password + openssl rand -hex 16 /var/lib/crabbox/vnc.password fi { head -c 8 /var/lib/crabbox/vnc.password; printf '\n'; head -c 8 /var/lib/crabbox/vnc.password; printf '\n\n'; } | x11vnc -storepasswd /var/lib/crabbox/vnc.pass >/dev/null 2>&1 chown "$user:$group" /var/lib/crabbox/vnc.password /var/lib/crabbox/vnc.pass @@ -1040,19 +1047,19 @@ RestartSec=1 [Install] WantedBy=multi-user.target UNIT - systemctl daemon-reload - systemctl enable --now crabbox-xvfb.service crabbox-desktop.service crabbox-x11vnc.service + systemctl daemon-reload /dev/null 2>&1 || true - systemctl restart ssh >/dev/null 2>&1 || systemctl restart ssh.socket >/dev/null 2>&1 || true + systemctl enable ssh /dev/null 2>&1 || true + systemctl restart ssh /dev/null 2>&1 || systemctl restart ssh.socket /dev/null 2>&1 || true fi if command -v sw_vers >/dev/null 2>&1; then mkdir -p /usr/local/bin remote_login_log=/tmp/crabbox-remote-login.log - /bin/launchctl load -w /System/Library/LaunchDaemons/ssh.plist >"$remote_login_log" 2>&1 || - /bin/launchctl bootstrap system /System/Library/LaunchDaemons/ssh.plist >>"$remote_login_log" 2>&1 || true - /bin/launchctl enable system/com.openssh.sshd >>"$remote_login_log" 2>&1 || true - /bin/launchctl kickstart -k system/com.openssh.sshd >>"$remote_login_log" 2>&1 || true + /bin/launchctl load -w /System/Library/LaunchDaemons/ssh.plist "$remote_login_log" 2>&1 || + /bin/launchctl bootstrap system /System/Library/LaunchDaemons/ssh.plist >"$remote_login_log" 2>&1 || true + /bin/launchctl enable system/com.openssh.sshd >"$remote_login_log" 2>&1 || true + /bin/launchctl kickstart -k system/com.openssh.sshd >"$remote_login_log" 2>&1 || true if [ "$desktop" = true ]; then %s fi @@ -1087,7 +1094,7 @@ READY fi chmod 0755 /usr/local/bin/crabbox-ready touch /var/lib/crabbox/bootstrapped 2>/dev/null || true -/usr/local/bin/crabbox-ready +/usr/local/bin/crabbox-ready /usr/local/bin/crabbox-ready", "apt-get install", "test -w '/work/test'"} { + if argv := strings.Join(runner.lastReq.Args, " "); argv != "exec vm1 /bin/sh -s" { + t.Fatalf("argv=%q", argv) + } + got := runner.lastStdin + for _, want := range []string{"desktop=false", "cat >/usr/local/bin/crabbox-ready", "apt-get install", "test -w '/work/test'"} { if !strings.Contains(got, want) { t.Fatalf("guest prep command missing %q:\n%s", want, got) } @@ -624,7 +627,7 @@ func TestParallelsEnsureGuestReadyUpgradesReadyGuestForDesktop(t *testing.T) { if err != nil { t.Fatal(err) } - got := strings.Join(runner.lastReq.Args, "\n") + got := runner.lastStdin for _, want := range []string{ "desktop=true", "command -v websockify", @@ -650,7 +653,7 @@ func TestParallelsEnsureGuestReadyEnablesMacOSRemoteLogin(t *testing.T) { if err != nil { t.Fatal(err) } - got := strings.Join(runner.lastReq.Args, "\n") + got := runner.lastStdin for _, want := range []string{"launchctl load -w /System/Library/LaunchDaemons/ssh.plist", "launchctl enable system/com.openssh.sshd", "launchctl kickstart -k system/com.openssh.sshd"} { if !strings.Contains(got, want) { t.Fatalf("macOS guest prep missing %q:\n%s", want, got) @@ -670,7 +673,7 @@ func TestParallelsEnsureGuestReadyVerifiesMacOSSSHListener(t *testing.T) { if err != nil { t.Fatal(err) } - got := strings.Join(runner.lastReq.Args, "\n") + got := runner.lastStdin // Best-effort launchctl calls do not establish listener availability. // Authenticated SSH readiness remains a separate, later check. for _, want := range []string{ @@ -703,7 +706,7 @@ func TestParallelsEnsureGuestReadyRechecksMacOSSSHListenerWhenHelperExists(t *te if err != nil { t.Fatal(err) } - got := strings.Join(runner.lastReq.Args, "\n") + got := runner.lastStdin // A guest prepared by an older crabbox carries a crabbox-ready that predates // the listener probe. If the early exit trusts that helper alone, such a // guest skips remote-login setup entirely and the new check never runs. @@ -730,7 +733,7 @@ func TestParallelsEnsureGuestReadyEnablesMacOSScreenSharing(t *testing.T) { if err != nil { t.Fatal(err) } - got := strings.Join(runner.lastReq.Args, "\n") + got := runner.lastStdin for _, want := range []string{ "desktop=true", "mkdir -p /var/db/crabbox", @@ -770,7 +773,7 @@ func TestParallelsEnsureGuestReadyUsesMacOSAccountCredentialsWithoutReset(t *tes if err != nil { t.Fatal(err) } - got := strings.Join(runner.lastReq.Args, "\n") + got := runner.lastStdin for _, want := range []string{ "-access -on -users \"$user\" -privs -all", "VNCAlwaysStartOnConsole -bool true", @@ -1052,12 +1055,25 @@ type parallelsFakeRunner struct { stdout string deleteCalled bool lastReq LocalCommandRequest + lastStdin string requests []LocalCommandRequest + stdins []string } func (r *parallelsFakeRunner) Run(_ context.Context, req LocalCommandRequest) (LocalCommandResult, error) { + // Drain stdin the way a real child would, so a test can assert on what the + // command was handed rather than on a reader nobody consumed. + r.lastStdin = "" + if req.Stdin != nil { + data, err := io.ReadAll(req.Stdin) + if err != nil { + return LocalCommandResult{}, err + } + r.lastStdin = string(data) + } r.lastReq = req r.requests = append(r.requests, req) + r.stdins = append(r.stdins, r.lastStdin) if len(req.Args) > 0 && req.Args[0] == "delete" { r.deleteCalled = true } @@ -1226,3 +1242,140 @@ test -w '/work/crabbox' t.Fatal("Linux crabbox-ready changed; the Node baseline is macOS-only") } } + +// `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, so a +// script handed over as a single argv element loses its word boundaries and its +// leading `set -eu` is swallowed as arguments to an inner shell. stdin is +// preserved verbatim, so both preparation scripts have to travel there. +// https://github.com/openclaw/crabbox/issues/2396 +// +// The runner is mocked, so this cannot observe the guest-side reconstruction +// itself; it pins the transport crabbox chooses, which is the part that is +// wrong today. +func TestParallelsGuestPrepScriptsTravelOnStdinNotArgv(t *testing.T) { + steps := []struct { + name string + run func(*ParallelsClient) error + marker string + }{ + { + name: "install ssh key", + run: func(c *ParallelsClient) error { + return c.InstallSSHKey(context.Background(), "vm1", Config{SSHUser: "runner", TargetOS: targetLinux}, "ssh-ed25519 AAAAlease") + }, + marker: `chmod 600 "$home/.ssh/authorized_keys"`, + }, + { + name: "ensure guest ready", + run: func(c *ParallelsClient) error { + return c.EnsureGuestReady(context.Background(), "vm1", Config{SSHUser: "runner", WorkRoot: "/work/test", TargetOS: targetLinux}) + }, + marker: "cat >/usr/local/bin/crabbox-ready", + }, + } + routes := []struct { + name string + cfg Config + wantName string + }{ + {name: "local", cfg: Config{}, wantName: "prlctl"}, + { + name: "remote", + cfg: Config{Parallels: ParallelsConfig{Host: "mac.example", HostUser: "build", HostKey: "/Users/build/.ssh/host"}}, + wantName: directSSHExecutable(), + }, + } + for _, route := range routes { + for _, step := range steps { + t.Run(route.name+"/"+step.name, func(t *testing.T) { + runner := ¶llelsFakeRunner{} + client := NewParallelsClient(route.cfg, runner) + if err := step.run(client); err != nil { + t.Fatal(err) + } + req := runner.lastReq + if req.Name != route.wantName { + t.Fatalf("name=%q want %q", req.Name, route.wantName) + } + + script := runner.lastStdin + if !strings.HasPrefix(script, "set -eu\n") { + t.Fatalf("script does not reach the guest shell on stdin; got %d bytes: %q", len(script), script) + } + if !strings.Contains(script, step.marker) { + t.Fatalf("script on stdin is missing %q:\n%s", step.marker, script) + } + + argv := strings.Join(append([]string{req.Name}, req.Args...), "\n") + for _, banned := range []string{"set -eu", step.marker, "-lc"} { + if strings.Contains(argv, banned) { + t.Fatalf("script body travels in argv, where prlctl exec re-parses it: %q present in\n%s", banned, argv) + } + } + + if route.wantName == "prlctl" { + if got, want := strings.Join(req.Args, " "), "exec vm1 /bin/sh -s"; got != want { + t.Fatalf("local prlctl argv=%q want %q", got, want) + } + return + } + // The remote route keeps its host selection, key handling and the + // PATH prefix prlctl needs on a non-login shell. + if len(req.Args) < 2 { + t.Fatalf("remote ssh argv too short: %#v", req.Args) + } + if got, want := req.Args[len(req.Args)-2], "build@mac.example"; got != want { + t.Fatalf("remote host=%q want %q", got, want) + } + remote := req.Args[len(req.Args)-1] + for _, want := range []string{"PATH=/usr/local/bin:", `'prlctl' 'exec' 'vm1' '/bin/sh' '-s'`} { + if !strings.Contains(remote, want) { + t.Fatalf("remote command missing %q: %s", want, remote) + } + } + }) + } + } +} + +// Both preparation scripts now arrive on stdin on the prlctl routes too, so the +// same rule PR https://github.com/openclaw/crabbox/pull/2387 established for the +// macOS Node children applies to every child either script runs: one that reads +// stdin silently eats the remainder of the script while the shell still exits 0. +// The Linux branch was never reachable over stdin before, and `apt-get` is the +// obvious offender. +func TestParallelsPrepScriptsKeepStdinOffEveryChild(t *testing.T) { + ready := parallelsPOSIXEnsureReadyScript("parallels-01", "/work/test", true, false, []string{"22", "2222"}) + for _, want := range []string{ + // The readiness gate runs mid-script; anything it consumes is lost. So + // does the SSH-listener probe from + // https://github.com/openclaw/crabbox/pull/2399 that guards it. + "/usr/local/bin/crabbox-ready /tmp/crabbox-ready.log 2>&1", + `nc -z 127.0.0.1 "$port" /dev/null 2>&1`, + "nc -z 127.0.0.1 5900 "$remote_login_log" 2>&1`, + `"$kickstart" -activate -configure -allowAccessFor -specifiedUsers /dev/null 2>&1`, + } { + if !strings.Contains(ready, want) { + t.Fatalf("child may consume the script from stdin: missing %q", want) + } + } + + install := parallelsPOSIXInstallSSHKeyScript("parallels-01", "ssh-ed25519 AAAAlease") + for _, want := range []string{ + `getent passwd "$user" /dev/null`, + `dscl . -read "/Users/$user" NFSHomeDirectory /dev/null`, + } { + if !strings.Contains(install, want) { + t.Fatalf("child may consume the script from stdin: missing %q", want) + } + } +}