diff --git a/Cargo.lock b/Cargo.lock index c30f890914..3446d65916 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,6 +63,7 @@ dependencies = [ "cfg-if", "getrandom 0.3.4", "once_cell", + "serde", "version_check", "zerocopy", ] @@ -127,7 +128,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -138,7 +139,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -735,6 +736,21 @@ dependencies = [ "sha2 0.11.0", ] +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "1.3.2" @@ -841,6 +857,12 @@ dependencies = [ "serde_repr", ] +[[package]] +name = "borrow-or-share" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc0b364ead1874514c8c2855ab558056ebfeb775653e7ae45ff72f28f8f3166c" + [[package]] name = "bstr" version = "1.12.1" @@ -857,6 +879,12 @@ version = "3.20.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d20789868f4b01b2f2caec9f5c4e0213b41e3e5702a50157d699ae31ced2fcb" +[[package]] +name = "bytecount" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" + [[package]] name = "byteorder" version = "1.5.0" @@ -1134,6 +1162,26 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "container-device-interface" +version = "1.2.0" +source = "git+https://github.com/cncf-tags/container-device-interface-rs?rev=b8a056e92dbd159423c129cd306c93e410651b18#b8a056e92dbd159423c129cd306c93e410651b18" +dependencies = [ + "anyhow", + "const_format", + "jsonschema", + "lazy_static", + "libc", + "oci-spec 0.10.0", + "once_cell", + "path-clean", + "regex", + "semver", + "serde", + "serde_json", + "serde_yaml", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -1677,6 +1725,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "email_address" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e079f19b08ca6239f47f8ba8509c11cf3ea30095831f7fed61441475edd8c449" +dependencies = [ + "serde", +] + [[package]] name = "encode_unicode" version = "1.0.0" @@ -1728,7 +1785,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -1753,6 +1810,17 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "fancy-regex" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1e1dacd0d2082dfcf1351c4bdd566bbe89a2b263235a2b50058f1e130a47277" +dependencies = [ + "bit-set", + "regex-automata", + "regex-syntax", +] + [[package]] name = "fastrand" version = "2.4.1" @@ -1808,6 +1876,17 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "fluent-uri" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc74ac4d8359ae70623506d512209619e5cf8f347124910440dbc221714b328e" +dependencies = [ + "borrow-or-share", + "ref-cast", + "serde", +] + [[package]] name = "flume" version = "0.11.1" @@ -1846,6 +1925,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fraction" +version = "0.15.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e076045bb43dac435333ed5f04caf35c7463631d0dae2deb2638d94dd0a5b872" +dependencies = [ + "lazy_static", + "num", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -2145,6 +2234,8 @@ version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" dependencies = [ + "allocator-api2", + "equivalent", "foldhash 0.2.0", ] @@ -2443,7 +2534,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.6.3", + "socket2 0.5.10", "tokio", "tower-service", "tracing", @@ -2860,6 +2951,42 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "jsonschema" +version = "0.46.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0a699d3e77675e6aa4bfffe3b907c8b5f7ed3241f9965bffb25475ad4b08d05" +dependencies = [ + "ahash", + "bytecount", + "data-encoding", + "email_address", + "fancy-regex", + "fraction", + "getrandom 0.3.4", + "idna", + "itoa", + "jsonschema-regex", + "num-cmp", + "num-traits", + "percent-encoding", + "referencing", + "regex", + "serde", + "serde_json", + "unicode-general-category", + "uuid-simd", +] + +[[package]] +name = "jsonschema-regex" +version = "0.46.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6dbd1086b01b9349fd4ef9a07433965af64c8ce8159abe633a189e4ff817bd13" +dependencies = [ + "regex-syntax", +] + [[package]] name = "jsonwebtoken" version = "10.3.0" @@ -3263,6 +3390,12 @@ dependencies = [ "sketches-ddsketch", ] +[[package]] +name = "micromap" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a86d3146ed3995b5913c414f6664344b9617457320782e64f0bb44afd49d74" + [[package]] name = "miette" version = "7.6.0" @@ -3449,6 +3582,20 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "num" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + [[package]] name = "num-bigint" version = "0.4.6" @@ -3475,6 +3622,21 @@ dependencies = [ "zeroize", ] +[[package]] +name = "num-cmp" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63335b2e2c34fae2fb0aa2cecfd9f0832a1e24b3b32ecec612c3426d46dc8aaa" + +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.2.1" @@ -3501,6 +3663,17 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -3533,7 +3706,7 @@ version = "5.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "51e219e79014df21a225b1860a479e2dcd7cbd9130f4defd4bd0e191ea31d67d" dependencies = [ - "base64 0.22.1", + "base64 0.21.7", "chrono", "getrandom 0.2.17", "http 1.4.0", @@ -3569,7 +3742,7 @@ dependencies = [ "http-auth", "jsonwebtoken", "lazy_static", - "oci-spec", + "oci-spec 0.9.0", "olpc-cjson", "regex", "reqwest 0.13.2", @@ -3599,6 +3772,23 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "oci-spec" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3df6f876ad774d6a676f7e968f5c3edacc32f90e65fe680a8b686235396556fb" +dependencies = [ + "const_format", + "derive_builder", + "getset", + "regex", + "serde", + "serde_json", + "strum 0.27.2", + "strum_macros 0.27.2", + "thiserror 2.0.18", +] + [[package]] name = "oid-registry" version = "0.7.1" @@ -3708,6 +3898,7 @@ dependencies = [ "async-trait", "base64 0.22.1", "chrono", + "container-device-interface", "glob", "ipnet", "miette", @@ -4538,6 +4729,12 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" +[[package]] +name = "path-clean" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17359afc20d7ab31fdb42bb844c8b3bb1dabd7dcf7e68428492da7f16966fcef" + [[package]] name = "pbkdf2" version = "0.13.0" @@ -5076,7 +5273,7 @@ dependencies = [ "quinn-udp", "rustc-hash 2.1.2", "rustls 0.23.38", - "socket2 0.6.3", + "socket2 0.5.10", "thiserror 2.0.18", "tokio", "tracing", @@ -5114,9 +5311,9 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.6.3", + "socket2 0.5.10", "tracing", - "windows-sys 0.60.2", + "windows-sys 0.59.0", ] [[package]] @@ -5285,6 +5482,43 @@ dependencies = [ "bitflags 2.11.1", ] +[[package]] +name = "ref-cast" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "216e8f773d7923bcba9ceb86a86c93cabb3903a11872fc3f138c49630e50b96d" +dependencies = [ + "ref-cast-impl", +] + +[[package]] +name = "ref-cast-impl" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c9283685feec7d69af75fb0e858d5e7378f33fe4fc699383b2916ab9273e03c" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", +] + +[[package]] +name = "referencing" +version = "0.46.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fbf332a2f81899f6836f22c03da73dae8a664c32e3016b84692c23cddadc95d" +dependencies = [ + "ahash", + "fluent-uri", + "getrandom 0.3.4", + "hashbrown 0.16.1", + "itoa", + "micromap", + "parking_lot", + "percent-encoding", + "serde_json", +] + [[package]] name = "regex" version = "1.12.3" @@ -5650,7 +5884,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -5730,7 +5964,7 @@ dependencies = [ "security-framework", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -6260,7 +6494,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a766e1110788c36f4fa1c2b71b387a7815aa65f88ce0229841826633d93723e" dependencies = [ "libc", - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -6707,6 +6941,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "1.0.2" @@ -6757,7 +7002,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix 1.1.4", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] @@ -7407,6 +7652,12 @@ version = "0.3.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c1cb5db39152898a79168971543b1cb5020dff7fe43c8dc468b0885f5e29df5" +[[package]] +name = "unicode-general-category" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b993bddc193ae5bd0d623b49ec06ac3e9312875fdae725a975c51db1cc1677f" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -7545,6 +7796,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "uuid-simd" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b082222b4f6619906941c17eb2297fff4c2fb96cb60164170522942a200bd8" +dependencies = [ + "outref", + "vsimd", +] + [[package]] name = "valuable" version = "0.1.1" @@ -7790,7 +8051,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 150df10d69..08b4ce3276 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,6 +78,7 @@ rustix = { version = "1.1", features = ["process"] } socket2 = "0.6" # Serialization +container-device-interface = { git = "https://github.com/cncf-tags/container-device-interface-rs", rev = "b8a056e92dbd159423c129cd306c93e410651b18" } serde = { version = "1", features = ["derive"] } serde_json = "1" serde_yml = "0.0.12" diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index d22b4ef677..6fb5c444e0 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -182,6 +182,18 @@ Resource requirements enter the driver layer through `SandboxSpec.resource_requi can request a specific number of GPUs or the driver-specific default behaviour. For all in-tree drivers, this is equivalent to selecting a single GPU. +For Docker GPU sandboxes, the driver treats CDI specs as runtime metadata for +both outer injection and inner sandbox policy. It selects opaque CDI device IDs, +passes them to Docker, mounts daemon-reported CDI spec directories into +supervisor-only paths, and bind-mounts a gateway-owned versioned CDI context +read-only before creating the container. The supervisor resolves that context +inside the sandbox and derives Landlock paths and supplemental groups from CDI +`containerEdits`. Host-side CDI spec paths are diagnostic only and are never +treated as sandbox policy paths. +Kubernetes must not infer CDI device IDs from the `nvidia.com/gpu` resource +request; it needs a node-local selected-device handoff before using the same +supervisor resolver. + VM runtime state paths are derived only from driver-validated sandbox IDs matching `[A-Za-z0-9._-]{1,128}`. The gateway-owned VM driver socket uses a private `run/` directory plus Unix peer UID/PID checks. Standalone diff --git a/architecture/security-policy.md b/architecture/security-policy.md index 0cc3aa618d..1e227a1647 100644 --- a/architecture/security-policy.md +++ b/architecture/security-policy.md @@ -23,10 +23,20 @@ dynamic and can be hot-reloaded when the new policy validates successfully. Before applying Landlock, the supervisor enriches baseline filesystem paths that the runtime needs. Missing baseline paths are skipped so one absent runtime path -does not weaken the whole ruleset. When GPU devices are present, GPU baseline -enrichment adds existing GPU device nodes as read-write paths and promotes -`/proc` to read-write because CUDA workloads write thread metadata under -`/proc//task//comm`. +does not weaken the whole ruleset. When GPU devices are present without a CDI +context, GPU baseline enrichment adds existing GPU device nodes as read-write +paths. GPU sandboxes with CDI context use CDI-derived paths instead of the +hard-coded GPU baseline. Both paths promote `/proc` to read-write because CUDA +workloads write thread metadata under `/proc//task//comm`. + +GPU/CDI sandboxes can also carry a supervisor-only CDI context from the compute +driver. The supervisor resolves selected CDI IDs from mounted CDI specs and +adds derived device nodes, library mount destinations, and supplemental GIDs +before agent exec. CDI host paths are ignored for policy. Derived mount +destinations default to read-only; writable CDI single-file mounts require an +exact `filesystem_policy.read_write` opt-in, and writable CDI directory mounts +fail closed. CDI resolution errors are security-relevant startup failures and +emit OCSF findings. Landlock rules are tailored to the inode type reported by the already-opened path descriptor. Directories retain the requested directory and file rights; diff --git a/crates/openshell-core/Cargo.toml b/crates/openshell-core/Cargo.toml index 8b28fafa23..6afcc1c812 100644 --- a/crates/openshell-core/Cargo.toml +++ b/crates/openshell-core/Cargo.toml @@ -36,6 +36,9 @@ tempfile = { version = "3", optional = true } [target.'cfg(unix)'.dependencies] nix = { workspace = true } +[target.'cfg(target_os = "linux")'.dependencies] +container-device-interface = { workspace = true } + [features] default = ["telemetry"] ## Compile in anonymous telemetry emission support. On by default; disable with diff --git a/crates/openshell-core/src/cdi.rs b/crates/openshell-core/src/cdi.rs new file mode 100644 index 0000000000..d71059d352 --- /dev/null +++ b/crates/openshell-core/src/cdi.rs @@ -0,0 +1,146 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared CDI context schema and resolver helpers. + +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +pub const CDI_CONTEXT_VERSION: u32 = 1; + +/// File name used for the serialized CDI context. +pub const CDI_CONTEXT_FILE_NAME: &str = "cdi-context.json"; + +/// Absolute supervisor path for the CDI context file mounted by a compute driver. +pub const CDI_CONTEXT_PATH: &str = "/run/openshell/supervisor/cdi-context.json"; + +/// Base supervisor path under which compute drivers mount CDI specification directories. +pub const CDI_SPEC_DIR_BASE: &str = "/run/openshell/supervisor/cdi-specs"; + +/// Return the supervisor path used for a CDI specification directory. +#[must_use] +pub fn cdi_spec_mount_path(index: usize) -> String { + format!("{CDI_SPEC_DIR_BASE}/{index}") +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CdiContext { + pub version: u32, + pub selected_devices: Vec, + pub spec_dirs: Vec, +} + +impl CdiContext { + #[must_use] + pub fn new(selected_devices: Vec, spec_dirs: Vec) -> Self { + Self { + version: CDI_CONTEXT_VERSION, + selected_devices, + spec_dirs, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CdiSpecDirectory { + pub path: String, + pub source: String, +} + +impl CdiSpecDirectory { + #[must_use] + pub fn new(path: impl Into, source: impl Into) -> Self { + Self { + path: path.into(), + source: source.into(), + } + } +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct CdiDerivedRequirements { + pub device_node_paths: Vec, + pub read_only_mount_paths: Vec, + pub read_write_mount_paths: Vec, + pub additional_gids: Vec, +} + +#[derive(Debug, thiserror::Error)] +pub enum CdiError { + #[error("CDI policy resolution is unavailable on this platform")] + UnsupportedPlatform, + #[error("failed to read CDI context '{}': {source}", path.display())] + ContextRead { + path: PathBuf, + source: std::io::Error, + }, + #[error("failed to parse CDI context '{}': {source}", path.display())] + ContextParse { + path: PathBuf, + source: serde_json::Error, + }, + #[error("unsupported CDI context version {0}")] + UnsupportedContextVersion(u32), + #[error("CDI spec dir '{path}' from source '{diagnostic_source}' is unsafe: {reason}")] + UnsafeSpecDir { + path: String, + diagnostic_source: String, + reason: &'static str, + }, + #[error("selected CDI device '{0}' was not found in mounted CDI specs")] + MissingDevice(String), + #[error( + "selected CDI device '{device}' was not found in mounted CDI specs after CDI spec refresh reported: {refresh_error}" + )] + MissingDeviceAfterRefresh { + device: String, + refresh_error: String, + }, + #[error("failed to merge CDI edits for '{device}': {error}")] + EditMerge { device: String, error: String }, + #[error("failed to encode resolved CDI edits: {source}")] + EditEncode { source: serde_json::Error }, + #[error("failed to decode resolved CDI edits: {source}")] + EditDecode { source: serde_json::Error }, + #[error("CDI-derived path '{path}' is unsafe: {reason}")] + UnsafePolicyPath { path: String, reason: &'static str }, + #[error("CDI path '{path}' requested conflicting access modes")] + ConflictingAccess { path: String }, + #[error( + "CDI writable mount '{path}' is not explicitly listed in the sandbox policy read_write paths" + )] + WritableMountNotAllowed { path: String }, + #[error("CDI writable mount '{path}' must target a single file, found {kind}")] + WritableMountNotFile { path: String, kind: String }, + #[error("CDI device node '{path}' must target a character or block device, found {kind}")] + DeviceNodeNotDevice { path: String, kind: String }, + #[error("CDI additionalGids must not contain root GID 0")] + RootAdditionalGid, + #[error("CDI mount '{path}' has conflicting ro/rw options")] + ConflictingMountOptions { path: String }, +} + +pub fn read_context(path: impl AsRef) -> Result { + let path = path.as_ref(); + let json = std::fs::read_to_string(path).map_err(|source| CdiError::ContextRead { + path: path.to_path_buf(), + source, + })?; + serde_json::from_str(&json).map_err(|source| CdiError::ContextParse { + path: path.to_path_buf(), + source, + }) +} + +#[cfg(target_os = "linux")] +#[path = "cdi_linux.rs"] +mod cdi_linux; +#[cfg(target_os = "linux")] +pub use cdi_linux::resolve_cdi_context; + +#[cfg(not(target_os = "linux"))] +#[path = "cdi_stub.rs"] +mod cdi_stub; +#[cfg(not(target_os = "linux"))] +pub use cdi_stub::resolve_cdi_context; diff --git a/crates/openshell-core/src/cdi_linux.rs b/crates/openshell-core/src/cdi_linux.rs new file mode 100644 index 0000000000..78f2974041 --- /dev/null +++ b/crates/openshell-core/src/cdi_linux.rs @@ -0,0 +1,881 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Linux implementation of CDI policy resolution. + +use super::{CDI_CONTEXT_VERSION, CdiContext, CdiDerivedRequirements, CdiError, CdiSpecDirectory}; +use crate::paths::normalize_path; +use container_device_interface::{ + cache::{Cache, with_auto_refresh}, + container_edits::ContainerEdits as UpstreamContainerEdits, + spec_dirs::with_spec_dirs, +}; +use serde::Deserialize; +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::fmt; +use std::hash::BuildHasher; +use std::os::unix::fs::FileTypeExt; +use std::path::{Component, Path}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CdiPathKind { + File, + Directory, + CharacterDevice, + BlockDevice, + Other, +} + +impl fmt::Display for CdiPathKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::File => f.write_str("file"), + Self::Directory => f.write_str("directory"), + Self::CharacterDevice => f.write_str("character device"), + Self::BlockDevice => f.write_str("block device"), + Self::Other => f.write_str("other"), + } + } +} + +// Temporary view over upstream-resolved CDI edits. The Rust CDI crate currently +// keeps some spec-model fields crate-private even though they are serialized and +// public in specs-go, so OpenShell serializes the merged upstream model and +// decodes only the policy-relevant fields here. +#[derive(Debug, Default, Deserialize)] +struct CdiContainerEdits { + #[serde(default, rename = "deviceNodes")] + device_nodes: Vec, + #[serde(default)] + mounts: Vec, + #[serde(default, rename = "additionalGids")] + additional_gids: Vec, +} + +#[derive(Debug, Deserialize)] +struct CdiDeviceNode { + path: String, +} + +#[derive(Debug, Deserialize)] +struct CdiMount { + #[serde(rename = "containerPath")] + container_path: String, + #[serde(default)] + options: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CdiAccess { + ReadOnly, + ReadWrite, +} + +#[derive(Debug, Default)] +struct RequirementAccumulator { + device_node_paths: BTreeSet, + mount_paths: BTreeMap, + additional_gids: BTreeSet, +} + +impl RequirementAccumulator { + fn add_device_node(&mut self, path: String, path_kind: &F) -> Result<(), CdiError> + where + F: Fn(&str) -> Option, + { + let kind = path_kind(&path); + if !matches!( + kind, + Some(CdiPathKind::CharacterDevice | CdiPathKind::BlockDevice) + ) { + return Err(CdiError::DeviceNodeNotDevice { + path, + kind: kind.map_or_else(|| "missing".to_string(), |kind| kind.to_string()), + }); + } + match self.mount_paths.get(&path).copied() { + Some(CdiAccess::ReadOnly) => Err(CdiError::ConflictingAccess { path }), + Some(CdiAccess::ReadWrite) | None => { + self.device_node_paths.insert(path); + Ok(()) + } + } + } + + fn add_mount(&mut self, path: String, access: CdiAccess) -> Result<(), CdiError> { + if access == CdiAccess::ReadOnly && self.device_node_paths.contains(&path) { + return Err(CdiError::ConflictingAccess { path }); + } + match self.mount_paths.get(&path).copied() { + Some(existing) if existing != access => Err(CdiError::ConflictingAccess { path }), + Some(_) => Ok(()), + None => { + self.mount_paths.insert(path, access); + Ok(()) + } + } + } + + fn add_gid(&mut self, gid: u32) -> Result<(), CdiError> { + if gid == 0 { + return Err(CdiError::RootAdditionalGid); + } + self.additional_gids.insert(gid); + Ok(()) + } + + fn validate_writable_mounts( + &self, + normalized_writable_file_allowlist: &HashSet, + path_kind: &F, + ) -> Result<(), CdiError> + where + F: Fn(&str) -> Option, + { + for (path, access) in &self.mount_paths { + if *access != CdiAccess::ReadWrite { + continue; + } + if !normalized_writable_file_allowlist.contains(path) { + return Err(CdiError::WritableMountNotAllowed { path: path.clone() }); + } + let kind = path_kind(path); + if kind != Some(CdiPathKind::File) { + return Err(CdiError::WritableMountNotFile { + path: path.clone(), + kind: kind.map_or_else(|| "missing".to_string(), |kind| kind.to_string()), + }); + } + } + Ok(()) + } + + fn build(self) -> CdiDerivedRequirements { + let mut read_only_mount_paths = Vec::new(); + let mut read_write_mount_paths = Vec::new(); + for (path, access) in self.mount_paths { + match access { + CdiAccess::ReadOnly => read_only_mount_paths.push(path), + CdiAccess::ReadWrite => read_write_mount_paths.push(path), + } + } + CdiDerivedRequirements { + device_node_paths: self.device_node_paths.into_iter().collect(), + read_only_mount_paths, + read_write_mount_paths, + additional_gids: self.additional_gids.into_iter().collect(), + } + } +} + +pub fn resolve_cdi_context( + context: &CdiContext, + writable_file_allowlist: &HashSet, +) -> Result { + resolve_cdi_context_with_path_kind(context, writable_file_allowlist, filesystem_path_kind) +} + +fn resolve_cdi_context_with_path_kind( + context: &CdiContext, + writable_file_allowlist: &HashSet, + path_kind: F, +) -> Result +where + F: Fn(&str) -> Option, + S: BuildHasher, +{ + validate_context(context)?; + let selected_devices = selected_cdi_devices(&context.selected_devices); + if selected_devices.is_empty() || context.spec_dirs.is_empty() { + return Ok(CdiDerivedRequirements::default()); + } + + let normalized_allowlist = writable_file_allowlist + .iter() + .map(|path| normalize_path(path)) + .collect::>(); + + let edits = resolve_container_edits(context, &selected_devices)?; + let mut accumulator = RequirementAccumulator::default(); + accumulate_requirements(&edits, &normalized_allowlist, &path_kind, &mut accumulator)?; + Ok(accumulator.build()) +} + +fn validate_context(context: &CdiContext) -> Result<(), CdiError> { + if context.version != CDI_CONTEXT_VERSION { + return Err(CdiError::UnsupportedContextVersion(context.version)); + } + for spec_dir in &context.spec_dirs { + validate_absolute_no_parent(&spec_dir.path).map_err(|reason| CdiError::UnsafeSpecDir { + path: spec_dir.path.clone(), + diagnostic_source: spec_dir.source.clone(), + reason, + })?; + } + Ok(()) +} + +fn selected_cdi_devices(device_ids: &[String]) -> Vec { + let mut seen = HashSet::new(); + let mut parsed = Vec::new(); + for raw in device_ids { + let raw = raw.trim(); + if raw.is_empty() { + continue; + } + if seen.insert(raw.to_string()) { + parsed.push(raw.to_string()); + } + } + parsed +} + +fn resolve_container_edits( + context: &CdiContext, + selected_devices: &[String], +) -> Result { + let (mut cache, refresh_error) = build_cache(&context.spec_dirs); + let mut merged = UpstreamContainerEdits::new(); + let mut applied_specs = BTreeSet::new(); + + for device_id in selected_devices { + let device = cache + .get_device(device_id) + .cloned() + .ok_or_else(|| missing_device_error(device_id, refresh_error.as_deref()))?; + let mut spec = device.get_spec(); + let spec_path = spec.get_path(); + if applied_specs.insert(spec_path) + && let Some(spec_edits) = spec.edits() + { + merged + .append(spec_edits) + .map_err(|err| CdiError::EditMerge { + device: device_id.clone(), + error: err.to_string(), + })?; + } + merged + .append(device.edits()) + .map_err(|err| CdiError::EditMerge { + device: device_id.clone(), + error: err.to_string(), + })?; + } + + let value = serde_json::to_value(&merged.container_edits) + .map_err(|source| CdiError::EditEncode { source })?; + serde_json::from_value(value).map_err(|source| CdiError::EditDecode { source }) +} + +fn missing_device_error(device: &str, refresh_error: Option<&str>) -> CdiError { + refresh_error.map_or_else( + || CdiError::MissingDevice(device.to_string()), + |refresh_error| CdiError::MissingDeviceAfterRefresh { + device: device.to_string(), + refresh_error: refresh_error.to_string(), + }, + ) +} + +fn build_cache(spec_dirs: &[CdiSpecDirectory]) -> (Cache, Option) { + let spec_dir_paths = spec_dirs + .iter() + .map(|spec_dir| spec_dir.path.as_str()) + .collect::>(); + let mut cache = Cache::default(); + cache.configure(vec![ + with_spec_dirs(&spec_dir_paths), + with_auto_refresh(false), + ]); + let refresh_error = cache.refresh().err().map(|err| { + tracing::debug!( + error = %err, + "Ignoring CDI cache refresh error; requested device lookup will determine availability" + ); + err.to_string() + }); + (cache, refresh_error) +} + +fn accumulate_requirements( + edits: &CdiContainerEdits, + normalized_writable_file_allowlist: &HashSet, + path_kind: &F, + accumulator: &mut RequirementAccumulator, +) -> Result<(), CdiError> +where + F: Fn(&str) -> Option, +{ + for device_node in &edits.device_nodes { + let path = normalize_policy_path(&device_node.path)?; + accumulator.add_device_node(path, path_kind)?; + } + + for mount in &edits.mounts { + let path = normalize_policy_path(&mount.container_path)?; + let access = mount_access(&path, &mount.options)?; + accumulator.add_mount(path, access)?; + } + + for gid in &edits.additional_gids { + accumulator.add_gid(*gid)?; + } + + accumulator.validate_writable_mounts(normalized_writable_file_allowlist, path_kind)?; + Ok(()) +} + +fn mount_access(path: &str, options: &[String]) -> Result { + let read_only_requested = options + .iter() + .any(|option| option.eq_ignore_ascii_case("ro")); + let read_write_requested = options + .iter() + .any(|option| option.eq_ignore_ascii_case("rw")); + // CDI mount options are stringly typed and runtime-specific normalization + // can differ. Treat simultaneous ro/rw as malformed instead of guessing + // which option a later mount syscall would effectively apply. + if read_only_requested && read_write_requested { + return Err(CdiError::ConflictingMountOptions { + path: path.to_string(), + }); + } + if read_write_requested { + Ok(CdiAccess::ReadWrite) + } else { + Ok(CdiAccess::ReadOnly) + } +} + +fn normalize_policy_path(path: &str) -> Result { + validate_absolute_no_parent(path).map_err(|reason| CdiError::UnsafePolicyPath { + path: path.to_string(), + reason, + })?; + let normalized = normalize_path(path); + if matches!( + normalized.as_str(), + "/" | "/dev" | "/proc" | "/sys" | "/run" | "/usr" + ) { + return Err(CdiError::UnsafePolicyPath { + path: path.to_string(), + reason: "broad root path is not allowed", + }); + } + Ok(normalized) +} + +fn validate_absolute_no_parent(path: &str) -> Result<(), &'static str> { + let path = Path::new(path); + if !path.is_absolute() { + return Err("path must be absolute"); + } + for component in path.components() { + match component { + Component::ParentDir => return Err("path must not contain '..'"), + Component::Prefix(_) => return Err("path must be a Unix-style absolute path"), + Component::CurDir => return Err("path must be normalized"), + Component::RootDir | Component::Normal(_) => {} + } + } + Ok(()) +} + +fn filesystem_path_kind(path: &str) -> Option { + let metadata = std::fs::metadata(path).ok()?; + let file_type = metadata.file_type(); + if file_type.is_char_device() { + return Some(CdiPathKind::CharacterDevice); + } + if file_type.is_block_device() { + return Some(CdiPathKind::BlockDevice); + } + if file_type.is_file() { + Some(CdiPathKind::File) + } else if file_type.is_dir() { + Some(CdiPathKind::Directory) + } else { + Some(CdiPathKind::Other) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write_spec(dir: &Path, name: &str, yaml: &str) { + std::fs::write(dir.join(name), yaml).unwrap(); + } + + fn context(dir: &Path, selected_devices: &[&str]) -> CdiContext { + CdiContext::new( + selected_devices + .iter() + .map(|device| (*device).to_string()) + .collect(), + vec![CdiSpecDirectory::new( + dir.to_string_lossy().into_owned(), + "/host/cdi", + )], + ) + } + + fn resolve_with_kind( + context: &CdiContext, + writable: &[&str], + kind: impl Fn(&str) -> Option, + ) -> Result { + let writable_file_allowlist: HashSet = + writable.iter().map(|path| (*path).to_string()).collect(); + resolve_cdi_context_with_path_kind(context, &writable_file_allowlist, kind) + } + + fn always_missing(_: &str) -> Option { + None + } + + fn fake_device_node(path: &str) -> Option { + path.starts_with("/dev/") + .then_some(CdiPathKind::CharacterDevice) + } + + #[test] + fn resolves_native_single_device_requirements() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r#" +cdiVersion: 0.6.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/nvidiactl + - path: /dev/nvidia0 + mounts: + - hostPath: /host/libcuda.so.1 + containerPath: /usr/local/cuda/lib64/libcuda.so.1 +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + fake_device_node, + ) + .unwrap(); + + assert_eq!( + requirements.device_node_paths, + vec!["/dev/nvidia0", "/dev/nvidiactl"] + ); + assert_eq!( + requirements.read_only_mount_paths, + vec!["/usr/local/cuda/lib64/libcuda.so.1"] + ); + } + + #[test] + fn resolves_native_all_device_requirements() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: all + containerEdits: + deviceNodes: + - path: /dev/nvidiactl + - path: /dev/nvidia0 + - path: /dev/nvidia1 +", + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=all"]), + &[], + fake_device_node, + ) + .unwrap(); + + assert_eq!( + requirements.device_node_paths, + vec!["/dev/nvidia0", "/dev/nvidia1", "/dev/nvidiactl"] + ); + } + + #[test] + fn resolves_wsl_shape_requirements() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "wsl.yaml", + r" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: all + containerEdits: + deviceNodes: + - path: /dev/dxg + mounts: + - hostPath: /host/wsl/lib/libcuda.so.1 + containerPath: /usr/lib/wsl/lib/libcuda.so.1 +", + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=all"]), + &[], + fake_device_node, + ) + .unwrap(); + + assert_eq!(requirements.device_node_paths, vec!["/dev/dxg"]); + assert_eq!( + requirements.read_only_mount_paths, + vec!["/usr/lib/wsl/lib/libcuda.so.1"] + ); + } + + #[test] + fn resolves_tegra_shape_requirements_and_gids() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "tegra.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +containerEdits: + additionalGids: [44] +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/nvmap + - path: /dev/nvhost-gpu +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + fake_device_node, + ) + .unwrap(); + + assert_eq!(requirements.additional_gids, vec![44]); + assert_eq!( + requirements.device_node_paths, + vec!["/dev/nvhost-gpu", "/dev/nvmap"] + ); + } + + #[test] + fn accepts_writable_single_file_mount_with_explicit_policy_opt_in() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "rw-file.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + mounts: + - hostPath: /host/nvidia/cache.db + containerPath: /opt/nvidia/cache.db + options: [rw] +"#, + ); + + let requirements = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &["/opt/nvidia/cache.db"], + |path| (path == "/opt/nvidia/cache.db").then_some(CdiPathKind::File), + ) + .unwrap(); + + assert_eq!( + requirements.read_write_mount_paths, + vec!["/opt/nvidia/cache.db"] + ); + } + + #[test] + fn rejects_writable_directory_mount() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "rw-dir.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + mounts: + - hostPath: /host/nvidia/cache + containerPath: /opt/nvidia/cache + options: [rw] +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &["/opt/nvidia/cache"], + |path| (path == "/opt/nvidia/cache").then_some(CdiPathKind::Directory), + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::WritableMountNotFile { .. })); + } + + #[test] + fn rejects_missing_selected_device() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + env: + - OPEN_SHELL_TEST=1 +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=1"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::MissingDevice(device) if device == "nvidia.com/gpu=1")); + } + + #[test] + fn reports_refresh_errors_when_requested_device_is_missing() { + let dir = tempfile::tempdir().unwrap(); + write_spec(dir.path(), "broken.yaml", "kind: ["); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + fake_device_node, + ) + .unwrap_err(); + + assert!( + matches!(err, CdiError::MissingDeviceAfterRefresh { device, refresh_error } + if device == "nvidia.com/gpu=0" && !refresh_error.is_empty()) + ); + } + + #[test] + fn rejects_duplicate_cdi_device_names() { + let dir = tempfile::tempdir().unwrap(); + for (name, value) in [("first.yaml", "FIRST"), ("second.yaml", "SECOND")] { + write_spec( + dir.path(), + name, + &format!( + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + env: + - OPEN_SHELL_TEST={value} +"# + ), + ); + } + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!( + matches!(err, CdiError::MissingDeviceAfterRefresh { device, refresh_error } + if device == "nvidia.com/gpu=0" + && refresh_error.contains("conflicting device nvidia.com/gpu=0")) + ); + } + + #[test] + fn empty_selection_is_noop() { + let dir = tempfile::tempdir().unwrap(); + let requirements = resolve_with_kind(&context(dir.path(), &[]), &[], always_missing) + .expect("empty selection should resolve to empty requirements"); + + assert_eq!(requirements, CdiDerivedRequirements::default()); + } + + #[test] + fn empty_spec_dirs_are_noop() { + let context = CdiContext::new(vec!["nvidia.com/gpu=0".to_string()], Vec::new()); + + let requirements = resolve_with_kind(&context, &[], always_missing) + .expect("empty spec dirs should resolve to empty requirements"); + + assert_eq!(requirements, CdiDerivedRequirements::default()); + } + + #[test] + fn defers_device_id_shape_to_upstream_resolution() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "nvidia.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + env: + - OPEN_SHELL_TEST=1 +"#, + ); + + let err = resolve_with_kind(&context(dir.path(), &["not-a-cdi-id"]), &[], always_missing) + .unwrap_err(); + + assert!(matches!(err, CdiError::MissingDevice(device) if device == "not-a-cdi-id")); + } + + #[test] + fn rejects_unsafe_policy_paths() { + for path in [ + "relative", "/dev", "/proc", "/sys", "/run", "/usr", "/a/../b", + ] { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "unsafe.yaml", + &format!( + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: {path} +"# + ), + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!( + matches!(err, CdiError::UnsafePolicyPath { .. }), + "expected unsafe path error for {path}, got {err:?}" + ); + } + } + + #[test] + fn rejects_duplicate_access_conflicts() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "conflict.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /dev/nvidia0 + mounts: + - hostPath: /host/dev/nvidia0 + containerPath: /dev/nvidia0 + options: [ro] +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + fake_device_node, + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::ConflictingAccess { .. })); + } + + #[test] + fn rejects_root_additional_gid() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "root-gid.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + additionalGids: [0, 44] +"#, + ); + + let err = resolve_with_kind( + &context(dir.path(), &["nvidia.com/gpu=0"]), + &[], + always_missing, + ) + .unwrap_err(); + + assert!(matches!(err, CdiError::RootAdditionalGid)); + } + + #[test] + fn rejects_device_node_that_is_not_device() { + let dir = tempfile::tempdir().unwrap(); + write_spec( + dir.path(), + "regular-file-node.yaml", + r#" +cdiVersion: 1.1.0 +kind: nvidia.com/gpu +devices: + - name: "0" + containerEdits: + deviceNodes: + - path: /opt/nvidia/not-a-device +"#, + ); + + let err = resolve_with_kind(&context(dir.path(), &["nvidia.com/gpu=0"]), &[], |path| { + (path == "/opt/nvidia/not-a-device").then_some(CdiPathKind::File) + }) + .unwrap_err(); + + assert!(matches!(err, CdiError::DeviceNodeNotDevice { .. })); + } +} diff --git a/crates/openshell-core/src/cdi_stub.rs b/crates/openshell-core/src/cdi_stub.rs new file mode 100644 index 0000000000..552e5ad8a1 --- /dev/null +++ b/crates/openshell-core/src/cdi_stub.rs @@ -0,0 +1,16 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Fail-closed CDI policy resolution for platforms other than Linux. + +use std::collections::HashSet; +use std::hash::BuildHasher; + +use super::{CdiContext, CdiDerivedRequirements, CdiError}; + +pub fn resolve_cdi_context( + _context: &CdiContext, + _writable_file_allowlist: &HashSet, +) -> Result { + Err(CdiError::UnsupportedPlatform) +} diff --git a/crates/openshell-core/src/lib.rs b/crates/openshell-core/src/lib.rs index d373d656ed..ff3a550d7c 100644 --- a/crates/openshell-core/src/lib.rs +++ b/crates/openshell-core/src/lib.rs @@ -11,6 +11,7 @@ pub mod activity; pub mod auth; +pub mod cdi; pub mod config; pub mod container_paths; pub mod denial; diff --git a/crates/openshell-core/src/policy.rs b/crates/openshell-core/src/policy.rs index 1645b9da44..30c19ca4dc 100644 --- a/crates/openshell-core/src/policy.rs +++ b/crates/openshell-core/src/policy.rs @@ -83,6 +83,12 @@ pub struct ProcessPolicy { /// Group name to run the sandboxed process as. pub run_as_group: Option, + + /// Linux supplemental groups to apply before dropping privileges. + /// + /// Runtime-specific inputs can use different terminology; CDI + /// `additionalGids` are converted into this process-level representation. + pub supplemental_groups: Vec, } #[derive(Debug, Clone, Default)] @@ -162,6 +168,7 @@ impl From for ProcessPolicy { } else { Some(proto.run_as_group) }, + supplemental_groups: Vec::new(), } } } diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index 1549258fa3..fe13efc18b 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -85,6 +85,14 @@ pub const SANDBOX_TOKEN_FILE: &str = "OPENSHELL_SANDBOX_TOKEN_FILE"; /// SSH child processes (which use `env_clear()` for security isolation). pub const USER_ENVIRONMENT: &str = "OPENSHELL_USER_ENVIRONMENT"; +/// Path to the supervisor-only CDI context JSON file. +/// +/// Set by CDI-capable compute drivers only when a sandbox requested GPU/CDI +/// devices. The sandbox supervisor reads this file before policy construction +/// and derives inner filesystem and process requirements from mounted CDI +/// specs. +pub const CDI_CONTEXT: &str = "OPENSHELL_CDI_CONTEXT"; + /// Path to the projected `ServiceAccount` JWT (Kubernetes driver). /// /// Used to bootstrap a gateway-minted JWT via `IssueSandboxToken`. Kubelet diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index fe705d0b50..90d9399a62 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -91,9 +91,35 @@ contract: | `restart_policy = unless-stopped` | Keeps managed sandboxes resumable across daemon or gateway restarts. | | `PidsLimit` | Enforces the sandbox PID budget at the Docker cgroup layer. Set `[openshell.drivers.docker].sandbox_pids_limit = 0` to inherit the Docker/runtime default. | | CDI GPU request | Uses opaque `driver_config.cdi_devices` values when set; otherwise selects the requested count of NVIDIA CDI GPUs in round-robin order when daemon CDI support is detected. Docker daemon `/info` can permit `nvidia.com/gpu=all` as a WSL2 all-only compatibility fallback, where it counts as one selectable device. Exact CDI device lists must not contain duplicates and must match the effective GPU count. | +| CDI context mount | For GPU/CDI sandboxes only, creates a gateway-owned context file and bind-mounts it read-only at `/run/openshell/supervisor/cdi-context.json`; daemon-reported CDI spec directories are mounted read-only under `/run/openshell/supervisor/cdi-specs/`. | The agent child process does not retain these supervisor privileges. +## CDI GPU Metadata + +Docker remains the source of truth for GPU injection. The driver selects opaque +CDI device IDs from `driver_config.cdi_devices` or the daemon's discovered CDI +inventory, then passes the same IDs to Docker with a CDI `DeviceRequest`. + +When a GPU/CDI request is present, the driver also mounts the Docker +daemon-reported `Info.CDISpecDirs` into supervisor-only paths. Before container +creation, it writes a small versioned CDI context in gateway-owned state and +bind-mounts it read-only into the supervisor. The context uses container-side +spec paths for resolution and keeps host-side spec sources diagnostic-only. If +context or token creation fails, the driver removes any created state files; if +container creation or start fails, it also removes the container and state +files before reporting the failure. + +The sandbox supervisor resolves the selected IDs from those mounted specs +before it launches agent processes. CDI device nodes become read-write +Landlock paths, mount destinations default to read-only paths, and +`additionalGids` become supplemental groups for the entrypoint and SSH child +processes. Writable CDI mount destinations are accepted only for exact +single-file paths already listed in the sandbox policy `read_write` list; +writable CDI directory mounts fail closed. Kubernetes, Podman, WSL2 hardware +validation, and Tegra/Jetson hardware validation are separate follow-up +targets. + ## Driver Config Mounts The gateway forwards the `docker` block from `--driver-config-json` to this diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index cd1a0a8c51..846f14be32 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -19,6 +19,7 @@ use bollard::query_parameters::{ }; use bytes::Bytes; use futures::{Stream, StreamExt}; +use openshell_core::cdi::{CdiContext, CdiSpecDirectory, cdi_spec_mount_path}; use openshell_core::config::{ DEFAULT_DOCKER_NETWORK_NAME, DEFAULT_SANDBOX_PIDS_LIMIT, DEFAULT_STOP_TIMEOUT_SECS, }; @@ -185,12 +186,64 @@ struct DockerDriverRuntimeConfig { supervisor_bin: PathBuf, guest_tls: Option, daemon_version: String, - supports_gpu: bool, - allow_all_default_gpu: bool, + gpu: DockerGpuRuntimeConfig, sandbox_pids_limit: i64, enable_bind_mounts: bool, } +#[derive(Debug, Clone, Default)] +struct DockerGpuRuntimeConfig { + cdi_spec_dirs: Vec, + allow_all_default: bool, +} + +impl DockerGpuRuntimeConfig { + fn supports_gpu(&self) -> bool { + !self.cdi_spec_dirs.is_empty() + } + + fn cdi_context(&self, gpu_device_ids: Option<&[String]>) -> Result, Status> { + let Some(gpu_device_ids) = gpu_device_ids.filter(|device_ids| !device_ids.is_empty()) + else { + return Ok(None); + }; + self.require_cdi_spec_dirs()?; + Ok(Some(CdiContext::new( + gpu_device_ids.to_vec(), + self.cdi_spec_dirs + .iter() + .enumerate() + .map(|(index, source)| CdiSpecDirectory::new(cdi_spec_mount_path(index), source)) + .collect(), + ))) + } + + fn cdi_spec_bind_strings( + &self, + gpu_device_ids: Option<&[String]>, + ) -> Result, Status> { + let Some(_) = gpu_device_ids.filter(|device_ids| !device_ids.is_empty()) else { + return Ok(Vec::new()); + }; + self.require_cdi_spec_dirs()?; + Ok(self + .cdi_spec_dirs + .iter() + .enumerate() + .map(|(index, source)| format!("{source}:{}:ro,z", cdi_spec_mount_path(index))) + .collect()) + } + + fn require_cdi_spec_dirs(&self) -> Result<(), Status> { + if self.cdi_spec_dirs.is_empty() { + return Err(Status::failed_precondition( + "docker GPU sandboxes require Docker CDI spec directories reported by the daemon", + )); + } + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] enum DockerGatewayRoute { Bridge { @@ -412,12 +465,11 @@ impl DockerComputeDriver { let info = docker.info().await.map_err(|err| { Error::execution(format!("failed to query Docker daemon info: {err}")) })?; - let supports_gpu = info - .cdi_spec_dirs - .as_ref() - .is_some_and(|dirs| !dirs.is_empty()); + let gpu = DockerGpuRuntimeConfig { + cdi_spec_dirs: info.cdi_spec_dirs.clone().unwrap_or_default(), + allow_all_default: docker_info_reports_wsl2(&info), + }; let cdi_gpu_inventory = docker_cdi_gpu_inventory(&info); - let allow_all_default_gpu = docker_info_reports_wsl2(&info); validate_sandbox_pids_limit(docker_config.sandbox_pids_limit)?; let gateway_port = config.bind_address.port(); if gateway_port == 0 { @@ -467,8 +519,7 @@ impl DockerComputeDriver { supervisor_bin, guest_tls, daemon_version: version.version.unwrap_or_else(|| "unknown".to_string()), - supports_gpu, - allow_all_default_gpu, + gpu: gpu.clone(), sandbox_pids_limit: docker_config.sandbox_pids_limit, enable_bind_mounts: docker_config.enable_bind_mounts, }, @@ -476,7 +527,7 @@ impl DockerComputeDriver { pending: Arc::new(Mutex::new(HashMap::new())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( cdi_gpu_inventory, - allow_all_default_gpu, + gpu.allow_all_default, )), lifecycle_event_fences: DockerLifecycleEventFences::default(), }; @@ -525,7 +576,7 @@ impl DockerComputeDriver { DockerSandboxDriverConfig::from_template(template).map_err(Status::invalid_argument)?; validate_docker_driver_mounts(&driver_config.mounts, config.enable_bind_mounts)?; let gpu_requirements = driver_gpu_requirements(spec.resource_requirements.as_ref()); - Self::validate_gpu_request(gpu_requirements, config.supports_gpu, &driver_config)?; + Self::validate_gpu_request(gpu_requirements, config.gpu.supports_gpu(), &driver_config)?; Ok(ValidatedDockerSandbox { template, driver_config, @@ -633,7 +684,7 @@ impl DockerComputeDriver { .map_err(|err| internal_status("query Docker daemon info", err))?; self.gpu_selector.refresh( docker_cdi_gpu_inventory(&info), - self.config.allow_all_default_gpu, + self.config.gpu.allow_all_default, ); Ok(()) } @@ -779,11 +830,6 @@ impl DockerComputeDriver { .map_err(|status| { DockerProvisioningFailure::new("ImagePullFailed", status.message()) })?; - let token_file_created = write_sandbox_token_file(sandbox, &self.config) - .await - .map_err(|status| { - DockerProvisioningFailure::new("SandboxTokenWriteFailed", status.message()) - })?; let container_name = container_name_for_sandbox(sandbox); let gpu_devices = self @@ -794,9 +840,13 @@ impl DockerComputeDriver { ) .await .map_err(|status| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } + DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) + })?; + let cdi_context = self + .config + .gpu + .cdi_context(gpu_devices.as_deref()) + .map_err(|status| { DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; let create_body = build_container_create_body_for_image( @@ -807,12 +857,26 @@ impl DockerComputeDriver { &image, ) .map_err(|status| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; - self.docker + if let Some(cdi_context) = cdi_context.as_ref() + && let Err(status) = write_cdi_context_file(sandbox, &self.config, cdi_context) + { + cleanup_cdi_context_file(sandbox, &self.config); + return Err(DockerProvisioningFailure::new( + "CdiContextWriteFailed", + status.message(), + )); + } + if let Err(status) = write_sandbox_token_file(sandbox, &self.config).await { + cleanup_cdi_context_file(sandbox, &self.config); + return Err(DockerProvisioningFailure::new( + "SandboxTokenWriteFailed", + status.message(), + )); + } + if let Err(err) = self + .docker .create_container( Some( CreateContainerOptionsBuilder::default() @@ -822,15 +886,13 @@ impl DockerComputeDriver { create_body, ) .await - .map_err(|err| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } - DockerProvisioningFailure::from_status( - "ContainerCreateFailed", - create_status_from_docker_error("create docker sandbox container", err), - ) - })?; + { + cleanup_sandbox_state_files(sandbox, &self.config); + return Err(DockerProvisioningFailure::from_status( + "ContainerCreateFailed", + create_status_from_docker_error("create docker sandbox container", err), + )); + } self.publish_docker_progress( &sandbox.id, "Created", @@ -839,24 +901,13 @@ impl DockerComputeDriver { ); if let Err(err) = self.docker.start_container(&container_name, None).await { - let cleanup = self - .docker - .remove_container( - &container_name, - Some(RemoveContainerOptionsBuilder::default().force(true).build()), - ) - .await; - if let Err(cleanup_err) = cleanup { - warn!( - sandbox_id = %sandbox.id, - container_name, - error = %cleanup_err, - "Failed to clean up Docker container after start failure" - ); - } - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } + self.cleanup_created_container_after_failure( + &sandbox.id, + &container_name, + "container start failure", + ) + .await; + cleanup_sandbox_state_files(sandbox, &self.config); return Err(DockerProvisioningFailure::from_status( "ContainerStartFailed", create_status_from_docker_error("start docker sandbox container", err), @@ -882,6 +933,30 @@ impl DockerComputeDriver { Ok(()) } + async fn cleanup_created_container_after_failure( + &self, + sandbox_id: &str, + container_name: &str, + phase: &'static str, + ) { + let cleanup = self + .docker + .remove_container( + container_name, + Some(RemoveContainerOptionsBuilder::default().force(true).build()), + ) + .await; + if let Err(cleanup_err) = cleanup { + warn!( + sandbox_id = %sandbox_id, + container_name = %container_name, + phase, + error = %cleanup_err, + "Failed to clean up Docker container after provisioning failure" + ); + } + } + async fn delete_sandbox_inner( &self, sandbox_id: &str, @@ -909,11 +984,11 @@ impl DockerComputeDriver { .await { Ok(()) => { - cleanup_sandbox_token_file(&record.sandbox, &self.config); + cleanup_sandbox_state_files(&record.sandbox, &self.config); return Ok(true); } Err(err) if is_not_found_error(&err) => { - cleanup_sandbox_token_file(&record.sandbox, &self.config); + cleanup_sandbox_state_files(&record.sandbox, &self.config); return Ok(true); } Err(err) => { @@ -936,11 +1011,11 @@ impl DockerComputeDriver { .await { Ok(()) => { - cleanup_sandbox_token_file_for_delete(sandbox_id, pending.as_ref(), &self.config); + cleanup_sandbox_state_files_for_delete(sandbox_id, pending.as_ref(), &self.config); Ok(true) } Err(err) if is_not_found_error(&err) => { - cleanup_sandbox_token_file_for_delete(sandbox_id, pending.as_ref(), &self.config); + cleanup_sandbox_state_files_for_delete(sandbox_id, pending.as_ref(), &self.config); Ok(pending.is_some()) } Err(err) => Err(internal_status("delete docker sandbox container", err)), @@ -956,7 +1031,7 @@ impl DockerComputeDriver { if let Some(task) = record.task { task.abort(); } - cleanup_sandbox_token_file(&record.sandbox, &self.config); + cleanup_sandbox_state_files(&record.sandbox, &self.config); self.publish_deleted(record.sandbox.id); return Ok(()); } @@ -1121,7 +1196,7 @@ impl DockerComputeDriver { sandbox: &DriverSandbox, failure: &DockerProvisioningFailure, ) { - cleanup_sandbox_token_file(sandbox, &self.config); + cleanup_sandbox_state_files(sandbox, &self.config); let snapshot = pending_sandbox_snapshot( sandbox, &self.config.sandbox_namespace, @@ -2209,6 +2284,7 @@ fn docker_volume_is_bind_backed(volume: &bollard::models::Volume) -> bool { fn build_binds( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, + gpu_device_ids: Option<&[String]>, ) -> Result, Status> { let mut binds = vec![format!( "{}:{}:ro,z", @@ -2235,6 +2311,13 @@ fn build_binds( SANDBOX_TOKEN_MOUNT_PATH )); } + if cdi_context_requested(gpu_device_ids) { + binds.push(format!( + "{}:{}:ro,z", + cdi_context_host_path(sandbox, config)?.display(), + openshell_core::cdi::CDI_CONTEXT_PATH + )); + } Ok(binds) } @@ -2261,6 +2344,57 @@ fn sandbox_token_host_path_by_id( }) } +fn cdi_context_host_path( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, +) -> Result { + cdi_context_host_path_by_id(&sandbox.id, config) +} + +fn cdi_context_host_path_by_id( + sandbox_id: &str, + config: &DockerDriverRuntimeConfig, +) -> Result { + openshell_core::driver_utils::sandbox_token_path( + "docker-cdi-contexts", + Some(&config.sandbox_namespace), + sandbox_id, + ) + .map(|path| path.with_file_name(openshell_core::cdi::CDI_CONTEXT_FILE_NAME)) + .map_err(|err| Status::internal(format!("resolve CDI context state directory failed: {err}"))) +} + +fn write_cdi_context_file( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + context: &CdiContext, +) -> Result<(), Status> { + let path = cdi_context_host_path(sandbox, config)?; + if let Some(parent) = path.parent() { + openshell_core::paths::create_dir_restricted(parent).map_err(|err| { + Status::internal(format!( + "create CDI context directory {} failed: {err}", + parent.display() + )) + })?; + } + let json = serde_json::to_vec(context) + .map_err(|err| Status::internal(format!("encode CDI context failed: {err}")))?; + std::fs::write(&path, json).map_err(|err| { + Status::internal(format!( + "write CDI context file {} failed: {err}", + path.display() + )) + })?; + openshell_core::paths::set_file_owner_only(&path).map_err(|err| { + Status::internal(format!( + "restrict CDI context file {} failed: {err}", + path.display() + )) + })?; + Ok(()) +} + async fn write_sandbox_token_file( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, @@ -2301,6 +2435,15 @@ fn cleanup_sandbox_token_file(sandbox: &DriverSandbox, config: &DockerDriverRunt cleanup_sandbox_token_file_by_id(&sandbox.id, config); } +fn cleanup_cdi_context_file(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) { + cleanup_cdi_context_file_by_id(&sandbox.id, config); +} + +fn cleanup_sandbox_state_files(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) { + cleanup_sandbox_token_file(sandbox, config); + cleanup_cdi_context_file(sandbox, config); +} + fn cleanup_sandbox_token_file_for_delete( sandbox_id: &str, pending: Option<&PendingSandboxRecord>, @@ -2313,6 +2456,27 @@ fn cleanup_sandbox_token_file_for_delete( } } +fn cleanup_cdi_context_file_for_delete( + sandbox_id: &str, + pending: Option<&PendingSandboxRecord>, + config: &DockerDriverRuntimeConfig, +) { + if !sandbox_id.is_empty() { + cleanup_cdi_context_file_by_id(sandbox_id, config); + } else if let Some(record) = pending { + cleanup_cdi_context_file(&record.sandbox, config); + } +} + +fn cleanup_sandbox_state_files_for_delete( + sandbox_id: &str, + pending: Option<&PendingSandboxRecord>, + config: &DockerDriverRuntimeConfig, +) { + cleanup_sandbox_token_file_for_delete(sandbox_id, pending, config); + cleanup_cdi_context_file_for_delete(sandbox_id, pending, config); +} + fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRuntimeConfig) { let Ok(path) = sandbox_token_host_path_by_id(sandbox_id, config) else { return; @@ -2332,15 +2496,39 @@ fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRunti } } +fn cleanup_cdi_context_file_by_id(sandbox_id: &str, config: &DockerDriverRuntimeConfig) { + let Ok(path) = cdi_context_host_path_by_id(sandbox_id, config) else { + return; + }; + if let Err(err) = std::fs::remove_file(&path) + && err.kind() != std::io::ErrorKind::NotFound + { + warn!( + sandbox_id = %sandbox_id, + path = %path.display(), + error = %err, + "Failed to remove Docker CDI context file" + ); + } + if let Some(dir) = path.parent() { + let _ = std::fs::remove_dir(dir); + } +} + #[cfg(test)] -fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) -> Vec { - build_environment_for_oci_user(sandbox, config, "") +fn build_environment( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + include_cdi_context: bool, +) -> Vec { + build_environment_for_oci_user(sandbox, config, "", include_cdi_context) } fn build_environment_for_oci_user( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, oci_user: &str, + include_cdi_context: bool, ) -> Vec { let mut environment = HashMap::from([ ("HOME".to_string(), "/root".to_string()), @@ -2393,6 +2581,14 @@ fn build_environment_for_oci_user( openshell_core::sandbox_env::TELEMETRY_ENABLED.to_string(), openshell_core::telemetry::enabled_env_value().to_string(), ); + environment.insert( + openshell_core::sandbox_env::CDI_CONTEXT.to_string(), + if include_cdi_context { + openshell_core::cdi::CDI_CONTEXT_PATH.to_string() + } else { + String::new() + }, + ); // The root supervisor executes namespace helpers during bootstrap; keep // their search path driver-owned even when the template/spec set PATH. environment.insert("PATH".to_string(), SUPERVISOR_PATH.to_string()); @@ -2480,6 +2676,10 @@ fn docker_gpu_selection_status(err: CdiGpuSelectionError) -> Status { Status::failed_precondition(err.to_string()) } +fn cdi_context_requested(gpu_device_ids: Option<&[String]>) -> bool { + gpu_device_ids.is_some_and(|device_ids| !device_ids.is_empty()) +} + #[cfg(test)] fn build_container_create_body( sandbox: &DriverSandbox, @@ -2616,7 +2816,12 @@ fn build_container_create_body_for_image( // The image workspace may need to be created or rejected by the // supervisor, so do not let the OCI runtime chdir there first. working_dir: Some("/".to_string()), - env: Some(build_environment_for_oci_user(sandbox, config, &image.user)), + env: Some(build_environment_for_oci_user( + sandbox, + config, + &image.user, + cdi_context_requested(gpu_device_ids), + )), entrypoint: Some(vec![SUPERVISOR_MOUNT_PATH.to_string()]), // Replace the image CMD with the supervisor's resolved workspace // argument so Docker cannot append inherited image arguments. @@ -2628,7 +2833,8 @@ fn build_container_create_body_for_image( pids_limit: docker_pids_limit(config.sandbox_pids_limit)?, device_requests, binds: { - let mut binds = build_binds(sandbox, config)?; + let mut binds = build_binds(sandbox, config, gpu_device_ids)?; + binds.extend(config.gpu.cdi_spec_bind_strings(gpu_device_ids)?); binds.extend(user_bind_strings); Some(binds) }, diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index eddfd778bd..a1bc44d1d6 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -88,7 +88,10 @@ fn gpu_resources(count: Option) -> ResourceRequirements { } } -fn runtime_config() -> DockerDriverRuntimeConfig { +const TEST_CDI_SPEC_DIR: &str = "/opt/openshell-test/cdi"; +const TEST_CDI_SPEC_DIR_ALT: &str = "/srv/openshell-test/cdi"; + +fn runtime_config(supports_gpu: bool) -> DockerDriverRuntimeConfig { DockerDriverRuntimeConfig { default_image: "image:latest".to_string(), image_pull_policy: String::new(), @@ -116,13 +119,32 @@ fn runtime_config() -> DockerDriverRuntimeConfig { key: PathBuf::from("/tmp/tls.key"), }), daemon_version: "28.0.0".to_string(), - supports_gpu: false, - allow_all_default_gpu: false, + gpu: gpu_runtime_config(supports_gpu), sandbox_pids_limit: DEFAULT_SANDBOX_PIDS_LIMIT, enable_bind_mounts: false, } } +fn runtime_config_with_cdi_spec_dirs(cdi_spec_dirs: &[&str]) -> DockerDriverRuntimeConfig { + let mut config = runtime_config(false); + config.gpu.cdi_spec_dirs = cdi_spec_dirs + .iter() + .map(|path| (*path).to_string()) + .collect(); + config +} + +fn gpu_runtime_config(supports_gpu: bool) -> DockerGpuRuntimeConfig { + if supports_gpu { + DockerGpuRuntimeConfig { + cdi_spec_dirs: vec![TEST_CDI_SPEC_DIR.to_string()], + ..Default::default() + } + } else { + DockerGpuRuntimeConfig::default() + } +} + fn json_struct(value: serde_json::Value) -> prost_types::Struct { let serde_json::Value::Object(object) = value else { panic!("expected JSON object"); @@ -147,7 +169,7 @@ fn inspected_volume(driver: &str, options: HashMap) -> bollard:: } fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDriver { - let allow_all_default_gpu = config.allow_all_default_gpu; + let allow_all_default_gpu = config.gpu.allow_all_default; DockerComputeDriver { docker: Arc::new( Docker::connect_with_http("http://127.0.0.1:2375", 1, bollard::API_DEFAULT_VERSION) @@ -166,7 +188,7 @@ fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDr #[tokio::test] async fn gateway_listener_requirements_report_managed_bridge_address() { - let config = runtime_config(); + let config = runtime_config(false); let expected_address = match config.gateway_route { DockerGatewayRoute::Bridge { bind_address, .. } => bind_address, DockerGatewayRoute::HostGateway => panic!("test config must use a managed bridge"), @@ -188,7 +210,7 @@ async fn gateway_listener_requirements_report_managed_bridge_address() { #[tokio::test] async fn gateway_listener_requirements_are_empty_for_host_gateway_route() { - let mut config = runtime_config(); + let mut config = runtime_config(false); config.gateway_route = DockerGatewayRoute::HostGateway; config.gateway_callback_bind_address = None; let driver = test_driver_with_config(config); @@ -204,7 +226,7 @@ async fn gateway_listener_requirements_are_empty_for_host_gateway_route() { #[tokio::test] async fn host_gateway_route_reports_ipv4_loopback_callback_listener() { - let mut config = runtime_config(); + let mut config = runtime_config(false); config.gateway_route = DockerGatewayRoute::HostGateway; config.gateway_callback_bind_address = Some("127.0.0.1:17670".parse().unwrap()); let driver = test_driver_with_config(config); @@ -605,14 +627,14 @@ fn docker_compute_config_disables_bind_mounts_by_default() { #[test] fn container_create_body_sets_driver_owned_pids_limit() { - let body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); let host_config = body.host_config.expect("host config"); assert_eq!(host_config.pids_limit, Some(DEFAULT_SANDBOX_PIDS_LIMIT)); } #[test] fn build_environment_sets_docker_tls_paths() { - let env = build_environment(&test_sandbox(), &runtime_config()); + let env = build_environment(&test_sandbox(), &runtime_config(false), false); assert!(env.contains(&format!("OPENSHELL_TLS_CA={TLS_CA_MOUNT_PATH}"))); assert!(env.contains(&format!("OPENSHELL_TLS_CERT={TLS_CERT_MOUNT_PATH}"))); assert!(env.contains(&format!("OPENSHELL_TLS_KEY={TLS_KEY_MOUNT_PATH}"))); @@ -633,7 +655,7 @@ fn build_environment_protects_oci_identity_metadata() { spec.environment.insert(key.to_string(), value.to_string()); } - let env = build_environment_for_oci_user(&sandbox, &runtime_config(), "app:staff"); + let env = build_environment_for_oci_user(&sandbox, &runtime_config(false), "app:staff", false); assert!(env.contains(&format!( "{}=app:staff", @@ -654,7 +676,7 @@ fn build_environment_strips_gateway_tls_server_name() { "evil.attacker.example.com".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); assert!( !env.iter().any(|entry| entry.starts_with(&format!( @@ -676,7 +698,7 @@ fn container_creation_uses_inspected_immutable_image() { }; let body = build_container_create_body_for_image( &sandbox, - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -706,7 +728,7 @@ fn container_creation_rejects_invalid_oci_working_dir() { }; let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -727,7 +749,7 @@ fn container_creation_rejects_openshell_control_path_working_dir() { }; let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -750,7 +772,7 @@ fn container_creation_rejects_image_volume_that_masks_working_dir() { let error = build_container_create_body_for_image( &sandbox, - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -772,7 +794,7 @@ fn container_creation_rejects_image_volume_over_configured_ssh_socket() { working_dir: "/workspace".to_string(), volumes: vec!["/custom-runtime".to_string()], }; - let mut config = runtime_config(); + let mut config = runtime_config(false); config.ssh_socket_path = "/custom-runtime/ssh.sock".to_string(); let error = build_container_create_body_for_image( @@ -801,7 +823,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( .unwrap(); let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &root_mount, None, &metadata, @@ -823,7 +845,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( }; let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &ancestor_mount, None, &nested_metadata, @@ -840,7 +862,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( .unwrap(); build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &nested_mount, None, &metadata, @@ -854,7 +876,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( .unwrap(); build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &compatibility_path_mount, None, &metadata, @@ -874,7 +896,7 @@ fn build_environment_keeps_path_driver_controlled() { .environment .insert("PATH".to_string(), "/malicious/template/bin".to_string()); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); let path_entries = env .iter() .filter(|entry| entry.starts_with("PATH=")) @@ -900,7 +922,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { "true".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); let telemetry_entries = env .iter() .filter(|entry| { @@ -922,7 +944,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { #[test] fn build_binds_uses_docker_tls_directory() { - let binds = build_binds(&test_sandbox(), &runtime_config()).unwrap(); + let binds = build_binds(&test_sandbox(), &runtime_config(false), None).unwrap(); let targets = binds .iter() .filter_map(|bind| bind.split(':').nth(1).map(String::from)) @@ -961,7 +983,7 @@ fn build_container_create_body_includes_driver_config_mounts() { ] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -1009,7 +1031,7 @@ fn driver_config_defaults_volume_mounts_to_read_only() { }] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -1038,7 +1060,7 @@ fn driver_config_allows_explicit_writable_volume_mounts() { }] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -1072,7 +1094,7 @@ fn driver_config_rejects_duplicate_mount_targets() { ] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!( @@ -1099,7 +1121,7 @@ fn driver_config_rejects_bind_mounts_unless_enabled() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("enable_bind_mounts = true")); @@ -1125,7 +1147,7 @@ fn build_container_create_body_includes_bind_mounts_when_enabled() { "read_only": true }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1170,7 +1192,7 @@ fn driver_config_defaults_enabled_bind_mounts_to_read_only() { "target": "/sandbox/host" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1208,7 +1230,7 @@ fn bind_mount_selinux_shared_label() { "selinux_label": "shared" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1246,7 +1268,7 @@ fn bind_mount_selinux_private_label() { "selinux_label": "private" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1283,7 +1305,7 @@ fn bind_mount_without_selinux_label() { "read_only": false }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1317,7 +1339,7 @@ fn driver_config_rejects_missing_bind_source() { "target": "/sandbox/data" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let err = build_container_create_body(&sandbox, &config).unwrap_err(); @@ -1347,7 +1369,7 @@ fn driver_config_rejects_relative_bind_sources_when_enabled() { "target": "/sandbox/host" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let err = build_container_create_body(&sandbox, &config).unwrap_err(); @@ -1377,7 +1399,7 @@ fn driver_config_rejects_image_mounts() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("invalid docker driver_config")); @@ -1401,7 +1423,7 @@ fn driver_config_rejects_reserved_mount_targets() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("reserved OpenShell path")); @@ -1422,7 +1444,7 @@ fn driver_config_rejects_mount_over_configured_ssh_socket() { working_dir: "/workspace".to_string(), volumes: Vec::new(), }; - let mut config = runtime_config(); + let mut config = runtime_config(false); config.ssh_socket_path = "/custom-runtime/ssh.sock".to_string(); let error = build_container_create_body_for_image( @@ -1499,7 +1521,7 @@ fn build_environment_uses_token_file_without_raw_token_env() { "user-provided-token".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); assert!(!env.iter().any(|entry| { entry.starts_with(&format!("{}=", openshell_core::sandbox_env::SANDBOX_TOKEN)) @@ -1523,7 +1545,7 @@ fn managed_container_label_filters_include_gateway_namespace() { #[test] fn build_container_create_body_replaces_inherited_cmd_with_workspace_arg() { - let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let create_body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); assert_eq!( create_body.entrypoint, @@ -1572,7 +1594,7 @@ fn build_container_create_body_replaces_inherited_cmd_with_workspace_arg() { #[test] fn validate_sandbox_rejects_gpu_when_cdi_unavailable() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1584,7 +1606,7 @@ fn validate_sandbox_rejects_gpu_when_cdi_unavailable() { #[test] fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1598,7 +1620,7 @@ fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() { #[test] fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1613,7 +1635,7 @@ fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() { #[test] fn validate_sandbox_rejects_unknown_driver_config_fields() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1628,8 +1650,7 @@ fn validate_sandbox_rejects_unknown_driver_config_fields() { #[test] fn validate_sandbox_accepts_gpu_count_request_shape() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(Some(2))); @@ -1639,8 +1660,7 @@ fn validate_sandbox_accepts_gpu_count_request_shape() { #[test] fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1655,8 +1675,7 @@ fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() { #[test] fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1668,8 +1687,7 @@ fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() { #[test] fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1689,8 +1707,7 @@ fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() { #[test] fn validate_sandbox_rejects_cdi_devices_without_gpu_request() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox .spec @@ -1709,8 +1726,7 @@ fn validate_sandbox_rejects_cdi_devices_without_gpu_request() { #[test] fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1727,7 +1743,7 @@ fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() { #[test] fn validate_sandbox_rejects_template_errors_before_device_config() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1765,8 +1781,7 @@ fn validate_sandbox_auth_accepts_gateway_token() { #[test] fn build_container_create_body_maps_default_gpu_to_selected_cdi_device() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1793,10 +1808,106 @@ fn build_container_create_body_maps_default_gpu_to_selected_cdi_device() { ); } +#[test] +fn build_container_create_body_adds_cdi_context_env_and_spec_mounts_for_gpu() { + let config = runtime_config_with_cdi_spec_dirs(&[TEST_CDI_SPEC_DIR, TEST_CDI_SPEC_DIR_ALT]); + let mut sandbox = test_sandbox(); + sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); + + let driver_config = DockerSandboxDriverConfig::default(); + let gpu_devices = vec!["nvidia.com/gpu=1".to_string()]; + let create_body = build_container_create_body_with_gpu_devices( + &sandbox, + &config, + &driver_config, + Some(&gpu_devices), + ) + .unwrap(); + + let env = create_body.env.expect("env should be set"); + assert!(env.iter().any(|entry| { + entry + == &format!( + "{}={}", + openshell_core::sandbox_env::CDI_CONTEXT, + openshell_core::cdi::CDI_CONTEXT_PATH + ) + })); + + let binds = create_body + .host_config + .expect("host config") + .binds + .expect("binds should be set"); + assert!( + binds.iter().any(|bind| { + bind == &format!("{TEST_CDI_SPEC_DIR}:{}:ro,z", cdi_spec_mount_path(0)) + }) + ); + assert!(binds.iter().any(|bind| { + bind == &format!("{TEST_CDI_SPEC_DIR_ALT}:{}:ro,z", cdi_spec_mount_path(1)) + })); + assert!(binds.iter().any(|bind| { + bind == &format!( + "{}:{}:ro,z", + cdi_context_host_path(&sandbox, &config).unwrap().display(), + openshell_core::cdi::CDI_CONTEXT_PATH + ) + })); +} + +#[test] +fn build_container_create_body_clears_cdi_context_for_non_gpu() { + let mut config = runtime_config(false); + config.gpu.cdi_spec_dirs = vec![TEST_CDI_SPEC_DIR.to_string()]; + let create_body = build_container_create_body(&test_sandbox(), &config).unwrap(); + + let env = create_body.env.expect("env should be set"); + assert!( + env.iter() + .any(|entry| { entry == &format!("{}=", openshell_core::sandbox_env::CDI_CONTEXT) }) + ); + + let binds = create_body + .host_config + .expect("host config") + .binds + .expect("binds should be set"); + assert!( + !binds + .iter() + .any(|bind| bind.contains(openshell_core::cdi::CDI_SPEC_DIR_BASE)) + ); +} + +#[test] +fn write_cdi_context_file_materializes_owned_host_context() { + let _guard = ENV_LOCK.lock().unwrap(); + let state_dir = tempfile::tempdir().unwrap(); + let sandbox = test_sandbox(); + let config = runtime_config(true); + let context = CdiContext::new( + vec!["nvidia.com/gpu=0".to_string()], + vec![CdiSpecDirectory::new( + cdi_spec_mount_path(0), + TEST_CDI_SPEC_DIR, + )], + ); + + temp_env::with_var("XDG_STATE_HOME", Some(state_dir.path()), || { + write_cdi_context_file(&sandbox, &config, &context).expect("write CDI context"); + let path = cdi_context_host_path(&sandbox, &config).expect("context path"); + let contents = fs::read(&path).expect("read CDI context"); + let parsed: CdiContext = serde_json::from_slice(&contents).expect("parse CDI context"); + assert_eq!(parsed, context); + cleanup_cdi_context_file(&sandbox, &config); + assert!(!path.exists()); + }); +} + #[test] fn build_container_create_body_omits_devices_without_resolved_default_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1813,8 +1924,7 @@ fn build_container_create_body_omits_devices_without_resolved_default_cdi_device #[test] fn build_container_create_body_passes_explicit_cdi_device_id_through() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1837,8 +1947,7 @@ fn build_container_create_body_passes_explicit_cdi_device_id_through() { #[test] fn build_container_create_body_rejects_gpu_count_mismatched_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1865,7 +1974,7 @@ fn build_container_create_body_rejects_cdi_devices_without_gpu_request() { .unwrap() .driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"])); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::InvalidArgument); assert!(err.message().contains("requires a gpu request")); } @@ -1877,15 +1986,14 @@ fn build_container_create_body_rejects_empty_cdi_devices() { spec.resource_requirements = Some(gpu_resources(None)); spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[])); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::InvalidArgument); assert!(err.message().contains("non-empty list")); } #[test] fn driver_default_gpu_selection_consumes_distinct_devices_for_creates() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let driver = test_driver_with_config(config); driver.gpu_selector.refresh( CdiGpuInventory::new(["nvidia.com/gpu=0", "nvidia.com/gpu=1"]), @@ -2017,7 +2125,7 @@ fn require_sandbox_identifier_rejects_when_id_and_name_are_empty() { #[test] fn build_container_create_body_uses_bridge_network() { - let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let create_body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); let host_config = create_body.host_config.expect("host_config is populated"); assert_eq!( @@ -2043,7 +2151,7 @@ fn build_container_create_body_uses_runtime_namespace_label() { // with that empty value would not match subsequent list/get/find // queries (which filter on `config.sandbox_namespace`), leaking // sandboxes that the driver itself cannot observe. - let mut config = runtime_config(); + let mut config = runtime_config(false); config.sandbox_namespace = "tenant-a".to_string(); let mut sandbox = test_sandbox(); sandbox.namespace = "ignored-by-driver".to_string(); diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index c1dbada149..c1e3d9e3ac 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -15,7 +15,9 @@ mod metadata_server; mod sidecar_control; use miette::{IntoDiagnostic, Result, WrapErr}; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::future::Future; +use std::path::PathBuf; use std::sync::Arc; #[cfg(target_os = "linux")] use std::sync::atomic::Ordering; @@ -510,7 +512,7 @@ pub async fn run_sandbox( retained_proto.clone(), openshell_endpoint.clone(), sandbox_id.clone(), - std::path::PathBuf::from(trusted_ssh_socket_path), + PathBuf::from(trusted_ssh_socket_path), ); } @@ -877,11 +879,11 @@ fn process_enforcement_mode() -> ProcessEnforcementMode { } } -fn sidecar_control_socket() -> Option { +fn sidecar_control_socket() -> Option { std::env::var(openshell_core::sandbox_env::SIDECAR_CONTROL_SOCKET) .ok() .filter(|path| !path.is_empty()) - .map(std::path::PathBuf::from) + .map(PathBuf::from) } #[cfg_attr(not(target_os = "linux"), allow(dead_code))] @@ -993,7 +995,7 @@ fn spawn_sidecar_entrypoint_handler( retained_proto: Option, openshell_endpoint: Option, sandbox_id: Option, - trusted_ssh_socket_path: std::path::PathBuf, + trusted_ssh_socket_path: PathBuf, ) { tokio::spawn(async move { let mut session_started = false; @@ -1057,7 +1059,7 @@ fn spawn_sidecar_entrypoint_handler( }); } -fn sidecar_ca_file_paths() -> Option<(std::path::PathBuf, std::path::PathBuf)> { +fn sidecar_ca_file_paths() -> Option<(PathBuf, PathBuf)> { let tls_dir = std::env::var(openshell_core::sandbox_env::PROXY_TLS_DIR) .unwrap_or_else(|_| SIDECAR_TLS_DIR.to_string()); let cert = std::path::Path::new(&tls_dir).join(SIDECAR_CA_CERT); @@ -1225,42 +1227,33 @@ const PROXY_BASELINE_READ_ONLY: &[&str] = &[ /// The active workspace is granted separately through `include_workdir`. const PROXY_BASELINE_READ_WRITE: &[&str] = &["/tmp"]; -/// GPU read-only paths. +/// GPU read-only paths for the legacy device-scan fallback. /// /// `/run/nvidia-persistenced`: NVML tries to connect to the persistenced /// socket at init time. If the directory exists but Landlock denies traversal /// (EACCES vs ECONNREFUSED), NVML returns `NVML_ERROR_INSUFFICIENT_PERMISSIONS` /// even though the daemon is optional. Only read/traversal access is needed. /// -/// `/usr/lib/wsl`: On WSL2, CDI bind-mounts GPU libraries (libdxcore.so, -/// libcuda.so.1.1, etc.) into paths under `/usr/lib/wsl/`. Although `/usr` -/// is already in `PROXY_BASELINE_READ_ONLY`, individual file bind-mounts may -/// not be covered by the parent-directory Landlock rule when the mount crosses -/// a filesystem boundary. Listing `/usr/lib/wsl` explicitly ensures traversal -/// is permitted regardless of Landlock's cross-mount behaviour. +/// `/usr/lib/wsl`: retained for the legacy device-scan fallback. CDI +/// sandboxes use resolved mount destinations instead of this broad directory +/// baseline. const GPU_BASELINE_READ_ONLY: &[&str] = &[ "/run/nvidia-persistenced", - "/usr/lib/wsl", // WSL2: CDI-injected GPU library directory + "/usr/lib/wsl", // Legacy fallback; CDI uses resolved mount destinations. ]; -/// GPU read-write paths (static). +/// GPU read-write paths for the legacy device-scan fallback. /// /// `/dev/nvidiactl`, `/dev/nvidia-uvm`, `/dev/nvidia-uvm-tools`, -/// `/dev/nvidia-modeset`: control and UVM devices injected by CDI on native -/// Linux. Landlock restricts `open(2)` on device files even when DAC allows -/// it; these need read-write because NVML/CUDA opens them with `O_RDWR`. -/// These devices do not exist on WSL2 and will be skipped by the existence -/// check in `enrich_proto_baseline_paths()`. +/// `/dev/nvidia-modeset`: control and UVM devices. Landlock restricts +/// `open(2)` on device files even when DAC allows it; these need read-write +/// because NVML/CUDA opens them with `O_RDWR`. CDI sandboxes derive device +/// nodes from the selected CDI specs instead of this hard-coded list. /// /// `/dev/dxg`: On WSL2, NVIDIA GPUs are exposed through the DXG kernel driver -/// (DirectX Graphics) rather than the native nvidia* devices. CDI injects -/// `/dev/dxg` as the sole GPU device node; it does not exist on native Linux -/// and will be skipped there by the existence check. -/// -/// `/proc`: CUDA writes to `/proc//task//comm` during `cuInit()` -/// to set thread names. Without write access, `cuInit()` returns error 304. -/// Must use `/proc` (not `/proc/self/task`) because Landlock rules bind to -/// inodes and child processes have different procfs inodes than the parent. +/// (DirectX Graphics) rather than the native nvidia* devices. This is retained +/// for the legacy device-scan fallback; CDI sandboxes derive it from specs when +/// needed. /// /// Per-GPU device files (`/dev/nvidia0`, …) are enumerated at runtime by /// `enumerate_gpu_device_nodes()` since the count varies. @@ -1269,190 +1262,533 @@ const GPU_BASELINE_READ_WRITE: &[&str] = &[ "/dev/nvidia-uvm", "/dev/nvidia-uvm-tools", "/dev/nvidia-modeset", - "/dev/dxg", // WSL2: DXG device (GPU via DirectX kernel driver, injected by CDI) - "/proc", + "/dev/dxg", // WSL2: DXG device exposed through the DirectX kernel driver. ]; -/// Returns true if GPU devices are present in the container. -/// -/// Checks both the native Linux NVIDIA control device (`/dev/nvidiactl`) and -/// the WSL2 DXG device (`/dev/dxg`). CDI injects exactly one of these -/// depending on the host kernel; the other will not exist. -fn has_gpu_devices() -> bool { - std::path::Path::new("/dev/nvidiactl").exists() || std::path::Path::new("/dev/dxg").exists() +/// CUDA writes to `/proc//task//comm` during `cuInit()` to set thread +/// names. Without write access, `cuInit()` returns error 304. Must use `/proc` +/// (not `/proc/self/task`) because Landlock rules bind to inodes and child +/// processes have different procfs inodes than the parent. +const GPU_PROC_READ_WRITE: &str = "/proc"; + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +struct EnrichmentPathSources { + baseline: bool, + runtime: bool, } -/// Enumerate per-GPU device nodes (`/dev/nvidia0`, `/dev/nvidia1`, …). -fn enumerate_gpu_device_nodes() -> Vec { - let mut paths = Vec::new(); - if let Ok(entries) = std::fs::read_dir("/dev") { - for entry in entries.flatten() { - let name = entry.file_name(); - let name = name.to_string_lossy(); - if let Some(suffix) = name.strip_prefix("nvidia") { - if suffix.is_empty() || !suffix.chars().all(|c| c.is_ascii_digit()) { - continue; - } - paths.push(entry.path().to_string_lossy().into_owned()); - } +impl EnrichmentPathSources { + fn baseline() -> Self { + Self { + baseline: true, + runtime: false, } } - paths + + fn runtime() -> Self { + Self { + baseline: false, + runtime: true, + } + } + + fn merge(&mut self, other: Self) { + self.baseline |= other.baseline; + self.runtime |= other.runtime; + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum MissingPathBehavior { + Skip, + Error, } -fn push_unique(paths: &mut Vec, path: String) { - if !paths.iter().any(|p| p == &path) { - paths.push(path); +impl MissingPathBehavior { + fn merge(self, other: Self) -> Self { + match (self, other) { + (Self::Error, _) | (_, Self::Error) => Self::Error, + (Self::Skip, Self::Skip) => Self::Skip, + } } } -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()); +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReadOnlyConflictBehavior { + KeepReadOnly, + PromoteToReadWrite, + Reject, +} + +impl ReadOnlyConflictBehavior { + fn merge(self, other: Self) -> Self { + match (self, other) { + (Self::Reject, _) | (_, Self::Reject) => Self::Reject, + (Self::PromoteToReadWrite, _) | (_, Self::PromoteToReadWrite) => { + Self::PromoteToReadWrite + } + (Self::KeepReadOnly, Self::KeepReadOnly) => Self::KeepReadOnly, } - for &path in PROXY_BASELINE_READ_WRITE { - push_unique(&mut rw, path.to_string()); + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct EnrichmentPathPolicy { + sources: EnrichmentPathSources, + missing_path: MissingPathBehavior, + read_only_conflict: ReadOnlyConflictBehavior, +} + +impl EnrichmentPathPolicy { + fn baseline() -> Self { + Self { + sources: EnrichmentPathSources::baseline(), + missing_path: MissingPathBehavior::Skip, + read_only_conflict: ReadOnlyConflictBehavior::KeepReadOnly, } } - if include_gpu { - for &path in GPU_BASELINE_READ_ONLY { - push_unique(&mut ro, path.to_string()); + fn runtime_device_node() -> Self { + Self { + sources: EnrichmentPathSources::runtime(), + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::KeepReadOnly, } - for &path in GPU_BASELINE_READ_WRITE { - push_unique(&mut rw, path.to_string()); + } + + fn runtime_mount() -> Self { + Self { + sources: EnrichmentPathSources::runtime(), + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::KeepReadOnly, } - for path in gpu_device_nodes { - push_unique(&mut rw, path); + } + + fn runtime_writable_mount() -> Self { + Self { + sources: EnrichmentPathSources::runtime(), + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::Reject, } } - // A path promoted to read_write (e.g. /proc for GPU) should not also - // appear in read_only — Landlock handles the overlap correctly but the - // duplicate is confusing when inspecting the effective policy. - ro.retain(|p| !rw.contains(p)); + fn gpu_proc(sources: EnrichmentPathSources) -> Self { + Self { + sources, + missing_path: MissingPathBehavior::Error, + read_only_conflict: ReadOnlyConflictBehavior::PromoteToReadWrite, + } + } - (ro, rw) + fn merge(&mut self, other: Self) { + self.sources.merge(other.sources); + self.missing_path = self.missing_path.merge(other.missing_path); + self.read_only_conflict = self.read_only_conflict.merge(other.read_only_conflict); + } } -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) +#[derive(Debug, Clone, PartialEq, Eq)] +struct EnrichmentPath { + path: String, + policy: EnrichmentPathPolicy, } -/// 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) +impl EnrichmentPath { + fn should_apply(&self, access: &str, path_exists: &F) -> Result + where + F: Fn(&str) -> bool, + { + if path_exists(&self.path) { + return Ok(true); + } + + match self.policy.missing_path { + MissingPathBehavior::Skip => { + debug!( + path = %self.path, + access, + "Baseline enrichment path does not exist, skipping" + ); + Ok(false) + } + MissingPathBehavior::Error => Err(miette::miette!( + "Runtime-derived enrichment path '{}' does not exist", + self.path + )), + } + } } -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; +#[derive(Debug, Clone, Default, PartialEq, Eq)] +struct EnrichmentPlan { + read_only: BTreeMap, + read_write: BTreeMap, + additional_gids: BTreeSet, +} + +impl EnrichmentPlan { + fn for_proto_policy( + proto: &openshell_core::proto::SandboxPolicy, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + ) -> Self { + Self::active(!proto.network_policies.is_empty(), cdi_requirements) + } + + fn for_sandbox_policy( + policy: &SandboxPolicy, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + ) -> Self { + Self::active( + matches!(policy.network.mode, NetworkMode::Proxy), + cdi_requirements, + ) } - let fs = proto - .filesystem - .get_or_insert_with(|| openshell_core::proto::FilesystemPolicy { - include_workdir: true, - ..Default::default() - }); + fn active( + include_proxy: bool, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + ) -> Self { + let mut plan = Self::default(); + if include_proxy { + plan = plan.merge(Self::proxy_baseline()); + } + let gpu_plan = cdi_requirements.map_or_else( + || { + if has_gpu_devices() { + Self::legacy_gpu_fallback(enumerate_gpu_device_nodes()) + } else { + Self::default() + } + }, + Self::cdi_gpu, + ); + plan.merge(gpu_plan) + } - let mut modified = false; - for path in ro { - if !fs.read_only.iter().any(|p| p == path) && !fs.read_write.iter().any(|p| p == path) { - if !path_exists(path) { - debug!( - path, - "Baseline read-only path does not exist, skipping enrichment" - ); + fn proxy_baseline() -> Self { + Self::from_baseline_paths(PROXY_BASELINE_READ_ONLY, PROXY_BASELINE_READ_WRITE) + } + + fn legacy_gpu_fallback(gpu_device_nodes: Vec) -> Self { + let mut plan = Self::from_baseline_paths(GPU_BASELINE_READ_ONLY, GPU_BASELINE_READ_WRITE); + for path in gpu_device_nodes { + plan.insert_read_write_path(path, EnrichmentPathPolicy::baseline()); + } + plan.insert_read_write_path( + GPU_PROC_READ_WRITE, + EnrichmentPathPolicy::gpu_proc(EnrichmentPathSources::baseline()), + ); + plan + } + + fn cdi_gpu(requirements: &openshell_core::cdi::CdiDerivedRequirements) -> Self { + let mut plan = Self::default(); + for path in &requirements.read_only_mount_paths { + plan.insert_read_only_path(path, EnrichmentPathPolicy::runtime_mount()); + } + for path in &requirements.device_node_paths { + plan.insert_read_write_path(path, EnrichmentPathPolicy::runtime_device_node()); + } + for path in &requirements.read_write_mount_paths { + plan.insert_read_write_path(path, EnrichmentPathPolicy::runtime_writable_mount()); + } + plan.insert_read_write_path( + GPU_PROC_READ_WRITE, + EnrichmentPathPolicy::gpu_proc(EnrichmentPathSources::runtime()), + ); + for gid in &requirements.additional_gids { + plan.insert_additional_gid(*gid); + } + plan + } + + fn from_baseline_paths(read_only: &[&str], read_write: &[&str]) -> Self { + let mut plan = Self::default(); + for &path in read_only { + plan.insert_read_only_path(path, EnrichmentPathPolicy::baseline()); + } + for &path in read_write { + plan.insert_read_write_path(path, EnrichmentPathPolicy::baseline()); + } + plan + } + + fn merge(mut self, other: Self) -> Self { + for (path, policy) in other.read_only { + self.insert_read_only_path(path, policy); + } + for (path, policy) in other.read_write { + self.insert_read_write_path(path, policy); + } + for gid in other.additional_gids { + self.insert_additional_gid(gid); + } + self + } + + fn insert_read_only_path(&mut self, path: impl Into, policy: EnrichmentPathPolicy) { + insert_enrichment_path(&mut self.read_only, path.into(), policy); + } + + fn insert_read_write_path(&mut self, path: impl Into, policy: EnrichmentPathPolicy) { + insert_enrichment_path(&mut self.read_write, path.into(), policy); + } + + fn insert_additional_gid(&mut self, gid: u32) { + self.additional_gids.insert(gid); + } + + fn entries(&self) -> (Vec, Vec) { + let mut read_write = self.read_write.clone(); + let mut read_only = Vec::new(); + + // A path promoted to read_write (e.g. /proc for GPU) should not also + // appear in read_only — Landlock handles the overlap correctly but the + // duplicate is confusing when inspecting the effective policy. + for (path, policy) in &self.read_only { + if let Some(read_write_policy) = read_write.get_mut(path) { + read_write_policy.merge(*policy); + } else { + read_only.push(EnrichmentPath { + path: path.clone(), + policy: *policy, + }); + } + } + + let read_write = read_write + .into_iter() + .map(|(path, policy)| EnrichmentPath { path, policy }) + .collect(); + (read_only, read_write) + } + + fn has_additional_gids(&self) -> bool { + !self.additional_gids.is_empty() + } + + fn additional_gids(&self) -> Vec { + self.additional_gids.iter().copied().collect() + } + + fn apply_to_proto_policy( + &self, + proto: &mut openshell_core::proto::SandboxPolicy, + ) -> Result { + // 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). + self.apply_to_proto_policy_with(proto, |path| std::path::Path::new(path).exists()) + } + + fn apply_to_proto_policy_with( + &self, + proto: &mut openshell_core::proto::SandboxPolicy, + path_exists: F, + ) -> Result + where + F: Fn(&str) -> bool, + { + let (read_only, read_write) = self.entries(); + if read_only.is_empty() && read_write.is_empty() { + return Ok(EnrichmentApplication::default()); + } + + let fs = proto + .filesystem + .get_or_insert_with(|| openshell_core::proto::FilesystemPolicy { + include_workdir: true, + ..Default::default() + }); + + let mut application = EnrichmentApplication::default(); + for addition in read_only { + if !addition.should_apply("read_only", &path_exists)? { continue; } - fs.read_only.push(path.clone()); - modified = true; + if !fs.read_only.iter().any(|p| p == &addition.path) + && !fs.read_write.iter().any(|p| p == &addition.path) + { + fs.read_only.push(addition.path); + application.record(addition.policy.sources); + } } + for addition in read_write { + if !addition.should_apply("read_write", &path_exists)? { + continue; + } + if fs.read_write.iter().any(|p| p == &addition.path) { + continue; + } + if fs.read_only.iter().any(|p| p == &addition.path) { + match addition.policy.read_only_conflict { + ReadOnlyConflictBehavior::KeepReadOnly => {} + ReadOnlyConflictBehavior::PromoteToReadWrite => { + info!( + path = %addition.path, + "Promoting /proc from read-only to read-write for GPU runtime compatibility" + ); + fs.read_only.retain(|p| p != &addition.path); + fs.read_write.push(addition.path); + application.record(addition.policy.sources); + } + ReadOnlyConflictBehavior::Reject => { + return Err(miette::miette!( + "Runtime-derived read-write path '{}' conflicts with sandbox policy read_only", + addition.path + )); + } + } + continue; + } + fs.read_write.push(addition.path); + application.record(addition.policy.sources); + } + + Ok(application) } - for path in rw { - if fs.read_write.iter().any(|p| p == path) { - continue; + + fn apply_to_sandbox_policy(&self, policy: &mut SandboxPolicy) -> Result { + let (read_only, read_write) = self.entries(); + let mut application = EnrichmentApplication::default(); + + for addition in read_only { + if !addition.should_apply("read_only", &|path| std::path::Path::new(path).exists())? { + continue; + } + let p = PathBuf::from(&addition.path); + if !policy.filesystem.read_only.contains(&p) + && !policy.filesystem.read_write.contains(&p) + { + policy.filesystem.read_only.push(p); + application.record(addition.policy.sources); + } } - if !path_exists(path) { - debug!( - path, - "Baseline read-write path does not exist, skipping enrichment" - ); - continue; + for addition in read_write { + if !addition.should_apply("read_write", &|path| std::path::Path::new(path).exists())? { + continue; + } + let p = PathBuf::from(&addition.path); + if policy.filesystem.read_write.contains(&p) { + continue; + } + if policy.filesystem.read_only.contains(&p) { + match addition.policy.read_only_conflict { + ReadOnlyConflictBehavior::KeepReadOnly => {} + ReadOnlyConflictBehavior::PromoteToReadWrite => { + info!( + path = %addition.path, + "Promoting /proc from read-only to read-write for GPU runtime compatibility" + ); + policy + .filesystem + .read_only + .retain(|existing| existing != &p); + policy.filesystem.read_write.push(p); + application.record(addition.policy.sources); + } + ReadOnlyConflictBehavior::Reject => { + return Err(miette::miette!( + "Runtime-derived read-write path '{}' conflicts with sandbox policy read_only", + addition.path + )); + } + } + continue; + } + policy.filesystem.read_write.push(p); + application.record(addition.policy.sources); } - if fs.read_only.iter().any(|p| p == path) { - if path == "/proc" { - info!( - path, - "Promoting /proc from read-only to read-write for GPU runtime compatibility" - ); - fs.read_only.retain(|p| p != path); - fs.read_write.push(path.clone()); - modified = true; + + if self.has_additional_gids() { + let additional_gids = self.additional_gids(); + if policy.process.supplemental_groups != additional_gids { + // CDI calls these `additionalGids`; the supervisor applies them + // as Linux supplemental groups before dropping privileges. + policy.process.supplemental_groups = additional_gids; + application.record(EnrichmentPathSources::runtime()); } - continue; } - fs.read_write.push(path.clone()); - modified = true; + + Ok(application) } - modified + #[cfg(test)] + fn paths(&self) -> (Vec, Vec) { + let (read_only, read_write) = self.entries(); + ( + read_only.into_iter().map(|path| path.path).collect(), + read_write.into_iter().map(|path| path.path).collect(), + ) + } } -/// 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 GPU devices are present in the container. /// -/// 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() - }); +/// Checks both the native Linux NVIDIA control device (`/dev/nvidiactl`) and +/// the WSL2 DXG device (`/dev/dxg`) for the legacy fallback path. +fn has_gpu_devices() -> bool { + std::path::Path::new("/dev/nvidiactl").exists() || std::path::Path::new("/dev/dxg").exists() +} - if modified { - ocsf_emit!( - ConfigStateChangeBuilder::new(ocsf_ctx()) - .severity(SeverityId::Informational) - .status(StatusId::Success) - .state(StateId::Enabled, "enriched") - .message("Enriched policy with baseline filesystem paths for proxy mode") - .build() - ); +/// Enumerate per-GPU device nodes (`/dev/nvidia0`, `/dev/nvidia1`, …). +fn enumerate_gpu_device_nodes() -> Vec { + let mut paths = Vec::new(); + if let Ok(entries) = std::fs::read_dir("/dev") { + for entry in entries.flatten() { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if let Some(suffix) = name.strip_prefix("nvidia") { + if suffix.is_empty() || !suffix.chars().all(|c| c.is_ascii_digit()) { + continue; + } + paths.push(entry.path().to_string_lossy().into_owned()); + } + } } + paths +} - modified +fn insert_enrichment_path( + paths: &mut BTreeMap, + path: String, + policy: EnrichmentPathPolicy, +) { + if let Some(existing) = paths.get_mut(&path) { + existing.merge(policy); + } else { + paths.insert(path, policy); + } +} + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +struct EnrichmentApplication { + baseline_modified: bool, + runtime_modified: bool, +} + +impl EnrichmentApplication { + fn modified(self) -> bool { + self.baseline_modified || self.runtime_modified + } + + fn record(&mut self, sources: EnrichmentPathSources) { + if sources.baseline { + self.baseline_modified = true; + } + if sources.runtime { + self.runtime_modified = true; + } + } +} + +/// 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) { + EnrichmentPlan::active(true, None).paths() } fn strip_proto_provider_policy_entries(proto: &mut openshell_core::proto::SandboxPolicy) -> bool { @@ -1472,59 +1808,315 @@ fn proto_sync_payload_for_enriched_policy( Some(sync_policy) } -/// Ensure a `SandboxPolicy` (Rust type) includes the baseline filesystem -/// 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) { - let (ro, rw) = - active_baseline_enrichment_paths(matches!(policy.network.mode, NetworkMode::Proxy)); - if ro.is_empty() && rw.is_empty() { - return; +fn cdi_writable_file_allowlist_from_proto( + proto: &openshell_core::proto::SandboxPolicy, +) -> HashSet { + proto + .filesystem + .as_ref() + .map(|fs| fs.read_write.iter().cloned().collect()) + .unwrap_or_default() +} + +fn cdi_writable_file_allowlist_from_policy(policy: &SandboxPolicy) -> HashSet { + policy + .filesystem + .read_write + .iter() + .map(|path| path.to_string_lossy().into_owned()) + .collect() +} + +struct CdiPolicyEnrichment { + enriched: bool, + additional_gids: Vec, +} + +fn enrich_sandbox_policy_with_baseline_and_cdi(policy: &mut SandboxPolicy) -> Result<()> { + let cdi_writable_file_allowlist = cdi_writable_file_allowlist_from_policy(policy); + let requirements = resolve_cdi_requirements_from_env(&cdi_writable_file_allowlist)?; + let plan = EnrichmentPlan::for_sandbox_policy(policy, requirements.as_ref()); + let application = plan.apply_to_sandbox_policy(policy)?; + emit_policy_enrichment_events(application, requirements.as_ref(), &plan); + Ok(()) +} + +fn enrich_proto_policy_with_baseline_and_cdi( + proto: &mut openshell_core::proto::SandboxPolicy, +) -> Result { + let cdi_writable_file_allowlist = cdi_writable_file_allowlist_from_proto(proto); + let requirements = resolve_cdi_requirements_from_env(&cdi_writable_file_allowlist)?; + let plan = EnrichmentPlan::for_proto_policy(proto, requirements.as_ref()); + let application = plan.apply_to_proto_policy(proto)?; + emit_policy_enrichment_events(application, requirements.as_ref(), &plan); + + Ok(CdiPolicyEnrichment { + enriched: application.modified(), + additional_gids: plan.additional_gids(), + }) +} + +fn resolve_cdi_requirements_from_env( + writable_file_allowlist: &HashSet, +) -> Result> { + let Ok(context_path) = std::env::var(openshell_core::sandbox_env::CDI_CONTEXT) else { + return Ok(None); + }; + let context_path = context_path.trim(); + if context_path.is_empty() { + return Ok(None); + } + if context_path != openshell_core::cdi::CDI_CONTEXT_PATH { + let message = format!( + "CDI context path must be '{}', got '{context_path}'", + openshell_core::cdi::CDI_CONTEXT_PATH + ); + emit_cdi_validation_failure(&message); + return Err(miette::miette!("{message}")); } - let mut modified = false; - for path in &ro { - let p = std::path::PathBuf::from(path); - if !policy.filesystem.read_only.contains(&p) && !policy.filesystem.read_write.contains(&p) { - if !p.exists() { - debug!( - path, - "Baseline read-only path does not exist, skipping enrichment" - ); - continue; - } - policy.filesystem.read_only.push(p); - modified = true; + let context = openshell_core::cdi::read_context(context_path).map_err(|err| { + emit_cdi_validation_failure(&err.to_string()); + miette::miette!("Failed to load CDI context from {context_path}: {err}") + })?; + validate_cdi_context_projection(&context)?; + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "loaded") + .unmapped( + "cdi_selected_device_count", + serde_json::json!(context.selected_devices.len()) + ) + .message(format!("Loaded CDI context [path:{context_path}]")) + .build() + ); + + let requirements = openshell_core::cdi::resolve_cdi_context(&context, writable_file_allowlist) + .map_err(|err| { + emit_cdi_validation_failure(&err.to_string()); + miette::miette!("Failed to resolve CDI requirements: {err}") + })?; + Ok(Some(requirements)) +} + +fn validate_cdi_context_projection(context: &openshell_core::cdi::CdiContext) -> Result<()> { + if !context + .selected_devices + .iter() + .any(|device| !device.trim().is_empty()) + { + let message = "CDI context must select at least one device"; + emit_cdi_validation_failure(message); + return Err(miette::miette!(message)); + } + if context.spec_dirs.is_empty() { + let message = "CDI context must list at least one spec directory"; + emit_cdi_validation_failure(message); + return Err(miette::miette!(message)); + } + let expected_prefix = format!("{}/", openshell_core::cdi::CDI_SPEC_DIR_BASE); + for (index, spec_dir) in context.spec_dirs.iter().enumerate() { + let normalized = openshell_core::paths::normalize_path(&spec_dir.path); + if !normalized.starts_with(&expected_prefix) { + let message = format!( + "CDI spec dir must be under '{}', got '{}'", + openshell_core::cdi::CDI_SPEC_DIR_BASE, + spec_dir.path + ); + emit_cdi_validation_failure(&message); + return Err(miette::miette!("{message}")); + } + let expected_path = cdi_spec_mount_path(index); + if spec_dir.path != expected_path { + let message = format!( + "CDI spec dir at index {index} must be '{expected_path}', got '{}'", + spec_dir.path + ); + emit_cdi_validation_failure(&message); + return Err(miette::miette!("{message}")); } } - for path in &rw { - let p = std::path::PathBuf::from(path); - if policy.filesystem.read_only.contains(&p) || policy.filesystem.read_write.contains(&p) { - continue; + validate_cdi_projection_mounts(context) +} + +#[cfg(target_os = "linux")] +fn validate_cdi_projection_mounts(context: &openshell_core::cdi::CdiContext) -> Result<()> { + let mounts = read_mount_info()?; + validate_cdi_projection_mounts_with(context, &mounts) +} + +#[cfg(target_os = "linux")] +fn validate_cdi_projection_mounts_with( + context: &openshell_core::cdi::CdiContext, + mounts: &[MountInfoEntry], +) -> Result<()> { + let mut expected_mounts = Vec::with_capacity(context.spec_dirs.len() + 1); + expected_mounts.push(openshell_core::cdi::CDI_CONTEXT_PATH); + expected_mounts.extend( + context + .spec_dirs + .iter() + .map(|spec_dir| spec_dir.path.as_str()), + ); + + for expected_path in expected_mounts { + let Some(mount) = mounts + .iter() + .find(|mount| mount.mount_point == expected_path) + else { + let message = format!("CDI projection path '{expected_path}' is not a mount"); + emit_cdi_validation_failure(&message); + return Err(miette::miette!("{message}")); + }; + if !mount.read_only { + let message = format!("CDI projection mount '{expected_path}' must be read-only"); + emit_cdi_validation_failure(&message); + return Err(miette::miette!("{message}")); } - if !p.exists() { - debug!( - path, - "Baseline read-write path does not exist, skipping enrichment" - ); + } + Ok(()) +} + +#[cfg(not(target_os = "linux"))] +#[expect( + clippy::unnecessary_wraps, + reason = "matches the Linux implementation so shared CDI validation can propagate errors uniformly" +)] +fn validate_cdi_projection_mounts(_context: &openshell_core::cdi::CdiContext) -> Result<()> { + Ok(()) +} + +fn cdi_spec_mount_path(index: usize) -> String { + format!("{}/{index}", openshell_core::cdi::CDI_SPEC_DIR_BASE) +} + +#[cfg(target_os = "linux")] +#[derive(Debug, PartialEq, Eq)] +struct MountInfoEntry { + mount_point: String, + read_only: bool, +} + +#[cfg(target_os = "linux")] +fn read_mount_info() -> Result> { + let mount_info = std::fs::read_to_string("/proc/self/mountinfo") + .into_diagnostic() + .wrap_err("read /proc/self/mountinfo")?; + mount_info.lines().map(parse_mount_info_entry).collect() +} + +#[cfg(target_os = "linux")] +fn parse_mount_info_entry(line: &str) -> Result { + let fields = line.split_whitespace().collect::>(); + let mount_point = fields + .get(4) + .ok_or_else(|| miette::miette!("mountinfo entry is missing a mount point: {line}"))?; + let mount_options = fields + .get(5) + .ok_or_else(|| miette::miette!("mountinfo entry is missing mount options: {line}"))?; + Ok(MountInfoEntry { + mount_point: unescape_mount_info_path(mount_point)?, + read_only: mount_options.split(',').any(|option| option == "ro"), + }) +} + +#[cfg(target_os = "linux")] +fn unescape_mount_info_path(path: &str) -> Result { + let mut decoded = Vec::with_capacity(path.len()); + let bytes = path.as_bytes(); + let mut index = 0; + while index < bytes.len() { + if bytes[index] != b'\\' { + decoded.push(bytes[index]); + index += 1; continue; } - policy.filesystem.read_write.push(p); - modified = true; + let octal = bytes + .get(index + 1..index + 4) + .ok_or_else(|| miette::miette!("invalid mountinfo path escape in '{path}'"))?; + if !octal.iter().all(|byte| matches!(byte, b'0'..=b'7')) { + return Err(miette::miette!("invalid mountinfo path escape in '{path}'")); + } + decoded.push((octal[0] - b'0') * 64 + (octal[1] - b'0') * 8 + (octal[2] - b'0')); + index += 4; } + String::from_utf8(decoded).into_diagnostic() +} - if modified { - ocsf_emit!( - ConfigStateChangeBuilder::new(ocsf_ctx()) - .severity(SeverityId::Informational) - .status(StatusId::Success) - .state(StateId::Enabled, "enriched") - .message("Enriched policy with baseline filesystem paths for proxy mode") - .build() - ); +fn emit_cdi_validation_failure(message: &str) { + ocsf_emit!( + DetectionFindingBuilder::new(ocsf_ctx()) + .activity(ActivityId::Open) + .severity(SeverityId::High) + .action(ActionId::Denied) + .disposition(DispositionId::Blocked) + .finding_info( + FindingInfo::new( + "cdi-policy-validation-failed", + "CDI Policy Validation Failed", + ) + .with_desc(message), + ) + .message(format!("CDI validation failed: {message}")) + .build() + ); +} + +fn emit_baseline_enrichment_event() { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "enriched") + .message("Enriched policy with baseline filesystem paths") + .build() + ); +} + +fn emit_policy_enrichment_events( + application: EnrichmentApplication, + cdi_requirements: Option<&openshell_core::cdi::CdiDerivedRequirements>, + plan: &EnrichmentPlan, +) { + if application.baseline_modified { + emit_baseline_enrichment_event(); + } + if let Some(requirements) = cdi_requirements + && (application.runtime_modified || plan.has_additional_gids()) + { + emit_cdi_enrichment_event(requirements); } } +fn emit_cdi_enrichment_event(requirements: &openshell_core::cdi::CdiDerivedRequirements) { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "enriched") + .unmapped( + "cdi_device_node_count", + serde_json::json!(requirements.device_node_paths.len()) + ) + .unmapped( + "cdi_read_only_mount_count", + serde_json::json!(requirements.read_only_mount_paths.len()) + ) + .unmapped( + "cdi_read_write_mount_count", + serde_json::json!(requirements.read_write_mount_paths.len()) + ) + .unmapped( + "cdi_additional_gid_count", + serde_json::json!(requirements.additional_gids.len()) + ) + .message("Enriched policy with CDI-derived filesystem and process requirements") + .build() + ); +} + #[cfg(test)] #[allow( clippy::needless_raw_string_hashes, @@ -1623,7 +2215,8 @@ mod baseline_tests { }, ); - enrich_proto_baseline_paths(&mut policy); + let plan = EnrichmentPlan::for_proto_policy(&policy, None); + plan.apply_to_proto_policy(&mut policy).unwrap(); let filesystem = policy.filesystem.expect("filesystem policy"); assert!( @@ -1726,16 +2319,17 @@ mod baseline_tests { 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 plan = EnrichmentPlan::legacy_gpu_fallback(vec!["/dev/nvidia0".to_string()]); - let enriched = enrich_proto_baseline_paths_with(&mut policy, &ro, &rw, |path| { - matches!(path, "/proc" | "/dev/nvidia0") - }); + let application = plan + .apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/proc" | "/dev/nvidia0") + }) + .unwrap(); let filesystem = policy.filesystem.expect("filesystem policy"); assert!( - enriched, + application.modified(), "GPU enrichment should not require network policies" ); assert!( @@ -1753,13 +2347,235 @@ mod baseline_tests { } #[test] - fn gpu_baseline_read_write_contains_dxg() { - // /dev/dxg must be present so WSL2 sandboxes get the Landlock - // read-write rule for the CDI-injected DXG device. The existence - // check in enrich_proto_baseline_paths() skips it on native Linux. + fn cdi_gpu_baseline_uses_proc_without_legacy_gpu_paths() { + let requirements = openshell_core::cdi::CdiDerivedRequirements::default(); + let (ro, rw) = EnrichmentPlan::cdi_gpu(&requirements).paths(); + + assert!( + rw.contains(&GPU_PROC_READ_WRITE.to_string()), + "CDI GPU baseline should keep the CUDA procfs write exception" + ); + for path in [ + "/dev/nvidiactl", + "/dev/nvidia-uvm", + "/dev/dxg", + "/dev/nvidia0", + ] { + assert!( + !rw.contains(&path.to_string()), + "CDI GPU baseline should not include legacy read-write path {path}" + ); + } + for path in ["/run/nvidia-persistenced", "/usr/lib/wsl"] { + assert!( + !ro.contains(&path.to_string()), + "CDI GPU baseline should not include legacy read-only path {path}" + ); + } + } + + #[test] + fn active_cdi_gpu_baseline_ignores_detected_gpu_devices() { + let requirements = openshell_core::cdi::CdiDerivedRequirements::default(); + let (ro, rw) = EnrichmentPlan::active(false, Some(&requirements)).paths(); + + assert!( + ro.is_empty(), + "CDI-only GPU baseline should not add read-only legacy paths" + ); + assert_eq!(rw, vec![GPU_PROC_READ_WRITE.to_string()]); + } + + #[test] + fn enrichment_plan_composes_proxy_baseline_and_cdi_gpu_paths() { + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/dxg".to_string()], + read_only_mount_paths: vec!["/usr/lib/wsl/lib/libcuda.so.1".to_string()], + read_write_mount_paths: Vec::new(), + additional_gids: vec![44], + }; + let plan = EnrichmentPlan::proxy_baseline().merge(EnrichmentPlan::cdi_gpu(&requirements)); + let (ro, rw) = plan.paths(); + + assert!(ro.contains(&"/usr".to_string())); + assert!(ro.contains(&"/usr/lib/wsl/lib/libcuda.so.1".to_string())); + assert!(rw.contains(&"/tmp".to_string())); + assert!(rw.contains(&"/dev/dxg".to_string())); + assert!(rw.contains(&GPU_PROC_READ_WRITE.to_string())); + assert!( + !ro.contains(&GPU_PROC_READ_WRITE.to_string()), + "read_write /proc should normalize away proxy read_only /proc" + ); + assert_eq!(plan.additional_gids(), vec![44]); + } + + #[test] + fn enrichment_plan_merges_baseline_and_runtime_path_sources() { + let mut plan = EnrichmentPlan::default(); + plan.insert_read_write_path("/shared", EnrichmentPathPolicy::baseline()); + plan.insert_read_write_path("/shared", EnrichmentPathPolicy::runtime_device_node()); + + let (_ro, rw) = plan.entries(); + assert_eq!(rw.len(), 1); + assert!(rw[0].policy.sources.baseline); + assert!(rw[0].policy.sources.runtime); + } + + #[test] + fn proto_cdi_enrichment_adds_derived_paths() { + let mut policy = openshell_policy::restrictive_default_policy(); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/dxg".to_string()], + read_only_mount_paths: vec!["/usr/lib/wsl/lib/libcuda.so.1".to_string()], + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + assert!( + plan.apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/usr/lib/wsl/lib/libcuda.so.1" | "/dev/dxg" | "/proc") + }) + .unwrap() + .modified() + ); + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!( + filesystem + .read_only + .contains(&"/usr/lib/wsl/lib/libcuda.so.1".to_string()) + ); + assert!(filesystem.read_write.contains(&"/dev/dxg".to_string())); + } + + #[test] + fn proto_cdi_enrichment_errors_for_missing_runtime_path() { + let mut policy = openshell_policy::restrictive_default_policy(); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/dxg".to_string()], + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + let err = plan + .apply_to_proto_policy_with(&mut policy, |path| path == "/proc") + .unwrap_err(); + assert!( + err.to_string().contains("does not exist"), + "unexpected error: {err}" + ); + } + + #[test] + fn proto_cdi_device_node_conflict_keeps_explicit_read_only() { + let mut policy = openshell_policy::restrictive_default_policy(); + policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_only: vec!["/dev/nvidia0".to_string()], + read_write: Vec::new(), + include_workdir: false, + }); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/dev/nvidia0".to_string()], + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + plan.apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/dev/nvidia0" | "/proc") + }) + .unwrap(); + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!( + filesystem.read_only.contains(&"/dev/nvidia0".to_string()), + "CDI device nodes should not promote existing read_only policy entries" + ); + assert!( + !filesystem.read_write.contains(&"/dev/nvidia0".to_string()), + "existing read_only device node should remain read_only" + ); + } + + #[test] + fn proto_cdi_writable_mount_rejects_read_write_conflict() { + let mut policy = openshell_policy::restrictive_default_policy(); + policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_only: vec!["/opt/nvidia/cache.db".to_string()], + read_write: Vec::new(), + include_workdir: false, + }); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: Vec::new(), + read_only_mount_paths: Vec::new(), + read_write_mount_paths: vec!["/opt/nvidia/cache.db".to_string()], + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + let err = plan + .apply_to_proto_policy_with(&mut policy, |path| { + matches!(path, "/opt/nvidia/cache.db" | "/proc") + }) + .unwrap_err(); + assert!( + err.to_string().contains("conflicts"), + "unexpected error: {err}" + ); + } + + #[test] + fn proto_cdi_enrichment_promotes_proc_read_write() { + let mut policy = openshell_policy::restrictive_default_policy(); + policy.filesystem = Some(openshell_core::proto::FilesystemPolicy { + read_only: vec!["/proc".to_string()], + read_write: Vec::new(), + include_workdir: false, + }); + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: vec!["/proc".to_string()], + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: Vec::new(), + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + assert!(plan.apply_to_proto_policy(&mut policy).unwrap().modified()); + let filesystem = policy.filesystem.expect("filesystem policy"); + assert!(!filesystem.read_only.contains(&"/proc".to_string())); + assert!(filesystem.read_write.contains(&"/proc".to_string())); + } + + #[test] + fn local_cdi_enrichment_applies_additional_gids_as_supplemental_groups() { + let mut policy = SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy::default(), + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process: ProcessPolicy::default(), + }; + let requirements = openshell_core::cdi::CdiDerivedRequirements { + device_node_paths: Vec::new(), + read_only_mount_paths: Vec::new(), + read_write_mount_paths: Vec::new(), + additional_gids: vec![44, 107], + }; + + let plan = EnrichmentPlan::cdi_gpu(&requirements); + plan.apply_to_sandbox_policy(&mut policy).unwrap(); + assert_eq!(policy.process.supplemental_groups, vec![44, 107]); + } + + #[test] + fn legacy_gpu_baseline_read_write_contains_dxg() { + // /dev/dxg stays in the legacy device-scan fallback. CDI mode derives + // device nodes from the selected specs instead of this list. assert!( GPU_BASELINE_READ_WRITE.contains(&"/dev/dxg"), - "/dev/dxg must be in GPU_BASELINE_READ_WRITE for WSL2 support" + "/dev/dxg should remain in the legacy GPU read-write fallback" ); } @@ -1768,7 +2584,7 @@ mod baseline_tests { let mut policy = SandboxPolicy { version: 1, filesystem: FilesystemPolicy { - read_only: vec![std::path::PathBuf::from("/tmp")], + read_only: vec![PathBuf::from("/tmp")], read_write: vec![], include_workdir: false, }, @@ -1780,31 +2596,61 @@ mod baseline_tests { process: ProcessPolicy::default(), }; - enrich_sandbox_baseline_paths(&mut policy); + let plan = EnrichmentPlan::for_sandbox_policy(&policy, None); + plan.apply_to_sandbox_policy(&mut policy).unwrap(); assert!( - policy - .filesystem - .read_only - .contains(&std::path::PathBuf::from("/tmp")), + policy.filesystem.read_only.contains(&PathBuf::from("/tmp")), "explicit read_only baseline path should be preserved" ); assert!( !policy .filesystem .read_write - .contains(&std::path::PathBuf::from("/tmp")), + .contains(&PathBuf::from("/tmp")), "baseline enrichment must not promote explicit read_only /tmp to read_write" ); } #[test] - fn gpu_baseline_read_only_contains_usr_lib_wsl() { - // /usr/lib/wsl must be present so CDI-injected WSL2 GPU library - // bind-mounts are accessible under Landlock. Skipped on native Linux. + fn legacy_gpu_baseline_read_only_contains_usr_lib_wsl() { + // /usr/lib/wsl stays in the legacy device-scan fallback. CDI mode uses + // resolved mount destinations instead of this broad directory. assert!( GPU_BASELINE_READ_ONLY.contains(&"/usr/lib/wsl"), - "/usr/lib/wsl must be in GPU_BASELINE_READ_ONLY for WSL2 CDI library paths" + "/usr/lib/wsl should remain in the legacy GPU read-only fallback" + ); + } + + #[test] + fn local_cdi_baseline_promotes_proc_read_write() { + let mut policy = SandboxPolicy { + version: 1, + filesystem: FilesystemPolicy { + read_only: vec![PathBuf::from(GPU_PROC_READ_WRITE)], + read_write: Vec::new(), + include_workdir: false, + }, + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process: ProcessPolicy::default(), + }; + + let requirements = openshell_core::cdi::CdiDerivedRequirements::default(); + let plan = EnrichmentPlan::for_sandbox_policy(&policy, Some(&requirements)); + plan.apply_to_sandbox_policy(&mut policy).unwrap(); + + assert!( + !policy + .filesystem + .read_only + .contains(&PathBuf::from(GPU_PROC_READ_WRITE)) + ); + assert!( + policy + .filesystem + .read_write + .contains(&PathBuf::from(GPU_PROC_READ_WRITE)) ); } @@ -1950,7 +2796,7 @@ async fn load_policy( landlock: config.landlock, process: config.process, }; - enrich_sandbox_baseline_paths(&mut policy); + enrich_sandbox_policy_with_baseline_and_cdi(&mut policy)?; // File mode has no operator-registered middleware to connect. return Ok(( policy, @@ -1992,7 +2838,7 @@ async fn load_policy( let mut discovered = discover_policy_from_disk_or_default(); // Enrich before syncing so the gateway baseline includes // baseline paths from the start. - enrich_proto_baseline_paths(&mut discovered); + enrich_proto_policy_with_baseline_and_cdi(&mut discovered)?; strip_proto_provider_policy_entries(&mut discovered); let sandbox = sandbox.as_deref().ok_or_else(|| { miette::miette!( @@ -2028,7 +2874,8 @@ async fn load_policy( // Ensure baseline filesystem paths are present for proxy-mode // sandboxes. If the policy was enriched, sync the updated version // back to the gateway so users can see the effective policy. - let enriched = enrich_proto_baseline_paths(&mut proto_policy); + let cdi_enrichment = enrich_proto_policy_with_baseline_and_cdi(&mut proto_policy)?; + let enriched = cdi_enrichment.enriched; let sync_policy = proto_sync_payload_for_enriched_policy(&proto_policy, enriched); if let Some(sync_policy) = sync_policy { if let Some(sandbox_name) = sandbox.as_deref() { @@ -2172,7 +3019,7 @@ async fn load_policy( }; let opa_engine = Some(engine); - let policy = match SandboxPolicy::try_from(proto_policy.clone()) { + let mut policy = match SandboxPolicy::try_from(proto_policy.clone()) { Ok(policy) => policy, Err(e) => { report_initial_policy_failure(endpoint, id, loaded_policy_revision.as_ref(), &e) @@ -2180,6 +3027,9 @@ async fn load_policy( return Err(e); } }; + if !cdi_enrichment.additional_gids.is_empty() { + policy.process.supplemental_groups = cdi_enrichment.additional_gids; + } return Ok(( policy, opa_engine, @@ -2987,7 +3837,7 @@ fn retain_extension_credentials( .map(|service| service.name.as_str()) .collect() } else { - std::collections::HashSet::default() + HashSet::default() }; store.retain(&retained); } @@ -3997,6 +4847,52 @@ mod tests { } } + #[cfg(target_os = "linux")] + #[test] + fn cdi_projection_requires_read_only_context_and_spec_mounts() { + let context = openshell_core::cdi::CdiContext::new( + vec!["nvidia.com/gpu=0".to_string()], + vec![openshell_core::cdi::CdiSpecDirectory::new( + cdi_spec_mount_path(0), + "/host/cdi", + )], + ); + let mounts = vec![ + MountInfoEntry { + mount_point: openshell_core::cdi::CDI_CONTEXT_PATH.to_string(), + read_only: true, + }, + MountInfoEntry { + mount_point: cdi_spec_mount_path(0), + read_only: true, + }, + ]; + + validate_cdi_projection_mounts_with(&context, &mounts) + .expect("read-only CDI context and spec mounts should be accepted"); + + let mut writable_mounts = mounts; + writable_mounts[1].read_only = false; + let err = validate_cdi_projection_mounts_with(&context, &writable_mounts) + .expect_err("writable CDI spec mount must be rejected"); + assert!(err.to_string().contains("must be read-only")); + } + + #[cfg(target_os = "linux")] + #[test] + fn parse_mount_info_entry_decodes_mount_path_and_access_mode() { + let mount = parse_mount_info_entry( + "42 35 0:42 / /run/openshell/supervisor/cdi-specs/0\\040with-space ro,nosuid - tmpfs tmpfs rw", + ) + .expect("mountinfo fixture should parse"); + + assert_eq!( + mount.mount_point, + "/run/openshell/supervisor/cdi-specs/0 with-space" + ); + assert!(mount.read_only); + } + #[test] fn sidecar_process_policy_sets_loopback_proxy_addr() { let policy = proxy_policy(None); @@ -4131,8 +5027,8 @@ mod tests { apply_agent_proposals_enabled(&agent_proposals, true, "test", Some(1), None, || { installs.fetch_add(1, Ordering::Relaxed); Ok(skills::InstalledSkills { - policy_advisor: std::path::PathBuf::from("/tmp/policy_advisor.md"), - policy_advisor_skill: std::path::PathBuf::from("/tmp/SKILL.md"), + policy_advisor: PathBuf::from("/tmp/policy_advisor.md"), + policy_advisor_skill: PathBuf::from("/tmp/SKILL.md"), agents: None, }) }); @@ -4142,8 +5038,8 @@ mod tests { apply_agent_proposals_enabled(&agent_proposals, true, "test", Some(2), None, || { installs.fetch_add(1, Ordering::Relaxed); Ok(skills::InstalledSkills { - policy_advisor: std::path::PathBuf::from("/tmp/policy_advisor.md"), - policy_advisor_skill: std::path::PathBuf::from("/tmp/SKILL.md"), + policy_advisor: PathBuf::from("/tmp/policy_advisor.md"), + policy_advisor_skill: PathBuf::from("/tmp/SKILL.md"), agents: None, }) }); @@ -4152,8 +5048,8 @@ mod tests { apply_agent_proposals_enabled(&agent_proposals, false, "test", Some(3), None, || { installs.fetch_add(1, Ordering::Relaxed); Ok(skills::InstalledSkills { - policy_advisor: std::path::PathBuf::from("/tmp/policy_advisor.md"), - policy_advisor_skill: std::path::PathBuf::from("/tmp/SKILL.md"), + policy_advisor: PathBuf::from("/tmp/policy_advisor.md"), + policy_advisor_skill: PathBuf::from("/tmp/SKILL.md"), agents: None, }) }); diff --git a/crates/openshell-supervisor-network/src/opa.rs b/crates/openshell-supervisor-network/src/opa.rs index 33c14e9bd8..0ea7d850b8 100644 --- a/crates/openshell-supervisor-network/src/opa.rs +++ b/crates/openshell-supervisor-network/src/opa.rs @@ -1227,6 +1227,7 @@ fn parse_process_policy(val: ®orus::Value) -> ProcessPolicy { ProcessPolicy { run_as_user: get_str(val, "run_as_user"), run_as_group: get_str(val, "run_as_group"), + supplemental_groups: Vec::new(), } } diff --git a/crates/openshell-supervisor-process/src/child_env.rs b/crates/openshell-supervisor-process/src/child_env.rs index 32eecbee35..0d176f427f 100644 --- a/crates/openshell-supervisor-process/src/child_env.rs +++ b/crates/openshell-supervisor-process/src/child_env.rs @@ -1,9 +1,277 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +use std::io::{Read, Write}; use std::path::Path; +use openshell_core::policy::SandboxPolicy; + const LOCAL_NO_PROXY: &str = "127.0.0.1,localhost,::1"; +pub const DEFAULT_CHILD_PATH: &str = "/usr/local/bin:/usr/bin:/bin"; +const STANDARD_SBIN_PATHS: &[&str] = &["/usr/local/sbin", "/usr/sbin", "/sbin"]; +const ENSURE_STANDARD_SBIN_PATHS_SCRIPT: &str = "for dir in /usr/local/sbin /usr/sbin /sbin; do case \":${PATH:-}:\" in *:\"$dir\":*) ;; *) PATH=\"${PATH:+$PATH:}$dir\" ;; esac; done; export PATH"; +const STARTUP_SNIPPET_MARKER: &str = "# OpenShell standard sbin PATH"; +const PROFILE_D_SNIPPET_PATH: &str = "/etc/profile.d/openshell-standard-sbin-path.sh"; + +enum StartupFile { + Missing, + Regular(String), + Unsafe, +} + +pub fn standard_sbin_path_repair_enabled(policy: &SandboxPolicy) -> bool { + let cdi_context = std::env::var(openshell_core::sandbox_env::CDI_CONTEXT); + standard_sbin_path_repair_enabled_for_context(policy, cdi_context.as_deref().ok()) +} + +fn standard_sbin_path_repair_enabled_for_context( + policy: &SandboxPolicy, + cdi_context: Option<&str>, +) -> bool { + cdi_context + .map(str::trim) + .is_some_and(|path| path == openshell_core::cdi::CDI_CONTEXT_PATH) + && policy_has_standard_sbin_path(policy) +} + +fn policy_has_standard_sbin_path(policy: &SandboxPolicy) -> bool { + policy + .filesystem + .read_only + .iter() + .chain(policy.filesystem.read_write.iter()) + .any(|path| STANDARD_SBIN_PATHS.iter().any(|dir| path.starts_with(dir))) +} + +pub fn child_path_from_env(repair_standard_sbin: bool) -> String { + let path = std::env::var("PATH") + .ok() + .filter(|path| !path.trim().is_empty()) + .unwrap_or_else(|| DEFAULT_CHILD_PATH.to_string()); + + maybe_path_with_standard_sbin_paths(&path, repair_standard_sbin) +} + +pub fn maybe_path_with_standard_sbin_paths(path: &str, repair_standard_sbin: bool) -> String { + if repair_standard_sbin { + path_with_standard_sbin_paths(path) + } else { + path.to_string() + } +} + +pub fn path_with_standard_sbin_paths(path: &str) -> String { + let mut path = if path.trim().is_empty() { + DEFAULT_CHILD_PATH.to_string() + } else { + path.to_string() + }; + + for dir in STANDARD_SBIN_PATHS { + if !path.split(':').any(|entry| entry == *dir) { + if !path.is_empty() { + path.push(':'); + } + path.push_str(dir); + } + } + + path +} + +pub fn shell_command_with_standard_sbin_paths(command: &str) -> String { + format!("{ENSURE_STANDARD_SBIN_PATHS_SCRIPT}\n{command}") +} + +pub fn maybe_shell_command_with_standard_sbin_paths( + command: &str, + repair_standard_sbin: bool, +) -> String { + if repair_standard_sbin { + shell_command_with_standard_sbin_paths(command) + } else { + command.to_string() + } +} + +pub fn install_standard_sbin_path_startup_files(home: Option<&str>) { + let profile_path = Path::new(PROFILE_D_SNIPPET_PATH); + if let Err(error) = write_profile_snippet(profile_path) { + tracing::debug!( + path = %profile_path.display(), + error = %error, + "failed to install OpenShell PATH profile snippet" + ); + } + + if let Some(home) = home { + let bashrc_path = Path::new(home).join(".bashrc"); + if let Err(error) = append_startup_snippet(&bashrc_path) { + tracing::debug!( + path = %bashrc_path.display(), + error = %error, + "failed to install OpenShell PATH shell startup snippet" + ); + } + } +} + +fn startup_snippet() -> String { + format!("{STARTUP_SNIPPET_MARKER}\n{ENSURE_STANDARD_SBIN_PATHS_SCRIPT}\n") +} + +fn write_profile_snippet(path: &Path) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + ensure_directory_without_symlink(parent)?; + } + + let snippet = startup_snippet(); + match read_startup_file(path)? { + StartupFile::Regular(content) if content == snippet => return Ok(()), + StartupFile::Regular(_) | StartupFile::Missing => {} + StartupFile::Unsafe => return Ok(()), + } + write_startup_file(path, &snippet) +} + +fn append_startup_snippet(path: &Path) -> std::io::Result<()> { + if let Some(parent) = path.parent() + && !existing_directory_without_symlink(parent)? + { + return Ok(()); + } + + let existing = match read_startup_file(path)? { + StartupFile::Regular(content) => content, + StartupFile::Missing | StartupFile::Unsafe => return Ok(()), + }; + if existing.contains(STARTUP_SNIPPET_MARKER) { + return Ok(()); + } + + let snippet = startup_snippet(); + let mut file = open_startup_file_for_append(path)?; + if !existing.is_empty() && !existing.ends_with('\n') { + file.write_all(b"\n")?; + } + file.write_all(snippet.as_bytes()) +} + +fn read_startup_file(path: &Path) -> std::io::Result { + let metadata = match std::fs::symlink_metadata(path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return Ok(StartupFile::Missing); + } + Err(error) => return Err(error), + }; + let file_type = metadata.file_type(); + if file_type.is_symlink() || !file_type.is_file() { + tracing::debug!( + path = %path.display(), + "skipping OpenShell PATH startup repair for non-regular file" + ); + return Ok(StartupFile::Unsafe); + } + + let mut content = String::new(); + open_startup_file_for_read(path)?.read_to_string(&mut content)?; + Ok(StartupFile::Regular(content)) +} + +fn ensure_directory_without_symlink(path: &Path) -> std::io::Result<()> { + match std::fs::symlink_metadata(path) { + Ok(metadata) if metadata.file_type().is_symlink() => Err(std::io::Error::other(format!( + "directory '{}' is a symlink", + path.display() + ))), + Ok(metadata) if metadata.is_dir() => Ok(()), + Ok(_) => Err(std::io::Error::other(format!( + "'{}' is not a directory", + path.display() + ))), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + if let Some(parent) = path.parent() { + ensure_directory_without_symlink(parent)?; + } + std::fs::create_dir(path) + } + Err(error) => Err(error), + } +} + +fn existing_directory_without_symlink(path: &Path) -> std::io::Result { + if let Some(parent) = path.parent() + && parent != path + && !parent.as_os_str().is_empty() + && !existing_directory_without_symlink(parent)? + { + return Ok(false); + } + + match std::fs::symlink_metadata(path) { + Ok(metadata) if metadata.file_type().is_symlink() => { + tracing::debug!( + path = %path.display(), + "skipping OpenShell PATH startup repair through symlink directory" + ); + Ok(false) + } + Ok(metadata) if metadata.is_dir() => Ok(true), + Ok(_) => { + tracing::debug!( + path = %path.display(), + "skipping OpenShell PATH startup repair through non-directory path" + ); + Ok(false) + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(error) => Err(error), + } +} + +fn write_startup_file(path: &Path, content: &str) -> std::io::Result<()> { + let mut file = no_follow_options() + .write(true) + .create(true) + .truncate(true) + .open(path)?; + ensure_opened_file_is_regular(&file, path)?; + file.write_all(content.as_bytes()) +} + +fn open_startup_file_for_read(path: &Path) -> std::io::Result { + let file = no_follow_options().read(true).open(path)?; + ensure_opened_file_is_regular(&file, path)?; + Ok(file) +} + +fn open_startup_file_for_append(path: &Path) -> std::io::Result { + let file = no_follow_options().append(true).open(path)?; + ensure_opened_file_is_regular(&file, path)?; + Ok(file) +} + +fn ensure_opened_file_is_regular(file: &std::fs::File, path: &Path) -> std::io::Result<()> { + let metadata = file.metadata()?; + if metadata.is_file() { + return Ok(()); + } + Err(std::io::Error::other(format!( + "'{}' is not a regular file", + path.display() + ))) +} + +fn no_follow_options() -> std::fs::OpenOptions { + let mut options = std::fs::OpenOptions::new(); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt as _; + options.custom_flags(libc::O_NOFOLLOW | libc::O_CLOEXEC); + } + options +} pub fn proxy_env_vars(proxy_url: &str) -> [(&'static str, String); 9] { [ @@ -45,6 +313,249 @@ mod tests { use std::process::Command; use std::process::Stdio; + #[test] + fn path_with_standard_sbin_paths_uses_default_for_empty_path() { + assert_eq!( + path_with_standard_sbin_paths(""), + "/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin" + ); + } + + #[test] + fn path_with_standard_sbin_paths_appends_missing_sbin_dirs() { + assert_eq!( + path_with_standard_sbin_paths("/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin"), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin" + ); + } + + #[test] + fn path_with_standard_sbin_paths_is_idempotent() { + let path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"; + assert_eq!(path_with_standard_sbin_paths(path), path); + } + + #[test] + fn maybe_path_with_standard_sbin_paths_respects_gate() { + let path = "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin"; + + assert_eq!(maybe_path_with_standard_sbin_paths(path, false), path); + assert_eq!( + maybe_path_with_standard_sbin_paths(path, true), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin" + ); + } + + #[test] + fn maybe_shell_command_with_standard_sbin_paths_respects_gate() { + assert_eq!( + maybe_shell_command_with_standard_sbin_paths("nvidia-smi -L", false), + "nvidia-smi -L" + ); + assert!( + maybe_shell_command_with_standard_sbin_paths("nvidia-smi -L", true) + .contains("/usr/sbin") + ); + } + + #[test] + fn standard_sbin_repair_requires_expected_cdi_context_and_policy_path() { + let policy = policy_with_read_only(["/usr/sbin/nvidia-smi"]); + + assert!(standard_sbin_path_repair_enabled_for_context( + &policy, + Some(openshell_core::cdi::CDI_CONTEXT_PATH) + )); + assert!(!standard_sbin_path_repair_enabled_for_context( + &policy, + Some("/tmp/cdi-context.json") + )); + assert!(!standard_sbin_path_repair_enabled_for_context( + &policy, None + )); + } + + #[test] + fn standard_sbin_repair_requires_standard_sbin_policy_path() { + let policy = policy_with_read_only(["/usr/local/bin/nvidia-smi"]); + + assert!(!standard_sbin_path_repair_enabled_for_context( + &policy, + Some(openshell_core::cdi::CDI_CONTEXT_PATH) + )); + } + + #[test] + fn standard_sbin_repair_accepts_read_write_standard_sbin_policy_path() { + let mut policy = policy_with_read_only(std::iter::empty::<&str>()); + policy + .filesystem + .read_write + .push("/sbin/vendor-tool".into()); + + assert!(standard_sbin_path_repair_enabled_for_context( + &policy, + Some(openshell_core::cdi::CDI_CONTEXT_PATH) + )); + } + + fn policy_with_read_only( + paths: impl IntoIterator>, + ) -> SandboxPolicy { + SandboxPolicy { + version: 1, + filesystem: openshell_core::policy::FilesystemPolicy { + read_only: paths.into_iter().map(Into::into).collect(), + read_write: Vec::new(), + include_workdir: false, + }, + network: openshell_core::policy::NetworkPolicy::default(), + landlock: openshell_core::policy::LandlockPolicy::default(), + process: openshell_core::policy::ProcessPolicy::default(), + } + } + + #[test] + fn shell_command_with_standard_sbin_paths_extends_runtime_path() { + let command = shell_command_with_standard_sbin_paths("printf '%s' \"$PATH\""); + let output = Command::new("/bin/sh") + .arg("-c") + .arg(format!( + "PATH=/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin\n{command}" + )) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .expect("spawn shell"); + + assert!( + output.status.success(), + "shell command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + String::from_utf8(output.stdout).expect("utf8"), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin" + ); + } + + #[test] + fn startup_snippets_are_idempotent() { + let dir = tempfile::tempdir().expect("tempdir"); + let profile_path = dir.path().join("etc/profile.d/openshell-path.sh"); + let home = dir.path().join("sandbox"); + std::fs::create_dir_all(&home).expect("home dir"); + let bashrc_path = home.join(".bashrc"); + std::fs::write( + &bashrc_path, + "export PATH=\"/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin\"\n", + ) + .expect("write bashrc"); + + write_profile_snippet(&profile_path).expect("write profile snippet"); + append_startup_snippet(&bashrc_path).expect("append startup snippet"); + write_profile_snippet(&profile_path).expect("rewrite profile snippet"); + append_startup_snippet(&bashrc_path).expect("append startup snippet again"); + + let profile = std::fs::read_to_string(&profile_path).expect("read profile"); + let bashrc = std::fs::read_to_string(&bashrc_path).expect("read bashrc"); + + assert_eq!(profile.matches(STARTUP_SNIPPET_MARKER).count(), 1); + assert_eq!(bashrc.matches(STARTUP_SNIPPET_MARKER).count(), 1); + assert!(bashrc.contains("export PATH=\"/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin\"")); + } + + #[cfg(unix)] + #[test] + fn profile_snippet_skips_symlink() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().expect("tempdir"); + let target = dir.path().join("target"); + std::fs::write(&target, "keep me").expect("write target"); + let profile_path = dir.path().join("openshell-path.sh"); + symlink(&target, &profile_path).expect("symlink profile"); + + write_profile_snippet(&profile_path).expect("skip symlink profile"); + + assert_eq!( + std::fs::read_to_string(&target).expect("read target"), + "keep me" + ); + assert!( + std::fs::symlink_metadata(&profile_path) + .expect("profile metadata") + .file_type() + .is_symlink() + ); + } + + #[cfg(unix)] + #[test] + fn profile_snippet_rejects_symlink_parent() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().expect("tempdir"); + let target_dir = dir.path().join("target-dir"); + std::fs::create_dir(&target_dir).expect("target dir"); + let parent = dir.path().join("profile.d"); + symlink(&target_dir, &parent).expect("symlink parent"); + let profile_path = parent.join("openshell-path.sh"); + + let error = write_profile_snippet(&profile_path).expect_err("reject symlink parent"); + + assert!(error.to_string().contains("symlink")); + assert!(!target_dir.join("openshell-path.sh").exists()); + } + + #[cfg(unix)] + #[test] + fn bashrc_snippet_skips_symlink() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().expect("tempdir"); + let target = dir.path().join("target"); + std::fs::write(&target, "keep me").expect("write target"); + let bashrc_path = dir.path().join(".bashrc"); + symlink(&target, &bashrc_path).expect("symlink bashrc"); + + append_startup_snippet(&bashrc_path).expect("skip symlink bashrc"); + + assert_eq!( + std::fs::read_to_string(&target).expect("read target"), + "keep me" + ); + assert!( + std::fs::symlink_metadata(&bashrc_path) + .expect("bashrc metadata") + .file_type() + .is_symlink() + ); + } + + #[cfg(unix)] + #[test] + fn bashrc_snippet_skips_symlink_parent() { + use std::os::unix::fs::symlink; + + let dir = tempfile::tempdir().expect("tempdir"); + let target_dir = dir.path().join("target-dir"); + std::fs::create_dir(&target_dir).expect("target dir"); + std::fs::write( + target_dir.join(".bashrc"), + "export PATH=\"/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin\"\n", + ) + .expect("target bashrc"); + let home = dir.path().join("home"); + symlink(&target_dir, &home).expect("symlink home"); + + append_startup_snippet(&home.join(".bashrc")).expect("skip symlink home"); + + let bashrc = std::fs::read_to_string(target_dir.join(".bashrc")).expect("read target"); + assert!(!bashrc.contains(STARTUP_SNIPPET_MARKER)); + } + #[test] fn apply_proxy_env_includes_node_proxy_opt_in_and_local_bypass() { let mut cmd = Command::new("/usr/bin/env"); diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 9270681f56..d30e53244e 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -13,7 +13,10 @@ use miette::{IntoDiagnostic, Result}; use nix::sys::signal::{self, Signal}; use nix::unistd::{Gid, Group, Pid, Uid, User}; use openshell_core::policy::{NetworkMode, SandboxPolicy}; +#[cfg(unix)] +use std::collections::BTreeSet; use std::collections::HashMap; +#[cfg(target_os = "linux")] use std::ffi::CString; #[cfg(target_os = "linux")] use std::os::fd::{AsRawFd, OwnedFd, RawFd}; @@ -163,12 +166,23 @@ fn strip_supervisor_only_env(cmd: &mut Command) { } } -fn inject_provider_env(cmd: &mut Command, provider_env: &HashMap) { +fn inject_provider_env( + cmd: &mut Command, + provider_env: &HashMap, + repair_standard_sbin: bool, +) { for (key, value) in provider_env { if is_supervisor_only_env_var(key) { continue; } - cmd.env(key, value); + if key == "PATH" { + cmd.env( + key, + child_env::maybe_path_with_standard_sbin_paths(value, repair_standard_sbin), + ); + } else { + cmd.env(key, value); + } } } @@ -547,6 +561,35 @@ pub struct ProcessHandle { pid: u32, } +fn shell_command_arg_index(args: &[String]) -> Option { + for (index, arg) in args.iter().enumerate() { + if arg == "-c" || (arg.starts_with('-') && !arg.starts_with("--") && arg.contains('c')) { + return (index + 1 < args.len()).then_some(index + 1); + } + } + None +} + +fn process_args_with_standard_sbin_paths( + program: &str, + args: &[String], + repair_standard_sbin: bool, +) -> Vec { + let mut args = args.to_vec(); + if !repair_standard_sbin { + return args; + } + + let basename = program.rsplit('/').next().unwrap_or(program); + if matches!(basename, "bash" | "sh") + && let Some(command_index) = shell_command_arg_index(&args) + { + args[command_index] = + child_env::shell_command_with_standard_sbin_paths(&args[command_index]); + } + args +} + impl ProcessHandle { /// Spawn a new process. /// @@ -626,13 +669,16 @@ impl ProcessHandle { ca_paths: Option<&(PathBuf, PathBuf)>, provider_env: &HashMap, ) -> Result { + let repair_standard_sbin = child_env::standard_sbin_path_repair_enabled(policy); + let args = process_args_with_standard_sbin_paths(program, args, repair_standard_sbin); let mut cmd = Command::new(program); - cmd.args(args) + cmd.args(&args) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) .kill_on_drop(true) - .env(openshell_core::sandbox_env::SANDBOX, "1"); + .env(openshell_core::sandbox_env::SANDBOX, "1") + .env("PATH", child_env::child_path_from_env(repair_standard_sbin)); // Strip supervisor-only identity material from the entrypoint's // inherited environment. The entrypoint drops to the sandbox user @@ -640,7 +686,7 @@ impl ProcessHandle { // supervisor credentials from its inherited environment. strip_supervisor_only_env(&mut cmd); - inject_provider_env(&mut cmd, provider_env); + inject_provider_env(&mut cmd, provider_env, repair_standard_sbin); if let Some(dir) = workspace.root() { cmd.current_dir(dir); @@ -783,19 +829,22 @@ impl ProcessHandle { ca_paths: Option<&(PathBuf, PathBuf)>, provider_env: &HashMap, ) -> Result { + let repair_standard_sbin = child_env::standard_sbin_path_repair_enabled(policy); + let args = process_args_with_standard_sbin_paths(program, args, repair_standard_sbin); let mut cmd = Command::new(program); - cmd.args(args) + cmd.args(&args) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) .kill_on_drop(true) - .env(openshell_core::sandbox_env::SANDBOX, "1"); + .env(openshell_core::sandbox_env::SANDBOX, "1") + .env("PATH", child_env::child_path_from_env(repair_standard_sbin)); // Strip supervisor-only identity material from the entrypoint's // inherited environment. strip_supervisor_only_env(&mut cmd); - inject_provider_env(&mut cmd, provider_env); + inject_provider_env(&mut cmd, provider_env, repair_standard_sbin); if let Some(dir) = workspace.root() { cmd.current_dir(dir); @@ -1943,7 +1992,11 @@ fn resolve_filesystem_identity( _ => Vec::new(), }; - Ok((uid, gid, supplementary_gids)) + Ok(( + uid, + gid, + merged_supplemental_gids(supplementary_gids, &policy.process.supplemental_groups)?, + )) } #[cfg(not(unix))] @@ -1953,6 +2006,24 @@ pub fn prepare_filesystem(_policy: &SandboxPolicy) -> Result<()> { // `effective_gid`/`effective_uid` are intentionally parallel names (same role // for different identifiers) and the noise from renaming would obscure intent. +#[cfg(unix)] +fn merged_supplemental_gids(base: Vec, extra: &[u32]) -> Result> { + let mut groups = base + .into_iter() + .map(Gid::as_raw) + .filter(|gid| *gid != 0) + .collect::>(); + + for raw_gid in extra { + if *raw_gid == 0 { + return Err(miette::miette!("Supplemental group GID 0 is not allowed")); + } + groups.insert(*raw_gid); + } + + Ok(groups.into_iter().map(Gid::from_raw).collect()) +} + #[cfg(unix)] #[allow(clippy::similar_names)] pub fn drop_privileges(policy: &SandboxPolicy) -> Result<()> { @@ -1985,6 +2056,11 @@ pub fn drop_privileges_with_identity( fallback.process.run_as_group = Some("sandbox".into()); return drop_privileges_with_identity(&fallback, resolved_identity); } + if !policy.process.supplemental_groups.is_empty() { + return Err(miette::miette!( + "Supplemental groups require a privileged supervisor process" + )); + } return Ok(()); } @@ -2035,61 +2111,44 @@ pub fn drop_privileges_with_identity( }, }; - // Resolve the name for initgroups only for the existing explicit-policy - // path. OCI-derived users carry a numeric UID from the bounded parser and - // must not be looked up again through NSS. - let user_name_is_numeric = user_name.is_some_and(|n| n.parse::().is_ok()); - let initgroups_name = - if user_name.is_some() && !user_name_is_numeric && resolved_identity.uid().is_none() { - Some( - User::from_uid(target_uid) - .into_diagnostic()? - .ok_or_else(|| { - miette::miette!("Failed to resolve user record for UID {target_uid}") - })? - .name, - ) - } else { - None - }; - + #[cfg(target_os = "linux")] if target_uid != nix::unistd::geteuid() { - if resolved_identity.uses_oci_user_fallback() { + // Resolve the name for initgroups only for the existing explicit-policy + // path. OCI-derived users carry a numeric UID from the bounded parser and + // must not be looked up again through NSS. + let user_name_is_numeric = user_name.is_some_and(|n| n.parse::().is_ok()); + let initgroups_name = + if user_name.is_some() && !user_name_is_numeric && resolved_identity.uid().is_none() { + Some( + User::from_uid(target_uid) + .into_diagnostic()? + .ok_or_else(|| { + miette::miette!("Failed to resolve user record for UID {target_uid}") + })? + .name, + ) + } else { + None + }; + let supplemental_groups = if resolved_identity.uses_oci_user_fallback() { // OCI named users use the bounded /etc/group parser shared with // workspace validation. Numeric OCI users resolve to an empty // list. Never retain the root supervisor's inherited groups. - #[cfg(not(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - )))] - { - let (_, _, supplementary_gids) = - resolve_filesystem_identity(policy, resolved_identity)?; - nix::unistd::setgroups(&supplementary_gids).into_diagnostic()?; - } + resolve_filesystem_identity(policy, resolved_identity)?.2 } else if let Some(ref user_name) = initgroups_name { let user_cstr = CString::new(user_name.as_str()) .map_err(|_| miette::miette!("Invalid user name"))?; - #[cfg(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - ))] - { - let _ = user_cstr; - } - #[cfg(not(any( - target_os = "macos", - target_os = "ios", - target_os = "haiku", - target_os = "redox" - )))] - { - nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; - } + nix::unistd::initgroups(user_cstr.as_c_str(), target_gid).into_diagnostic()?; + merged_supplemental_gids( + nix::unistd::getgroups().into_diagnostic()?, + &policy.process.supplemental_groups, + )? + } else { + merged_supplemental_gids(Vec::new(), &policy.process.supplemental_groups)? + }; + + if !supplemental_groups.is_empty() || resolved_identity.uses_oci_user_fallback() { + nix::unistd::setgroups(&supplemental_groups).into_diagnostic()?; } } @@ -2206,6 +2265,35 @@ mod tests { use std::mem::size_of; use std::process::Stdio as StdStdio; + #[test] + fn process_args_wrap_shell_c_command_with_standard_sbin_paths() { + let args = vec!["-lc".to_string(), "nvidia-smi -L".to_string()]; + + let wrapped = process_args_with_standard_sbin_paths("sh", &args, true); + + assert_eq!(wrapped[0], "-lc"); + assert!(wrapped[1].contains("/usr/sbin")); + assert!(wrapped[1].contains("nvidia-smi -L")); + } + + #[test] + fn process_args_leave_non_shell_commands_unchanged() { + let args = vec!["nvidia-smi -L".to_string()]; + + let wrapped = process_args_with_standard_sbin_paths("python", &args, true); + + assert_eq!(wrapped, args); + } + + #[test] + fn process_args_do_not_wrap_when_standard_sbin_repair_disabled() { + let args = vec!["-lc".to_string(), "nvidia-smi -L".to_string()]; + + let wrapped = process_args_with_standard_sbin_paths("sh", &args, false); + + assert_eq!(wrapped, args); + } + /// Helper to create a minimal `SandboxPolicy` with the given process policy. fn policy_with_process(process: ProcessPolicy) -> SandboxPolicy { SandboxPolicy { @@ -2234,6 +2322,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("101".into()), run_as_group: Some("102".into()), + ..Default::default() }); assert!(validate_sandbox_user(&policy).is_ok()); @@ -2246,6 +2335,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("app".into()), run_as_group: Some("staff".into()), + ..Default::default() }); let resolved = ResolvedProcessIdentity::new(Some(101), Some(102)); @@ -2259,10 +2349,12 @@ mod tests { let root_user = policy_with_process(ProcessPolicy { run_as_user: Some("0".into()), run_as_group: Some("102".into()), + ..Default::default() }); let root_group = policy_with_process(ProcessPolicy { run_as_user: Some("101".into()), run_as_group: Some("0".into()), + ..Default::default() }); assert!(validate_sandbox_user(&root_user).is_err()); @@ -2275,6 +2367,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("__oci_name_not_in_host_nss__".into()), run_as_group: Some("__oci_group_not_in_host_nss__".into()), + ..Default::default() }); let resolved = ResolvedProcessIdentity::new(Some(1234), Some(1235)); @@ -2288,6 +2381,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("__explicit_name_not_in_host_nss__".into()), run_as_group: Some("__oci_group_not_in_host_nss__".into()), + ..Default::default() }); let resolved = ResolvedProcessIdentity::new(None, Some(1235)); @@ -2407,6 +2501,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }); if nix::unistd::geteuid().is_root() { // As root, drop_privileges falls back to "sandbox:sandbox". @@ -2424,6 +2519,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(String::new()), run_as_group: Some(String::new()), + ..Default::default() }); if nix::unistd::geteuid().is_root() { let has_sandbox = User::from_name("sandbox").ok().flatten().is_some(); @@ -2433,6 +2529,21 @@ mod tests { } } + #[test] + fn merged_supplemental_gids_deduplicates_and_rejects_root() { + let merged = merged_supplemental_gids( + vec![Gid::from_raw(44), Gid::from_raw(44), Gid::from_raw(0)], + &[44, 107], + ) + .unwrap(); + + assert_eq!( + merged.iter().map(|gid| gid.as_raw()).collect::>(), + vec![44, 107] + ); + assert!(merged_supplemental_gids(Vec::new(), &[0]).is_err()); + } + #[test] fn drop_privileges_succeeds_for_current_group() { // Set only run_as_group (no run_as_user) so that initgroups() is not @@ -2447,6 +2558,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: Some(current_group.name), + ..Default::default() }); let result = drop_privileges(&policy); @@ -2484,6 +2596,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: Some(current_group.name), + ..Default::default() }); let mut cmd = std::process::Command::new(std::env::current_exe().expect("current exe")); @@ -2525,6 +2638,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(current_user.name), run_as_group: Some(current_group.name), + ..Default::default() }); assert!(drop_privileges(&policy).is_ok()); @@ -2535,6 +2649,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("__nonexistent_test_user_42__".to_string()), run_as_group: None, + ..Default::default() }); let result = drop_privileges(&policy); @@ -2548,6 +2663,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: None, run_as_group: Some("__nonexistent_test_group_42__".to_string()), + ..Default::default() }); let result = drop_privileges(&policy); @@ -2696,13 +2812,61 @@ mod tests { )) .collect(); - inject_provider_env(&mut cmd, &provider_env); + inject_provider_env(&mut cmd, &provider_env, false); let output = cmd.output().await.expect("spawn env"); let stdout = String::from_utf8(output.stdout).expect("utf8"); assert!(stdout.contains("ANTHROPIC_API_KEY=openshell:resolve:env:ANTHROPIC_API_KEY")); } + #[tokio::test] + async fn inject_provider_env_appends_standard_sbin_to_path_when_enabled() { + let mut cmd = Command::new("/usr/bin/env"); + cmd.env_clear() + .stdin(StdStdio::null()) + .stdout(StdStdio::piped()) + .stderr(StdStdio::null()); + + let provider_env = HashMap::from([( + "PATH".to_string(), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin".to_string(), + )]); + + inject_provider_env(&mut cmd, &provider_env, true); + + let output = cmd.output().await.expect("spawn env"); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).expect("utf8"); + assert!(stdout.lines().any(|line| { + line == "PATH=/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin" + })); + } + + #[tokio::test] + async fn inject_provider_env_leaves_path_unchanged_when_standard_sbin_repair_disabled() { + let mut cmd = Command::new("/usr/bin/env"); + cmd.env_clear() + .stdin(StdStdio::null()) + .stdout(StdStdio::piped()) + .stderr(StdStdio::null()); + + let provider_env = HashMap::from([( + "PATH".to_string(), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin".to_string(), + )]); + + inject_provider_env(&mut cmd, &provider_env, false); + + let output = cmd.output().await.expect("spawn env"); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).expect("utf8"); + assert!( + stdout + .lines() + .any(|line| { line == "PATH=/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin" }) + ); + } + #[cfg(unix)] fn sandbox_policy_with_read_write( path: PathBuf, @@ -2721,6 +2885,7 @@ mod tests { process: ProcessPolicy { run_as_user, run_as_group, + ..Default::default() }, } } @@ -3579,7 +3744,7 @@ mod tests { ), ]); - inject_provider_env(&mut cmd, &provider_env); + inject_provider_env(&mut cmd, &provider_env, false); let output = cmd.output().await.expect("spawn env"); assert!(output.status.success()); @@ -3683,6 +3848,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(uid_raw.to_string()), run_as_group: Some(gid_raw.to_string()), + ..Default::default() }); assert!( @@ -3709,6 +3875,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some(current_uid.to_string()), // numeric UID, no passwd entry needed run_as_group: Some(current_group.name), // name-based group + ..Default::default() }); assert!( @@ -3725,6 +3892,7 @@ mod tests { let policy = policy_with_process(ProcessPolicy { run_as_user: Some("999999".into()), run_as_group: Some("999999".into()), + ..Default::default() }); match drop_privileges(&policy) { Ok(()) => {} diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index 91e56b7ec8..de89e8c351 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -104,6 +104,10 @@ pub async fn run_process( )?; } + if crate::child_env::standard_sbin_path_repair_enabled(policy) { + crate::child_env::install_standard_sbin_path_startup_files(workspace.home()); + } + // Eagerly fetch initial settings and install the agent skill if the // proposals flag is on at startup, rather than waiting for the policy // poll loop's first tick. In offline/file-mode there is no gateway, so diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 07302da953..84bf56148d 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -743,6 +743,19 @@ fn session_user_and_home(policy: &SandboxPolicy, workdir_home: Option<&str>) -> (user, home) } +fn bash_login_command(command: String, repair_standard_sbin: bool) -> Command { + let mut cmd = Command::new("/bin/bash"); + // Login profiles can rewrite PATH after the supervisor-provided child + // environment is applied. Re-append standard sbin directories so + // CDI-injected tools such as /usr/sbin/nvidia-smi remain discoverable. + cmd.arg("-lc") + .arg(child_env::maybe_shell_command_with_standard_sbin_paths( + &command, + repair_standard_sbin, + )); + cmd +} + #[allow(clippy::too_many_arguments)] fn apply_child_env( cmd: &mut Command, @@ -753,8 +766,9 @@ fn apply_child_env( ca_file_paths: Option<&(PathBuf, PathBuf)>, provider_env: &HashMap, user_environment: &HashMap, + repair_standard_sbin: bool, ) { - let path = std::env::var("PATH").unwrap_or_else(|_| "/usr/local/bin:/usr/bin:/bin".into()); + let path = child_env::child_path_from_env(repair_standard_sbin); cmd.env_clear() .env(openshell_core::sandbox_env::SANDBOX, "1") @@ -766,7 +780,14 @@ fn apply_child_env( for (key, value) in user_environment { if !key.starts_with("OPENSHELL_") { - cmd.env(key, value); + if key == "PATH" { + cmd.env( + key, + child_env::maybe_path_with_standard_sbin_paths(value, repair_standard_sbin), + ); + } else { + cmd.env(key, value); + } } } @@ -786,7 +807,14 @@ fn apply_child_env( if is_supervisor_only_env_var(key) { continue; } - cmd.env(key, value); + if key == "PATH" { + cmd.env( + key, + child_env::maybe_path_with_standard_sbin_paths(value, repair_standard_sbin), + ); + } else { + cmd.env(key, value); + } } } @@ -823,17 +851,14 @@ fn spawn_pty_shell( let mut reader = master.try_clone()?; let mut writer = master.try_clone()?; + let repair_standard_sbin = child_env::standard_sbin_path_repair_enabled(policy); let mut cmd = command.map_or_else( || { let mut c = Command::new("/bin/bash"); c.arg("-i"); c }, - |command| { - let mut c = Command::new("/bin/bash"); - c.arg("-lc").arg(command); - c - }, + |command| bash_login_command(command, repair_standard_sbin), ); let term = if pty.term.is_empty() { @@ -854,6 +879,7 @@ fn spawn_pty_shell( ca_file_paths.as_deref(), provider_env, user_environment, + repair_standard_sbin, ); cmd.stdin(stdin).stdout(stdout).stderr(stderr); @@ -978,6 +1004,7 @@ fn spawn_pipe_exec( resolved_identity: ResolvedProcessIdentity, enforcement_mode: ProcessEnforcementMode, ) -> anyhow::Result>> { + let repair_standard_sbin = child_env::standard_sbin_path_repair_enabled(policy); let mut cmd = command.map_or_else( || { // No command — read from stdin. Do *not* pass `-i`; interactive @@ -989,12 +1016,10 @@ fn spawn_pipe_exec( Command::new("/bin/bash") }, |command| { - let mut c = Command::new("/bin/bash"); // Use login shell (-l) so that .profile/.bashrc are sourced and // tool-specific env vars (VIRTUAL_ENV, UV_PYTHON_INSTALL_DIR, etc.) // are available without hardcoding them here. - c.arg("-lc").arg(command); - c + bash_login_command(command, repair_standard_sbin) }, ); @@ -1008,6 +1033,7 @@ fn spawn_pipe_exec( ca_file_paths.as_deref(), provider_env, user_environment, + repair_standard_sbin, ); cmd.stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -1499,6 +1525,70 @@ mod tests { assert_eq!(output.stdout, b"hello"); } + #[test] + fn apply_child_env_appends_standard_sbin_to_user_path_when_enabled() { + let mut cmd = Command::new("/usr/bin/env"); + cmd.stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + let user_environment = HashMap::from([( + "PATH".to_string(), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin".to_string(), + )]); + apply_child_env( + &mut cmd, + "/sandbox", + "sandbox", + "dumb", + None, + None, + &HashMap::new(), + &user_environment, + true, + ); + + let output = cmd.output().expect("spawn env"); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).expect("utf8"); + assert!(stdout.lines().any(|line| { + line == "PATH=/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin" + })); + } + + #[test] + fn apply_child_env_leaves_user_path_unchanged_when_standard_sbin_repair_disabled() { + let mut cmd = Command::new("/usr/bin/env"); + cmd.stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + + let user_environment = HashMap::from([( + "PATH".to_string(), + "/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin".to_string(), + )]); + apply_child_env( + &mut cmd, + "/sandbox", + "sandbox", + "dumb", + None, + None, + &HashMap::new(), + &user_environment, + false, + ); + + let output = cmd.output().expect("spawn env"); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).expect("utf8"); + assert!( + stdout + .lines() + .any(|line| line == "PATH=/sandbox/.venv/bin:/usr/local/bin:/usr/bin:/bin") + ); + } + /// Verify that the stdin writer delivers all buffered data before exiting /// when the sender is dropped. This ensures channel_eof doesn't cause /// data loss — only signals "no more data after this". @@ -1725,6 +1815,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("1000".into()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy, None); @@ -1746,6 +1837,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("1234".into()), run_as_group: Some("1235".into()), + ..Default::default() }, }; @@ -1767,6 +1859,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("sandbox".into()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy, None); @@ -1788,6 +1881,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some(String::new()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy, None); @@ -1808,6 +1902,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy, None); @@ -1828,6 +1923,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("1000660000".into()), run_as_group: None, + ..Default::default() }, }; let (user, home) = session_user_and_home(&policy, None); @@ -1857,6 +1953,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, }; @@ -1888,6 +1985,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, }, None, @@ -1930,6 +2028,7 @@ mod tests { process: ProcessPolicy { run_as_user: Some("__oci_user_not_in_host_nss__".into()), run_as_group: Some("__oci_group_not_in_host_nss__".into()), + ..Default::default() }, }; let resolved = ResolvedProcessIdentity::new( @@ -2002,6 +2101,7 @@ mod tests { process: ProcessPolicy { run_as_user: None, run_as_group: None, + ..Default::default() }, } } diff --git a/deny.toml b/deny.toml index c4dab12334..19053fa796 100644 --- a/deny.toml +++ b/deny.toml @@ -67,4 +67,4 @@ external-default-features = "allow" unknown-registry = "deny" unknown-git = "deny" allow-registry = ["https://github.com/rust-lang/crates.io-index"] -allow-git = [] +allow-git = ["https://github.com/cncf-tags/container-device-interface-rs"] diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 735d99a69a..c5e971b79c 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -155,6 +155,17 @@ idempotent start request. Explicitly stopped sandboxes remain stopped. For GPU-backed Docker sandboxes, configure Docker CDI before starting the gateway so OpenShell can detect the daemon capability. +For Docker GPU/CDI sandboxes, OpenShell uses Docker's selected CDI device IDs +and daemon-reported CDI spec directories to build a supervisor-only CDI +context. The driver mounts the spec directories read-only into the sandbox +container. Before creation, it writes a gateway-owned `cdi-context.json` and +bind-mounts it read-only into the supervisor. If context or token creation +fails, the driver removes the created state files; if container creation or +start fails, it also removes the container and state files. The supervisor +resolves the context inside the sandbox and derives the inner filesystem and +supplemental group requirements from CDI specs. Non-GPU Docker sandboxes do not +receive the CDI context, spec mounts, or CDI-derived policy changes. + ### Docker Driver Config Mounts Docker driver config accepts user-supplied `volume` and `tmpfs` mounts. It also diff --git a/docs/sandboxes/policies.mdx b/docs/sandboxes/policies.mdx index 83150c39d1..56d0232498 100644 --- a/docs/sandboxes/policies.mdx +++ b/docs/sandboxes/policies.mdx @@ -102,7 +102,23 @@ See [Supervisor Middleware](/extensibility/supervisor-middleware) for registrati When a sandbox runs in proxy mode (the default), OpenShell automatically adds baseline filesystem paths required for the sandbox child process to function: `/usr`, `/lib`, `/etc`, and `/var/log` (read-only), plus `/tmp` (read-write). When `filesystem.include_workdir` is `true`, OpenShell also adds the resolved working directory as read-write. Paths like `/app` are included in the baseline set but are only added if they exist in the container image. -For GPU sandboxes, OpenShell also adds existing GPU device nodes as read-write paths. CUDA workloads require write access to procfs for thread metadata, so GPU baseline enrichment moves `/proc` from read-only to read-write when GPU devices are present. +For GPU sandboxes without a CDI context, OpenShell also adds existing GPU +device nodes as read-write paths. CUDA workloads require write access to procfs +for thread metadata, so GPU baseline enrichment moves `/proc` from read-only to +read-write when GPU devices or CDI specs are present. + +Docker GPU/CDI sandboxes receive CDI-derived filesystem requirements instead +of the hard-coded GPU device and library baseline. The supervisor resolves the +selected CDI device specs inside the sandbox and adds CDI device nodes as +read-write paths and CDI mount destinations as read-only paths. CDI +`additionalGids` are applied as supplemental groups before agent processes drop +privileges. + +Writable CDI mount destinations are fail-closed. OpenShell accepts a writable +CDI mount only when it targets a single file and that exact path is already +listed in `filesystem_policy.read_write`. Writable CDI directory mounts and +CDI paths such as `/`, `/dev`, `/proc`, `/sys`, `/run`, or `/usr` are rejected +during sandbox startup. This filtering prevents a missing baseline path from degrading Landlock enforcement. Without it, a single missing path could cause the entire Landlock ruleset to fail, leaving the sandbox with no filesystem restrictions at all.