diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index c62a7cde55..40a7f0a72f 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -8,6 +8,7 @@ //! supervisor process (which reads them on startup). Using constants here //! prevents typos from producing silently broken sandboxes. +use base64::Engine as _; use serde::{Deserialize, Serialize}; /// Name of the sandbox (used for policy sync and identification). @@ -25,9 +26,14 @@ pub const SSH_SOCKET_PATH: &str = "OPENSHELL_SSH_SOCKET_PATH"; /// Log level for the sandbox supervisor (e.g. `"debug"`, `"info"`, `"warn"`). pub const LOG_LEVEL: &str = "OPENSHELL_LOG_LEVEL"; -/// Versioned JSON specification for the exact canonical main process. +/// Versioned specification for the exact canonical main process. +/// +/// Most drivers use JSON directly. Transports that cannot preserve spaces in +/// environment values may use the `base64url:`-prefixed representation. pub const MAIN_PROCESS_SPEC: &str = "OPENSHELL_MAIN_PROCESS_SPEC"; +const MAIN_PROCESS_SPEC_BASE64URL_PREFIX: &str = "base64url:"; + /// Lossless driver-to-supervisor representation of the canonical process. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct MainProcessConfig { @@ -61,7 +67,18 @@ impl MainProcessConfig { } /// Decode the versioned transport without shell interpretation. - pub fn decode(json: &str) -> Result { + pub fn decode(encoded: &str) -> Result { + let decoded; + let json = if let Some(payload) = encoded.strip_prefix(MAIN_PROCESS_SPEC_BASE64URL_PREFIX) { + let bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload) + .map_err(|error| format!("invalid {MAIN_PROCESS_SPEC} base64url: {error}"))?; + decoded = String::from_utf8(bytes) + .map_err(|error| format!("invalid {MAIN_PROCESS_SPEC} UTF-8: {error}"))?; + decoded.as_str() + } else { + encoded + }; let config: Self = serde_json::from_str(json) .map_err(|error| format!("invalid {MAIN_PROCESS_SPEC}: {error}"))?; if config.version != Self::VERSION { @@ -82,6 +99,16 @@ impl MainProcessConfig { ) -> Result { serde_json::to_string(&Self::from_driver_spec(spec)) } + + /// Encode the versioned transport without whitespace for constrained + /// environment-variable transports such as libkrun. + pub fn encode_driver_spec_base64url( + spec: Option<&crate::proto::compute::v1::DriverSandboxSpec>, + ) -> Result { + let json = Self::encode_driver_spec(spec)?; + let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(json); + Ok(format!("{MAIN_PROCESS_SPEC_BASE64URL_PREFIX}{payload}")) + } } /// Deployment-controlled telemetry toggle propagated to the sandbox supervisor. @@ -214,6 +241,25 @@ mod tests { assert!(!decoded.tty); } + #[test] + fn base64url_main_process_transport_preserves_spaces() { + let spec = crate::proto::compute::v1::DriverSandboxSpec { + command: vec![ + "/bin/sh".into(), + "-c".into(), + "echo ready; while true; do sleep 1; done".into(), + ], + tty: false, + ..Default::default() + }; + let encoded = MainProcessConfig::encode_driver_spec_base64url(Some(&spec)).unwrap(); + + assert!(!encoded.contains(char::is_whitespace)); + let decoded = MainProcessConfig::decode(&encoded).unwrap(); + assert_eq!(decoded.command, spec.command); + assert!(!decoded.tty); + } + #[test] fn main_process_transport_rejects_unknown_version() { let error = diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index bfbbab1c8f..13e57f546d 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -4489,9 +4489,14 @@ fn build_guest_environment( openshell_core::sandbox_env::SSH_SOCKET_PATH.to_string(), GUEST_SSH_SOCKET_PATH.to_string(), ); + // The libkrun guest environment path does not preserve spaces in values + // before guest startup. Use a whitespace-free base64url envelope so + // command arguments remain lossless. let main_process = - openshell_core::sandbox_env::MainProcessConfig::encode_driver_spec(sandbox.spec.as_ref()) - .expect("main process config serialization cannot fail"); + openshell_core::sandbox_env::MainProcessConfig::encode_driver_spec_base64url( + sandbox.spec.as_ref(), + ) + .expect("main process config serialization cannot fail"); environment.insert( openshell_core::sandbox_env::MAIN_PROCESS_SPEC.to_string(), main_process, @@ -7096,6 +7101,43 @@ mod tests { ); } + #[test] + fn build_guest_environment_preserves_main_command_spaces() { + let config = VmDriverConfig { + openshell_endpoint: "https://127.0.0.1:8080".to_string(), + ..Default::default() + }; + let command = vec![ + "sh".to_string(), + "-lc".to_string(), + "echo ready; while true; do sleep 1; done".to_string(), + ]; + let sandbox = Sandbox { + id: "space-command".to_string(), + name: "space-command".to_string(), + spec: Some(SandboxSpec { + command: command.clone(), + ..Default::default() + }), + ..Default::default() + }; + + let env = build_guest_environment(&sandbox, &config, None); + let encoded = env + .iter() + .find_map(|entry| { + entry.strip_prefix(&format!( + "{}=", + openshell_core::sandbox_env::MAIN_PROCESS_SPEC + )) + }) + .expect("main process environment"); + + assert!(!encoded.contains(char::is_whitespace)); + let main = openshell_core::sandbox_env::MainProcessConfig::decode(encoded).unwrap(); + assert_eq!(main.command, command); + } + #[test] fn build_guest_environment_uses_token_file_without_raw_token_env() { let config = VmDriverConfig { diff --git a/e2e/mcp-conformance.sh b/e2e/mcp-conformance.sh index 87b1f22e9a..c1b46fe53a 100755 --- a/e2e/mcp-conformance.sh +++ b/e2e/mcp-conformance.sh @@ -338,7 +338,7 @@ create_client_sandbox() { --from "${CLIENT_IMAGE}" \ --policy "${policy_file}" \ --no-tty \ - -- true; then + -- sleep infinity; then rm -f "${policy_file}" return 1 fi diff --git a/e2e/rust/tests/oidc_pkce.rs b/e2e/rust/tests/oidc_pkce.rs index f8edc3d7b2..9ab6848861 100644 --- a/e2e/rust/tests/oidc_pkce.rs +++ b/e2e/rust/tests/oidc_pkce.rs @@ -28,6 +28,8 @@ use url::Url; static SANDBOX_LIFECYCLE_LOCK: Mutex<()> = Mutex::const_new(()); +const DURABLE_MAIN_SCRIPT: &str = r#"echo "$1"; exec sleep infinity"#; + #[derive(Clone, Copy)] struct IdentityScenario { gateway_name: &'static str, @@ -913,7 +915,10 @@ async fn workspace_user_cannot_create_sandbox_in_another_workspace() { "oidc-xcreate-denied", "--no-tty", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "denied", ], ) @@ -1384,7 +1389,10 @@ async fn assert_can_create_sandbox(session: &LoginSession, workspace: &str, sand sandbox_name, "--no-tty", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", &marker, ], ) @@ -1431,7 +1439,10 @@ async fn assert_can_delete_sandbox(session: &LoginSession, workspace: &str, sand sandbox_name, "--no-tty", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", &marker, ], ) diff --git a/e2e/rust/tests/workspace_namespace_managed.rs b/e2e/rust/tests/workspace_namespace_managed.rs index 38de1da6a3..70d6ea96a9 100644 --- a/e2e/rust/tests/workspace_namespace_managed.rs +++ b/e2e/rust/tests/workspace_namespace_managed.rs @@ -20,6 +20,8 @@ use std::time::Duration; use openshell_e2e::harness::binary::{openshell_bin, openshell_cmd}; use openshell_e2e::harness::output::strip_ansi; +const DURABLE_MAIN_SCRIPT: &str = r#"echo "$1"; exec sleep infinity"#; + fn kube_context() -> String { std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE") .expect("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE must be set") @@ -147,7 +149,10 @@ async fn managed_creates_namespace_with_labels() { "--name", "mgd-sb", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "managed-ok", ]) .await; @@ -279,7 +284,10 @@ async fn managed_namespace_survives_with_remaining_sandboxes() { "--name", "sb-a", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "a", ]) .await; @@ -293,7 +301,10 @@ async fn managed_namespace_survives_with_remaining_sandboxes() { "--name", "sb-b", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "b", ]) .await; @@ -355,7 +366,10 @@ async fn managed_isolates_workspaces_into_separate_namespaces() { "--name", "sb-iso-a", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "a", ]) .await; @@ -369,7 +383,10 @@ async fn managed_isolates_workspaces_into_separate_namespaces() { "--name", "sb-iso-b", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "b", ]) .await; @@ -444,7 +461,10 @@ async fn managed_workspace_delete_removes_namespace() { "--name", "del-sb", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "del-ok", ]) .await; @@ -516,7 +536,10 @@ async fn managed_tls_secret_copied_to_namespace() { "--name", "tls-sb", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "tls-ok", ]) .await; @@ -582,7 +605,10 @@ async fn managed_rejects_namespace_owned_by_different_gateway() { "--name", "conflict-sb", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "nope", ]) .await; @@ -612,7 +638,10 @@ async fn managed_full_lifecycle_with_multiple_sandboxes() { "--name", "lc-a", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "a", ]) .await; @@ -626,7 +655,10 @@ async fn managed_full_lifecycle_with_multiple_sandboxes() { "--name", "lc-b", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "b", ]) .await; @@ -705,7 +737,10 @@ async fn managed_stop_waits_for_workspace_pod_to_disappear() { "--name", sandbox, "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "ready", ]) .await; @@ -754,7 +789,10 @@ async fn managed_rejects_invalid_dns1123_sandbox_name() { "--name", "my_bad_name", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "nope", ]) .await; @@ -775,7 +813,10 @@ async fn managed_rejects_invalid_dns1123_sandbox_name() { "--name", "MyBadName", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "nope", ]) .await; @@ -793,7 +834,10 @@ async fn managed_rejects_invalid_dns1123_sandbox_name() { "--name", "trailing-", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "nope", ]) .await; diff --git a/e2e/rust/tests/workspace_namespace_operator.rs b/e2e/rust/tests/workspace_namespace_operator.rs index f421aa6ada..c393711a12 100644 --- a/e2e/rust/tests/workspace_namespace_operator.rs +++ b/e2e/rust/tests/workspace_namespace_operator.rs @@ -18,6 +18,7 @@ use openshell_e2e::harness::output::strip_ansi; const OPERATOR_LABEL: &str = "openshell.ai/e2e-operator-workspace=true"; const SA_NAME: &str = "openshell-sandbox"; +const DURABLE_MAIN_SCRIPT: &str = r#"echo "$1"; exec sleep infinity"#; fn kube_context() -> String { std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE") @@ -172,7 +173,10 @@ async fn operator_sandbox_in_labeled_namespace() { "--name", "op-sb", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "operator-ok", ]) .await; @@ -256,7 +260,10 @@ async fn operator_rejects_unlabeled_namespace() { "--name", "should-fail", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "nope", ]) .await; @@ -292,7 +299,10 @@ async fn operator_rejects_nonexistent_namespace() { "--name", "should-fail", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "nope", ]) .await; @@ -329,7 +339,10 @@ async fn operator_workspace_delete_preserves_namespace() { "--name", "opdel-sb", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "opdel-ok", ]) .await; @@ -391,7 +404,10 @@ async fn operator_label_removal_blocks_sandbox_creation() { "--name", "lbl-sb1", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "lbl-ok", ]) .await; @@ -428,7 +444,10 @@ async fn operator_label_removal_blocks_sandbox_creation() { "--name", "lbl-sb2", "--", - "echo", + "sh", + "-c", + DURABLE_MAIN_SCRIPT, + "_", "should-fail", ]) .await;