diff --git a/crates/openshell-core/src/gpu.rs b/crates/openshell-core/src/gpu.rs index f5ff67cd35..09b8337f05 100644 --- a/crates/openshell-core/src/gpu.rs +++ b/crates/openshell-core/src/gpu.rs @@ -302,6 +302,62 @@ fn cdi_nvidia_gpu_suffix(id: &str) -> Option<&str> { id.strip_prefix(CDI_NVIDIA_GPU_PREFIX) } +// Local equivalent of the upstream CDI parser until its public API is released: +// https://github.com/cncf-tags/container-device-interface-rs/pull/177 +fn is_qualified_name(device: &str) -> bool { + let Some((kind, name)) = device.split_once('=') else { + return false; + }; + let Some((vendor, class)) = kind.split_once('/') else { + return false; + }; + let valid_component = |value: &str, device_name: bool| { + let bytes = value.as_bytes(); + bytes.first().is_some_and(|byte| { + if device_name { + byte.is_ascii_alphanumeric() + } else { + byte.is_ascii_alphabetic() + } + }) && bytes.last().is_some_and(u8::is_ascii_alphanumeric) + && bytes.iter().all(|byte| { + byte.is_ascii_alphanumeric() + || matches!(byte, b'_' | b'-' | b'.') + || (device_name && *byte == b':') + }) + }; + valid_component(vendor, false) && valid_component(class, false) && valid_component(name, true) +} + +/// Validate vendor-agnostic CDI qualified names without resolving or authorizing devices. +/// +/// # Errors +/// Returns an error for host paths or malformed `/=` selectors. +pub fn validate_cdi_device_names(devices: &[String], field: &str) -> Result<(), String> { + if devices.iter().any(|device| !is_qualified_name(device)) { + return Err(format!( + "{field} must contain CDI qualified names (/=)" + )); + } + Ok(()) +} + +/// Validate explicit CDI selectors and their GPU request requirements. +/// +/// Docker and Podman use this helper; drivers with other identifier formats use +/// [`validate_specific_gpu_device_request`] directly. +/// +/// # Errors +/// Returns an error for malformed CDI names or invalid GPU request requirements. +pub fn validate_cdi_gpu_device_request( + gpu: Option<&DriverGpuResourceRequirements>, + devices: &[String], + field: &str, +) -> Result<(), String> { + validate_cdi_device_names(devices, field)?; + validate_specific_gpu_device_request(gpu, devices, field) +} + /// Validate a compute-driver GPU request against driver-owned specific devices. /// /// Drivers call this when a sandbox request combines portable GPU requirements @@ -347,6 +403,64 @@ pub fn validate_specific_gpu_device_request( mod tests { use super::*; + #[test] + fn cdi_device_names_accept_vendor_agnostic_selectors() { + for device in [ + "nvidia.com/gpu=0", + "nvidia.com/gpu=all", + "nvidia.com/gpu=GPU-5b2d", + "nvidia.com/gpu=MIG-5b2d", + "nvidia.com/gpu=0:1", + "example.com/accelerator=card_0.1", + "intel.com/gpu=0", + "amd.com/gpu=0", + "v/c=0", + ] { + let devices = vec![device.to_string()]; + validate_cdi_device_names(&devices, "driver_config.cdi_devices") + .unwrap_or_else(|error| panic!("{device:?}: {error}")); + } + } + + #[test] + fn cdi_device_names_reject_host_paths_and_malformed_selectors() { + for device in [ + "", + "/dev/sda", + "/dev", + "/dev/sda:/dev/sda:rwm", + "dev/sda", + "../dev/sda", + "vendor/gpu", + "/gpu=0", + "vendor/=0", + "vendor/gpu=", + "vendor/gpu=/dev/sda", + "vendor/gpu=0/1", + "vendor/gpu=0=1", + "vendor/gpu=0,1", + "vendor/gpu=-0", + "vendor/gpu=0:", + "vendor/gpu=0 ", + " vendor/gpu=0", + "vendor/gpu=0\n", + "vendor/gpu=0\0", + "vendor/gpu=é", + "véndor/gpu=0", + "vendor/gpü=0", + "0vendor/gpu=0", + "vendor/0gpu=0", + "vendor./gpu=0", + "vendor/gpu-=0", + "vendor/gpu/other=0", + ] { + let error = + validate_cdi_device_names(&[device.to_string()], "driver_config.cdi_devices") + .unwrap_err(); + assert!(error.contains("CDI qualified names"), "{device:?}: {error}"); + } + } + #[test] fn effective_driver_gpu_count_normalizes_missing_count() { let gpu = DriverGpuResourceRequirements { count: None }; @@ -547,6 +661,18 @@ mod tests { ); } + #[test] + fn specific_gpu_device_request_preserves_non_cdi_identifiers() { + let gpu = DriverGpuResourceRequirements { count: Some(1) }; + let devices = vec!["0000:2d:00.0".to_string()]; + validate_specific_gpu_device_request(Some(&gpu), &devices, "driver_config.gpu_device_ids") + .expect("generic GPU request validation must accept PCI device identifiers"); + let error = + validate_cdi_gpu_device_request(Some(&gpu), &devices, "driver_config.cdi_devices") + .expect_err("CDI drivers must reject non-CDI identifiers"); + assert!(error.contains("CDI qualified names")); + } + #[test] fn validate_specific_gpu_device_request_ignores_empty_devices() { validate_specific_gpu_device_request(None, &[], "driver_config.cdi_devices") diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 70c44bcdea..7dc656e9da 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -33,7 +33,7 @@ use openshell_core::driver_utils::{ }; use openshell_core::gpu::{ CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements, - effective_driver_gpu_count, validate_specific_gpu_device_request, + effective_driver_gpu_count, validate_cdi_device_names, validate_cdi_gpu_device_request, }; use openshell_core::progress::{ PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX, @@ -1126,7 +1126,7 @@ impl DockerComputeDriver { } if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() { - validate_specific_gpu_device_request( + validate_cdi_gpu_device_request( gpu_requirements, cdi_devices, "driver_config.cdi_devices", @@ -1309,7 +1309,7 @@ impl DockerComputeDriver { ) -> Result, CdiGpuSelectionError>, ) -> Result>, Status> { if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() { - validate_specific_gpu_device_request( + validate_cdi_gpu_device_request( gpu_requirements, cdi_devices, "driver_config.cdi_devices", @@ -5618,12 +5618,8 @@ fn build_container_create_body( .as_ref() .and_then(|spec| driver_gpu_requirements(spec.resource_requirements.as_ref())); let cdi_devices = if let Some(cdi_devices) = driver_config.cdi_devices.as_ref() { - validate_specific_gpu_device_request( - gpu_requirements, - cdi_devices, - "driver_config.cdi_devices", - ) - .map_err(Status::invalid_argument)?; + validate_cdi_gpu_device_request(gpu_requirements, cdi_devices, "driver_config.cdi_devices") + .map_err(Status::invalid_argument)?; Some(cdi_devices.as_slice()) } else { None @@ -5674,6 +5670,10 @@ fn build_container_create_body_for_image( image: &DockerImageMetadata, workload_identity: &ResolvedWorkloadIdentity, ) -> Result { + if let Some(device_ids) = gpu_device_ids { + validate_cdi_device_names(device_ids, "driver_config.cdi_devices") + .map_err(Status::invalid_argument)?; + } let spec = sandbox .spec .as_ref() diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 075300ec26..d8d835281b 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -2719,6 +2719,91 @@ fn build_container_create_body_omits_devices_without_resolved_default_cdi_device ); } +#[test] +fn container_spec_accepts_non_nvidia_explicit_and_resolved_cdi_devices() { + let mut config = runtime_config(); + config.gpu.cdi_supported = true; + let mut sandbox = test_sandbox(); + sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); + for device in ["example.com/gpu=0", "intel.com/gpu=0", "amd.com/gpu=0"] { + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(cdi_devices_config(&[device])); + let explicit = build_container_create_body(&sandbox, &config).unwrap(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = None; + let resolved = build_container_create_body_with_gpu_devices( + &sandbox, + &config, + &DockerSandboxDriverConfig::default(), + Some(&[device.to_string()]), + ) + .unwrap(); + for body in [explicit, resolved] { + let requests = body.host_config.unwrap().device_requests.unwrap(); + assert_eq!(requests[0].driver.as_deref(), Some("cdi")); + assert_eq!( + requests[0].device_ids.as_ref().unwrap(), + &[device.to_string()] + ); + } + } +} + +#[test] +fn container_spec_rejects_host_paths_in_explicit_and_resolved_cdi_devices() { + let mut config = runtime_config(); + config.gpu.cdi_supported = true; + let mut sandbox = test_sandbox(); + sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); + for device in [ + "/dev/sda", + "/dev", + "/dev/sda:/dev/sda:rwm", + "vendor/gpu=0/1", + ] { + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(cdi_devices_config(&[device])); + let explicit_error = build_container_create_body(&sandbox, &config).unwrap_err(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = None; + let resolved_error = build_container_create_body_with_gpu_devices( + &sandbox, + &config, + &DockerSandboxDriverConfig::default(), + Some(&[device.to_string()]), + ) + .unwrap_err(); + for error in [explicit_error, resolved_error] { + assert_eq!(error.code(), tonic::Code::InvalidArgument); + assert!(error.message().contains("CDI qualified names")); + } + } +} + #[test] fn build_container_create_body_passes_explicit_cdi_device_id_through() { let mut config = runtime_config(); diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index f6b1c71b59..4666d4f81d 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -6,8 +6,9 @@ use crate::config::PodmanComputeConfig; use openshell_core::ComputeDriverError; use openshell_core::driver_mounts::SelinuxLabel; +use openshell_core::gpu::validate_cdi_device_names; #[cfg(test)] -use openshell_core::gpu::{driver_gpu_requirements, validate_specific_gpu_device_request}; +use openshell_core::gpu::{driver_gpu_requirements, validate_cdi_gpu_device_request}; use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate}; use openshell_core::proto_struct::deserialize_optional_non_empty_string_list; use openshell_core::{driver_mounts, proto_struct}; @@ -1039,12 +1040,8 @@ pub fn try_build_container_spec_with_token( .as_ref() .and_then(|spec| driver_gpu_requirements(spec.resource_requirements.as_ref())); let cdi_devices = if let Some(cdi_devices) = driver_config.cdi_devices.as_ref() { - validate_specific_gpu_device_request( - gpu_requirements, - cdi_devices, - "driver_config.cdi_devices", - ) - .map_err(ComputeDriverError::InvalidArgument)?; + validate_cdi_gpu_device_request(gpu_requirements, cdi_devices, "driver_config.cdi_devices") + .map_err(ComputeDriverError::InvalidArgument)?; Some(cdi_devices.as_slice()) } else { None @@ -1112,6 +1109,10 @@ fn build_base_spec( supervisor_bin_path: Option<&Path>, tls_secret_names: Option<&[String; 1]>, ) -> Result { + if let Some(device_ids) = gpu_device_ids { + validate_cdi_device_names(device_ids, "driver_config.cdi_devices") + .map_err(ComputeDriverError::InvalidArgument)?; + } let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); @@ -2149,6 +2150,83 @@ mod tests { assert!(spec.get("devices").is_none()); } + #[test] + fn container_spec_accepts_non_nvidia_explicit_and_resolved_cdi_devices() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + resource_requirements: Some(gpu_resources(None)), + template: Some(DriverSandboxTemplate::default()), + ..Default::default() + }); + let config = test_config(); + for device in ["example.com/gpu=0", "intel.com/gpu=0", "amd.com/gpu=0"] { + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(cdi_devices_config(&[device])); + let explicit = try_build_container_spec_with_token(&sandbox, &config, None).unwrap(); + sandbox.spec.as_mut().unwrap().template = None; + let resolved = build_container_spec_with_token_and_gpu_devices( + &sandbox, + &config, + None, + Some(&[device.to_string()]), + ) + .unwrap(); + for spec in [explicit, resolved] { + assert_eq!(spec["devices"][0]["path"], device); + } + sandbox.spec.as_mut().unwrap().template = Some(DriverSandboxTemplate::default()); + } + } + + #[test] + fn container_spec_rejects_host_paths_in_explicit_and_resolved_cdi_devices() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + resource_requirements: Some(gpu_resources(None)), + template: Some(DriverSandboxTemplate::default()), + ..Default::default() + }); + let config = test_config(); + for device in [ + "/dev/sda", + "/dev", + "/dev/sda:/dev/sda:rwm", + "vendor/gpu=0/1", + ] { + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(cdi_devices_config(&[device])); + let explicit_error = + try_build_container_spec_with_token(&sandbox, &config, None).unwrap_err(); + sandbox.spec.as_mut().unwrap().template = None; + let resolved_error = build_container_spec_with_token_and_gpu_devices( + &sandbox, + &config, + None, + Some(&[device.to_string()]), + ) + .unwrap_err(); + for error in [explicit_error, resolved_error] { + assert!(matches!(error, ComputeDriverError::InvalidArgument(_))); + assert!(error.to_string().contains("CDI qualified names")); + } + sandbox.spec.as_mut().unwrap().template = Some(DriverSandboxTemplate::default()); + } + } + #[test] fn container_spec_passes_explicit_cdi_device_id_through() { use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 0b5d0ddcd4..e1f222539a 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -18,7 +18,7 @@ use openshell_core::driver_utils::{ }; use openshell_core::gpu::{ CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements, - effective_driver_gpu_count, validate_specific_gpu_device_request, + effective_driver_gpu_count, validate_cdi_gpu_device_request, }; use openshell_core::proto::compute::v1::{ CpuResourceCapabilities, DriverSandbox, GetCapabilitiesResponse, GpuResourceCapabilities, @@ -667,7 +667,7 @@ impl PodmanComputeDriver { let _ = effective_driver_gpu_count(gpu_requirements) .map_err(ComputeDriverError::InvalidArgument)?; if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() { - validate_specific_gpu_device_request( + validate_cdi_gpu_device_request( gpu_requirements, cdi_devices, "driver_config.cdi_devices", @@ -693,7 +693,7 @@ impl PodmanComputeDriver { ) -> Result, CdiGpuSelectionError>, ) -> Result>, ComputeDriverError> { if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() { - validate_specific_gpu_device_request( + validate_cdi_gpu_device_request( gpu_requirements, cdi_devices, "driver_config.cdi_devices", @@ -2948,6 +2948,32 @@ mod tests { assert!(err.to_string().contains("nvidia.com/gpu=all")); } + #[tokio::test] + async fn validate_sandbox_create_rejects_host_device_paths() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + + let driver = PodmanComputeDriver::for_tests(PodmanComputeConfig { + allow_driver_config: true, + ..Default::default() + }); + for device in ["/dev/sda", "/dev", "/dev/sda:/dev/sda:rwm"] { + let sandbox = DriverSandbox { + spec: Some(DriverSandboxSpec { + resource_requirements: Some(gpu_resources(None)), + template: Some(DriverSandboxTemplate { + driver_config: Some(cdi_devices_config(&[device])), + ..Default::default() + }), + ..Default::default() + }), + ..Default::default() + }; + let err = driver.validate_sandbox_create(&sandbox).await.unwrap_err(); + assert!(matches!(err, ComputeDriverError::InvalidArgument(_))); + assert!(err.to_string().contains("CDI qualified names")); + } + } + #[tokio::test] async fn validate_sandbox_create_passes_explicit_cdi_device_id_without_inventory() { use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; diff --git a/docs/how-it-works/gateways/configuration.mdx b/docs/how-it-works/gateways/configuration.mdx index a2b72381a1..46ef9c6e7b 100644 --- a/docs/how-it-works/gateways/configuration.mdx +++ b/docs/how-it-works/gateways/configuration.mdx @@ -1017,6 +1017,10 @@ path is never mounted into or exposed to the workload container. Each Podman sandbox uses two containers. The workload container runs `openshell-sandbox` with `network=none`; the supervisor container runs on the host network and initiates policy-approved upstream connections. A private volume carries their authenticated Unix-domain socket. Configure the supervisor gateway CA once under `[openshell.gateway]`; the gateway validates and injects it into the selected local driver. The supervisor authenticates RPCs with a sandbox bearer token. +Caller `cdi_devices` entries must be valid CDI qualified names +(`/=`). Host-device paths are rejected. See +[GPU selection](/how-it-works/sandboxes/overview#gpu-resources). + ```toml [openshell] version = 2 diff --git a/docs/how-it-works/sandboxes/overview.mdx b/docs/how-it-works/sandboxes/overview.mdx index 5be0d554ba..c300bc319c 100644 --- a/docs/how-it-works/sandboxes/overview.mdx +++ b/docs/how-it-works/sandboxes/overview.mdx @@ -157,7 +157,9 @@ device. Exact GPU device selection is driver-specific and still requires `--gpu`. For Docker or Podman, pass CDI IDs through `cdi_devices`. The top-level key must match the active driver; replace `docker` with `podman` when using Podman. CDI -IDs are treated as opaque strings. The list must not contain duplicate IDs, and +IDs must be valid CDI qualified names (`/=`); host-device +paths are rejected. Explicit selectors are not restricted to NVIDIA names. +The list must not contain duplicate IDs, and its length must match the effective GPU count: ```shell diff --git a/skills/debug-openshell-cluster/SKILL.md b/skills/debug-openshell-cluster/SKILL.md index 56c98e1074..4f45b7de77 100644 --- a/skills/debug-openshell-cluster/SKILL.md +++ b/skills/debug-openshell-cluster/SKILL.md @@ -308,6 +308,7 @@ Common findings: - Supervisor runtime validation fails: verify `supervisor_image` contains an `/openshell-supervisor` executable from the same release as the sandbox runtime, and that the dynamic loader and shared libraries it links against are available inside that image. `docker run --rm --network none --entrypoint /openshell-supervisor --version` should print that release; a `no such file or directory` error for a binary that exists means the loader or a library is missing. The supervisor runs from its own image and does not need to be static; only `/openshell-sandbox` must be. - The sandbox fails its enforcement probe: inspect the sandbox log for the exact nested seccomp user-notification, task-memory, Landlock, loopback DNS, or socket-injection check that failed. A runtime may return `ENOSYS` for `process_vm_readv` and `process_vm_writev` while satisfying the production parent-to-workload-child task-memory probe through `/proc//mem`; only failure of both backends is fatal. Do not add capabilities or switch to an unconfined seccomp profile; use a runtime whose default profile permits the unprivileged probe. - A GPU sandbox fails because Docker reports no discovered NVIDIA CDI devices: verify `.DiscoveredDevices` contains entries such as `nvidia.com/gpu=all`, verify `/etc/cdi` or `/var/run/cdi` contains a generated NVIDIA spec, and check that `nvidia-cdi-refresh.service` and `nvidia-cdi-refresh.path` from NVIDIA Container Toolkit are enabled and healthy. The service is a one-shot unit, so `inactive (dead)` can be normal after a successful run; use `systemctl status` and `journalctl` to distinguish success from a skipped or failed refresh. Restart `nvidia-cdi-refresh.service` to regenerate missing or stale CDI specs, then restart or reload Docker and re-check `docker info`. +- Docker or Podman rejects an explicit `cdi_devices` selector: use a CDI qualified name (`/=`), not a host-device path. Syntax validation accepts other vendors; it does not establish that the runtime has the device or that its GPU workload is supported. Consult the published GPU selection documentation for request requirements. #### Corporate upstream proxy