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-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 c5e9a833..fc909a11 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, @@ -1053,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 @@ -1219,6 +1235,11 @@ 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); + // 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,6 +1656,113 @@ mod tests { ); } + #[test] + 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, + 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"), + "OPENSHELL_SANDBOX_COMMAND should always be 'sleep infinity' (user commands are delivered via K8s args)" + ); + } + + #[test] + 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, + &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"); + assert_eq!( + cmd_entry.get("value").and_then(|v| v.as_str()), + 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" + ); + } + #[test] fn supervisor_sideload_injects_run_as_user_zero() { let mut pod_template = serde_json::json!({ @@ -1795,6 +1923,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1829,6 +1958,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1859,6 +1989,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1902,6 +2033,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1929,6 +2061,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1960,6 +2093,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -1986,6 +2120,7 @@ mod tests { "sandbox-id", "sandbox-name", "https://gateway.example.com", + &[], "0.0.0.0:2222", "secret", 300, @@ -2126,6 +2261,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.