diff --git a/Cargo.lock b/Cargo.lock index 4bc657be3..d0cd77f85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3482,6 +3482,7 @@ dependencies = [ "futures", "openshell-core", "serde", + "serde_json", "tar", "temp-env", "tempfile", diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index d4a19c4bf..ac2b50392 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1275,6 +1275,10 @@ enum SandboxCommands { #[arg(long = "label")] labels: Vec, + /// Environment variables to inject into the sandbox (KEY=VALUE format, repeatable). + #[arg(long = "env", value_name = "KEY=VALUE")] + envs: Vec, + /// Approval mode for agent-authored policy proposals. /// /// `manual` (default): every proposal lands in the draft inbox for @@ -1381,6 +1385,10 @@ enum SandboxCommands { #[arg(long, overrides_with = "tty")] no_tty: bool, + /// Environment variables to set for the command (KEY=VALUE format, repeatable). + #[arg(long = "env", value_name = "KEY=VALUE")] + envs: Vec, + /// Command and arguments to execute. #[arg(required = true, trailing_var_arg = true, allow_hyphen_values = true)] command: Vec, @@ -2558,6 +2566,7 @@ async fn main() -> Result<()> { auto_providers, no_auto_providers, labels, + envs, approval_mode, command, } => { @@ -2592,6 +2601,9 @@ async fn main() -> Result<()> { labels_map.insert(parts[0].to_string(), parts[1].to_string()); } + // Parse --env flags into a HashMap. + let env_map = run::parse_env_pairs(&envs)?; + // Parse --upload specs into [(local_path, sandbox_path, git_ignore)]. let upload_specs: Vec<(String, Option, bool)> = upload .iter() @@ -2639,6 +2651,7 @@ async fn main() -> Result<()> { tty_override, auto_providers_override, &labels_map, + &env_map, &approval_mode, &tls, )) @@ -2751,6 +2764,7 @@ async fn main() -> Result<()> { timeout, tty, no_tty, + envs, command, } => { let name = resolve_sandbox_name(name, &ctx.name)?; @@ -2762,6 +2776,7 @@ async fn main() -> Result<()> { } else { None // auto-detect }; + let env_map = run::parse_env_pairs(&envs)?; let exit_code = run::sandbox_exec_grpc( endpoint, &name, @@ -2769,6 +2784,7 @@ async fn main() -> Result<()> { workdir.as_deref(), timeout, tty_override, + &env_map, &tls, ) .await?; diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 64145d883..edbc9630c 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1756,6 +1756,7 @@ pub async fn sandbox_create( tty_override: Option, auto_providers_override: Option, labels: &HashMap, + environment: &HashMap, approval_mode: &str, tls: &TlsOptions, ) -> Result<()> { @@ -1836,6 +1837,7 @@ pub async fn sandbox_create( spec: Some(SandboxSpec { gpu: requested_gpu, gpu_device: gpu_device.unwrap_or_default().to_string(), + environment: environment.clone(), policy, providers: configured_providers, template, @@ -2638,6 +2640,7 @@ 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. +#[allow(clippy::too_many_arguments, clippy::implicit_hasher)] pub async fn sandbox_exec_grpc( server: &str, name: &str, @@ -2645,6 +2648,7 @@ pub async fn sandbox_exec_grpc( workdir: Option<&str>, timeout_seconds: u32, tty_override: Option, + environment: &HashMap, tls: &TlsOptions, ) -> Result { let mut client = grpc_client(server, tls).await?; @@ -2699,8 +2703,15 @@ pub async fn sandbox_exec_grpc( .unwrap_or_else(|| std::io::stdin().is_terminal() && std::io::stdout().is_terminal()); if tty_override == Some(true) && std::io::stdin().is_terminal() { - return sandbox_exec_interactive_grpc(client, &sandbox, command, workdir, timeout_seconds) - .await; + return sandbox_exec_interactive_grpc( + client, + &sandbox, + command, + workdir, + timeout_seconds, + environment, + ) + .await; } // Make the streaming gRPC call. @@ -2709,7 +2720,7 @@ pub async fn sandbox_exec_grpc( sandbox_id: sandbox.object_id().to_string(), command: command.to_vec(), workdir: workdir.unwrap_or_default().to_string(), - environment: HashMap::new(), + environment: environment.clone(), timeout_seconds, stdin: stdin_payload, tty, @@ -3055,6 +3066,7 @@ async fn sandbox_exec_interactive_grpc( command: &[String], workdir: Option<&str>, timeout_seconds: u32, + environment: &HashMap, ) -> Result { use openshell_core::proto::{ExecSandboxInput, ExecSandboxWindowResize, exec_sandbox_input}; use tokio_stream::wrappers::ReceiverStream; @@ -3070,7 +3082,7 @@ async fn sandbox_exec_interactive_grpc( sandbox_id: sandbox.object_id().to_string(), command: command.to_vec(), workdir: workdir.unwrap_or_default().to_string(), - environment: HashMap::new(), + environment: environment.clone(), timeout_seconds, stdin: Vec::new(), tty: true, @@ -3888,7 +3900,7 @@ async fn auto_create_provider( Ok(()) } -fn parse_key_value_pairs(items: &[String], flag: &str) -> Result> { +pub fn parse_key_value_pairs(items: &[String], flag: &str) -> Result> { let mut map = HashMap::new(); for item in items { @@ -3907,6 +3919,34 @@ fn parse_key_value_pairs(items: &[String], flag: &str) -> Result Result> { + let map = parse_key_value_pairs(items, "--env")?; + for key in map.keys() { + if !is_valid_env_name(key) { + return Err(miette::miette!( + "--env key must match [A-Za-z_][A-Za-z0-9_]*; got '{key}'" + )); + } + if key.starts_with("OPENSHELL_") { + return Err(miette::miette!( + "--env keys starting with OPENSHELL_ are reserved; got '{key}'" + )); + } + } + Ok(map) +} + +fn is_valid_env_name(key: &str) -> bool { + let mut bytes = key.bytes(); + let Some(first) = bytes.next() else { + return false; + }; + if !(first == b'_' || first.is_ascii_alphabetic()) { + return false; + } + bytes.all(|b| b == b'_' || b.is_ascii_alphanumeric()) +} + fn parse_credential_pairs(items: &[String]) -> Result> { let mut map = HashMap::new(); diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index 96821655d..be71c0a36 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -795,6 +795,7 @@ async fn sandbox_create_keeps_command_sessions_by_default() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -838,6 +839,7 @@ async fn sandbox_create_sends_cpu_and_memory_limits_only() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -915,6 +917,7 @@ async fn sandbox_create_sends_driver_config_json() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -989,6 +992,7 @@ async fn sandbox_create_does_not_infer_command_providers_when_v2_enabled() { Some(true), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1047,6 +1051,7 @@ async fn sandbox_create_returns_vm_error_without_waiting_for_timeout() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1101,6 +1106,7 @@ async fn sandbox_create_keeps_waiting_while_vm_progress_arrives() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1147,6 +1153,7 @@ async fn sandbox_create_times_out_when_only_logs_arrive() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1189,6 +1196,7 @@ async fn sandbox_create_deletes_command_sessions_with_no_keep() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1235,6 +1243,7 @@ async fn sandbox_create_deletes_shell_sessions_with_no_keep() { Some(true), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1281,6 +1290,7 @@ async fn sandbox_create_keeps_sandbox_with_hidden_keep_flag() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1327,6 +1337,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1335,3 +1346,99 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() { assert!(deleted_names(&server).await.is_empty()); } + +#[tokio::test] +async fn sandbox_create_sends_environment_variables() { + let server = run_server().await; + let fake_ssh_dir = tempfile::tempdir().unwrap(); + let xdg_dir = tempfile::tempdir().unwrap(); + let _env = test_env(&fake_ssh_dir, &xdg_dir); + let tls = test_tls(&server); + install_fake_ssh(&fake_ssh_dir); + + let mut env_map = HashMap::new(); + env_map.insert("FOO".to_string(), "bar".to_string()); + env_map.insert("BAZ".to_string(), "qux=with=equals".to_string()); + + run::sandbox_create( + &server.endpoint, + Some("env-test"), + None, + "openshell", + &[], + true, + false, + None, + None, + None, + None, + None, + &[], + None, + None, + &["echo".to_string(), "OK".to_string()], + Some(false), + Some(false), + &HashMap::new(), + &env_map, + "manual", + &tls, + ) + .await + .expect("sandbox create should succeed"); + + let requests = create_requests(&server).await; + let environment = &requests[0] + .spec + .as_ref() + .expect("spec should be present") + .environment; + assert_eq!(environment.get("FOO").map(String::as_str), Some("bar")); + assert_eq!( + environment.get("BAZ").map(String::as_str), + Some("qux=with=equals") + ); + assert_eq!(environment.len(), 2); +} + +#[tokio::test] +async fn sandbox_create_env_rejects_invalid_format() { + let err = run::parse_key_value_pairs( + &["VALID=ok".to_string(), "NOEQUALSSIGN".to_string()], + "--env", + ) + .unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("--env") && msg.contains("NOEQUALSSIGN"), + "error should mention the flag and bad value, got: {msg}" + ); +} + +#[tokio::test] +async fn sandbox_create_env_rejects_reserved_prefix() { + let err = run::parse_env_pairs(&["VALID=ok".to_string(), "OPENSHELL_SECRET=bad".to_string()]) + .unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("OPENSHELL_") && msg.contains("reserved"), + "error should mention reserved prefix, got: {msg}" + ); +} + +#[tokio::test] +async fn sandbox_create_env_rejects_invalid_key_name() { + let err = run::parse_env_pairs(&["1BAD=value".to_string()]).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("1BAD"), + "error should mention invalid key, got: {msg}" + ); + + let err = run::parse_env_pairs(&["BAD-NAME=value".to_string()]).unwrap_err(); + let msg = format!("{err}"); + assert!( + msg.contains("BAD-NAME"), + "error should mention invalid key, got: {msg}" + ); +} diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index 9ffbf79f0..1059c0d08 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -50,6 +50,13 @@ pub const SANDBOX_TOKEN: &str = "OPENSHELL_SANDBOX_TOKEN"; /// the token is held in process memory thereafter. pub const SANDBOX_TOKEN_FILE: &str = "OPENSHELL_SANDBOX_TOKEN_FILE"; +/// JSON-serialized map of user-specified environment variables. +/// +/// Set by compute drivers from `SandboxSpec.environment`. The sandbox +/// supervisor deserializes this at startup and injects the variables into +/// SSH child processes (which use `env_clear()` for security isolation). +pub const USER_ENVIRONMENT: &str = "OPENSHELL_USER_ENVIRONMENT"; + /// Path to the projected `ServiceAccount` JWT (Kubernetes driver). /// /// Used to bootstrap a gateway-minted JWT via `IssueSandboxToken`. Kubelet diff --git a/crates/openshell-driver-docker/Cargo.toml b/crates/openshell-driver-docker/Cargo.toml index 0cdc205ed..fb2a643ea 100644 --- a/crates/openshell-driver-docker/Cargo.toml +++ b/crates/openshell-driver-docker/Cargo.toml @@ -20,6 +20,7 @@ tokio-stream = { workspace = true } tracing = { workspace = true } bytes = { workspace = true } serde = { workspace = true } +serde_json = { workspace = true } bollard = { version = "0.20" } tar = "0.4" tempfile = "3" diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 864d91f22..11ffab7d6 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -1643,10 +1643,20 @@ fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig ]); if let Some(spec) = sandbox.spec.as_ref() { + let mut user_env = HashMap::new(); if let Some(template) = spec.template.as_ref() { - environment.extend(template.environment.clone()); + user_env.extend(template.environment.clone()); + } + user_env.extend(spec.environment.clone()); + environment.extend(user_env.clone()); + if !user_env.is_empty() + && let Ok(json) = serde_json::to_string(&user_env) + { + environment.insert( + openshell_core::sandbox_env::USER_ENVIRONMENT.to_string(), + json, + ); } - environment.extend(spec.environment.clone()); } environment.insert( diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 449cee58d..583f3cb99 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -1625,6 +1625,17 @@ fn build_env_list( let mut env = existing_env.cloned().unwrap_or_default(); apply_env_map(&mut env, template_environment); apply_env_map(&mut env, spec_environment); + let mut user_env = template_environment.clone(); + user_env.extend(spec_environment.clone()); + if !user_env.is_empty() + && let Ok(json) = serde_json::to_string(&user_env) + { + upsert_env( + &mut env, + openshell_core::sandbox_env::USER_ENVIRONMENT, + &json, + ); + } apply_required_env( &mut env, sandbox_id, diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 13f053e93..cdf074619 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -244,6 +244,13 @@ fn build_env( let mut env: BTreeMap = BTreeMap::new(); // 1. User-supplied environment (lowest priority). + // Template vars first, then spec overwrites (spec is user-specified). + let mut user_env: BTreeMap = BTreeMap::new(); + if let Some(t) = template { + for (k, v) in &t.environment { + user_env.insert(k.clone(), v.clone()); + } + } if let Some(s) = spec { if !s.log_level.is_empty() { env.insert( @@ -252,13 +259,14 @@ fn build_env( ); } for (k, v) in &s.environment { - env.insert(k.clone(), v.clone()); + user_env.insert(k.clone(), v.clone()); } } - if let Some(t) = template { - for (k, v) in &t.environment { - env.insert(k.clone(), v.clone()); - } + env.extend(user_env.clone()); + if !user_env.is_empty() + && let Ok(json) = serde_json::to_string(&user_env) + { + env.insert(openshell_core::sandbox_env::USER_ENVIRONMENT.into(), json); } // 2. Required driver vars (highest priority -- always overwrite). diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 445905a1e..5338c576e 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -3450,55 +3450,64 @@ fn build_guest_environment( || guest_visible_openshell_endpoint(&config.openshell_endpoint), String::from, ); - let mut environment = HashMap::from([ - ("HOME".to_string(), "/root".to_string()), - ( - "PATH".to_string(), - "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin".to_string(), - ), - ("TERM".to_string(), "xterm".to_string()), - ( - openshell_core::sandbox_env::ENDPOINT.to_string(), - openshell_endpoint, - ), - ( - openshell_core::sandbox_env::SANDBOX_ID.to_string(), - sandbox.id.clone(), - ), - ( - openshell_core::sandbox_env::SANDBOX.to_string(), - sandbox.name.clone(), - ), - ( - openshell_core::sandbox_env::SSH_SOCKET_PATH.to_string(), - GUEST_SSH_SOCKET_PATH.to_string(), - ), - ( - openshell_core::sandbox_env::SANDBOX_COMMAND.to_string(), - "tail -f /dev/null".to_string(), - ), - ( - openshell_core::sandbox_env::LOG_LEVEL.to_string(), - openshell_core::driver_utils::sandbox_log_level(sandbox, &config.log_level), - ), - ]); + // 1. User-supplied environment (lowest priority). + let user_env = merged_environment(sandbox); + let mut environment: HashMap = HashMap::new(); + environment.extend(user_env.clone()); + if !user_env.is_empty() + && let Ok(json) = serde_json::to_string(&user_env) + { + environment.insert( + openshell_core::sandbox_env::USER_ENVIRONMENT.to_string(), + json, + ); + } + + // 2. Required driver vars (highest priority -- always overwrite). + environment.insert("HOME".to_string(), "/root".to_string()); + environment.insert( + "PATH".to_string(), + "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin".to_string(), + ); + environment.insert("TERM".to_string(), "xterm".to_string()); + environment.insert( + openshell_core::sandbox_env::ENDPOINT.to_string(), + openshell_endpoint, + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_ID.to_string(), + sandbox.id.clone(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX.to_string(), + sandbox.name.clone(), + ); + environment.insert( + openshell_core::sandbox_env::SSH_SOCKET_PATH.to_string(), + GUEST_SSH_SOCKET_PATH.to_string(), + ); + environment.insert( + openshell_core::sandbox_env::SANDBOX_COMMAND.to_string(), + "tail -f /dev/null".to_string(), + ); + environment.insert( + openshell_core::sandbox_env::LOG_LEVEL.to_string(), + openshell_core::driver_utils::sandbox_log_level(sandbox, &config.log_level), + ); if config.requires_tls_materials() { - environment.extend(HashMap::from([ - ( - openshell_core::sandbox_env::TLS_CA.to_string(), - GUEST_TLS_CA_PATH.to_string(), - ), - ( - openshell_core::sandbox_env::TLS_CERT.to_string(), - GUEST_TLS_CERT_PATH.to_string(), - ), - ( - openshell_core::sandbox_env::TLS_KEY.to_string(), - GUEST_TLS_KEY_PATH.to_string(), - ), - ])); + environment.insert( + openshell_core::sandbox_env::TLS_CA.to_string(), + GUEST_TLS_CA_PATH.to_string(), + ); + environment.insert( + openshell_core::sandbox_env::TLS_CERT.to_string(), + GUEST_TLS_CERT_PATH.to_string(), + ); + environment.insert( + openshell_core::sandbox_env::TLS_KEY.to_string(), + GUEST_TLS_KEY_PATH.to_string(), + ); } - environment.extend(merged_environment(sandbox)); environment.insert( openshell_core::sandbox_env::TELEMETRY_ENABLED.to_string(), openshell_core::telemetry::enabled_env_value().to_string(), diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index e9d8921b6..231b588ea 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -424,6 +424,12 @@ pub async fn run_sandbox( ); let provider_env = provider_credentials.snapshot().child_env.clone(); + let user_environment: std::collections::HashMap = + std::env::var(openshell_core::sandbox_env::USER_ENVIRONMENT) + .ok() + .and_then(|json| serde_json::from_str(&json).ok()) + .unwrap_or_default(); + // Create identity cache for SHA256 TOFU when OPA is active let identity_cache = opa_engine .as_ref() @@ -806,6 +812,7 @@ pub async fn run_sandbox( let netns_fd = ssh_netns_fd; let ca_paths = ca_file_paths.clone(); let provider_credentials_clone = provider_credentials.clone(); + let user_env_clone = user_environment.clone(); let (ssh_ready_tx, ssh_ready_rx) = tokio::sync::oneshot::channel(); @@ -819,6 +826,7 @@ pub async fn run_sandbox( proxy_url, ca_paths, provider_credentials_clone, + user_env_clone, ) .await { diff --git a/crates/openshell-sandbox/src/ssh.rs b/crates/openshell-sandbox/src/ssh.rs index 67fbc7e57..9db2bf97d 100644 --- a/crates/openshell-sandbox/src/ssh.rs +++ b/crates/openshell-sandbox/src/ssh.rs @@ -107,6 +107,7 @@ pub async fn run_ssh_server( proxy_url: Option, ca_file_paths: Option<(PathBuf, PathBuf)>, provider_credentials: ProviderCredentialState, + user_environment: HashMap, ) -> Result<()> { let (listener, config, ca_paths) = match ssh_server_init(&listen_path, &ca_file_paths) { Ok(v) => { @@ -131,6 +132,7 @@ pub async fn run_ssh_server( let proxy_url = proxy_url.clone(); let ca_paths = ca_paths.clone(); let provider_credentials = provider_credentials.clone(); + let user_environment = user_environment.clone(); tokio::spawn(async move { if let Err(err) = handle_connection( @@ -142,6 +144,7 @@ pub async fn run_ssh_server( proxy_url, ca_paths, provider_credentials, + user_environment, ) .await { @@ -168,6 +171,7 @@ async fn handle_connection( proxy_url: Option, ca_file_paths: Option>, provider_credentials: ProviderCredentialState, + user_environment: HashMap, ) -> Result<()> { // Access is gated by the Unix-socket filesystem permissions (root-only), // not by an application-level preface. The supervisor bridges the @@ -190,6 +194,7 @@ async fn handle_connection( proxy_url, ca_file_paths, provider_credentials, + user_environment, ); russh::server::run_stream(config, stream, handler) .await @@ -217,10 +222,12 @@ struct SshHandler { proxy_url: Option, ca_file_paths: Option>, provider_credentials: ProviderCredentialState, + user_environment: HashMap, channels: HashMap, } impl SshHandler { + #[allow(clippy::too_many_arguments)] fn new( policy: SandboxPolicy, workdir: Option, @@ -228,6 +235,7 @@ impl SshHandler { proxy_url: Option, ca_file_paths: Option>, provider_credentials: ProviderCredentialState, + user_environment: HashMap, ) -> Self { Self { policy, @@ -236,6 +244,7 @@ impl SshHandler { proxy_url, ca_file_paths, provider_credentials, + user_environment, channels: HashMap::new(), } } @@ -458,6 +467,7 @@ impl russh::server::Handler for SshHandler { self.proxy_url.clone(), self.ca_file_paths.clone(), &self.provider_credentials.snapshot().child_env, + &self.user_environment, )?; let state = self.channels.get_mut(&channel).ok_or_else(|| { anyhow::anyhow!("subsystem_request on unknown channel {channel:?}") @@ -553,6 +563,7 @@ impl SshHandler { self.proxy_url.clone(), self.ca_file_paths.clone(), &provider_snapshot.child_env, + &self.user_environment, )?; state.pty_master = Some(pty_master); state.input_sender = Some(input_sender); @@ -570,6 +581,7 @@ impl SshHandler { self.proxy_url.clone(), self.ca_file_paths.clone(), &provider_snapshot.child_env, + &self.user_environment, )?; state.input_sender = Some(input_sender); } @@ -668,6 +680,7 @@ fn session_user_and_home(policy: &SandboxPolicy) -> (String, String) { } } +#[allow(clippy::too_many_arguments)] fn apply_child_env( cmd: &mut Command, session_home: &str, @@ -676,6 +689,7 @@ fn apply_child_env( proxy_url: Option<&str>, ca_file_paths: Option<&(PathBuf, PathBuf)>, provider_env: &HashMap, + user_environment: &HashMap, ) { let path = std::env::var("PATH").unwrap_or_else(|_| "/usr/local/bin:/usr/bin:/bin".into()); @@ -687,6 +701,12 @@ fn apply_child_env( .env("PATH", &path) .env("TERM", term); + for (key, value) in user_environment { + if !key.starts_with("OPENSHELL_") { + cmd.env(key, value); + } + } + if let Some(url) = proxy_url { for (key, value) in child_env::proxy_env_vars(url) { cmd.env(key, value); @@ -716,6 +736,7 @@ fn spawn_pty_shell( proxy_url: Option, ca_file_paths: Option>, provider_env: &HashMap, + user_environment: &HashMap, ) -> anyhow::Result<(std::fs::File, mpsc::Sender>)> { let winsize = Winsize { ws_row: to_u16(pty.row_height.max(1)), @@ -764,6 +785,7 @@ fn spawn_pty_shell( proxy_url.as_deref(), ca_file_paths.as_deref(), provider_env, + user_environment, ); cmd.stdin(stdin).stdout(stdout).stderr(stderr); @@ -879,6 +901,7 @@ fn spawn_pipe_exec( proxy_url: Option, ca_file_paths: Option>, provider_env: &HashMap, + user_environment: &HashMap, ) -> anyhow::Result>> { let mut cmd = command.map_or_else( || { @@ -909,6 +932,7 @@ fn spawn_pipe_exec( proxy_url.as_deref(), ca_file_paths.as_deref(), provider_env, + user_environment, ); cmd.stdin(Stdio::piped()) .stdout(Stdio::piped()) diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 268c143d2..03a69d6e9 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -56,6 +56,7 @@ pub(super) fn validate_exec_request_fields(req: &ExecSandboxRequest) -> Result<( } reject_control_chars(value, &format!("environment value for '{key}'"))?; } + validate_exec_env_entries(&req.environment, "environment")?; if !req.workdir.is_empty() { if req.workdir.len() > MAX_EXEC_WORKDIR_LEN { return Err(Status::invalid_argument(format!( @@ -125,10 +126,12 @@ pub(super) fn validate_sandbox_spec( MAX_MAP_VALUE_LEN, "spec.environment", )?; + validate_env_entries(&spec.environment, "spec.environment")?; // --- spec.template --- if let Some(ref tmpl) = spec.template { validate_sandbox_template(tmpl)?; + validate_env_entries(&tmpl.environment, "spec.template.environment")?; } // --- spec.policy serialized size --- @@ -243,6 +246,57 @@ pub(super) fn validate_string_map( Ok(()) } +/// OPENSHELL_* keys that are allowed in exec environment. The Python SDK's +/// `exec_python()` sends a serialized callable via this key. +const EXEC_ALLOWED_OPENSHELL_KEYS: &[&str] = &["OPENSHELL_PYFUNC_B64"]; + +/// Maximum total serialized size of user environment (bytes). The drivers +/// serialize the full map as JSON into a single `OPENSHELL_USER_ENVIRONMENT` +/// env var; capping the input prevents driver/runtime-specific startup +/// failures from oversized env blocks. +const MAX_ENV_SERIALIZED_SIZE: usize = 256 * 1024; // 256 KiB + +fn validate_env_entries( + map: &std::collections::HashMap, + field_name: &str, +) -> Result<(), Status> { + let total_size: usize = map.iter().map(|(k, v)| k.len() + v.len()).sum(); + if total_size > MAX_ENV_SERIALIZED_SIZE { + return Err(Status::invalid_argument(format!( + "{field_name} total size exceeds {MAX_ENV_SERIALIZED_SIZE} byte limit ({total_size} bytes)" + ))); + } + validate_env_entries_inner(map, field_name, &[]) +} + +fn validate_exec_env_entries( + map: &std::collections::HashMap, + field_name: &str, +) -> Result<(), Status> { + validate_env_entries_inner(map, field_name, EXEC_ALLOWED_OPENSHELL_KEYS) +} + +fn validate_env_entries_inner( + map: &std::collections::HashMap, + field_name: &str, + allowed_openshell_keys: &[&str], +) -> Result<(), Status> { + for (key, value) in map { + if !super::provider::is_valid_env_key(key) { + return Err(Status::invalid_argument(format!( + "{field_name} keys must match ^[A-Za-z_][A-Za-z0-9_]*$; got '{key}'" + ))); + } + if key.starts_with("OPENSHELL_") && !allowed_openshell_keys.contains(&key.as_str()) { + return Err(Status::invalid_argument(format!( + "{field_name} keys starting with OPENSHELL_ are reserved; got '{key}'" + ))); + } + reject_control_chars(value, &format!("{field_name} value for '{key}'"))?; + } + Ok(()) +} + // --------------------------------------------------------------------------- // Provider field validation // --------------------------------------------------------------------------- @@ -894,6 +948,133 @@ mod tests { assert!(validate_sandbox_spec("my-sandbox", &spec).is_ok()); } + #[test] + fn validate_sandbox_spec_rejects_reserved_env_key() { + let spec = SandboxSpec { + environment: std::iter::once(("OPENSHELL_SECRET".to_string(), "val".to_string())) + .collect(), + ..Default::default() + }; + let err = validate_sandbox_spec("s", &spec).unwrap_err(); + assert!( + err.message().contains("OPENSHELL_") && err.message().contains("reserved"), + "expected reserved key error, got: {}", + err.message() + ); + } + + #[test] + fn validate_sandbox_spec_rejects_reserved_template_env_key() { + let spec = SandboxSpec { + template: Some(SandboxTemplate { + environment: std::iter::once(( + "OPENSHELL_ENDPOINT".to_string(), + "evil".to_string(), + )) + .collect(), + ..Default::default() + }), + ..Default::default() + }; + let err = validate_sandbox_spec("s", &spec).unwrap_err(); + assert!( + err.message().contains("OPENSHELL_") && err.message().contains("reserved"), + "expected reserved key error, got: {}", + err.message() + ); + } + + #[test] + fn validate_exec_request_rejects_reserved_env_key() { + let req = ExecSandboxRequest { + sandbox_id: "id".to_string(), + command: vec!["echo".to_string()], + environment: std::iter::once(("OPENSHELL_SANDBOX_ID".to_string(), "evil".to_string())) + .collect(), + ..Default::default() + }; + let err = validate_exec_request_fields(&req).unwrap_err(); + assert!( + err.message().contains("OPENSHELL_") && err.message().contains("reserved"), + "expected reserved key error, got: {}", + err.message() + ); + } + + #[test] + fn validate_exec_request_allows_pyfunc_helper_key() { + let req = ExecSandboxRequest { + sandbox_id: "id".to_string(), + command: vec!["python".to_string()], + environment: std::iter::once(("OPENSHELL_PYFUNC_B64".to_string(), "data".to_string())) + .collect(), + ..Default::default() + }; + assert!(validate_exec_request_fields(&req).is_ok()); + } + + #[test] + fn validate_sandbox_spec_rejects_template_env_value_with_control_chars() { + let spec = SandboxSpec { + template: Some(SandboxTemplate { + environment: std::iter::once(("KEY".to_string(), "val\nue".to_string())).collect(), + ..Default::default() + }), + ..Default::default() + }; + let err = validate_sandbox_spec("s", &spec).unwrap_err(); + assert!( + err.message().contains("newline"), + "expected control char error, got: {}", + err.message() + ); + } + + #[test] + fn validate_sandbox_spec_rejects_invalid_env_key_name() { + let spec = SandboxSpec { + environment: std::iter::once(("1BAD".to_string(), "val".to_string())).collect(), + ..Default::default() + }; + let err = validate_sandbox_spec("s", &spec).unwrap_err(); + assert!( + err.message().contains("1BAD"), + "expected invalid key error, got: {}", + err.message() + ); + } + + #[test] + fn validate_sandbox_spec_rejects_env_value_with_control_chars() { + let spec = SandboxSpec { + environment: std::iter::once(("KEY".to_string(), "val\nue".to_string())).collect(), + ..Default::default() + }; + let err = validate_sandbox_spec("s", &spec).unwrap_err(); + assert!( + err.message().contains("newline"), + "expected control char error, got: {}", + err.message() + ); + } + + #[test] + fn validate_sandbox_spec_rejects_invalid_template_env_key_name() { + let spec = SandboxSpec { + template: Some(SandboxTemplate { + environment: std::iter::once(("BAD-NAME".to_string(), "val".to_string())).collect(), + ..Default::default() + }), + ..Default::default() + }; + let err = validate_sandbox_spec("s", &spec).unwrap_err(); + assert!( + err.message().contains("BAD-NAME"), + "expected invalid key error, got: {}", + err.message() + ); + } + // ---- Provider field validation ---- fn one_credential() -> HashMap { diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index 0db6d7678..469e04361 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -153,6 +153,27 @@ OpenShell allocates a TTY automatically when both stdin and stdout are terminals | `--timeout` | Command timeout in seconds. `0` disables the timeout. | | `--tty` | Force TTY allocation. | | `--no-tty` | Disable TTY allocation even when attached to a terminal. | +| `--env` | Set an environment variable for the command (`KEY=VALUE`, repeatable). | + +## Set Environment Variables + +Inject environment variables into the sandbox at creation time: + +```shell +openshell sandbox create --env API_KEY=sk-test --env DEBUG=1 -- my-agent +``` + +Variables set with `--env` are available to all processes in the sandbox, including interactive shells and exec commands. + +You can also set per-command environment variables with `sandbox exec`: + +```shell +openshell sandbox exec -n my-sandbox --env MY_VAR=hello -- printenv MY_VAR +``` + +Per-command variables override the sandbox-level environment for that command only. + +Environment variable names starting with `OPENSHELL_` are reserved. Keys must match `[A-Za-z_][A-Za-z0-9_]*`. ## Label a Sandbox