From 53e7b71f1903584d0350465971f9a0663a1cac30 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Tue, 29 Sep 2026 22:48:12 -0700 Subject: [PATCH 1/4] fix(podman): create managed workspace volumes owned by the workload identity Podman now creates the managed /sandbox volume with uid/gid options for the resolved workload identity, so the workload starts directly as that identity. This fixes rootful sandboxes whose image USER or policy run_as_user could not write to a root-owned /sandbox, and removes the root-then-drop workspace chown start path. Resource admission accepts the managed workspace volume when its options match the workload container's final identity, or are empty for volumes created by older gateways. The channel volume still requires empty options. Signed-off-by: Matthew Grossman --- crates/openshell-driver-podman/README.md | 3 +- crates/openshell-driver-podman/src/client.rs | 78 +++++++++- .../openshell-driver-podman/src/container.rs | 89 +++-------- crates/openshell-driver-podman/src/driver.rs | 144 +++++++++++++++--- e2e/rust/tests/podman_oci_identity.rs | 5 +- 5 files changed, 226 insertions(+), 93 deletions(-) diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index abf2147513..85a31f2cf8 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -43,7 +43,8 @@ stopped Podman container does not populate nested named volumes. Restart restore only the channel bootstrap into the existing channel volume, preserving the workspace. The workload starts before the supervisor so its user namespace exists when the supervisor joins it; a stopped supervisor resolves that namespace again -on its next start. +on its next start. The driver creates the managed workspace volume owned by +the workload's final UID and GID, so the workload never starts as root. The runtime must pass the sandbox's unprivileged enforcement probe, including nested seccomp notification and Landlock. Unsupported runtime defaults fail diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 80e24513a3..f29ab1113e 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -166,6 +166,8 @@ pub struct PortBinding { pub struct ContainerConfig { #[serde(default)] pub labels: HashMap, + #[serde(default)] + pub user: String, } /// Immutable image metadata needed to bind OCI identity inspection to launch. @@ -187,6 +189,25 @@ pub struct ImageConfig { pub env: Vec, } +/// Whether a driver-owned volume has exactly the options `OpenShell` creates it +/// with: none, or `uid`/`gid` for `owner`. Podman records the parsed `UID` and +/// `GID` next to the raw `o` option. +pub fn volume_options_match_owner( + options: &HashMap, + owner: Option<(u32, u32)>, +) -> bool { + let Some((uid, gid)) = owner else { + return options.is_empty(); + }; + options.get("o").map(String::as_str) == Some(format!("uid={uid},gid={gid}").as_str()) + && options.iter().all(|(key, value)| match key.as_str() { + "o" => true, + "UID" => *value == uid.to_string(), + "GID" => *value == gid.to_string(), + _ => false, + }) +} + /// A container summary returned by the list API. #[derive(Debug, Clone, serde::Deserialize)] #[serde(rename_all = "PascalCase")] @@ -701,12 +722,18 @@ impl PodmanClient { // ── Volume operations ──────────────────────────────────────────────── /// Never adopt an unrelated existing volume on a private provisioning path. + /// + /// With `owner`, Podman creates the volume root owned by that UID and GID, + /// so a non-root workload can use it without a privileged chown. pub(crate) async fn create_owned_volume( &self, name: &str, sandbox_id: &str, workspace: &str, + owner: Option<(u32, u32)>, ) -> Result<(), PodmanApiError> { + let owned_as_requested = + |options: &HashMap| volume_options_match_owner(options, owner); let labels = HashMap::from([ ( openshell_core::driver_utils::LABEL_SANDBOX_ID.to_string(), @@ -720,7 +747,7 @@ impl PodmanClient { match self.inspect_volume(name).await { Ok(existing) => { if existing.driver != "local" - || !existing.options.is_empty() + || !owned_as_requested(&existing.options) || existing.labels.as_ref() != Some(&labels) { return Err(PodmanApiError::InvalidInput( @@ -732,14 +759,15 @@ impl PodmanClient { Err(PodmanApiError::NotFound(_)) => {} Err(error) => return Err(error), } - self.create_ignore_conflict( - "/libpod/volumes/create", - &serde_json::json!({"Name":name,"Driver":"local","Labels":labels}), - ) - .await?; + let mut body = serde_json::json!({"Name":name,"Driver":"local","Labels":labels}); + if let Some((uid, gid)) = owner { + body["Options"] = serde_json::json!({ "o": format!("uid={uid},gid={gid}") }); + } + self.create_ignore_conflict("/libpod/volumes/create", &body) + .await?; let created = self.inspect_volume(name).await?; if created.driver != "local" - || !created.options.is_empty() + || !owned_as_requested(&created.options) || created.labels.as_ref() != Some(&labels) { return Err(PodmanApiError::InvalidInput( @@ -1157,6 +1185,42 @@ mod tests { let _ = std::fs::remove_file(socket_path); } + #[tokio::test] + async fn create_owned_volume_verifies_requested_owner() { + let labels = + r#"{"openshell.ai/sandbox-id":"sandbox-1","openshell.ai/sandbox-workspace":"team-a"}"#; + for (options, accepted) in [ + ( + r#"{"o":"uid=1234,gid=1235","UID":"1234","GID":"1235"}"#, + true, + ), + (r#"{"o":"uid=1234,gid=1235"}"#, true), + (r#"{"o":"uid=1234,gid=1235","UID":"0","GID":"1235"}"#, false), + (r#"{"o":"uid=1234,gid=1235","device":"/srv/work"}"#, false), + ("{}", false), + ] { + let (socket_path, _, handle) = spawn_podman_stub( + "owned-volume", + vec![ + StubResponse::new(StatusCode::NOT_FOUND, ""), + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new( + StatusCode::OK, + format!( + r#"{{"Name":"work","Driver":"local","Options":{options},"Labels":{labels}}}"# + ), + ), + ], + ); + let result = PodmanClient::new(socket_path.clone()) + .create_owned_volume("work", "sandbox-1", "team-a", Some((1234, 1235))) + .await; + assert_eq!(result.is_ok(), accepted, "options {options}: {result:?}"); + handle.await.expect("stub task should finish"); + let _ = std::fs::remove_file(socket_path); + } + } + #[tokio::test] async fn inspect_image_reads_immutable_id_and_oci_user() { let (socket_path, request_log, handle) = spawn_podman_stub( diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 51245a244a..dfd8b29443 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -1406,8 +1406,6 @@ pub struct IsolationSpecInput<'a> { pub supervisor_bin: Option<&'a Path>, pub tls_secrets: Option<&'a [String; 1]>, pub identity: &'a openshell_isolation_interface::contract::ResolvedWorkloadIdentity, - /// Whether this workload is created by a rootless Podman service. - pub rootless: bool, } pub struct IsolationSpecs { @@ -1459,44 +1457,19 @@ pub fn build_isolation_specs( .iter() .filter_map(|entry| entry.split_once('=').map(|(key, _)| key.to_string())) .collect(); - if input.rootless || input.identity.source == "default" { - // Podman's archive endpoint leaves named-volume contents owned by - // container root for rootless services and for a rootful USER-less - // image's newly-created workspace. Start the trusted runtime as root - // only long enough to chown the workspace, then irreversibly drop to - // the resolved workload identity before reading bootstrap material or - // accepting a control connection. - workload.command = vec![ - "launch-capability-free".into(), - input.identity.uid.to_string(), - input.identity.gid.to_string(), - crate::isolation::BOOTSTRAP_PATH.into(), - driver_mounts::DEFAULT_WORKSPACE_ROOT.into(), - ]; - workload.user = "0:0".into(); - workload.groups.clear(); - workload.cap_drop = vec!["ALL".into()]; - workload.cap_add = vec![ - "CHOWN".into(), - "SETGID".into(), - "SETUID".into(), - "SETPCAP".into(), - ]; - } else { - workload.command = vec![ - "--bootstrap".into(), - crate::isolation::BOOTSTRAP_PATH.into(), - ]; - workload.user.clone_from(&user); - workload.groups = input - .identity - .supplementary_gids - .iter() - .map(ToString::to_string) - .collect(); - workload.cap_drop = vec!["ALL".into()]; - workload.cap_add.clear(); - } + workload.command = vec![ + "--bootstrap".into(), + crate::isolation::BOOTSTRAP_PATH.into(), + ]; + workload.user.clone_from(&user); + workload.groups = input + .identity + .supplementary_gids + .iter() + .map(ToString::to_string) + .collect(); + workload.cap_drop = vec!["ALL".into()]; + workload.cap_add.clear(); workload.apparmor_profile = input .config .app_armor_profile @@ -1790,7 +1763,6 @@ mod tests { supervisor_bin: None, tls_secrets: None, identity: &identity, - rootless: true, }) .unwrap(); for spec in [&specs.workload, &specs.supervisor] { @@ -1798,21 +1770,14 @@ mod tests { assert!(spec.seccomp_profile_path.is_empty()); assert!(spec.no_new_privileges); } - assert_eq!(specs.workload.user, "0:0"); - assert!(specs.workload.groups.is_empty()); - assert_eq!( - specs.workload.cap_add, - vec!["CHOWN", "SETGID", "SETUID", "SETPCAP"] - ); + // The driver creates the managed workspace volume owned by the + // workload identity, so the workload never starts as root. + assert_eq!(specs.workload.user, "1000:1001"); + assert_eq!(specs.workload.groups, vec!["2000"]); + assert!(specs.workload.cap_add.is_empty()); assert_eq!( specs.workload.command, - vec![ - "launch-capability-free", - "1000", - "1001", - crate::isolation::BOOTSTRAP_PATH, - driver_mounts::DEFAULT_WORKSPACE_ROOT, - ] + vec!["--bootstrap", crate::isolation::BOOTSTRAP_PATH] ); assert_eq!(specs.supervisor.user, "1000:1001"); assert_eq!(specs.supervisor.groups, vec!["2000"]); @@ -1841,7 +1806,7 @@ mod tests { "sha256:image".into(), ) .unwrap(); - let rootful_specs = build_isolation_specs(IsolationSpecInput { + let default_specs = build_isolation_specs(IsolationSpecInput { sandbox: &sandbox, config: &config, token_secret: Some("jwt"), @@ -1854,19 +1819,13 @@ mod tests { supervisor_bin: None, tls_secrets: None, identity: &default_identity, - rootless: false, }) .unwrap(); - assert_eq!(rootful_specs.workload.user, "0:0"); + assert_eq!(default_specs.workload.user, "1000:1000"); + assert!(default_specs.workload.cap_add.is_empty()); assert_eq!( - rootful_specs.workload.command, - vec![ - "launch-capability-free", - "1000", - "1000", - crate::isolation::BOOTSTRAP_PATH, - driver_mounts::DEFAULT_WORKSPACE_ROOT, - ] + default_specs.workload.command, + vec!["--bootstrap", crate::isolation::BOOTSTRAP_PATH] ); let workload_json = serde_json::to_string(&specs.workload).unwrap(); assert!(workload_json.contains("\"apparmor_profile\":\"openshell-sandbox\"")); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index d3023cddd8..c7678a9c66 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -110,8 +110,6 @@ impl From for ComputeDriverError { pub struct PodmanComputeDriver { client: PodmanClient, config: PodmanComputeConfig, - /// Whether Podman's service is running without root privileges. - rootless: bool, gpu_selector: Arc, gpu_inventory_refresh: Arc (CdiGpuInventory, bool) + Send + Sync>, lifecycle_event_fences: LifecycleEventFences, @@ -123,7 +121,6 @@ impl std::fmt::Debug for PodmanComputeDriver { .field("socket_path", &self.config.socket_path) .field("default_image", &self.config.default_image) .field("network_name", &self.config.network_name) - .field("rootless", &self.rootless) .field("gpu_inventory", &self.gpu_selector.device_ids()) .finish() } @@ -145,6 +142,12 @@ fn validated_container_name(sandbox: &DriverSandbox) -> Result Option<(u32, u32)> { + let (uid, gid) = user.split_once(':')?; + Some((uid.parse().ok()?, gid.parse().ok()?)) +} + fn podman_volume_is_bind_backed(volume: &VolumeInspect) -> bool { (volume.driver.is_empty() || volume.driver == "local") && volume.options.get("o").is_some_and(|options| { @@ -464,7 +467,7 @@ impl PodmanComputeDriver { } // Verify cgroups v2, detect rootless mode, and log system info. - let rootless = match client.system_info().await { + match client.system_info().await { Ok(info) => { if info.host.cgroup_version != "v2" { return Err(PodmanApiError::Connection(format!( @@ -486,14 +489,13 @@ impl PodmanComputeDriver { apparmor_enabled = info.host.security.apparmor_enabled, "Connected to Podman" ); - info.host.security.rootless } Err(e) => { return Err(PodmanApiError::Connection(format!( "failed to query Podman system info: {e}" ))); } - }; + } // Rootless pre-flight: warn if subuid/subgid ranges look missing. // Not a hard error because some systems configure these via LDAP or @@ -529,7 +531,6 @@ impl PodmanComputeDriver { let driver = Self { client, config, - rootless, gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -775,14 +776,25 @@ impl PodmanComputeDriver { if volume.name != name { return Err(missing()); } - if name == container::volume_name(sandbox_id) - || name == crate::isolation::channel_volume_name(sandbox_id) + let workspace_volume = name == container::volume_name(sandbox_id); + if workspace_volume || name == crate::isolation::channel_volume_name(sandbox_id) { let owned = volume.labels.as_ref().is_some_and(|labels| { labels.get(LABEL_SANDBOX_ID) == Some(sandbox_id) && labels.get(container::LABEL_SANDBOX_WORKSPACE) == Some(workspace) }); - if !owned || volume.driver != "local" || !volume.options.is_empty() { + // The channel volume has no options. The managed + // workspace is owned by the container's final identity, + // or has no options when an older gateway created it. + let options_ok = volume.options.is_empty() + || (workspace_volume + && numeric_user(&inspect.config.user).is_some_and(|owner| { + crate::client::volume_options_match_owner( + &volume.options, + Some(owner), + ) + })); + if !owned || volume.driver != "local" || !options_ok { return Err(missing()); } } else { @@ -1032,7 +1044,12 @@ impl PodmanComputeDriver { let phase_status = openshell_otel::ErrorStatusGuard::current(); let result = async { self.client - .create_owned_volume(&vol_name, &sandbox.id, &sandbox.workspace) + .create_owned_volume( + &vol_name, + &sandbox.id, + &sandbox.workspace, + Some((identity.uid, identity.gid)), + ) .await .map_err(ComputeDriverError::from)?; let resolver_secret_name = @@ -1160,7 +1177,6 @@ impl PodmanComputeDriver { supervisor_bin: supervisor_bin_path.as_deref(), tls_secrets: tls_secret_names.as_ref(), identity: &identity, - rootless: self.rootless, }); let mut specs = match specs { Ok(spec) => spec, @@ -1175,7 +1191,7 @@ impl PodmanComputeDriver { let identities = self.validate_user_volume_mounts_available(sandbox).await?; specs.record_resource_identities(&identities)?; self.client - .create_owned_volume(&channel_volume, &sandbox.id, &sandbox.workspace) + .create_owned_volume(&channel_volume, &sandbox.id, &sandbox.workspace, None) .await?; channel_owned.store(true, std::sync::atomic::Ordering::Relaxed); let workload_id = self.client.create_typed_container(&specs.workload).await?; @@ -1806,7 +1822,6 @@ impl PodmanComputeDriver { Self { client, config, - rootless: false, gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -3022,7 +3037,7 @@ mod tests { assert!( driver .client - .create_owned_volume("private-collision", "sandbox-123", "team-a") + .create_owned_volume("private-collision", "sandbox-123", "team-a", None) .await .is_err() ); @@ -3079,6 +3094,80 @@ mod tests { } } + #[tokio::test] + async fn admission_accepts_workspace_volume_owned_by_workload_identity() { + let owned = serde_json::json!({"o":"uid=1234,gid=1235","UID":"1234","GID":"1235"}); + for (workspace_options, channel_options, allowed) in [ + (owned.clone(), serde_json::json!({}), true), + // Created by a gateway that did not set volume ownership. + (serde_json::json!({}), serde_json::json!({}), true), + ( + serde_json::json!({"o":"uid=0,gid=0","UID":"0","GID":"0"}), + serde_json::json!({}), + false, + ), + (owned.clone(), owned.clone(), false), + ] { + let sandbox_id = "sandbox-owned"; + let workspace_volume = container::volume_name(sandbox_id); + let channel_volume = crate::isolation::channel_volume_name(sandbox_id); + let volume = |name: &str, options: &serde_json::Value| { + StubResponse::new( + StatusCode::OK, + serde_json::json!({ + "Name": name, "Driver": "local", "Options": options, + "Labels": {LABEL_SANDBOX_ID: sandbox_id, container::LABEL_SANDBOX_WORKSPACE: "team-a"} + }) + .to_string(), + ) + }; + let container = serde_json::json!({ + "Id": "workload", "Name": "workload", "State": {"Status": "created", "Running": false}, + "Config": { + "User": "1234:1235", + "Labels": { + LABEL_SANDBOX_ID: sandbox_id, + container::LABEL_SANDBOX_WORKSPACE: "team-a", + openshell_core::resource_admission::CONFIG_USED_LABEL: "false", + openshell_core::resource_admission::IDENTITIES_LABEL: "{}", + } + }, + "Mounts": [ + {"Type": "volume", "Name": workspace_volume}, + {"Type": "volume", "Name": channel_volume}, + ] + }); + let (socket, _, handle) = spawn_podman_stub( + "admission-owned", + vec![ + StubResponse::new(StatusCode::OK, container.to_string()), + volume(&workspace_volume, &workspace_options), + volume(&channel_volume, &channel_options), + ], + ); + let driver = PodmanComputeDriver::for_tests(PodmanComputeConfig { + socket_path: Some(socket.clone()), + resource_admission: openshell_core::resource_admission::ResourceAdmissionConfig { + enabled: true, + ..Default::default() + }, + ..Default::default() + }); + let result = driver.admit_container_resources("workload").await; + assert_eq!( + result.is_ok(), + allowed, + "workspace {workspace_options}, channel {channel_options}: {result:?}" + ); + if allowed { + handle.await.unwrap(); + } else { + handle.abort(); + } + let _ = fs::remove_file(socket); + } + } + #[tokio::test] async fn admission_driver_config_denial_does_not_contact_podman() { for enabled in [true, false] { @@ -3538,7 +3627,11 @@ mod tests { image_response("sha256:supervisor"), StubResponse::new(StatusCode::NOT_FOUND, ""), // no existing private workspace StubResponse::new(StatusCode::CREATED, "{}"), // workspace volume - owned_volume_response(&container::volume_name(sandbox_id), sandbox_id), + owned_volume_response( + &container::volume_name(sandbox_id), + sandbox_id, + Some((1234, 1235)), // the stub image's OCI user + ), StubResponse::new(StatusCode::CREATED, "{}"), // resolver secret ]; if proxy_secret { @@ -3555,15 +3648,30 @@ mod tests { responses.push(owned_volume_response( &crate::isolation::channel_volume_name(sandbox_id), sandbox_id, + None, )); responses } - fn owned_volume_response(name: &str, sandbox_id: &str) -> StubResponse { + fn owned_volume_response( + name: &str, + sandbox_id: &str, + owner: Option<(u32, u32)>, + ) -> StubResponse { + let options = owner.map_or_else( + || serde_json::json!({}), + |(uid, gid)| { + serde_json::json!({ + "o": format!("uid={uid},gid={gid}"), + "UID": uid.to_string(), + "GID": gid.to_string(), + }) + }, + ); StubResponse::new( StatusCode::OK, serde_json::json!({ - "Name": name, "Driver": "local", "Options": {}, + "Name": name, "Driver": "local", "Options": options, "Labels": {LABEL_SANDBOX_ID: sandbox_id, container::LABEL_SANDBOX_WORKSPACE: ""} }) .to_string(), diff --git a/e2e/rust/tests/podman_oci_identity.rs b/e2e/rust/tests/podman_oci_identity.rs index 3405d730c9..d78e4c7454 100644 --- a/e2e/rust/tests/podman_oci_identity.rs +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -258,8 +258,9 @@ async fn assert_isolated_pair(image: &ImageGuard, sandbox: &SandboxGuard, contai ) .unwrap(); assert_eq!( - workload_user, "0:0", - "the trusted rootless boundary starts as container root before dropping to the OCI identity" + workload_user, + format!("{OCI_UID}:{OCI_GID}"), + "the workload must start directly as the final OCI identity" ); let supervisor_user = run_engine( &image.engine, From 20aae0d396f6b38e1b40f0dbb5c9cdbcd5ad5e7f Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 2 Oct 2026 13:38:32 +0200 Subject: [PATCH 2/4] test(podman): cover managed volume reuse and workspace access Signed-off-by: Evan Lezar --- crates/openshell-driver-podman/src/client.rs | 79 +++++++++++++++----- e2e/rust/tests/podman_oci_identity.rs | 17 +++++ 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index f29ab1113e..df985dc814 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -1186,38 +1186,77 @@ mod tests { } #[tokio::test] - async fn create_owned_volume_verifies_requested_owner() { + async fn create_owned_volume_verifies_requested_options() { let labels = r#"{"openshell.ai/sandbox-id":"sandbox-1","openshell.ai/sandbox-workspace":"team-a"}"#; - for (options, accepted) in [ + for (owner, options, accepted) in [ ( + Some((1234, 1235)), r#"{"o":"uid=1234,gid=1235","UID":"1234","GID":"1235"}"#, true, ), - (r#"{"o":"uid=1234,gid=1235"}"#, true), - (r#"{"o":"uid=1234,gid=1235","UID":"0","GID":"1235"}"#, false), - (r#"{"o":"uid=1234,gid=1235","device":"/srv/work"}"#, false), - ("{}", false), + (Some((1234, 1235)), r#"{"o":"uid=1234,gid=1235"}"#, true), + // Podman accepts either order, but OpenShell always requests uid first. + (Some((1234, 1235)), r#"{"o":"gid=1235,uid=1234"}"#, false), + ( + Some((1234, 1235)), + r#"{"o":"uid=1234,gid=1235","UID":"0","GID":"1235"}"#, + false, + ), + ( + Some((1234, 1235)), + r#"{"o":"uid=1234,gid=1235","device":"/srv/work"}"#, + false, + ), + (Some((1234, 1235)), "{}", false), + (None, "{}", true), + (None, r#"{"o":"uid=1234,gid=1235"}"#, false), + (None, r#"{"o":"bind","device":"/srv/work"}"#, false), ] { - let (socket_path, _, handle) = spawn_podman_stub( - "owned-volume", - vec![ - StubResponse::new(StatusCode::NOT_FOUND, ""), - StubResponse::new(StatusCode::CREATED, "{}"), + for existing in [false, true] { + let inspected = || { StubResponse::new( StatusCode::OK, format!( r#"{{"Name":"work","Driver":"local","Options":{options},"Labels":{labels}}}"# ), - ), - ], - ); - let result = PodmanClient::new(socket_path.clone()) - .create_owned_volume("work", "sandbox-1", "team-a", Some((1234, 1235))) - .await; - assert_eq!(result.is_ok(), accepted, "options {options}: {result:?}"); - handle.await.expect("stub task should finish"); - let _ = std::fs::remove_file(socket_path); + ) + }; + let responses = if existing { + vec![inspected()] + } else { + vec![ + StubResponse::new(StatusCode::NOT_FOUND, ""), + StubResponse::new(StatusCode::CREATED, "{}"), + inspected(), + ] + }; + let (socket_path, request_log, handle) = + spawn_podman_stub("owned-volume", responses); + let result = PodmanClient::new(socket_path.clone()) + .create_owned_volume("work", "sandbox-1", "team-a", owner) + .await; + assert_eq!( + result.is_ok(), + accepted, + "owner {owner:?}, options {options}, existing {existing}: {result:?}" + ); + handle.await.expect("stub task should finish"); + let expected_requests = if existing { + vec!["GET /v5.0.0/libpod/volumes/work/json"] + } else { + vec![ + "GET /v5.0.0/libpod/volumes/work/json", + "POST /v5.0.0/libpod/volumes/create", + "GET /v5.0.0/libpod/volumes/work/json", + ] + }; + assert_eq!( + request_log.lock().expect("request log lock").as_slice(), + expected_requests, + ); + let _ = std::fs::remove_file(socket_path); + } } } diff --git a/e2e/rust/tests/podman_oci_identity.rs b/e2e/rust/tests/podman_oci_identity.rs index d78e4c7454..50aaba399f 100644 --- a/e2e/rust/tests/podman_oci_identity.rs +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -244,6 +244,23 @@ async fn podman_uses_oci_identity_and_inspected_image_id() { "Podman sandbox must launch the immutable image ID inspected before creation" ); + let workspace_output = sandbox + .exec(&[ + "sh", + "-c", + "set -eu; stat -c 'workspace-owner=%u:%g' /sandbox; touch /sandbox/probe; rm /sandbox/probe; echo podman-workspace-write-ok", + ]) + .await + .expect("OCI workload should be able to write to the managed workspace"); + assert!( + workspace_output.contains(&format!("workspace-owner={OCI_UID}:{OCI_GID}")), + "expected workspace owner {OCI_UID}:{OCI_GID}:\n{workspace_output}" + ); + assert!( + workspace_output.contains("podman-workspace-write-ok"), + "expected workspace write marker:\n{workspace_output}" + ); + assert_isolated_pair(&image, &sandbox, &container_id).await; sandbox.cleanup().await; } From 59ba0b889f8223bdba7b94e97f18333e7eeeb4e3 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 2 Oct 2026 13:41:48 +0200 Subject: [PATCH 3/4] refactor(podman): clarify managed volume creation and validation Signed-off-by: Evan Lezar --- crates/openshell-driver-podman/src/client.rs | 96 ++++++++++++-------- crates/openshell-driver-podman/src/driver.rs | 5 +- 2 files changed, 61 insertions(+), 40 deletions(-) diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index df985dc814..3d6b79f622 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -189,25 +189,6 @@ pub struct ImageConfig { pub env: Vec, } -/// Whether a driver-owned volume has exactly the options `OpenShell` creates it -/// with: none, or `uid`/`gid` for `owner`. Podman records the parsed `UID` and -/// `GID` next to the raw `o` option. -pub fn volume_options_match_owner( - options: &HashMap, - owner: Option<(u32, u32)>, -) -> bool { - let Some((uid, gid)) = owner else { - return options.is_empty(); - }; - options.get("o").map(String::as_str) == Some(format!("uid={uid},gid={gid}").as_str()) - && options.iter().all(|(key, value)| match key.as_str() { - "o" => true, - "UID" => *value == uid.to_string(), - "GID" => *value == gid.to_string(), - _ => false, - }) -} - /// A container summary returned by the list API. #[derive(Debug, Clone, serde::Deserialize)] #[serde(rename_all = "PascalCase")] @@ -259,6 +240,38 @@ pub struct VolumeInspect { } impl VolumeInspect { + /// Whether metadata matches a managed local volume with the exact labels + /// and requested ownership options. This does not inspect filesystem ownership. + pub(crate) fn matches_managed_volume( + &self, + labels: &HashMap, + requested_owner: Option<(u32, u32)>, + ) -> bool { + self.driver == "local" + && self.labels.as_ref() == Some(labels) + && self.options_match_requested_owner(requested_owner) + } + + /// Whether option metadata matches the requested owner. `None` means no + /// ownership options were requested and requires an empty options map. + /// This does not inspect filesystem ownership. Podman records the parsed + /// `UID` and `GID` next to the raw `o` option. + pub(crate) fn options_match_requested_owner( + &self, + requested_owner: Option<(u32, u32)>, + ) -> bool { + let Some((uid, gid)) = requested_owner else { + return self.options.is_empty(); + }; + self.options.get("o").map(String::as_str) == Some(format!("uid={uid},gid={gid}").as_str()) + && self.options.iter().all(|(key, value)| match key.as_str() { + "o" => true, + "UID" => *value == uid.to_string(), + "GID" => *value == gid.to_string(), + _ => false, + }) + } + pub(crate) fn admission_identity(&self) -> Value { serde_json::json!({"name": self.name, "driver": self.driver, "options": self.options, "created_at": self.created_at}) } @@ -721,6 +734,28 @@ impl PodmanClient { // ── Volume operations ──────────────────────────────────────────────── + /// Create and inspect a local volume. HTTP 409 conflicts also proceed to + /// inspection; callers must verify the returned labels and options. + async fn create_volume( + &self, + name: &str, + labels: &HashMap, + options: &HashMap, + ) -> Result { + validate_name(name)?; + let mut body = serde_json::json!({ + "Name": name, + "Driver": "local", + "Labels": labels, + }); + if !options.is_empty() { + body["Options"] = serde_json::json!(options); + } + self.create_ignore_conflict("/libpod/volumes/create", &body) + .await?; + self.inspect_volume(name).await + } + /// Never adopt an unrelated existing volume on a private provisioning path. /// /// With `owner`, Podman creates the volume root owned by that UID and GID, @@ -732,8 +767,6 @@ impl PodmanClient { workspace: &str, owner: Option<(u32, u32)>, ) -> Result<(), PodmanApiError> { - let owned_as_requested = - |options: &HashMap| volume_options_match_owner(options, owner); let labels = HashMap::from([ ( openshell_core::driver_utils::LABEL_SANDBOX_ID.to_string(), @@ -746,10 +779,7 @@ impl PodmanClient { ]); match self.inspect_volume(name).await { Ok(existing) => { - if existing.driver != "local" - || !owned_as_requested(&existing.options) - || existing.labels.as_ref() != Some(&labels) - { + if !existing.matches_managed_volume(&labels, owner) { return Err(PodmanApiError::InvalidInput( "private volume name collides with an unrelated resource".into(), )); @@ -759,17 +789,11 @@ impl PodmanClient { Err(PodmanApiError::NotFound(_)) => {} Err(error) => return Err(error), } - let mut body = serde_json::json!({"Name":name,"Driver":"local","Labels":labels}); - if let Some((uid, gid)) = owner { - body["Options"] = serde_json::json!({ "o": format!("uid={uid},gid={gid}") }); - } - self.create_ignore_conflict("/libpod/volumes/create", &body) - .await?; - let created = self.inspect_volume(name).await?; - if created.driver != "local" - || !owned_as_requested(&created.options) - || created.labels.as_ref() != Some(&labels) - { + let options = owner.map_or_else(HashMap::new, |(uid, gid)| { + HashMap::from([("o".to_string(), format!("uid={uid},gid={gid}"))]) + }); + let created = self.create_volume(name, &labels, &options).await?; + if !created.matches_managed_volume(&labels, owner) { return Err(PodmanApiError::InvalidInput( "private volume ownership verification failed".into(), )); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index c7678a9c66..8360f6451c 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -789,10 +789,7 @@ impl PodmanComputeDriver { let options_ok = volume.options.is_empty() || (workspace_volume && numeric_user(&inspect.config.user).is_some_and(|owner| { - crate::client::volume_options_match_owner( - &volume.options, - Some(owner), - ) + volume.options_match_requested_owner(Some(owner)) })); if !owned || volume.driver != "local" || !options_ok { return Err(missing()); From bd04726448ff64279a66cca96e837b4a19f0b516 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Fri, 2 Oct 2026 14:02:59 +0200 Subject: [PATCH 4/4] test(podman): verify workspace access across user namespaces Signed-off-by: Evan Lezar --- .../drivers/podman/tests/default_userns.rs | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/suites/drivers/podman/tests/default_userns.rs b/tests/suites/drivers/podman/tests/default_userns.rs index f9aaf4354c..95cb8c814f 100644 --- a/tests/suites/drivers/podman/tests/default_userns.rs +++ b/tests/suites/drivers/podman/tests/default_userns.rs @@ -16,19 +16,33 @@ const SANDBOX_TIMEOUT: Duration = Duration::from_secs(300); const PODMAN_TEST_INPUT_DIR_ENV: &str = "OPENSHELL_TEST_INPUT_DIR"; const PODMAN_TEST_IMAGE_ENV: &str = "OPENSHELL_PODMAN_TEST_IMAGE"; +const WORKSPACE_AND_UID_MAP_PROBE: &str = r#"set -eu +workload_owner="$(id -u):$(id -g)" +workspace_owner="$(stat -c '%u:%g' /sandbox)" +printf 'workload-owner=%s\nworkspace-owner=%s\n' "$workload_owner" "$workspace_owner" +test "$(id -u)" -ne 0 +test "$workspace_owner" = "$workload_owner" +probe=$(mktemp /sandbox/userns-probe.XXXXXX) +printf 'workspace probe\n' > "$probe" +rm "$probe" +echo podman-userns-workspace-ok +cat /proc/self/uid_map +"#; + /// Verify that the gateway's user-namespace configuration matches Podman's -/// direct behavior for the same profile. +/// direct behavior for the same profile and preserves workspace access. /// /// The test runs a short-lived sandbox command and compares its user-namespace /// mapping with the direct-Podman reference stored at /// `OPENSHELL_TEST_INPUT_DIR/reference-uid-map`. The tmachine pre-test /// playbook creates that reference in the same gateway-user context. This deliberately /// avoids baking a particular Podman mapping into OpenShell's test contract. -/// +/// The workload also verifies that the managed workspace is owned by its +/// non-root UID/GID and that it can create, write, and remove a file there. #[tokio::test] async fn configured_userns_matches_podman_reference() { - let mut runner = OpenShellRunner::from_env("podman-userns") - .expect("candidate openshell CLI is available"); + let mut runner = + OpenShellRunner::from_env("podman-userns").expect("candidate openshell CLI is available"); let result = async { runner.check_gateway_status().await?; assert_podman_gateway(&runner).await?; @@ -59,15 +73,18 @@ async fn configured_userns_matches_podman_reference() { if let Some(image) = workload_image.as_deref() { create_args.extend(["--from", image]); } - create_args.extend(["--no-tty", "--", "cat", "/proc/self/uid_map"]); + create_args.extend(["--no-tty", "--", "sh", "-c", WORKSPACE_AND_UID_MAP_PROBE]); let run = runner - .step("userns/uid-map") - .description("sandbox exposes its UID map") + .step("userns/workspace-and-uid-map") + .description("sandbox can write to its owned workspace and exposes its UID map") .with_timeout(SANDBOX_TIMEOUT) .run(&create_args) .await .map_err(|error| error.to_string())?; run.require_success()?; + if !run.stdout().contains("podman-userns-workspace-ok") { + return Err(run.failure_diagnostic("non-root workload owns and can write to /sandbox")); + } let sandbox_uid_map = normalize_uid_map(run.stdout()).ok_or_else(|| { run.failure_diagnostic("sandbox returns a non-empty UID map") })?;