From ad87da4369cc7ffe05bf763db57c8c3c9f88a352 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 3 Apr 2026 19:01:36 -0700 Subject: [PATCH 1/2] feat(sandbox): persist startup command across gateway stop/start cycles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-provided command from `sandbox create -- ` was previously ephemeral—executed via SSH on first connect but lost when the sandbox pod was recreated after a gateway restart. This change persists the command in the SandboxSpec protobuf, stores it in both SQLite and the Kubernetes CRD, and sets it as OPENSHELL_SANDBOX_COMMAND in the pod spec. When the CRD controller recreates the pod after a gateway stop/start, the supervisor re-executes the stored command instead of falling back to `sleep infinity`. --- crates/openshell-cli/src/run.rs | 52 +++----------- crates/openshell-cli/src/ssh.rs | 1 + crates/openshell-server/src/sandbox/mod.rs | 82 +++++++++++++++++++++- proto/datamodel.proto | 3 + 4 files changed, 95 insertions(+), 43 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 2c06ab94..a9a9302e 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1936,7 +1936,7 @@ pub async fn sandbox_create( policy: Option<&str>, forward: Option, command: &[String], - tty_override: Option, + _tty_override: Option, bootstrap_override: Option, auto_providers_override: Option, tls: &TlsOptions, @@ -2038,6 +2038,7 @@ pub async fn sandbox_create( policy, providers: configured_providers, template, + command: command.to_vec(), ..SandboxSpec::default() }), name: name.unwrap_or_default().to_string(), @@ -2374,49 +2375,16 @@ pub async fn sandbox_create( return Ok(()); } - if command.is_empty() { - let connect_result = if persist { - sandbox_connect(&effective_server, &sandbox_name, &effective_tls).await - } else { - crate::ssh::sandbox_connect_without_exec( - &effective_server, - &sandbox_name, - &effective_tls, - ) - .await - }; - - return finalize_sandbox_create_session( - &effective_server, - &sandbox_name, - persist, - connect_result, - &effective_tls, - gateway_name, - ) - .await; - } - - // Resolve TTY mode: explicit --tty / --no-tty wins, otherwise - // auto-detect from the local terminal. - let tty = tty_override.unwrap_or_else(|| { - std::io::stdin().is_terminal() && std::io::stdout().is_terminal() - }); - let exec_result = if persist { - sandbox_exec( - &effective_server, - &sandbox_name, - command, - tty, - &effective_tls, - ) - .await + // When a command is provided it is persisted in the sandbox spec + // and runs as the entrypoint via OPENSHELL_SANDBOX_COMMAND. We + // always open an interactive shell here so the user can inspect + // the sandbox without executing the command a second time. + let connect_result = if persist { + sandbox_connect(&effective_server, &sandbox_name, &effective_tls).await } else { - crate::ssh::sandbox_exec_without_exec( + crate::ssh::sandbox_connect_without_exec( &effective_server, &sandbox_name, - command, - tty, &effective_tls, ) .await @@ -2426,7 +2394,7 @@ pub async fn sandbox_create( &effective_server, &sandbox_name, persist, - exec_result, + connect_result, &effective_tls, gateway_name, ) diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index ebcbbeb4..53a0f90b 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -437,6 +437,7 @@ pub async fn sandbox_exec( sandbox_exec_with_mode(server, name, command, tty, tls, true).await } +#[allow(dead_code)] pub(crate) async fn sandbox_exec_without_exec( server: &str, name: &str, diff --git a/crates/openshell-server/src/sandbox/mod.rs b/crates/openshell-server/src/sandbox/mod.rs index c5e9a833..4bc40fa4 100644 --- a/crates/openshell-server/src/sandbox/mod.rs +++ b/crates/openshell-server/src/sandbox/mod.rs @@ -920,6 +920,7 @@ fn sandbox_to_k8s_spec( sandbox_id, sandbox_name, grpc_endpoint, + &spec.command, ssh_listen_addr, ssh_handshake_secret, ssh_handshake_skew_secs, @@ -953,7 +954,11 @@ fn sandbox_to_k8s_spec( // podTemplate is required by the Kubernetes CRD - ensure it's always present if !root.contains_key("podTemplate") { let empty_env = std::collections::HashMap::new(); + let empty_cmd: Vec = Vec::new(); let spec_env = spec.as_ref().map_or(&empty_env, |s| &s.environment); + let spec_cmd = spec + .as_ref() + .map_or(empty_cmd.as_slice(), |s| s.command.as_slice()); root.insert( "podTemplate".to_string(), sandbox_template_to_k8s( @@ -964,6 +969,7 @@ fn sandbox_to_k8s_spec( sandbox_id, sandbox_name, grpc_endpoint, + spec_cmd, ssh_listen_addr, ssh_handshake_secret, ssh_handshake_skew_secs, @@ -989,6 +995,7 @@ fn sandbox_template_to_k8s( sandbox_id: &str, sandbox_name: &str, grpc_endpoint: &str, + sandbox_command: &[String], ssh_listen_addr: &str, ssh_handshake_secret: &str, ssh_handshake_skew_secs: u64, @@ -1045,6 +1052,7 @@ fn sandbox_template_to_k8s( sandbox_id, sandbox_name, grpc_endpoint, + sandbox_command, ssh_listen_addr, ssh_handshake_secret, ssh_handshake_skew_secs, @@ -1176,6 +1184,7 @@ fn build_env_list( sandbox_id: &str, sandbox_name: &str, grpc_endpoint: &str, + sandbox_command: &[String], ssh_listen_addr: &str, ssh_handshake_secret: &str, ssh_handshake_skew_secs: u64, @@ -1189,6 +1198,7 @@ fn build_env_list( sandbox_id, sandbox_name, grpc_endpoint, + sandbox_command, ssh_listen_addr, ssh_handshake_secret, ssh_handshake_skew_secs, @@ -1211,6 +1221,7 @@ fn apply_required_env( sandbox_id: &str, sandbox_name: &str, grpc_endpoint: &str, + sandbox_command: &[String], ssh_listen_addr: &str, ssh_handshake_secret: &str, ssh_handshake_skew_secs: u64, @@ -1219,7 +1230,14 @@ fn apply_required_env( upsert_env(env, "OPENSHELL_SANDBOX_ID", sandbox_id); upsert_env(env, "OPENSHELL_SANDBOX", sandbox_name); upsert_env(env, "OPENSHELL_ENDPOINT", grpc_endpoint); - upsert_env(env, "OPENSHELL_SANDBOX_COMMAND", "sleep infinity"); + // Use the user-provided command if present, otherwise fall back to + // `sleep infinity` so the sandbox pod stays alive for interactive SSH. + let command_value = if sandbox_command.is_empty() { + "sleep infinity".to_string() + } else { + sandbox_command.join(" ") + }; + upsert_env(env, "OPENSHELL_SANDBOX_COMMAND", &command_value); if !ssh_listen_addr.is_empty() { upsert_env(env, "OPENSHELL_SSH_LISTEN_ADDR", ssh_listen_addr); } @@ -1617,6 +1635,7 @@ mod tests { "sandbox-1", "my-sandbox", "https://endpoint:8080", + &[], "0.0.0.0:2222", "my-secret-value", 300, @@ -1635,6 +1654,58 @@ mod tests { ); } + #[test] + fn apply_required_env_uses_sleep_infinity_when_no_command() { + let mut env = Vec::new(); + apply_required_env( + &mut env, + "sandbox-1", + "my-sandbox", + "https://endpoint:8080", + &[], + "0.0.0.0:2222", + "secret", + 300, + false, + ); + + let cmd_entry = env + .iter() + .find(|e| e.get("name").and_then(|v| v.as_str()) == Some("OPENSHELL_SANDBOX_COMMAND")) + .expect("OPENSHELL_SANDBOX_COMMAND must be present in env"); + assert_eq!( + cmd_entry.get("value").and_then(|v| v.as_str()), + Some("sleep infinity"), + "default sandbox command should be 'sleep infinity'" + ); + } + + #[test] + fn apply_required_env_uses_user_command_when_provided() { + let mut env = Vec::new(); + apply_required_env( + &mut env, + "sandbox-1", + "my-sandbox", + "https://endpoint:8080", + &["python".to_string(), "app.py".to_string()], + "0.0.0.0:2222", + "secret", + 300, + false, + ); + + let cmd_entry = env + .iter() + .find(|e| e.get("name").and_then(|v| v.as_str()) == Some("OPENSHELL_SANDBOX_COMMAND")) + .expect("OPENSHELL_SANDBOX_COMMAND must be present in env"); + assert_eq!( + cmd_entry.get("value").and_then(|v| v.as_str()), + Some("python app.py"), + "sandbox command should reflect user-provided command" + ); + } + #[test] fn supervisor_sideload_injects_run_as_user_zero() { let mut pod_template = serde_json::json!({ @@ -1747,6 +1818,7 @@ mod tests { "sandbox-1", "my-sandbox", "https://endpoint:8080", + &[], "0.0.0.0:2222", "secret", 300, @@ -1795,6 +1867,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1829,6 +1902,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1859,6 +1933,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1902,6 +1977,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1929,6 +2005,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1960,6 +2037,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1986,6 +2064,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -2126,6 +2205,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, diff --git a/proto/datamodel.proto b/proto/datamodel.proto index 2232a122..9a4c8ef9 100644 --- a/proto/datamodel.proto +++ b/proto/datamodel.proto @@ -33,6 +33,9 @@ message SandboxSpec { repeated string providers = 8; // Request NVIDIA GPU resources for this sandbox. bool gpu = 9; + // User-provided startup command. Persisted so it is re-executed when the + // sandbox pod is recreated (e.g. after gateway stop/start). + repeated string command = 10; } // Sandbox template mapped onto Kubernetes pod template inputs. From b4ba7335b494dee73c4fccd340fe9c6a73e13eba Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sat, 4 Apr 2026 21:30:21 -0700 Subject: [PATCH 2/2] fix(sandbox): deliver user command via K8s args instead of env var MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The join(" ")/split_whitespace() round-trip through OPENSHELL_SANDBOX_COMMAND mangled arguments containing embedded spaces (e.g. python -c "print('hello world')"). Pass the command as K8s container args instead, which delivers each token as a separate argv entry — preserving argument boundaries exactly with no shell interpretation or encoding. --- crates/openshell-sandbox/src/main.rs | 6 +- crates/openshell-server/src/sandbox/mod.rs | 114 +++++++++++++++------ 2 files changed, 89 insertions(+), 31 deletions(-) diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index cdf5f6ff..30fea05a 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -163,11 +163,13 @@ async fn main() -> Result<()> { None }; - // Get command - either from CLI args, environment variable, or default to /bin/bash + // Get command - prefer CLI args (delivered via K8s container `args` which + // preserves argument boundaries exactly), then fall back to the + // OPENSHELL_SANDBOX_COMMAND env var (always `sleep infinity` when set by + // the server), then `/bin/bash` as a last resort. let command = if !args.command.is_empty() { args.command } else if let Ok(c) = std::env::var("OPENSHELL_SANDBOX_COMMAND") { - // Simple shell-like splitting on whitespace c.split_whitespace().map(String::from).collect() } else { vec!["/bin/bash".to_string()] diff --git a/crates/openshell-server/src/sandbox/mod.rs b/crates/openshell-server/src/sandbox/mod.rs index 4bc40fa4..fc909a11 100644 --- a/crates/openshell-server/src/sandbox/mod.rs +++ b/crates/openshell-server/src/sandbox/mod.rs @@ -1052,7 +1052,6 @@ fn sandbox_template_to_k8s( sandbox_id, sandbox_name, grpc_endpoint, - sandbox_command, ssh_listen_addr, ssh_handshake_secret, ssh_handshake_skew_secs, @@ -1061,6 +1060,15 @@ fn sandbox_template_to_k8s( container.insert("env".to_string(), serde_json::Value::Array(env)); + // Pass the user's command as container args so it reaches the sandbox + // binary as argv elements (via clap's trailing_var_arg). This preserves + // argument boundaries exactly — no shell interpretation, no whitespace + // splitting — because Kubernetes delivers `args` entries as separate + // argv strings to the process specified in `command`. + if !sandbox_command.is_empty() { + container.insert("args".to_string(), serde_json::json!(sandbox_command)); + } + // The sandbox process needs SYS_ADMIN (for seccomp filter installation and // network namespace creation), NET_ADMIN (for network namespace veth setup), // SYS_PTRACE (for the CONNECT proxy to read /proc//fd/ of sandbox-user @@ -1184,7 +1192,6 @@ fn build_env_list( sandbox_id: &str, sandbox_name: &str, grpc_endpoint: &str, - sandbox_command: &[String], ssh_listen_addr: &str, ssh_handshake_secret: &str, ssh_handshake_skew_secs: u64, @@ -1198,7 +1205,6 @@ fn build_env_list( sandbox_id, sandbox_name, grpc_endpoint, - sandbox_command, ssh_listen_addr, ssh_handshake_secret, ssh_handshake_skew_secs, @@ -1221,7 +1227,6 @@ fn apply_required_env( sandbox_id: &str, sandbox_name: &str, grpc_endpoint: &str, - sandbox_command: &[String], ssh_listen_addr: &str, ssh_handshake_secret: &str, ssh_handshake_skew_secs: u64, @@ -1230,14 +1235,12 @@ fn apply_required_env( upsert_env(env, "OPENSHELL_SANDBOX_ID", sandbox_id); upsert_env(env, "OPENSHELL_SANDBOX", sandbox_name); upsert_env(env, "OPENSHELL_ENDPOINT", grpc_endpoint); - // Use the user-provided command if present, otherwise fall back to - // `sleep infinity` so the sandbox pod stays alive for interactive SSH. - let command_value = if sandbox_command.is_empty() { - "sleep infinity".to_string() - } else { - sandbox_command.join(" ") - }; - upsert_env(env, "OPENSHELL_SANDBOX_COMMAND", &command_value); + // Default fallback command so the sandbox pod stays alive for interactive + // SSH when no user command is provided. When the user *does* supply a + // command it is delivered as K8s container `args` (preserving argument + // boundaries exactly) and the sandbox binary's clap parser picks it up + // from argv before ever consulting this env var. + upsert_env(env, "OPENSHELL_SANDBOX_COMMAND", "sleep infinity"); if !ssh_listen_addr.is_empty() { upsert_env(env, "OPENSHELL_SSH_LISTEN_ADDR", ssh_listen_addr); } @@ -1635,7 +1638,6 @@ mod tests { "sandbox-1", "my-sandbox", "https://endpoint:8080", - &[], "0.0.0.0:2222", "my-secret-value", 300, @@ -1655,14 +1657,13 @@ mod tests { } #[test] - fn apply_required_env_uses_sleep_infinity_when_no_command() { + fn apply_required_env_always_sets_sleep_infinity() { let mut env = Vec::new(); apply_required_env( &mut env, "sandbox-1", "my-sandbox", "https://endpoint:8080", - &[], "0.0.0.0:2222", "secret", 300, @@ -1676,33 +1677,89 @@ mod tests { assert_eq!( cmd_entry.get("value").and_then(|v| v.as_str()), Some("sleep infinity"), - "default sandbox command should be 'sleep infinity'" + "OPENSHELL_SANDBOX_COMMAND should always be 'sleep infinity' (user commands are delivered via K8s args)" ); } #[test] - fn apply_required_env_uses_user_command_when_provided() { - let mut env = Vec::new(); - apply_required_env( - &mut env, - "sandbox-1", - "my-sandbox", - "https://endpoint:8080", - &["python".to_string(), "app.py".to_string()], + fn user_command_delivered_as_container_args() { + let user_cmd = vec![ + "python".to_string(), + "-c".to_string(), + "print('hello world')".to_string(), + ]; + let pod_template = sandbox_template_to_k8s( + &SandboxTemplate::default(), + false, + "openshell/sandbox:latest", + "", + "sandbox-id", + "sandbox-name", + "https://gateway.example.com", + &user_cmd, "0.0.0.0:2222", "secret", 300, - false, + &std::collections::HashMap::new(), + "", + "", + true, ); + // The user command should appear as container args, preserving each + // argument as a separate element (no whitespace join/split). + let args = pod_template["spec"]["containers"][0]["args"] + .as_array() + .expect("container args should be set when user provides a command"); + assert_eq!( + args, + &[ + serde_json::json!("python"), + serde_json::json!("-c"), + serde_json::json!("print('hello world')"), + ], + "args must preserve argument boundaries exactly" + ); + + // The env var should still be sleep infinity (fallback only). + let env = pod_template["spec"]["containers"][0]["env"] + .as_array() + .expect("env should exist"); let cmd_entry = env .iter() .find(|e| e.get("name").and_then(|v| v.as_str()) == Some("OPENSHELL_SANDBOX_COMMAND")) - .expect("OPENSHELL_SANDBOX_COMMAND must be present in env"); + .expect("OPENSHELL_SANDBOX_COMMAND must be present"); assert_eq!( cmd_entry.get("value").and_then(|v| v.as_str()), - Some("python app.py"), - "sandbox command should reflect user-provided command" + Some("sleep infinity"), + "env var should be sleep infinity even when user command is provided" + ); + } + + #[test] + fn no_container_args_when_command_empty() { + let pod_template = sandbox_template_to_k8s( + &SandboxTemplate::default(), + false, + "openshell/sandbox:latest", + "", + "sandbox-id", + "sandbox-name", + "https://gateway.example.com", + &[], + "0.0.0.0:2222", + "secret", + 300, + &std::collections::HashMap::new(), + "", + "", + true, + ); + + // No args should be set when the user didn't provide a command. + assert!( + pod_template["spec"]["containers"][0]["args"].is_null(), + "container args should not be set when no user command is provided" ); } @@ -1818,7 +1875,6 @@ mod tests { "sandbox-1", "my-sandbox", "https://endpoint:8080", - &[], "0.0.0.0:2222", "secret", 300,