-
Notifications
You must be signed in to change notification settings - Fork 5
feat(ssh): add -o json support for --setup-only #106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Add machine-readable JSON output to `kernel browsers ssh --setup-only` so scripts can extract vm_domain, session_id, ssh_key_file, and proxy_command without fragile grep/awk parsing. When -o json is used with --setup-only, all pterm progress output is suppressed and a clean JSON object is written to stdout. Ephemeral key cleanup is also skipped so the key file remains available for the caller. Co-authored-by: Cursor <cursoragent@cursor.com>
- Guard pterm output in setupVMSSH when quiet/json mode is active, preventing stdout corruption when SSH services are already running - Add missing -o LogLevel=ERROR to the ssh_command in JSON output, matching BuildSSHCommand behavior Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot Autofix prepared fixes for 2 of the 2 bugs found in the latest run.
Or push these changes by commenting: Preview (292d0d7603)diff --git a/cmd/ssh.go b/cmd/ssh.go
--- a/cmd/ssh.go
+++ b/cmd/ssh.go
@@ -176,7 +176,7 @@
if !jsonOutput {
pterm.Info.Println("Setting up SSH services on VM...")
}
- if err := setupVMSSH(ctx, client, browser.SessionID, publicKey); err != nil {
+ if err := setupVMSSH(ctx, client, browser.SessionID, publicKey, jsonOutput); err != nil {
return fmt.Errorf("failed to setup SSH on VM: %w", err)
}
if !jsonOutput {
@@ -186,7 +186,7 @@
if cfg.SetupOnly {
if jsonOutput {
proxyCmd := fmt.Sprintf("websocat --binary wss://%s:2222", vmDomain)
- sshCommand := fmt.Sprintf("ssh -o 'ProxyCommand=%s' -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i %s root@localhost", proxyCmd, keyFile)
+ sshCommand := fmt.Sprintf("ssh -o 'ProxyCommand=%s' -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR -i %s root@localhost", proxyCmd, keyFile)
result := sshSetupResult{
VMDomain: vmDomain,
SessionID: browser.SessionID,
@@ -240,7 +240,7 @@
}
// setupVMSSH installs and configures sshd + websocat on the VM using process.exec
-func setupVMSSH(ctx context.Context, client kernel.Client, sessionID, publicKey string) error {
+func setupVMSSH(ctx context.Context, client kernel.Client, sessionID, publicKey string, jsonOutput bool) error {
// First check if services are already running
checkScript := ssh.CheckServicesScript()
checkResp, err := client.Browsers.Process.Exec(ctx, sessionID, kernel.BrowserProcessExecParams{
@@ -253,7 +253,9 @@
} else if checkResp != nil && checkResp.StdoutB64 != "" {
stdout, _ := base64.StdEncoding.DecodeString(checkResp.StdoutB64)
if strings.TrimSpace(string(stdout)) == "RUNNING" {
- pterm.Info.Println("SSH services already running, injecting key...")
+ if !jsonOutput {
+ pterm.Info.Println("SSH services already running, injecting key...")
+ }
// Just inject the key
return injectSSHKey(ctx, client, sessionID, publicKey)
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before Autofix could start.
| // Cleanup temp key on exit | ||
| if cleanupKey { | ||
| // Cleanup temp key on exit (skip if JSON setup-only, since the caller needs the key) | ||
| if cleanupKey && !(cfg.SetupOnly && jsonOutput) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ephemeral key cleanup condition too narrow for setup-only
Medium Severity
The cleanup guard !(cfg.SetupOnly && jsonOutput) only preserves the ephemeral key file when JSON output is requested. In the non-JSON --setup-only path, the key is still deleted by the deferred os.Remove before the user can use the printed manual SSH command. The condition recognizes "the caller needs the key" (per the comment) but the guard needs to be !cfg.SetupOnly to cover both output modes.



Summary
-o jsonflag tokernel browsers ssh --setup-onlyfor machine-readable outputvm_domain,session_id,ssh_key_file,proxy_command, andssh_commandMotivation
Scripting around
--setup-onlycurrently requires fragilegrep/awkparsing of human-readable output:With
-o json, this becomes:Example output
{ "vm_domain": "actual-vm-domain.onkernel.app", "session_id": "abc123", "ssh_key_file": "/tmp/kernel-scp-key", "proxy_command": "websocat --binary wss://actual-vm-domain.onkernel.app:2222", "ssh_command": "ssh -o 'ProxyCommand=websocat --binary wss://...:2222' -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /tmp/kernel-scp-key root@localhost" }Test plan
kernel browsers ssh <id> -i <key> --setup-only -o jsonand verify clean JSON on stdoutkernel browsers ssh <id> --setup-only -o json(ephemeral key) and verify key file is not cleaned upkernel browsers ssh <id> --setup-onlywithout-oand verify existing behavior is unchangedkernel browsers ssh <id> -o jsonwithout--setup-onlyand verify it errorskernel browsers ssh <id> -o yamland verify it errorsNote
Low Risk
CLI-only behavior changes gated behind
--setup-only -o json, with limited impact on the default interactive SSH path; main risk is leaving temporary key files behind when JSON output is used.Overview
Adds
-o/--output jsonsupport tokernel browsers sshwhen used with--setup-only, emitting a single JSON object (vm domain, session id, key path, and ready-to-run proxy/ssh commands) for scripting.When JSON output is enabled, progress/log messaging is suppressed,
setupVMSSHgains a quiet mode for key-injection/setup messaging, and ephemeral key cleanup is skipped so callers can reuse the generated key file; invalid output formats and-o jsonwithout--setup-onlynow error.Written by Cursor Bugbot for commit 405d824. This will update automatically on new commits. Configure here.