From 0e7618b362a44a58cf51a653be4fe589306e25c5 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Fri, 21 Aug 2026 06:56:11 +0000 Subject: [PATCH 1/5] feat(sandbox): support rootfs tar as --from source for VM driver Accept flat rootfs tar archives (.tar, .tar.gz, .tgz) via the --from flag for VM-backed gateways. The CLI detects the archive extension, validates that the gateway uses the VM compute driver, and passes the tar path through driver_config. The VM driver copies the tar into its staging area and feeds it into the existing rootfs extraction and ext4 disk creation pipeline, skipping the container image pull/export steps. Closes #2175 Signed-off-by: Philippe Martin --- crates/openshell-cli/src/main.rs | 8 +- crates/openshell-cli/src/run.rs | 247 ++++++++++++++++++++--- crates/openshell-driver-vm/src/driver.rs | 161 +++++++++++++-- docs/sandboxes/manage-sandboxes.mdx | 9 +- e2e/rust/Cargo.toml | 5 + e2e/rust/tests/rootfs_tar.rs | 116 +++++++++++ 6 files changed, 501 insertions(+), 45 deletions(-) create mode 100644 e2e/rust/tests/rootfs_tar.rs diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index aaabf26625..9513c232e1 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1340,15 +1340,17 @@ enum SandboxCommands { name: Option, /// Sandbox source: a community sandbox name (e.g., `ollama`), a path - /// to a Dockerfile or directory containing one, or a full container - /// image reference (e.g., `myregistry.com/img:tag`). + /// to a Dockerfile or directory containing one, a rootfs tar archive + /// (`.tar`, `.tar.gz`, or `.tgz`), or a full container image reference + /// (e.g., `myregistry.com/img:tag`). /// /// Community names are resolved to /// `ghcr.io/nvidia/openshell-community/sandboxes/:latest` /// (override the prefix with `OPENSHELL_COMMUNITY_REGISTRY`). /// /// When given a Dockerfile or directory, the image is built into the - /// local Docker daemon before creating the sandbox. + /// local Docker daemon before creating the sandbox. When given a + /// rootfs tar, it is passed directly to the VM compute driver. #[arg(long, value_hint = ValueHint::AnyPath)] from: Option, diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 0a0b21a7f4..e36624018d 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -40,13 +40,13 @@ use openshell_core::proto::{ DeleteProviderProfileRequest, DeleteProviderRefreshRequest, DeleteProviderRequest, DeleteSandboxRequest, DeleteServiceRequest, DetachSandboxProviderRequest, ExecSandboxRequest, ExposeServiceRequest, GetCurrentUserRequest, GetDraftHistoryRequest, GetDraftPolicyRequest, - GetGatewayConfigRequest, GetInferenceRouteRequest, GetProviderProfileRequest, - GetProviderRefreshStatusRequest, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxConfigResponse, GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, - GetSandboxRequest, GetServiceRequest, GpuResourceRequirements, ImportProviderProfilesRequest, - LintProviderProfilesRequest, ListProviderProfilesRequest, ListProvidersRequest, - ListSandboxPoliciesRequest, ListSandboxProvidersRequest, ListSandboxesRequest, - ListServicesRequest, PolicySource, PolicyStatus, Provider, + GetGatewayConfigRequest, GetGatewayInfoRequest, GetInferenceRouteRequest, + GetProviderProfileRequest, GetProviderRefreshStatusRequest, GetProviderRequest, + GetSandboxConfigRequest, GetSandboxConfigResponse, GetSandboxLogsRequest, + GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, GpuResourceRequirements, + ImportProviderProfilesRequest, LintProviderProfilesRequest, ListProviderProfilesRequest, + ListProvidersRequest, ListSandboxPoliciesRequest, ListSandboxProvidersRequest, + ListSandboxesRequest, ListServicesRequest, PolicySource, PolicyStatus, Provider, ProviderCredentialRefreshRecoveryAction, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, ProviderCredentialTokenGrantType, ProviderProfile, ProviderProfileDiagnostic, ProviderProfileImportItem, RejectDraftChunkRequest, @@ -474,22 +474,26 @@ pub async fn sandbox_create( let effective_tls = tls.clone(); // Resolve the --from flag into a container image reference, building from - // a Dockerfile first if necessary. - let image: Option = match from { + // a Dockerfile first if necessary, or a rootfs tar path for the VM driver. + let (image, rootfs_tar_path): (Option, Option) = match from { Some(val) => { let resolved = resolve_from(val)?; match resolved { - ResolvedSource::Image(img) => Some(img), + ResolvedSource::Image(img) => (Some(img), None), ResolvedSource::Dockerfile { dockerfile, context, } => { let tag = build_from_dockerfile(&dockerfile, &context, gateway_name).await?; - Some(tag) + (Some(tag), None) + } + ResolvedSource::RootfsTar { path } => { + validate_rootfs_tar_source(gateway_name, &mut client, &path).await?; + (None, Some(path)) } } } - None => None, + None => (None, None), }; let inferred_provider = inferred_provider_type(command); let providers_v2_enabled = @@ -514,11 +518,20 @@ pub async fn sandbox_create( let policy = load_sandbox_policy(policy)?; let resource_limits = build_sandbox_resource_limits(cpu, memory)?; - let driver_config = driver_config_json + let mut driver_config = driver_config_json .map(parse_driver_config_json) .transpose()?; - let template = if image.is_some() || resource_limits.is_some() || driver_config.is_some() { + if let Some(tar_path) = &rootfs_tar_path { + let rootfs_config = rootfs_tar_driver_config(tar_path)?; + driver_config = Some(merge_driver_config(driver_config, rootfs_config)); + } + + let template = if image.is_some() + || resource_limits.is_some() + || driver_config.is_some() + || rootfs_tar_path.is_some() + { Some(SandboxTemplate { image: image.unwrap_or_default(), resources: resource_limits, @@ -1032,17 +1045,23 @@ enum ResolvedSource { dockerfile: PathBuf, context: PathBuf, }, + /// A flat rootfs tar archive (`.tar`, `.tar.gz`, `.tgz`) to pass directly + /// to the VM compute driver. + RootfsTar { path: PathBuf }, } -/// Classify the `--from` value into an image reference or a Dockerfile that -/// needs building. +/// Classify the `--from` value into an image reference, a Dockerfile that +/// needs building, or a rootfs tar to pass to the VM driver. /// /// Resolution order: -/// 1. Existing file whose name contains "Dockerfile" → build from file. +/// 1. Existing file whose name contains "dockerfile" → build from Dockerfile. /// 2. Existing directory that contains a `Dockerfile` → build from directory. -/// 3. Missing explicit local paths → local error, not image pull. -/// 4. Value contains `/`, `:`, or `.` → treat as a full image reference. -/// 5. Otherwise → community sandbox name, expanded via the registry prefix. +/// 3. Existing file with `.tar`, `.tar.gz`, or `.tgz` extension → rootfs tar archive. +/// 4. Other existing local paths → error. +/// 5. Non-existent path-like values (`./…`, `../…`, `/…`, `~/…`) → local +/// error, so they don't reach the gateway as broken image-pull requests. +/// 6. Value contains `/`, `:`, or `.` → treat as a full image reference. +/// 7. Otherwise → community sandbox name, expanded via the registry prefix. fn resolve_from(value: &str) -> Result { let path = Path::new(value); @@ -1063,9 +1082,17 @@ fn resolve_from(value: &str) -> Result { }); } + if filename_looks_like_rootfs_tar(path) { + let tar_path = path + .canonicalize() + .into_diagnostic() + .wrap_err_with(|| format!("failed to resolve path: {}", path.display()))?; + return Ok(ResolvedSource::RootfsTar { path: tar_path }); + } + if value_looks_like_local_source(value) { return Err(miette::miette!( - "local --from file is not a Dockerfile: {}", + "local --from file is not a Dockerfile or rootfs tar (.tar/.tar.gz/.tgz): {}", path.display() )); } @@ -1104,7 +1131,7 @@ fn resolve_from(value: &str) -> Result { if value_looks_like_local_source(value) { return Err(miette::miette!( "local --from path does not exist: {}\n\ - Use an existing Dockerfile, a directory containing Dockerfile, or a container image reference.", + Use an existing Dockerfile, directory containing Dockerfile, rootfs tar (.tar/.tar.gz/.tgz), or a container image reference.", path.display() )); } @@ -1122,7 +1149,17 @@ fn filename_looks_like_dockerfile(path: &Path) -> bool { .map(|n| n.to_string_lossy()) .unwrap_or_default(); let lower = name.to_lowercase(); - lower.contains("dockerfile") || lower.ends_with(".dockerfile") + lower.contains("dockerfile") +} + +#[allow(clippy::case_sensitive_file_extension_comparisons)] // already lowercased +fn filename_looks_like_rootfs_tar(path: &Path) -> bool { + let name = path + .file_name() + .map(|n| n.to_string_lossy()) + .unwrap_or_default(); + let lower = name.to_lowercase(); + lower.ends_with(".tar.gz") || lower.ends_with(".tar") || lower.ends_with(".tgz") } fn value_looks_like_local_source(value: &str) -> bool { @@ -1204,6 +1241,73 @@ async fn build_from_dockerfile( Ok(tag) } +/// Validate that a rootfs tar source is usable with the current gateway. +async fn validate_rootfs_tar_source( + gateway_name: &str, + client: &mut crate::tls::GrpcClient, + tar_path: &Path, +) -> Result<()> { + let metadata = get_gateway_metadata(gateway_name); + if !dockerfile_sources_supported_for_gateway(metadata.as_ref()) { + return Err(miette!( + "local rootfs tar sources are only supported for local gateways; gateway '{}' is remote", + gateway_name + )); + } + + let info = client + .get_gateway_info(GetGatewayInfoRequest {}) + .await + .into_diagnostic() + .wrap_err("failed to query gateway compute driver")? + .into_inner(); + + let driver_name = info.compute_drivers.first().map_or("", |d| d.name.as_str()); + + if driver_name != "vm" { + return Err(miette!( + "rootfs tar sources are only supported by the VM compute driver, \ + but gateway '{}' uses the '{}' driver", + gateway_name, + driver_name + )); + } + + eprintln!( + "Using rootfs tar {} for gateway '{}'", + tar_path.display().to_string().cyan(), + gateway_name, + ); + eprintln!(); + + Ok(()) +} + +/// Build a `driver_config` struct carrying the rootfs tar path for the VM driver. +fn rootfs_tar_driver_config(tar_path: &Path) -> Result { + let fields = serde_json::Map::from_iter([( + "rootfs_tar_path".to_string(), + serde_json::Value::String(tar_path.to_string_lossy().into_owned()), + )]); + openshell_core::proto_struct::json_object_to_struct(fields) + .into_diagnostic() + .wrap_err("failed to encode rootfs_tar_path in driver_config") +} + +/// Merge a rootfs tar config into an existing `driver_config`, if any. +fn merge_driver_config( + base: Option, + overlay: prost_types::Struct, +) -> prost_types::Struct { + match base { + Some(mut base) => { + base.fields.extend(overlay.fields); + base + } + None => overlay, + } +} + /// Load sandbox policy YAML. /// /// Resolution order: `--policy` flag > `OPENSHELL_SANDBOX_POLICY` env var. @@ -7987,8 +8091,8 @@ mod tests { .expect("failed to canonicalize context") ); } - super::ResolvedSource::Image(image) => { - panic!("expected Dockerfile source, got image {image}"); + other => { + panic!("expected Dockerfile source, got {other:?}"); } } } @@ -8013,12 +8117,101 @@ mod tests { match resolve_from(image_ref).expect("expected image source") { super::ResolvedSource::Image(image) => assert_eq!(image, image_ref), - super::ResolvedSource::Dockerfile { .. } => { - panic!("expected image ref, got Dockerfile source"); + other => { + panic!("expected image ref, got {other:?}"); + } + } + } + + #[test] + fn resolve_from_classifies_tar_archive() { + let temp = tempfile::tempdir().expect("failed to create tempdir"); + let archive = temp.path().join("rootfs.tar"); + fs::write(&archive, b"fake tar content").expect("failed to write archive"); + + match resolve_from(archive.to_str().expect("temp path is not UTF-8")) + .expect("expected RootfsTar source") + { + super::ResolvedSource::RootfsTar { path } => { + assert_eq!( + path, + archive + .canonicalize() + .expect("failed to canonicalize archive") + ); } + other => panic!("expected RootfsTar source, got {other:?}"), } } + #[test] + fn resolve_from_classifies_tar_gz_archive() { + let temp = tempfile::tempdir().expect("failed to create tempdir"); + let archive = temp.path().join("rootfs.tar.gz"); + fs::write(&archive, b"fake tar.gz content").expect("failed to write archive"); + + match resolve_from(archive.to_str().expect("temp path is not UTF-8")) + .expect("expected RootfsTar source") + { + super::ResolvedSource::RootfsTar { path } => { + assert_eq!( + path, + archive + .canonicalize() + .expect("failed to canonicalize archive") + ); + } + other => panic!("expected RootfsTar source, got {other:?}"), + } + } + + #[test] + fn resolve_from_classifies_tgz_archive() { + let temp = tempfile::tempdir().expect("failed to create tempdir"); + let archive = temp.path().join("rootfs.tgz"); + fs::write(&archive, b"fake tgz content").expect("failed to write archive"); + + match resolve_from(archive.to_str().expect("temp path is not UTF-8")) + .expect("expected RootfsTar source") + { + super::ResolvedSource::RootfsTar { path } => { + assert_eq!( + path, + archive + .canonicalize() + .expect("failed to canonicalize archive") + ); + } + other => panic!("expected RootfsTar source, got {other:?}"), + } + } + + #[test] + fn resolve_from_rejects_missing_tar_archive() { + let temp = tempfile::tempdir().expect("failed to create tempdir"); + let missing = temp.path().join("missing.tar"); + + let err = resolve_from(missing.to_str().expect("temp path is not UTF-8")) + .expect_err("expected missing archive to be rejected"); + + assert!( + err.to_string().contains("local --from path does not exist"), + "unexpected error: {err}" + ); + } + + #[test] + fn filename_looks_like_rootfs_tar_detects_extensions() { + use super::filename_looks_like_rootfs_tar; + assert!(filename_looks_like_rootfs_tar(Path::new("rootfs.tar"))); + assert!(filename_looks_like_rootfs_tar(Path::new("rootfs.tar.gz"))); + assert!(filename_looks_like_rootfs_tar(Path::new("rootfs.tgz"))); + assert!(filename_looks_like_rootfs_tar(Path::new("IMAGE.TAR"))); + assert!(filename_looks_like_rootfs_tar(Path::new("my-image.TAR.GZ"))); + assert!(!filename_looks_like_rootfs_tar(Path::new("Dockerfile"))); + assert!(!filename_looks_like_rootfs_tar(Path::new("image.zip"))); + } + #[test] fn dockerfile_sources_are_rejected_for_remote_gateways() { let metadata = GatewayMetadata { diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 13e57f546d..2bdaf25e55 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -95,6 +95,7 @@ struct VmSandboxDriverConfig { deserialize_with = "deserialize_optional_non_empty_string_list" )] gpu_device_ids: Option>, + rootfs_tar_path: Option, } impl VmSandboxDriverConfig { @@ -532,9 +533,11 @@ impl VmDriver { #[allow(clippy::result_large_err)] pub fn validate_sandbox(&self, sandbox: &Sandbox) -> Result<(), Status> { validate_vm_sandbox(sandbox, self.config.gpu_enabled)?; - if self.resolved_sandbox_image(sandbox).is_none() { + let has_rootfs_tar = + VmSandboxDriverConfig::from_sandbox(sandbox).is_ok_and(|c| c.rootfs_tar_path.is_some()); + if self.resolved_sandbox_image(sandbox).is_none() && !has_rootfs_tar { return Err(Status::failed_precondition( - "vm sandboxes require template.image or a configured default sandbox image", + "vm sandboxes require template.image, rootfs_tar_path in driver_config, or a configured default sandbox image", )); } Ok(()) @@ -552,11 +555,20 @@ impl VmDriver { validate_vm_sandbox(sandbox, self.config.gpu_enabled)?; let state_dir = sandbox_state_dir(&self.config.state_dir, &sandbox.id)?; - let image_ref = self.resolved_sandbox_image(sandbox).ok_or_else(|| { - Status::failed_precondition( - "vm sandboxes require template.image or a configured default sandbox image", - ) - })?; + let has_rootfs_tar = + VmSandboxDriverConfig::from_sandbox(sandbox).is_ok_and(|c| c.rootfs_tar_path.is_some()); + let image_ref = self + .resolved_sandbox_image(sandbox) + .or_else(|| { + has_rootfs_tar + .then(|| self.bootstrap_image_ref_default()) + .flatten() + }) + .ok_or_else(|| { + Status::failed_precondition( + "vm sandboxes require template.image, rootfs_tar_path in driver_config, or a configured default sandbox image", + ) + })?; info!( sandbox_id = %sandbox.id, image_ref = %image_ref, @@ -721,6 +733,10 @@ impl VmDriver { .and_then(|spec| spec.resource_requirements.as_ref()) .and_then(|requirements| driver_gpu_requirements(Some(requirements))) .is_some(); + let driver_config = + VmSandboxDriverConfig::from_sandbox(&sandbox).map_err(Status::invalid_argument)?; + let rootfs_tar_path = driver_config.rootfs_tar_path.map(PathBuf::from); + self.publish_platform_event( sandbox.id.clone(), platform_event( @@ -731,7 +747,9 @@ impl VmDriver { ), ); - let image_plan = self.prepare_runtime_images(&sandbox.id, &image_ref).await?; + let image_plan = self + .prepare_runtime_images(&sandbox.id, &image_ref, rootfs_tar_path.as_deref()) + .await?; let image_identity = image_plan.image_identity.clone(); self.ensure_provisioning_active(&sandbox.id).await?; info!( @@ -1462,7 +1480,14 @@ impl VmDriver { clear_stop_marker: bool, reconciliation_span: &tracing::Span, ) -> bool { - let Some(image_ref) = self.resolved_sandbox_image(&sandbox) else { + let has_rootfs_tar = VmSandboxDriverConfig::from_sandbox(&sandbox) + .is_ok_and(|c| c.rootfs_tar_path.is_some()); + + let Some(image_ref) = self.resolved_sandbox_image(&sandbox).or_else(|| { + has_rootfs_tar + .then(|| self.bootstrap_image_ref_default()) + .flatten() + }) else { warn!( sandbox_id = %sandbox.id, sandbox_name = %sandbox.name, @@ -1988,6 +2013,7 @@ impl VmDriver { &self, sandbox_id: &str, image_ref: &str, + rootfs_tar_path: Option<&Path>, ) -> Result { let span_status = openshell_otel::ErrorStatusGuard::current(); let bootstrap_image_ref = self.bootstrap_image_ref(image_ref); @@ -1996,6 +2022,18 @@ impl VmDriver { .await?; let root_disk = image_cache_rootfs_image(&self.config.state_dir, &bootstrap_image_identity); + if let Some(tar_path) = rootfs_tar_path { + let prepared = self + .ensure_prepared_rootfs_tar_disk(sandbox_id, tar_path, &root_disk) + .await?; + return Ok(RuntimeImagePlan { + root_disk, + image_disk: Some(prepared.disk_path), + image_identity: prepared.image_identity, + bootstrap_image_identity, + }); + } + if image_ref.trim() == bootstrap_image_ref.trim() { return span_status.finish(Ok(RuntimeImagePlan { root_disk, @@ -2017,15 +2055,20 @@ impl VmDriver { } fn bootstrap_image_ref(&self, sandbox_image_ref: &str) -> String { + self.bootstrap_image_ref_default() + .unwrap_or_else(|| sandbox_image_ref.to_string()) + } + + fn bootstrap_image_ref_default(&self) -> Option { let configured = self.config.bootstrap_image.trim(); if !configured.is_empty() { - return configured.to_string(); + return Some(configured.to_string()); } let default = self.config.default_image.trim(); if !default.is_empty() { - return default.to_string(); + return Some(default.to_string()); } - sandbox_image_ref.to_string() + None } #[tracing::instrument( @@ -2525,6 +2568,100 @@ impl VmDriver { }) } + async fn ensure_prepared_rootfs_tar_disk( + &self, + sandbox_id: &str, + tar_path: &Path, + bootstrap_root_disk: &Path, + ) -> Result { + let metadata = tokio::fs::metadata(tar_path).await.map_err(|err| { + Status::failed_precondition(format!( + "rootfs tar not accessible at {}: {err}", + tar_path.display() + )) + })?; + let mtime = metadata + .modified() + .unwrap_or(std::time::SystemTime::UNIX_EPOCH) + .duration_since(std::time::SystemTime::UNIX_EPOCH) + .unwrap_or_default() + .as_secs(); + let tar_identity = format!("rootfs-tar:{}:{mtime}", tar_path.display()); + let cache_identity = prepared_image_cache_identity(&tar_identity); + let image_path = image_cache_rootfs_image(&self.config.state_dir, &cache_identity); + let tar_display = tar_path.display().to_string(); + + if tokio::fs::metadata(&image_path).await.is_ok() { + self.publish_prepared_cache_hit( + sandbox_id, + &tar_display, + "rootfs_tar", + &cache_identity, + ); + return Ok(PreparedImageDisk { + image_identity: cache_identity, + disk_path: image_path, + }); + } + + self.publish_prepared_cache_miss(sandbox_id, &tar_display, "rootfs_tar", &cache_identity); + let _cache_guard = self.image_cache_lock.lock().await; + if tokio::fs::metadata(&image_path).await.is_ok() { + self.publish_prepared_cache_hit( + sandbox_id, + &tar_display, + "rootfs_tar", + &cache_identity, + ); + return Ok(PreparedImageDisk { + image_identity: cache_identity, + disk_path: image_path, + }); + } + + let staging_dir = image_cache_staging_dir(&self.config.state_dir, &cache_identity); + let rootfs_archive = staging_dir.join(IMAGE_EXPORT_ROOTFS_ARCHIVE); + self.reset_image_staging_dir(&staging_dir).await?; + + self.publish_vm_progress( + sandbox_id, + "CopyingRootfsTar", + format!("Copying rootfs tar \"{tar_display}\""), + HashMap::from([ + ("rootfs_tar_path".to_string(), tar_display.clone()), + ("image_source".to_string(), "rootfs_tar".to_string()), + ("image_identity".to_string(), cache_identity.clone()), + ]), + ); + if let Err(err) = tokio::fs::copy(tar_path, &rootfs_archive).await { + let _ = tokio::fs::remove_dir_all(&staging_dir).await; + return Err(Status::internal(format!( + "failed to copy rootfs tar to staging: {err}" + ))); + } + + let payload = GuestImagePayload { + image_ref: tar_display.clone(), + image_identity: cache_identity.clone(), + source: GuestImagePayloadSource::LocalDocker { rootfs_archive }, + }; + self.build_prepared_image_disk( + sandbox_id, + &tar_display, + "rootfs_tar", + &cache_identity, + bootstrap_root_disk, + &staging_dir, + &payload, + ) + .await?; + + Ok(PreparedImageDisk { + image_identity: cache_identity, + disk_path: image_path, + }) + } + async fn ensure_prepared_registry_image_disk( &self, sandbox_id: &str, diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index e5a27879f3..c1f256f65a 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -127,19 +127,22 @@ openshell sandbox create \ ### Custom Containers -Use `--from` to create a sandbox from the base image, another pre-built sandbox name, a local directory, or a container image: +Use `--from` to create a sandbox from the base image, another pre-built sandbox name, a local directory, a rootfs tar archive, or a container image: ```shell openshell sandbox create --from base openshell sandbox create --from ollama openshell sandbox create --from ./my-sandbox-dir +openshell sandbox create --from ./rootfs.tar openshell sandbox create --from my-registry.example.com/my-image:latest ``` Bare names such as `base` and `ollama` resolve to images under `ghcr.io/nvidia/openshell-community/sandboxes`. Set `OPENSHELL_COMMUNITY_REGISTRY` when you need to use an internal mirror. -Local directories and Dockerfiles require a local gateway because the CLI builds -through the local Docker daemon. Use a registry image reference for remote +Local directories and Dockerfiles require a local gateway because the CLI +builds images through the local Docker daemon. Rootfs tar archives +(`.tar`, `.tar.gz`, `.tgz`) also require a local gateway and are passed +directly to the VM compute driver. Use a registry image reference for remote gateways. ## Base Sandbox Container diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index 44881e1682..fd0eb7390a 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -52,6 +52,11 @@ name = "custom_image" path = "tests/custom_image.rs" required-features = ["e2e-docker"] +[[test]] +name = "rootfs_tar" +path = "tests/rootfs_tar.rs" +required-features = ["e2e-vm"] + [[test]] name = "docker_preflight" path = "tests/docker_preflight.rs" diff --git a/e2e/rust/tests/rootfs_tar.rs b/e2e/rust/tests/rootfs_tar.rs new file mode 100644 index 0000000000..e3d303654c --- /dev/null +++ b/e2e/rust/tests/rootfs_tar.rs @@ -0,0 +1,116 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e")] + +//! E2E test: create a sandbox from a flat rootfs tar archive. +//! +//! Prerequisites: +//! - A running VM-backed openshell gateway with a default sandbox image configured +//! - Docker daemon running (for image build + container export) +//! - The `openshell` binary (built automatically from the workspace) + +use openshell_e2e::harness::container::ContainerEngine; +use openshell_e2e::harness::output::strip_ansi; +use openshell_e2e::harness::sandbox::SandboxGuard; + +const DOCKERFILE_CONTENT: &str = r#"FROM public.ecr.aws/docker/library/python:3.13-slim + +# iproute2 is required for sandbox network namespace isolation. +RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ + && rm -rf /var/lib/apt/lists/* + +# Create the sandbox user/group so the supervisor can switch to it. +RUN groupadd -g 1000660000 sandbox && \ + useradd -m -u 1000660000 -g sandbox sandbox + +RUN echo "rootfs-tar-e2e-marker" > /etc/marker.txt + +CMD ["sleep", "infinity"] +"#; + +const MARKER: &str = "rootfs-tar-e2e-marker"; + +/// Build a Docker image, export its filesystem as a flat rootfs tar, then +/// create a sandbox from that tar and verify it contains the expected marker. +#[tokio::test] +async fn sandbox_from_rootfs_tar() { + let engine = ContainerEngine::from_env().expect("container engine available"); + let tmpdir = tempfile::tempdir().expect("create tmpdir"); + + // Step 1: Write a Dockerfile and build an image. + let dockerfile_path = tmpdir.path().join("Dockerfile"); + std::fs::write(&dockerfile_path, DOCKERFILE_CONTENT).expect("write Dockerfile"); + + let tag = format!( + "openshell/e2e-rootfs-tar-test:{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs() + ); + + let build_output = engine + .command() + .args(["build", "-t", &tag, "-f"]) + .arg(&dockerfile_path) + .arg(tmpdir.path()) + .output() + .expect("spawn docker build"); + + assert!( + build_output.status.success(), + "docker build failed:\n{}", + String::from_utf8_lossy(&build_output.stderr) + ); + + // Step 2: Create a temporary container and export its filesystem as a + // flat rootfs tar (equivalent to `docker export`). + let container_name = format!("openshell-e2e-rootfs-export-{}", std::process::id()); + + let create_output = engine + .command() + .args(["create", "--name", &container_name, &tag]) + .output() + .expect("spawn docker create"); + + assert!( + create_output.status.success(), + "docker create failed:\n{}", + String::from_utf8_lossy(&create_output.stderr) + ); + + let rootfs_tar_path = tmpdir.path().join("rootfs.tar"); + let export_output = engine + .command() + .args(["export", "-o"]) + .arg(&rootfs_tar_path) + .arg(&container_name) + .output() + .expect("spawn docker export"); + + assert!( + export_output.status.success(), + "docker export failed:\n{}", + String::from_utf8_lossy(&export_output.stderr) + ); + + // Clean up the temporary container and image. + let _ = engine.command().args(["rm", &container_name]).output(); + let _ = engine.command().args(["rmi", &tag]).output(); + + // Step 3: Create a sandbox from the rootfs tar. + let tar_str = rootfs_tar_path.to_str().expect("tar path is UTF-8"); + let mut guard = SandboxGuard::create(&["--from", tar_str, "--", "cat", "/etc/marker.txt"]) + .await + .expect("sandbox create from rootfs tar"); + + // Step 4: Verify the marker file content appears in the output. + let clean_output = strip_ansi(&guard.create_output); + assert!( + clean_output.contains(MARKER), + "expected marker '{MARKER}' in sandbox output:\n{clean_output}" + ); + + guard.cleanup().await; +} From 75e1f2be4b522e53381ab4e2a958c6dba7e031ff Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 26 Aug 2026 07:39:56 +0000 Subject: [PATCH 2/5] fix(sandbox): validate rootfs tar path at the VM driver boundary The rootfs_tar_path field in driver_config was passed from the API caller directly to tokio::fs::copy without validation. An authenticated user bypassing the CLI could supply arbitrary host paths (e.g. /dev/zero for disk exhaustion, or readable host files for data exfiltration). Introduce a trusted staging directory that the VM driver creates on startup and advertises via GetCapabilities. The CLI now copies the tar into the staging directory before creating the sandbox, and the driver validates that the received path is a regular file inside the staging root and within a configurable size limit (default 10 GiB) before any I/O. New VmDriverConfig options: - rootfs_tar_staging_dir: override the staging directory (default: /rootfs-tar-staging) - rootfs_tar_max_bytes: override the size limit (default: 10 GiB) Addresses GATOR-28b5152e-01. Signed-off-by: Philippe Martin --- crates/openshell-cli/src/run.rs | 44 +++++++-- crates/openshell-driver-docker/src/lib.rs | 1 + .../openshell-driver-kubernetes/src/driver.rs | 1 + crates/openshell-driver-podman/src/driver.rs | 1 + crates/openshell-driver-vm/src/driver.rs | 93 ++++++++++++++++++- crates/openshell-driver-vm/src/main.rs | 8 ++ crates/openshell-server/src/compute/mod.rs | 8 ++ crates/openshell-server/src/grpc/mod.rs | 1 + crates/openshell-server/src/test_support.rs | 1 + proto/compute_driver.proto | 5 + proto/openshell.proto | 3 + 11 files changed, 157 insertions(+), 9 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index e36624018d..97d980cab4 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -488,8 +488,9 @@ pub async fn sandbox_create( (Some(tag), None) } ResolvedSource::RootfsTar { path } => { - validate_rootfs_tar_source(gateway_name, &mut client, &path).await?; - (None, Some(path)) + let staged = + validate_and_stage_rootfs_tar(gateway_name, &mut client, &path).await?; + (None, Some(staged)) } } } @@ -1241,12 +1242,13 @@ async fn build_from_dockerfile( Ok(tag) } -/// Validate that a rootfs tar source is usable with the current gateway. -async fn validate_rootfs_tar_source( +/// Validate that a rootfs tar source is usable with the current gateway, then +/// copy it into the driver's staging directory. Returns the staged path. +async fn validate_and_stage_rootfs_tar( gateway_name: &str, client: &mut crate::tls::GrpcClient, tar_path: &Path, -) -> Result<()> { +) -> Result { let metadata = get_gateway_metadata(gateway_name); if !dockerfile_sources_supported_for_gateway(metadata.as_ref()) { return Err(miette!( @@ -1262,7 +1264,11 @@ async fn validate_rootfs_tar_source( .wrap_err("failed to query gateway compute driver")? .into_inner(); - let driver_name = info.compute_drivers.first().map_or("", |d| d.name.as_str()); + let driver = info + .compute_drivers + .first() + .ok_or_else(|| miette!("gateway '{}' has no compute drivers", gateway_name))?; + let driver_name = driver.name.as_str(); if driver_name != "vm" { return Err(miette!( @@ -1273,14 +1279,36 @@ async fn validate_rootfs_tar_source( )); } + let staging_dir = driver + .capabilities + .as_ref() + .map_or("", |c| c.rootfs_tar_staging_dir.as_str()); + if staging_dir.is_empty() { + return Err(miette!( + "gateway '{}' VM driver did not advertise a rootfs tar staging directory", + gateway_name + )); + } + let staging_dir = PathBuf::from(staging_dir); + + let file_name = tar_path + .file_name() + .ok_or_else(|| miette!("rootfs tar path has no filename"))?; + let staged_name = format!("{}-{}", std::process::id(), file_name.to_string_lossy()); + let staged_path = staging_dir.join(&staged_name); + eprintln!( - "Using rootfs tar {} for gateway '{}'", + "Staging rootfs tar {} for gateway '{}'", tar_path.display().to_string().cyan(), gateway_name, ); + tokio::fs::copy(tar_path, &staged_path) + .await + .into_diagnostic() + .wrap_err_with(|| format!("failed to stage rootfs tar to {}", staged_path.display()))?; eprintln!(); - Ok(()) + Ok(staged_path) } /// Build a `driver_config` struct carrying the rootfs tar path for the VM driver. diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index b1859f54cc..2cd897d4f4 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -635,6 +635,7 @@ impl DockerComputeDriver { driver_version: self.config.daemon_version.clone(), default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, + rootfs_tar_staging_dir: String::new(), } } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 84d7029de4..b2a95e7232 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -549,6 +549,7 @@ impl KubernetesComputeDriver { driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), }) } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index a11189cbc6..08e9188292 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -507,6 +507,7 @@ impl PodmanComputeDriver { driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, + rootfs_tar_staging_dir: String::new(), }) } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 2bdaf25e55..0a657caf33 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -86,6 +86,9 @@ const DEFAULT_MEM_MIB: u32 = 2048; const DEFAULT_OVERLAY_DISK_MIB: u64 = 4096; const DEFAULT_REGISTRY_LAYER_DOWNLOAD_CONCURRENCY: usize = 4; const MAX_REGISTRY_LAYER_DOWNLOAD_CONCURRENCY: usize = 16; +/// 10 GiB — configurable via `rootfs_tar_max_bytes`. +const DEFAULT_ROOTFS_TAR_MAX_BYTES: u64 = 10 * 1024 * 1024 * 1024; +const ROOTFS_TAR_STAGING_DIR: &str = "rootfs-tar-staging"; #[derive(Debug, Clone, Default, serde::Deserialize)] #[serde(default, deny_unknown_fields)] @@ -244,6 +247,13 @@ pub struct VmDriverConfig { /// When empty, defaults to the resolved UID. #[serde(default, skip_serializing_if = "Option::is_none")] pub sandbox_gid: Option, + /// Directory where rootfs tar files must be staged before they can be + /// referenced in a `CreateSandbox` request. Defaults to `/rootfs-tar-staging`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rootfs_tar_staging_dir: Option, + /// Maximum rootfs tar file size in bytes. Defaults to 10 GiB. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub rootfs_tar_max_bytes: Option, } /// Default sandbox UID used by the VM driver when no config value is set. @@ -270,6 +280,8 @@ impl Default for VmDriverConfig { gpu_vcpus: 4, sandbox_uid: None, sandbox_gid: None, + rootfs_tar_staging_dir: None, + rootfs_tar_max_bytes: None, } } } @@ -308,6 +320,17 @@ impl VmDriverConfig { Ok(()) } + fn rootfs_tar_staging_dir(&self) -> PathBuf { + self.rootfs_tar_staging_dir + .clone() + .unwrap_or_else(|| self.state_dir.join(ROOTFS_TAR_STAGING_DIR)) + } + + fn rootfs_tar_max_bytes(&self) -> u64 { + self.rootfs_tar_max_bytes + .unwrap_or(DEFAULT_ROOTFS_TAR_MAX_BYTES) + } + fn requires_tls_materials(&self) -> bool { self.openshell_endpoint.starts_with("https://") } @@ -478,6 +501,13 @@ impl VmDriver { image_cache_root.display() ) })?; + let staging_dir = config.rootfs_tar_staging_dir(); + create_private_dir_all(&staging_dir).await.map_err(|err| { + format!( + "failed to create rootfs tar staging dir '{}': {err}", + staging_dir.display() + ) + })?; let launcher_bin = if let Some(path) = config.launcher_bin.clone() { path @@ -518,6 +548,59 @@ impl VmDriver { Ok(driver) } + async fn validate_rootfs_tar_path(&self, raw: &Path) -> Result { + let staging_dir = self.config.rootfs_tar_staging_dir(); + let canonical_staging = tokio::fs::canonicalize(&staging_dir).await.map_err(|err| { + Status::internal(format!( + "rootfs tar staging dir not accessible at {}: {err}", + staging_dir.display() + )) + })?; + + let canonical = tokio::fs::canonicalize(raw).await.map_err(|err| { + Status::failed_precondition(format!( + "rootfs tar path not accessible at {}: {err}", + raw.display() + )) + })?; + + if !canonical.starts_with(&canonical_staging) { + return Err(Status::permission_denied(format!( + "rootfs tar path {} is outside the staging directory {}", + canonical.display(), + canonical_staging.display() + ))); + } + + let metadata = tokio::fs::symlink_metadata(&canonical) + .await + .map_err(|err| { + Status::failed_precondition(format!( + "rootfs tar not accessible at {}: {err}", + canonical.display() + )) + })?; + if !metadata.file_type().is_file() { + return Err(Status::invalid_argument(format!( + "rootfs tar path {} is not a regular file", + canonical.display() + ))); + } + + let max_bytes = self.config.rootfs_tar_max_bytes(); + let file_size = metadata.len(); + if file_size > max_bytes { + return Err(Status::invalid_argument(format!( + "rootfs tar {} is {} bytes, exceeding the {} byte limit", + canonical.display(), + file_size, + max_bytes + ))); + } + + Ok(canonical) + } + #[must_use] pub fn capabilities(&self) -> GetCapabilitiesResponse { GetCapabilitiesResponse { @@ -525,6 +608,11 @@ impl VmDriver { driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, + rootfs_tar_staging_dir: self + .config + .rootfs_tar_staging_dir() + .to_string_lossy() + .into_owned(), } } @@ -735,7 +823,10 @@ impl VmDriver { .is_some(); let driver_config = VmSandboxDriverConfig::from_sandbox(&sandbox).map_err(Status::invalid_argument)?; - let rootfs_tar_path = driver_config.rootfs_tar_path.map(PathBuf::from); + let rootfs_tar_path = match driver_config.rootfs_tar_path { + Some(raw) => Some(self.validate_rootfs_tar_path(Path::new(&raw)).await?), + None => None, + }; self.publish_platform_event( sandbox.id.clone(), diff --git a/crates/openshell-driver-vm/src/main.rs b/crates/openshell-driver-vm/src/main.rs index 949d4ce05c..748361758b 100644 --- a/crates/openshell-driver-vm/src/main.rs +++ b/crates/openshell-driver-vm/src/main.rs @@ -143,6 +143,12 @@ struct Args { #[arg(long, env = "OPENSHELL_VM_SANDBOX_GID")] sandbox_gid: Option, + #[arg(long, env = "OPENSHELL_VM_ROOTFS_TAR_STAGING_DIR")] + rootfs_tar_staging_dir: Option, + + #[arg(long, env = "OPENSHELL_VM_ROOTFS_TAR_MAX_BYTES")] + rootfs_tar_max_bytes: Option, + #[arg(long, hide = true)] vm_backend: Option, @@ -238,6 +244,8 @@ async fn main() -> Result<()> { gpu_vcpus: args.gpu_vcpus, sandbox_uid: args.sandbox_uid, sandbox_gid: args.sandbox_gid, + rootfs_tar_staging_dir: args.rootfs_tar_staging_dir.clone(), + rootfs_tar_max_bytes: args.rootfs_tar_max_bytes, }) .await .map_err(|err| miette::miette!("{err}"))?; diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 51f05a395c..ef936ad77c 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -280,6 +280,8 @@ pub struct ComputeDriverInfoSnapshot { pub driver_version: String, /// Whether the driver asks the gateway to reconcile compute across restarts. pub gateway_manages_lifecycle: bool, + /// Directory where rootfs tar files must be staged. + pub rootfs_tar_staging_dir: String, } /// Interval between store-vs-backend reconciliation sweeps. @@ -617,6 +619,7 @@ impl ComputeRuntime { driver_name: capabilities.driver_name, driver_version: capabilities.driver_version, gateway_manages_lifecycle: capabilities.gateway_manages_lifecycle, + rootfs_tar_staging_dir: capabilities.rootfs_tar_staging_dir, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -4105,6 +4108,7 @@ impl ComputeDriver for NoopTestDriver { driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), }, )) } @@ -4246,6 +4250,7 @@ pub async fn new_test_runtime_with_driver( driver_name: driver_name.to_string(), driver_version: "test".to_string(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4409,6 +4414,7 @@ mod tests { driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), })) } @@ -4727,6 +4733,7 @@ mod tests { driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), })) } @@ -4930,6 +4937,7 @@ mod tests { driver_name: driver_name.to_string(), driver_version: "test".to_string(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index f502369bcf..b2529ac031 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -255,6 +255,7 @@ impl OpenShell for OpenShellService { capabilities: Some(ComputeDriverCapabilities { driver_name: driver.driver_name.clone(), driver_version: driver.driver_version.clone(), + rootfs_tar_staging_dir: driver.rootfs_tar_staging_dir.clone(), }), }) .collect(); diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index 957db46fab..1cdc3f4b20 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -95,6 +95,7 @@ impl FakeComputeDriver { driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, + rootfs_tar_staging_dir: String::new(), }, gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index afa93f1b18..02b96c5212 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -78,6 +78,11 @@ message GetCapabilitiesResponse { // Whether the gateway should stop running sandbox compute during graceful // shutdown and restart the retained running intent on startup. bool gateway_manages_lifecycle = 6; + + // Absolute path to the directory where rootfs tar files must be staged + // before being referenced in a CreateSandbox request. The driver rejects + // paths outside this directory. + string rootfs_tar_staging_dir = 7; } message GetGatewayListenerRequirementsRequest {} diff --git a/proto/openshell.proto b/proto/openshell.proto index 246fe0626f..32dff3b33f 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -789,6 +789,9 @@ message ComputeDriverCapabilities { // Driver-reported implementation version from the startup capability snapshot. string driver_version = 2; + // Absolute path where rootfs tar files must be staged before creating a + // sandbox. Empty when the driver does not support rootfs tar sources. + string rootfs_tar_staging_dir = 3; } // Public sandbox resource exposed by the OpenShell API. From 168b9210cc70a99b003a18b18b2bead91d30f0b9 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 26 Aug 2026 09:12:01 +0000 Subject: [PATCH 3/5] fix(sandbox): request-scoped staging, size pre-check, and cleanup for rootfs tar Tighten the rootfs tar staging flow to address the remaining GATOR-01 obligations: - Request-scoped staging: the CLI creates a unique per-request subdirectory (req-) under the staging root instead of placing files directly in the shared directory. The driver enforces that the tar path is at depth 2 (staging_root//), preventing cross-request path selection. - Size pre-check: the driver advertises rootfs_tar_max_bytes via GetCapabilities. The CLI reads this limit and rejects oversized files before copying, avoiding disk exhaustion in the staging directory. - Cleanup: the driver removes the request staging subdirectory after consuming the tar (on cache hit, copy success, or copy failure), ensuring staged data does not persist beyond the request. Signed-off-by: Philippe Martin --- crates/openshell-cli/src/run.rs | 47 +++++++++++++++---- crates/openshell-driver-docker/src/lib.rs | 1 + .../openshell-driver-kubernetes/src/driver.rs | 1 + crates/openshell-driver-podman/src/driver.rs | 1 + crates/openshell-driver-vm/src/driver.rs | 21 +++++++++ crates/openshell-server/src/compute/mod.rs | 8 ++++ crates/openshell-server/src/grpc/mod.rs | 1 + crates/openshell-server/src/test_support.rs | 1 + proto/compute_driver.proto | 4 ++ proto/openshell.proto | 3 ++ 10 files changed, 78 insertions(+), 10 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 97d980cab4..bf7134a2a5 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1279,10 +1279,8 @@ async fn validate_and_stage_rootfs_tar( )); } - let staging_dir = driver - .capabilities - .as_ref() - .map_or("", |c| c.rootfs_tar_staging_dir.as_str()); + let caps = driver.capabilities.as_ref(); + let staging_dir = caps.map_or("", |c| c.rootfs_tar_staging_dir.as_str()); if staging_dir.is_empty() { return Err(miette!( "gateway '{}' VM driver did not advertise a rootfs tar staging directory", @@ -1291,21 +1289,50 @@ async fn validate_and_stage_rootfs_tar( } let staging_dir = PathBuf::from(staging_dir); + let max_bytes = caps.map_or(0, |c| c.rootfs_tar_max_bytes); + if max_bytes > 0 { + let source_meta = tokio::fs::metadata(tar_path) + .await + .into_diagnostic() + .wrap_err_with(|| format!("failed to read {}", tar_path.display()))?; + if source_meta.len() > max_bytes { + return Err(miette!( + "rootfs tar {} is {} bytes, exceeding the gateway limit of {} bytes", + tar_path.display(), + source_meta.len(), + max_bytes + )); + } + } + + let request_dir = staging_dir.join(format!("req-{}", std::process::id())); + tokio::fs::create_dir_all(&request_dir) + .await + .into_diagnostic() + .wrap_err_with(|| { + format!( + "failed to create staging directory {}", + request_dir.display() + ) + })?; + let file_name = tar_path .file_name() .ok_or_else(|| miette!("rootfs tar path has no filename"))?; - let staged_name = format!("{}-{}", std::process::id(), file_name.to_string_lossy()); - let staged_path = staging_dir.join(&staged_name); + let staged_path = request_dir.join(file_name); eprintln!( "Staging rootfs tar {} for gateway '{}'", tar_path.display().to_string().cyan(), gateway_name, ); - tokio::fs::copy(tar_path, &staged_path) - .await - .into_diagnostic() - .wrap_err_with(|| format!("failed to stage rootfs tar to {}", staged_path.display()))?; + if let Err(err) = tokio::fs::copy(tar_path, &staged_path).await { + let _ = tokio::fs::remove_dir_all(&request_dir).await; + return Err(miette!( + "failed to stage rootfs tar to {}: {err}", + staged_path.display() + )); + } eprintln!(); Ok(staged_path) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 2cd897d4f4..4b9cf76c44 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -636,6 +636,7 @@ impl DockerComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, } } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index b2a95e7232..9394588512 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -550,6 +550,7 @@ impl KubernetesComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, }) } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 08e9188292..4aeb4adfce 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -508,6 +508,7 @@ impl PodmanComputeDriver { default_image: self.config.default_image.clone(), gateway_manages_lifecycle: true, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, }) } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 0a657caf33..69e681daec 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -572,6 +572,15 @@ impl VmDriver { ))); } + let relative = canonical.strip_prefix(&canonical_staging).unwrap(); + let depth = relative.components().count(); + if depth != 2 { + return Err(Status::permission_denied(format!( + "rootfs tar path {} must be inside a request subdirectory of the staging root", + canonical.display(), + ))); + } + let metadata = tokio::fs::symlink_metadata(&canonical) .await .map_err(|err| { @@ -613,6 +622,7 @@ impl VmDriver { .rootfs_tar_staging_dir() .to_string_lossy() .into_owned(), + rootfs_tar_max_bytes: self.config.rootfs_tar_max_bytes(), } } @@ -2665,6 +2675,13 @@ impl VmDriver { tar_path: &Path, bootstrap_root_disk: &Path, ) -> Result { + let request_staging_dir = tar_path.parent().map(Path::to_path_buf); + let cleanup_request_staging = || async { + if let Some(d) = &request_staging_dir { + let _ = tokio::fs::remove_dir_all(d).await; + } + }; + let metadata = tokio::fs::metadata(tar_path).await.map_err(|err| { Status::failed_precondition(format!( "rootfs tar not accessible at {}: {err}", @@ -2689,6 +2706,7 @@ impl VmDriver { "rootfs_tar", &cache_identity, ); + cleanup_request_staging().await; return Ok(PreparedImageDisk { image_identity: cache_identity, disk_path: image_path, @@ -2704,6 +2722,7 @@ impl VmDriver { "rootfs_tar", &cache_identity, ); + cleanup_request_staging().await; return Ok(PreparedImageDisk { image_identity: cache_identity, disk_path: image_path, @@ -2726,10 +2745,12 @@ impl VmDriver { ); if let Err(err) = tokio::fs::copy(tar_path, &rootfs_archive).await { let _ = tokio::fs::remove_dir_all(&staging_dir).await; + cleanup_request_staging().await; return Err(Status::internal(format!( "failed to copy rootfs tar to staging: {err}" ))); } + cleanup_request_staging().await; let payload = GuestImagePayload { image_ref: tar_display.clone(), diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index ef936ad77c..38b680f3b1 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -282,6 +282,8 @@ pub struct ComputeDriverInfoSnapshot { pub gateway_manages_lifecycle: bool, /// Directory where rootfs tar files must be staged. pub rootfs_tar_staging_dir: String, + /// Maximum rootfs tar file size in bytes. + pub rootfs_tar_max_bytes: u64, } /// Interval between store-vs-backend reconciliation sweeps. @@ -620,6 +622,7 @@ impl ComputeRuntime { driver_version: capabilities.driver_version, gateway_manages_lifecycle: capabilities.gateway_manages_lifecycle, rootfs_tar_staging_dir: capabilities.rootfs_tar_staging_dir, + rootfs_tar_max_bytes: capabilities.rootfs_tar_max_bytes, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -4109,6 +4112,7 @@ impl ComputeDriver for NoopTestDriver { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, }, )) } @@ -4251,6 +4255,7 @@ pub async fn new_test_runtime_with_driver( driver_version: "test".to_string(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4415,6 +4420,7 @@ mod tests { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, })) } @@ -4734,6 +4740,7 @@ mod tests { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, })) } @@ -4938,6 +4945,7 @@ mod tests { driver_version: "test".to_string(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index b2529ac031..d12b7739a7 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -256,6 +256,7 @@ impl OpenShell for OpenShellService { driver_name: driver.driver_name.clone(), driver_version: driver.driver_version.clone(), rootfs_tar_staging_dir: driver.rootfs_tar_staging_dir.clone(), + rootfs_tar_max_bytes: driver.rootfs_tar_max_bytes, }), }) .collect(); diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index 1cdc3f4b20..b60cf176c1 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -96,6 +96,7 @@ impl FakeComputeDriver { default_image: "openshell/sandbox:test".to_string(), gateway_manages_lifecycle: false, rootfs_tar_staging_dir: String::new(), + rootfs_tar_max_bytes: 0, }, gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 02b96c5212..0f2bf0f9da 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -83,6 +83,10 @@ message GetCapabilitiesResponse { // before being referenced in a CreateSandbox request. The driver rejects // paths outside this directory. string rootfs_tar_staging_dir = 7; + + // Maximum rootfs tar file size in bytes accepted by the driver. Zero means + // the driver does not support rootfs tar sources. + uint64 rootfs_tar_max_bytes = 8; } message GetGatewayListenerRequirementsRequest {} diff --git a/proto/openshell.proto b/proto/openshell.proto index 32dff3b33f..0f46c0327d 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -792,6 +792,9 @@ message ComputeDriverCapabilities { // Absolute path where rootfs tar files must be staged before creating a // sandbox. Empty when the driver does not support rootfs tar sources. string rootfs_tar_staging_dir = 3; + + // Maximum rootfs tar file size in bytes accepted by the driver. + uint64 rootfs_tar_max_bytes = 4; } // Public sandbox resource exposed by the OpenShell API. From 8ec3e99844bc6061e55b6c28421af9f94a676264 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 26 Aug 2026 10:36:49 +0000 Subject: [PATCH 4/5] fix(vm): restore rootfs-tar sandboxes from persisted image identity On restore or restart, the one-shot staged tar archive has already been cleaned up. Reading the persisted image identity from the sandbox state directory and resolving the cached disk path directly avoids re-accessing the deleted staging path. Addresses GATOR-168b9210-01. Signed-off-by: Philippe Martin --- crates/openshell-driver-vm/src/driver.rs | 41 +++++++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 69e681daec..b04f9b2f18 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -833,9 +833,12 @@ impl VmDriver { .is_some(); let driver_config = VmSandboxDriverConfig::from_sandbox(&sandbox).map_err(Status::invalid_argument)?; + let driver_config_had_rootfs_tar = driver_config.rootfs_tar_path.is_some(); let rootfs_tar_path = match driver_config.rootfs_tar_path { - Some(raw) => Some(self.validate_rootfs_tar_path(Path::new(&raw)).await?), - None => None, + Some(raw) if overlay_preparation == OverlayPreparation::Fresh => { + Some(self.validate_rootfs_tar_path(Path::new(&raw)).await?) + } + Some(_) | None => None, }; self.publish_platform_event( @@ -848,9 +851,32 @@ impl VmDriver { ), ); - let image_plan = self - .prepare_runtime_images(&sandbox.id, &image_ref, rootfs_tar_path.as_deref()) - .await?; + let image_plan = if overlay_preparation == OverlayPreparation::PreserveExisting + && driver_config_had_rootfs_tar + { + let persisted_identity = + read_persisted_image_identity(&state_dir).await.map_err(|err| { + Status::internal(format!( + "cannot restore rootfs-tar sandbox: persisted image identity not found: {err}" + )) + })?; + let bootstrap_image_ref = self.bootstrap_image_ref(&image_ref); + let bootstrap_image_identity = self + .ensure_cached_bootstrap_rootfs_image(&sandbox.id, &bootstrap_image_ref) + .await?; + let root_disk = + image_cache_rootfs_image(&self.config.state_dir, &bootstrap_image_identity); + let image_disk = image_cache_rootfs_image(&self.config.state_dir, &persisted_identity); + RuntimeImagePlan { + root_disk, + image_disk: Some(image_disk), + image_identity: persisted_identity, + bootstrap_image_identity, + } + } else { + self.prepare_runtime_images(&sandbox.id, &image_ref, rootfs_tar_path.as_deref()) + .await? + }; let image_identity = image_plan.image_identity.clone(); self.ensure_provisioning_active(&sandbox.id).await?; info!( @@ -5066,6 +5092,11 @@ async fn write_sandbox_image_metadata( Ok(()) } +async fn read_persisted_image_identity(state_dir: &Path) -> Result { + let raw = tokio::fs::read_to_string(state_dir.join(IMAGE_IDENTITY_FILE)).await?; + Ok(raw.trim().to_string()) +} + async fn write_sandbox_request(state_dir: &Path, sandbox: &Sandbox) -> Result<(), std::io::Error> { restrict_owner_only_dir(state_dir).await?; write_private_file( From c88045bd670bbfff1ba249aa196e2550b4541ea9 Mon Sep 17 00:00:00 2001 From: Philippe Martin Date: Wed, 26 Aug 2026 12:07:22 +0000 Subject: [PATCH 5/5] fix(cli): use random staging dirs and enforce byte limit during rootfs tar copy Replace PID-based request staging directories with tempfile-generated random names to prevent collisions and make paths unpredictable. Replace bare tokio::fs::copy with a streaming copy loop that enforces the advertised max_bytes limit during transfer, closing the TOCTOU gap between the pre-copy size check and the actual copy. Signed-off-by: Philippe Martin Signed-off-by: Philippe Martin --- crates/openshell-cli/src/run.rs | 58 +++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index bf7134a2a5..d9615280f1 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -1305,16 +1305,12 @@ async fn validate_and_stage_rootfs_tar( } } - let request_dir = staging_dir.join(format!("req-{}", std::process::id())); - tokio::fs::create_dir_all(&request_dir) - .await + let request_dir = tempfile::Builder::new() + .prefix("req-") + .tempdir_in(&staging_dir) .into_diagnostic() - .wrap_err_with(|| { - format!( - "failed to create staging directory {}", - request_dir.display() - ) - })?; + .wrap_err("failed to create request staging directory")? + .keep(); let file_name = tar_path .file_name() @@ -1326,7 +1322,7 @@ async fn validate_and_stage_rootfs_tar( tar_path.display().to_string().cyan(), gateway_name, ); - if let Err(err) = tokio::fs::copy(tar_path, &staged_path).await { + if let Err(err) = copy_with_byte_limit(tar_path, &staged_path, max_bytes).await { let _ = tokio::fs::remove_dir_all(&request_dir).await; return Err(miette!( "failed to stage rootfs tar to {}: {err}", @@ -1338,6 +1334,48 @@ async fn validate_and_stage_rootfs_tar( Ok(staged_path) } +/// Copy `src` to `dst`, aborting if total bytes written exceeds `limit`. +/// A limit of 0 disables enforcement. +async fn copy_with_byte_limit( + src: &Path, + dst: &Path, + limit: u64, +) -> std::result::Result<(), String> { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let mut reader = tokio::fs::File::open(src) + .await + .map_err(|e| format!("open source: {e}"))?; + let mut writer = tokio::fs::File::create(dst) + .await + .map_err(|e| format!("create destination: {e}"))?; + + let mut buf = vec![0u8; 64 * 1024]; + let mut total: u64 = 0; + loop { + let n = reader + .read(&mut buf) + .await + .map_err(|e| format!("read: {e}"))?; + if n == 0 { + break; + } + total += n as u64; + if limit > 0 && total > limit { + return Err(format!( + "{} exceeds the {} byte limit", + src.display(), + limit + )); + } + writer + .write_all(&buf[..n]) + .await + .map_err(|e| format!("write: {e}"))?; + } + Ok(()) +} + /// Build a `driver_config` struct carrying the rootfs tar path for the VM driver. fn rootfs_tar_driver_config(tar_path: &Path) -> Result { let fields = serde_json::Map::from_iter([(