From f342cff2eb0d347d8d0f60c7ad4d99e7b1c04b89 Mon Sep 17 00:00:00 2001 From: Fabien Penso Date: Wed, 19 Aug 2026 00:06:50 -0400 Subject: [PATCH 1/3] fix(sandbox): apply Apple Container resource limits Apple Container sandbox creation ignored configured memory, CPU, and process limits, leaving workloads subject to backend defaults and causing fork failures under low PID ceilings. Pass memory and whole-CPU allocations through the native run flags and map pids_max to Apple Container's nproc ulimit syntax. Reject fractional CPU quotas explicitly because Apple Container cannot represent them without weakening the requested limit. --- crates/tools/src/sandbox/apple.rs | 13 +++-- crates/tools/src/sandbox/containers.rs | 26 ++++++++- crates/tools/src/sandbox/tests/apple.rs | 78 +++++++++++++++++++++++-- docs/src/sandbox.md | 6 +- 4 files changed, 111 insertions(+), 12 deletions(-) diff --git a/crates/tools/src/sandbox/apple.rs b/crates/tools/src/sandbox/apple.rs index b5889488e..cd08f82be 100644 --- a/crates/tools/src/sandbox/apple.rs +++ b/crates/tools/src/sandbox/apple.rs @@ -33,8 +33,9 @@ use super::paths::{ }; #[cfg(target_os = "macos")] use super::types::{ - BuildImageResult, DEFAULT_SANDBOX_IMAGE, ManagedFilesMount, NetworkPolicy, SANDBOX_FILES_DIR, - SANDBOX_HOME_DIR, Sandbox, SandboxConfig, SandboxId, truncate_output_for_display, + BuildImageResult, DEFAULT_SANDBOX_IMAGE, ManagedFilesMount, NetworkPolicy, ResourceLimits, + SANDBOX_FILES_DIR, SANDBOX_HOME_DIR, Sandbox, SandboxConfig, SandboxId, + truncate_output_for_display, }; #[cfg(target_os = "macos")] use crate::error::{Error, Result}; @@ -466,8 +467,10 @@ impl AppleContainerSandbox { image: &str, tz: Option<&str>, volumes: &[String], + resource_limits: &ResourceLimits, ) -> std::result::Result<(), CreateError> { - let args = apple_container_run_args(name, image, tz, volumes); + let args = apple_container_run_args(name, image, tz, volumes, resource_limits) + .map_err(|error| CreateError::Other(error.to_string()))?; let output = tokio::process::Command::new("container") .args(&args) @@ -865,7 +868,9 @@ impl Sandbox for AppleContainerSandbox { // Phase 2: Create a new container. info!(name, image = %image, attempt, "creating apple container"); - match Self::run_container(&name, &image, tz, &volumes).await { + match Self::run_container(&name, &image, tz, &volumes, &self.config.resource_limits) + .await + { Ok(()) => {}, Err(CreateError::AlreadyExists) => { warn!( diff --git a/crates/tools/src/sandbox/containers.rs b/crates/tools/src/sandbox/containers.rs index dfbda5d84..373763081 100644 --- a/crates/tools/src/sandbox/containers.rs +++ b/crates/tools/src/sandbox/containers.rs @@ -10,6 +10,8 @@ use { #[cfg(any(target_os = "macos", test))] use super::types::APPLE_CONTAINER_SAFE_WORKDIR; +#[cfg(target_os = "macos")] +use super::types::ResourceLimits; use { super::types::{ GO_TOOL_INSTALLS, GOGCLI_MODULE_PATH, GOGCLI_VERSION, SANDBOX_HOME_DIR, @@ -119,7 +121,8 @@ pub(crate) fn apple_container_run_args( image: &str, tz: Option<&str>, volumes: &[String], -) -> Vec { + resource_limits: &ResourceLimits, +) -> Result> { let mut args = vec![ "run".to_string(), "-d".to_string(), @@ -132,6 +135,25 @@ pub(crate) fn apple_container_run_args( if let Some(tz) = tz { args.extend(["-e".to_string(), format!("TZ={tz}")]); } + if let Some(ref memory) = resource_limits.memory_limit { + args.extend(["--memory".to_string(), memory.clone()]); + } + if let Some(cpus) = resource_limits.cpu_quota { + let cpus = cpus.to_string().parse::().map_err(|_| { + Error::message(format!( + "Apple Container requires cpu_quota to be a positive whole number, got {cpus}" + )) + })?; + if cpus < 1 { + return Err(Error::message(format!( + "Apple Container requires cpu_quota to be a positive whole number, got {cpus}" + ))); + } + args.extend(["--cpus".to_string(), cpus.to_string()]); + } + if let Some(pids) = resource_limits.pids_max { + args.extend(["--ulimit".to_string(), format!("nproc={pids}")]); + } for volume in volumes { args.extend(["--volume".to_string(), volume.to_string()]); } @@ -142,7 +164,7 @@ pub(crate) fn apple_container_run_args( "-c".to_string(), apple_container_bootstrap_command(), ]); - args + Ok(args) } #[cfg(any(target_os = "macos", test))] diff --git a/crates/tools/src/sandbox/tests/apple.rs b/crates/tools/src/sandbox/tests/apple.rs index f830c99d1..b0d33dad7 100644 --- a/crates/tools/src/sandbox/tests/apple.rs +++ b/crates/tools/src/sandbox/tests/apple.rs @@ -185,7 +185,14 @@ fn test_apple_container_bootstrap_command_uses_portable_sleep() { #[test] fn test_apple_container_run_args_pin_workdir_and_bootstrap_home() { - let args = apple_container_run_args("moltis-sandbox-test", "ubuntu:25.10", Some("UTC"), &[]); + let args = apple_container_run_args( + "moltis-sandbox-test", + "ubuntu:25.10", + Some("UTC"), + &[], + &ResourceLimits::default(), + ) + .unwrap(); let expected = vec![ "run", "-d", @@ -208,9 +215,14 @@ fn test_apple_container_run_args_pin_workdir_and_bootstrap_home() { #[test] fn test_apple_container_run_args_with_home_volume() { - let args = apple_container_run_args("moltis-sandbox-test", "ubuntu:25.10", Some("UTC"), &[ - "/tmp/home:/home/sandbox".to_string(), - ]); + let args = apple_container_run_args( + "moltis-sandbox-test", + "ubuntu:25.10", + Some("UTC"), + &["/tmp/home:/home/sandbox".to_string()], + &ResourceLimits::default(), + ) + .unwrap(); let expected = vec![ "run", "-d", @@ -239,7 +251,14 @@ fn test_apple_container_run_args_with_multiple_volumes() { "/tmp/home:/home/sandbox".to_string(), "/tmp/files:/home/sandbox/files:ro".to_string(), ]; - let args = apple_container_run_args("moltis-sandbox-test", "ubuntu:25.10", None, &volumes); + let args = apple_container_run_args( + "moltis-sandbox-test", + "ubuntu:25.10", + None, + &volumes, + &ResourceLimits::default(), + ) + .unwrap(); assert_eq!( args.windows(2) @@ -250,6 +269,55 @@ fn test_apple_container_run_args_with_multiple_volumes() { ); } +#[test] +fn test_apple_container_run_args_apply_resource_limits() { + let args = apple_container_run_args( + "moltis-sandbox-test", + "ubuntu:25.10", + None, + &[], + &ResourceLimits { + memory_limit: Some("1G".into()), + cpu_quota: Some(2.0), + pids_max: Some(512), + }, + ) + .unwrap(); + + assert_eq!( + args.windows(2) + .filter(|window| matches!(window[0].as_str(), "--memory" | "--cpus" | "--ulimit")) + .map(|window| (window[0].as_str(), window[1].as_str())) + .collect::>(), + vec![ + ("--memory", "1G"), + ("--cpus", "2"), + ("--ulimit", "nproc=512") + ] + ); +} + +#[test] +fn test_apple_container_run_args_reject_fractional_cpu_quota() { + let error = apple_container_run_args( + "moltis-sandbox-test", + "ubuntu:25.10", + None, + &[], + &ResourceLimits { + cpu_quota: Some(0.5), + ..Default::default() + }, + ) + .unwrap_err(); + + assert!( + error + .to_string() + .contains("cpu_quota to be a positive whole number") + ); +} + #[cfg(target_os = "macos")] #[test] fn test_apple_container_managed_files_mount_coexists_with_home_persistence() { diff --git a/docs/src/sandbox.md b/docs/src/sandbox.md index 6176b03cb..0b3ac7b59 100644 --- a/docs/src/sandbox.md +++ b/docs/src/sandbox.md @@ -448,7 +448,11 @@ How resource limits are applied depends on the backend: |-------|--------|-----------------|------|-----------------|----------------| | `memory_limit` | `--memory` | `--memory` | Wasmtime reservation | `ulimit -v` | `MemoryMax=` | | `cpu_quota` | `--cpus` | `--cpus` | epoch timeout | `ulimit -t` (seconds) | `CPUQuota=` | -| `pids_max` | `--pids-limit` | `--pids-limit` | n/a | `ulimit -u` | `TasksMax=` | +| `pids_max` | `--pids-limit` | `--ulimit nproc=` | n/a | `ulimit -u` | `TasksMax=` | + +Apple Container requires version 0.9 or newer to apply `pids_max`. +Its `--cpus` option only accepts positive whole CPU counts, so fractional +`cpu_quota` values are not supported by that backend. ## Comparison From 83994843c66ff4e3c899c1b8ba510db40778435b Mon Sep 17 00:00:00 2001 From: Fabien Penso Date: Wed, 19 Aug 2026 00:20:37 -0400 Subject: [PATCH 2/3] fix(sandbox): recreate containers when policy changes Apple Container reuse could return before validating the configured limits or confirming that the running VM was created with the current policy. Validate resource settings before inspection and associate each reusable container with a fingerprint covering mount and resource policy so changed limits force recreation. --- crates/tools/src/sandbox/apple.rs | 27 ++++++++++++++------- crates/tools/src/sandbox/containers.rs | 31 +++++++++++++++++-------- crates/tools/src/sandbox/tests/apple.rs | 24 +++++++++++++++++++ 3 files changed, 64 insertions(+), 18 deletions(-) diff --git a/crates/tools/src/sandbox/apple.rs b/crates/tools/src/sandbox/apple.rs index 5a6ac79dc..00bdb5499 100644 --- a/crates/tools/src/sandbox/apple.rs +++ b/crates/tools/src/sandbox/apple.rs @@ -1,7 +1,7 @@ //! Apple Container sandbox backend (macOS 26+, Apple Silicon). #[cfg(target_os = "macos")] -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; #[cfg(target_os = "macos")] use std::process::Command; #[cfg(target_os = "macos")] @@ -22,6 +22,7 @@ use super::containers::{ apple_container_run_state_from_inspect, is_apple_container_daemon_stale_error, is_apple_container_exists_error, is_apple_container_service_error, rebuildable_sandbox_image_tag, sandbox_image_exists, unmark_zombie, + validate_apple_container_resource_limits, }; #[cfg(target_os = "macos")] use super::host::provision_packages; @@ -53,7 +54,7 @@ use crate::sandbox::file_system::{ pub struct AppleContainerSandbox { pub config: SandboxConfig, name_generations: RwLock>, - mount_policy_validated: Mutex>, + container_policy_fingerprints: Mutex>, /// Cached host gateway IP for proxy routing in Trusted mode. host_gateway_cache: RwLock>, } @@ -64,7 +65,7 @@ impl AppleContainerSandbox { Self { config, name_generations: RwLock::new(HashMap::new()), - mount_policy_validated: Mutex::new(HashSet::new()), + container_policy_fingerprints: Mutex::new(HashMap::new()), host_gateway_cache: RwLock::new(None), } } @@ -127,6 +128,13 @@ impl AppleContainerSandbox { .unwrap_or("moltis-sandbox") } + pub(crate) fn container_policy_fingerprint(&self) -> String { + format!( + "{:?}\0{:?}", + self.config.managed_files_mount, self.config.resource_limits + ) + } + fn base_container_name(&self, id: &SandboxId) -> String { format!("{}-{}", self.container_prefix(), id.key) } @@ -782,25 +790,28 @@ impl Sandbox for AppleContainerSandbox { } async fn ensure_ready(&self, id: &SandboxId, image_override: Option<&str>) -> Result<()> { + validate_apple_container_resource_limits(&self.config.resource_limits)?; + let mut name = self.container_name(id).await; // This state lock also serializes Apple Container creation. The CLI has // no atomic create-or-inspect primitive, so releasing it after policy // validation would let a concurrent caller remove the fresh winner. - let mut validated = self.mount_policy_validated.lock().await; - if !validated.contains(&name) { + let mut fingerprints = self.container_policy_fingerprints.lock().await; + let desired_fingerprint = self.container_policy_fingerprint(); + if fingerprints.get(&name) != Some(&desired_fingerprint) { if Self::container_exists(&name).await? { warn!( name, - "recreating existing apple container to apply managed Files mount policy" + "recreating existing apple container to apply sandbox policy" ); Self::force_remove_and_wait(&name).await; if Self::container_exists(&name).await? { return Err(Error::message(format!( - "failed to remove apple container '{name}' after managed Files mount policy changed" + "failed to remove apple container '{name}' after sandbox policy changed" ))); } } - validated.insert(name.clone()); + fingerprints.insert(name.clone(), desired_fingerprint); } let requested_image = image_override.unwrap_or_else(|| self.image()); let image = self.resolve_local_image(requested_image).await?; diff --git a/crates/tools/src/sandbox/containers.rs b/crates/tools/src/sandbox/containers.rs index 700fb9315..959f53ed9 100644 --- a/crates/tools/src/sandbox/containers.rs +++ b/crates/tools/src/sandbox/containers.rs @@ -123,6 +123,8 @@ pub(crate) fn apple_container_run_args( volumes: &[String], resource_limits: &ResourceLimits, ) -> Result> { + validate_apple_container_resource_limits(resource_limits)?; + let mut args = vec![ "run".to_string(), "-d".to_string(), @@ -139,16 +141,6 @@ pub(crate) fn apple_container_run_args( args.extend(["--memory".to_string(), memory.clone()]); } if let Some(cpus) = resource_limits.cpu_quota { - let cpus = cpus.to_string().parse::().map_err(|_| { - Error::message(format!( - "Apple Container requires cpu_quota to be a positive whole number, got {cpus}" - )) - })?; - if cpus < 1 { - return Err(Error::message(format!( - "Apple Container requires cpu_quota to be a positive whole number, got {cpus}" - ))); - } args.extend(["--cpus".to_string(), cpus.to_string()]); } if let Some(pids) = resource_limits.pids_max { @@ -167,6 +159,25 @@ pub(crate) fn apple_container_run_args( Ok(args) } +#[cfg(target_os = "macos")] +pub(crate) fn validate_apple_container_resource_limits( + resource_limits: &ResourceLimits, +) -> Result<()> { + let Some(cpus) = resource_limits.cpu_quota else { + return Ok(()); + }; + let valid = cpus.is_finite() + && cpus >= 1.0 + && cpus.fract() == 0.0 + && cpus.to_string().parse::().is_ok(); + if !valid { + return Err(Error::message(format!( + "Apple Container requires cpu_quota to be a positive whole number, got {cpus}" + ))); + } + Ok(()) +} + #[cfg(any(target_os = "macos", test))] pub(crate) fn apple_container_exec_args(name: &str, shell_command: String) -> Vec { vec![ diff --git a/crates/tools/src/sandbox/tests/apple.rs b/crates/tools/src/sandbox/tests/apple.rs index 170206a89..61ee5711e 100644 --- a/crates/tools/src/sandbox/tests/apple.rs +++ b/crates/tools/src/sandbox/tests/apple.rs @@ -318,6 +318,30 @@ fn test_apple_container_run_args_reject_fractional_cpu_quota() { ); } +#[cfg(target_os = "macos")] +#[test] +fn test_apple_container_policy_fingerprint_includes_resource_limits() { + let first = AppleContainerSandbox::new(SandboxConfig { + resource_limits: ResourceLimits { + pids_max: Some(256), + ..Default::default() + }, + ..Default::default() + }); + let second = AppleContainerSandbox::new(SandboxConfig { + resource_limits: ResourceLimits { + pids_max: Some(512), + ..Default::default() + }, + ..Default::default() + }); + + assert_ne!( + first.container_policy_fingerprint(), + second.container_policy_fingerprint() + ); +} + #[cfg(target_os = "macos")] #[test] fn test_apple_container_managed_files_mount_coexists_with_home_persistence() { From 2cb77aac6717c1e77d914d47d9238be11086f710 Mon Sep 17 00:00:00 2001 From: Fabien Penso Date: Wed, 19 Aug 2026 00:27:37 -0400 Subject: [PATCH 3/3] fix(sandbox): retain policy after name rotation Record the resource-policy fingerprint only after the current Apple container name is ready. This keeps AlreadyExists generation rotation associated with the successfully created container and avoids destructive recreation on the next readiness check. --- crates/tools/src/sandbox/apple.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/tools/src/sandbox/apple.rs b/crates/tools/src/sandbox/apple.rs index 00bdb5499..e6eb4a1da 100644 --- a/crates/tools/src/sandbox/apple.rs +++ b/crates/tools/src/sandbox/apple.rs @@ -798,20 +798,19 @@ impl Sandbox for AppleContainerSandbox { // validation would let a concurrent caller remove the fresh winner. let mut fingerprints = self.container_policy_fingerprints.lock().await; let desired_fingerprint = self.container_policy_fingerprint(); - if fingerprints.get(&name) != Some(&desired_fingerprint) { + if fingerprints.get(&name) != Some(&desired_fingerprint) + && Self::container_exists(&name).await? + { + warn!( + name, + "recreating existing apple container to apply sandbox policy" + ); + Self::force_remove_and_wait(&name).await; if Self::container_exists(&name).await? { - warn!( - name, - "recreating existing apple container to apply sandbox policy" - ); - Self::force_remove_and_wait(&name).await; - if Self::container_exists(&name).await? { - return Err(Error::message(format!( - "failed to remove apple container '{name}' after sandbox policy changed" - ))); - } + return Err(Error::message(format!( + "failed to remove apple container '{name}' after sandbox policy changed" + ))); } - fingerprints.insert(name.clone(), desired_fingerprint); } let requested_image = image_override.unwrap_or_else(|| self.image()); let image = self.resolve_local_image(requested_image).await?; @@ -830,6 +829,7 @@ impl Sandbox for AppleContainerSandbox { info!(name, "apple container already running"); match Self::wait_for_container_exec_ready(&name).await { Ok(()) => { + fingerprints.insert(name.clone(), desired_fingerprint.clone()); unmark_zombie(&name); return Ok(()); }, @@ -850,6 +850,7 @@ impl Sandbox for AppleContainerSandbox { info!(name, "apple container restarted"); match Self::wait_for_container_exec_ready(&name).await { Ok(()) => { + fingerprints.insert(name.clone(), desired_fingerprint.clone()); unmark_zombie(&name); return Ok(()); }, @@ -945,6 +946,7 @@ impl Sandbox for AppleContainerSandbox { provision_packages("container", &name, &self.config.packages).await?; } + fingerprints.insert(name.clone(), desired_fingerprint.clone()); return Ok(()); }, Err(error) => { @@ -976,6 +978,7 @@ impl Sandbox for AppleContainerSandbox { ) .await?; } + fingerprints.insert(name.clone(), desired_fingerprint.clone()); return Ok(()); }, Err(restart_error) => {