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 21f15834b..4e8778c41 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1267,6 +1267,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 @@ -1373,6 +1377,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, @@ -2549,6 +2557,7 @@ async fn main() -> Result<()> { auto_providers, no_auto_providers, labels, + envs, approval_mode, command, } => { @@ -2583,6 +2592,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() @@ -2629,6 +2641,7 @@ async fn main() -> Result<()> { tty_override, auto_providers_override, &labels_map, + &env_map, &approval_mode, &tls, )) @@ -2741,6 +2754,7 @@ async fn main() -> Result<()> { timeout, tty, no_tty, + envs, command, } => { let name = resolve_sandbox_name(name, &ctx.name)?; @@ -2752,6 +2766,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, @@ -2759,6 +2774,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 3b2adfc59..d1d8c3389 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1709,6 +1709,7 @@ pub async fn sandbox_create( tty_override: Option, auto_providers_override: Option, labels: &HashMap, + environment: &HashMap, approval_mode: &str, tls: &TlsOptions, ) -> Result<()> { @@ -1785,6 +1786,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, @@ -2587,6 +2589,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, @@ -2594,6 +2597,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?; @@ -2648,8 +2652,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. @@ -2658,7 +2669,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, @@ -3004,6 +3015,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; @@ -3019,7 +3031,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, @@ -3837,7 +3849,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 { @@ -3856,6 +3868,18 @@ 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 key.starts_with("OPENSHELL_") { + return Err(miette::miette!( + "--env keys starting with OPENSHELL_ are reserved; got '{key}'" + )); + } + } + Ok(map) +} + 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 828dbd998..ab7039b8a 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -794,6 +794,7 @@ async fn sandbox_create_keeps_command_sessions_by_default() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -836,6 +837,7 @@ async fn sandbox_create_sends_cpu_and_memory_limits_only() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -913,6 +915,7 @@ async fn sandbox_create_does_not_infer_command_providers_when_v2_enabled() { Some(true), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -970,6 +973,7 @@ async fn sandbox_create_returns_vm_error_without_waiting_for_timeout() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1023,6 +1027,7 @@ async fn sandbox_create_keeps_waiting_while_vm_progress_arrives() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1068,6 +1073,7 @@ async fn sandbox_create_times_out_when_only_logs_arrive() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1109,6 +1115,7 @@ async fn sandbox_create_deletes_command_sessions_with_no_keep() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1154,6 +1161,7 @@ async fn sandbox_create_deletes_shell_sessions_with_no_keep() { Some(true), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1199,6 +1207,7 @@ async fn sandbox_create_keeps_sandbox_with_hidden_keep_flag() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1244,6 +1253,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() { Some(false), Some(false), &HashMap::new(), + &HashMap::new(), "manual", &tls, ) @@ -1252,3 +1262,81 @@ 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, + &["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}" + ); +} 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 830b85225..fc1f74929 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -1482,6 +1482,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 83658ef7b..5680acd86 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}'"))?; } + reject_reserved_env_keys(&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", )?; + reject_reserved_env_keys(&spec.environment, "spec.environment")?; // --- spec.template --- if let Some(ref tmpl) = spec.template { validate_sandbox_template(tmpl)?; + reject_reserved_env_keys(&tmpl.environment, "spec.template.environment")?; } // --- spec.policy serialized size --- @@ -235,6 +238,20 @@ pub(super) fn validate_string_map( Ok(()) } +fn reject_reserved_env_keys( + map: &std::collections::HashMap, + field_name: &str, +) -> Result<(), Status> { + for key in map.keys() { + if key.starts_with("OPENSHELL_") { + return Err(Status::invalid_argument(format!( + "{field_name} keys starting with OPENSHELL_ are reserved; got '{key}'" + ))); + } + } + Ok(()) +} + // --------------------------------------------------------------------------- // Provider field validation // --------------------------------------------------------------------------- @@ -886,6 +903,59 @@ 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() + ); + } + // ---- Provider field validation ---- fn one_credential() -> HashMap { diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index 8a154c1b3..27a414387 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -134,6 +134,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 and rejected by the CLI. ## Label a Sandbox