From 2ab6ea3c18d306a26e5b19ee52dd1982a37aff98 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Thu, 30 Jul 2026 13:19:21 -0700 Subject: [PATCH 1/7] feat(podman): honor OCI image working directories Signed-off-by: Matthew Grossman --- .../skills/debug-openshell-cluster/SKILL.md | 2 + architecture/compute-runtimes.md | 23 +- crates/openshell-core/src/sandbox_env.rs | 7 + crates/openshell-driver-docker/src/lib.rs | 8 + crates/openshell-driver-docker/src/tests.rs | 9 + crates/openshell-driver-podman/README.md | 40 +- crates/openshell-driver-podman/src/client.rs | 128 +++++- .../openshell-driver-podman/src/container.rs | 419 +++++++++++++++++- crates/openshell-driver-podman/src/driver.rs | 307 +++++++++++-- crates/openshell-sandbox/src/lib.rs | 34 +- crates/openshell-sandbox/src/main.rs | 105 ++++- crates/openshell-server/src/compute/mod.rs | 42 ++ .../src/process.rs | 209 ++++++++- .../openshell-supervisor-process/src/ssh.rs | 4 +- docs/reference/sandbox-compute-drivers.mdx | 36 +- e2e/rust/tests/custom_image.rs | 3 + e2e/rust/tests/driver_config_volume.rs | 12 +- e2e/rust/tests/podman_oci_identity.rs | 51 ++- proto/compute_driver.proto | 11 + 19 files changed, 1320 insertions(+), 130 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index cbff462d45..4a9859b366 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -202,6 +202,8 @@ Common findings: - Rootless networking unavailable: inspect Podman network configuration. - Sandbox image missing or pull denied: verify image reference and registry credentials. - Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected. +- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must be an absolute normalized directory with no symlink components; the final policy UID, primary GID, or supplementary groups must already be able to traverse every parent and write and enter the directory. Podman checks the original image in a networkless temporary probe before attaching the workspace volume, so inspect the probe failure in gateway logs. +- If Podman reports probe cleanup or timeout failures, inspect temporary containers with `podman ps -a --filter name=workdir-probe` and gateway logs. The driver force-removes the probe on every normal success or failure path. - Supervisor cannot call back: check callback endpoint and gateway logs. - Gateway exits before becoming healthy with a callback-listener discovery error: inspect `podman info --debug`, the configured Podman network, and the diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 646b6320bd..7ae07a934e 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -185,8 +185,8 @@ The gateway preserves whether each policy process field was omitted. The active driver then supplies one authoritative identity input to the supervisor: - Docker and Podman inspect the final sandbox image, pin container creation to - its immutable image ID, and pass its raw OCI `Config.User`. Docker also - resolves the workspace from OCI `Config.WorkingDir` during that inspection. + its immutable image ID, and pass its raw OCI `Config.User`. They also resolve + the workspace from OCI `Config.WorkingDir` during that inspection. - Kubernetes passes its platform-resolved numeric UID/GID, including OpenShift SCC-derived values. - VM keeps its existing guest identity behavior. @@ -199,7 +199,7 @@ and uses the same privilege-drop path for direct and SSH children. When a declaration omits the group, the supervisor fills it with the user's numeric primary GID. It does not rewrite the account files. -Docker uses an absolute OCI working directory as the workspace. An +Docker and Podman use an absolute OCI working directory as the workspace. An empty, root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which OpenShell creates and owns as a compatibility workspace. Any other workdir must already exist in the immutable image without symlink components. The completed @@ -211,11 +211,18 @@ Path checks reserve the standard OCI runtime namespaces under `/proc`, `/sys`, and `/dev`, while separate collision checks are derived from actual OpenShell control paths. Docker performs the check in the final container before workload launch and -rejects image `VOLUME` declarations that would mask the workdir ancestry. The -resolved workspace is the child cwd and `HOME`; when -`filesystem.include_workdir` is enabled, it becomes the automatic writable -policy path. Podman, Kubernetes/OpenShift, and VM retain their existing -`/sandbox` workspace behavior. +rejects image `VOLUME` declarations that would mask the workdir ancestry. +Podman performs it in a minimal networkless container from the same pinned +image ID before its managed workspace volume covers the path. The probe retains +only the capabilities needed to adopt the completed process identity, drops to +that identity, and asks the kernel to validate access. It emits a normalized +identity attestation that the final supervisor must match, including when the +image supplies the default process policy. The driver captures a bounded, +sanitized diagnostic before removing a failed probe. The resolved workspace is +the child cwd and `HOME`; when `filesystem.include_workdir` is enabled, it +becomes the automatic writable policy path. Kubernetes/OpenShift keep their +`/sandbox` PVC and `fsGroup` behavior, and VM keeps its `/sandbox` guest +initialization path. Sandbox creation fails before the workload becomes ready when a required image identity is absent, malformed, unknown, ambiguous, or resolves to UID/GID 0. diff --git a/crates/openshell-core/src/sandbox_env.rs b/crates/openshell-core/src/sandbox_env.rs index 1549258fa3..22a82e110c 100644 --- a/crates/openshell-core/src/sandbox_env.rs +++ b/crates/openshell-core/src/sandbox_env.rs @@ -126,6 +126,13 @@ pub const SANDBOX_GID: &str = "OPENSHELL_SANDBOX_GID"; /// OCI only for the former contract. pub const OCI_IMAGE_USER: &str = "OPENSHELL_OCI_IMAGE_USER"; +/// Normalized UID/GID/supplementary-group identity attested by the Podman +/// immutable-image workspace probe. +/// +/// A non-empty value also asserts that the +/// original workspace was validated before Podman's managed volume covered it. +pub const OCI_WORKSPACE_IDENTITY: &str = "OPENSHELL_OCI_WORKSPACE_IDENTITY"; + // The corporate upstream-proxy configuration deliberately has no reserved // environment variables: it travels on the supervisor's argv // (`--upstream-proxy` and friends), which a sandbox image cannot forge the diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index dd4d9ef0f0..d1c096e0e9 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2195,6 +2195,7 @@ fn build_environment_for_oci_user( user_env.extend(template.environment.clone()); } user_env.extend(spec.environment.clone()); + user_env.remove(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY); environment.extend(user_env.clone()); if !user_env.is_empty() && let Ok(json) = serde_json::to_string(&user_env) @@ -2250,6 +2251,13 @@ fn build_environment_for_oci_user( environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); + // Docker never uses the Podman prevalidation contract. Emit an explicit + // driver-owned empty value so image-baked ENV entries cannot select the + // managed-workspace mutation path for an image-provided workdir. + environment.insert( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.to_string(), + String::new(), + ); environment.insert( openshell_core::sandbox_env::OCI_IMAGE_USER.to_string(), oci_user.to_string(), diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index ac525c705c..c7b83b0cec 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -45,6 +45,7 @@ fn test_sandbox() -> DriverSandbox { }), resource_requirements: None, sandbox_token: String::new(), + workspace_validation_identity: None, }), status: None, workspace: String::new(), @@ -578,6 +579,10 @@ fn build_environment_protects_oci_identity_metadata() { (openshell_core::sandbox_env::OCI_IMAGE_USER, "spoofed"), (openshell_core::sandbox_env::SANDBOX_UID, "9999"), (openshell_core::sandbox_env::SANDBOX_GID, "9999"), + ( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY, + "9999:9999:", + ), ] { spec.environment.insert(key.to_string(), value.to_string()); } @@ -590,6 +595,10 @@ fn build_environment_protects_oci_identity_metadata() { ))); assert!(env.contains(&format!("{}=", openshell_core::sandbox_env::SANDBOX_UID))); assert!(env.contains(&format!("{}=", openshell_core::sandbox_env::SANDBOX_GID))); + assert!(env.contains(&format!( + "{}=", + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY + ))); assert!(!env.iter().any(|entry| entry.ends_with("=spoofed"))); assert!(!env.iter().any(|entry| entry.ends_with("=9999"))); } diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 965a295d19..4762d92480 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -8,14 +8,30 @@ isolation enforcement to the `openshell-sandbox` supervisor binary, which is sideloaded into each container via an OCI image volume mount. Before creating the container, the driver inspects the final sandbox image and -captures its immutable image ID and raw OCI `Config.User`. Container creation -uses that image ID with pulling disabled, preventing a mutable tag from changing -between inspection and launch. The supervisor runs as root, resolves omitted -policy identity fields from the image declaration, and drops only agent -children to the completed identity. Named OCI components remain names after -validation; a missing group is filled with the user's numeric primary GID. Explicit -`process.run_as_user` and `process.run_as_group` values take precedence -independently. +captures its immutable image ID, raw OCI `Config.User`, and OCI +`Config.WorkingDir`. Container creation uses that image ID with pulling +disabled, preventing a mutable tag from changing between inspection and launch. +The supervisor runs as root, resolves omitted policy identity fields from the +image declaration, and drops only agent children to the completed identity. +Named OCI components remain names after validation; a missing group is filled +with the user's numeric primary GID. Explicit `process.run_as_user` and +`process.run_as_group` values take precedence independently. + +An absolute OCI working directory becomes the agent workspace. An empty, +root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which OpenShell +creates when necessary and owns as a compatibility workspace. For any other workdir, Podman +first starts a minimal, networkless probe from the pinned image ID without the +workspace volume, tokens, or TLS secrets. The probe retains only `SETUID` and +`SETGID`, resolves and adopts the completed identity, including supplementary +groups, and uses the kernel to verify that it can traverse every parent and +write and enter the workdir. The path must be a real directory without symlink +components. The probe also rejects kernel-managed filesystems and overlaps with +concrete OpenShell control resources. It emits a normalized identity +attestation; the final supervisor must resolve to the same identity, including +when the image supplies the default process policy. On failure, the driver +captures a bounded, sanitized diagnostic before removing the probe. Only then +does Podman mount and prepare the managed workspace volume at that path; normal +copy-up preserves image content. The workspace is the child cwd and `HOME`. For a rootless networking deep dive, see [NETWORKING.md](NETWORKING.md). @@ -87,9 +103,11 @@ optional `selinux_label` of `shared` (applies `:z`) or `private` (applies read-only by default; set `read_only: false` to make them writable. Podman image and volume mounts do not support `subpath` in OpenShell driver config. Mount `source` and `target` values must not contain surrounding whitespace. -Mount targets must be absolute container paths and must not replace -the workspace root (`/sandbox`) or overlap OpenShell supervisor files, -`/etc/openshell`, `/etc/openshell-tls`, or `/run/netns`. +Mount targets must be absolute container paths and must not replace the +resolved workspace root or any of its parents. Nested workspace mounts remain +valid. Mounts also must not contain or be contained by concrete OpenShell +control targets such as the supervisor mount, TLS and token files, runtime +socket, or `/run/netns`. Example named-volume usage: diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 9fe39cf7e2..1c5e7e05bf 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -25,6 +25,7 @@ const API_TIMEOUT: Duration = Duration::from_secs(30); /// Maximum allowed size for the event stream line buffer (1 MB). const MAX_EVENT_BUFFER: usize = 1_048_576; +const MAX_CONTAINER_LOG_BYTES: usize = 16 * 1024; #[derive(Debug, thiserror::Error)] pub enum PodmanApiError { @@ -117,6 +118,16 @@ pub struct ContainerState { pub finished_at: Option, } +#[derive(Debug, serde::Deserialize)] +#[serde(untagged)] +enum ContainerWaitResponse { + ExitCode(i64), + Status { + #[serde(rename = "StatusCode")] + status_code: i64, + }, +} + #[derive(Debug, Clone, serde::Deserialize)] #[serde(rename_all = "PascalCase")] pub struct HealthState { @@ -177,6 +188,8 @@ pub struct ImageInspect { pub struct ImageConfig { #[serde(default)] pub user: String, + #[serde(default)] + pub working_dir: String, } /// A container summary returned by the list API. @@ -352,6 +365,43 @@ impl PodmanClient { Ok((status, bytes)) } + /// Send a request while retaining at most `max_bytes` of the response. + async fn send_request_bounded( + &self, + req: Request>, + timeout: Duration, + max_bytes: usize, + ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { + use hyper::body::Body; + + let mut sender = self.connect().await?; + let response = tokio::time::timeout(timeout, sender.send_request(req)) + .await + .map_err(|_| PodmanApiError::Timeout(timeout))? + .map_err(|error| PodmanApiError::Connection(error.to_string()))?; + let status = response.status(); + let deadline = tokio::time::Instant::now() + timeout; + let mut body = response.into_body(); + let mut bytes = Vec::with_capacity(max_bytes.min(4096)); + while bytes.len() < max_bytes { + let frame = tokio::time::timeout_at( + deadline, + std::future::poll_fn(|context| Pin::new(&mut body).poll_frame(context)), + ) + .await + .map_err(|_| PodmanApiError::Timeout(timeout))?; + let Some(frame) = frame else { + break; + }; + let frame = frame.map_err(|error| PodmanApiError::Connection(error.to_string()))?; + if let Some(data) = frame.data_ref() { + let remaining = max_bytes - bytes.len(); + bytes.extend_from_slice(&data[..data.len().min(remaining)]); + } + } + Ok((status, Bytes::from(bytes))) + } + /// Perform a versioned HTTP request and return status + body bytes. async fn request( &self, @@ -457,6 +507,22 @@ impl PodmanClient { .await } + /// Wait for a container to exit and return its exit code. + pub async fn wait_container(&self, name: &str) -> Result { + validate_name(name)?; + let response: ContainerWaitResponse = self + .request_json( + hyper::Method::POST, + &format!("/libpod/containers/{name}/wait?condition=exited"), + None, + ) + .await?; + Ok(match response { + ContainerWaitResponse::ExitCode(code) => code, + ContainerWaitResponse::Status { status_code } => status_code, + }) + } + /// Stop a container with a grace period in seconds. pub async fn stop_container( &self, @@ -524,6 +590,27 @@ impl PodmanClient { .await } + /// Fetch a bounded tail of stdout and stderr from a container. + pub async fn container_logs(&self, name: &str) -> Result { + validate_name(name)?; + let req = Self::build_request( + hyper::Method::GET, + &format!( + "/{API_VERSION}/libpod/containers/{name}/logs?stdout=true&stderr=true&tail=20×tamps=false" + ), + Full::new(Bytes::new()), + None, + ); + let (status, bytes) = self + .send_request_bounded(req, API_TIMEOUT, MAX_CONTAINER_LOG_BYTES) + .await?; + if status.is_success() { + Ok(String::from_utf8_lossy(&bytes).into_owned()) + } else { + Err(error_from_response(status.as_u16(), &bytes)) + } + } + /// List containers matching label filters (e.g. `&["openshell.managed=true"]`). pub async fn list_containers( &self, @@ -973,12 +1060,12 @@ mod tests { } #[tokio::test] - async fn inspect_image_reads_immutable_id_and_oci_user() { + async fn inspect_image_reads_immutable_id_and_oci_config() { let (socket_path, request_log, handle) = spawn_podman_stub( "inspect-image", vec![StubResponse::new( StatusCode::OK, - r#"{"Id":"sha256:immutable","Config":{"User":"app:staff"}}"#, + r#"{"Id":"sha256:immutable","Config":{"User":"app:staff","WorkingDir":"/workspace/project"}}"#, )], ); let client = PodmanClient::new(socket_path.clone()); @@ -993,6 +1080,13 @@ mod tests { image.config.as_ref().map(|config| config.user.as_str()), Some("app:staff") ); + assert_eq!( + image + .config + .as_ref() + .map(|config| config.working_dir.as_str()), + Some("/workspace/project") + ); handle.await.expect("stub task should finish"); assert_eq!( request_log @@ -1055,4 +1149,34 @@ mod tests { handle.await.expect("stub task should finish"); let _ = std::fs::remove_file(socket_path); } + + #[tokio::test] + async fn container_logs_fetches_bounded_stdout_and_stderr_tail() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "container-logs", + vec![StubResponse::new( + StatusCode::OK, + "workspace validation failed\n", + )], + ); + let client = PodmanClient::new(socket_path.clone()); + + let logs = client + .container_logs("workdir-probe") + .await + .expect("container logs should be returned"); + + assert_eq!(logs, "workspace validation failed\n"); + 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/containers/workdir-probe/logs?stdout=true&stderr=true&tail=20×tamps=false" + ] + ); + let _ = std::fs::remove_file(socket_path); + } } diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 005f688a19..4b778bd3af 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -3,6 +3,7 @@ //! Container spec construction for the Podman driver. +use crate::client::ImageInspect; use crate::config::PodmanComputeConfig; use openshell_core::ComputeDriverError; use openshell_core::driver_mounts::SelinuxLabel; @@ -190,6 +191,9 @@ struct ContainerSpec { volumes: Vec, image_volumes: Vec, hostname: String, + /// Start the supervisor independently of the image workspace. The + /// supervisor validates and prepares that workspace before child launch. + work_dir: String, /// Overrides the image's ENTRYPOINT. In Podman's libpod API, `command` /// only overrides CMD (appended as args to the entrypoint). We must set /// `entrypoint` explicitly so the supervisor binary runs directly, @@ -423,6 +427,7 @@ fn build_env( user_env.insert(k.clone(), v.clone()); } } + user_env.remove(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY); env.extend(user_env.clone()); if !user_env.is_empty() && let Ok(json) = serde_json::to_string(&user_env) @@ -483,6 +488,13 @@ fn build_env( env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); env.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); + // The final spec overwrites this only after a successful immutable-image + // probe. An explicit empty value prevents image ENV from forging the + // attestation contract on the /sandbox compatibility path. + env.insert( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.into(), + String::new(), + ); env.insert( openshell_core::sandbox_env::OCI_IMAGE_USER.into(), oci_user.to_string(), @@ -495,7 +507,6 @@ fn build_env( openshell_core::sandbox_env::SANDBOX_GID.into(), String::new(), ); - // 4. Gateway-minted sandbox JWT. Keep the raw bearer out of container // metadata; the supervisor reads it from a driver-owned bind mount. if let Some(s) = spec @@ -612,6 +623,7 @@ pub fn podman_driver_image_mount_sources( fn podman_user_mounts( sandbox: &DriverSandbox, enable_bind_mounts: bool, + workspace_root: &str, ) -> Result { let template = sandbox .spec @@ -623,6 +635,13 @@ fn podman_user_mounts( let config = podman_driver_config(template, enable_bind_mounts)?; let mut result = PodmanUserMounts::default(); for mount in config.mounts { + let target = match &mount { + PodmanDriverMountConfig::Bind { target, .. } + | PodmanDriverMountConfig::Volume { target, .. } + | PodmanDriverMountConfig::Tmpfs { target, .. } + | PodmanDriverMountConfig::Image { target, .. } => target, + }; + driver_mounts::validate_workspace_mount_target(target, workspace_root)?; match mount { PodmanDriverMountConfig::Bind { source, @@ -903,8 +922,11 @@ pub fn build_container_spec_with_token_and_gpu_devices( token_secret_name, gpu_device_ids, image, - image, - "", + &ImageInspect { + id: image.to_string(), + config: None, + }, + None, ) } @@ -914,16 +936,31 @@ pub fn build_container_spec_for_image( token_secret_name: Option<&str>, gpu_device_ids: Option<&[String]>, requested_image: &str, - image_id: &str, - oci_user: &str, + inspected_image: &ImageInspect, + workspace_identity: Option<&str>, ) -> Result { let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); + let oci_user = inspected_image + .config + .as_ref() + .map_or("", |config| config.user.as_str()); + let oci_working_dir = inspected_image + .config + .as_ref() + .map_or("", |config| config.working_dir.as_str()); + let workspace_root = driver_mounts::resolve_oci_workspace_root(oci_working_dir) + .map_err(ComputeDriverError::Precondition)?; + driver_mounts::validate_workspace_control_path( + &workspace_root, + &config.sandbox_ssh_socket_path, + ) + .map_err(ComputeDriverError::Precondition)?; - let env = build_env(sandbox, config, requested_image, oci_user); + let mut env = build_env(sandbox, config, requested_image, oci_user); let labels = build_labels(sandbox); let resource_limits = build_resource_limits(sandbox, config); - let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts) + let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts, &workspace_root) .map_err(ComputeDriverError::InvalidArgument)?; if sandbox .spec @@ -952,7 +989,7 @@ pub fn build_container_spec_for_image( let mut volumes = vec![NamedVolume { name: vol, - dest: "/sandbox".into(), + dest: workspace_root.clone(), options: vec!["rw".into()], }]; volumes.extend(user_mounts.volumes); @@ -963,15 +1000,19 @@ pub fn build_container_spec_for_image( rw: false, }]; image_volumes.extend(user_mounts.image_volumes); - let mut command = vec![ - "--workdir".to_string(), - driver_mounts::DEFAULT_WORKSPACE_ROOT.to_string(), - ]; + + let mut command = vec!["--workdir".to_string(), workspace_root]; command.extend(upstream_proxy_cli_args(config)); + if let Some(identity) = workspace_identity { + env.insert( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.into(), + identity.into(), + ); + } let container_spec = ContainerSpec { name, - image: image_id.to_string(), + image: inspected_image.id.clone(), labels, env, volumes, @@ -982,6 +1023,7 @@ pub fn build_container_spec_for_image( // /openshell-sandbox, so it appears at /opt/openshell/bin/openshell-sandbox. image_volumes, hostname: format!("sandbox-{}", sandbox.name), + work_dir: "/".to_string(), // Override the image's ENTRYPOINT so the supervisor binary runs // directly. Sandbox images (e.g. the community base image) set // ENTRYPOINT ["/bin/bash"], and Podman's `command` field only @@ -989,10 +1031,9 @@ pub fn build_container_spec_for_image( // Without this, the container would run the entrypoint binary with // the supervisor path as an argument instead of executing it directly. entrypoint: vec![SUPERVISOR_BINARY_PATH.into()], - // Keep Podman's existing /sandbox workspace contract explicit while - // the supervisor supports driver-selected workdirs. Operator-owned - // corporate proxy flags follow it; the workload command comes from - // the reserved environment variable. + // Operator-owned corporate proxy flags. The workload command is not + // part of argv (the supervisor takes it from the reserved command + // env var), so these flags are the whole command list. command, // Force the supervisor to run as root (UID 0). Sandbox images may // set a non-root USER directive (e.g. `USER sandbox`), but the @@ -1188,6 +1229,77 @@ pub fn build_container_spec_for_image( Ok(serde_json::to_value(container_spec).expect("ContainerSpec serialization cannot fail")) } +/// Build a minimal one-shot container that validates the image's original +/// workdir before the final Podman workspace volume covers it. +pub fn build_workspace_probe_spec( + sandbox: &DriverSandbox, + config: &PodmanComputeConfig, + inspected_image: &ImageInspect, + probe_name: &str, +) -> Result, ComputeDriverError> { + let image_config = inspected_image.config.as_ref(); + let oci_user = image_config.map_or("", |config| config.user.as_str()); + let oci_working_dir = image_config.map_or("", |config| config.working_dir.as_str()); + let workspace_root = driver_mounts::resolve_oci_workspace_root(oci_working_dir) + .map_err(ComputeDriverError::Precondition)?; + driver_mounts::validate_workspace_control_path( + &workspace_root, + &config.sandbox_ssh_socket_path, + ) + .map_err(ComputeDriverError::Precondition)?; + if workspace_root == driver_mounts::DEFAULT_WORKSPACE_ROOT { + return Ok(None); + } + + let spec = sandbox + .spec + .as_ref() + .ok_or_else(|| ComputeDriverError::Precondition("sandbox.spec is required".into()))?; + let mut command = vec![ + "probe-workspace".to_string(), + "--workdir".to_string(), + workspace_root, + "--oci-user".to_string(), + oci_user.to_string(), + ]; + if let Some(identity) = &spec.workspace_validation_identity { + for (flag, value) in [ + ("--run-as-user", identity.run_as_user.as_str()), + ("--run-as-group", identity.run_as_group.as_str()), + ] { + if !value.is_empty() { + command.push(flag.to_string()); + command.push(value.to_string()); + } + } + if identity.discover_from_image_policy { + command.push("--discover-policy-identity".to_string()); + } + } + + Ok(Some(serde_json::json!({ + "name": probe_name, + "image": inspected_image.id, + "entrypoint": [SUPERVISOR_BINARY_PATH], + "command": command, + "user": "0:0", + "work_dir": "/", + "image_volumes": [{ + "source": config.supervisor_image, + "destination": SUPERVISOR_MOUNT_DIR, + "rw": false + }], + // Do not materialize OCI VOLUME declarations: the probe must inspect + // the immutable image layer rather than a fresh anonymous volume. + "image_volume_mode": "ignore", + "netns": {"nsmode": "none"}, + "no_new_privileges": true, + "cap_drop": ["ALL"], + "cap_add": ["SETUID", "SETGID"], + "image_pull_policy": "never" + }))) +} + fn hostadd_entries(config: &PodmanComputeConfig) -> Vec { let host_gateway_ip = config.host_gateway_ip.trim(); if host_gateway_ip.is_empty() { @@ -1260,6 +1372,7 @@ fn parse_memory_to_bytes(quantity: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::client::ImageConfig; use openshell_core::proto::compute::v1::{GpuResourceRequirements, ResourceRequirements}; static ENV_LOCK: std::sync::LazyLock> = @@ -1279,6 +1392,16 @@ mod tests { } } + fn inspected_image(id: &str, user: &str, working_dir: &str) -> ImageInspect { + ImageInspect { + id: id.to_string(), + config: Some(ImageConfig { + user: user.to_string(), + working_dir: working_dir.to_string(), + }), + } + } + #[test] fn parse_cpu_millicore() { assert_eq!(parse_cpu_to_microseconds("500m"), Some(50_000)); @@ -1373,6 +1496,10 @@ mod tests { (openshell_core::sandbox_env::OCI_IMAGE_USER, "spoofed"), (openshell_core::sandbox_env::SANDBOX_UID, "9999"), (openshell_core::sandbox_env::SANDBOX_GID, "9999"), + ( + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY, + "9999:9999:", + ), ] { spec.environment.insert(key.to_string(), value.to_string()); } @@ -1383,8 +1510,8 @@ mod tests { None, None, "registry.example/app:latest", - "sha256:immutable", - "app:staff", + &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), + Some("1000:1000:"), ) .unwrap(); @@ -1395,6 +1522,18 @@ mod tests { ); assert_eq!(container["user"].as_str(), Some("0:0")); assert_eq!(container["image_pull_policy"].as_str(), Some("never")); + assert_eq!( + container["command"], + serde_json::json!(["--workdir", "/workspace/project"]) + ); + assert_eq!(container["work_dir"].as_str(), Some("/")); + assert!(container["volumes"].as_array().is_some_and(|volumes| { + volumes.iter().any(|volume| { + volume["name"].as_str() == Some("openshell-sandbox-test-id-workspace") + && volume["dest"].as_str() == Some("/workspace/project") + && volume["options"] == serde_json::json!(["rw"]) + }) + })); assert_eq!( container["env"][openshell_core::sandbox_env::OCI_IMAGE_USER].as_str(), Some("app:staff") @@ -1408,11 +1547,249 @@ mod tests { Some("") ); assert_eq!( - container["command"], - serde_json::json!(["--workdir", "/sandbox"]) + container["env"][openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY].as_str(), + Some("1000:1000:") + ); + } + + #[test] + fn compatibility_workspace_emits_empty_attestation() { + let container = build_container_spec_for_image( + &test_sandbox("test-id", "test-name"), + &test_config(), + None, + None, + "registry.example/app:latest", + &inspected_image("sha256:immutable", "app:staff", ""), + None, + ) + .unwrap(); + + assert_eq!( + container["env"][openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY].as_str(), + Some("") + ); + } + + #[test] + fn container_spec_rejects_invalid_oci_working_dir() { + let err = build_container_spec_for_image( + &test_sandbox("test-id", "test-name"), + &test_config(), + None, + None, + "registry.example/app:latest", + &inspected_image("sha256:immutable", "app:staff", "relative/workspace"), + None, + ) + .unwrap_err(); + + assert!( + err.to_string() + .contains("must be an absolute container path") + ); + } + + #[test] + fn container_spec_rejects_openshell_control_path_working_dir() { + let err = build_container_spec_for_image( + &test_sandbox("test-id", "test-name"), + &test_config(), + None, + None, + "registry.example/app:latest", + &inspected_image( + "sha256:immutable", + "app:staff", + "/opt/openshell/bin/project", + ), + None, + ) + .unwrap_err(); + + assert!(err.to_string().contains("OpenShell control path")); + } + + #[test] + fn workspace_probe_uses_pinned_image_without_workspace_or_network() { + let mut sandbox = test_sandbox("test-id", "test-name"); + let spec = sandbox.spec.get_or_insert_default(); + spec.workspace_validation_identity = Some( + openshell_core::proto::compute::v1::WorkspaceValidationIdentity { + run_as_user: "policy-user".into(), + run_as_group: "policy-group".into(), + discover_from_image_policy: false, + }, + ); + let probe = build_workspace_probe_spec( + &sandbox, + &test_config(), + &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), + "openshell-test-probe", + ) + .unwrap() + .unwrap(); + + assert_eq!(probe["image"], "sha256:immutable"); + assert_eq!(probe["netns"]["nsmode"], "none"); + assert_eq!(probe["image_volume_mode"], "ignore"); + assert_eq!(probe["cap_drop"], serde_json::json!(["ALL"])); + assert_eq!(probe["cap_add"], serde_json::json!(["SETUID", "SETGID"])); + assert!(probe.get("volumes").is_none()); + assert!(probe.get("secrets").is_none()); + assert!(probe.get("env").is_none()); + assert_eq!( + probe["command"], + serde_json::json!([ + "probe-workspace", + "--workdir", + "/workspace/project", + "--oci-user", + "app:staff", + "--run-as-user", + "policy-user", + "--run-as-group", + "policy-group" + ]) ); } + #[test] + fn workspace_probe_discovers_image_policy_identity_when_requested() { + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox + .spec + .get_or_insert_default() + .workspace_validation_identity = Some( + openshell_core::proto::compute::v1::WorkspaceValidationIdentity { + discover_from_image_policy: true, + ..Default::default() + }, + ); + + let probe = build_workspace_probe_spec( + &sandbox, + &test_config(), + &inspected_image("sha256:immutable", "app:staff", "/home/app/project"), + "openshell-test-probe", + ) + .unwrap() + .unwrap(); + + assert_eq!( + probe["command"], + serde_json::json!([ + "probe-workspace", + "--workdir", + "/home/app/project", + "--oci-user", + "app:staff", + "--discover-policy-identity" + ]) + ); + } + + #[test] + fn workspace_probe_skips_sandbox_compatibility_fallback() { + let probe = build_workspace_probe_spec( + &test_sandbox("test-id", "test-name"), + &test_config(), + &inspected_image("sha256:immutable", "sandbox:sandbox", "/"), + "openshell-test-probe", + ) + .unwrap(); + assert!(probe.is_none()); + } + + #[test] + fn container_spec_reserves_resolved_workspace_root_but_allows_nested_mounts() { + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(openshell_core::proto::compute::v1::DriverSandboxSpec { + template: Some(DriverSandboxTemplate::default()), + ..Default::default() + }); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "tmpfs", + "target": "/workspace" + }] + }))); + + let err = build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + &inspected_image("sha256:immutable", "app:staff", "/workspace"), + None, + ) + .unwrap_err(); + assert!( + err.to_string() + .contains("reserved for the OpenShell workspace") + ); + + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "tmpfs", + "target": "/workspace" + }] + }))); + let err = build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), + None, + ) + .unwrap_err(); + assert!( + err.to_string() + .contains("reserved for the OpenShell workspace") + ); + + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "tmpfs", + "target": "/workspace/cache" + }] + }))); + build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + &inspected_image("sha256:immutable", "app:staff", "/workspace"), + None, + ) + .expect("nested workspace mounts remain supported"); + } + #[test] fn volume_name_uses_id() { assert_eq!( diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 51c689fb29..00d62cfde6 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -54,6 +54,10 @@ pub struct PodmanComputeDriver { rootless: bool, /// Rootless network helper reported by Podman, such as `pasta`. rootless_network_cmd: String, + /// Serializes container image-volume attachment and removal. Rootless + /// Podman can otherwise detach a shared type=image mount while another + /// sandbox container is starting. + container_lifecycle_lock: Arc>, gpu_selector: Arc, gpu_inventory_refresh: Arc (CdiGpuInventory, bool) + Send + Sync>, } @@ -87,6 +91,15 @@ fn validated_container_name(sandbox: &DriverSandbox) -> Result String { + const SUFFIX: &str = "-workdir-probe"; + let keep = 255usize.saturating_sub(SUFFIX.len()); + format!( + "{}{SUFFIX}", + &container_name[..container_name.len().min(keep)] + ) +} + fn podman_volume_is_bind_backed(volume: &VolumeInspect) -> bool { (volume.driver.is_empty() || volume.driver == "local") && volume.options.get("o").is_some_and(|options| { @@ -222,6 +235,36 @@ fn podman_gpu_selection_error(err: CdiGpuSelectionError) -> ComputeDriverError { ComputeDriverError::Precondition(err.to_string()) } +const WORKSPACE_IDENTITY_MARKER: &str = "OPENSHELL_WORKSPACE_IDENTITY="; + +fn parse_workspace_identity(logs: &str) -> Result { + logs.match_indices(WORKSPACE_IDENTITY_MARKER) + .filter_map(|(start, _)| { + logs[start + WORKSPACE_IDENTITY_MARKER.len()..] + .split(|character: char| character.is_control()) + .next() + }) + .find_map(|identity| serde_json::from_str::(identity).ok()) + .ok_or_else(|| { + ComputeDriverError::Precondition( + "OCI WorkingDir validation probe did not report its process identity".to_string(), + ) + }) +} + +fn sanitized_probe_diagnostic(logs: &str) -> String { + let sanitized = logs + .chars() + .filter(|character| !character.is_control() || matches!(character, '\n' | '\t')) + .collect::(); + let sanitized = sanitized.trim(); + if sanitized.is_empty() { + "no probe diagnostic was emitted".to_string() + } else { + sanitized.chars().take(2048).collect() + } +} + /// Resolve the socket to connect to: explicit configuration wins, otherwise /// fall back to `detect`. Returns an error if neither resolves. /// @@ -243,6 +286,80 @@ fn resolve_socket_path( } impl PodmanComputeDriver { + async fn validate_image_workspace( + &self, + sandbox: &DriverSandbox, + container_name: &str, + inspected_image: &crate::client::ImageInspect, + ) -> Result, ComputeDriverError> { + let probe_name = workspace_probe_name(container_name); + crate::client::validate_name(&probe_name) + .map_err(|error| ComputeDriverError::Precondition(error.to_string()))?; + let Some(spec) = container::build_workspace_probe_spec( + sandbox, + &self.config, + inspected_image, + &probe_name, + )? + else { + return Ok(None); + }; + + let start = { + let _lifecycle_guard = self.container_lifecycle_lock.lock().await; + self.client + .create_container(&spec) + .await + .map_err(ComputeDriverError::from)?; + self.client + .start_container(&probe_name) + .await + .map_err(ComputeDriverError::from) + }; + let validation = async { + start?; + let exit_code = self + .client + .wait_container(&probe_name) + .await + .map_err(ComputeDriverError::from)?; + let logs = self.client.container_logs(&probe_name).await; + if exit_code == 0 { + let logs = logs.map_err(ComputeDriverError::from)?; + parse_workspace_identity(&logs).map(Some) + } else { + let diagnostic = logs.map_or_else( + |error| format!("unable to read probe diagnostic: {error}"), + |logs| sanitized_probe_diagnostic(&logs), + ); + Err(ComputeDriverError::Precondition(format!( + "OCI WorkingDir validation failed for image '{}' (probe exited with code {exit_code}): {diagnostic}", + inspected_image.id, + ))) + } + } + .await; + let cleanup = { + let _lifecycle_guard = self.container_lifecycle_lock.lock().await; + self.client + .remove_container(&probe_name, self.config.stop_timeout_secs) + .await + }; + match (validation, cleanup) { + (Ok(identity), Ok(())) => Ok(identity), + (Ok(_), Err(error)) => Err(ComputeDriverError::from(error)), + (Err(error), Ok(())) => Err(error), + (Err(error), Err(cleanup_error)) => { + warn!( + probe = %probe_name, + %cleanup_error, + "Failed to remove workspace validation probe" + ); + Err(error) + } + } + } + /// Create a new driver, verifying the Podman socket is reachable. pub async fn new(mut config: PodmanComputeConfig) -> Result { const MAX_PING_RETRIES: u32 = 5; @@ -391,6 +508,7 @@ impl PodmanComputeDriver { network_gateway_ip, rootless, rootless_network_cmd, + container_lifecycle_lock: Arc::new(tokio::sync::Mutex::new(())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -692,11 +810,9 @@ impl PodmanComputeDriver { "podman image '{image}' inspection did not return an immutable image ID" ))); } - let image_user = inspected_image - .config - .as_ref() - .map_or("", |config| config.user.as_str()); - + let workspace_identity = self + .validate_image_workspace(sandbox, &name, &inspected_image) + .await?; for image in container::podman_driver_image_mount_sources(sandbox, self.config.enable_bind_mounts) .map_err(ComputeDriverError::Precondition)? @@ -761,8 +877,8 @@ impl PodmanComputeDriver { token_secret_name.as_deref(), gpu_devices.as_deref(), image, - &inspected_image.id, - image_user, + &inspected_image, + workspace_identity.as_deref(), ) { Ok(spec) => spec, Err(e) => { @@ -770,8 +886,31 @@ impl PodmanComputeDriver { return Err(e); } }; - match self.client.create_container(&spec).await { - Ok(_) => {} + // Podman implements the supervisor as a shared type=image mount. + // Keep attach/start atomic with respect to another sandbox's removal. + let lifecycle_result = { + let _lifecycle_guard = self.container_lifecycle_lock.lock().await; + match self.client.create_container(&spec).await { + Ok(_) => match self.client.start_container(&name).await { + Ok(()) => Ok(()), + Err(e) => { + warn!( + sandbox_name = %sandbox.name, + error = %e, + "Failed to start container; cleaning up" + ); + let _ = self + .client + .remove_container(&name, self.config.stop_timeout_secs) + .await; + Err(e) + } + }, + Err(e) => Err(e), + } + }; + match lifecycle_result { + Ok(()) => {} Err(PodmanApiError::Conflict(_)) => { // Clean up the volume we just created. It is keyed by *this* // sandbox's ID, not the conflicting container's ID (which @@ -786,21 +925,6 @@ impl PodmanComputeDriver { } } - // 5. Start container. - if let Err(e) = self.client.start_container(&name).await { - warn!( - sandbox_name = %sandbox.name, - error = %e, - "Failed to start container; cleaning up" - ); - let _ = self - .client - .remove_container(&name, self.config.stop_timeout_secs) - .await; - cleanup_created().await; - return Err(ComputeDriverError::from(e)); - } - info!( sandbox_id = %sandbox.id, sandbox_name = %sandbox.name, @@ -862,17 +986,20 @@ impl PodmanComputeDriver { }; info!(sandbox_id = %sandbox_id, container = %container_id, "Deleting sandbox container"); - // Keep stop, timeout, and removal in one Podman operation. Splitting - // stop and remove can race with another container starting an image - // mount when the stop reaches its timeout. - let container_existed = match self - .client - .remove_container(&container_id, self.config.stop_timeout_secs) - .await - { - Ok(()) => true, - Err(PodmanApiError::NotFound(_)) => false, - Err(e) => return Err(ComputeDriverError::from(e)), + // Keep stop, timeout, and removal in one Podman operation. Removing a + // container detaches its type=image mounts, so do not overlap it with + // another sandbox container's create/start window. + let container_existed = { + let _lifecycle_guard = self.container_lifecycle_lock.lock().await; + match self + .client + .remove_container(&container_id, self.config.stop_timeout_secs) + .await + { + Ok(()) => true, + Err(PodmanApiError::NotFound(_)) => false, + Err(e) => return Err(ComputeDriverError::from(e)), + } }; // Remove workspace volume. @@ -1009,6 +1136,7 @@ impl PodmanComputeDriver { network_gateway_ip: None, rootless: false, rootless_network_cmd: String::new(), + container_lifecycle_lock: Arc::new(tokio::sync::Mutex::new(())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -2058,6 +2186,90 @@ mod tests { } } + #[tokio::test] + async fn workspace_probe_waits_for_success_and_always_removes_container() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-success", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":0}"#), + StubResponse::new( + StatusCode::OK, + "OPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:\"\n", + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let mut sandbox = plain_sandbox("sandbox-probe", "demo"); + sandbox.spec = Some(DriverSandboxSpec::default()); + let image = crate::client::ImageInspect { + id: "sha256:immutable".into(), + config: Some(crate::client::ImageConfig { + user: "1234:1235".into(), + working_dir: "/workspace".into(), + }), + }; + let name = validated_container_name(&sandbox).unwrap(); + + let identity = driver + .validate_image_workspace(&sandbox, &name, &image) + .await + .expect("successful probe should pass"); + assert_eq!(identity.as_deref(), Some("1234:1235:")); + + handle.await.expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert_eq!(requests.len(), 5); + assert!(requests[0].contains("/libpod/containers/create")); + assert!(requests[1].contains("/start")); + assert!(requests[2].contains("/wait?condition=exited")); + assert!(requests[3].contains("/logs?")); + assert!(requests[4].starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + + #[tokio::test] + async fn workspace_probe_removes_container_after_validation_failure() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-failure", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":1}"#), + StubResponse::new( + StatusCode::OK, + "image workspace path component '/workspace' is not writable and traversable\n", + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let mut sandbox = plain_sandbox("sandbox-probe-fail", "demo"); + sandbox.spec = Some(DriverSandboxSpec::default()); + let image = crate::client::ImageInspect { + id: "sha256:immutable".into(), + config: Some(crate::client::ImageConfig { + user: "1234:1235".into(), + working_dir: "/workspace".into(), + }), + }; + let name = validated_container_name(&sandbox).unwrap(); + + let error = driver + .validate_image_workspace(&sandbox, &name, &image) + .await + .unwrap_err(); + assert!(error.to_string().contains("exited with code 1")); + assert!(error.to_string().contains("not writable and traversable")); + + handle.await.expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests.last().unwrap().starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + fn secret_delete_request(sandbox_id: &str) -> String { format!( "DELETE {}", @@ -2245,4 +2457,29 @@ mod tests { ); let _ = fs::remove_file(socket_path); } + + #[test] + fn workspace_identity_parser_accepts_plain_and_multiplexed_logs() { + assert_eq!( + parse_workspace_identity( + "probe startup\nOPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:7,8\"\n" + ) + .unwrap(), + "1234:1235:7,8" + ); + assert_eq!( + parse_workspace_identity( + "\u{1}\0\0\0\0\0\0.OPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:\"\n" + ) + .unwrap(), + "1234:1235:" + ); + assert!( + parse_workspace_identity( + "OPENSHELL_WORKSPACE_IDENTITY=not-json\n\ + OPENSHELL_WORKSPACE_IDENTITY={\"uid\":1234}\n" + ) + .is_err() + ); + } } diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 956fed927c..39fa050b27 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -189,8 +189,8 @@ pub async fn run_sandbox( // Normalize the active driver's identity contract once, while both the // policy and launched image filesystem are available. Kubernetes and - // OpenShift retain their authoritative numeric pair; Docker fills only - // omitted policy fields from OCI Config.User. + // OpenShift retain their authoritative numeric pair; Docker and Podman + // fill only omitted policy fields from OCI Config.User. #[cfg(unix)] let (resolved_process_identity, workspace) = { let driver_identity = openshell_supervisor_process::identity::DriverIdentity::from_env()?; @@ -202,6 +202,24 @@ pub async fn run_sandbox( &mut policy, &driver_identity, )?; + if matches!( + &driver_identity, + openshell_supervisor_process::identity::DriverIdentity::OciUser { .. } + ) && std::env::var_os(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) + .is_some_and(|value| !value.is_empty()) + { + let expected = std::env::var(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) + .map_err(|_| miette::miette!("Podman workspace identity attestation is missing"))?; + let actual = + openshell_supervisor_process::process::resolved_workspace_identity_attestation( + &policy, resolved, + )?; + if expected != actual { + return Err(miette::miette!( + "process identity changed after OCI workspace validation" + )); + } + } ( resolved, openshell_supervisor_process::process::ResolvedWorkspace::new( @@ -2156,6 +2174,18 @@ fn discover_policy_from_disk_or_default() -> openshell_core::proto::SandboxPolic discover_policy_from_path(primary) } +/// Discover only the process identity portion of the immutable image policy. +/// +/// The Podman workspace probe uses this when no policy existed at create time, +/// matching the final supervisor's disk-policy discovery before the gateway +/// backfills that policy. +pub fn discover_process_policy_from_disk_or_default() -> openshell_core::policy::ProcessPolicy { + discover_policy_from_disk_or_default() + .process + .map(Into::into) + .unwrap_or_default() +} + /// Try to read a sandbox policy YAML from `path`, falling back to the /// hardcoded restrictive default if the file is missing or invalid. fn discover_policy_from_path(path: &std::path::Path) -> openshell_core::proto::SandboxPolicy { diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 98af7f9ea9..8297c8145e 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -33,6 +33,7 @@ const COPY_SELF_SUBCOMMAND: &str = "copy-self"; /// to confirm the cross-sandbox IDOR guard fires. const DEBUG_RPC_SUBCOMMAND: &str = "debug-rpc"; const VALIDATE_WORKSPACE_SUBCOMMAND: &str = "validate-workspace"; +const PROBE_WORKSPACE_SUBCOMMAND: &str = "probe-workspace"; /// Default `--mode` value: run both supervisor leaves in a single binary. const DEFAULT_MODE: &str = "network,process"; @@ -272,7 +273,76 @@ fn validate_workspace(args: &[String]) -> Result<()> { #[cfg(not(target_os = "linux"))] fn validate_workspace(_args: &[String]) -> Result<()> { Err(miette::miette!( - "workspace validation is only supported on Unix" + "workspace validation is only supported on Linux" + )) +} + +/// Internal one-shot command used by Podman to validate the immutable image +/// before the final workspace volume covers its OCI workdir. +#[derive(Parser, Debug)] +#[command(name = "probe-workspace", hide = true)] +struct ProbeWorkspaceArgs { + #[arg(long)] + workdir: String, + #[arg(long)] + oci_user: String, + #[arg(long)] + run_as_user: Option, + #[arg(long)] + run_as_group: Option, + #[arg(long)] + discover_policy_identity: bool, +} + +#[cfg(unix)] +fn probe_workspace(args: &[String]) -> Result<()> { + use openshell_core::policy::{ + FilesystemPolicy, LandlockPolicy, NetworkPolicy, ProcessPolicy, SandboxPolicy, + }; + use openshell_supervisor_process::identity::{DriverIdentity, resolve_process_identity}; + + let args = ProbeWorkspaceArgs::try_parse_from( + std::iter::once(PROBE_WORKSPACE_SUBCOMMAND.to_string()).chain(args.iter().cloned()), + ) + .into_diagnostic()?; + let process = if args.discover_policy_identity { + openshell_sandbox::discover_process_policy_from_disk_or_default() + } else { + ProcessPolicy { + run_as_user: args.run_as_user, + run_as_group: args.run_as_group, + } + }; + let mut policy = SandboxPolicy { + version: 0, + filesystem: FilesystemPolicy::default(), + network: NetworkPolicy::default(), + landlock: LandlockPolicy::default(), + process, + }; + let resolved_identity = resolve_process_identity( + &mut policy, + &DriverIdentity::OciUser { + declaration: args.oci_user, + }, + )?; + let identity = + openshell_supervisor_process::process::validate_oci_workspace_as_process_identity( + &policy, + resolved_identity, + Path::new(&args.workdir), + )?; + println!( + "OPENSHELL_WORKSPACE_IDENTITY={}", + serde_json::to_string(&identity).into_diagnostic()? + ); + Ok(()) +} + +#[cfg(not(unix))] +fn probe_workspace(_args: &[String]) -> Result<()> { + Err(miette::miette!( + "workspace probing is only supported on Unix" )) } @@ -527,6 +597,9 @@ fn main() -> Result<()> { if raw_args.get(1).map(String::as_str) == Some(VALIDATE_WORKSPACE_SUBCOMMAND) { return validate_workspace(&raw_args[2..]); } + if raw_args.get(1).map(String::as_str) == Some(PROBE_WORKSPACE_SUBCOMMAND) { + return probe_workspace(&raw_args[2..]); + } let args = Args::parse(); @@ -721,6 +794,36 @@ mod tests { validate_workspace(&args).expect("current identity should retain workspace authority"); } + #[cfg(unix)] + #[test] + fn workspace_probe_subcommand_resolves_final_policy_identity() { + let uid = nix::unistd::geteuid().as_raw(); + let gid = nix::unistd::getegid().as_raw(); + if uid < 1000 || gid < 1000 { + // Process policies intentionally reject system identities. Local + // macOS users commonly have IDs below 1000, so this test cannot + // exercise the credential transition for those accounts. + return; + } + let dir = tempfile::tempdir_in("/tmp").unwrap(); + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o711)).unwrap(); + let root = dir.path().canonicalize().unwrap().join("workspace"); + std::fs::create_dir(&root).unwrap(); + std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o777)).unwrap(); + let args = vec![ + "--workdir".to_string(), + root.display().to_string(), + "--oci-user".to_string(), + "unused".to_string(), + "--run-as-user".to_string(), + uid.to_string(), + "--run-as-group".to_string(), + gid.to_string(), + ]; + + probe_workspace(&args).expect("current identity should retain workspace authority"); + } + /// Drives `copy_self`'s file-copy logic against an arbitrary source path /// so tests don't depend on `current_exe()`. fn copy_executable(src: &Path, dest: &Path) -> Result<()> { diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index a1c33e49ff..b2d70be671 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -2515,6 +2515,17 @@ fn driver_sandbox_spec_from_public( spec: &SandboxSpec, driver_name: &str, ) -> Result> { + let workspace_validation_identity = matches!(driver_name, "docker" | "podman").then(|| { + let process = spec + .policy + .as_ref() + .and_then(|policy| policy.process.as_ref()); + openshell_core::proto::compute::v1::WorkspaceValidationIdentity { + run_as_user: process.map_or_else(String::new, |process| process.run_as_user.clone()), + run_as_group: process.map_or_else(String::new, |process| process.run_as_group.clone()), + discover_from_image_policy: spec.policy.is_none(), + } + }); Ok(DriverSandboxSpec { log_level: spec.log_level.clone(), environment: spec.environment.clone(), @@ -2532,6 +2543,7 @@ fn driver_sandbox_spec_from_public( } }), sandbox_token: String::new(), + workspace_validation_identity, }) } @@ -3283,6 +3295,36 @@ mod tests { assert_eq!(gpu.count, Some(2)); } + #[test] + fn driver_sandbox_spec_projects_process_identity_for_local_image_probe() { + let public = SandboxSpec { + policy: Some(openshell_core::proto::SandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "app".into(), + run_as_group: "staff".into(), + }), + ..Default::default() + }), + ..Default::default() + }; + + let driver = + driver_sandbox_spec_from_public(&public, "podman").expect("driver spec should map"); + let identity = driver.workspace_validation_identity.unwrap(); + assert_eq!(identity.run_as_user, "app"); + assert_eq!(identity.run_as_group, "staff"); + assert!(!identity.discover_from_image_policy); + + let without_policy = + driver_sandbox_spec_from_public(&SandboxSpec::default(), "podman").unwrap(); + assert!( + without_policy + .workspace_validation_identity + .unwrap() + .discover_from_image_policy + ); + } + #[test] fn select_driver_config_forwards_only_matching_driver_block() { let config = prost_types::Struct { diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 659fe3dc06..ce6c1043ef 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -142,6 +142,7 @@ pub(crate) fn prepare_child_sandbox( const SUPERVISOR_ONLY_ENV_VARS: &[&str] = &[ openshell_core::sandbox_env::OCI_IMAGE_USER, + openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY, openshell_core::sandbox_env::SANDBOX_UID, openshell_core::sandbox_env::SANDBOX_GID, openshell_core::sandbox_env::SANDBOX_TOKEN, @@ -1394,6 +1395,101 @@ fn validate_oci_workspace_in_subprocess( )) } +/// Drop a one-shot workspace probe to the completed sandbox identity, validate +/// using the kernel's real path-access checks, and return a normalized identity +/// attestation for the final supervisor. +#[cfg(unix)] +pub fn validate_oci_workspace_as_process_identity( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, + workdir: &Path, +) -> Result { + validate_sandbox_user_with_identity(policy, resolved_identity)?; + validate_sandbox_group_with_identity(policy, resolved_identity)?; + let (uid, gid, mut supplementary_gids) = + resolve_filesystem_identity(policy, resolved_identity)?; + let uid = uid.ok_or_else(|| miette::miette!("workspace probe UID is unresolved"))?; + let gid = gid.ok_or_else(|| miette::miette!("workspace probe GID is unresolved"))?; + supplementary_gids.sort_unstable_by_key(|group| group.as_raw()); + supplementary_gids.dedup(); + + if nix::unistd::geteuid().is_root() { + #[cfg(not(any( + target_os = "macos", + target_os = "ios", + target_os = "haiku", + target_os = "redox" + )))] + nix::unistd::setgroups(&supplementary_gids).into_diagnostic()?; + nix::unistd::setgid(gid).into_diagnostic()?; + nix::unistd::setuid(uid).into_diagnostic()?; + } + + let effective_identity = (nix::unistd::geteuid(), nix::unistd::getegid()); + if effective_identity != (uid, gid) { + return Err(miette::miette!( + "workspace probe privilege drop failed: expected {uid}:{gid}, got {}:{}", + effective_identity.0, + effective_identity.1 + )); + } + #[cfg(target_os = "linux")] + validate_oci_workspace_as_effective_identity(workdir)?; + #[cfg(not(target_os = "linux"))] + validate_oci_workspace( + workdir, + Some(effective_identity.0), + Some(effective_identity.1), + &supplementary_gids, + )?; + Ok(workspace_identity_attestation( + Some(uid), + Some(gid), + &supplementary_gids, + )) +} + +/// Normalize a completed process identity for comparison with the immutable +/// image probe. +#[cfg(unix)] +pub fn workspace_identity_attestation( + uid: Option, + gid: Option, + supplementary_gids: &[Gid], +) -> String { + let mut groups = supplementary_gids + .iter() + .map(|group| group.as_raw()) + .collect::>(); + groups.sort_unstable(); + groups.dedup(); + format!( + "{}:{}:{}", + uid.map_or_else(String::new, |value| value.as_raw().to_string()), + gid.map_or_else(String::new, |value| value.as_raw().to_string()), + groups + .iter() + .map(u32::to_string) + .collect::>() + .join(",") + ) +} + +/// Resolve and normalize the completed policy/driver identity without changing +/// process credentials. +#[cfg(unix)] +pub fn resolved_workspace_identity_attestation( + policy: &SandboxPolicy, + resolved_identity: ResolvedProcessIdentity, +) -> Result { + let (uid, gid, supplementary_gids) = resolve_filesystem_identity(policy, resolved_identity)?; + Ok(workspace_identity_attestation( + uid, + gid, + &supplementary_gids, + )) +} + #[cfg(unix)] fn validate_workspace_component( path: &Path, @@ -1427,6 +1523,7 @@ fn validate_workspace_component( path.display() )); } + reject_special_workspace_filesystem(path)?; let required = if is_workspace { 0o3 } else { 0o1 }; if !identity_has_permissions(&metadata, uid, gid, supplementary_gids, required) { let requirement = if is_workspace { @@ -1450,6 +1547,7 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { let open_flags = OFlags::PATH | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; let mut current_path = PathBuf::from("/"); let mut current_fd = rustix::fs::open("/", open_flags, Mode::empty()).into_diagnostic()?; + reject_special_workspace_filesystem_fd(¤t_fd, ¤t_path)?; rustix::fs::accessat( ¤t_fd, ".", @@ -1464,6 +1562,7 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { })?; let last_component = components.len().saturating_sub(1); + for (index, component) in components.into_iter().enumerate() { current_path.push(&component); let stat = rustix::fs::statat(¤t_fd, &component, AtFlags::SYMLINK_NOFOLLOW).map_err( @@ -1509,22 +1608,71 @@ pub fn validate_oci_workspace_as_effective_identity(root: &Path) -> Result<()> { ) })?; - let next_fd = rustix::fs::openat(¤t_fd, &component, open_flags, Mode::empty()) + current_fd = rustix::fs::openat(¤t_fd, &component, open_flags, Mode::empty()) .map_err(|error| { miette::miette!( "failed to open image workspace path component '{}': {error}", current_path.display() ) })?; + reject_special_workspace_filesystem_fd(¤t_fd, ¤t_path)?; if is_workspace { - validate_effective_workspace_write(&next_fd, ¤t_path)?; + validate_effective_workspace_write(¤t_fd, ¤t_path)?; } - current_fd = next_fd; } Ok(()) } +#[cfg(target_os = "linux")] +fn reject_special_workspace_filesystem_fd(fd: &impl std::os::fd::AsFd, path: &Path) -> Result<()> { + let fs = rustix::fs::fstatfs(fd).into_diagnostic()?; + #[allow(clippy::cast_sign_loss)] + reject_special_workspace_filesystem_type(path, fs.f_type as u64) +} + +#[cfg(target_os = "linux")] +fn reject_special_workspace_filesystem(path: &Path) -> Result<()> { + // Linux filesystem magic values for virtual/kernel-managed filesystems. + // The decision is based on the mounted filesystem, not its conventional + // distro path, so renamed or unusually mounted control filesystems remain + // protected without rejecting ordinary application directories. + let fs = rustix::fs::statfs(path).into_diagnostic()?; + #[allow(clippy::cast_sign_loss)] + let filesystem_type = fs.f_type as u64; + reject_special_workspace_filesystem_type(path, filesystem_type) +} + +#[cfg(target_os = "linux")] +fn reject_special_workspace_filesystem_type(path: &Path, filesystem_type: u64) -> Result<()> { + const SPECIAL_FILESYSTEMS: &[u64] = &[ + 0x0000_1cd1, // devpts + 0x0000_9fa0, // proc + 0x0102_1994, // tmpfs (including the container /dev tree) + 0x1980_0202, // mqueue + 0x0027_e0eb, // cgroup + 0x4249_4e4d, // bpf + 0x6265_6572, // sysfs + 0x6367_7270, // cgroup2 + 0x6462_6720, // debugfs + 0x7363_6673, // securityfs + 0x7472_6163, // tracefs + ]; + if SPECIAL_FILESYSTEMS.contains(&filesystem_type) { + return Err(miette::miette!( + "workspace path component '{}' is on a kernel-managed filesystem (type {filesystem_type:#x})", + path.display() + )); + } + Ok(()) +} + +#[cfg(not(target_os = "linux"))] +#[allow(clippy::unnecessary_wraps)] +fn reject_special_workspace_filesystem(_path: &Path) -> Result<()> { + Ok(()) +} + #[cfg(target_os = "linux")] fn validate_effective_workspace_write(fd: &impl std::os::fd::AsFd, path: &Path) -> Result<()> { use rustix::fs::{AtFlags, Mode, OFlags}; @@ -1831,17 +1979,20 @@ pub fn prepare_filesystem_with_identity( let (uid, gid, supplementary_gids) = resolve_filesystem_identity(policy, resolved_identity)?; - // Docker owns workspace resolution and must make the selected root usable - // by the final effective identity, including when both policy identity - // fields were explicit. Validate it before processing any user-authored - // read-write paths so an unsafe image path fails first. Other drivers - // retain their preparation. + // Docker and Podman own workspace resolution and must make the selected + // root usable by the final effective identity, including when both policy + // identity fields were explicit. Validate it before processing any + // user-authored read-write paths so an unsafe image path fails first. + // Other drivers retain their preparation. if prepare_workspace { let workspace = workdir.ok_or_else(|| { miette::miette!("local container driver did not supply a workspace workdir") })?; let workspace = Path::new(workspace); - if workspace == Path::new(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT) { + if workspace == Path::new(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT) + || std::env::var_os(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) + .is_some_and(|value| !value.is_empty()) + { info!(path = %workspace.display(), ?uid, ?gid, "Preparing managed workspace"); prepare_oci_workspace(workspace, uid, gid, &supplementary_gids)?; } else { @@ -1867,8 +2018,8 @@ pub fn prepare_filesystem_with_identity( } // Retain the existing Kubernetes/OpenShift behavior for driver-injected - // numeric identities. Docker clears this variable and does not receive - // identity-specific workspace preparation. + // numeric identities. Docker and Podman clear this variable and do not + // receive identity-specific workspace preparation. if std::env::var(openshell_core::sandbox_env::SANDBOX_UID).is_ok_and(|uid| !uid.is_empty()) { let sandbox_home = Path::new("/sandbox"); if sandbox_home.exists() { @@ -3008,6 +3159,23 @@ mod tests { .expect("supplementary group already has write and traverse authority"); } + #[cfg(unix)] + #[test] + fn workspace_identity_attestation_sorts_and_deduplicates_groups() { + assert_eq!( + workspace_identity_attestation( + Some(Uid::from_raw(1000)), + Some(Gid::from_raw(1001)), + &[ + Gid::from_raw(1003), + Gid::from_raw(1002), + Gid::from_raw(1003), + ], + ), + "1000:1001:1002,1003" + ); + } + #[cfg(unix)] #[test] fn validate_oci_workspace_rejects_unwritable_directory() { @@ -3157,6 +3325,25 @@ mod tests { } } + #[cfg(target_os = "linux")] + #[test] + fn effective_identity_validation_uses_kernel_access_checks() { + if nix::unistd::geteuid().is_root() { + return; + } + + let dir = tempfile::tempdir_in("/tmp").unwrap(); + let root = dir.path().canonicalize().unwrap().join("project"); + std::fs::create_dir(&root).unwrap(); + std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o700)).unwrap(); + validate_oci_workspace_as_effective_identity(&root) + .expect("current identity can write and enter its directory"); + + std::fs::set_permissions(&root, std::fs::Permissions::from_mode(0o500)).unwrap(); + let error = validate_oci_workspace_as_effective_identity(&root).unwrap_err(); + assert!(error.to_string().contains("not writable")); + } + #[cfg(unix)] #[test] fn validate_oci_workspace_rejects_restrictive_parent() { diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index cb9115d9ea..df7c5ca03b 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -699,8 +699,8 @@ impl Default for PtyRequest { /// (or defaults to `/home/{user}`). /// /// For numeric UIDs, there is no passwd entry, so the default remains -/// `("{uid}", "/sandbox")`. Docker replaces that default with its resolved -/// image workspace. +/// `("{uid}", "/sandbox")`. Docker and Podman replace that default with their +/// resolved image workspace. fn session_user_and_home(policy: &SandboxPolicy, workdir_home: Option<&str>) -> (String, String) { let (user, default_home) = match policy.process.run_as_user.as_deref() { Some(user) if !user.is_empty() => { diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 2132f3360e..593fdee8cd 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -251,9 +251,10 @@ Podman mount schema: Podman `volume` and `image` mounts do not support `subpath` in OpenShell driver config, and OpenShell rejects `subpath` for those mount types. OpenShell rejects mount `source` and `target` values with surrounding whitespace. OpenShell also -rejects mount targets that replace the workspace root, container root, supervisor -files, `/etc/openshell`, `/etc/openshell-tls`, authentication material, or -network namespace paths. These checks do not make host bind mounts safe. +rejects mount targets that replace or contain the workspace root, target the +container root, or contain or are contained by concrete OpenShell control +targets such as the supervisor mount, TLS and token files, runtime socket, or +network namespace mount. These checks do not make host bind mounts safe. ## MicroVM Driver @@ -444,7 +445,7 @@ declared name or numeric components for both direct and SSH children. When `USER` omits the group, the supervisor uses the user's numeric primary GID. It does not modify `/etc/passwd` or `/etc/group`. -Docker also inspects OCI `WorkingDir`. An absolute value becomes the +Docker and Podman also inspect OCI `WorkingDir`. An absolute value becomes the agent workspace; an empty, root (`/`), or explicit `/sandbox` value uses the managed `/sandbox` compatibility workspace. OpenShell creates and owns that compatibility workspace. Any other workdir must @@ -457,10 +458,17 @@ workdirs that overlap the OCI runtime namespaces under `/proc`, `/sys`, or `/dev`, and rejects overlap with actual OpenShell control paths. Docker checks the original image filesystem in the final supervisor and rejects image `VOLUME` declarations that would mask the workdir or one of its parents before -validation. The resolved workspace is the cwd and `HOME` for direct and SSH -children. The supervisor itself starts from `/`, so a missing or invalid -workspace is handled during readiness instead of preventing the container -runtime from starting it. +validation. Podman checks the same +pinned image ID in a temporary networkless probe without the workspace volume, +token, or TLS secrets. The probe retains only `SETUID` and `SETGID`, drops to +the completed process identity, and uses the kernel to validate access. The +final supervisor must match the probe's normalized identity attestation, +including when the image supplies the default process policy. Podman captures +a bounded, sanitized failure diagnostic, removes the probe, and only then +creates the final volume-backed sandbox. The resolved workspace is the cwd and +`HOME` for direct and SSH children. The supervisor itself starts from `/`, so a +missing or invalid workspace is handled during readiness instead of preventing +the container runtime from starting it. Sandbox creation fails before readiness if a required `USER` component is missing, malformed, unknown, ambiguous, or resolves to UID/GID 0. An image @@ -491,9 +499,9 @@ The VM driver injects the sandbox UID into the rootfs guest's `/etc/passwd`, `/e Docker and Podman custom images do not need a baked-in `"sandbox"` user. Declare a non-root OCI `USER`, or set both process identity fields explicitly in policy. Named image users require matching account entries; a numeric `UID:GID` pair -does not. For Docker, declare an absolute OCI `WORKDIR` to select the workspace. -Images with no working directory, `WORKDIR /`, or `WORKDIR /sandbox` use -OpenShell's managed `/sandbox` compatibility workspace. For any other Docker -path, create the directory in the image and grant the final process identity -write and execute permission in the Dockerfile. Podman, Kubernetes/OpenShift, -and VM sandboxes continue to use `/sandbox`. +does not. Declare an absolute OCI `WORKDIR` to select the workspace. Images with +no working directory, `WORKDIR /`, or `WORKDIR /sandbox` use OpenShell's +managed `/sandbox` compatibility workspace. +For any other path, create the directory in the image and grant the final +process identity write and execute permission in the Dockerfile. +Kubernetes/OpenShift and VM sandboxes continue to use `/sandbox`. diff --git a/e2e/rust/tests/custom_image.rs b/e2e/rust/tests/custom_image.rs index 5652a0011e..7b2a3bd759 100644 --- a/e2e/rust/tests/custom_image.rs +++ b/e2e/rust/tests/custom_image.rs @@ -56,6 +56,9 @@ RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ && useradd -m -u 3234 -g appstaff app WORKDIR /workspace/project +# Image metadata must not be able to forge the driver's successful-validation +# attestation. +ENV OPENSHELL_OCI_WORKSPACE_IDENTITY=3234:3235: USER app CMD ["sleep", "infinity"] "#; diff --git a/e2e/rust/tests/driver_config_volume.rs b/e2e/rust/tests/driver_config_volume.rs index 0702a4637d..2a91f9ce1d 100644 --- a/e2e/rust/tests/driver_config_volume.rs +++ b/e2e/rust/tests/driver_config_volume.rs @@ -25,9 +25,9 @@ use serde_json::{Map, Value}; const TEST_IMAGE: &str = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest"; const VOLUME_TARGET: &str = "/sandbox/e2e-volume"; const BIND_TARGET: &str = "/sandbox/e2e-bind"; -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] const OCI_VOLUME_TARGET: &str = "/workspace/project/e2e-volume"; -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] const OCI_USER_DOCKERFILE: &str = r#"FROM public.ecr.aws/docker/library/python:3.13-slim RUN apt-get update && apt-get install -y --no-install-recommends iproute2 \ @@ -169,12 +169,12 @@ async fn sandbox_mounts_existing_driver_config_volume() { } #[tokio::test] -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] async fn oci_workspace_preparation_skips_nested_volume_ownership() { let driver = e2e_driver().expect("OPENSHELL_E2E_DRIVER must be set by the e2e wrapper"); assert!( - driver == "docker", - "OCI workspace mount e2e requires docker, got {driver}" + matches!(driver.as_str(), "docker" | "podman"), + "OCI workspace mount e2e requires docker or podman, got {driver}" ); let volume = VolumeGuard::create(&driver) @@ -342,7 +342,7 @@ async fn verify_volume(volume: &VolumeGuard) -> Result<(), String> { Ok(()) } -#[cfg(feature = "e2e-docker")] +#[cfg(any(feature = "e2e-docker", feature = "e2e-podman"))] async fn verify_volume_ownership(volume: &VolumeGuard) -> Result<(), String> { let output = run_volume_container( volume, diff --git a/e2e/rust/tests/podman_oci_identity.rs b/e2e/rust/tests/podman_oci_identity.rs index e30516bf09..8dcc25155c 100644 --- a/e2e/rust/tests/podman_oci_identity.rs +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -3,14 +3,14 @@ #![cfg(feature = "e2e-podman")] -//! Podman-specific E2E coverage for OCI identity inspection and immutable-image -//! launch. +//! Podman-specific E2E coverage for OCI identity/workspace inspection, +//! workspace-volume copy-up, and immutable-image launch. //! //! The test builds an image through the selected Podman engine, creates a -//! sandbox from its mutable tag, and verifies both the child identity and the -//! image ID recorded on the real sandbox container. This exercises the Podman -//! API inspect → protected metadata → create path rather than only its unit -//! serialization boundaries. +//! sandbox from its mutable tag, and verifies the child identity, workspace, +//! copied image content, and image ID recorded on the real sandbox container. +//! This exercises the Podman API inspect → protected metadata → create path +//! rather than only its unit serialization boundaries. use std::process::Stdio; @@ -54,7 +54,16 @@ impl ImageGuard { let containerfile = context.path().join("Containerfile"); std::fs::write( &containerfile, - format!("FROM {BASE_IMAGE}\nUSER {OCI_UID}:{OCI_GID}\n"), + format!( + "FROM {BASE_IMAGE}\n\ + USER 0:0\n\ + RUN mkdir -p /home/app/project && \ + chown {OCI_UID}:{OCI_GID} /home/app /home/app/project && \ + chmod 0700 /home/app\n\ + WORKDIR /home/app/project\n\ + RUN printf root-owned > root-owned.txt && chown {OCI_UID}:{OCI_GID} .\n\ + USER {OCI_UID}:{OCI_GID}\n" + ), ) .map_err(|err| format!("write Containerfile: {err}"))?; @@ -164,7 +173,7 @@ fn normalized_image_id(image_id: &str) -> &str { } #[tokio::test] -async fn podman_uses_oci_identity_and_inspected_image_id() { +async fn podman_uses_oci_identity_workspace_and_inspected_image_id() { if !is_e2e_driver("podman") { eprintln!("Skipping Podman OCI identity test: e2e driver is not podman"); return; @@ -178,17 +187,19 @@ async fn podman_uses_oci_identity_and_inspected_image_id() { std::fs::write(policy.path(), OCI_FALLBACK_POLICY).expect("write OCI fallback policy"); let policy_path = policy.path().to_str().expect("policy path is UTF-8"); let mut sandbox = SandboxGuard::create_keep_with_args( - &[ - "--from", - &image.tag, - "--policy", - policy_path, - "--no-tty", - ], + &["--from", &image.tag, "--policy", policy_path, "--no-tty"], &[ "sh", "-c", - "set -eu; printf 'direct-identity=%s:%s\n' \"$(id -u)\" \"$(id -g)\"; echo podman-oci-identity-ready; sleep infinity", + "set -eu; \ + test \"$(pwd -P)\" = /home/app/project; \ + test \"$HOME\" = /home/app/project; \ + test \"$(cat root-owned.txt)\" = root-owned; \ + test \"$(stat -c %u:%g .)\" = 2345:2346; \ + test \"$(stat -c %u:%g root-owned.txt)\" = 0:0; \ + touch direct-workspace-write; \ + printf 'direct-identity=%s:%s\n' \"$(id -u)\" \"$(id -g)\"; \ + echo podman-oci-identity-ready; sleep infinity", ], READY_MARKER, ) @@ -205,7 +216,13 @@ async fn podman_uses_oci_identity_and_inspected_image_id() { .exec(&[ "sh", "-c", - "test \"$(id -u):$(id -g)\" = 2345:2346; echo podman-ssh-identity-ok", + "set -eu; \ + test \"$(id -u):$(id -g)\" = 2345:2346; \ + test \"$(pwd -P)\" = /home/app/project; \ + test \"$HOME\" = /home/app/project; \ + test -f direct-workspace-write; \ + touch ssh-workspace-write; \ + echo podman-ssh-identity-ok", ]) .await .expect("SSH child should use Podman OCI identity"); diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index e3f18af19f..fd3e58554c 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -132,6 +132,17 @@ message DriverSandboxSpec { // ServiceAccount token bootstrap instead). Never echoed to the public // Sandbox proto. string sandbox_token = 11 [(openshell.options.v1.secret) = true]; + // Final process-identity inputs needed by local container drivers when they + // validate an OCI image workspace before launching the supervisor. + WorkspaceValidationIdentity workspace_validation_identity = 12; +} + +message WorkspaceValidationIdentity { + string run_as_user = 1; + string run_as_group = 2; + // No public policy was supplied, so the supervisor must discover an + // image-provided policy before resolving the final process identity. + bool discover_from_image_policy = 3; } message ResourceRequirements { From d7d5cf7fe00c494393f72762a2297bb71c6768f6 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Tue, 4 Aug 2026 11:59:58 -0700 Subject: [PATCH 2/7] fix(podman): harden workspace probe lifecycle Signed-off-by: Matthew Grossman --- crates/openshell-driver-podman/README.md | 12 +- .../openshell-driver-podman/src/container.rs | 102 ++++- crates/openshell-driver-podman/src/driver.rs | 358 +++++++++++++++--- crates/openshell-server/src/compute/mod.rs | 7 +- e2e/rust/tests/podman_oci_identity.rs | 19 +- 5 files changed, 406 insertions(+), 92 deletions(-) diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 4762d92480..1c1978b3f6 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -28,10 +28,14 @@ write and enter the workdir. The path must be a real directory without symlink components. The probe also rejects kernel-managed filesystems and overlaps with concrete OpenShell control resources. It emits a normalized identity attestation; the final supervisor must resolve to the same identity, including -when the image supplies the default process policy. On failure, the driver -captures a bounded, sanitized diagnostic before removing the probe. Only then -does Podman mount and prepare the managed workspace volume at that path; normal -copy-up preserves image content. The workspace is the child cwd and `HOME`. +when the image supplies the default process policy. The probe inherits the +sandbox CPU, memory, and PID limits and has a bounded runtime. Its lifecycle +continues to forced cleanup if the create request is cancelled, and a dedicated +label lets a restarted gateway remove any probe left by process termination. +On failure, the driver captures a bounded, sanitized diagnostic before removing +the probe. Only then does Podman mount and prepare the managed workspace volume +at that path; normal copy-up preserves image content. The workspace is the +child cwd and `HOME`. For a rootless networking deep dive, see [NETWORKING.md](NETWORKING.md). diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 4b778bd3af..5a5cd6de6c 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -45,6 +45,10 @@ pub use openshell_core::driver_utils::{ pub const LABEL_MANAGED: &str = "openshell.managed"; /// Label filter string for list/event queries. pub const LABEL_MANAGED_FILTER: &str = "openshell.managed=true"; +/// Label applied only to temporary OCI workdir validation probes. +pub const LABEL_WORKSPACE_PROBE: &str = "openshell.workspace-probe"; +/// Label filter used to recover probes left behind by a stopped gateway. +pub const LABEL_WORKSPACE_PROBE_FILTER: &str = "openshell.workspace-probe=true"; /// Container name prefix to avoid collisions with user containers. const CONTAINER_PREFIX: &str = "openshell-"; @@ -235,6 +239,30 @@ struct ContainerSpec { portmappings: Vec, } +/// Minimal Podman spec for an OpenShell-owned auxiliary container. +/// +/// Keep this typed separately from [`ContainerSpec`] so probes cannot +/// accidentally inherit sandbox networking, secrets, or workspace mounts, +/// while still sharing resource-limit construction and recovery metadata. +#[derive(Serialize)] +struct WorkspaceProbeSpec { + name: String, + image: String, + labels: BTreeMap, + entrypoint: Vec, + command: Vec, + user: String, + work_dir: String, + image_volumes: Vec, + image_volume_mode: String, + netns: NetNS, + no_new_privileges: bool, + cap_drop: Vec, + cap_add: Vec, + image_pull_policy: String, + resource_limits: ResourceLimits, +} + /// A port mapping entry for the libpod `SpecGenerator`. #[derive(Serialize)] struct PortMapping { @@ -544,6 +572,16 @@ fn build_labels(sandbox: &DriverSandbox) -> BTreeMap { labels } +fn build_workspace_probe_labels(sandbox: &DriverSandbox) -> BTreeMap { + BTreeMap::from([ + (LABEL_WORKSPACE_PROBE.into(), "true".into()), + (LABEL_SANDBOX_ID.into(), sandbox.id.clone()), + (LABEL_SANDBOX_NAME.into(), sandbox.name.clone()), + (LABEL_SANDBOX_NAMESPACE.into(), sandbox.namespace.clone()), + (LABEL_SANDBOX_WORKSPACE.into(), sandbox.workspace.clone()), + ]) +} + /// Parse resource limits from the sandbox template, falling back to defaults. fn build_resource_limits(sandbox: &DriverSandbox, config: &PodmanComputeConfig) -> ResourceLimits { let resources = sandbox @@ -1277,27 +1315,35 @@ pub fn build_workspace_probe_spec( } } - Ok(Some(serde_json::json!({ - "name": probe_name, - "image": inspected_image.id, - "entrypoint": [SUPERVISOR_BINARY_PATH], - "command": command, - "user": "0:0", - "work_dir": "/", - "image_volumes": [{ - "source": config.supervisor_image, - "destination": SUPERVISOR_MOUNT_DIR, - "rw": false + let probe_spec = WorkspaceProbeSpec { + name: probe_name.to_string(), + image: inspected_image.id.clone(), + labels: build_workspace_probe_labels(sandbox), + entrypoint: vec![SUPERVISOR_BINARY_PATH.into()], + command, + user: "0:0".into(), + work_dir: "/".into(), + image_volumes: vec![ImageVolume { + source: config.supervisor_image.clone(), + destination: SUPERVISOR_MOUNT_DIR.into(), + rw: false, }], // Do not materialize OCI VOLUME declarations: the probe must inspect // the immutable image layer rather than a fresh anonymous volume. - "image_volume_mode": "ignore", - "netns": {"nsmode": "none"}, - "no_new_privileges": true, - "cap_drop": ["ALL"], - "cap_add": ["SETUID", "SETGID"], - "image_pull_policy": "never" - }))) + image_volume_mode: "ignore".into(), + netns: NetNS { + nsmode: "none".into(), + }, + no_new_privileges: true, + cap_drop: vec!["ALL".into()], + cap_add: vec!["SETUID".into(), "SETGID".into()], + image_pull_policy: "never".into(), + resource_limits: build_resource_limits(sandbox, config), + }; + + Ok(Some( + serde_json::to_value(probe_spec).expect("WorkspaceProbeSpec serialization cannot fail"), + )) } fn hostadd_entries(config: &PodmanComputeConfig) -> Vec { @@ -1614,6 +1660,13 @@ mod tests { fn workspace_probe_uses_pinned_image_without_workspace_or_network() { let mut sandbox = test_sandbox("test-id", "test-name"); let spec = sandbox.spec.get_or_insert_default(); + spec.template.get_or_insert_default().resources = Some( + openshell_core::proto::compute::v1::DriverResourceRequirements { + cpu_limit: "750m".into(), + memory_limit: "1Gi".into(), + ..Default::default() + }, + ); spec.workspace_validation_identity = Some( openshell_core::proto::compute::v1::WorkspaceValidationIdentity { run_as_user: "policy-user".into(), @@ -1621,9 +1674,11 @@ mod tests { discover_from_image_policy: false, }, ); + let mut config = test_config(); + config.sandbox_pids_limit = 37; let probe = build_workspace_probe_spec( &sandbox, - &test_config(), + &config, &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), "openshell-test-probe", ) @@ -1635,6 +1690,15 @@ mod tests { assert_eq!(probe["image_volume_mode"], "ignore"); assert_eq!(probe["cap_drop"], serde_json::json!(["ALL"])); assert_eq!(probe["cap_add"], serde_json::json!(["SETUID", "SETGID"])); + assert_eq!(probe["labels"][LABEL_WORKSPACE_PROBE], "true"); + assert_eq!(probe["labels"][LABEL_SANDBOX_ID], "test-id"); + assert!(probe["labels"].get(LABEL_MANAGED).is_none()); + assert_eq!(probe["resource_limits"]["cpu"]["quota"], 75_000); + assert_eq!( + probe["resource_limits"]["memory"]["limit"], + 1024 * 1024 * 1024_u64 + ); + assert_eq!(probe["resource_limits"]["PidsLimit"], 37); assert!(probe.get("volumes").is_none()); assert!(probe.get("secrets").is_none()); assert!(probe.get("env").is_none()); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 00d62cfde6..66579f8109 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -5,7 +5,10 @@ use crate::client::{PodmanApiError, PodmanClient, VolumeInspect}; use crate::config::PodmanComputeConfig; -use crate::container::{self, LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID, PodmanSandboxDriverConfig}; +use crate::container::{ + self, LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID, LABEL_WORKSPACE_PROBE_FILTER, + PodmanSandboxDriverConfig, +}; use crate::watcher::{ self, WatchStream, driver_sandbox_from_inspect, driver_sandbox_from_list_entry, }; @@ -32,6 +35,8 @@ use std::time::Duration; use tracing::{debug, info, warn}; use url::Url; +const WORKSPACE_PROBE_TIMEOUT: Duration = Duration::from_secs(30); + impl From for ComputeDriverError { fn from(value: PodmanApiError) -> Self { match value { @@ -265,6 +270,101 @@ fn sanitized_probe_diagnostic(logs: &str) -> String { } } +async fn remove_workspace_probes( + client: &PodmanClient, + lifecycle_lock: &tokio::sync::Mutex<()>, + stop_timeout_secs: u32, +) -> Result { + let probes = client + .list_containers(&[LABEL_WORKSPACE_PROBE_FILTER]) + .await?; + let mut removed = 0; + for probe in probes { + let _lifecycle_guard = lifecycle_lock.lock().await; + match client.remove_container(&probe.id, stop_timeout_secs).await { + Ok(()) => removed += 1, + Err(PodmanApiError::NotFound(_)) => {} + Err(error) => return Err(error), + } + } + Ok(removed) +} + +async fn run_workspace_probe( + client: PodmanClient, + lifecycle_lock: Arc>, + stop_timeout_secs: u32, + spec: serde_json::Value, + probe_name: String, + image_id: String, + timeout: Duration, +) -> Result, ComputeDriverError> { + let mut created = false; + let validation = tokio::time::timeout(timeout, async { + { + let _lifecycle_guard = lifecycle_lock.lock().await; + client + .create_container(&spec) + .await + .map_err(ComputeDriverError::from)?; + created = true; + client + .start_container(&probe_name) + .await + .map_err(ComputeDriverError::from)?; + } + + let exit_code = client + .wait_container(&probe_name) + .await + .map_err(ComputeDriverError::from)?; + let logs = client.container_logs(&probe_name).await; + if exit_code == 0 { + let logs = logs.map_err(ComputeDriverError::from)?; + parse_workspace_identity(&logs).map(Some) + } else { + let diagnostic = logs.map_or_else( + |error| format!("unable to read probe diagnostic: {error}"), + |logs| sanitized_probe_diagnostic(&logs), + ); + Err(ComputeDriverError::Precondition(format!( + "OCI WorkingDir validation failed for image '{image_id}' (probe exited with code {exit_code}): {diagnostic}", + ))) + } + }) + .await + .unwrap_or_else(|_| { + Err(ComputeDriverError::Precondition(format!( + "OCI WorkingDir validation timed out for image '{image_id}' after {} seconds", + timeout.as_secs() + ))) + }); + + if !created { + return validation; + } + + let cleanup = { + let _lifecycle_guard = lifecycle_lock.lock().await; + client + .remove_container(&probe_name, stop_timeout_secs) + .await + }; + match (validation, cleanup) { + (Ok(identity), Ok(())) => Ok(identity), + (Ok(_), Err(error)) => Err(ComputeDriverError::from(error)), + (Err(error), Ok(())) => Err(error), + (Err(error), Err(cleanup_error)) => { + warn!( + probe = %probe_name, + %cleanup_error, + "Failed to remove workspace validation probe" + ); + Err(error) + } + } +} + /// Resolve the socket to connect to: explicit configuration wins, otherwise /// fall back to `detect`. Returns an error if neither resolves. /// @@ -305,59 +405,20 @@ impl PodmanComputeDriver { return Ok(None); }; - let start = { - let _lifecycle_guard = self.container_lifecycle_lock.lock().await; - self.client - .create_container(&spec) - .await - .map_err(ComputeDriverError::from)?; - self.client - .start_container(&probe_name) - .await - .map_err(ComputeDriverError::from) - }; - let validation = async { - start?; - let exit_code = self - .client - .wait_container(&probe_name) - .await - .map_err(ComputeDriverError::from)?; - let logs = self.client.container_logs(&probe_name).await; - if exit_code == 0 { - let logs = logs.map_err(ComputeDriverError::from)?; - parse_workspace_identity(&logs).map(Some) - } else { - let diagnostic = logs.map_or_else( - |error| format!("unable to read probe diagnostic: {error}"), - |logs| sanitized_probe_diagnostic(&logs), - ); - Err(ComputeDriverError::Precondition(format!( - "OCI WorkingDir validation failed for image '{}' (probe exited with code {exit_code}): {diagnostic}", - inspected_image.id, - ))) - } - } - .await; - let cleanup = { - let _lifecycle_guard = self.container_lifecycle_lock.lock().await; - self.client - .remove_container(&probe_name, self.config.stop_timeout_secs) - .await - }; - match (validation, cleanup) { - (Ok(identity), Ok(())) => Ok(identity), - (Ok(_), Err(error)) => Err(ComputeDriverError::from(error)), - (Err(error), Ok(())) => Err(error), - (Err(error), Err(cleanup_error)) => { - warn!( - probe = %probe_name, - %cleanup_error, - "Failed to remove workspace validation probe" - ); - Err(error) - } - } + let task = tokio::spawn(run_workspace_probe( + self.client.clone(), + Arc::clone(&self.container_lifecycle_lock), + self.config.stop_timeout_secs, + spec, + probe_name, + inspected_image.id.clone(), + WORKSPACE_PROBE_TIMEOUT, + )); + task.await.map_err(|error| { + ComputeDriverError::Message(format!( + "OCI WorkingDir validation probe task failed: {error}" + )) + })? } /// Create a new driver, verifying the Podman socket is reachable. @@ -494,6 +555,17 @@ impl PodmanComputeDriver { "Bridge network ready" ); + let container_lifecycle_lock = Arc::new(tokio::sync::Mutex::new(())); + let removed_probes = + remove_workspace_probes(&client, &container_lifecycle_lock, config.stop_timeout_secs) + .await?; + if removed_probes > 0 { + info!( + count = removed_probes, + "Removed workspace validation probes left by a previous gateway process" + ); + } + let (gpu_inventory, allow_all_default_gpu) = local_podman_gpu_selector_state(); if !gpu_inventory.is_empty() { info!( @@ -508,7 +580,7 @@ impl PodmanComputeDriver { network_gateway_ip, rootless, rootless_network_cmd, - container_lifecycle_lock: Arc::new(tokio::sync::Mutex::new(())), + container_lifecycle_lock, gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -1607,6 +1679,7 @@ mod tests { }"#, ), StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::OK, "[]"), ], ); let config = PodmanComputeConfig { @@ -1622,17 +1695,70 @@ mod tests { assert!(driver.network_gateway_ip().is_none()); assert!(driver.gateway_listener_requirements().unwrap().is_empty()); handle.await.expect("stub task should finish"); + let requests = request_log + .lock() + .expect("request log lock should not be poisoned"); assert_eq!( - request_log - .lock() - .expect("request log lock should not be poisoned") - .as_slice(), + &requests[..3], [ "GET /_ping".to_string(), format!("GET {}", api_path("/libpod/info")), format!("POST {}", api_path("/libpod/networks/create")), ] ); + assert!(requests[3].contains("/libpod/containers/json?all=true&filters=")); + } + + #[tokio::test] + async fn constructor_removes_workspace_probes_left_by_previous_process() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "stale-workspace-probe", + vec![ + StubResponse::new(StatusCode::OK, ""), + StubResponse::new( + StatusCode::OK, + r#"{ + "host": { + "cgroupVersion": "v2", + "networkBackend": "netavark", + "security": {"rootless": false} + } + }"#, + ), + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new( + StatusCode::OK, + r#"[{ + "Id": "stale-probe-id", + "Names": ["openshell-stale-workdir-probe"], + "State": "exited", + "Labels": {"openshell.workspace-probe": "true"} + }]"#, + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let config = PodmanComputeConfig { + socket_path: Some(socket_path.clone()), + grpc_endpoint: "https://gateway.example.test:9443".to_string(), + ..PodmanComputeConfig::default() + }; + + PodmanComputeDriver::new(config) + .await + .expect("startup should remove a stale workspace probe"); + + handle.await.expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests[3].contains("/libpod/containers/json?all=true&filters=")); + assert!(requests[3].contains("openshell.workspace-probe")); + assert_eq!( + requests[4], + format!( + "DELETE {}?force=true&volumes=true&timeout=45", + api_path("/libpod/containers/stale-probe-id") + ) + ); } #[test] @@ -2270,6 +2396,120 @@ mod tests { let _ = fs::remove_file(socket_path); } + #[tokio::test] + async fn workspace_probe_cleanup_survives_caller_cancellation() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-cancelled-caller", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":0}"#) + .with_delay(Duration::from_millis(100)), + StubResponse::new( + StatusCode::OK, + "OPENSHELL_WORKSPACE_IDENTITY=\"1234:1235:\"\n", + ), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let mut sandbox = plain_sandbox("sandbox-probe-cancel", "demo"); + sandbox.spec = Some(DriverSandboxSpec::default()); + let image = crate::client::ImageInspect { + id: "sha256:immutable".into(), + config: Some(crate::client::ImageConfig { + user: "1234:1235".into(), + working_dir: "/workspace".into(), + }), + }; + let name = validated_container_name(&sandbox).unwrap(); + + let caller = tokio::spawn(async move { + driver + .validate_image_workspace(&sandbox, &name, &image) + .await + }); + tokio::time::timeout(Duration::from_secs(2), async { + loop { + if request_log + .lock() + .unwrap() + .iter() + .any(|request| request.contains("/wait?condition=exited")) + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("probe should reach wait before cancellation"); + caller.abort(); + let _ = caller.await; + + tokio::time::timeout(Duration::from_secs(2), handle) + .await + .expect("owned probe task should finish after caller cancellation") + .expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests.last().unwrap().starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + + #[tokio::test] + async fn workspace_probe_timeout_forces_container_cleanup() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-timeout", + vec![ + StubResponse::new(StatusCode::CREATED, "{}"), + StubResponse::new(StatusCode::NO_CONTENT, ""), + StubResponse::new(StatusCode::OK, r#"{"StatusCode":0}"#) + .with_delay(Duration::from_millis(100)), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let mut sandbox = plain_sandbox("sandbox-probe-timeout", "demo"); + sandbox.spec = Some(DriverSandboxSpec::default()); + let image = crate::client::ImageInspect { + id: "sha256:immutable".into(), + config: Some(crate::client::ImageConfig { + user: "1234:1235".into(), + working_dir: "/workspace".into(), + }), + }; + let name = validated_container_name(&sandbox).unwrap(); + let spec = container::build_workspace_probe_spec( + &sandbox, + &driver.config, + &image, + &workspace_probe_name(&name), + ) + .unwrap() + .unwrap(); + + let error = run_workspace_probe( + driver.client.clone(), + Arc::clone(&driver.container_lifecycle_lock), + driver.config.stop_timeout_secs, + spec, + workspace_probe_name(&name), + image.id, + Duration::from_millis(25), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("timed out")); + + tokio::time::timeout(Duration::from_secs(2), handle) + .await + .expect("timed-out probe should be force-removed") + .expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert!(requests.last().unwrap().starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + fn secret_delete_request(sandbox_id: &str) -> String { format!( "DELETE {}", diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index b2d70be671..070e6563dd 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -2515,7 +2515,7 @@ fn driver_sandbox_spec_from_public( spec: &SandboxSpec, driver_name: &str, ) -> Result> { - let workspace_validation_identity = matches!(driver_name, "docker" | "podman").then(|| { + let workspace_validation_identity = (driver_name == "podman").then(|| { let process = spec .policy .as_ref() @@ -3296,7 +3296,7 @@ mod tests { } #[test] - fn driver_sandbox_spec_projects_process_identity_for_local_image_probe() { + fn driver_sandbox_spec_projects_process_identity_only_for_podman_probe() { let public = SandboxSpec { policy: Some(openshell_core::proto::SandboxPolicy { process: Some(openshell_core::proto::ProcessPolicy { @@ -3323,6 +3323,9 @@ mod tests { .unwrap() .discover_from_image_policy ); + + let docker = driver_sandbox_spec_from_public(&public, "docker").unwrap(); + assert!(docker.workspace_validation_identity.is_none()); } #[test] diff --git a/e2e/rust/tests/podman_oci_identity.rs b/e2e/rust/tests/podman_oci_identity.rs index 8dcc25155c..ff492bbb0b 100644 --- a/e2e/rust/tests/podman_oci_identity.rs +++ b/e2e/rust/tests/podman_oci_identity.rs @@ -22,7 +22,7 @@ const BASE_IMAGE: &str = "ghcr.io/nvidia/openshell-community/sandboxes/base:late const READY_MARKER: &str = "podman-oci-identity-ready"; const OCI_UID: &str = "2345"; const OCI_GID: &str = "2346"; -const OCI_FALLBACK_POLICY: &str = r#"version: 1 +const IMAGE_POLICY: &str = r#"version: 1 filesystem_policy: include_workdir: true @@ -32,6 +32,10 @@ landlock: compatibility: best_effort network_policies: {} + +process: + run_as_user: "2345" + run_as_group: "2346" "#; struct ImageGuard { @@ -52,6 +56,8 @@ impl ImageGuard { let context = tempfile::tempdir().map_err(|err| format!("create build context: {err}"))?; let containerfile = context.path().join("Containerfile"); + std::fs::write(context.path().join("policy.yaml"), IMAGE_POLICY) + .map_err(|err| format!("write image policy: {err}"))?; std::fs::write( &containerfile, format!( @@ -60,6 +66,7 @@ impl ImageGuard { RUN mkdir -p /home/app/project && \ chown {OCI_UID}:{OCI_GID} /home/app /home/app/project && \ chmod 0700 /home/app\n\ + COPY policy.yaml /etc/openshell/policy.yaml\n\ WORKDIR /home/app/project\n\ RUN printf root-owned > root-owned.txt && chown {OCI_UID}:{OCI_GID} .\n\ USER {OCI_UID}:{OCI_GID}\n" @@ -180,14 +187,10 @@ async fn podman_uses_oci_identity_workspace_and_inspected_image_id() { } let image = ImageGuard::build().expect("build Podman OCI identity image"); - // The community base image contains a baked default policy with an - // explicit `sandbox` process identity. Supply a complete policy that - // intentionally omits `process` so this test exercises OCI fallback. - let policy = tempfile::NamedTempFile::new().expect("create OCI fallback policy"); - std::fs::write(policy.path(), OCI_FALLBACK_POLICY).expect("write OCI fallback policy"); - let policy_path = policy.path().to_str().expect("policy path is UTF-8"); + // Do not supply a policy at create time. The probe must discover the + // image's policy before the workspace volume hides the immutable layer. let mut sandbox = SandboxGuard::create_keep_with_args( - &["--from", &image.tag, "--policy", policy_path, "--no-tty"], + &["--from", &image.tag, "--no-tty"], &[ "sh", "-c", From e1f6745657592635bba57f6783b33ab193e2a0e6 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Tue, 4 Aug 2026 13:10:08 -0700 Subject: [PATCH 3/7] refactor(podman): simplify workspace probe flow Signed-off-by: Matthew Grossman --- architecture/compute-runtimes.md | 11 +- crates/openshell-driver-podman/README.md | 25 +- crates/openshell-driver-podman/src/client.rs | 25 +- .../openshell-driver-podman/src/container.rs | 284 ++++++++---------- crates/openshell-driver-podman/src/driver.rs | 175 ++++------- crates/openshell-sandbox/src/lib.rs | 25 +- .../src/process.rs | 26 +- .../openshell-supervisor-process/src/run.rs | 1 + 8 files changed, 266 insertions(+), 306 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 7ae07a934e..b043f883a0 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -213,13 +213,10 @@ control paths. Docker performs the check in the final container before workload launch and rejects image `VOLUME` declarations that would mask the workdir ancestry. Podman performs it in a minimal networkless container from the same pinned -image ID before its managed workspace volume covers the path. The probe retains -only the capabilities needed to adopt the completed process identity, drops to -that identity, and asks the kernel to validate access. It emits a normalized -identity attestation that the final supervisor must match, including when the -image supplies the default process policy. The driver captures a bounded, -sanitized diagnostic before removing a failed probe. The resolved workspace is -the child cwd and `HOME`; when `filesystem.include_workdir` is enabled, it +image ID before its managed workspace volume covers the path. The probe adopts +the completed process identity and emits an attestation that the final +supervisor must match. The resolved workspace is the child cwd and `HOME`; +when `filesystem.include_workdir` is enabled, it becomes the automatic writable policy path. Kubernetes/OpenShift keep their `/sandbox` PVC and `fsGroup` behavior, and VM keeps its `/sandbox` guest initialization path. diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 1c1978b3f6..54391f402a 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -19,23 +19,14 @@ with the user's numeric primary GID. Explicit `process.run_as_user` and An absolute OCI working directory becomes the agent workspace. An empty, root (`/`), or explicit `/sandbox` declaration uses `/sandbox`, which OpenShell -creates when necessary and owns as a compatibility workspace. For any other workdir, Podman -first starts a minimal, networkless probe from the pinned image ID without the -workspace volume, tokens, or TLS secrets. The probe retains only `SETUID` and -`SETGID`, resolves and adopts the completed identity, including supplementary -groups, and uses the kernel to verify that it can traverse every parent and -write and enter the workdir. The path must be a real directory without symlink -components. The probe also rejects kernel-managed filesystems and overlaps with -concrete OpenShell control resources. It emits a normalized identity -attestation; the final supervisor must resolve to the same identity, including -when the image supplies the default process policy. The probe inherits the -sandbox CPU, memory, and PID limits and has a bounded runtime. Its lifecycle -continues to forced cleanup if the create request is cancelled, and a dedicated -label lets a restarted gateway remove any probe left by process termination. -On failure, the driver captures a bounded, sanitized diagnostic before removing -the probe. Only then does Podman mount and prepare the managed workspace volume -at that path; normal copy-up preserves image content. The workspace is the -child cwd and `HOME`. +creates and owns as a compatibility workspace. For any other workdir, a +resource-limited, networkless probe verifies the original pinned image before +Podman covers the path with the managed workspace volume. The completed process +identity must already be able to traverse every parent and write and enter the +directory, without symlink components or OpenShell control-path collisions. +The final supervisor must match the probe's identity before preparing the +volume. See [Compute runtimes](../../architecture/compute-runtimes.md#process-identity) +for the invariant and probe lifecycle. For a rootless networking deep dive, see [NETWORKING.md](NETWORKING.md). diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 1c5e7e05bf..f7b3cc83a3 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -345,17 +345,26 @@ impl PodmanClient { builder.body(body).expect("valid request") } - /// Send a pre-built HTTP request and return status + body bytes. - async fn send_request( + /// Send a pre-built HTTP request for bounded or complete body collection. + async fn send_response( &self, req: Request>, timeout: Duration, - ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { + ) -> Result, PodmanApiError> { let mut sender = self.connect().await?; - let response = tokio::time::timeout(timeout, sender.send_request(req)) + tokio::time::timeout(timeout, sender.send_request(req)) .await .map_err(|_| PodmanApiError::Timeout(timeout))? - .map_err(|e| PodmanApiError::Connection(e.to_string()))?; + .map_err(|error| PodmanApiError::Connection(error.to_string())) + } + + /// Send a pre-built HTTP request and return status + body bytes. + async fn send_request( + &self, + req: Request>, + timeout: Duration, + ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { + let response = self.send_response(req, timeout).await?; let status = response.status(); let bytes = tokio::time::timeout(timeout, response.into_body().collect()) .await @@ -374,11 +383,7 @@ impl PodmanClient { ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { use hyper::body::Body; - let mut sender = self.connect().await?; - let response = tokio::time::timeout(timeout, sender.send_request(req)) - .await - .map_err(|_| PodmanApiError::Timeout(timeout))? - .map_err(|error| PodmanApiError::Connection(error.to_string()))?; + let response = self.send_response(req, timeout).await?; let status = response.status(); let deadline = tokio::time::Instant::now() + timeout; let mut body = response.into_body(); diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 5a5cd6de6c..0e4da403ec 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -186,6 +186,39 @@ pub fn short_id(id: &str) -> String { // Typed container spec structs for the Podman libpod create API. // --------------------------------------------------------------------------- +/// Immutable image metadata normalized once for both probe and final launch. +#[derive(Debug, Clone)] +pub struct ResolvedPodmanImage { + id: String, + oci_user: String, + workspace_root: String, +} + +impl ResolvedPodmanImage { + pub fn from_inspect( + inspected: &ImageInspect, + config: &PodmanComputeConfig, + ) -> Result { + let image_config = inspected.config.as_ref(); + let workspace_root = driver_mounts::resolve_oci_workspace_root( + image_config.map_or("", |config| config.working_dir.as_str()), + ) + .map_err(ComputeDriverError::Precondition)?; + driver_mounts::validate_workspace_control_path( + &workspace_root, + &config.sandbox_ssh_socket_path, + ) + .map_err(ComputeDriverError::Precondition)?; + Ok(Self { + id: inspected.id.clone(), + oci_user: image_config + .map_or("", |config| config.user.as_str()) + .to_string(), + workspace_root, + }) + } +} + #[derive(Serialize)] struct ContainerSpec { name: String, @@ -245,7 +278,7 @@ struct ContainerSpec { /// accidentally inherit sandbox networking, secrets, or workspace mounts, /// while still sharing resource-limit construction and recovery metadata. #[derive(Serialize)] -struct WorkspaceProbeSpec { +pub struct WorkspaceProbeSpec { name: String, image: String, labels: BTreeMap, @@ -263,6 +296,20 @@ struct WorkspaceProbeSpec { resource_limits: ResourceLimits, } +impl WorkspaceProbeSpec { + pub fn name(&self) -> &str { + &self.name + } + + pub fn image_id(&self) -> &str { + &self.image + } + + pub fn to_value(&self) -> Value { + serde_json::to_value(self).expect("WorkspaceProbeSpec serialization cannot fail") + } +} + /// A port mapping entry for the libpod `SpecGenerator`. #[derive(Serialize)] struct PortMapping { @@ -572,14 +619,14 @@ fn build_labels(sandbox: &DriverSandbox) -> BTreeMap { labels } -fn build_workspace_probe_labels(sandbox: &DriverSandbox) -> BTreeMap { - BTreeMap::from([ - (LABEL_WORKSPACE_PROBE.into(), "true".into()), - (LABEL_SANDBOX_ID.into(), sandbox.id.clone()), - (LABEL_SANDBOX_NAME.into(), sandbox.name.clone()), - (LABEL_SANDBOX_NAMESPACE.into(), sandbox.namespace.clone()), - (LABEL_SANDBOX_WORKSPACE.into(), sandbox.workspace.clone()), - ]) +fn workspace_probe_name(sandbox: &DriverSandbox) -> String { + const SUFFIX: &str = "-workdir-probe"; + let container_name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); + let keep = 255usize.saturating_sub(SUFFIX.len()); + format!( + "{}{SUFFIX}", + &container_name[..container_name.len().min(keep)] + ) } /// Parse resource limits from the sandbox template, falling back to defaults. @@ -954,16 +1001,20 @@ pub fn build_container_spec_with_token_and_gpu_devices( gpu_device_ids: Option<&[String]>, ) -> Result { let image = resolve_image(sandbox, config); + let resolved_image = ResolvedPodmanImage::from_inspect( + &ImageInspect { + id: image.to_string(), + config: None, + }, + config, + )?; build_container_spec_for_image( sandbox, config, token_secret_name, gpu_device_ids, image, - &ImageInspect { - id: image.to_string(), - config: None, - }, + &resolved_image, None, ) } @@ -974,31 +1025,16 @@ pub fn build_container_spec_for_image( token_secret_name: Option<&str>, gpu_device_ids: Option<&[String]>, requested_image: &str, - inspected_image: &ImageInspect, + image: &ResolvedPodmanImage, workspace_identity: Option<&str>, ) -> Result { let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); - let oci_user = inspected_image - .config - .as_ref() - .map_or("", |config| config.user.as_str()); - let oci_working_dir = inspected_image - .config - .as_ref() - .map_or("", |config| config.working_dir.as_str()); - let workspace_root = driver_mounts::resolve_oci_workspace_root(oci_working_dir) - .map_err(ComputeDriverError::Precondition)?; - driver_mounts::validate_workspace_control_path( - &workspace_root, - &config.sandbox_ssh_socket_path, - ) - .map_err(ComputeDriverError::Precondition)?; - let mut env = build_env(sandbox, config, requested_image, oci_user); + let mut env = build_env(sandbox, config, requested_image, &image.oci_user); let labels = build_labels(sandbox); let resource_limits = build_resource_limits(sandbox, config); - let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts, &workspace_root) + let user_mounts = podman_user_mounts(sandbox, config.enable_bind_mounts, &image.workspace_root) .map_err(ComputeDriverError::InvalidArgument)?; if sandbox .spec @@ -1027,7 +1063,7 @@ pub fn build_container_spec_for_image( let mut volumes = vec![NamedVolume { name: vol, - dest: workspace_root.clone(), + dest: image.workspace_root.clone(), options: vec!["rw".into()], }]; volumes.extend(user_mounts.volumes); @@ -1039,7 +1075,7 @@ pub fn build_container_spec_for_image( }]; image_volumes.extend(user_mounts.image_volumes); - let mut command = vec!["--workdir".to_string(), workspace_root]; + let mut command = vec!["--workdir".to_string(), image.workspace_root.clone()]; command.extend(upstream_proxy_cli_args(config)); if let Some(identity) = workspace_identity { env.insert( @@ -1050,7 +1086,7 @@ pub fn build_container_spec_for_image( let container_spec = ContainerSpec { name, - image: inspected_image.id.clone(), + image: image.id.clone(), labels, env, volumes, @@ -1272,20 +1308,9 @@ pub fn build_container_spec_for_image( pub fn build_workspace_probe_spec( sandbox: &DriverSandbox, config: &PodmanComputeConfig, - inspected_image: &ImageInspect, - probe_name: &str, -) -> Result, ComputeDriverError> { - let image_config = inspected_image.config.as_ref(); - let oci_user = image_config.map_or("", |config| config.user.as_str()); - let oci_working_dir = image_config.map_or("", |config| config.working_dir.as_str()); - let workspace_root = driver_mounts::resolve_oci_workspace_root(oci_working_dir) - .map_err(ComputeDriverError::Precondition)?; - driver_mounts::validate_workspace_control_path( - &workspace_root, - &config.sandbox_ssh_socket_path, - ) - .map_err(ComputeDriverError::Precondition)?; - if workspace_root == driver_mounts::DEFAULT_WORKSPACE_ROOT { + image: &ResolvedPodmanImage, +) -> Result, ComputeDriverError> { + if image.workspace_root == driver_mounts::DEFAULT_WORKSPACE_ROOT { return Ok(None); } @@ -1296,9 +1321,9 @@ pub fn build_workspace_probe_spec( let mut command = vec![ "probe-workspace".to_string(), "--workdir".to_string(), - workspace_root, + image.workspace_root.clone(), "--oci-user".to_string(), - oci_user.to_string(), + image.oci_user.clone(), ]; if let Some(identity) = &spec.workspace_validation_identity { for (flag, value) in [ @@ -1316,9 +1341,12 @@ pub fn build_workspace_probe_spec( } let probe_spec = WorkspaceProbeSpec { - name: probe_name.to_string(), - image: inspected_image.id.clone(), - labels: build_workspace_probe_labels(sandbox), + name: workspace_probe_name(sandbox), + image: image.id.clone(), + labels: BTreeMap::from([ + (LABEL_WORKSPACE_PROBE.into(), "true".into()), + (LABEL_SANDBOX_ID.into(), sandbox.id.clone()), + ]), entrypoint: vec![SUPERVISOR_BINARY_PATH.into()], command, user: "0:0".into(), @@ -1341,9 +1369,7 @@ pub fn build_workspace_probe_spec( resource_limits: build_resource_limits(sandbox, config), }; - Ok(Some( - serde_json::to_value(probe_spec).expect("WorkspaceProbeSpec serialization cannot fail"), - )) + Ok(Some(probe_spec)) } fn hostadd_entries(config: &PodmanComputeConfig) -> Vec { @@ -1438,7 +1464,7 @@ mod tests { } } - fn inspected_image(id: &str, user: &str, working_dir: &str) -> ImageInspect { + fn image_inspect(id: &str, user: &str, working_dir: &str) -> ImageInspect { ImageInspect { id: id.to_string(), config: Some(ImageConfig { @@ -1448,6 +1474,11 @@ mod tests { } } + fn resolved_image(id: &str, user: &str, working_dir: &str) -> ResolvedPodmanImage { + ResolvedPodmanImage::from_inspect(&image_inspect(id, user, working_dir), &test_config()) + .unwrap() + } + #[test] fn parse_cpu_millicore() { assert_eq!(parse_cpu_to_microseconds("500m"), Some(50_000)); @@ -1556,7 +1587,7 @@ mod tests { None, None, "registry.example/app:latest", - &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), + &resolved_image("sha256:immutable", "app:staff", "/workspace/project"), Some("1000:1000:"), ) .unwrap(); @@ -1606,7 +1637,7 @@ mod tests { None, None, "registry.example/app:latest", - &inspected_image("sha256:immutable", "app:staff", ""), + &resolved_image("sha256:immutable", "app:staff", ""), None, ) .unwrap(); @@ -1619,14 +1650,9 @@ mod tests { #[test] fn container_spec_rejects_invalid_oci_working_dir() { - let err = build_container_spec_for_image( - &test_sandbox("test-id", "test-name"), + let err = ResolvedPodmanImage::from_inspect( + &image_inspect("sha256:immutable", "app:staff", "relative/workspace"), &test_config(), - None, - None, - "registry.example/app:latest", - &inspected_image("sha256:immutable", "app:staff", "relative/workspace"), - None, ) .unwrap_err(); @@ -1638,18 +1664,13 @@ mod tests { #[test] fn container_spec_rejects_openshell_control_path_working_dir() { - let err = build_container_spec_for_image( - &test_sandbox("test-id", "test-name"), - &test_config(), - None, - None, - "registry.example/app:latest", - &inspected_image( + let err = ResolvedPodmanImage::from_inspect( + &image_inspect( "sha256:immutable", "app:staff", "/opt/openshell/bin/project", ), - None, + &test_config(), ) .unwrap_err(); @@ -1679,11 +1700,11 @@ mod tests { let probe = build_workspace_probe_spec( &sandbox, &config, - &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), - "openshell-test-probe", + &resolved_image("sha256:immutable", "app:staff", "/workspace/project"), ) .unwrap() - .unwrap(); + .unwrap() + .to_value(); assert_eq!(probe["image"], "sha256:immutable"); assert_eq!(probe["netns"]["nsmode"], "none"); @@ -1734,11 +1755,11 @@ mod tests { let probe = build_workspace_probe_spec( &sandbox, &test_config(), - &inspected_image("sha256:immutable", "app:staff", "/home/app/project"), - "openshell-test-probe", + &resolved_image("sha256:immutable", "app:staff", "/home/app/project"), ) .unwrap() - .unwrap(); + .unwrap() + .to_value(); assert_eq!( probe["command"], @@ -1758,8 +1779,7 @@ mod tests { let probe = build_workspace_probe_spec( &test_sandbox("test-id", "test-name"), &test_config(), - &inspected_image("sha256:immutable", "sandbox:sandbox", "/"), - "openshell-test-probe", + &resolved_image("sha256:immutable", "sandbox:sandbox", "/"), ) .unwrap(); assert!(probe.is_none()); @@ -1772,83 +1792,45 @@ mod tests { template: Some(DriverSandboxTemplate::default()), ..Default::default() }); - sandbox - .spec - .as_mut() - .unwrap() - .template - .as_mut() - .unwrap() - .driver_config = Some(json_struct(serde_json::json!({ - "mounts": [{ - "type": "tmpfs", - "target": "/workspace" - }] - }))); - - let err = build_container_spec_for_image( - &sandbox, - &test_config(), - None, - None, - "registry.example/app:latest", - &inspected_image("sha256:immutable", "app:staff", "/workspace"), - None, - ) - .unwrap_err(); - assert!( - err.to_string() - .contains("reserved for the OpenShell workspace") - ); + let set_tmpfs_target = |sandbox: &mut DriverSandbox, target: &str| { + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{"type": "tmpfs", "target": target}] + }))); + }; - sandbox - .spec - .as_mut() - .unwrap() - .template - .as_mut() - .unwrap() - .driver_config = Some(json_struct(serde_json::json!({ - "mounts": [{ - "type": "tmpfs", - "target": "/workspace" - }] - }))); - let err = build_container_spec_for_image( - &sandbox, - &test_config(), - None, - None, - "registry.example/app:latest", - &inspected_image("sha256:immutable", "app:staff", "/workspace/project"), - None, - ) - .unwrap_err(); - assert!( - err.to_string() - .contains("reserved for the OpenShell workspace") - ); + for workspace in ["/workspace", "/workspace/project"] { + set_tmpfs_target(&mut sandbox, "/workspace"); + let err = build_container_spec_for_image( + &sandbox, + &test_config(), + None, + None, + "registry.example/app:latest", + &resolved_image("sha256:immutable", "app:staff", workspace), + None, + ) + .unwrap_err(); + assert!( + err.to_string() + .contains("reserved for the OpenShell workspace") + ); + } - sandbox - .spec - .as_mut() - .unwrap() - .template - .as_mut() - .unwrap() - .driver_config = Some(json_struct(serde_json::json!({ - "mounts": [{ - "type": "tmpfs", - "target": "/workspace/cache" - }] - }))); + set_tmpfs_target(&mut sandbox, "/workspace/cache"); build_container_spec_for_image( &sandbox, &test_config(), None, None, "registry.example/app:latest", - &inspected_image("sha256:immutable", "app:staff", "/workspace"), + &resolved_image("sha256:immutable", "app:staff", "/workspace"), None, ) .expect("nested workspace mounts remain supported"); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 66579f8109..69422083b0 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -96,15 +96,6 @@ fn validated_container_name(sandbox: &DriverSandbox) -> Result String { - const SUFFIX: &str = "-workdir-probe"; - let keep = 255usize.saturating_sub(SUFFIX.len()); - format!( - "{}{SUFFIX}", - &container_name[..container_name.len().min(keep)] - ) -} - fn podman_volume_is_bind_backed(volume: &VolumeInspect) -> bool { (volume.driver.is_empty() || volume.driver == "local") && volume.options.get("o").is_some_and(|options| { @@ -272,7 +263,6 @@ fn sanitized_probe_diagnostic(logs: &str) -> String { async fn remove_workspace_probes( client: &PodmanClient, - lifecycle_lock: &tokio::sync::Mutex<()>, stop_timeout_secs: u32, ) -> Result { let probes = client @@ -280,7 +270,6 @@ async fn remove_workspace_probes( .await?; let mut removed = 0; for probe in probes { - let _lifecycle_guard = lifecycle_lock.lock().await; match client.remove_container(&probe.id, stop_timeout_secs).await { Ok(()) => removed += 1, Err(PodmanApiError::NotFound(_)) => {} @@ -294,17 +283,18 @@ async fn run_workspace_probe( client: PodmanClient, lifecycle_lock: Arc>, stop_timeout_secs: u32, - spec: serde_json::Value, - probe_name: String, - image_id: String, + probe: container::WorkspaceProbeSpec, timeout: Duration, -) -> Result, ComputeDriverError> { +) -> Result { + let probe_name = probe.name().to_string(); + let image_id = probe.image_id().to_string(); + let create_body = probe.to_value(); let mut created = false; let validation = tokio::time::timeout(timeout, async { { let _lifecycle_guard = lifecycle_lock.lock().await; client - .create_container(&spec) + .create_container(&create_body) .await .map_err(ComputeDriverError::from)?; created = true; @@ -321,7 +311,7 @@ async fn run_workspace_probe( let logs = client.container_logs(&probe_name).await; if exit_code == 0 { let logs = logs.map_err(ComputeDriverError::from)?; - parse_workspace_identity(&logs).map(Some) + parse_workspace_identity(&logs) } else { let diagnostic = logs.map_or_else( |error| format!("unable to read probe diagnostic: {error}"), @@ -350,19 +340,17 @@ async fn run_workspace_probe( .remove_container(&probe_name, stop_timeout_secs) .await }; - match (validation, cleanup) { - (Ok(identity), Ok(())) => Ok(identity), - (Ok(_), Err(error)) => Err(ComputeDriverError::from(error)), - (Err(error), Ok(())) => Err(error), - (Err(error), Err(cleanup_error)) => { - warn!( - probe = %probe_name, - %cleanup_error, - "Failed to remove workspace validation probe" - ); - Err(error) + if let Err(cleanup_error) = cleanup { + if validation.is_ok() { + return Err(ComputeDriverError::from(cleanup_error)); } + warn!( + probe = %probe_name, + %cleanup_error, + "Failed to remove workspace validation probe" + ); } + validation } /// Resolve the socket to connect to: explicit configuration wins, otherwise @@ -389,36 +377,28 @@ impl PodmanComputeDriver { async fn validate_image_workspace( &self, sandbox: &DriverSandbox, - container_name: &str, - inspected_image: &crate::client::ImageInspect, + image: &container::ResolvedPodmanImage, ) -> Result, ComputeDriverError> { - let probe_name = workspace_probe_name(container_name); - crate::client::validate_name(&probe_name) - .map_err(|error| ComputeDriverError::Precondition(error.to_string()))?; - let Some(spec) = container::build_workspace_probe_spec( - sandbox, - &self.config, - inspected_image, - &probe_name, - )? + let Some(probe) = container::build_workspace_probe_spec(sandbox, &self.config, image)? else { return Ok(None); }; + crate::client::validate_name(probe.name()) + .map_err(|error| ComputeDriverError::Precondition(error.to_string()))?; let task = tokio::spawn(run_workspace_probe( self.client.clone(), Arc::clone(&self.container_lifecycle_lock), self.config.stop_timeout_secs, - spec, - probe_name, - inspected_image.id.clone(), + probe, WORKSPACE_PROBE_TIMEOUT, )); - task.await.map_err(|error| { + let identity = task.await.map_err(|error| { ComputeDriverError::Message(format!( "OCI WorkingDir validation probe task failed: {error}" )) - })? + })??; + Ok(Some(identity)) } /// Create a new driver, verifying the Podman socket is reachable. @@ -555,10 +535,7 @@ impl PodmanComputeDriver { "Bridge network ready" ); - let container_lifecycle_lock = Arc::new(tokio::sync::Mutex::new(())); - let removed_probes = - remove_workspace_probes(&client, &container_lifecycle_lock, config.stop_timeout_secs) - .await?; + let removed_probes = remove_workspace_probes(&client, config.stop_timeout_secs).await?; if removed_probes > 0 { info!( count = removed_probes, @@ -580,7 +557,7 @@ impl PodmanComputeDriver { network_gateway_ip, rootless, rootless_network_cmd, - container_lifecycle_lock, + container_lifecycle_lock: Arc::new(tokio::sync::Mutex::new(())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( gpu_inventory, allow_all_default_gpu, @@ -882,8 +859,10 @@ impl PodmanComputeDriver { "podman image '{image}' inspection did not return an immutable image ID" ))); } + let resolved_image = + container::ResolvedPodmanImage::from_inspect(&inspected_image, &self.config)?; let workspace_identity = self - .validate_image_workspace(sandbox, &name, &inspected_image) + .validate_image_workspace(sandbox, &resolved_image) .await?; for image in container::podman_driver_image_mount_sources(sandbox, self.config.enable_bind_mounts) @@ -949,7 +928,7 @@ impl PodmanComputeDriver { token_secret_name.as_deref(), gpu_devices.as_deref(), image, - &inspected_image, + &resolved_image, workspace_identity.as_deref(), ) { Ok(spec) => spec, @@ -2312,6 +2291,26 @@ mod tests { } } + fn workspace_probe_sandbox(id: &str) -> DriverSandbox { + let mut sandbox = plain_sandbox(id, "demo"); + sandbox.spec = Some(DriverSandboxSpec::default()); + sandbox + } + + fn workspace_probe_image(config: &PodmanComputeConfig) -> container::ResolvedPodmanImage { + container::ResolvedPodmanImage::from_inspect( + &crate::client::ImageInspect { + id: "sha256:immutable".into(), + config: Some(crate::client::ImageConfig { + user: "1234:1235".into(), + working_dir: "/workspace".into(), + }), + }, + config, + ) + .unwrap() + } + #[tokio::test] async fn workspace_probe_waits_for_success_and_always_removes_container() { let (socket_path, request_log, handle) = spawn_podman_stub( @@ -2328,19 +2327,11 @@ mod tests { ], ); let driver = test_driver(socket_path.clone()); - let mut sandbox = plain_sandbox("sandbox-probe", "demo"); - sandbox.spec = Some(DriverSandboxSpec::default()); - let image = crate::client::ImageInspect { - id: "sha256:immutable".into(), - config: Some(crate::client::ImageConfig { - user: "1234:1235".into(), - working_dir: "/workspace".into(), - }), - }; - let name = validated_container_name(&sandbox).unwrap(); + let sandbox = workspace_probe_sandbox("sandbox-probe"); + let image = workspace_probe_image(&driver.config); let identity = driver - .validate_image_workspace(&sandbox, &name, &image) + .validate_image_workspace(&sandbox, &image) .await .expect("successful probe should pass"); assert_eq!(identity.as_deref(), Some("1234:1235:")); @@ -2372,19 +2363,11 @@ mod tests { ], ); let driver = test_driver(socket_path.clone()); - let mut sandbox = plain_sandbox("sandbox-probe-fail", "demo"); - sandbox.spec = Some(DriverSandboxSpec::default()); - let image = crate::client::ImageInspect { - id: "sha256:immutable".into(), - config: Some(crate::client::ImageConfig { - user: "1234:1235".into(), - working_dir: "/workspace".into(), - }), - }; - let name = validated_container_name(&sandbox).unwrap(); + let sandbox = workspace_probe_sandbox("sandbox-probe-fail"); + let image = workspace_probe_image(&driver.config); let error = driver - .validate_image_workspace(&sandbox, &name, &image) + .validate_image_workspace(&sandbox, &image) .await .unwrap_err(); assert!(error.to_string().contains("exited with code 1")); @@ -2413,22 +2396,11 @@ mod tests { ], ); let driver = test_driver(socket_path.clone()); - let mut sandbox = plain_sandbox("sandbox-probe-cancel", "demo"); - sandbox.spec = Some(DriverSandboxSpec::default()); - let image = crate::client::ImageInspect { - id: "sha256:immutable".into(), - config: Some(crate::client::ImageConfig { - user: "1234:1235".into(), - working_dir: "/workspace".into(), - }), - }; - let name = validated_container_name(&sandbox).unwrap(); + let sandbox = workspace_probe_sandbox("sandbox-probe-cancel"); + let image = workspace_probe_image(&driver.config); - let caller = tokio::spawn(async move { - driver - .validate_image_workspace(&sandbox, &name, &image) - .await - }); + let caller = + tokio::spawn(async move { driver.validate_image_workspace(&sandbox, &image).await }); tokio::time::timeout(Duration::from_secs(2), async { loop { if request_log @@ -2469,32 +2441,17 @@ mod tests { ], ); let driver = test_driver(socket_path.clone()); - let mut sandbox = plain_sandbox("sandbox-probe-timeout", "demo"); - sandbox.spec = Some(DriverSandboxSpec::default()); - let image = crate::client::ImageInspect { - id: "sha256:immutable".into(), - config: Some(crate::client::ImageConfig { - user: "1234:1235".into(), - working_dir: "/workspace".into(), - }), - }; - let name = validated_container_name(&sandbox).unwrap(); - let spec = container::build_workspace_probe_spec( - &sandbox, - &driver.config, - &image, - &workspace_probe_name(&name), - ) - .unwrap() - .unwrap(); + let sandbox = workspace_probe_sandbox("sandbox-probe-timeout"); + let image = workspace_probe_image(&driver.config); + let probe = container::build_workspace_probe_spec(&sandbox, &driver.config, &image) + .unwrap() + .unwrap(); let error = run_workspace_probe( driver.client.clone(), Arc::clone(&driver.container_lifecycle_lock), driver.config.stop_timeout_secs, - spec, - workspace_probe_name(&name), - image.id, + probe, Duration::from_millis(25), ) .await diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 39fa050b27..80377ee602 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -202,14 +202,20 @@ pub async fn run_sandbox( &mut policy, &driver_identity, )?; - if matches!( + let workspace_attestation = if matches!( &driver_identity, openshell_supervisor_process::identity::DriverIdentity::OciUser { .. } - ) && std::env::var_os(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) - .is_some_and(|value| !value.is_empty()) - { - let expected = std::env::var(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) - .map_err(|_| miette::miette!("Podman workspace identity attestation is missing"))?; + ) { + match std::env::var_os(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) { + Some(value) if !value.is_empty() => Some(value.into_string().map_err(|_| { + miette::miette!("Podman workspace identity attestation is not valid UTF-8") + })?), + _ => None, + } + } else { + None + }; + if let Some(expected) = workspace_attestation.as_deref() { let actual = openshell_supervisor_process::process::resolved_workspace_identity_attestation( &policy, resolved, @@ -225,13 +231,18 @@ pub async fn run_sandbox( openshell_supervisor_process::process::ResolvedWorkspace::new( workdir.clone(), use_workdir_as_home, + workspace_attestation.is_some(), ), ) }; #[cfg(not(unix))] let (resolved_process_identity, workspace) = ( openshell_supervisor_process::process::ResolvedProcessIdentity::default(), - openshell_supervisor_process::process::ResolvedWorkspace::new(workdir.clone(), false), + openshell_supervisor_process::process::ResolvedWorkspace::new( + workdir.clone(), + false, + false, + ), ); #[cfg_attr(not(target_os = "linux"), allow(unused_mut))] diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index ce6c1043ef..90d70bd82c 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -86,12 +86,17 @@ impl ResolvedProcessIdentity { pub struct ResolvedWorkspace { root: Option, use_as_home: bool, + prevalidated: bool, } impl ResolvedWorkspace { #[must_use] - pub fn new(root: Option, use_as_home: bool) -> Self { - Self { root, use_as_home } + pub fn new(root: Option, use_as_home: bool, prevalidated: bool) -> Self { + Self { + root, + use_as_home, + prevalidated, + } } #[must_use] @@ -108,6 +113,11 @@ impl ResolvedWorkspace { pub fn home(&self) -> Option<&str> { self.use_as_home.then(|| self.root()).flatten() } + + #[must_use] + pub const fn prevalidated(&self) -> bool { + self.prevalidated + } } impl ProcessEnforcementMode { @@ -1950,7 +1960,13 @@ fn chown_recursive( /// UIDs/GIDs (passed directly to `chown` without a passwd lookup). #[cfg(unix)] pub fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { - prepare_filesystem_with_identity(policy, ResolvedProcessIdentity::default(), None, false) + prepare_filesystem_with_identity( + policy, + ResolvedProcessIdentity::default(), + None, + false, + false, + ) } #[cfg(unix)] @@ -1959,6 +1975,7 @@ pub fn prepare_filesystem_with_identity( resolved_identity: ResolvedProcessIdentity, workdir: Option<&str>, prepare_workspace: bool, + workspace_prevalidated: bool, ) -> Result<()> { use nix::unistd::chown; @@ -1990,8 +2007,7 @@ pub fn prepare_filesystem_with_identity( })?; let workspace = Path::new(workspace); if workspace == Path::new(openshell_core::driver_mounts::DEFAULT_WORKSPACE_ROOT) - || std::env::var_os(openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY) - .is_some_and(|value| !value.is_empty()) + || workspace_prevalidated { info!(path = %workspace.display(), ?uid, ?gid, "Preparing managed workspace"); prepare_oci_workspace(workspace, uid, gid, &supplementary_gids)?; diff --git a/crates/openshell-supervisor-process/src/run.rs b/crates/openshell-supervisor-process/src/run.rs index 91e56b7ec8..9e375a3361 100644 --- a/crates/openshell-supervisor-process/src/run.rs +++ b/crates/openshell-supervisor-process/src/run.rs @@ -101,6 +101,7 @@ pub async fn run_process( resolved_process_identity, workspace.root(), workspace.home().is_some(), + workspace.prevalidated(), )?; } From 5348906b6898400aaa5d8e0bc0c76aa7633e3880 Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Tue, 4 Aug 2026 13:18:28 -0700 Subject: [PATCH 4/7] refactor(podman): clarify request execution flow Signed-off-by: Matthew Grossman --- crates/openshell-driver-podman/src/client.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index f7b3cc83a3..e43deed8cf 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -345,8 +345,8 @@ impl PodmanClient { builder.body(body).expect("valid request") } - /// Send a pre-built HTTP request for bounded or complete body collection. - async fn send_response( + /// Execute a pre-built HTTP request and return the streaming response. + async fn execute_request( &self, req: Request>, timeout: Duration, @@ -364,7 +364,7 @@ impl PodmanClient { req: Request>, timeout: Duration, ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { - let response = self.send_response(req, timeout).await?; + let response = self.execute_request(req, timeout).await?; let status = response.status(); let bytes = tokio::time::timeout(timeout, response.into_body().collect()) .await @@ -383,7 +383,7 @@ impl PodmanClient { ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { use hyper::body::Body; - let response = self.send_response(req, timeout).await?; + let response = self.execute_request(req, timeout).await?; let status = response.status(); let deadline = tokio::time::Instant::now() + timeout; let mut body = response.into_body(); From 2069f04785a9ae0742057684c25f479176e0902e Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Tue, 4 Aug 2026 17:01:37 -0700 Subject: [PATCH 5/7] fix(podman): recover ambiguous workspace probes Signed-off-by: Matthew Grossman --- .agents/skills/openshell-cli/SKILL.md | 7 ++ Cargo.lock | 1 + crates/openshell-driver-podman/Cargo.toml | 1 + .../openshell-driver-podman/src/container.rs | 23 ++++--- crates/openshell-driver-podman/src/driver.rs | 64 ++++++++++++++++--- 5 files changed, 78 insertions(+), 18 deletions(-) diff --git a/.agents/skills/openshell-cli/SKILL.md b/.agents/skills/openshell-cli/SKILL.md index 866e57008a..1b51187dde 100644 --- a/.agents/skills/openshell-cli/SKILL.md +++ b/.agents/skills/openshell-cli/SKILL.md @@ -446,6 +446,13 @@ field wins independently; omitted fields fall back to the image declaration. An image with no `USER` fails before readiness unless policy supplies both fields. +Docker and Podman gateways also honor the image's OCI `WORKDIR`. An empty +value, `/`, or `/sandbox` uses the compatibility workspace at `/sandbox`. +Any other value must be an absolute, normalized path that already exists in +the image, contains no symlink components, and is traversable and writable by +the sandbox UID, GID, and supplementary groups. Podman validates that access +against the pinned image before its managed workspace volume covers the path. + ### Forward ports ```bash diff --git a/Cargo.lock b/Cargo.lock index 9f3f7dcdca..c78e385148 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3818,6 +3818,7 @@ dependencies = [ "tracing", "tracing-subscriber", "url", + "uuid", ] [[package]] diff --git a/crates/openshell-driver-podman/Cargo.toml b/crates/openshell-driver-podman/Cargo.toml index e46d2eed85..0397f86238 100644 --- a/crates/openshell-driver-podman/Cargo.toml +++ b/crates/openshell-driver-podman/Cargo.toml @@ -35,6 +35,7 @@ tracing-subscriber = { workspace = true } thiserror = { workspace = true } miette = { workspace = true } url = { workspace = true } +uuid = { workspace = true } [dev-dependencies] prost-types = { workspace = true } diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 0e4da403ec..94424644ba 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -619,14 +619,8 @@ fn build_labels(sandbox: &DriverSandbox) -> BTreeMap { labels } -fn workspace_probe_name(sandbox: &DriverSandbox) -> String { - const SUFFIX: &str = "-workdir-probe"; - let container_name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); - let keep = 255usize.saturating_sub(SUFFIX.len()); - format!( - "{}{SUFFIX}", - &container_name[..container_name.len().min(keep)] - ) +fn workspace_probe_name() -> String { + format!("openshell-workdir-probe-{}", uuid::Uuid::new_v4().simple()) } /// Parse resource limits from the sandbox template, falling back to defaults. @@ -1341,7 +1335,9 @@ pub fn build_workspace_probe_spec( } let probe_spec = WorkspaceProbeSpec { - name: workspace_probe_name(sandbox), + // An attempt-unique name prevents an indeterminate create response + // from making a retry conflict with the first probe. + name: workspace_probe_name(), image: image.id.clone(), labels: BTreeMap::from([ (LABEL_WORKSPACE_PROBE.into(), "true".into()), @@ -1739,6 +1735,15 @@ mod tests { ); } + #[test] + fn workspace_probe_names_are_attempt_unique() { + let first = workspace_probe_name(); + let second = workspace_probe_name(); + + assert!(first.starts_with("openshell-workdir-probe-")); + assert_ne!(first, second); + } + #[test] fn workspace_probe_discovers_image_policy_identity_when_requested() { let mut sandbox = test_sandbox("test-id", "test-name"); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 69422083b0..8dd39058ff 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -289,15 +289,21 @@ async fn run_workspace_probe( let probe_name = probe.name().to_string(); let image_id = probe.image_id().to_string(); let create_body = probe.to_value(); - let mut created = false; + // Set before awaiting create because a timeout or transport error can + // leave us unable to tell whether Podman created the container. + let mut cleanup_required = false; let validation = tokio::time::timeout(timeout, async { { let _lifecycle_guard = lifecycle_lock.lock().await; - client - .create_container(&create_body) - .await - .map_err(ComputeDriverError::from)?; - created = true; + cleanup_required = true; + if let Err(error) = client.create_container(&create_body).await { + if matches!(error, PodmanApiError::Conflict(_)) { + // Do not remove a container that this attempt did not + // create, even though UUID name collisions are unlikely. + cleanup_required = false; + } + return Err(ComputeDriverError::from(error)); + } client .start_container(&probe_name) .await @@ -330,7 +336,7 @@ async fn run_workspace_probe( ))) }); - if !created { + if !cleanup_required { return validation; } @@ -340,7 +346,9 @@ async fn run_workspace_probe( .remove_container(&probe_name, stop_timeout_secs) .await }; - if let Err(cleanup_error) = cleanup { + if let Err(cleanup_error) = cleanup + && !matches!(cleanup_error, PodmanApiError::NotFound(_)) + { if validation.is_ok() { return Err(ComputeDriverError::from(cleanup_error)); } @@ -2458,7 +2466,7 @@ mod tests { .unwrap_err(); assert!(error.to_string().contains("timed out")); - tokio::time::timeout(Duration::from_secs(2), handle) + tokio::time::timeout(Duration::from_secs(10), handle) .await .expect("timed-out probe should be force-removed") .expect("stub task should finish"); @@ -2467,6 +2475,44 @@ mod tests { let _ = fs::remove_file(socket_path); } + #[tokio::test] + async fn workspace_probe_create_timeout_still_attempts_cleanup() { + let (socket_path, request_log, handle) = spawn_podman_stub( + "workspace-probe-create-timeout", + vec![ + StubResponse::new(StatusCode::CREATED, "{}").with_delay(Duration::from_millis(100)), + StubResponse::new(StatusCode::NO_CONTENT, ""), + ], + ); + let driver = test_driver(socket_path.clone()); + let sandbox = workspace_probe_sandbox("sandbox-probe-create-timeout"); + let image = workspace_probe_image(&driver.config); + let probe = container::build_workspace_probe_spec(&sandbox, &driver.config, &image) + .unwrap() + .unwrap(); + + let error = run_workspace_probe( + driver.client.clone(), + Arc::clone(&driver.container_lifecycle_lock), + driver.config.stop_timeout_secs, + probe, + Duration::from_millis(25), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("timed out")); + + tokio::time::timeout(Duration::from_secs(2), handle) + .await + .expect("ambiguous create should be reconciled") + .expect("stub task should finish"); + let requests = request_log.lock().unwrap(); + assert_eq!(requests.len(), 2); + assert!(requests[0].contains("/libpod/containers/create")); + assert!(requests[1].starts_with("DELETE ")); + let _ = fs::remove_file(socket_path); + } + fn secret_delete_request(sandbox_id: &str) -> String { format!( "DELETE {}", From f8c46286bf8d0e86bfc2326cc481c67586e38eca Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Wed, 5 Aug 2026 12:02:23 -0700 Subject: [PATCH 6/7] fix(podman): align workspace probe identity Signed-off-by: Matthew Grossman --- .../skills/debug-openshell-cluster/SKILL.md | 4 +- architecture/compute-runtimes.md | 7 +- crates/openshell-driver-podman/README.md | 11 +- .../openshell-driver-podman/src/container.rs | 82 +++++++----- crates/openshell-driver-podman/src/driver.rs | 99 ++------------- crates/openshell-server/src/compute/mod.rs | 118 +++++++++--------- crates/openshell-server/src/grpc/policy.rs | 85 +++++++++++++ crates/openshell-server/src/grpc/sandbox.rs | 41 +++--- docs/reference/sandbox-compute-drivers.mdx | 12 +- proto/compute_driver.proto | 20 ++- 10 files changed, 276 insertions(+), 203 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 4a9859b366..a07517c500 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -202,8 +202,8 @@ Common findings: - Rootless networking unavailable: inspect Podman network configuration. - Sandbox image missing or pull denied: verify image reference and registry credentials. - Sandbox fails before readiness with an identity-resolution error: inspect the image's OCI `USER` and matching `/etc/passwd` and `/etc/group` entries, or explicitly set both process identity fields in policy. Root and missing identities are rejected. -- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must be an absolute normalized directory with no symlink components; the final policy UID, primary GID, or supplementary groups must already be able to traverse every parent and write and enter the directory. Podman checks the original image in a networkless temporary probe before attaching the workspace volume, so inspect the probe failure in gateway logs. -- If Podman reports probe cleanup or timeout failures, inspect temporary containers with `podman ps -a --filter name=workdir-probe` and gateway logs. The driver force-removes the probe on every normal success or failure path. +- Sandbox fails before readiness with an OCI workspace validation error: inspect the image's `WorkingDir` using the immutable image ID reported by the gateway. Empty, `/`, and explicit `/sandbox` use the managed `/sandbox` compatibility workspace. Any other workdir must be an absolute normalized directory with no symlink components; the effective global-or-sandbox policy UID, primary GID, or supplementary groups must already be able to traverse every parent and write and enter the directory. Podman checks the original image in a networkless temporary probe before attaching the workspace volume, so inspect the probe failure in gateway logs. +- If Podman reports probe cleanup or timeout failures, inspect temporary containers with `podman ps -a --filter name=openshell-workdir-probe` and gateway logs. The driver force-removes the exact attempt-unique probe on every normal success or failure path. A gateway process crash can leave a stopped probe; verify its generated name and image before removing it manually. The driver deliberately does not sweep containers by a forgeable label or name prefix. - Supervisor cannot call back: check callback endpoint and gateway logs. - Gateway exits before becoming healthy with a callback-listener discovery error: inspect `podman info --debug`, the configured Podman network, and the diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index b043f883a0..16614d1f39 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -214,8 +214,11 @@ Docker performs the check in the final container before workload launch and rejects image `VOLUME` declarations that would mask the workdir ancestry. Podman performs it in a minimal networkless container from the same pinned image ID before its managed workspace volume covers the path. The probe adopts -the completed process identity and emits an attestation that the final -supervisor must match. The resolved workspace is the child cwd and `HOME`; +the identity source from the effective global-or-sandbox policy, or discovers +the image policy when neither exists, and emits a normalized attestation that +the final supervisor must match. This internal Podman-only contract is required +because the managed volume hides the original image tree before the final +supervisor starts. The resolved workspace is the child cwd and `HOME`; when `filesystem.include_workdir` is enabled, it becomes the automatic writable policy path. Kubernetes/OpenShift keep their `/sandbox` PVC and `fsGroup` behavior, and VM keeps its `/sandbox` guest diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 54391f402a..3b9b9ecf19 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -24,7 +24,9 @@ resource-limited, networkless probe verifies the original pinned image before Podman covers the path with the managed workspace volume. The completed process identity must already be able to traverse every parent and write and enter the directory, without symlink components or OpenShell control-path collisions. -The final supervisor must match the probe's identity before preparing the +The gateway supplies the identity source from the effective global-or-sandbox +policy, or requests image-policy discovery when neither exists. The final +supervisor must match the probe's normalized identity before preparing the volume. See [Compute runtimes](../../architecture/compute-runtimes.md#process-identity) for the invariant and probe lifecycle. @@ -304,6 +306,13 @@ sequenceDiagram D->>P: pull_image(supervisor, "missing") D->>P: pull_image(sandbox_image, policy) + D->>P: inspect_image(sandbox_image) + + opt Non-default OCI workdir + D->>P: create + start validation probe + D->>P: wait + read bounded logs + D->>P: force-remove exact probe name + end D->>P: create_volume(workspace) Note over D: On failure below, rollback volume diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 94424644ba..e3bec033bf 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -9,7 +9,9 @@ use openshell_core::ComputeDriverError; use openshell_core::driver_mounts::SelinuxLabel; #[cfg(test)] use openshell_core::gpu::{driver_gpu_requirements, validate_specific_gpu_device_request}; -use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate}; +use openshell_core::proto::compute::v1::{ + DriverSandbox, DriverSandboxTemplate, workspace_validation_identity, +}; use openshell_core::proto_struct::deserialize_optional_non_empty_string_list; use openshell_core::{driver_mounts, proto_struct}; use serde::Serialize; @@ -45,10 +47,6 @@ pub use openshell_core::driver_utils::{ pub const LABEL_MANAGED: &str = "openshell.managed"; /// Label filter string for list/event queries. pub const LABEL_MANAGED_FILTER: &str = "openshell.managed=true"; -/// Label applied only to temporary OCI workdir validation probes. -pub const LABEL_WORKSPACE_PROBE: &str = "openshell.workspace-probe"; -/// Label filter used to recover probes left behind by a stopped gateway. -pub const LABEL_WORKSPACE_PROBE_FILTER: &str = "openshell.workspace-probe=true"; /// Container name prefix to avoid collisions with user containers. const CONTAINER_PREFIX: &str = "openshell-"; @@ -276,12 +274,11 @@ struct ContainerSpec { /// /// Keep this typed separately from [`ContainerSpec`] so probes cannot /// accidentally inherit sandbox networking, secrets, or workspace mounts, -/// while still sharing resource-limit construction and recovery metadata. +/// while still sharing resource-limit construction. #[derive(Serialize)] pub struct WorkspaceProbeSpec { name: String, image: String, - labels: BTreeMap, entrypoint: Vec, command: Vec, user: String, @@ -1319,17 +1316,26 @@ pub fn build_workspace_probe_spec( "--oci-user".to_string(), image.oci_user.clone(), ]; - if let Some(identity) = &spec.workspace_validation_identity { - for (flag, value) in [ - ("--run-as-user", identity.run_as_user.as_str()), - ("--run-as-group", identity.run_as_group.as_str()), - ] { - if !value.is_empty() { - command.push(flag.to_string()); - command.push(value.to_string()); + let identity = spec.workspace_validation_identity.as_ref().ok_or_else(|| { + ComputeDriverError::Precondition( + "workspace_validation_identity is required for a non-default OCI workdir".into(), + ) + })?; + match identity.source.as_ref().ok_or_else(|| { + ComputeDriverError::Precondition("workspace_validation_identity.source is required".into()) + })? { + workspace_validation_identity::Source::Policy(policy) => { + for (flag, value) in [ + ("--run-as-user", policy.run_as_user.as_str()), + ("--run-as-group", policy.run_as_group.as_str()), + ] { + if !value.is_empty() { + command.push(flag.to_string()); + command.push(value.to_string()); + } } } - if identity.discover_from_image_policy { + workspace_validation_identity::Source::Image(_) => { command.push("--discover-policy-identity".to_string()); } } @@ -1339,10 +1345,6 @@ pub fn build_workspace_probe_spec( // from making a retry conflict with the first probe. name: workspace_probe_name(), image: image.id.clone(), - labels: BTreeMap::from([ - (LABEL_WORKSPACE_PROBE.into(), "true".into()), - (LABEL_SANDBOX_ID.into(), sandbox.id.clone()), - ]), entrypoint: vec![SUPERVISOR_BINARY_PATH.into()], command, user: "0:0".into(), @@ -1441,7 +1443,9 @@ fn parse_memory_to_bytes(quantity: &str) -> Option { mod tests { use super::*; use crate::client::ImageConfig; - use openshell_core::proto::compute::v1::{GpuResourceRequirements, ResourceRequirements}; + use openshell_core::proto::compute::v1::{ + DriverSandboxSpec, GpuResourceRequirements, ResourceRequirements, + }; static ENV_LOCK: std::sync::LazyLock> = std::sync::LazyLock::new(|| std::sync::Mutex::new(())); @@ -1686,9 +1690,12 @@ mod tests { ); spec.workspace_validation_identity = Some( openshell_core::proto::compute::v1::WorkspaceValidationIdentity { - run_as_user: "policy-user".into(), - run_as_group: "policy-group".into(), - discover_from_image_policy: false, + source: Some(workspace_validation_identity::Source::Policy( + openshell_core::proto::compute::v1::PolicyProcessIdentity { + run_as_user: "policy-user".into(), + run_as_group: "policy-group".into(), + }, + )), }, ); let mut config = test_config(); @@ -1707,9 +1714,7 @@ mod tests { assert_eq!(probe["image_volume_mode"], "ignore"); assert_eq!(probe["cap_drop"], serde_json::json!(["ALL"])); assert_eq!(probe["cap_add"], serde_json::json!(["SETUID", "SETGID"])); - assert_eq!(probe["labels"][LABEL_WORKSPACE_PROBE], "true"); - assert_eq!(probe["labels"][LABEL_SANDBOX_ID], "test-id"); - assert!(probe["labels"].get(LABEL_MANAGED).is_none()); + assert!(probe.get("labels").is_none()); assert_eq!(probe["resource_limits"]["cpu"]["quota"], 75_000); assert_eq!( probe["resource_limits"]["memory"]["limit"], @@ -1752,8 +1757,9 @@ mod tests { .get_or_insert_default() .workspace_validation_identity = Some( openshell_core::proto::compute::v1::WorkspaceValidationIdentity { - discover_from_image_policy: true, - ..Default::default() + source: Some(workspace_validation_identity::Source::Image( + openshell_core::proto::compute::v1::ImagePolicyDiscovery {}, + )), }, ); @@ -1779,6 +1785,22 @@ mod tests { ); } + #[test] + fn workspace_probe_requires_an_explicit_identity_source() { + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec::default()); + + let Err(error) = build_workspace_probe_spec( + &sandbox, + &test_config(), + &resolved_image("sha256:immutable", "app:staff", "/home/app/project"), + ) else { + panic!("non-default workdir should require an identity source"); + }; + + assert!(error.to_string().contains("workspace_validation_identity")); + } + #[test] fn workspace_probe_skips_sandbox_compatibility_fallback() { let probe = build_workspace_probe_spec( @@ -1793,7 +1815,7 @@ mod tests { #[test] fn container_spec_reserves_resolved_workspace_root_but_allows_nested_mounts() { let mut sandbox = test_sandbox("test-id", "test-name"); - sandbox.spec = Some(openshell_core::proto::compute::v1::DriverSandboxSpec { + sandbox.spec = Some(DriverSandboxSpec { template: Some(DriverSandboxTemplate::default()), ..Default::default() }); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 8dd39058ff..414b50f3f8 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -5,10 +5,7 @@ use crate::client::{PodmanApiError, PodmanClient, VolumeInspect}; use crate::config::PodmanComputeConfig; -use crate::container::{ - self, LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID, LABEL_WORKSPACE_PROBE_FILTER, - PodmanSandboxDriverConfig, -}; +use crate::container::{self, LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID, PodmanSandboxDriverConfig}; use crate::watcher::{ self, WatchStream, driver_sandbox_from_inspect, driver_sandbox_from_list_entry, }; @@ -261,24 +258,6 @@ fn sanitized_probe_diagnostic(logs: &str) -> String { } } -async fn remove_workspace_probes( - client: &PodmanClient, - stop_timeout_secs: u32, -) -> Result { - let probes = client - .list_containers(&[LABEL_WORKSPACE_PROBE_FILTER]) - .await?; - let mut removed = 0; - for probe in probes { - match client.remove_container(&probe.id, stop_timeout_secs).await { - Ok(()) => removed += 1, - Err(PodmanApiError::NotFound(_)) => {} - Err(error) => return Err(error), - } - } - Ok(removed) -} - async fn run_workspace_probe( client: PodmanClient, lifecycle_lock: Arc>, @@ -543,14 +522,6 @@ impl PodmanComputeDriver { "Bridge network ready" ); - let removed_probes = remove_workspace_probes(&client, config.stop_timeout_secs).await?; - if removed_probes > 0 { - info!( - count = removed_probes, - "Removed workspace validation probes left by a previous gateway process" - ); - } - let (gpu_inventory, allow_all_default_gpu) = local_podman_gpu_selector_state(); if !gpu_inventory.is_empty() { info!( @@ -1285,7 +1256,8 @@ mod tests { use crate::test_utils::{StubResponse, spawn_podman_stub}; use hyper::StatusCode; use openshell_core::proto::compute::v1::{ - DriverSandboxSpec, DriverSandboxTemplate, ResourceRequirements, + DriverSandboxSpec, DriverSandboxTemplate, ImagePolicyDiscovery, ResourceRequirements, + WorkspaceValidationIdentity, workspace_validation_identity, }; use std::collections::HashMap; use std::fs; @@ -1666,7 +1638,6 @@ mod tests { }"#, ), StubResponse::new(StatusCode::CREATED, "{}"), - StubResponse::new(StatusCode::OK, "[]"), ], ); let config = PodmanComputeConfig { @@ -1686,66 +1657,13 @@ mod tests { .lock() .expect("request log lock should not be poisoned"); assert_eq!( - &requests[..3], + requests.as_slice(), [ "GET /_ping".to_string(), format!("GET {}", api_path("/libpod/info")), format!("POST {}", api_path("/libpod/networks/create")), ] ); - assert!(requests[3].contains("/libpod/containers/json?all=true&filters=")); - } - - #[tokio::test] - async fn constructor_removes_workspace_probes_left_by_previous_process() { - let (socket_path, request_log, handle) = spawn_podman_stub( - "stale-workspace-probe", - vec![ - StubResponse::new(StatusCode::OK, ""), - StubResponse::new( - StatusCode::OK, - r#"{ - "host": { - "cgroupVersion": "v2", - "networkBackend": "netavark", - "security": {"rootless": false} - } - }"#, - ), - StubResponse::new(StatusCode::CREATED, "{}"), - StubResponse::new( - StatusCode::OK, - r#"[{ - "Id": "stale-probe-id", - "Names": ["openshell-stale-workdir-probe"], - "State": "exited", - "Labels": {"openshell.workspace-probe": "true"} - }]"#, - ), - StubResponse::new(StatusCode::NO_CONTENT, ""), - ], - ); - let config = PodmanComputeConfig { - socket_path: Some(socket_path.clone()), - grpc_endpoint: "https://gateway.example.test:9443".to_string(), - ..PodmanComputeConfig::default() - }; - - PodmanComputeDriver::new(config) - .await - .expect("startup should remove a stale workspace probe"); - - handle.await.expect("stub task should finish"); - let requests = request_log.lock().unwrap(); - assert!(requests[3].contains("/libpod/containers/json?all=true&filters=")); - assert!(requests[3].contains("openshell.workspace-probe")); - assert_eq!( - requests[4], - format!( - "DELETE {}?force=true&volumes=true&timeout=45", - api_path("/libpod/containers/stale-probe-id") - ) - ); } #[test] @@ -2301,7 +2219,14 @@ mod tests { fn workspace_probe_sandbox(id: &str) -> DriverSandbox { let mut sandbox = plain_sandbox(id, "demo"); - sandbox.spec = Some(DriverSandboxSpec::default()); + sandbox.spec = Some(DriverSandboxSpec { + workspace_validation_identity: Some(WorkspaceValidationIdentity { + source: Some(workspace_validation_identity::Source::Image( + ImagePolicyDiscovery {}, + )), + }), + ..Default::default() + }); sandbox } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 070e6563dd..5d7ec24732 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -33,9 +33,9 @@ use openshell_core::proto::compute::v1::{ GetGatewayListenerRequirementsResponse, GetSandboxRequest, GpuResourceRequirements as DriverGpuResourceRequirements, ListSandboxesRequest, ResourceRequirements as DriverSandboxResourceRequirements, ValidateSandboxCreateRequest, - WatchSandboxesEvent, WatchSandboxesRequest, compute_driver_client::ComputeDriverClient, - compute_driver_server::ComputeDriver, gateway_listener_requirement::Selector, - watch_sandboxes_event, + WatchSandboxesEvent, WatchSandboxesRequest, WorkspaceValidationIdentity, + compute_driver_client::ComputeDriverClient, compute_driver_server::ComputeDriver, + gateway_listener_requirement::Selector, watch_sandboxes_event, }; use openshell_core::proto::{ PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxSpec, SandboxStatus, @@ -840,9 +840,17 @@ impl ComputeRuntime { &self.gateway_listener_requirements } - pub async fn validate_sandbox_create(&self, sandbox: &Sandbox) -> Result<(), Status> { - let driver_sandbox = driver_sandbox_from_public(sandbox, &self.driver_info.name) - .map_err(|status| *status)?; + pub async fn validate_sandbox_create( + &self, + sandbox: &Sandbox, + workspace_validation_identity: Option, + ) -> Result<(), Status> { + let driver_sandbox = driver_sandbox_from_public( + sandbox, + &self.driver_info.name, + workspace_validation_identity, + ) + .map_err(|status| *status)?; self.driver .call( "driver.validate_sandbox_create", @@ -863,10 +871,15 @@ impl ComputeRuntime { &self, sandbox: Sandbox, sandbox_token: Option, + workspace_validation_identity: Option, ) -> Result { let sandbox_id = sandbox.object_id().to_string(); - let mut driver_sandbox = driver_sandbox_from_public(&sandbox, &self.driver_info.name) - .map_err(|status| *status)?; + let mut driver_sandbox = driver_sandbox_from_public( + &sandbox, + &self.driver_info.name, + workspace_validation_identity, + ) + .map_err(|status| *status)?; // Create with MustCreate condition to prevent duplicate creation race self.sandbox_index.update_from_sandbox(&sandbox); @@ -2496,6 +2509,7 @@ pub async fn connect_remote_compute_driver( fn driver_sandbox_from_public( sandbox: &Sandbox, driver_name: &str, + workspace_validation_identity: Option, ) -> Result> { Ok(DriverSandbox { id: sandbox.object_id().to_string(), @@ -2504,7 +2518,9 @@ fn driver_sandbox_from_public( spec: sandbox .spec .as_ref() - .map(|spec| driver_sandbox_spec_from_public(spec, driver_name)) + .map(|spec| { + driver_sandbox_spec_from_public(spec, driver_name, workspace_validation_identity) + }) .transpose()?, status: sandbox.status.as_ref().map(driver_status_from_public), workspace: sandbox.object_workspace().to_string(), @@ -2514,18 +2530,8 @@ fn driver_sandbox_from_public( fn driver_sandbox_spec_from_public( spec: &SandboxSpec, driver_name: &str, + workspace_validation_identity: Option, ) -> Result> { - let workspace_validation_identity = (driver_name == "podman").then(|| { - let process = spec - .policy - .as_ref() - .and_then(|policy| policy.process.as_ref()); - openshell_core::proto::compute::v1::WorkspaceValidationIdentity { - run_as_user: process.map_or_else(String::new, |process| process.run_as_user.clone()), - run_as_group: process.map_or_else(String::new, |process| process.run_as_group.clone()), - discover_from_image_policy: spec.policy.is_none(), - } - }); Ok(DriverSandboxSpec { log_level: spec.log_level.clone(), environment: spec.environment.clone(), @@ -2543,7 +2549,11 @@ fn driver_sandbox_spec_from_public( } }), sandbox_token: String::new(), - workspace_validation_identity, + workspace_validation_identity: if driver_name == "podman" { + workspace_validation_identity + } else { + None + }, }) } @@ -3284,7 +3294,7 @@ mod tests { ..Default::default() }; - let driver = driver_sandbox_spec_from_public(&public, "test-driver") + let driver = driver_sandbox_spec_from_public(&public, "test-driver", None) .expect("driver spec should map"); let gpu = driver @@ -3296,35 +3306,26 @@ mod tests { } #[test] - fn driver_sandbox_spec_projects_process_identity_only_for_podman_probe() { - let public = SandboxSpec { - policy: Some(openshell_core::proto::SandboxPolicy { - process: Some(openshell_core::proto::ProcessPolicy { + fn driver_sandbox_spec_forwards_workspace_identity_only_to_podman() { + use openshell_core::proto::compute::v1::{ + PolicyProcessIdentity, workspace_validation_identity, + }; + + let public = SandboxSpec::default(); + let identity = WorkspaceValidationIdentity { + source: Some(workspace_validation_identity::Source::Policy( + PolicyProcessIdentity { run_as_user: "app".into(), run_as_group: "staff".into(), - }), - ..Default::default() - }), - ..Default::default() + }, + )), }; - let driver = - driver_sandbox_spec_from_public(&public, "podman").expect("driver spec should map"); - let identity = driver.workspace_validation_identity.unwrap(); - assert_eq!(identity.run_as_user, "app"); - assert_eq!(identity.run_as_group, "staff"); - assert!(!identity.discover_from_image_policy); - - let without_policy = - driver_sandbox_spec_from_public(&SandboxSpec::default(), "podman").unwrap(); - assert!( - without_policy - .workspace_validation_identity - .unwrap() - .discover_from_image_policy - ); + let podman = driver_sandbox_spec_from_public(&public, "podman", Some(identity.clone())) + .expect("driver spec should map"); + assert_eq!(podman.workspace_validation_identity, Some(identity.clone())); - let docker = driver_sandbox_spec_from_public(&public, "docker").unwrap(); + let docker = driver_sandbox_spec_from_public(&public, "docker", Some(identity)).unwrap(); assert!(docker.workspace_validation_identity.is_none()); } @@ -4364,7 +4365,7 @@ mod tests { let traced = test_exporter::install_traced(); async { runtime - .create_sandbox(sandbox, None) + .create_sandbox(sandbox, None, None) .await .expect("create succeeds"); } @@ -4488,7 +4489,7 @@ mod tests { let traced = test_exporter::install_traced(); async { runtime - .create_sandbox(sandbox, None) + .create_sandbox(sandbox, None, None) .await .expect_err("driver refuses the create"); } @@ -4703,7 +4704,7 @@ mod tests { ..Default::default() }); - runtime.create_sandbox(sandbox, None).await.unwrap(); + runtime.create_sandbox(sandbox, None, None).await.unwrap(); runtime .apply_sandbox_update(ready_driver_sandbox("sb-1", "sandbox-a")) .await @@ -7003,8 +7004,11 @@ mod tests { ..Default::default() }); - runtime.validate_sandbox_create(&sandbox).await.unwrap(); - runtime.create_sandbox(sandbox, None).await.unwrap(); + runtime + .validate_sandbox_create(&sandbox, None) + .await + .unwrap(); + runtime.create_sandbox(sandbox, None, None).await.unwrap(); assert!( runtime .delete_sandbox("default", "uds-sandbox") @@ -7113,7 +7117,7 @@ mod tests { deletion_timestamp_ms: 0, }); - let created = runtime.create_sandbox(sandbox, None).await.unwrap(); + let created = runtime.create_sandbox(sandbox, None, None).await.unwrap(); assert_eq!( created.metadata.as_ref().unwrap().resource_version, @@ -7148,7 +7152,7 @@ mod tests { .labels .insert("env".to_string(), "prod".to_string()); - runtime.create_sandbox(sandbox, None).await.unwrap(); + runtime.create_sandbox(sandbox, None, None).await.unwrap(); let matching = runtime .store @@ -7172,11 +7176,13 @@ mod tests { // Spawn two concurrent creation attempts for the same sandbox let runtime1 = runtime.clone(); let sandbox1 = sandbox.clone(); - let handle1 = tokio::spawn(async move { runtime1.create_sandbox(sandbox1, None).await }); + let handle1 = + tokio::spawn(async move { runtime1.create_sandbox(sandbox1, None, None).await }); let runtime2 = runtime.clone(); let sandbox2 = sandbox.clone(); - let handle2 = tokio::spawn(async move { runtime2.create_sandbox(sandbox2, None).await }); + let handle2 = + tokio::spawn(async move { runtime2.create_sandbox(sandbox2, None, None).await }); // Wait for both to complete let result1 = handle1.await.unwrap(); @@ -7234,7 +7240,7 @@ mod tests { }), ..Default::default() }; - let driver_sb = driver_sandbox_from_public(&sandbox, "kubernetes").unwrap(); + let driver_sb = driver_sandbox_from_public(&sandbox, "kubernetes", None).unwrap(); assert_eq!(driver_sb.workspace, "alpha"); assert_eq!(driver_sb.name, "work"); assert_eq!(driver_sb.id, "sb-1"); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 5c5faae8a6..1b5f653496 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -23,6 +23,10 @@ use crate::provider_profile_sources::EffectiveProviderProfileCatalog; #[cfg(test)] use crate::provider_profile_sources::ProviderProfileSources; use openshell_core::net::{is_always_blocked_ip, is_internal_ip}; +use openshell_core::proto::compute::v1::{ + ImagePolicyDiscovery, PolicyProcessIdentity, WorkspaceValidationIdentity, + workspace_validation_identity, +}; use openshell_core::proto::policy_merge_operation; use openshell_core::proto::setting_value; use openshell_core::proto::{ @@ -4826,6 +4830,37 @@ fn decode_policy_from_global_settings( Ok(Some(policy)) } +fn workspace_validation_identity_from_policy( + policy: Option<&ProtoSandboxPolicy>, +) -> WorkspaceValidationIdentity { + let source = policy.map_or( + workspace_validation_identity::Source::Image(ImagePolicyDiscovery {}), + |policy| { + let process = policy.process.as_ref(); + workspace_validation_identity::Source::Policy(PolicyProcessIdentity { + run_as_user: process.map_or_else(String::new, |p| p.run_as_user.clone()), + run_as_group: process.map_or_else(String::new, |p| p.run_as_group.clone()), + }) + }, + ); + WorkspaceValidationIdentity { + source: Some(source), + } +} + +/// Resolve the process-identity source used by a new Podman workspace probe. +/// Global policy has the same precedence here as it does in `GetSandboxConfig`. +pub(super) async fn workspace_validation_identity_for_create( + state: &ServerState, + sandbox_policy: Option<&ProtoSandboxPolicy>, +) -> Result { + let global_settings = load_global_settings(state.store.as_ref()).await?; + let global_policy = decode_policy_from_global_settings(&global_settings)?; + Ok(workspace_validation_identity_from_policy( + global_policy.as_ref().or(sandbox_policy), + )) +} + fn merge_effective_settings( global: &StoredSettings, sandbox: &StoredSettings, @@ -11809,6 +11844,56 @@ mod tests { assert_eq!(decoded.version, 7); } + #[test] + fn workspace_validation_uses_image_identity_without_an_effective_policy() { + let identity = workspace_validation_identity_from_policy(None); + assert!(matches!( + identity.source, + Some(workspace_validation_identity::Source::Image(_)) + )); + } + + #[tokio::test] + async fn workspace_validation_uses_global_policy_identity() { + let state = test_server_state().await; + let global_policy = ProtoSandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "2000".into(), + run_as_group: "2000".into(), + }), + ..Default::default() + }; + let global_settings = StoredSettings { + revision: 1, + settings: std::iter::once(( + POLICY_SETTING_KEY.to_string(), + StoredSettingValue::Bytes(hex::encode(global_policy.encode_to_vec())), + )) + .collect(), + ..Default::default() + }; + save_global_settings(state.store.as_ref(), &global_settings) + .await + .unwrap(); + + let sandbox_policy = ProtoSandboxPolicy { + process: Some(openshell_core::proto::ProcessPolicy { + run_as_user: "1000".into(), + run_as_group: "1000".into(), + }), + ..Default::default() + }; + let identity = workspace_validation_identity_for_create(&state, Some(&sandbox_policy)) + .await + .unwrap(); + + let Some(workspace_validation_identity::Source::Policy(process)) = identity.source else { + panic!("global policy should supply the workspace-validation identity"); + }; + assert_eq!(process.run_as_user, "2000"); + assert_eq!(process.run_as_group, "2000"); + } + #[test] fn config_revision_changes_when_effective_setting_changes() { let policy = ProtoSandboxPolicy::default(); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index dafd3e6e36..3c0e704984 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -30,7 +30,7 @@ use openshell_core::telemetry::{ LifecycleOperation, LifecycleResource, SandboxTemplateSource, TelemetryComputeDriver, TelemetryOutcome, }; -use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; +use openshell_core::{ComputeDriverKind, ObjectId, ObjectName, ObjectWorkspace}; use prost::Message; use std::collections::HashMap; use std::net::IpAddr; @@ -203,9 +203,7 @@ fn emit_sandbox_create_telemetry( ); } -fn telemetry_compute_driver( - driver_kind: Option, -) -> TelemetryComputeDriver { +fn telemetry_compute_driver(driver_kind: Option) -> TelemetryComputeDriver { TelemetryComputeDriver::from_driver_kind(driver_kind) } @@ -275,6 +273,19 @@ async fn handle_create_sandbox_inner( crate::middleware::validate_policy(state.middleware_registry.as_ref(), policy).await?; } + let workspace_validation_identity = + if matches!(state.compute.driver_kind(), Some(ComputeDriverKind::Podman)) { + Some( + super::policy::workspace_validation_identity_for_create( + state, + spec.policy.as_ref(), + ) + .await?, + ) + } else { + None + }; + let id = uuid::Uuid::new_v4().to_string(); let name = if request.name.is_empty() { generate_routable_name() @@ -305,7 +316,7 @@ async fn handle_create_sandbox_inner( state .compute - .validate_sandbox_create(&sandbox) + .validate_sandbox_create(&sandbox, workspace_validation_identity.clone()) .await .map_err(|status| { warn!(error = %status, "Rejecting sandbox create request"); @@ -332,7 +343,10 @@ async fn handle_create_sandbox_inner( None => None, }; - let sandbox = state.compute.create_sandbox(sandbox, sandbox_token).await?; + let sandbox = state + .compute + .create_sandbox(sandbox, sandbox_token, workspace_validation_identity) + .await?; info!( sandbox_id = %id, @@ -2282,19 +2296,19 @@ mod tests { #[test] fn telemetry_compute_driver_uses_resolved_driver_kind() { assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Docker)), + telemetry_compute_driver(Some(ComputeDriverKind::Docker)), TelemetryComputeDriver::Docker ); assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Kubernetes)), + telemetry_compute_driver(Some(ComputeDriverKind::Kubernetes)), TelemetryComputeDriver::Kubernetes ); assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Podman)), + telemetry_compute_driver(Some(ComputeDriverKind::Podman)), TelemetryComputeDriver::Podman ); assert_eq!( - telemetry_compute_driver(Some(openshell_core::ComputeDriverKind::Vm)), + telemetry_compute_driver(Some(ComputeDriverKind::Vm)), TelemetryComputeDriver::Vm ); assert_eq!( @@ -3184,8 +3198,7 @@ mod tests { #[tokio::test] async fn create_and_get_preserve_partial_process_identity() { - let state = - test_server_state_with_driver(openshell_core::ComputeDriverKind::Docker.as_str()).await; + let state = test_server_state_with_driver(ComputeDriverKind::Docker.as_str()).await; let policy = openshell_core::proto::SandboxPolicy { version: 1, process: Some(openshell_core::proto::ProcessPolicy { @@ -3248,9 +3261,7 @@ mod tests { #[tokio::test] async fn create_and_get_restore_legacy_identity_defaults_for_non_local_driver() { - let state = - test_server_state_with_driver(openshell_core::ComputeDriverKind::Kubernetes.as_str()) - .await; + let state = test_server_state_with_driver(ComputeDriverKind::Kubernetes.as_str()).await; let policy = openshell_core::proto::SandboxPolicy { version: 1, process: Some(openshell_core::proto::ProcessPolicy { diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 593fdee8cd..1e23e09611 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -461,11 +461,13 @@ the original image filesystem in the final supervisor and rejects image validation. Podman checks the same pinned image ID in a temporary networkless probe without the workspace volume, token, or TLS secrets. The probe retains only `SETUID` and `SETGID`, drops to -the completed process identity, and uses the kernel to validate access. The -final supervisor must match the probe's normalized identity attestation, -including when the image supplies the default process policy. Podman captures -a bounded, sanitized failure diagnostic, removes the probe, and only then -creates the final volume-backed sandbox. The resolved workspace is the cwd and +the identity selected by the effective global-or-sandbox policy, and uses the +kernel to validate access. When neither policy exists, it discovers the image +policy identity. The final supervisor must match the probe's normalized +identity attestation. Podman captures a bounded, sanitized failure diagnostic, +removes the exact probe it created, and only then creates the final +volume-backed sandbox. Probe names are unique to each attempt, so an ambiguous +create response cannot poison a retry. The resolved workspace is the cwd and `HOME` for direct and SSH children. The supervisor itself starts from `/`, so a missing or invalid workspace is handled during readiness instead of preventing the container runtime from starting it. diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index fd3e58554c..6aaf34180e 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -132,19 +132,29 @@ message DriverSandboxSpec { // ServiceAccount token bootstrap instead). Never echoed to the public // Sandbox proto. string sandbox_token = 11 [(openshell.options.v1.secret) = true]; - // Final process-identity inputs needed by local container drivers when they - // validate an OCI image workspace before launching the supervisor. + // Effective process-identity source needed by local container drivers when + // they validate an OCI image workspace before launching the supervisor. WorkspaceValidationIdentity workspace_validation_identity = 12; } message WorkspaceValidationIdentity { + oneof source { + // A sandbox or global policy is authoritative. Empty fields retain the + // OCI USER component for that field. + PolicyProcessIdentity policy = 1; + // No effective policy exists, so discover an image-provided policy before + // resolving the final process identity. + ImagePolicyDiscovery image = 2; + } +} + +message PolicyProcessIdentity { string run_as_user = 1; string run_as_group = 2; - // No public policy was supplied, so the supervisor must discover an - // image-provided policy before resolving the final process identity. - bool discover_from_image_policy = 3; } +message ImagePolicyDiscovery {} + message ResourceRequirements { // GPU requirements for the sandbox. Presence indicates a GPU request. GpuResourceRequirements gpu = 1; From fdf4a19e6fd1b82bfe5a6b8b286e6e8cca433edb Mon Sep 17 00:00:00 2001 From: Matthew Grossman Date: Wed, 5 Aug 2026 12:11:44 -0700 Subject: [PATCH 7/7] fix(podman): reject masking image volumes Signed-off-by: Matthew Grossman --- architecture/compute-runtimes.md | 18 +++++----- crates/openshell-driver-docker/src/lib.rs | 4 +-- crates/openshell-driver-podman/src/client.rs | 13 +++---- .../openshell-driver-podman/src/container.rs | 34 +++++++++++++++++++ crates/openshell-driver-podman/src/driver.rs | 1 + docs/reference/sandbox-compute-drivers.mdx | 12 +++---- 6 files changed, 56 insertions(+), 26 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 16614d1f39..846df471a0 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -210,15 +210,15 @@ uses kernel effective-access checks so POSIX ACL and LSM decisions are honored. Path checks reserve the standard OCI runtime namespaces under `/proc`, `/sys`, and `/dev`, while separate collision checks are derived from actual OpenShell control paths. -Docker performs the check in the final container before workload launch and -rejects image `VOLUME` declarations that would mask the workdir ancestry. -Podman performs it in a minimal networkless container from the same pinned -image ID before its managed workspace volume covers the path. The probe adopts -the identity source from the effective global-or-sandbox policy, or discovers -the image policy when neither exists, and emits a normalized attestation that -the final supervisor must match. This internal Podman-only contract is required -because the managed volume hides the original image tree before the final -supervisor starts. The resolved workspace is the child cwd and `HOME`; +Both drivers reject image `VOLUME` declarations that would mask the workdir +ancestry. Docker performs the check in the final container before workload +launch. Podman performs it in a minimal networkless container from the same +pinned image ID before its managed workspace volume covers the path. The probe +adopts the identity source from the effective global-or-sandbox policy, or +discovers the image policy when neither exists, and emits a normalized +attestation that the final supervisor must match. This internal Podman-only +contract is required because the managed volume hides the original image tree +before the final supervisor starts. The resolved workspace is the child cwd and `HOME`; when `filesystem.include_workdir` is enabled, it becomes the automatic writable policy path. Kubernetes/OpenShift keep their `/sandbox` PVC and `fsGroup` behavior, and VM keeps its `/sandbox` guest diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index d1c096e0e9..0d139ebf29 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -2251,9 +2251,7 @@ fn build_environment_for_oci_user( environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN); environment.remove(openshell_core::sandbox_env::SANDBOX_TOKEN_FILE); - // Docker never uses the Podman prevalidation contract. Emit an explicit - // driver-owned empty value so image-baked ENV entries cannot select the - // managed-workspace mutation path for an image-provided workdir. + // Ignore image-provided Podman probe attestations. environment.insert( openshell_core::sandbox_env::OCI_WORKSPACE_IDENTITY.to_string(), String::new(), diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index e43deed8cf..c1d2d4ee85 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -190,6 +190,8 @@ pub struct ImageConfig { pub user: String, #[serde(default)] pub working_dir: String, + #[serde(default)] + pub volumes: Option>, } /// A container summary returned by the list API. @@ -381,20 +383,15 @@ impl PodmanClient { timeout: Duration, max_bytes: usize, ) -> Result<(hyper::StatusCode, Bytes), PodmanApiError> { - use hyper::body::Body; - let response = self.execute_request(req, timeout).await?; let status = response.status(); let deadline = tokio::time::Instant::now() + timeout; let mut body = response.into_body(); let mut bytes = Vec::with_capacity(max_bytes.min(4096)); while bytes.len() < max_bytes { - let frame = tokio::time::timeout_at( - deadline, - std::future::poll_fn(|context| Pin::new(&mut body).poll_frame(context)), - ) - .await - .map_err(|_| PodmanApiError::Timeout(timeout))?; + let frame = tokio::time::timeout_at(deadline, body.frame()) + .await + .map_err(|_| PodmanApiError::Timeout(timeout))?; let Some(frame) = frame else { break; }; diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index e3bec033bf..2850ae5463 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -207,6 +207,24 @@ impl ResolvedPodmanImage { &config.sandbox_ssh_socket_path, ) .map_err(ComputeDriverError::Precondition)?; + if let Some(volumes) = image_config.and_then(|config| config.volumes.as_ref()) { + for volume in volumes.keys() { + driver_mounts::validate_container_mount_target(volume).map_err(|error| { + ComputeDriverError::Precondition(format!( + "invalid image-declared volume '{volume}': {error}" + )) + })?; + driver_mounts::validate_workspace_mount_target(volume, &workspace_root).map_err( + |_| { + ComputeDriverError::Precondition(format!( + "image-declared volume '{volume}' masks OCI WorkingDir '{workspace_root}' before workspace validation" + )) + }, + )?; + driver_mounts::validate_mount_control_path(volume, &config.sandbox_ssh_socket_path) + .map_err(ComputeDriverError::Precondition)?; + } + } Ok(Self { id: inspected.id.clone(), oci_user: image_config @@ -1470,6 +1488,7 @@ mod tests { config: Some(ImageConfig { user: user.to_string(), working_dir: working_dir.to_string(), + volumes: None, }), } } @@ -1677,6 +1696,21 @@ mod tests { assert!(err.to_string().contains("OpenShell control path")); } + #[test] + fn container_spec_rejects_image_volume_masking_workdir() { + let mut image = image_inspect("sha256:immutable", "app:staff", "/home/app/project"); + image + .config + .as_mut() + .unwrap() + .volumes + .get_or_insert_default() + .insert("/home".into(), serde_json::json!({})); + + let error = ResolvedPodmanImage::from_inspect(&image, &test_config()).unwrap_err(); + assert!(error.to_string().contains("masks OCI WorkingDir")); + } + #[test] fn workspace_probe_uses_pinned_image_without_workspace_or_network() { let mut sandbox = test_sandbox("test-id", "test-name"); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 414b50f3f8..bcb27ce2a6 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -2237,6 +2237,7 @@ mod tests { config: Some(crate::client::ImageConfig { user: "1234:1235".into(), working_dir: "/workspace".into(), + volumes: None, }), }, config, diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 1e23e09611..17e13d2cfd 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -455,12 +455,12 @@ and write and enter the workdir. OpenShell does not change that directory's ownership or mode. A one-shot validator drops to that identity and uses kernel effective-access checks, including POSIX ACL grants and LSM denials. It rejects workdirs that overlap the OCI runtime namespaces under `/proc`, `/sys`, or -`/dev`, and rejects overlap with actual OpenShell control paths. Docker checks -the original image filesystem in the final supervisor and rejects image -`VOLUME` declarations that would mask the workdir or one of its parents before -validation. Podman checks the same -pinned image ID in a temporary networkless probe without the workspace volume, -token, or TLS secrets. The probe retains only `SETUID` and `SETGID`, drops to +`/dev`, and rejects overlap with actual OpenShell control paths. Both drivers +reject image `VOLUME` declarations that would mask the workdir or one of its +parents before validation. Docker checks the original image filesystem in the +final supervisor. Podman checks the same pinned image ID in a temporary +networkless probe without the workspace volume, token, or TLS secrets. The +probe retains only `SETUID` and `SETGID`, drops to the identity selected by the effective global-or-sandbox policy, and uses the kernel to validate access. When neither policy exists, it discovers the image policy identity. The final supervisor must match the probe's normalized