From 0693dc01a9c68917b9341c3b630ad946fc36e93e Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 22 May 2026 10:29:42 +0200 Subject: [PATCH] fix(sandbox): decouple GPU baseline from network policy Signed-off-by: Evan Lezar --- crates/openshell-sandbox/src/lib.rs | 152 +++++++++++++++++++++------- 1 file changed, 113 insertions(+), 39 deletions(-) diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index ded56ce9e..576919444 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -1450,22 +1450,39 @@ fn enumerate_gpu_device_nodes() -> Vec { paths } -/// Collect all baseline paths for enrichment: proxy defaults + GPU (if present). -/// Returns `(read_only, read_write)` as owned `String` vecs. -fn baseline_enrichment_paths() -> (Vec, Vec) { - let mut ro: Vec = PROXY_BASELINE_READ_ONLY - .iter() - .map(|&s| s.to_string()) - .collect(); - let mut rw: Vec = PROXY_BASELINE_READ_WRITE - .iter() - .map(|&s| s.to_string()) - .collect(); +fn push_unique(paths: &mut Vec, path: String) { + if !paths.iter().any(|p| p == &path) { + paths.push(path); + } +} - if has_gpu_devices() { - ro.extend(GPU_BASELINE_READ_ONLY.iter().map(|&s| s.to_string())); - rw.extend(GPU_BASELINE_READ_WRITE.iter().map(|&s| s.to_string())); - rw.extend(enumerate_gpu_device_nodes()); +fn collect_baseline_enrichment_paths( + include_proxy: bool, + include_gpu: bool, + gpu_device_nodes: Vec, +) -> (Vec, Vec) { + let mut ro = Vec::new(); + let mut rw = Vec::new(); + + if include_proxy { + for &path in PROXY_BASELINE_READ_ONLY { + push_unique(&mut ro, path.to_string()); + } + for &path in PROXY_BASELINE_READ_WRITE { + push_unique(&mut rw, path.to_string()); + } + } + + if include_gpu { + for &path in GPU_BASELINE_READ_ONLY { + push_unique(&mut ro, path.to_string()); + } + for &path in GPU_BASELINE_READ_WRITE { + push_unique(&mut rw, path.to_string()); + } + for path in gpu_device_nodes { + push_unique(&mut rw, path); + } } // A path promoted to read_write (e.g. /proc for GPU) should not also @@ -1476,14 +1493,33 @@ fn baseline_enrichment_paths() -> (Vec, Vec) { (ro, rw) } -/// Ensure a proto `SandboxPolicy` includes the baseline filesystem paths -/// required for proxy-mode sandboxes. Paths are only added if missing; -/// user-specified paths are never removed. -/// -/// Returns `true` if the policy was modified (caller may want to sync back). -fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) -> bool { - // Only enrich if network_policies are present (proxy mode indicator). - if proto.network_policies.is_empty() { +fn active_baseline_enrichment_paths(include_proxy: bool) -> (Vec, Vec) { + let include_gpu = has_gpu_devices(); + let gpu_device_nodes = if include_gpu { + enumerate_gpu_device_nodes() + } else { + Vec::new() + }; + collect_baseline_enrichment_paths(include_proxy, include_gpu, gpu_device_nodes) +} + +/// Collect all active baseline paths for tests and diagnostics. +/// Returns `(read_only, read_write)` as owned `String` vecs. +#[cfg(test)] +fn baseline_enrichment_paths() -> (Vec, Vec) { + active_baseline_enrichment_paths(true) +} + +fn enrich_proto_baseline_paths_with( + proto: &mut openshell_core::proto::SandboxPolicy, + ro: &[String], + rw: &[String], + path_exists: F, +) -> bool +where + F: Fn(&str) -> bool, +{ + if ro.is_empty() && rw.is_empty() { return false; } @@ -1494,17 +1530,10 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) ..Default::default() }); - let (ro, rw) = baseline_enrichment_paths(); - - // Baseline paths are system-injected, not user-specified. Skip paths - // that do not exist in this container image to avoid noisy warnings from - // Landlock and, more critically, to prevent a single missing baseline - // path from abandoning the entire Landlock ruleset under best-effort - // mode (see issue #664). let mut modified = false; - for path in &ro { + for path in ro { if !fs.read_only.iter().any(|p| p == path) && !fs.read_write.iter().any(|p| p == path) { - if !std::path::Path::new(path).exists() { + if !path_exists(path) { debug!( path, "Baseline read-only path does not exist, skipping enrichment" @@ -1515,11 +1544,11 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) modified = true; } } - for path in &rw { + for path in rw { if fs.read_only.iter().any(|p| p == path) || fs.read_write.iter().any(|p| p == path) { continue; } - if !std::path::Path::new(path).exists() { + if !path_exists(path) { debug!( path, "Baseline read-write path does not exist, skipping enrichment" @@ -1530,6 +1559,26 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) modified = true; } + modified +} + +/// Ensure a proto `SandboxPolicy` includes the baseline filesystem paths +/// required by proxy-mode sandboxes and GPU runtimes. Paths are only added if +/// missing; user-specified paths are never removed. +/// +/// Returns `true` if the policy was modified (caller may want to sync back). +fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) -> bool { + let (ro, rw) = active_baseline_enrichment_paths(!proto.network_policies.is_empty()); + + // Baseline paths are system-injected, not user-specified. Skip paths + // that do not exist in this container image to avoid noisy warnings from + // Landlock and, more critically, to prevent a single missing baseline + // path from abandoning the entire Landlock ruleset under best-effort + // mode (see issue #664). + let modified = enrich_proto_baseline_paths_with(proto, &ro, &rw, |path| { + std::path::Path::new(path).exists() + }); + if modified { ocsf_emit!( ConfigStateChangeBuilder::new(ocsf_ctx()) @@ -1545,15 +1594,15 @@ fn enrich_proto_baseline_paths(proto: &mut openshell_core::proto::SandboxPolicy) } /// Ensure a `SandboxPolicy` (Rust type) includes the baseline filesystem -/// paths required for proxy-mode sandboxes. Used for the local-file code -/// path where no proto is available. +/// paths required by proxy-mode sandboxes and GPU runtimes. Used for the +/// local-file code path where no proto is available. fn enrich_sandbox_baseline_paths(policy: &mut SandboxPolicy) { - if !matches!(policy.network.mode, NetworkMode::Proxy) { + let (ro, rw) = + active_baseline_enrichment_paths(matches!(policy.network.mode, NetworkMode::Proxy)); + if ro.is_empty() && rw.is_empty() { return; } - let (ro, rw) = baseline_enrichment_paths(); - let mut modified = false; for path in &ro { let p = std::path::PathBuf::from(path); @@ -1708,6 +1757,31 @@ mod baseline_tests { ); } + #[test] + fn proto_gpu_enrichment_adds_devices_without_network_policy() { + let mut policy = openshell_policy::restrictive_default_policy(); + assert!( + policy.network_policies.is_empty(), + "regression setup must exercise the no-network default path" + ); + let (ro, rw) = + collect_baseline_enrichment_paths(false, true, vec!["/dev/nvidia0".to_string()]); + + let enriched = enrich_proto_baseline_paths_with(&mut policy, &ro, &rw, |path| { + matches!(path, "/proc" | "/dev/nvidia0") + }); + + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!( + enriched, + "GPU enrichment should not require network policies" + ); + assert!( + filesystem.read_write.contains(&"/dev/nvidia0".to_string()), + "GPU enrichment should add enumerated device nodes without network policies" + ); + } + #[test] fn gpu_baseline_read_write_contains_dxg() { // /dev/dxg must be present so WSL2 sandboxes get the Landlock