From c9095c4943fdc9837a3f301babcfa1536c62a0b2 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 10 Jun 2026 23:25:09 +0200 Subject: [PATCH 1/2] fix(podman): filter bind-backed named volumes Signed-off-by: Evan Lezar --- architecture/compute-runtimes.md | 2 +- crates/openshell-driver-podman/README.md | 4 +- crates/openshell-driver-podman/src/client.rs | 63 ++++-- crates/openshell-driver-podman/src/driver.rs | 203 ++++++++++++++++++- docs/reference/gateway-config.mdx | 3 + docs/reference/sandbox-compute-drivers.mdx | 9 +- 6 files changed, 255 insertions(+), 29 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 8b66efac6..90c294564 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -43,7 +43,7 @@ but currently ignores them. Docker and Podman also accept per-sandbox driver-config mounts for existing runtime-managed named volumes and tmpfs mounts. Podman additionally accepts image mounts through its image-volume API. User-supplied bind and volume mounts -default to read-only. Direct host bind mounts, and Docker local-driver +default to read-only. Direct host bind mounts, and Docker or Podman local-driver bind-backed named volumes, are available only when explicitly enabled in the active local driver table of `gateway.toml`. Host bind mounts are an unsafe operator override because they place gateway-host filesystem state inside the diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index ec5e5d12d..b35b54513 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -59,7 +59,9 @@ mount types: - `bind`: mounts an absolute host path when `[openshell.drivers.podman]` has `enable_bind_mounts = true`. - `volume`: mounts an existing Podman named volume. The driver validates that - the volume exists before provisioning and never creates or removes it. + the volume exists before provisioning and never creates or removes it. Podman + local-driver volumes created with bind options are treated as host bind + mounts and require `enable_bind_mounts = true`. - `tmpfs`: mounts an in-memory filesystem with optional `options`, `size_bytes`, and `mode`. - `image`: mounts an OCI image through Podman's image-volume API. The driver diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index a9d36b842..c93971b9f 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -196,6 +196,16 @@ pub struct PortMappingEntry { pub host_ip: String, } +/// A named Podman volume returned by the libpod inspect API. +#[derive(Debug, Clone, Default, serde::Deserialize)] +#[serde(rename_all = "PascalCase")] +pub struct VolumeInspect { + #[serde(default)] + pub driver: String, + #[serde(default)] + pub options: HashMap, +} + /// A Podman event from the events stream. #[derive(Debug, Clone, serde::Deserialize)] #[serde(rename_all = "PascalCase")] @@ -497,21 +507,15 @@ impl PodmanClient { } } - /// Return whether a named volume exists. Does not create the volume. - pub async fn volume_exists(&self, name: &str) -> Result { + /// Inspect a named volume. Does not create the volume. + pub async fn inspect_volume(&self, name: &str) -> Result { validate_name(name)?; - match self - .request_json::( - hyper::Method::GET, - &format!("/libpod/volumes/{name}/json"), - None, - ) - .await - { - Ok(_) => Ok(true), - Err(PodmanApiError::NotFound(_)) => Ok(false), - Err(e) => Err(e), - } + self.request_json( + hyper::Method::GET, + &format!("/libpod/volumes/{name}/json"), + None, + ) + .await } // ── Network operations ─────────────────────────────────────────────── @@ -755,6 +759,8 @@ fn url_encode(s: &str) -> String { #[cfg(test)] mod tests { use super::*; + use crate::test_utils::{StubResponse, spawn_podman_stub}; + use hyper::StatusCode; #[test] fn url_encode_encodes_special_characters() { @@ -794,4 +800,33 @@ mod tests { let exact_name = "a".repeat(MAX_NAME_LEN); assert!(validate_name(&exact_name).is_ok()); } + + #[tokio::test] + async fn inspect_volume_parses_driver_options() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "inspect-volume", + vec![StubResponse::new( + StatusCode::OK, + r#"{"Name":"work-bind","Driver":"local","Options":{"type":"none","o":"rw,bind","device":"/srv/work"}}"#, + )], + ); + let client = PodmanClient::new(socket_path.clone()); + + let volume = client + .inspect_volume("work-bind") + .await + .expect("volume inspect should parse"); + + assert_eq!(volume.driver, "local"); + assert_eq!(volume.options.get("o").map(String::as_str), Some("rw,bind")); + handle.await.expect("stub task should finish"); + assert_eq!( + request_log + .lock() + .expect("request log lock should not be poisoned") + .as_slice(), + ["GET /v5.0.0/libpod/volumes/work-bind/json"] + ); + let _ = std::fs::remove_file(socket_path); + } } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index df955ad1c..c4a49a278 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -3,7 +3,7 @@ //! Podman compute driver. -use crate::client::{PodmanApiError, PodmanClient}; +use crate::client::{PodmanApiError, PodmanClient, VolumeInspect}; use crate::config::PodmanComputeConfig; use crate::container::{self, LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID}; use crate::watcher::{ @@ -57,6 +57,15 @@ fn validated_container_name(sandbox_name: &str) -> Result bool { + (volume.driver.is_empty() || volume.driver == "local") + && volume.options.get("o").is_some_and(|options| { + options + .split(',') + .any(|option| option.trim().eq_ignore_ascii_case("bind")) + }) +} + fn sandbox_token_host_path(sandbox_id: &str) -> Result { openshell_core::driver_utils::sandbox_token_path("podman-sandbox-tokens", None, sandbox_id) .map_err(|err| ComputeDriverError::Message(format!("resolve state dir failed: {err}"))) @@ -304,15 +313,21 @@ impl PodmanComputeDriver { container::podman_driver_volume_mount_sources(sandbox, self.config.enable_bind_mounts) .map_err(ComputeDriverError::Precondition)?; for volume in volumes { - let exists = self - .client - .volume_exists(&volume) - .await - .map_err(ComputeDriverError::from)?; - if !exists { - return Err(ComputeDriverError::Precondition(format!( - "podman volume '{volume}' does not exist" - ))); + match self.client.inspect_volume(&volume).await { + Ok(volume_info) => { + if !self.config.enable_bind_mounts && podman_volume_is_bind_backed(&volume_info) + { + return Err(ComputeDriverError::Precondition(format!( + "podman volume '{volume}' is backed by a host bind mount and requires enable_bind_mounts = true in [openshell.drivers.podman]" + ))); + } + } + Err(PodmanApiError::NotFound(_)) => { + return Err(ComputeDriverError::Precondition(format!( + "podman volume '{volume}' does not exist" + ))); + } + Err(err) => return Err(ComputeDriverError::from(err)), } } Ok(()) @@ -674,6 +689,8 @@ mod tests { use super::*; use crate::test_utils::{StubResponse, spawn_podman_stub}; use hyper::StatusCode; + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + use std::collections::HashMap; use std::path::PathBuf; #[test] @@ -795,10 +812,176 @@ mod tests { PodmanComputeDriver::for_tests(config) } + fn test_driver_with_config(config: PodmanComputeConfig) -> PodmanComputeDriver { + PodmanComputeDriver::for_tests(config) + } + + fn json_value(value: serde_json::Value) -> prost_types::Value { + match value { + serde_json::Value::Null => prost_types::Value { kind: None }, + serde_json::Value::Bool(value) => prost_types::Value { + kind: Some(prost_types::value::Kind::BoolValue(value)), + }, + serde_json::Value::Number(value) => prost_types::Value { + kind: value.as_f64().map(prost_types::value::Kind::NumberValue), + }, + serde_json::Value::String(value) => prost_types::Value { + kind: Some(prost_types::value::Kind::StringValue(value)), + }, + serde_json::Value::Array(values) => prost_types::Value { + kind: Some(prost_types::value::Kind::ListValue( + prost_types::ListValue { + values: values.into_iter().map(json_value).collect(), + }, + )), + }, + serde_json::Value::Object(values) => prost_types::Value { + kind: Some(prost_types::value::Kind::StructValue(prost_types::Struct { + fields: values + .into_iter() + .map(|(key, value)| (key, json_value(value))) + .collect(), + })), + }, + } + } + + fn json_struct(value: serde_json::Value) -> prost_types::Struct { + match json_value(value).kind { + Some(prost_types::value::Kind::StructValue(value)) => value, + _ => panic!("expected JSON object"), + } + } + + fn sandbox_with_volume_mount(volume: &str) -> DriverSandbox { + DriverSandbox { + id: "sandbox-123".to_string(), + name: "demo".to_string(), + namespace: String::new(), + spec: Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate { + driver_config: Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "volume", + "source": volume, + "target": "/sandbox/work" + }] + }))), + ..Default::default() + }), + ..Default::default() + }), + status: None, + } + } + fn api_path(path: &str) -> String { format!("/v5.0.0{path}") } + #[test] + fn podman_local_volume_with_bind_option_is_bind_backed() { + let volume = VolumeInspect { + driver: "local".to_string(), + options: HashMap::from([("o".to_string(), "rw,bind".to_string())]), + }; + + assert!(podman_volume_is_bind_backed(&volume)); + } + + #[test] + fn podman_empty_driver_volume_with_bind_option_is_bind_backed() { + let volume = VolumeInspect { + driver: String::new(), + options: HashMap::from([("o".to_string(), "bind".to_string())]), + }; + + assert!(podman_volume_is_bind_backed(&volume)); + } + + #[test] + fn podman_local_volume_without_bind_option_is_not_bind_backed() { + let volume = VolumeInspect { + driver: "local".to_string(), + options: HashMap::from([("o".to_string(), "addr=127.0.0.1,rw".to_string())]), + }; + + assert!(!podman_volume_is_bind_backed(&volume)); + } + + #[test] + fn podman_nonlocal_volume_with_bind_option_is_not_bind_backed() { + let volume = VolumeInspect { + driver: "custom".to_string(), + options: HashMap::from([("o".to_string(), "bind".to_string())]), + }; + + assert!(!podman_volume_is_bind_backed(&volume)); + } + + #[tokio::test] + async fn validate_sandbox_rejects_bind_backed_named_volume_unless_enabled() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "bind-volume-disabled", + vec![StubResponse::new( + StatusCode::OK, + r#"{"Name":"work-bind","Driver":"local","Options":{"type":"none","o":"rw,bind","device":"/srv/work"}}"#, + )], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = sandbox_with_volume_mount("work-bind"); + + let err = driver + .validate_sandbox_create(&sandbox) + .await + .expect_err("bind-backed volume should require bind mount opt-in"); + + match err { + ComputeDriverError::Precondition(message) => { + assert!(message.contains("enable_bind_mounts = true")); + } + other => panic!("expected precondition error, got {other:?}"), + } + handle.await.expect("stub task should finish"); + assert_eq!( + request_log + .lock() + .expect("request log lock should not be poisoned") + .as_slice(), + [format!( + "GET {}", + api_path("/libpod/volumes/work-bind/json") + )] + ); + let _ = std::fs::remove_file(socket_path); + } + + #[tokio::test] + async fn validate_sandbox_allows_bind_backed_named_volume_when_enabled() { + let (socket_path, _request_log, handle) = spawn_podman_stub( + "bind-volume-enabled", + vec![StubResponse::new( + StatusCode::OK, + r#"{"Name":"work-bind","Driver":"local","Options":{"type":"none","o":"rw,bind","device":"/srv/work"}}"#, + )], + ); + let config = PodmanComputeConfig { + socket_path: socket_path.clone(), + enable_bind_mounts: true, + ..PodmanComputeConfig::default() + }; + let driver = test_driver_with_config(config); + let sandbox = sandbox_with_volume_mount("work-bind"); + + driver + .validate_sandbox_create(&sandbox) + .await + .expect("bind-backed volume should be allowed when bind mounts are enabled"); + + handle.await.expect("stub task should finish"); + let _ = std::fs::remove_file(socket_path); + } + #[tokio::test] async fn delete_sandbox_cleans_up_with_request_id_when_container_is_already_gone() { let sandbox_id = "sandbox-123"; diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 1ddf70552..e7785dc17 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -259,6 +259,9 @@ supervisor_image = "ghcr.io/nvidia/openshell/supervisor:latest" guest_tls_ca = "/etc/openshell/certs/ca.pem" guest_tls_cert = "/etc/openshell/certs/client.pem" guest_tls_key = "/etc/openshell/certs/client-key.pem" +# Unsafe operator override. Host bind mounts, including Podman local-driver +# bind-backed volumes, expose gateway-host paths inside sandboxes and can +# negate OpenShell isolation and filesystem controls. enable_bind_mounts = false # Set to 0 to leave Podman's runtime default unchanged. sandbox_pids_limit = 2048 diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 9526413fc..6e049c3c8 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -140,8 +140,11 @@ On macOS with `podman machine`, the driver uses gvproxy's host-loopback IP, `192 Podman driver config accepts user-supplied `volume`, `tmpfs`, and `image` mounts. It also accepts `bind` mounts when `[openshell.drivers.podman]` sets -`enable_bind_mounts = true` in `gateway.toml`. Host bind mounts expose gateway -host paths to sandbox requests, so they are disabled by default. +`enable_bind_mounts = true` in `gateway.toml`. Podman local-driver named +volumes created with bind options also expose gateway-host paths, so +OpenShell treats them like bind mounts and requires `enable_bind_mounts = true`. +Host bind mounts expose gateway host paths to sandbox requests, so they are +disabled by default. Use a `volume` mount for existing Podman named volumes: @@ -178,7 +181,7 @@ Podman mount schema: | Type | Fields | |---|---| | `bind` | `source`, `target`, optional `read_only` (`true` by default). `source` must be an absolute host path. Requires `enable_bind_mounts = true`. | -| `volume` | `source`, `target`, optional `read_only` (`true` by default). The named volume must already exist. | +| `volume` | `source`, `target`, optional `read_only` (`true` by default). The named volume must already exist. Podman local-driver bind-backed volumes require `enable_bind_mounts = true`. | | `tmpfs` | `target`, optional `options`, optional `size_bytes`, optional `mode`. | | `image` | `source`, `target`, optional `read_only` (`true` by default). | From d21b5f2f280d634e9f510c1312e9582caa3c329b Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 11 Jun 2026 08:50:07 +0200 Subject: [PATCH 2/2] fix(drivers): detect recursive bind-backed volumes Signed-off-by: Evan Lezar --- crates/openshell-driver-docker/src/lib.rs | 7 +-- crates/openshell-driver-docker/src/tests.rs | 14 +++++ crates/openshell-driver-podman/src/driver.rs | 54 ++++++++++++++++++-- 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 607e17bcb..fae8f82b9 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -1821,9 +1821,10 @@ fn docker_tmpfs_option(option: &str) -> Result, Status> { fn docker_volume_is_bind_backed(volume: &bollard::models::Volume) -> bool { volume.driver == "local" && volume.options.get("o").is_some_and(|options| { - options - .split(',') - .any(|option| option.trim().eq_ignore_ascii_case("bind")) + options.split(',').any(|option| { + let option = option.trim(); + option.eq_ignore_ascii_case("bind") || option.eq_ignore_ascii_case("rbind") + }) }) } diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index cb3ef5c12..896ee6a7b 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -871,6 +871,20 @@ fn docker_local_volume_with_bind_option_is_bind_backed() { assert!(docker_volume_is_bind_backed(&volume)); } +#[test] +fn docker_local_volume_with_rbind_option_is_bind_backed() { + let volume = inspected_volume( + "local", + HashMap::from([ + ("type".to_string(), "none".to_string()), + ("o".to_string(), "rw,rbind".to_string()), + ("device".to_string(), "/tmp/openshell".to_string()), + ]), + ); + + assert!(docker_volume_is_bind_backed(&volume)); +} + #[test] fn docker_local_volume_without_bind_option_is_not_bind_backed() { let volume = inspected_volume( diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index c4a49a278..bd744a102 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -60,9 +60,10 @@ fn validated_container_name(sandbox_name: &str) -> Result bool { (volume.driver.is_empty() || volume.driver == "local") && volume.options.get("o").is_some_and(|options| { - options - .split(',') - .any(|option| option.trim().eq_ignore_ascii_case("bind")) + options.split(',').any(|option| { + let option = option.trim(); + option.eq_ignore_ascii_case("bind") || option.eq_ignore_ascii_case("rbind") + }) }) } @@ -889,6 +890,16 @@ mod tests { assert!(podman_volume_is_bind_backed(&volume)); } + #[test] + fn podman_local_volume_with_rbind_option_is_bind_backed() { + let volume = VolumeInspect { + driver: "local".to_string(), + options: HashMap::from([("o".to_string(), "rw,rbind".to_string())]), + }; + + assert!(podman_volume_is_bind_backed(&volume)); + } + #[test] fn podman_empty_driver_volume_with_bind_option_is_bind_backed() { let volume = VolumeInspect { @@ -956,6 +967,43 @@ mod tests { let _ = std::fs::remove_file(socket_path); } + #[tokio::test] + async fn validate_sandbox_rejects_rbind_backed_named_volume_unless_enabled() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "rbind-volume-disabled", + vec![StubResponse::new( + StatusCode::OK, + r#"{"Name":"work-rbind","Driver":"local","Options":{"type":"none","o":"rw,rbind","device":"/srv/work"}}"#, + )], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = sandbox_with_volume_mount("work-rbind"); + + let err = driver + .validate_sandbox_create(&sandbox) + .await + .expect_err("rbind-backed volume should require bind mount opt-in"); + + match err { + ComputeDriverError::Precondition(message) => { + assert!(message.contains("enable_bind_mounts = true")); + } + other => panic!("expected precondition error, got {other:?}"), + } + handle.await.expect("stub task should finish"); + assert_eq!( + request_log + .lock() + .expect("request log lock should not be poisoned") + .as_slice(), + [format!( + "GET {}", + api_path("/libpod/volumes/work-rbind/json") + )] + ); + let _ = std::fs::remove_file(socket_path); + } + #[tokio::test] async fn validate_sandbox_allows_bind_backed_named_volume_when_enabled() { let (socket_path, _request_log, handle) = spawn_podman_stub(