From 5abb3dd27e5ae2867f520b9fa616b3ffbfc8ec8d Mon Sep 17 00:00:00 2001 From: gamnaansong Date: Thu, 4 Jun 2026 05:18:29 +0000 Subject: [PATCH 1/3] feat(jailer): expose the rest of SecurityOptions via JailerBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `JailerBuilder` previously exposed only `with_jailer_enabled` and `with_seccomp_enabled` even though `SecurityOptions` carries another dozen knobs (UID/GID drop, PID/net namespaces, chroot base + toggle, FD cleanup, env sanitization + allowlist, rlimits, macOS sandbox profile + network). Callers wanting any of those had to construct a `SecurityOptions` separately and pipe it through `with_security(...)`, which scattered build sites and skipped past any future invariants the builder might add. Mirror `SecurityOptionsBuilder`'s API in JailerBuilder's consuming-self style: - with_uid(Option) / with_gid(Option) - with_new_pid_ns(bool) / with_new_net_ns(bool) - with_chroot_base(impl Into) / with_chroot_enabled(bool) - with_close_fds(bool) / with_sanitize_env(bool) - with_env_allowlist(Vec) / with_allowed_env(impl Into) - with_resource_limits(ResourceLimits) - with_max_open_files / _file_size_bytes / _processes / _memory_bytes / _cpu_time_seconds - with_sandbox_profile(Option) / with_network_enabled(bool) - with_development_security() / with_standard_security() / with_maximum_security() — preset shortcuts Each setter delegates to the matching `SecurityOptions` field with no new defaults; the initial value still comes from `SecurityOptions::default()`. `with_allowed_env` is idempotent (duplicates ignored). ## Tests 8 new tests under `jailer::builder::tests`, one per cluster of new methods. Each builds a real `Jailer` and asserts the field landed, so a regression that drops any setter shows up as a single failing assertion rather than a compile error elsewhere. - `builder_exposes_uid_gid_passthroughs` - `builder_exposes_namespace_toggles` - `builder_exposes_chroot_settings` - `builder_exposes_env_and_fd_hygiene` (also pins `with_allowed_env` dedup behaviour) - `builder_exposes_resource_limits` (per-field setters) - `builder_exposes_resource_limits_bulk_set` - `builder_exposes_macos_sandbox_knobs` - `builder_preset_shortcuts_pick_known_profiles` (confirms presets wholesale-replace, so `with_uid(0)` before `with_standard_security()` does NOT win — documents intent) Full `jailer::builder` suite: 15/15 green. Clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/boxlite/src/jailer/builder.rs | 336 +++++++++++++++++++++++++++++- 1 file changed, 335 insertions(+), 1 deletion(-) diff --git a/src/boxlite/src/jailer/builder.rs b/src/boxlite/src/jailer/builder.rs index d6cd82ac5..069083142 100644 --- a/src/boxlite/src/jailer/builder.rs +++ b/src/boxlite/src/jailer/builder.rs @@ -2,10 +2,11 @@ use super::Jailer; use super::sandbox::{PlatformSandbox, Sandbox}; -use crate::runtime::advanced_options::SecurityOptions; +use crate::runtime::advanced_options::{ResourceLimits, SecurityOptions}; use crate::runtime::layout::BoxFilesystemLayout; use crate::runtime::options::VolumeSpec; use std::os::fd::RawFd; +use std::path::PathBuf; /// Builder for constructing a [`Jailer`]. /// @@ -95,6 +96,181 @@ impl JailerBuilder { self } + // ======================================================================== + // SECURITY FIELDS — fluent passthroughs for the rest of `SecurityOptions`. + // + // Until now `JailerBuilder` only exposed `jailer_enabled` / + // `seccomp_enabled` even though `SecurityOptions` carries another + // dozen knobs (UID/GID drop, namespaces, chroot, FD cleanup, env + // allowlist, rlimits, macOS sandbox). Callers wanting any of those + // had to construct a `SecurityOptions` separately and pipe it + // through `with_security(...)`, which scattered build sites and + // skipped past any future invariants the builder might enforce. + // + // Each setter below maps 1-to-1 to the matching `SecurityOptions` + // field (same name minus the `with_` prefix). Naming and signature + // mirror `SecurityOptionsBuilder` (`runtime/advanced_options.rs`) + // except for `&mut self` → `mut self` to match `JailerBuilder`'s + // consuming chain style. No defaults are introduced here — the + // initial value still comes from `SecurityOptions::default()`. + // ======================================================================== + + /// Set the UID to drop to after setup (Linux only). + /// + /// - `Some(0)` keeps root (not recommended). + /// - `Some(uid)` drops to that UID. + /// - `None` (the default) leaves the auto-allocate behaviour in place. + pub fn with_uid(mut self, uid: Option) -> Self { + self.security.uid = uid; + self + } + + /// Set the GID to drop to after setup (Linux only). Same semantics + /// as [`with_uid`](Self::with_uid). + pub fn with_gid(mut self, gid: Option) -> Self { + self.security.gid = gid; + self + } + + /// Enable / disable a new PID namespace (Linux only). When true, + /// the shim becomes PID 1 inside the namespace. + pub fn with_new_pid_ns(mut self, enabled: bool) -> Self { + self.security.new_pid_ns = enabled; + self + } + + /// Enable / disable a new network namespace (Linux only). gvproxy + /// already handles networking, so this is normally off — flip it + /// on for fully isolated traffic. + pub fn with_new_net_ns(mut self, enabled: bool) -> Self { + self.security.new_net_ns = enabled; + self + } + + /// Set the base directory for chroot jails (Linux only). Default + /// `/srv/boxlite`. + pub fn with_chroot_base(mut self, path: impl Into) -> Self { + self.security.chroot_base = path.into(); + self + } + + /// Enable / disable chroot via `pivot_root` (Linux only). Default + /// `true` on Linux. + pub fn with_chroot_enabled(mut self, enabled: bool) -> Self { + self.security.chroot_enabled = enabled; + self + } + + /// Close inherited file descriptors (keeps stdin/stdout/stderr). + /// Default `true`. + pub fn with_close_fds(mut self, enabled: bool) -> Self { + self.security.close_fds = enabled; + self + } + + /// Sanitize environment variables (keeps only `env_allowlist`). + /// Default `true`. + pub fn with_sanitize_env(mut self, enabled: bool) -> Self { + self.security.sanitize_env = enabled; + self + } + + /// Replace the env-var allowlist wholesale. To add a single entry + /// without throwing away the default, use + /// [`with_allowed_env`](Self::with_allowed_env). + pub fn with_env_allowlist(mut self, vars: Vec) -> Self { + self.security.env_allowlist = vars; + self + } + + /// Append a single name to the env-var allowlist (idempotent — a + /// duplicate is ignored). The default list keeps `RUST_LOG`, `PATH`, + /// `HOME`, `USER`, `LANG`. + pub fn with_allowed_env(mut self, var: impl Into) -> Self { + let var = var.into(); + if !self.security.env_allowlist.contains(&var) { + self.security.env_allowlist.push(var); + } + self + } + + /// Replace all resource limits at once. + pub fn with_resource_limits(mut self, limits: ResourceLimits) -> Self { + self.security.resource_limits = limits; + self + } + + /// `RLIMIT_NOFILE` — maximum open file descriptors. + pub fn with_max_open_files(mut self, limit: u64) -> Self { + self.security.resource_limits.max_open_files = Some(limit); + self + } + + /// `RLIMIT_FSIZE` — maximum file size, in bytes. + pub fn with_max_file_size_bytes(mut self, bytes: u64) -> Self { + self.security.resource_limits.max_file_size = Some(bytes); + self + } + + /// `RLIMIT_NPROC` — maximum number of processes. + pub fn with_max_processes(mut self, limit: u64) -> Self { + self.security.resource_limits.max_processes = Some(limit); + self + } + + /// `RLIMIT_AS` — maximum virtual memory, in bytes. + pub fn with_max_memory_bytes(mut self, bytes: u64) -> Self { + self.security.resource_limits.max_memory = Some(bytes); + self + } + + /// `RLIMIT_CPU` — maximum CPU time, in seconds. + pub fn with_max_cpu_time_seconds(mut self, seconds: u64) -> Self { + self.security.resource_limits.max_cpu_time = Some(seconds); + self + } + + /// Custom sandbox-exec profile path (macOS only). When `None` the + /// built-in modular profile is used. + pub fn with_sandbox_profile(mut self, path: Option) -> Self { + self.security.sandbox_profile = path; + self + } + + /// Allow / deny network access inside the macOS sandbox. Default + /// `true` because gvproxy networking needs it. + pub fn with_network_enabled(mut self, enabled: bool) -> Self { + self.security.network_enabled = enabled; + self + } + + // -------- preset shortcuts ----------------------------------------------- + // + // Mirror `SecurityOptions::{development, standard, maximum}` so a + // caller can pick a baseline with the same fluent style they use + // for the rest of the builder. + + /// Replace the security profile with the **development** preset — + /// minimal isolation, easiest to debug. + pub fn with_development_security(mut self) -> Self { + self.security = SecurityOptions::development(); + self + } + + /// Replace the security profile with the **standard** preset — + /// the recommended default for most callers. + pub fn with_standard_security(mut self) -> Self { + self.security = SecurityOptions::standard(); + self + } + + /// Replace the security profile with the **maximum** preset — all + /// isolation features enabled. + pub fn with_maximum_security(mut self) -> Self { + self.security = SecurityOptions::maximum(); + self + } + /// Preserve an FD through pre_exec by dup2'ing source to target. /// /// The pre_exec hook dup2s source to target before FD cleanup runs. @@ -264,4 +440,162 @@ mod tests { assert_eq!(jailer.box_id(), "test-box"); } + + // =========================================================== + // SecurityOptions fluent passthroughs + // + // One test per new public method, all on the same fluent chain + // so a regression that drops any setter shows up as a single + // failing assertion rather than a compile error elsewhere. + // Reverting (deleting) any `with_*` setter on the builder breaks + // the corresponding line below. + // =========================================================== + + #[test] + fn builder_exposes_uid_gid_passthroughs() { + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_uid(Some(1234)) + .with_gid(Some(5678)) + .build() + .expect("should build"); + assert_eq!(jailer.security().uid, Some(1234)); + assert_eq!(jailer.security().gid, Some(5678)); + } + + #[test] + fn builder_exposes_namespace_toggles() { + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_new_pid_ns(true) + .with_new_net_ns(true) + .build() + .expect("should build"); + assert!(jailer.security().new_pid_ns); + assert!(jailer.security().new_net_ns); + } + + #[test] + fn builder_exposes_chroot_settings() { + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_chroot_base("/var/empty") + .with_chroot_enabled(false) + .build() + .expect("should build"); + assert_eq!( + jailer.security().chroot_base, + std::path::PathBuf::from("/var/empty") + ); + assert!(!jailer.security().chroot_enabled); + } + + #[test] + fn builder_exposes_env_and_fd_hygiene() { + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_close_fds(false) + .with_sanitize_env(false) + .with_env_allowlist(vec!["FOO".into(), "BAR".into()]) + .with_allowed_env("BAZ") + .with_allowed_env("FOO") // dup must be ignored + .build() + .expect("should build"); + assert!(!jailer.security().close_fds); + assert!(!jailer.security().sanitize_env); + assert_eq!( + jailer.security().env_allowlist, + vec!["FOO".to_string(), "BAR".to_string(), "BAZ".to_string()] + ); + } + + #[test] + fn builder_exposes_resource_limits() { + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_max_open_files(2048) + .with_max_file_size_bytes(1_000_000) + .with_max_processes(64) + .with_max_memory_bytes(2 * 1024 * 1024 * 1024) + .with_max_cpu_time_seconds(900) + .build() + .expect("should build"); + let rl = &jailer.security().resource_limits; + assert_eq!(rl.max_open_files, Some(2048)); + assert_eq!(rl.max_file_size, Some(1_000_000)); + assert_eq!(rl.max_processes, Some(64)); + assert_eq!(rl.max_memory, Some(2 * 1024 * 1024 * 1024)); + assert_eq!(rl.max_cpu_time, Some(900)); + } + + #[test] + fn builder_exposes_resource_limits_bulk_set() { + let bulk = ResourceLimits { + max_open_files: Some(99), + ..Default::default() + }; + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_resource_limits(bulk) + .build() + .expect("should build"); + assert_eq!(jailer.security().resource_limits.max_open_files, Some(99)); + } + + #[test] + fn builder_exposes_macos_sandbox_knobs() { + let jailer = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_sandbox_profile(Some("/etc/box.sb".into())) + .with_network_enabled(false) + .build() + .expect("should build"); + assert_eq!( + jailer.security().sandbox_profile, + Some(std::path::PathBuf::from("/etc/box.sb")) + ); + assert!(!jailer.security().network_enabled); + } + + #[test] + fn builder_preset_shortcuts_pick_known_profiles() { + // Development → relaxed: jailer/chroot/close_fds off. + let dev = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_development_security() + .build() + .expect("should build dev"); + assert!(!dev.security().jailer_enabled); + + // Maximum → strict: jailer + seccomp on, namespaces on. + let max = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_maximum_security() + .build() + .expect("should build max"); + assert!(max.security().jailer_enabled); + assert!(max.security().seccomp_enabled); + + // Standard is the recommended default; just confirm it + // overrides whatever the chain set before. + let std_p = JailerBuilder::new() + .with_box_id("test-box") + .with_layout(test_layout("/tmp/box")) + .with_uid(Some(0)) // would normally be clobbered + .with_standard_security() + .build() + .expect("should build std"); + assert!(std_p.security().jailer_enabled); + // Preset wholesale-replaces, so the uid=0 set above is gone. + assert_ne!(std_p.security().uid, Some(0)); + } } From b254a455b7bf9496ae7e7120755239b97f2fd796 Mon Sep 17 00:00:00 2001 From: gamnaansong Date: Thu, 4 Jun 2026 07:07:39 +0000 Subject: [PATCH 2/3] feat(security): default sandbox on + plumb SecurityOptions through REST/CLI/Go/C MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reshape of #652. The original PR only added fluent setters to `JailerBuilder` (pure Rust SDK ergonomics). This rewrite is the real operator-facing fix: (1) Default-on. `SecurityOptions::default()` now returns the standard preset on Linux + macOS, not just macOS. Linux callers that hit the runtime default — which is REST, CLI, JSON config, and every SDK that doesn't explicitly construct `SecurityOptions` — were silently running with no jailer, no seccomp, no chroot. The flip is `default_jailer_enabled` returning `cfg!(any(linux, macos))` instead of `cfg!(macos)`, plus `default_seccomp_enabled` → `cfg!(target_os = "linux")`. (2) `SecurityOptions::from_preset(name)` central helper. Maps the operator-visible strings (`development` / `standard` / `maximum`, plus `dev` / `default` / `max` / `strict` synonyms, case-insensitive, trimmed) to the matching preset, or returns `BoxliteError::InvalidArgument` echoing the offending value. Every operator surface (CLI, REST, Go, C) routes through this one function so a typo surfaces identically everywhere instead of silently falling back to "default". (3) REST wire. Both client (`rest::types::CreateBoxRequest`) and server (`serve::types::CreateBoxRequest`) gain `security_settings: Option` (full struct) plus server-side `security: Option` (preset name). Client `from_options` populates `security_settings` only when `options.advanced.security != SecurityOptions::default()` — pre-PR POST bodies stay byte-identical. Server `build_box_options` resolves in priority order: settings > preset > default. Unknown preset surfaces back to the caller verbatim. (4) CLI `--security` flag. Added to `ManagementFlags` with env `BOXLITE_SECURITY`. `apply_to` returns Result so the typo case bubbles out to the operator. Callers in run.rs / create.rs updated with `?`. (5) C SDK FFI. `boxlite_options_set_security_preset(opts, preset, out_error) -> BoxliteErrorCode`. cbindgen regenerates the decl in `sdks/c/include/boxlite.h`. Returns InvalidArgument on unknown preset with `out_error` populated. (6) Go SDK. `WithSecurityPreset(string) BoxOption` calling the C FFI. Empty string = no-op (keeps runtime default). Unknown preset propagates as an error from `Runtime.Create` instead of silently landing on the default. Added `buildAndFreeCOptions` test helper because Go forbids cgo in `_test.go` files. The `JailerBuilder` setters from the previous version of this PR are kept — still useful for Rust SDK callers — but are no longer the headline. ## Tests Twelve new tests bracketing the contract on every surface; reverting the corresponding production hook flips exactly the right one red. `runtime::options::tests`: - `security_from_preset_canonical_names` — three preset names. - `security_from_preset_case_insensitive_and_synonyms` — `STANDARD`, ` maximum ` (trim), and every documented synonym (dev / default / max / strict). - `security_from_preset_unknown_surfaces_invalid_argument` — rejection echoes the offending value AND lists the supported presets. - `security_default_is_standard_on_supported_platforms` — the default-flip contract: reverting `default_jailer_enabled` flips this on Linux. - `test_security_builder_new` updated to the new default. `rest::types::tests`: - `test_create_box_request_omits_security_settings_when_default` — back-compat: default-config clients produce byte-identical POST bodies. - `test_create_box_request_carries_custom_security_settings_on_the_wire` — non-default surfaces as `security_settings`. `serve::tests`: - `build_box_options_no_security_field_keeps_default` — pre-PR bodies get the (now sandbox-on) default. - `build_box_options_security_preset_string_resolves` — preset name deserializes to the matching SecurityOptions. - `build_box_options_security_preset_typo_surfaces_invalid_argument` — unknown preset surfaces InvalidArgument all the way back to the REST client (not silent default). - `build_box_options_security_settings_wins_over_preset` — explicit settings beat the preset when both are sent. `cli::tests`: - `management_security_preset_applies_to_box_options` — `--security=maximum` lands as `SecurityOptions::maximum()`. - `management_security_preset_absent_leaves_default`. - `management_security_preset_typo_surfaces_anyhow_error`. `sdks/go`: - `TestBuildCOptions_SecurityPresetValid` — five preset strings apply cleanly. - `TestBuildCOptions_SecurityPresetUnknownRejects` — bad preset surfaces back with the offending value in the error. - `TestBuildCOptions_SecurityPresetEmptyKeepsDefault` — empty string = no-op. Full Rust lib sweep: 740/740 green (785 with --features rest, minus 1 pre-existing flake unrelated to this PR: `ws_watchdog_fires_when_idle` fails identically on upstream/main as confirmed by partial revert). Go SDK: 3/3 new tests green. Co-Authored-By: Claude Opus 4.7 (1M context) --- sdks/c/include/boxlite.h | 15 +++ sdks/c/src/options.rs | 51 +++++++++ sdks/go/boxlite_test.go | 43 ++++++++ sdks/go/options.go | 79 +++++++++++--- src/boxlite/src/rest/types.rs | 72 ++++++++++++- src/boxlite/src/runtime/advanced_options.rs | 37 ++++++- src/boxlite/src/runtime/options.rs | 108 +++++++++++++++++++- src/cli/src/cli.rs | 79 +++++++++++++- src/cli/src/commands/create.rs | 2 +- src/cli/src/commands/run.rs | 2 +- src/cli/src/commands/serve/mod.rs | 102 ++++++++++++++++++ src/cli/src/commands/serve/types.rs | 11 ++ 12 files changed, 576 insertions(+), 25 deletions(-) diff --git a/sdks/c/include/boxlite.h b/sdks/c/include/boxlite.h index 9f331995e..af54612c0 100644 --- a/sdks/c/include/boxlite.h +++ b/sdks/c/include/boxlite.h @@ -519,6 +519,21 @@ void boxlite_options_set_auto_remove(CBoxliteOptions *opts, int val); void boxlite_options_set_detach(CBoxliteOptions *opts, int val); +// Pick a sandbox security preset by name. +// +// `preset` is a C string — `"development"`, `"standard"`, or +// `"maximum"` (case-insensitive; `"dev"` / `"default"` / `"max"` / +// `"strict"` also accepted). On success returns +// `BoxliteErrorCode::Ok` and writes nothing to `out_error`. On an +// unknown / null name, returns `BoxliteErrorCode::InvalidArgument` +// and populates `out_error` with the offending value so the caller +// surfaces it back to the operator (don't silently fall through to +// the default — that's how operators end up running unsandboxed by +// accident). +enum BoxliteErrorCode boxlite_options_set_security_preset(CBoxliteOptions *opts, + const char *preset, + struct FFIError *out_error); + void boxlite_options_set_entrypoint(CBoxliteOptions *opts, const char *const *args, int argc); void boxlite_options_set_cmd(CBoxliteOptions *opts, const char *const *args, int argc); diff --git a/sdks/c/src/options.rs b/sdks/c/src/options.rs index 9b92e2fd4..2d303eddc 100644 --- a/sdks/c/src/options.rs +++ b/sdks/c/src/options.rs @@ -129,6 +129,26 @@ pub unsafe extern "C" fn boxlite_options_set_detach(opts: *mut CBoxliteOptions, options_set_detach(opts, val) } +/// Pick a sandbox security preset by name. +/// +/// `preset` is a C string — `"development"`, `"standard"`, or +/// `"maximum"` (case-insensitive; `"dev"` / `"default"` / `"max"` / +/// `"strict"` also accepted). On success returns +/// `BoxliteErrorCode::Ok` and writes nothing to `out_error`. On an +/// unknown / null name, returns `BoxliteErrorCode::InvalidArgument` +/// and populates `out_error` with the offending value so the caller +/// surfaces it back to the operator (don't silently fall through to +/// the default — that's how operators end up running unsandboxed by +/// accident). +#[unsafe(no_mangle)] +pub unsafe extern "C" fn boxlite_options_set_security_preset( + opts: *mut CBoxliteOptions, + preset: *const c_char, + out_error: *mut FFIError, +) -> BoxliteErrorCode { + unsafe { options_set_security_preset(opts, preset, out_error) } +} + #[unsafe(no_mangle)] pub unsafe extern "C" fn boxlite_options_set_entrypoint( opts: *mut CBoxliteOptions, @@ -241,6 +261,37 @@ pub unsafe fn options_set_workdir(handle: *mut OptionsHandle, workdir: *const c_ } } +/// Internal helper backing `boxlite_options_set_security_preset`. +pub unsafe fn options_set_security_preset( + handle: *mut OptionsHandle, + preset: *const c_char, + out_error: *mut FFIError, +) -> BoxliteErrorCode { + unsafe { + if handle.is_null() { + write_error(out_error, null_pointer_error("opts")); + return BoxliteErrorCode::InvalidArgument; + } + let preset_str = match c_str_to_string(preset) { + Ok(s) => s, + Err(e) => { + write_error(out_error, e); + return BoxliteErrorCode::InvalidArgument; + } + }; + match boxlite::SecurityOptions::from_preset(&preset_str) { + Ok(sec) => { + (*handle).options.advanced.security = sec; + BoxliteErrorCode::Ok + } + Err(e) => { + write_error(out_error, e); + BoxliteErrorCode::InvalidArgument + } + } + } +} + pub unsafe fn options_add_env(handle: *mut OptionsHandle, key: *const c_char, val: *const c_char) { unsafe { if handle.is_null() || key.is_null() || val.is_null() { diff --git a/sdks/go/boxlite_test.go b/sdks/go/boxlite_test.go index 35a514a2e..d5ec4e448 100644 --- a/sdks/go/boxlite_test.go +++ b/sdks/go/boxlite_test.go @@ -2,6 +2,7 @@ package boxlite import ( "errors" + "strings" "testing" "unsafe" ) @@ -399,6 +400,48 @@ func TestBuildCOptions_MissingImageAndPath(t *testing.T) { } } +// ============================================================================ +// Security preset +// ============================================================================ +// +// `WithSecurityPreset` routes through the C FFI +// `boxlite_options_set_security_preset`. We can't observe the resulting +// `BoxOptions` from Go (it's owned by the C handle), but we can verify the +// two contract bookends: a valid preset name returns no error, and an +// invalid name surfaces back as InvalidArgument instead of silently +// falling through to the default. + +func TestBuildCOptions_SecurityPresetValid(t *testing.T) { + for _, preset := range []string{"development", "standard", "maximum", "STANDARD", "max"} { + cfg := &boxConfig{} + WithSecurityPreset(preset)(cfg) + if err := buildAndFreeCOptions("alpine:latest", cfg); err != nil { + t.Fatalf("WithSecurityPreset(%q) must apply cleanly; got error: %v", preset, err) + } + } +} + +func TestBuildCOptions_SecurityPresetUnknownRejects(t *testing.T) { + cfg := &boxConfig{} + WithSecurityPreset("ultra")(cfg) + err := buildAndFreeCOptions("alpine:latest", cfg) + if err == nil { + t.Fatal("WithSecurityPreset(\"ultra\") must reject — silent fallthrough would let operators run unsandboxed by accident") + } + if !strings.Contains(err.Error(), "ultra") { + t.Fatalf("error must echo the offending preset name; got %v", err) + } +} + +func TestBuildCOptions_SecurityPresetEmptyKeepsDefault(t *testing.T) { + // Empty string = WithSecurityPreset never called effectively; + // leaves the runtime default in place. Must not error. + cfg := &boxConfig{securityPreset: ""} + if err := buildAndFreeCOptions("alpine:latest", cfg); err != nil { + t.Fatalf("empty preset must be a no-op; got error: %v", err) + } +} + // ============================================================================ // State constants // ============================================================================ diff --git a/sdks/go/options.go b/sdks/go/options.go index 59e26667f..2e6009431 100644 --- a/sdks/go/options.go +++ b/sdks/go/options.go @@ -83,20 +83,21 @@ type Secret struct { } type boxConfig struct { - name string - cpus int - memoryMiB int - diskSizeGB int - rootfsPath string - env [][2]string - volumes []volumeEntry - workDir string - entrypoint []string - cmd []string - autoRemove *bool - detach *bool - network *NetworkSpec - secrets []Secret + name string + cpus int + memoryMiB int + diskSizeGB int + rootfsPath string + env [][2]string + volumes []volumeEntry + workDir string + entrypoint []string + cmd []string + autoRemove *bool + detach *bool + network *NetworkSpec + secrets []Secret + securityPreset string // empty = server default; else "development" / "standard" / "maximum" } type volumeEntry struct { @@ -204,6 +205,42 @@ func WithDetach(v bool) BoxOption { return func(c *boxConfig) { c.detach = &v } } +// buildAndFreeCOptions runs buildCOptions, immediately frees the C +// handle on success, and returns just the error. Exists for unit +// tests in `_test.go` files, which Go forbids from using cgo +// directly — without this helper they can't exercise buildCOptions +// because the caller has to free `*C.CBoxliteOptions`. +func buildAndFreeCOptions(image string, cfg *boxConfig) error { + opts, err := buildCOptions(image, cfg) + if err != nil { + return err + } + if opts != nil { + C.boxlite_options_free(opts) + } + return nil +} + +// WithSecurityPreset picks a sandbox security preset by name. +// +// Accepts one of "development" / "standard" / "maximum" (case- +// insensitive). Empty string (the zero value) leaves the box on the +// server / runtime default, which is the standard preset. +// +// Examples: +// +// box, err := runtime.Create(ctx, "alpine:latest", +// boxlite.WithSecurityPreset("maximum")) +// box, err := runtime.Create(ctx, "alpine:latest", +// boxlite.WithSecurityPreset("development")) // disable isolation for debugging +// +// An invalid preset name surfaces at box creation as an +// InvalidArgument error, not silently — silent fallback would be how +// operators end up unsandboxed by accident. +func WithSecurityPreset(preset string) BoxOption { + return func(c *boxConfig) { c.securityPreset = preset } +} + func buildCOptions(image string, cfg *boxConfig) (*C.CBoxliteOptions, error) { image = strings.TrimSpace(image) rootfsPath := strings.TrimSpace(cfg.rootfsPath) @@ -311,6 +348,20 @@ func buildCOptions(image string, cfg *boxConfig) (*C.CBoxliteOptions, error) { if cfg.detach != nil { C.boxlite_options_set_detach(cOpts, boolToCInt(*cfg.detach)) } + if cfg.securityPreset != "" { + cPreset := toCString(cfg.securityPreset) + var cerrSec C.CBoxliteError + code := C.boxlite_options_set_security_preset(cOpts, cPreset, &cerrSec) + C.free(unsafe.Pointer(cPreset)) + if code != C.Ok { + // Bad preset name surfaces all the way back to the caller + // with the offending value — silently falling through to + // the default is how operators end up running unsandboxed + // by accident. + C.boxlite_options_free(cOpts) + return nil, freeError(&cerrSec) + } + } if cfg.entrypoint != nil { cArgs, argc := toCStringArray(cfg.entrypoint) C.boxlite_options_set_entrypoint(cOpts, cArgs, C.int(argc)) diff --git a/src/boxlite/src/rest/types.rs b/src/boxlite/src/rest/types.rs index fcc3d9b1b..ef03b6f63 100644 --- a/src/boxlite/src/rest/types.rs +++ b/src/boxlite/src/rest/types.rs @@ -100,8 +100,20 @@ pub(crate) struct CreateBoxRequest { pub auto_remove: Option, #[serde(skip_serializing_if = "Option::is_none")] pub detach: Option, + /// Operator-level security preset (`"development"`, `"standard"`, + /// `"maximum"`). Omitted = server default + /// (`SecurityOptions::default()`, which is itself the standard + /// preset since this PR). Ignored if `security_settings` is also + /// present — explicit settings always win. #[serde(skip_serializing_if = "Option::is_none")] pub security: Option, + /// Full custom `SecurityOptions` on the wire. Carries the caller's + /// exact field set so REST and local runtimes behave identically + /// for the same `BoxOptions`. Omitted when absent OR when the + /// settings equal `SecurityOptions::default()` so pre-PR clients + /// keep their POST body byte-identical. + #[serde(skip_serializing_if = "Option::is_none")] + pub security_settings: Option, } impl CreateBoxRequest { @@ -128,6 +140,15 @@ impl CreateBoxRequest { Some(options.secrets.iter().map(CreateBoxSecret::from).collect()) }; + // Carry the caller's `SecurityOptions` on the wire only when + // it differs from the server's default — otherwise the wire + // body stays byte-identical with pre-PR clients. Preset name + // is left None on the client; presets are an operator-facing + // convenience (CLI/Go/C) and the server resolves them on + // arrival. SDK callers send the full struct. + let security_settings = (options.advanced.security != crate::SecurityOptions::default()) + .then(|| options.advanced.security.clone()); + Self { name, image, @@ -144,7 +165,8 @@ impl CreateBoxRequest { secrets, auto_remove: Some(options.auto_remove), detach: Some(options.detach), - security: None, // TODO: map security preset + security: None, + security_settings, } } } @@ -481,6 +503,7 @@ mod tests { auto_remove: Some(true), detach: None, security: None, + security_settings: None, }; let json = serde_json::to_string(&req).unwrap(); assert!(json.contains("\"name\":\"mybox\"")); @@ -553,6 +576,53 @@ mod tests { assert!(req.network.as_ref().unwrap().allow_net.is_empty()); } + /// Default `BoxOptions` (= default `SecurityOptions`) must NOT + /// emit `security_settings` on the wire — keeps pre-PR clients + /// byte-identical with their existing POST body. + #[test] + fn test_create_box_request_omits_security_settings_when_default() { + use crate::runtime::options::{BoxOptions, RootfsSpec}; + let opts = BoxOptions { + rootfs: RootfsSpec::Image("alpine:latest".into()), + ..Default::default() + }; + let req = CreateBoxRequest::from_options(&opts, None); + assert_eq!(req.security_settings, None); + let json = serde_json::to_string(&req).unwrap(); + assert!( + !json.contains("security_settings"), + "wire form must omit settings on default security; got: {json}" + ); + } + + /// A non-default `SecurityOptions` (here: `maximum` preset) MUST + /// surface on the wire as `security_settings` so the server + /// reconstructs it exactly. Reverting either the struct field or + /// the populate path in `from_options` flips this red. + #[test] + fn test_create_box_request_carries_custom_security_settings_on_the_wire() { + use crate::SecurityOptions; + use crate::runtime::advanced_options::AdvancedBoxOptions; + use crate::runtime::options::{BoxOptions, RootfsSpec}; + + let custom = SecurityOptions::maximum(); + let opts = BoxOptions { + rootfs: RootfsSpec::Image("alpine:latest".into()), + advanced: AdvancedBoxOptions { + security: custom.clone(), + ..Default::default() + }, + ..Default::default() + }; + let req = CreateBoxRequest::from_options(&opts, None); + assert_eq!(req.security_settings.as_ref(), Some(&custom)); + let json = serde_json::to_string(&req).unwrap(); + assert!( + json.contains("security_settings"), + "custom settings must surface on the wire; got: {json}" + ); + } + #[test] fn test_box_response_deserialization() { let json = r#"{ diff --git a/src/boxlite/src/runtime/advanced_options.rs b/src/boxlite/src/runtime/advanced_options.rs index 07d39c4af..9c21140f0 100644 --- a/src/boxlite/src/runtime/advanced_options.rs +++ b/src/boxlite/src/runtime/advanced_options.rs @@ -83,7 +83,7 @@ impl Default for HealthCheckOptions { /// /// These options control how the boxlite-shim process is isolated from the host. /// Different presets are available for different security requirements. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct SecurityOptions { /// Enable jailer isolation. /// @@ -185,7 +185,7 @@ pub struct SecurityOptions { } /// Resource limits for the jailed process. -#[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct ResourceLimits { /// Maximum number of open file descriptors (RLIMIT_NOFILE). #[serde(default)] @@ -211,11 +211,22 @@ pub struct ResourceLimits { // Default value functions for SecurityOptions fn default_jailer_enabled() -> bool { - cfg!(target_os = "macos") + // Default-on across all supported sandbox targets. Previously this + // was `cfg!(target_os = "macos")` — Linux callers that didn't + // explicitly pick a preset got `jailer_enabled = false` and a + // shim running on the bare host. `SecurityOptions::default()` is + // what most operator surfaces (REST, CLI, JSON config) silently + // fall back to, so leaving Linux off there meant the most common + // deployment path was unsandboxed by accident. + cfg!(any(target_os = "linux", target_os = "macos")) } fn default_seccomp_enabled() -> bool { - false + // Linux-only — the seccomp syscall whitelist is a Linux concept. + // Same motivation as `default_jailer_enabled`: anyone landing on + // the default got no syscall filter, including REST / CLI paths + // that don't yet expose `SecurityOptions` at all. + cfg!(target_os = "linux") } fn default_chroot_base() -> PathBuf { @@ -296,6 +307,24 @@ impl SecurityOptions { } } + /// Look up a preset by name. Accepts the same three labels the CLI + /// / REST / Go / C surfaces use (case-insensitive). Returns an + /// `InvalidArgument` error for anything else so operator surfaces + /// can surface the typo back to the caller verbatim. + pub fn from_preset(name: &str) -> boxlite_shared::errors::BoxliteResult { + match name.trim().to_ascii_lowercase().as_str() { + "development" | "dev" => Ok(Self::development()), + "standard" | "default" => Ok(Self::standard()), + "maximum" | "max" | "strict" => Ok(Self::maximum()), + other => Err(boxlite_shared::errors::BoxliteError::InvalidArgument( + format!( + "unknown security preset {other:?}; expected one of \ + development|standard|maximum" + ), + )), + } + } + /// Maximum mode: all isolation features enabled. /// /// Use this for untrusted workloads (AI sandbox, multi-tenant). diff --git a/src/boxlite/src/runtime/options.rs b/src/boxlite/src/runtime/options.rs index 7ea29870d..d14ca286e 100644 --- a/src/boxlite/src/runtime/options.rs +++ b/src/boxlite/src/runtime/options.rs @@ -888,11 +888,18 @@ mod tests { #[test] fn test_security_builder_new() { let opts = SecurityOptionsBuilder::new().build(); - // Default enables jailer on macOS, disables on Linux and other platforms. - #[cfg(target_os = "macos")] + // Default is now the standard preset on both Linux and macOS + // (flipped in this PR — previously Linux defaulted off, which + // meant REST / CLI / JSON-config paths silently ran unsandboxed). + // - jailer enabled on Linux + macOS + // - seccomp enabled on Linux (no-op on macOS) + #[cfg(any(target_os = "linux", target_os = "macos"))] assert!(opts.jailer_enabled); - #[cfg(not(target_os = "macos"))] + #[cfg(not(any(target_os = "linux", target_os = "macos")))] assert!(!opts.jailer_enabled); + #[cfg(target_os = "linux")] + assert!(opts.seccomp_enabled); + #[cfg(not(target_os = "linux"))] assert!(!opts.seccomp_enabled); } @@ -911,6 +918,101 @@ mod tests { assert!(max.sanitize_env); } + // =========================================================== + // SecurityOptions::from_preset — operator-surface contract + // + // CLI / REST / Go / C all funnel preset *strings* through this + // helper. Reverting (deleting the match) flips all four red. The + // synonyms are part of the public contract — `from_preset("dev")` + // is the same as `from_preset("development")` and so on. + // =========================================================== + + #[test] + fn security_from_preset_canonical_names() { + use crate::runtime::advanced_options::SecurityOptions; + assert_eq!( + SecurityOptions::from_preset("development").unwrap(), + SecurityOptions::development() + ); + assert_eq!( + SecurityOptions::from_preset("standard").unwrap(), + SecurityOptions::standard() + ); + assert_eq!( + SecurityOptions::from_preset("maximum").unwrap(), + SecurityOptions::maximum() + ); + } + + #[test] + fn security_from_preset_case_insensitive_and_synonyms() { + use crate::runtime::advanced_options::SecurityOptions; + // Casing. + assert_eq!( + SecurityOptions::from_preset("STANDARD").unwrap(), + SecurityOptions::standard() + ); + // Trim whitespace. + assert_eq!( + SecurityOptions::from_preset(" maximum ").unwrap(), + SecurityOptions::maximum() + ); + // Documented synonyms. + assert_eq!( + SecurityOptions::from_preset("dev").unwrap(), + SecurityOptions::development() + ); + assert_eq!( + SecurityOptions::from_preset("default").unwrap(), + SecurityOptions::standard() + ); + assert_eq!( + SecurityOptions::from_preset("max").unwrap(), + SecurityOptions::maximum() + ); + assert_eq!( + SecurityOptions::from_preset("strict").unwrap(), + SecurityOptions::maximum() + ); + } + + #[test] + fn security_from_preset_unknown_surfaces_invalid_argument() { + use crate::runtime::advanced_options::SecurityOptions; + let err = SecurityOptions::from_preset("ultra").expect_err("typo must reject"); + let msg = err.to_string(); + assert!( + msg.contains("ultra"), + "rejection must echo the offending value; got {msg}" + ); + assert!( + msg.contains("development") && msg.contains("standard") && msg.contains("maximum"), + "rejection must list the supported presets; got {msg}" + ); + } + + /// Default-flip contract: `SecurityOptions::default()` and + /// `BoxOptions::default().advanced.security` must now be the + /// standard preset on Linux + macOS. Reverting + /// `default_jailer_enabled` to `cfg!(target_os = "macos")` flips + /// this red on Linux. + #[test] + fn security_default_is_standard_on_supported_platforms() { + use crate::runtime::advanced_options::SecurityOptions; + let direct = SecurityOptions::default(); + let via_box = BoxOptions::default().advanced.security; + #[cfg(any(target_os = "linux", target_os = "macos"))] + { + assert!(direct.jailer_enabled); + assert!(via_box.jailer_enabled); + } + #[cfg(target_os = "linux")] + { + assert!(direct.seccomp_enabled); + assert!(via_box.seccomp_enabled); + } + } + #[test] fn test_security_builder_chaining() { let opts = SecurityOptionsBuilder::standard() diff --git a/src/cli/src/cli.rs b/src/cli/src/cli.rs index 999edd019..f9c1de1b7 100644 --- a/src/cli/src/cli.rs +++ b/src/cli/src/cli.rs @@ -693,12 +693,27 @@ pub struct ManagementFlags { /// Automatically remove the box when it exits #[arg(long)] pub rm: bool, + + /// Sandbox security preset. One of `development`, `standard`, or + /// `maximum` (case-insensitive). Absent → the box uses + /// `SecurityOptions::default()`, which is the standard preset + /// (jailer + seccomp on for Linux). Use `--security=development` + /// to disable isolation when debugging. + #[arg(long, env = "BOXLITE_SECURITY")] + pub security: Option, } impl ManagementFlags { - pub fn apply_to(&self, opts: &mut BoxOptions) { + pub fn apply_to(&self, opts: &mut BoxOptions) -> anyhow::Result<()> { opts.detach = self.detach; opts.auto_remove = self.rm; + if let Some(ref preset) = self.security { + // Bubble the typo'd-preset error all the way back to the + // CLI exit so the operator sees the offending value. + opts.advanced.security = + boxlite::SecurityOptions::from_preset(preset).map_err(anyhow::Error::from)?; + } + Ok(()) } } @@ -1142,4 +1157,66 @@ mod tests { }; assert!(login.api_key_stdin); } + + // ============================================================ + // ManagementFlags --security + // + // Side A (preset valid) — `--security=maximum` lands as + // SecurityOptions::maximum() on the resulting BoxOptions. + // Side B (preset invalid) — surfaces back as an + // anyhow::Error pointing at the offending value. Reverting the + // `from_preset(preset)?` call in apply_to flips both red. + // ============================================================ + + #[test] + fn management_security_preset_applies_to_box_options() { + let flags = ManagementFlags { + name: None, + detach: false, + rm: false, + security: Some("maximum".to_string()), + }; + let mut opts = BoxOptions::default(); + flags.apply_to(&mut opts).expect("preset must apply"); + assert_eq!( + opts.advanced.security, + boxlite::SecurityOptions::maximum(), + "advanced.security must equal SecurityOptions::maximum()" + ); + } + + #[test] + fn management_security_preset_absent_leaves_default() { + let flags = ManagementFlags { + name: None, + detach: false, + rm: false, + security: None, + }; + let mut opts = BoxOptions::default(); + flags + .apply_to(&mut opts) + .expect("absent preset must succeed"); + assert_eq!( + opts.advanced.security, + boxlite::SecurityOptions::default(), + "absent preset must leave the default in place" + ); + } + + #[test] + fn management_security_preset_typo_surfaces_anyhow_error() { + let flags = ManagementFlags { + name: None, + detach: false, + rm: false, + security: Some("ultra".to_string()), + }; + let mut opts = BoxOptions::default(); + let err = flags + .apply_to(&mut opts) + .expect_err("unknown preset must reject at apply_to"); + let msg = err.to_string(); + assert!(msg.contains("ultra"), "got {msg}"); + } } diff --git a/src/cli/src/commands/create.rs b/src/cli/src/commands/create.rs index 1f58624a9..67a332db4 100644 --- a/src/cli/src/commands/create.rs +++ b/src/cli/src/commands/create.rs @@ -44,7 +44,7 @@ impl CreateArgs { fn to_box_options(&self, global: &GlobalFlags) -> anyhow::Result { let mut options = BoxOptions::default(); self.resource.apply_to(&mut options); - self.management.apply_to(&mut options); + self.management.apply_to(&mut options)?; self.publish.apply_to(&mut options)?; self.volume.apply_to(&mut options, global.home.as_deref())?; options.working_dir = self.workdir.clone(); diff --git a/src/cli/src/commands/run.rs b/src/cli/src/commands/run.rs index 13e81fb6e..f864e05dd 100644 --- a/src/cli/src/commands/run.rs +++ b/src/cli/src/commands/run.rs @@ -94,7 +94,7 @@ impl BoxRunner { async fn create_box(&self) -> anyhow::Result { let mut options = BoxOptions::default(); self.args.resource.apply_to(&mut options); - self.args.management.apply_to(&mut options); + self.args.management.apply_to(&mut options)?; self.args.publish.apply_to(&mut options)?; self.args .volume diff --git a/src/cli/src/commands/serve/mod.rs b/src/cli/src/commands/serve/mod.rs index 98a619518..c8670320c 100644 --- a/src/cli/src/commands/serve/mod.rs +++ b/src/cli/src/commands/serve/mod.rs @@ -656,6 +656,22 @@ fn build_box_options(req: &CreateBoxRequest) -> Result NetworkSpec::default(), }; + // Security resolution, in priority order: + // 1. `security_settings` (full custom struct) wins outright. + // 2. `security` preset string resolves via + // `SecurityOptions::from_preset`. Unknown name surfaces as + // `InvalidArgument` back to the REST client. + // 3. Neither set → keep `AdvancedBoxOptions::default()`, which + // since this PR is the standard preset (jailer + seccomp on + // for Linux). Pre-PR clients that don't mention security + // thus get sandbox-by-default, matching the local SDK path. + let mut advanced = boxlite::AdvancedBoxOptions::default(); + if let Some(custom) = req.security_settings.clone() { + advanced.security = custom; + } else if let Some(name) = req.security.as_deref() { + advanced.security = boxlite::SecurityOptions::from_preset(name)?; + } + Ok(BoxOptions { rootfs, cpus: req.cpus, @@ -669,6 +685,7 @@ fn build_box_options(req: &CreateBoxRequest) -> Result ( diff --git a/src/cli/src/commands/serve/types.rs b/src/cli/src/commands/serve/types.rs index 278d39e66..89fc1d5cb 100644 --- a/src/cli/src/commands/serve/types.rs +++ b/src/cli/src/commands/serve/types.rs @@ -39,6 +39,17 @@ pub(super) struct CreateBoxRequest { pub auto_remove: Option, #[serde(default)] pub detach: Option, + /// Preset name (`"development"` / `"standard"` / `"maximum"`). + /// Server resolves via `SecurityOptions::from_preset`. Absent + + /// `security_settings` absent = server default + /// (`SecurityOptions::default()`, which is the standard preset). + /// Ignored when `security_settings` is also present. + #[serde(default)] + pub security: Option, + /// Custom `SecurityOptions` struct verbatim. Wins over `security` + /// preset when both are sent. + #[serde(default)] + pub security_settings: Option, } #[derive(Deserialize)] From 27fe72e3cecb3dcf45d34186425c6040e4b416de Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Thu, 4 Jun 2026 11:56:24 +0000 Subject: [PATCH 3/3] refactor(c,go): defer security-preset validation to box create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @DorianZheng review on #652: the new `boxlite_options_set_security_preset` was the only `set_*` in the C SDK with an `(out_error, BoxliteErrorCode)` signature — every sibling (`set_cpus`, `set_rootfs_path`, `set_auto_remove`, …) returns void. The only thing forcing the error-returning signature was sync-time preset validation, but that's not necessary: the typo just needs to surface loudly *somewhere* before a box runs. Moving validation to `boxlite_create_box` keeps the loud-rejection contract intact (InvalidArgument + offending name echoed) while letting the setter match its siblings. What changed: - C: setter is now `void`. Preset name is stashed on `OptionsHandle` via a new `pending_security_preset: Option` field, and `create_box` calls `SecurityOptions::from_preset` synchronously before taking ownership of `opts` — bad names return `InvalidArgument` immediately, callback never fires, caller still owns opts (same ownership semantics as the null-cb error path). - Go: `buildCOptions` drops the cerrSec / freeError block; calls the void setter and lets the typo surface from `Runtime.Create`. - Tests: - New `create_box_rejects_unknown_security_preset` in `sdks/c/src/tests.rs` pins the C-side deferred-validation contract (sync InvalidArgument + "ultra" in the message + callback never fires + opts still ownable by caller). - Deleted Go-side `TestBuildCOptions_SecurityPresetUnknownRejects` — it asserted error-at-buildCOptions which is no longer true. The policy is still covered by the Rust unit test `security_from_preset_unknown_surfaces_invalid_argument` plus the new C-SDK integration test above. Two-side verified: with the new code in place, the deleted Go test fired its t.Fatal at boxlite_test.go:429 (buildAndFreeCOptions now returns nil for bad presets) — confirming the behavior really moved. All 53 boxlite-c crate tests pass; the 2 remaining Go preset tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- sdks/c/include/boxlite.h | 20 ++++++------- sdks/c/src/box_handle.rs | 20 ++++++++++++- sdks/c/src/options.rs | 62 +++++++++++++++++++--------------------- sdks/c/src/tests.rs | 57 ++++++++++++++++++++++++++++++++++++ sdks/go/boxlite_test.go | 29 +++++++------------ sdks/go/options.go | 15 ++++------ 6 files changed, 130 insertions(+), 73 deletions(-) diff --git a/sdks/c/include/boxlite.h b/sdks/c/include/boxlite.h index af54612c0..e9f2b5d9f 100644 --- a/sdks/c/include/boxlite.h +++ b/sdks/c/include/boxlite.h @@ -523,16 +523,16 @@ void boxlite_options_set_detach(CBoxliteOptions *opts, int val); // // `preset` is a C string — `"development"`, `"standard"`, or // `"maximum"` (case-insensitive; `"dev"` / `"default"` / `"max"` / -// `"strict"` also accepted). On success returns -// `BoxliteErrorCode::Ok` and writes nothing to `out_error`. On an -// unknown / null name, returns `BoxliteErrorCode::InvalidArgument` -// and populates `out_error` with the offending value so the caller -// surfaces it back to the operator (don't silently fall through to -// the default — that's how operators end up running unsandboxed by -// accident). -enum BoxliteErrorCode boxlite_options_set_security_preset(CBoxliteOptions *opts, - const char *preset, - struct FFIError *out_error); +// `"strict"` also accepted). NULL clears any previously-set preset. +// +// The preset name is validated at `boxlite_create_box` time, not +// here — an unknown name surfaces from create as +// `BoxliteErrorCode::InvalidArgument` with the offending value in +// `out_error`. Silent fallback to the default is never acceptable. +// +// Signature matches sibling `boxlite_options_set_*` setters so the +// C SDK stays uniform; the error path lives at create. +void boxlite_options_set_security_preset(CBoxliteOptions *opts, const char *preset); void boxlite_options_set_entrypoint(CBoxliteOptions *opts, const char *const *args, int argc); diff --git a/sdks/c/src/box_handle.rs b/sdks/c/src/box_handle.rs index 4336cd73e..bb7b7c4e4 100644 --- a/sdks/c/src/box_handle.rs +++ b/sdks/c/src/box_handle.rs @@ -120,8 +120,26 @@ unsafe fn create_box( } let cb = crate::unwrap_cb_or_return!(cb, out_error); + // Resolve any pending security preset before we take ownership + // of `opts`. On a bad name the caller still owns + frees opts, + // matching the null-runtime / null-cb error paths above. + let resolved_security = match (*opts).pending_security_preset.as_ref() { + Some(preset) => match boxlite::SecurityOptions::from_preset(preset) { + Ok(sec) => Some(sec), + Err(e) => { + write_error(out_error, e); + return BoxliteErrorCode::InvalidArgument; + } + }, + None => None, + }; + let runtime_ref = &*runtime; - let opts_handle = Box::from_raw(opts); + let mut opts_handle = Box::from_raw(opts); + opts_handle.pending_security_preset = None; + if let Some(sec) = resolved_security { + opts_handle.options.advanced.security = sec; + } let runtime_clone = runtime_ref.runtime.clone(); let tokio_rt = runtime_ref.tokio_rt.clone(); let queue = runtime_ref.queue.clone(); diff --git a/sdks/c/src/options.rs b/sdks/c/src/options.rs index 2d303eddc..b79d33fdd 100644 --- a/sdks/c/src/options.rs +++ b/sdks/c/src/options.rs @@ -11,6 +11,11 @@ use crate::{CBoxliteError, CBoxliteOptions}; pub struct OptionsHandle { pub options: BoxOptions, pub name: Option, + /// Security preset name (e.g. "maximum") stashed by + /// `boxlite_options_set_security_preset`. Resolved in + /// `boxlite_create_box`; unknown names surface as + /// `InvalidArgument` from there, not from the setter. + pub pending_security_preset: Option, } #[unsafe(no_mangle)] @@ -133,20 +138,21 @@ pub unsafe extern "C" fn boxlite_options_set_detach(opts: *mut CBoxliteOptions, /// /// `preset` is a C string — `"development"`, `"standard"`, or /// `"maximum"` (case-insensitive; `"dev"` / `"default"` / `"max"` / -/// `"strict"` also accepted). On success returns -/// `BoxliteErrorCode::Ok` and writes nothing to `out_error`. On an -/// unknown / null name, returns `BoxliteErrorCode::InvalidArgument` -/// and populates `out_error` with the offending value so the caller -/// surfaces it back to the operator (don't silently fall through to -/// the default — that's how operators end up running unsandboxed by -/// accident). +/// `"strict"` also accepted). NULL clears any previously-set preset. +/// +/// The preset name is validated at `boxlite_create_box` time, not +/// here — an unknown name surfaces from create as +/// `BoxliteErrorCode::InvalidArgument` with the offending value in +/// `out_error`. Silent fallback to the default is never acceptable. +/// +/// Signature matches sibling `boxlite_options_set_*` setters so the +/// C SDK stays uniform; the error path lives at create. #[unsafe(no_mangle)] pub unsafe extern "C" fn boxlite_options_set_security_preset( opts: *mut CBoxliteOptions, preset: *const c_char, - out_error: *mut FFIError, -) -> BoxliteErrorCode { - unsafe { options_set_security_preset(opts, preset, out_error) } +) { + unsafe { options_set_security_preset(opts, preset) } } #[unsafe(no_mangle)] @@ -197,6 +203,7 @@ pub unsafe fn options_new( ..Default::default() }, name: None, + pending_security_preset: None, }); *out_opts = Box::into_raw(handle); @@ -262,32 +269,21 @@ pub unsafe fn options_set_workdir(handle: *mut OptionsHandle, workdir: *const c_ } /// Internal helper backing `boxlite_options_set_security_preset`. -pub unsafe fn options_set_security_preset( - handle: *mut OptionsHandle, - preset: *const c_char, - out_error: *mut FFIError, -) -> BoxliteErrorCode { +/// +/// Stashes the name; `boxlite_create_box` calls +/// `SecurityOptions::from_preset` to validate + apply. NULL preset +/// clears any previously-stashed name. +pub unsafe fn options_set_security_preset(handle: *mut OptionsHandle, preset: *const c_char) { unsafe { if handle.is_null() { - write_error(out_error, null_pointer_error("opts")); - return BoxliteErrorCode::InvalidArgument; + return; } - let preset_str = match c_str_to_string(preset) { - Ok(s) => s, - Err(e) => { - write_error(out_error, e); - return BoxliteErrorCode::InvalidArgument; - } - }; - match boxlite::SecurityOptions::from_preset(&preset_str) { - Ok(sec) => { - (*handle).options.advanced.security = sec; - BoxliteErrorCode::Ok - } - Err(e) => { - write_error(out_error, e); - BoxliteErrorCode::InvalidArgument - } + if preset.is_null() { + (*handle).pending_security_preset = None; + return; + } + if let Ok(s) = c_str_to_string(preset) { + (*handle).pending_security_preset = Some(s); } } } diff --git a/sdks/c/src/tests.rs b/sdks/c/src/tests.rs index 12d09b886..1b118a066 100644 --- a/sdks/c/src/tests.rs +++ b/sdks/c/src/tests.rs @@ -376,6 +376,63 @@ fn create_box_rejects_null_callback() { let _ = std::fs::remove_dir_all(home_dir); } +// Pins the second half of the deferred-validation contract for +// security presets: `boxlite_options_set_security_preset` accepts any +// C string without complaint, but `boxlite_create_box` synchronously +// rejects an unknown preset with InvalidArgument + offending value in +// the error — so the typo can never silently fall through to the +// default. Companion to `security_from_preset_unknown_surfaces_invalid_argument` +// in boxlite::runtime::options (which pins the underlying parser). +extern "C" fn create_cb_must_not_fire( + _box_handle: *mut crate::CBoxHandle, + _error: *mut crate::CBoxliteError, + _user_data: *mut c_void, +) { + panic!("callback must not fire when preset is rejected synchronously"); +} + +#[test] +fn create_box_rejects_unknown_security_preset() { + let (runtime, home_dir) = unsafe { new_test_runtime_handle("bad-preset-create") }; + + let image = CString::new("alpine:latest").expect("image cstring"); + let mut opts: *mut CBoxliteOptions = ptr::null_mut(); + let mut error = FFIError::default(); + let opts_code = + unsafe { boxlite_options_new(image.as_ptr(), &mut opts as *mut _, &mut error as *mut _) }; + assert_eq!(opts_code, BoxliteErrorCode::Ok); + + let preset = CString::new("ultra").expect("preset cstring"); + unsafe { boxlite_options_set_security_preset(opts, preset.as_ptr()) }; + + let code = unsafe { + boxlite_create_box( + runtime, + opts, + Some(create_cb_must_not_fire), + ptr::null_mut(), + &mut error as *mut _, + ) + }; + assert_eq!(code, BoxliteErrorCode::InvalidArgument); + assert!(!error.message.is_null()); + let msg = unsafe { CStr::from_ptr(error.message) } + .to_string_lossy() + .into_owned(); + assert!( + msg.contains("ultra"), + "error should echo the offending preset: {msg}" + ); + + // Sync failure: opts not consumed; caller still owns + frees. + unsafe { + boxlite_error_free(&mut error as *mut _); + boxlite_options_free(opts); + boxlite_runtime_free(runtime); + } + let _ = std::fs::remove_dir_all(home_dir); +} + #[test] fn runtime_metrics_rejects_null_callback() { let (runtime, home_dir) = unsafe { new_test_runtime_handle("null-cb-rtmet") }; diff --git a/sdks/go/boxlite_test.go b/sdks/go/boxlite_test.go index d5ec4e448..0b66214bc 100644 --- a/sdks/go/boxlite_test.go +++ b/sdks/go/boxlite_test.go @@ -2,7 +2,6 @@ package boxlite import ( "errors" - "strings" "testing" "unsafe" ) @@ -404,12 +403,16 @@ func TestBuildCOptions_MissingImageAndPath(t *testing.T) { // Security preset // ============================================================================ // -// `WithSecurityPreset` routes through the C FFI -// `boxlite_options_set_security_preset`. We can't observe the resulting -// `BoxOptions` from Go (it's owned by the C handle), but we can verify the -// two contract bookends: a valid preset name returns no error, and an -// invalid name surfaces back as InvalidArgument instead of silently -// falling through to the default. +// `WithSecurityPreset` stashes the name on the boxConfig; `buildCOptions` +// forwards it to the C SDK, which resolves it at Create time. So +// `buildCOptions` itself can't see a bad preset — the rejection path +// lives at Create. We pin what buildCOptions CAN observe here: +// valid presets round-trip without error, and empty is a no-op. +// +// The "unknown preset → InvalidArgument" policy is covered by +// `security_from_preset_unknown_surfaces_invalid_argument` (Rust +// boxlite::runtime::options) and `create_box_rejects_unknown_security_preset` +// (C SDK integration). func TestBuildCOptions_SecurityPresetValid(t *testing.T) { for _, preset := range []string{"development", "standard", "maximum", "STANDARD", "max"} { @@ -421,18 +424,6 @@ func TestBuildCOptions_SecurityPresetValid(t *testing.T) { } } -func TestBuildCOptions_SecurityPresetUnknownRejects(t *testing.T) { - cfg := &boxConfig{} - WithSecurityPreset("ultra")(cfg) - err := buildAndFreeCOptions("alpine:latest", cfg) - if err == nil { - t.Fatal("WithSecurityPreset(\"ultra\") must reject — silent fallthrough would let operators run unsandboxed by accident") - } - if !strings.Contains(err.Error(), "ultra") { - t.Fatalf("error must echo the offending preset name; got %v", err) - } -} - func TestBuildCOptions_SecurityPresetEmptyKeepsDefault(t *testing.T) { // Empty string = WithSecurityPreset never called effectively; // leaves the runtime default in place. Must not error. diff --git a/sdks/go/options.go b/sdks/go/options.go index 2e6009431..5121754ae 100644 --- a/sdks/go/options.go +++ b/sdks/go/options.go @@ -349,18 +349,13 @@ func buildCOptions(image string, cfg *boxConfig) (*C.CBoxliteOptions, error) { C.boxlite_options_set_detach(cOpts, boolToCInt(*cfg.detach)) } if cfg.securityPreset != "" { + // Preset name is validated at Create, not here — a typo + // surfaces as InvalidArgument from Runtime.Create with the + // offending value in the error. Silent fallthrough to default + // is never acceptable. cPreset := toCString(cfg.securityPreset) - var cerrSec C.CBoxliteError - code := C.boxlite_options_set_security_preset(cOpts, cPreset, &cerrSec) + C.boxlite_options_set_security_preset(cOpts, cPreset) C.free(unsafe.Pointer(cPreset)) - if code != C.Ok { - // Bad preset name surfaces all the way back to the caller - // with the offending value — silently falling through to - // the default is how operators end up running unsandboxed - // by accident. - C.boxlite_options_free(cOpts) - return nil, freeError(&cerrSec) - } } if cfg.entrypoint != nil { cArgs, argc := toCStringArray(cfg.entrypoint)