From 5de9b86b70513da09225c7baba202694d931f901 Mon Sep 17 00:00:00 2001 From: Brian Luo <57960778+law-chain-hot@users.noreply.github.com> Date: Wed, 8 Jul 2026 21:26:28 +0800 Subject: [PATCH] fix(runtime): route user volumes through bwrap shared tree --- src/boxlite/src/jailer/builder.rs | 20 +- src/boxlite/src/jailer/mod.rs | 57 +++++- src/boxlite/src/jailer/sandbox/bwrap.rs | 69 +++++++ src/boxlite/src/jailer/sandbox/composite.rs | 1 + src/boxlite/src/jailer/sandbox/landlock.rs | 1 + src/boxlite/src/jailer/sandbox/mod.rs | 24 +++ .../src/litebox/init/tasks/vmm_spawn.rs | 183 ++++++++++++++---- src/boxlite/src/vmm/controller/shim.rs | 6 + src/boxlite/src/vmm/controller/spawn.rs | 11 +- src/boxlite/src/volumes/container_volume.rs | 45 ++++- src/guest/src/container/spec.rs | 39 +++- src/guest/src/service/container.rs | 5 +- src/shared/proto/boxlite/v1/service.proto | 2 +- 13 files changed, 399 insertions(+), 64 deletions(-) diff --git a/src/boxlite/src/jailer/builder.rs b/src/boxlite/src/jailer/builder.rs index 7389c82dd..77fe5b41b 100644 --- a/src/boxlite/src/jailer/builder.rs +++ b/src/boxlite/src/jailer/builder.rs @@ -1,7 +1,7 @@ //! JailerBuilder for constructing a [`Jailer`](super::Jailer). use super::Jailer; -use super::sandbox::{PlatformSandbox, Sandbox}; +use super::sandbox::{PlatformSandbox, Sandbox, SandboxBindMount}; use crate::runtime::advanced_options::{ResourceLimits, SecurityOptions}; use crate::runtime::layout::BoxFilesystemLayout; use crate::runtime::options::VolumeSpec; @@ -26,6 +26,7 @@ use std::path::PathBuf; pub struct JailerBuilder { security: SecurityOptions, volumes: Vec, + bind_mounts: Vec, box_id: Option, layout: Option, preserved_fds: Vec<(RawFd, i32)>, @@ -44,6 +45,7 @@ impl JailerBuilder { Self { security: SecurityOptions::default(), volumes: Vec::new(), + bind_mounts: Vec::new(), box_id: None, layout: None, preserved_fds: Vec::new(), @@ -84,6 +86,16 @@ impl JailerBuilder { self } + /// Set bind mounts that should be installed by namespace sandboxes. + /// + /// These are distinct from `volumes`: `volumes` describe user intent and + /// path-access policy, while sandbox bind mounts name exact source/target + /// pairs that bwrap should mount inside its private namespace. + pub fn with_bind_mounts(mut self, bind_mounts: Vec) -> Self { + self.bind_mounts = bind_mounts; + self + } + /// Enable or disable jailer isolation. pub fn with_jailer_enabled(mut self, enabled: bool) -> Self { self.security.jailer_enabled = enabled; @@ -324,6 +336,7 @@ impl JailerBuilder { sandbox, security: self.security, volumes: self.volumes, + bind_mounts: self.bind_mounts, box_id, layout, preserved_fds: self.preserved_fds, @@ -571,7 +584,8 @@ mod tests { .expect("should build dev"); assert!(!dev.security().jailer_enabled); - // Maximum → strict: jailer + seccomp on, namespaces on. + // Maximum → strict profile. Seccomp is Linux-only, matching + // SecurityOptions::default(). let max = JailerBuilder::new() .with_box_id("test-box") .with_layout(test_layout("/tmp/box")) @@ -579,7 +593,7 @@ mod tests { .build() .expect("should build max"); assert!(max.security().jailer_enabled); - assert!(max.security().seccomp_enabled); + assert_eq!(max.security().seccomp_enabled, cfg!(target_os = "linux")); // Standard is the recommended default; just confirm it // overrides whatever the chain set before. diff --git a/src/boxlite/src/jailer/mod.rs b/src/boxlite/src/jailer/mod.rs index fc7121a21..9c4ce13af 100644 --- a/src/boxlite/src/jailer/mod.rs +++ b/src/boxlite/src/jailer/mod.rs @@ -85,7 +85,8 @@ pub use crate::runtime::advanced_options::{ResourceLimits, SecurityOptions}; pub use builder::JailerBuilder; pub use error::{ConfigError, IsolationError, JailerError, SystemError}; pub use sandbox::{ - CompositeSandbox, NoopSandbox, PathAccess, PlatformSandbox, Sandbox, SandboxContext, + CompositeSandbox, NoopSandbox, PathAccess, PlatformSandbox, Sandbox, SandboxBindMount, + SandboxContext, }; // ============================================================================ @@ -321,9 +322,9 @@ fn build_path_access(layout: &BoxFilesystemLayout, volumes: &[VolumeSpec]) -> Ve } } - // User volumes. Directories are shared directly, so grant the VMM access. - // Single files are staged under shared_dir (granted above), so they need no - // grant here — this also keeps the file's host siblings out of the sandbox. + // Fallback user volumes. Direct directory virtio-fs shares need source + // access here. Volumes handled by SandboxBindMount skip this path, so the + // original host source is not also visible at its original path. for vol in volumes { let p = PathBuf::from(&vol.host_path); if let Some(VolumeShare::Dir(dir)) = classify_volume_share(&p) { @@ -378,6 +379,8 @@ pub struct Jailer { pub(crate) security: SecurityOptions, /// Volume mounts (for sandbox path restrictions). pub(crate) volumes: Vec, + /// Bind mounts installed inside namespace sandboxes. + pub(crate) bind_mounts: Vec, /// Unique box identifier. pub(crate) box_id: String, /// Box filesystem layout (provides typed path accessors). @@ -532,10 +535,16 @@ impl Jailer { /// /// Delegates to [`build_path_access`] for granular filesystem rules. fn context(&self) -> SandboxContext<'_> { - let paths = build_path_access(&self.layout, &self.volumes); + let path_volumes: &[VolumeSpec] = if self.bind_mounts.is_empty() { + self.volumes.as_slice() + } else { + &[] + }; + let paths = build_path_access(&self.layout, path_volumes); tracing::debug!( box_id = %self.box_id, path_count = paths.len(), + bind_mount_count = self.bind_mounts.len(), paths = ?paths, "Built sandbox path access list" ); @@ -546,6 +555,7 @@ impl Jailer { SandboxContext { id: &self.box_id, paths, + bind_mounts: self.bind_mounts.clone(), resource_limits: &self.security.resource_limits, network_enabled: self.security.network_enabled, sandbox_profile: self.security.sandbox_profile.as_deref(), @@ -843,6 +853,43 @@ mod tests { assert!(rw_vol.writable, "RW volume should be writable"); } + #[test] + fn test_context_with_sandbox_bind_mounts_does_not_expose_volume_source_path() { + let dir = tempdir().unwrap(); + let layout = test_layout(dir.path().join("box")); + + let host_volume = dir.path().join("host-data"); + std::fs::create_dir_all(&host_volume).unwrap(); + let sandbox_target = layout.shared_dir().join("containers/main/volumes/uservol0"); + + let jail = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(layout) + .with_volumes(vec![VolumeSpec { + host_path: host_volume.to_string_lossy().to_string(), + guest_path: "/data".to_string(), + read_only: false, + }]) + .with_bind_mounts(vec![SandboxBindMount::new( + host_volume.clone(), + sandbox_target.clone(), + true, + )]) + .build() + .expect("jailer should build"); + + let ctx = jail.context(); + + assert!( + ctx.paths.iter().all(|p| p.path != host_volume), + "sandbox bind-backed volumes should not also expose the host source at its original path" + ); + assert_eq!(ctx.bind_mounts.len(), 1); + assert_eq!(ctx.bind_mounts[0].source, host_volume); + assert_eq!(ctx.bind_mounts[0].target, sandbox_target); + assert!(ctx.bind_mounts[0].writable); + } + #[test] fn test_build_path_access_nonexistent_volume_skipped() { let dir = tempdir().unwrap(); diff --git a/src/boxlite/src/jailer/sandbox/bwrap.rs b/src/boxlite/src/jailer/sandbox/bwrap.rs index ae2f79e27..28d683b04 100644 --- a/src/boxlite/src/jailer/sandbox/bwrap.rs +++ b/src/boxlite/src/jailer/sandbox/bwrap.rs @@ -119,6 +119,23 @@ impl Sandbox for BwrapSandbox { bwrap_cmd.ro_bind(&pa.path, &pa.path); tracing::debug!(path = %pa.path.display(), "bwrap: ro-bind"); } + for mount in &ctx.bind_mounts { + if mount.writable { + bwrap_cmd.bind(&mount.source, &mount.target); + tracing::debug!( + source = %mount.source.display(), + target = %mount.target.display(), + "bwrap: bind user volume" + ); + } else { + bwrap_cmd.ro_bind(&mount.source, &mount.target); + tracing::debug!( + source = %mount.source.display(), + target = %mount.target.display(), + "bwrap: ro-bind user volume" + ); + } + } // ===================================================================== // Environment sanitization @@ -193,6 +210,7 @@ mod tests { let ctx = SandboxContext { id: "test-box", paths: vec![], + bind_mounts: vec![], resource_limits: limits, network_enabled: false, sandbox_profile: None, @@ -237,6 +255,7 @@ mod tests { let ctx = SandboxContext { id: "test-box", paths: vec![], + bind_mounts: vec![], resource_limits: limits, network_enabled: false, sandbox_profile: None, @@ -256,4 +275,54 @@ mod tests { "detached box must not get --die-with-parent or it is killed when run -d returns" ); } + + #[test] + fn apply_emits_namespace_bind_mounts() { + if !bwrap::is_available() { + eprintln!("skipping apply_emits_namespace_bind_mounts: bwrap not available"); + return; + } + + let limits = Box::leak(Box::new(ResourceLimits::default())); + let ctx = SandboxContext { + id: "test-box", + paths: vec![], + bind_mounts: vec![ + crate::jailer::SandboxBindMount::new( + "/host/rw", + "/box/shared/containers/main/volumes/rw", + true, + ), + crate::jailer::SandboxBindMount::new( + "/host/ro", + "/box/shared/containers/main/volumes/ro", + false, + ), + ], + resource_limits: limits, + network_enabled: false, + sandbox_profile: None, + detached: false, + }; + + let mut cmd = Command::new("/var/lib/boxlite/boxes/abc/bin/boxlite-shim"); + BwrapSandbox::new().apply(&ctx, &mut cmd); + let args: Vec = cmd + .get_args() + .map(|a| a.to_string_lossy().into_owned()) + .collect(); + + assert!( + args.windows(3).any(|w| w[0] == "--bind" + && w[1] == "/host/rw" + && w[2] == "/box/shared/containers/main/volumes/rw"), + "writable sandbox bind mount must be emitted" + ); + assert!( + args.windows(3).any(|w| w[0] == "--ro-bind" + && w[1] == "/host/ro" + && w[2] == "/box/shared/containers/main/volumes/ro"), + "read-only sandbox bind mount must be emitted" + ); + } } diff --git a/src/boxlite/src/jailer/sandbox/composite.rs b/src/boxlite/src/jailer/sandbox/composite.rs index fea1f8d20..55b28ab69 100644 --- a/src/boxlite/src/jailer/sandbox/composite.rs +++ b/src/boxlite/src/jailer/sandbox/composite.rs @@ -125,6 +125,7 @@ mod tests { SandboxContext { id: "test", paths: vec![], + bind_mounts: vec![], resource_limits: limits, network_enabled: false, sandbox_profile: None, diff --git a/src/boxlite/src/jailer/sandbox/landlock.rs b/src/boxlite/src/jailer/sandbox/landlock.rs index 7ece77d1d..cf016b8e8 100644 --- a/src/boxlite/src/jailer/sandbox/landlock.rs +++ b/src/boxlite/src/jailer/sandbox/landlock.rs @@ -88,6 +88,7 @@ mod tests { SandboxContext { id: "test", paths: vec![], + bind_mounts: vec![], resource_limits: limits, network_enabled: false, sandbox_profile: None, diff --git a/src/boxlite/src/jailer/sandbox/mod.rs b/src/boxlite/src/jailer/sandbox/mod.rs index 70c82f798..984b5b33a 100644 --- a/src/boxlite/src/jailer/sandbox/mod.rs +++ b/src/boxlite/src/jailer/sandbox/mod.rs @@ -102,6 +102,28 @@ pub struct PathAccess { pub writable: bool, } +/// A bind mount performed inside the sandbox mount namespace. +/// +/// Unlike [`PathAccess`], source and target are intentionally distinct. The +/// bwrap sandbox uses this for user volumes: host source paths are mounted onto +/// the existing box shared tree that libkrun exports to the guest. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SandboxBindMount { + pub source: PathBuf, + pub target: PathBuf, + pub writable: bool, +} + +impl SandboxBindMount { + pub fn new(source: impl Into, target: impl Into, writable: bool) -> Self { + Self { + source: source.into(), + target: target.into(), + writable, + } + } +} + // ============================================================================ // SandboxContext // ============================================================================ @@ -119,6 +141,8 @@ pub struct SandboxContext<'a> { pub id: &'a str, /// Pre-computed filesystem path access rules. pub paths: Vec, + /// Bind mounts to perform inside namespace sandboxes. + pub bind_mounts: Vec, /// Resource limits (for cgroup configuration). pub resource_limits: &'a ResourceLimits, /// Whether network access is enabled. diff --git a/src/boxlite/src/litebox/init/tasks/vmm_spawn.rs b/src/boxlite/src/litebox/init/tasks/vmm_spawn.rs index 2924e164b..5653fb98a 100644 --- a/src/boxlite/src/litebox/init/tasks/vmm_spawn.rs +++ b/src/boxlite/src/litebox/init/tasks/vmm_spawn.rs @@ -7,6 +7,7 @@ use super::guest_entrypoint::GuestEntrypointBuilder; use super::{InitCtx, log_task_error, task_start}; use crate::disk::DiskFormat; use crate::images::ContainerImageConfig; +use crate::jailer::SandboxBindMount; use crate::litebox::init::types::resolve_user_volumes; use crate::net::NetworkBackendConfig; use crate::pipeline::PipelineTask; @@ -26,11 +27,20 @@ use crate::volumes::{ use async_trait::async_trait; use boxlite_shared::Transport; use boxlite_shared::errors::{BoxliteError, BoxliteResult}; +use boxlite_shared::layout::SharedGuestLayout; use std::collections::{HashMap, HashSet}; use std::path::Path; pub struct VmmSpawnTask; +struct BuiltConfig { + instance_spec: InstanceSpec, + volume_mgr: GuestVolumeManager, + rootfs_init: crate::portal::interfaces::ContainerRootfsInitConfig, + container_mounts: Vec, + sandbox_bind_mounts: Vec, +} + #[async_trait] impl PipelineTask for VmmSpawnTask { async fn run(self: Box, ctx: InitCtx) -> BoxliteResult<()> { @@ -77,7 +87,7 @@ impl PipelineTask for VmmSpawnTask { }; // Build config and get outputs - let (instance_spec, volume_mgr, rootfs_init, container_mounts) = build_config( + let built = build_config( &box_id, &options, &layout, @@ -92,17 +102,24 @@ impl PipelineTask for VmmSpawnTask { .inspect_err(|e| log_task_error(&box_id, task_name, e))?; // Spawn VM - let handler = spawn_vm(&box_id, &instance_spec, &options, &layout) - .await - .inspect_err(|e| log_task_error(&box_id, task_name, e))?; + let handler = spawn_vm( + &box_id, + &built.instance_spec, + &options, + &layout, + built.sandbox_bind_mounts, + ) + .await + .inspect_err(|e| log_task_error(&box_id, task_name, e))?; let mut ctx = ctx.lock().await; ctx.guard.set_handler(handler); - ctx.volume_mgr = Some(volume_mgr); - ctx.rootfs_init = Some(rootfs_init); - ctx.container_mounts = Some(container_mounts); + ctx.volume_mgr = Some(built.volume_mgr); + ctx.rootfs_init = Some(built.rootfs_init); + ctx.container_mounts = Some(built.container_mounts); // Store CA cert PEM for Container.Init gRPC (passed as CACert proto field) - ctx.ca_cert_pem = instance_spec + ctx.ca_cert_pem = built + .instance_spec .network_config .as_ref() .and_then(|nc| nc.ca_cert_pem.clone()); @@ -126,21 +143,21 @@ async fn build_config( container_id: &ContainerID, runtime: &SharedRuntimeImpl, reuse_rootfs: bool, -) -> BoxliteResult<( - InstanceSpec, - GuestVolumeManager, - crate::portal::interfaces::ContainerRootfsInitConfig, - Vec, -)> { +) -> BoxliteResult { // Transport setup let transport = Transport::unix(layout.socket_path()); let ready_transport = Transport::unix(layout.ready_socket_path()); let user_volumes = resolve_user_volumes(&options.volumes)?; - // Prepare container directories (image/, rw/, rootfs/) - let container_layout = layout.shared_layout().container(container_id.as_str()); - container_layout.prepare()?; + // Prepare container directories in the host-write side of the shared tree. + // Guest/container mount sources stay on the existing BoxLiteShared path: + // /run/boxlite/shared/containers/{cid}/volumes/{tag}. + let host_shared_layout = SharedGuestLayout::new(layout.mounts_dir()); + let host_container_layout = host_shared_layout.container(container_id.as_str()); + host_container_layout.prepare()?; + let vmm_shared_layout = SharedGuestLayout::new(layout.shared_dir()); + let vmm_container_layout = vmm_shared_layout.container(container_id.as_str()); // Create GuestVolumeManager and configure volumes let mut volume_mgr = GuestVolumeManager::new(); @@ -173,31 +190,80 @@ async fn build_config( need_resize, // Only on fresh start with custom disk size }; + let mut sandbox_bind_mounts = Vec::with_capacity(user_volumes.len()); + #[cfg(target_os = "linux")] + let use_sandbox_volume_binds = + options.advanced.security.jailer_enabled && crate::jailer::is_bwrap_available(); + #[cfg(not(target_os = "linux"))] + let use_sandbox_volume_binds = false; + // Add user volumes via ContainerVolumeManager let mut container_mgr = ContainerVolumeManager::new(&mut volume_mgr); + for vol in &user_volumes { - // Single-file volume: stage the file into a dedicated dir under the box's - // shared tree (already granted to the VMM sandbox) and share that dir, so - // virtio-fs never exposes the file's host siblings. Directories share as-is. - let share_dir = match &vol.subpath { - None => vol.host_path.clone(), - Some(file_name) => { - let staging_dir = layout.shared_dir().join("user-volumes").join(&vol.tag); - stage_single_file(&staging_dir, &vol.host_path, file_name, vol.read_only)?; - staging_dir - } - }; - container_mgr.add_volume( - container_id.as_str(), - &vol.tag, - &vol.tag, - share_dir, - &vol.guest_path, - vol.read_only, - vol.owner_uid, - vol.owner_gid, - vol.subpath.clone(), - ); + if use_sandbox_volume_binds { + let host_volume_dir = host_container_layout.volume_dir(&vol.tag); + let sandbox_volume_dir = vmm_container_layout.volume_dir(&vol.tag); + let (sandbox_target, host_target) = match &vol.subpath { + None => { + prepare_volume_dir(&host_volume_dir)?; + (sandbox_volume_dir, host_volume_dir) + } + Some(file_name) => { + let host_file = host_volume_dir.join(file_name); + prepare_volume_file(&host_file)?; + (sandbox_volume_dir.join(file_name), host_file) + } + }; + sandbox_bind_mounts.push(SandboxBindMount::new( + vol.host_path.clone(), + sandbox_target, + !vol.read_only, + )); + tracing::debug!( + source = %vol.host_path.display(), + target = %host_target.display(), + read_only = vol.read_only, + "Prepared sandbox bind-backed user volume" + ); + container_mgr.add_bind_volume(ContainerMount { + volume_name: vol.tag.clone(), + destination: vol.guest_path.clone(), + read_only: vol.read_only, + owner_uid: vol.owner_uid, + owner_gid: vol.owner_gid, + subpath: vol.subpath.clone(), + }); + continue; + } + + if let Some(file_name) = &vol.subpath { + let staging_dir = host_container_layout.volume_dir(&vol.tag); + stage_single_file(&staging_dir, &vol.host_path, file_name, vol.read_only)?; + container_mgr.add_bind_volume(ContainerMount { + volume_name: vol.tag.clone(), + destination: vol.guest_path.clone(), + read_only: vol.read_only, + owner_uid: vol.owner_uid, + owner_gid: vol.owner_gid, + subpath: vol.subpath.clone(), + }); + } else { + // Directory fallback for macOS and Linux without bwrap: preserve + // existing direct virtio-fs semantics. Linux+jailer+bwrap takes the + // single BoxLiteShared path above and avoids per-volume devices. + container_mgr.add_volume( + container_id.as_str(), + &vol.tag, + &vol.tag, + vol.host_path.clone(), + &vol.guest_path, + vol.read_only, + vol.owner_uid, + vol.owner_gid, + vol.subpath.clone(), + ); + } } let container_mounts = container_mgr.build_container_mounts(); @@ -249,7 +315,42 @@ async fn build_config( detach: options.detach, }; - Ok((instance_spec, volume_mgr, rootfs_init, container_mounts)) + Ok(BuiltConfig { + instance_spec, + volume_mgr, + rootfs_init, + container_mounts, + sandbox_bind_mounts, + }) +} + +fn prepare_volume_dir(path: &Path) -> BoxliteResult<()> { + std::fs::create_dir_all(path).map_err(|e| { + BoxliteError::Storage(format!( + "Failed to create shared volume directory {}: {}", + path.display(), + e + )) + }) +} + +fn prepare_volume_file(path: &Path) -> BoxliteResult<()> { + if let Some(parent) = path.parent() { + prepare_volume_dir(parent)?; + } + std::fs::OpenOptions::new() + .create(true) + .write(true) + .truncate(true) + .open(path) + .map(|_| ()) + .map_err(|e| { + BoxliteError::Storage(format!( + "Failed to create shared volume file {}: {}", + path.display(), + e + )) + }) } /// Configure guest rootfs with device path from volume manager. @@ -389,6 +490,7 @@ async fn spawn_vm( config: &InstanceSpec, options: &BoxOptions, layout: &BoxFilesystemLayout, + bind_mounts: Vec, ) -> BoxliteResult> { let mut controller = ShimController::new( find_binary("boxlite-shim")?, @@ -396,6 +498,7 @@ async fn spawn_vm( box_id.clone(), options.clone(), layout.clone(), + bind_mounts, )?; controller.start(config).await diff --git a/src/boxlite/src/vmm/controller/shim.rs b/src/boxlite/src/vmm/controller/shim.rs index 192013ec8..dabe329f6 100644 --- a/src/boxlite/src/vmm/controller/shim.rs +++ b/src/boxlite/src/vmm/controller/shim.rs @@ -4,6 +4,7 @@ use std::{path::PathBuf, process::Child, sync::Mutex, time::Instant}; use crate::{ BoxID, + jailer::SandboxBindMount, runtime::layout::BoxFilesystemLayout, vmm::{InstanceSpec, VmmKind}, }; @@ -257,6 +258,8 @@ pub struct ShimController { options: crate::runtime::options::BoxOptions, /// Box filesystem layout (provides paths for stderr, sockets, etc.) layout: BoxFilesystemLayout, + /// Bind mounts installed by the jailer namespace before the shim starts. + bind_mounts: Vec, } impl ShimController { @@ -278,6 +281,7 @@ impl ShimController { box_id: BoxID, options: crate::runtime::options::BoxOptions, layout: BoxFilesystemLayout, + bind_mounts: Vec, ) -> BoxliteResult { // Verify that the shim binary exists if !binary_path.exists() { @@ -293,6 +297,7 @@ impl ShimController { box_id, options, layout, + bind_mounts, }) } } @@ -370,6 +375,7 @@ impl VmmController for ShimController { &self.layout, self.box_id.as_str(), &self.options, + &self.bind_mounts, ); let spawned = spawner.spawn(&config_json, config.detach)?; // spawn_duration: time to create Box subprocess diff --git a/src/boxlite/src/vmm/controller/spawn.rs b/src/boxlite/src/vmm/controller/spawn.rs index a4da35773..f67907539 100644 --- a/src/boxlite/src/vmm/controller/spawn.rs +++ b/src/boxlite/src/vmm/controller/spawn.rs @@ -5,7 +5,7 @@ use std::{ process::{Child, Stdio}, }; -use crate::jailer::{Jail, JailerBuilder}; +use crate::jailer::{Jail, JailerBuilder, SandboxBindMount}; use crate::runtime::layout::BoxFilesystemLayout; use crate::runtime::options::BoxOptions; use crate::util::configure_library_env; @@ -38,6 +38,7 @@ pub struct ShimSpawner<'a> { layout: &'a BoxFilesystemLayout, box_id: &'a str, options: &'a BoxOptions, + bind_mounts: &'a [SandboxBindMount], } impl<'a> ShimSpawner<'a> { @@ -46,12 +47,14 @@ impl<'a> ShimSpawner<'a> { layout: &'a BoxFilesystemLayout, box_id: &'a str, options: &'a BoxOptions, + bind_mounts: &'a [SandboxBindMount], ) -> Self { Self { binary_path, layout, box_id, options, + bind_mounts, } } @@ -79,6 +82,7 @@ impl<'a> ShimSpawner<'a> { .with_layout(self.layout.clone()) .with_security(self.options.advanced.security.clone()) .with_volumes(self.options.volumes.clone()) + .with_bind_mounts(self.bind_mounts.to_vec()) .with_detach(detach); if let Some(ref setup) = child_setup { @@ -203,6 +207,7 @@ mod tests { &layout, "test-box", &options, + &[], ); // No CLI args — config is sent via stdin pipe @@ -237,6 +242,7 @@ mod tests { &layout, "test-box", &options, + &[], ); let mut cmd = std::process::Command::new("/usr/bin/true"); @@ -291,6 +297,7 @@ mod tests { &layout, "test-box", &options, + &[], ); let mut cmd = std::process::Command::new("/usr/bin/true"); @@ -333,6 +340,7 @@ mod tests { &layout, "shimspawnertest", &options, + &[], ); let spawned = spawner.spawn("", true).expect("spawn detached"); @@ -387,6 +395,7 @@ mod tests { &layout, "shimspawnertest", &options, + &[], ); let spawned = spawner.spawn("", false).expect("spawn non-detached"); diff --git a/src/boxlite/src/volumes/container_volume.rs b/src/boxlite/src/volumes/container_volume.rs index 55dd26f3d..bc365748e 100644 --- a/src/boxlite/src/volumes/container_volume.rs +++ b/src/boxlite/src/volumes/container_volume.rs @@ -37,6 +37,7 @@ pub struct ContainerMount { /// Holds a reference to GuestVolumeManager and tracks bind mounts /// from guest VM paths into container namespace. pub struct ContainerVolumeManager<'a> { + #[cfg_attr(target_os = "linux", allow(dead_code))] guest: &'a mut GuestVolumeManager, container_mounts: Vec, } @@ -69,6 +70,7 @@ impl<'a> ContainerVolumeManager<'a> { /// * `container_path` - Mount point in container (user-specified) /// * `read_only` - Whether the mount is read-only #[allow(clippy::too_many_arguments)] + #[cfg_attr(target_os = "linux", allow(dead_code))] pub fn add_volume( &mut self, container_id: &str, @@ -105,16 +107,9 @@ impl<'a> ContainerVolumeManager<'a> { /// Add a container bind mount directly. /// /// Use when guest path already exists (e.g., from block device mount). - #[allow(dead_code)] - pub fn add_bind(&mut self, volume_name: &str, container_path: &str, read_only: bool) { - self.container_mounts.push(ContainerMount { - volume_name: volume_name.to_string(), - destination: container_path.to_string(), - read_only, - owner_uid: 0, - owner_gid: 0, - subpath: None, - }); + #[cfg_attr(not(target_os = "linux"), allow(dead_code))] + pub fn add_bind_volume(&mut self, mount: ContainerMount) { + self.container_mounts.push(mount); } /// Build container mount configuration. @@ -147,4 +142,34 @@ mod tests { assert_eq!(mounts.len(), 1); assert_eq!(mounts[0].subpath, Some("app.conf".to_string())); } + + #[test] + fn add_bind_volume_does_not_create_guest_virtiofs_share() { + let mut guest = GuestVolumeManager::new(); + let mut mgr = ContainerVolumeManager::new(&mut guest); + mgr.add_bind_volume(ContainerMount { + volume_name: "uservol0".to_string(), + destination: "/data".to_string(), + read_only: false, + owner_uid: 1000, + owner_gid: 1000, + subpath: Some("app.conf".to_string()), + }); + + let mounts = mgr.build_container_mounts(); + assert_eq!(mounts.len(), 1); + assert_eq!(mounts[0].volume_name, "uservol0"); + assert_eq!(mounts[0].subpath, Some("app.conf".to_string())); + + drop(mgr); + let vmm_config = guest.build_vmm_config(); + assert!( + vmm_config.fs_shares.shares().is_empty(), + "container mounts should not create per-volume virtiofs shares" + ); + assert!( + guest.build_guest_mounts().is_empty(), + "bind-backed user volumes should not require guest virtiofs mounts" + ); + } } diff --git a/src/guest/src/container/spec.rs b/src/guest/src/container/spec.rs index 3faad27d9..c1fc92bbc 100644 --- a/src/guest/src/container/spec.rs +++ b/src/guest/src/container/spec.rs @@ -62,9 +62,9 @@ pub fn create_oci_spec( // Add user-specified bind mounts for user_mount in user_mounts { let options = if user_mount.read_only { - vec!["bind".to_string(), "ro".to_string()] + vec!["rbind".to_string(), "ro".to_string()] } else { - vec!["bind".to_string(), "rw".to_string()] + vec!["rbind".to_string(), "rw".to_string()] }; mounts.push( @@ -584,6 +584,41 @@ mod tests { use super::*; use std::fs; + #[test] + fn create_oci_spec_uses_recursive_bind_for_user_mounts() { + let bundle = tempfile::tempdir().unwrap(); + let spec = create_oci_spec( + "test-container", + "/rootfs", + &["/bin/sh".to_string()], + &[], + "/", + 0, + 0, + bundle.path(), + &[UserMount { + source: "/run/boxlite/shared/containers/test/volumes/uservol0".to_string(), + destination: "/data".to_string(), + read_only: false, + owner_uid: 0, + owner_gid: 0, + }], + ) + .unwrap(); + + let mounts = spec.mounts().as_ref().expect("spec should include mounts"); + let user_mount = mounts + .iter() + .find(|mount| mount.destination() == Path::new("/data")) + .expect("user mount should be present"); + + assert_eq!(user_mount.typ().as_deref(), Some("bind")); + assert_eq!( + user_mount.options().as_ref().map(Vec::as_slice), + Some(["rbind".to_string(), "rw".to_string()].as_slice()) + ); + } + /// Create a temp rootfs with /etc/passwd and /etc/group for testing. /// /// Covers: root, regular users, system users (www-data, nobody), diff --git a/src/guest/src/service/container.rs b/src/guest/src/service/container.rs index a07869771..953e94aa6 100644 --- a/src/guest/src/service/container.rs +++ b/src/guest/src/service/container.rs @@ -208,8 +208,9 @@ impl ContainerService for GuestServer { } } - // Convert proto BindMount to UserMount for OCI spec - // Construct full source path from convention: /run/boxlite/shared/containers/{id}/volumes/{name} + // Convert proto BindMount to UserMount for OCI spec. + // Construct full source path from convention: + // /run/boxlite/shared/containers/{id}/volumes/{name} let guest_layout = boxlite_shared::layout::SharedGuestLayout::new("/run/boxlite/shared"); let container_layout = guest_layout.container(&container_id); diff --git a/src/shared/proto/boxlite/v1/service.proto b/src/shared/proto/boxlite/v1/service.proto index d3b1b0dae..e9ee1d979 100644 --- a/src/shared/proto/boxlite/v1/service.proto +++ b/src/shared/proto/boxlite/v1/service.proto @@ -215,7 +215,7 @@ message CACert { string pem = 1; // PEM-encoded X.509 certificate } -// Bind mount from guest volume to container path +// Bind mount from guest volume to container path. // Uses convention-based paths: /run/boxlite/shared/containers/{container_id}/volumes/{volume_name} message BindMount { // Volume name (used with container_id to construct guest path)