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

Author: drew

crates/openshell-cli/src/main.rs

@@ -1200,6 +1200,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).
@@ -1218,6 +1223,17 @@ enum SandboxCommands {
         #[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<String>,
@@ -2107,20 +2123,31 @@ async fn main() -> Result<()> {
                             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<bool> 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);
                             }

crates/openshell-cli/src/run.rs

@@ -24,13 +24,12 @@ use openshell_bootstrap::{
 use openshell_core::proto::{
     ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, ClearDraftChunksRequest,
     CreateProviderRequest, CreateSandboxRequest, DeleteProviderRequest, DeleteSandboxRequest,
-    ExecSandboxRequest, GetClusterInferenceRequest, GetDraftHistoryRequest,
-    GetDraftPolicyRequest, GetProviderRequest, GetSandboxLogsRequest,
-    GetSandboxPolicyStatusRequest, GetSandboxRequest, HealthRequest, ListProvidersRequest,
-    ListSandboxPoliciesRequest, ListSandboxesRequest, PolicyStatus, Provider,
-    RejectDraftChunkRequest, Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate,
-    SetClusterInferenceRequest, UpdateProviderRequest, UpdateSandboxPolicyRequest,
-    WatchSandboxRequest, exec_sandbox_event,
+    ExecSandboxRequest, GetClusterInferenceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest,
+    GetProviderRequest, GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, GetSandboxRequest,
+    HealthRequest, ListProvidersRequest, ListSandboxPoliciesRequest, ListSandboxesRequest,
+    PolicyStatus, Provider, RejectDraftChunkRequest, Sandbox, SandboxPhase, SandboxPolicy,
+    SandboxSpec, SandboxTemplate, SetClusterInferenceRequest, UpdateProviderRequest,
+    UpdateSandboxPolicyRequest, WatchSandboxRequest, exec_sandbox_event,
 };
 use openshell_providers::{
     ProviderRegistry, detect_provider_from_command, normalize_provider_type,
@@ -2599,6 +2598,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.
@@ -2609,6 +2612,7 @@ pub async fn sandbox_exec_grpc(
     workdir: Option<&str>,
     env_pairs: &[String],
     timeout_seconds: u32,
+    tty_override: Option<bool>,
     tls: &TlsOptions,
 ) -> Result<i32> {
     let mut client = grpc_client(server, tls).await?;
@@ -2624,26 +2628,55 @@ pub async fn sandbox_exec_grpc(
         .sandbox
         .ok_or_else(|| miette::miette!("sandbox not found"))?;
 
+    // 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)
+        ));
+    }
+
     // Parse KEY=VALUE environment pairs into a HashMap.
     let environment: HashMap<String, String> = env_pairs
         .iter()
         .map(|pair| {
             let (k, v) = pair.split_once('=').ok_or_else(|| {
-                miette::miette!("invalid env format '{}': expected KEY=VALUE", pair)
+                miette::miette!(
+                    "invalid env format '{}': expected KEY=VALUE (use KEY= for empty value)",
+                    pair
+                )
             })?;
             Ok((k.to_string(), v.to_string()))
         })
         .collect::<Result<_>>()?;
 
-    // Read stdin if piped (not a TTY).
+    // Read stdin if piped (not a TTY), using spawn_blocking to avoid blocking
+    // the async runtime.
     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 mut buf = Vec::new();
+            std::io::stdin()
+                .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 {
@@ -2653,6 +2686,7 @@ pub async fn sandbox_exec_grpc(
             environment,
             timeout_seconds,
             stdin: stdin_payload,
+            tty,
         })
         .await
         .into_diagnostic()?

crates/openshell-server/src/grpc.rs

@@ -922,6 +922,7 @@ impl OpenShell for OpenShellService {
         let command_str = build_remote_exec_command(&req);
         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();
 
@@ -935,6 +936,7 @@ impl OpenShell for OpenShellService {
                 &command_str,
                 stdin_payload,
                 timeout_seconds,
+                request_tty,
                 &handshake_secret,
             )
             .await
@@ -2825,6 +2827,7 @@ async fn stream_exec_over_ssh(
     command: &str,
     stdin_payload: Vec<u8>,
     timeout_seconds: u32,
+    request_tty: bool,
     handshake_secret: &str,
 ) -> Result<(), Status> {
     info!(
@@ -2869,8 +2872,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
@@ -2948,6 +2956,7 @@ async fn run_exec_with_russh(
     local_proxy_port: u16,
     command: &str,
     stdin_payload: Vec<u8>,
+    request_tty: bool,
     tx: mpsc::Sender<Result<ExecSandboxEvent, Status>>,
 ) -> Result<i32, Status> {
     let stream = TcpStream::connect(("127.0.0.1", local_proxy_port))
@@ -2977,6 +2986,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

proto/openshell.proto

@@ -243,6 +243,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.