From 5090fed06da83f3f131022cdfd81458bd0fa0a44 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Wed, 11 Mar 2026 08:41:06 -0700 Subject: [PATCH 1/2] feat(cli): add sandbox exec subcommand via gRPC ExecSandbox endpoint Add a new 'openshell sandbox exec' command that executes commands inside running sandboxes using the gRPC ExecSandbox streaming endpoint (the same endpoint the Python SDK uses). This provides a lightweight alternative to SSH-based execution for non-interactive command runs. Key features: - Real-time stdout/stderr streaming to the terminal - Exit code propagation (CLI exits with the remote command's exit code) - Piped stdin support (automatically detected when stdin is not a TTY) - Environment variable overrides via --env/-e flags - Working directory override via --workdir - Configurable timeout (exit code 124 on timeout, matching Unix convention) - Last-used sandbox fallback when --name is omitted --- crates/openshell-cli/src/main.rs | 53 +++++++++++++++++ crates/openshell-cli/src/run.rs | 99 ++++++++++++++++++++++++++++++-- 2 files changed, 146 insertions(+), 6 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index c8147645..8c7e9843 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1229,6 +1229,36 @@ enum SandboxCommands { all: bool, }, + /// Execute a command in a running sandbox. + /// + /// Runs a command inside an existing sandbox using the gRPC exec endpoint. + /// Output is streamed to the terminal in real-time. The CLI exits with the + /// remote command's exit code. + /// + /// For interactive shell sessions, use `sandbox connect` instead. + #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + Exec { + /// Sandbox name (defaults to last-used sandbox). + #[arg(long, short = 'n', add = ArgValueCompleter::new(completers::complete_sandbox_names))] + name: Option, + + /// Working directory inside the sandbox. + #[arg(long)] + workdir: Option, + + /// Set environment variables (KEY=VALUE). Can be specified multiple times. + #[arg(long = "env", short = 'e', value_name = "KEY=VALUE")] + env: Vec, + + /// Timeout in seconds (0 = no timeout). + #[arg(long, default_value_t = 0)] + timeout: u32, + + /// Command and arguments to execute. + #[arg(required = true, trailing_var_arg = true, allow_hyphen_values = true)] + command: Vec, + }, + /// Connect to a sandbox. /// /// When no name is given, reconnects to the last-used sandbox. @@ -2307,6 +2337,29 @@ async fn main() -> Result<()> { } let _ = save_last_sandbox(&ctx.name, &name); } + SandboxCommands::Exec { + name, + workdir, + env, + timeout, + command, + } => { + let name = resolve_sandbox_name(name, &ctx.name)?; + let _ = save_last_sandbox(&ctx.name, &name); + let exit_code = run::sandbox_exec_grpc( + endpoint, + &name, + &command, + workdir.as_deref(), + &env, + timeout, + &tls, + ) + .await?; + if exit_code != 0 { + std::process::exit(exit_code); + } + } SandboxCommands::SshConfig { name } => { let name = resolve_sandbox_name(name, &ctx.name)?; run::print_ssh_config(&ctx.name, &name); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 2c06ab94..35492157 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -24,13 +24,13 @@ use openshell_bootstrap::{ use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, ClearDraftChunksRequest, CreateProviderRequest, CreateSandboxRequest, DeleteProviderRequest, DeleteSandboxRequest, - GetClusterInferenceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest, - GetGatewayConfigRequest, GetProviderRequest, GetSandboxConfigRequest, GetSandboxLogsRequest, - GetSandboxPolicyStatusRequest, GetSandboxRequest, HealthRequest, ListProvidersRequest, - ListSandboxPoliciesRequest, ListSandboxesRequest, PolicyStatus, Provider, + ExecSandboxRequest, GetClusterInferenceRequest, GetDraftHistoryRequest, + GetDraftPolicyRequest, GetGatewayConfigRequest, GetProviderRequest, GetSandboxConfigRequest, + GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, GetSandboxRequest, HealthRequest, + ListProvidersRequest, ListSandboxPoliciesRequest, ListSandboxesRequest, PolicyStatus, Provider, RejectDraftChunkRequest, Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, SetClusterInferenceRequest, SettingScope, SettingValue, UpdateConfigRequest, - UpdateProviderRequest, WatchSandboxRequest, setting_value, + UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, setting_value, }; use openshell_core::settings::{self, SettingValueKind}; use openshell_providers::{ @@ -38,7 +38,7 @@ use openshell_providers::{ }; use owo_colors::OwoColorize; use std::collections::{HashMap, HashSet, VecDeque}; -use std::io::{IsTerminal, Write}; +use std::io::{IsTerminal, Read, Write}; use std::path::{Path, PathBuf}; use std::process::Command; use std::time::{Duration, Instant}; @@ -2693,6 +2693,93 @@ pub async fn sandbox_get(server: &str, name: &str, tls: &TlsOptions) -> Result<( Ok(()) } +/// Execute a command in a running sandbox via gRPC, streaming output to the terminal. +/// +/// Returns the remote command's exit code. +pub async fn sandbox_exec_grpc( + server: &str, + name: &str, + command: &[String], + workdir: Option<&str>, + env_pairs: &[String], + timeout_seconds: u32, + tls: &TlsOptions, +) -> Result { + let mut client = grpc_client(server, tls).await?; + + // Resolve sandbox name to id. + let sandbox = client + .get_sandbox(GetSandboxRequest { + name: name.to_string(), + }) + .await + .into_diagnostic()? + .into_inner() + .sandbox + .ok_or_else(|| miette::miette!("sandbox not found"))?; + + // Parse KEY=VALUE environment pairs into a HashMap. + let environment: HashMap = env_pairs + .iter() + .map(|pair| { + let (k, v) = pair.split_once('=').ok_or_else(|| { + miette::miette!("invalid env format '{}': expected KEY=VALUE", pair) + })?; + Ok((k.to_string(), v.to_string())) + }) + .collect::>()?; + + // Read stdin if piped (not a TTY). + let stdin_payload = if !std::io::stdin().is_terminal() { + let mut buf = Vec::new(); + std::io::stdin().read_to_end(&mut buf).into_diagnostic()?; + buf + } else { + Vec::new() + }; + + // Make the streaming gRPC call. + let mut stream = client + .exec_sandbox(ExecSandboxRequest { + sandbox_id: sandbox.id, + command: command.to_vec(), + workdir: workdir.unwrap_or_default().to_string(), + environment, + timeout_seconds, + stdin: stdin_payload, + }) + .await + .into_diagnostic()? + .into_inner(); + + // Stream output to terminal in real-time. + let mut exit_code = 0i32; + let stdout = std::io::stdout(); + let stderr = std::io::stderr(); + + while let Some(event) = stream.next().await { + let event = event.into_diagnostic()?; + match event.payload { + Some(exec_sandbox_event::Payload::Stdout(out)) => { + let mut handle = stdout.lock(); + handle.write_all(&out.data).into_diagnostic()?; + handle.flush().into_diagnostic()?; + } + Some(exec_sandbox_event::Payload::Stderr(err)) => { + let mut handle = stderr.lock(); + handle.write_all(&err.data).into_diagnostic()?; + handle.flush().into_diagnostic()?; + } + Some(exec_sandbox_event::Payload::Exit(exit)) => { + exit_code = exit.exit_code; + } + None => {} + } + } + + Ok(exit_code) +} + /// Print a single YAML line with dimmed keys and regular values. fn print_yaml_line(line: &str) { // Find leading whitespace From ab266c16441ccb90c86562999f054dbc717d845d Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 3 Apr 2026 19:00:14 -0700 Subject: [PATCH 2/2] fix(cli): add TTY support, phase check, and hardening to sandbox exec Add missing --tty/--no-tty flag required by the issue spec, plumbed through proto, CLI, and server-side PTY allocation over SSH. Harden the implementation with a client-side sandbox phase check, a 4 MiB stdin size limit to prevent OOM, and spawn_blocking for stdin reads to avoid blocking the async runtime. Move save_last_sandbox after exec succeeds for consistency. Closes #750 --- crates/openshell-cli/src/main.rs | 35 ++++++++++++---- crates/openshell-cli/src/run.rs | 63 ++++++++++++++++++++--------- crates/openshell-server/src/grpc.rs | 29 ++++++++++++- proto/openshell.proto | 3 ++ 4 files changed, 101 insertions(+), 29 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 8c7e9843..87d377b3 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1236,6 +1236,11 @@ enum SandboxCommands { /// remote command's exit code. /// /// For interactive shell sessions, use `sandbox connect` instead. + /// + /// Examples: + /// openshell sandbox exec --name my-sandbox -- ls -la /workspace + /// openshell sandbox exec -n my-sandbox --workdir /app -- python script.py + /// echo "hello" | openshell sandbox exec -n my-sandbox -- cat #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Exec { /// Sandbox name (defaults to last-used sandbox). @@ -1246,14 +1251,21 @@ enum SandboxCommands { #[arg(long)] workdir: Option, - /// Set environment variables (KEY=VALUE). Can be specified multiple times. - #[arg(long = "env", short = 'e', value_name = "KEY=VALUE")] - env: Vec, - /// Timeout in seconds (0 = no timeout). #[arg(long, default_value_t = 0)] timeout: u32, + /// Allocate a pseudo-terminal for the remote command. + /// Defaults to auto-detection (on when stdin and stdout are terminals). + /// Use --tty to force a PTY even when auto-detection fails, or + /// --no-tty to disable. + #[arg(long, overrides_with = "no_tty")] + tty: bool, + + /// Disable pseudo-terminal allocation. + #[arg(long, overrides_with = "tty")] + no_tty: bool, + /// Command and arguments to execute. #[arg(required = true, trailing_var_arg = true, allow_hyphen_values = true)] command: Vec, @@ -2340,22 +2352,31 @@ async fn main() -> Result<()> { SandboxCommands::Exec { name, workdir, - env, timeout, + tty, + no_tty, command, } => { let name = resolve_sandbox_name(name, &ctx.name)?; - let _ = save_last_sandbox(&ctx.name, &name); + // Resolve --tty / --no-tty into an Option override. + let tty_override = if no_tty { + Some(false) + } else if tty { + Some(true) + } else { + None // auto-detect + }; let exit_code = run::sandbox_exec_grpc( endpoint, &name, &command, workdir.as_deref(), - &env, timeout, + tty_override, &tls, ) .await?; + let _ = save_last_sandbox(&ctx.name, &name); if exit_code != 0 { std::process::exit(exit_code); } diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 35492157..c40640c3 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -24,10 +24,10 @@ use openshell_bootstrap::{ use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, ClearDraftChunksRequest, CreateProviderRequest, CreateSandboxRequest, DeleteProviderRequest, DeleteSandboxRequest, - ExecSandboxRequest, GetClusterInferenceRequest, GetDraftHistoryRequest, - GetDraftPolicyRequest, GetGatewayConfigRequest, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, GetSandboxRequest, HealthRequest, - ListProvidersRequest, ListSandboxPoliciesRequest, ListSandboxesRequest, PolicyStatus, Provider, + ExecSandboxRequest, GetClusterInferenceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest, + GetGatewayConfigRequest, GetProviderRequest, GetSandboxConfigRequest, GetSandboxLogsRequest, + GetSandboxPolicyStatusRequest, GetSandboxRequest, HealthRequest, ListProvidersRequest, + ListSandboxPoliciesRequest, ListSandboxesRequest, PolicyStatus, Provider, RejectDraftChunkRequest, Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, SetClusterInferenceRequest, SettingScope, SettingValue, UpdateConfigRequest, UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, setting_value, @@ -2693,6 +2693,10 @@ pub async fn sandbox_get(server: &str, name: &str, tls: &TlsOptions) -> Result<( Ok(()) } +/// Maximum stdin payload size (4 MiB). Prevents the CLI from reading unbounded +/// data into memory before the server rejects an oversized message. +const MAX_STDIN_PAYLOAD: usize = 4 * 1024 * 1024; + /// Execute a command in a running sandbox via gRPC, streaming output to the terminal. /// /// Returns the remote command's exit code. @@ -2701,8 +2705,8 @@ pub async fn sandbox_exec_grpc( name: &str, command: &[String], workdir: Option<&str>, - env_pairs: &[String], timeout_seconds: u32, + tty_override: Option, tls: &TlsOptions, ) -> Result { let mut client = grpc_client(server, tls).await?; @@ -2718,35 +2722,54 @@ pub async fn sandbox_exec_grpc( .sandbox .ok_or_else(|| miette::miette!("sandbox not found"))?; - // Parse KEY=VALUE environment pairs into a HashMap. - let environment: HashMap = env_pairs - .iter() - .map(|pair| { - let (k, v) = pair.split_once('=').ok_or_else(|| { - miette::miette!("invalid env format '{}': expected KEY=VALUE", pair) - })?; - Ok((k.to_string(), v.to_string())) - }) - .collect::>()?; + // Verify the sandbox is ready before issuing the exec. + if SandboxPhase::try_from(sandbox.phase) != Ok(SandboxPhase::Ready) { + return Err(miette::miette!( + "sandbox '{}' is not ready (phase: {}); wait for it to reach Ready state", + name, + phase_name(sandbox.phase) + )); + } - // Read stdin if piped (not a TTY). + // Read stdin if piped (not a TTY), using spawn_blocking to avoid blocking + // the async runtime. Cap the read at MAX_STDIN_PAYLOAD + 1 so we never + // buffer more than the limit into memory. let stdin_payload = if !std::io::stdin().is_terminal() { - let mut buf = Vec::new(); - std::io::stdin().read_to_end(&mut buf).into_diagnostic()?; - buf + tokio::task::spawn_blocking(|| { + let limit = (MAX_STDIN_PAYLOAD + 1) as u64; + let mut buf = Vec::new(); + std::io::stdin() + .take(limit) + .read_to_end(&mut buf) + .into_diagnostic()?; + if buf.len() > MAX_STDIN_PAYLOAD { + return Err(miette::miette!( + "stdin payload exceeds {} byte limit; pipe smaller inputs or use `sandbox upload`", + MAX_STDIN_PAYLOAD + )); + } + Ok(buf) + }) + .await + .into_diagnostic()?? // first ? unwraps JoinError, second ? unwraps Result } else { Vec::new() }; + // Resolve TTY mode: explicit --tty / --no-tty wins, otherwise auto-detect. + let tty = tty_override + .unwrap_or_else(|| std::io::stdin().is_terminal() && std::io::stdout().is_terminal()); + // Make the streaming gRPC call. let mut stream = client .exec_sandbox(ExecSandboxRequest { sandbox_id: sandbox.id, command: command.to_vec(), workdir: workdir.unwrap_or_default().to_string(), - environment, + environment: HashMap::new(), timeout_seconds, stdin: stdin_payload, + tty, }) .await .into_diagnostic()? diff --git a/crates/openshell-server/src/grpc.rs b/crates/openshell-server/src/grpc.rs index 288fb13d..d7ef4ccf 100644 --- a/crates/openshell-server/src/grpc.rs +++ b/crates/openshell-server/src/grpc.rs @@ -1043,6 +1043,7 @@ impl OpenShell for OpenShellService { .map_err(|e| Status::invalid_argument(format!("command construction failed: {e}")))?; let stdin_payload = req.stdin; let timeout_seconds = req.timeout_seconds; + let request_tty = req.tty; let sandbox_id = sandbox.id; let handshake_secret = self.state.config.ssh_handshake_secret.clone(); @@ -1056,6 +1057,7 @@ impl OpenShell for OpenShellService { &command_str, stdin_payload, timeout_seconds, + request_tty, &handshake_secret, ) .await @@ -3716,6 +3718,7 @@ async fn stream_exec_over_ssh( command: &str, stdin_payload: Vec, timeout_seconds: u32, + request_tty: bool, handshake_secret: &str, ) -> Result<(), Status> { let command_preview: String = command.chars().take(120).collect(); @@ -3764,8 +3767,13 @@ async fn stream_exec_over_ssh( } }; - let exec = - run_exec_with_russh(local_proxy_port, command, stdin_payload.clone(), tx.clone()); + let exec = run_exec_with_russh( + local_proxy_port, + command, + stdin_payload.clone(), + request_tty, + tx.clone(), + ); let exec_result = if timeout_seconds == 0 { exec.await @@ -3843,6 +3851,7 @@ async fn run_exec_with_russh( local_proxy_port: u16, command: &str, stdin_payload: Vec, + request_tty: bool, tx: mpsc::Sender>, ) -> Result { // Defense-in-depth: validate command at the transport boundary even though @@ -3886,6 +3895,22 @@ async fn run_exec_with_russh( .await .map_err(|e| Status::internal(format!("failed to open ssh channel: {e}")))?; + // Request a PTY before exec when the client asked for terminal allocation. + if request_tty { + channel + .request_pty( + false, + "xterm-256color", + 0, // col_width — 0 lets the server decide + 0, // row_height — 0 lets the server decide + 0, // pix_width + 0, // pix_height + &[], + ) + .await + .map_err(|e| Status::internal(format!("failed to allocate PTY: {e}")))?; + } + channel .exec(true, command.as_bytes()) .await diff --git a/proto/openshell.proto b/proto/openshell.proto index 22bd64b7..625a5413 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -247,6 +247,9 @@ message ExecSandboxRequest { // Optional stdin payload passed to the command. bytes stdin = 6; + + // Request a pseudo-terminal for the remote command. + bool tty = 7; } // One stdout chunk from a sandbox exec.